Windows Nightly Regression about crystal HOT 3 CLOSED

Reduced:

struct Time2
  def initialize(@seconds : Int64)
    @nanoseconds = uninitialized UInt32[3]
  end

  def <(other : Time2) : Bool
    @seconds < other.@seconds
  end
end

class Constraints::Range
  def initialize(@min : Int128 | Time2 | Nil)
  end
end

def validate(value : Time2, constraint) : Nil
  min = constraint.@min

  if min.is_a?(Time2?)
    if min
      if value < min
        LibC.printf("foo\n")
      end
    end
  end
end

value = Time2.new(123)
constraint = Constraints::Range.new(Time2.new(45))
validate(value, constraint)

This generates the following fragment for the #< call:

then2:                                            ; preds = %then
  %25 = getelementptr inbounds %"(Int128 | Time2 | Nil)", ptr %min, i32 0, i32 0
  %26 = load i32, ptr %25, align 4
  %27 = getelementptr inbounds %"(Int128 | Time2 | Nil)", ptr %min, i32 0, i32 1
  %28 = load %Time2, ptr %27, align 8 ; underaligned load occurs here
  %29 = call i1 @"*Time2#<<Time2>:Bool"(ptr %value1, %Time2 %28)
  br i1 %29, label %then4, label %else5

It looks like all loads that go through union_type_and_value_pointer must be aligned.

from crystal.

Changing the alignments in the emitted LLVM IR manually did not help. Instead it looks like the problem is with how the new memcpy in #14279 always picks the size of the larger union, regardless of whether an upcast or downcast is performed:

if min.is_a?(Time2?)
  if min # this downcast
    # ...

%"(Time2 | Nil)" = type { i32, [3 x i64] }

%11 = getelementptr inbounds %"(Int128 | Time2 | Nil)", ptr %min, i32 0, i32 0
%12 = load i32, ptr %11, align 4
%14 = getelementptr inbounds %"(Time2 | Nil)", ptr %0, i32 0, i32 0
store i32 %12, ptr %14, align 4
%15 = getelementptr inbounds %"(Time2 | Nil)", ptr %0, i32 0, i32 1
%16 = getelementptr inbounds %"(Int128 | Time2 | Nil)", ptr %min, i32 0, i32 1
call void @llvm.memcpy.p0.p0.i64(ptr align 8 %15, ptr align 16 %16, i64 32, i1 false) ; this 32

So we are corrupting the stack by copying 32 bytes to the data field of a Time2 | Nil, which only measures 24 bytes. Using the less size among the two unions seems to fix the issue.

We probably don't need to touch the load / store alignments for now. Maybe LLVM cannot prove that some align 16 values are always satisfied...?

from crystal.

The problem is that the getelementptr and the load instructions are not necessarily emitted in the same place. For example, the following part of the compiler handles the snippet above:

%27 is emitted by the request_value call on line 85 (which calls CodeGenVisitor#visit(Var) etc.), but %28 is emitted by the load on line 98, so we have to somehow carry this alignment information across those lines, otherwise %28 will pick up the alignment from alignof(Time2) automatically. Yet only the relevant LLVM instructions have alignment, not LLVM types, for reasons similar to the recent transition to opaque pointers.

It was mostly coincidence that underaligned operations for smaller sizes were working before on x86-64. With actual 16-byte-aligned types this is not the case anymore.

from crystal.