crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.47k stars 1.62k forks source link

Mixed union upcast is broken in interpreter on Alpine Linux #15041

Closed HertzDevil closed 1 month ago

HertzDevil commented 1 month ago
record Foo, x : Int32, y : Void*

x = Foo.new(1, Pointer(Void).new(0xdeadbeef))
x          # => Foo(@x=1, @y=Pointer(Void)@0xdeadbeef)
(x || nil) # => Foo(@x=1, @y=Pointer(Void)@0x1)
(x || 1)   # => Foo(@x=1, @y=Pointer(Void)@0x1)

This happens on any value that is larger than a pointer, e.g. {1, 2_i64}, StaticArray[1, 2, 3], and 123_i128 too.

Extracted from https://github.com/crystal-lang/crystal/issues/15034#issuecomment-2378009386

HertzDevil commented 1 month ago

This is a consequence of memcpy:

x = Slice.new(16, &.itself)
x.to_unsafe.copy_to(x.to_unsafe + 6, 10)
x # => Slice[0, 1, 2, 3, 4, 5, 0, 1, 2, 3, 4, 5, 0, 1, 2, 3]

The interpreter upcasts a value to a mixed union by placing it on top of the stack, and then copying the data portion to a higher position to reserve space for the type ID. Hence, when the size of the value exceeds that of the type ID, the copy_to here would occur between two overlapping ranges:

https://github.com/crystal-lang/crystal/blob/d58ede5bacba23fbefed9040066aacec44ca953d/src/compiler/crystal/interpreter/instructions.cr#L1310-L1313

This is undefined behavior in both ISO C and POSIX. Instead move_to must be used here (and most likely in a few other places too).