Open straight-shoota opened 1 month ago
This method is suspicious; does it calculate the alignment on a 8 bytes boundary?
Yeah, looks like it. I've stumbled over that as well. Although for size 16 it returns 16, so it probably doesn't matter for this issue...
Ah, no that indeed is a problem. align
is solely based on size
, not on the current stack pointer.
It needs to take the current stack pointer into account. If the address is not aligned to 16 bytes, we won't be able to get a 16-byte alignment.
The interpreter stack does not seem to anticipate an alignment larger than 8 bytes.
IIRC the interpreter memcpy
s the stack contents rather than dereferencing the stack pointer directly, so if there is an aligned 128-bit load/store at a misaligned address, it is probably somewhere else
stack_push
and stack_pop
use pointer dereferencing.
Or do you mean something else?
This issue has been mentioned on Crystal Forum. There might be relevant details there:
https://github.com/crystal-lang/crystal/pull/14843 seems to fix the memory access error. However, it does so by simply avoiding misaligned stack access errors. However, the misalignment still exists.
So it does feels a bit like a workaround. It does not fix the underlying issue that 128-bit integers are not properly aligned.
# interpreter with patch #14843 and LLVM 18
pad = 1_i8
x = 1_i128
pointerof(x).address % alignof(typeof(x)) # => 8
Note: This issue only depends on the LLVM version of the interpreter itself, not of the parent compiler. The interpreter's LLVM version determines the alignment value.
If the alignmed was correct, we would not need #14843 to make the interpreter work.
It's probably still a good idea to merge #14843, though. It removes a dependency on the alignment policy of the parent compiler. This seems like a good idea anyway.
An interpreter built with a LLVM 18 compiler crashes with invalid memory access when using a 128-bit integer type.
The reproduction requires a compiler with LLVM 18 which builds a compiler with interpreter. Using a 128-bit integer value in that interpreter causes an invalid memory access:
The bug does not reproduce when building the interpreter in debug mode, nor in non-release mode.
This is likely related to LLVM changing the alignment of 128-bit integer types to 128 bits in LLVM 18: https://reviews.llvm.org/D86310
alignof(UInt128) == 8
.alignof(UInt128) == 16
.The interpreter may expect the old alignment somewhere.
So far it has only materialized in the interpreter and it's quite likely that this is an interpreter-specific bug. But at this point we cannot certainly exclude any effect on other applications.
This issue appeared while trying to update the previous Crystal release to 1.13.1: https://github.com/crystal-lang/crystal/pull/14810