Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support LLVM 18.1 #14277

Merged
merged 4 commits into from
Feb 4, 2024
Merged

Conversation

HertzDevil
Copy link
Contributor

Tested using 18.1.0-rc1 which had no issues.

Note that LLVM's release process has changed; x.0.0 is now reserved for development branches, and the first stable release for each major version is now x.1.0, similar to GCC.

This PR also moves Windows CI to 18.1.0-rc1 directly. Linux CI will have to wait as we still depend on their volunteers' binary packages (they now have a nice note telling people to wait).

@HertzDevil HertzDevil added kind:feature platform:windows Windows support based on the MSVC toolchain / Win32 API topic:stdlib:llvm topic:infrastructure/ci labels Jan 30, 2024
@HertzDevil HertzDevil marked this pull request as draft January 30, 2024 14:49
@HertzDevil
Copy link
Contributor Author

HertzDevil commented Jan 30, 2024

Looks like this had issues after all and something about Socket::IPAddress is failing:

require "socket"

x = Socket::IPAddress.new("::1", 1)
x == x # Invalid memory access (signal 11) at address 0x0

This reduces to:

struct In6Addr
  @__in6_u = 0_i128

  def ==(other : self)
    @__in6_u == other.@__in6_u
  end

  def ==(other)
    false
  end
end

struct Int32
  def ==(other : In6Addr)
    false
  end
end

addr = In6Addr.new.as(Int32 | In6Addr)
addr == addr

This part of LLVM 18 looks suspicious:

The i128 type now matches GCC and clang's __int128 type. This mainly benefits external projects such as Rust which aim to be binary compatible with C, but also fixes code generation where LLVM already assumed that the type matched and called into libgcc helper functions.

Note that alignof(In6Addr) == 16 but alignof(Int32 | In6Addr) == 8. The same is also reproducible using Tuple(Int128) instead of In6Addr. So maybe this is related to #13609?

@HertzDevil
Copy link
Contributor Author

HertzDevil commented Jan 30, 2024

Digging a bit further, the above reduced snippet crashes at something like this, indeed an unaligned 128-bit read: (the SIGSEGV is indistinguishable from a null pointer exception since it appears x86-64 doesn't hand out the actual failing address to the signal handler)

Process 69176 stopped
* thread #1, name = 'test', stop reason = signal SIGSEGV: invalid address (fault address: 0x0)
    frame #0: 0x000055555555542c test`==(self=(__in6_u = 2596148429080290042522275961362632), other=(__in6_u = 140737488345184)) at test.cr:64:3
   61   struct In6Addr
   62     @__in6_u = 0_i128
   63  
-> 64     def ==(other : self)
   65       @__in6_u == other.@__in6_u
   66     end
   67  
(lldb) di -p
test`:
->  0x55555555542c <+60>: movaps (%rcx), %xmm0
    0x55555555542f <+63>: movaps (%rax), %xmm1
    0x555555555432 <+66>: pcmpeqb %xmm1, %xmm0
    0x555555555436 <+70>: pmovmskb %xmm0, %eax
(lldb) register read rcx xmm0
     rcx = 0x00007fffffffd8c8
    xmm0 = {0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}
(lldb) v
(In6Addr) self = (__in6_u = 2596148429080290042522275961362632)
(In6Addr) other = (__in6_u = 140737488345184)

(Int32 | In6Addr) being underaligned means other isn't even passed correctly, as 128-bit integers do not fit into ordinary registers. For comparison, the assembly for In6Addr#==(In6Addr) looks like this in LLVM 18 (this uses --no-debug rather than --debug, so it is slightly different):

"*In6Addr#==<In6Addr>:Bool":
	.cfi_startproc
	movq	%rdi, -48(%rsp)
	movq	%rdx, -40(%rsp)
	movq	%rsi, -32(%rsp)
	movq	-48(%rsp), %rax
	movq	-40(%rsp), %rcx
	movq	-32(%rsp), %rdx
	movq	%rdx, -24(%rsp)
	movq	%rcx, -16(%rsp)
	movaps	(%rax), %xmm0 ; crashes here
	movaps	-24(%rsp), %xmm1
	pcmpeqb	%xmm1, %xmm0
	pmovmskb	%xmm0, %eax
	subl	$65535, %eax
	sete	%al
	retq

Same method in LLVM 15:

"*In6Addr#==<In6Addr>:Bool":
	.cfi_startproc
	movq	%rdi, -40(%rsp)
	movq	%rdx, -32(%rsp)
	movq	%rsi, -24(%rsp)
	movq	-40(%rsp), %rax
	movq	-32(%rsp), %rcx
	movq	-24(%rsp), %rdx
	movq	%rdx, -16(%rsp)
	movq	%rcx, -8(%rsp)
	movups	(%rax), %xmm0 ; ok, movups can read from underaligned addresses
	movups	-16(%rsp), %xmm1
	pcmpeqb	%xmm1, %xmm0
	pmovmskb	%xmm0, %eax
	subl	$65535, %eax
	sete	%al
	retq

@HertzDevil HertzDevil marked this pull request as ready for review February 2, 2024 22:09
.github/workflows/win.yml Show resolved Hide resolved
@straight-shoota straight-shoota added this to the 1.12.0 milestone Feb 2, 2024
@straight-shoota straight-shoota merged commit c67883c into crystal-lang:master Feb 4, 2024
57 checks passed
@Blacksmoke16
Copy link
Member

Blacksmoke16 commented Feb 5, 2024

I haven't really looked into it too much yet, but Athena's nightly windows CI job fails now, while the latest release passes. I'll see about reproducing it and will go from there.

EDIT: Created #14285

@HertzDevil HertzDevil deleted the feature/llvm-18.1 branch February 8, 2024 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feature platform:windows Windows support based on the MSVC toolchain / Win32 API topic:infrastructure/ci topic:stdlib:llvm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants