Conversation
1771642 to
db6be7f
Compare
| } | ||
| } | ||
|
|
||
| if (t == NULL) { |
There was a problem hiding this comment.
Is it necessary to fallback to int_types? Otherwise using a pointer instead of int_types and float_types would be enough.
There was a problem hiding this comment.
Yes I think so.
Example:
union { uint32_t flags; float value; }
all_float is false. Alignment is 4. ffi_type_pointer has alignment 8 on 64-bit, so no match. It needs ffi_type_sint32 from int_types.
db6be7f to
158d32e
Compare
|
Ok, I have updated the PR and added tests. The tests pass on x86 and arm with the fix. Without it, the tests fail on arm. |
158d32e to
28787a8
Compare
To verify the impact of the change.
|
I see on failure now: Doesn't seem related to this change. |
Updates FFI so it picesk the correct ffi_type for unions composed of floating point values. This is needed for ARM64 (aarch64), where the calling convention for passing and returning structs by value depends on whether the type is a Homogeneous Floating-point Aggregate (HFA). HFAs are passed in floating-point registers (d0-d3), while non-HFAs use integer registers or memory. The __union! method replaces the actual union field types with a repeated "filler" type that matches the union's alignment. It searches an array of candidate types and picks the first one whose alignment matches. Since ffi_type_sint64 (alignment 8) appears before ffi_type_double (alignment 8) in the array, unions of doubles get described to libffi as [sint64, sint64, sint64, sint64] instead of [double, double, double, double]. On x86_64 this is harmless because both type classes use the same register/stack passing mechanism. On ARM64 it causes a silent ABI mismatch: libffi passes the union in integer registers while the C function expects it in floating-point registers, resulting in garbage values (typically ~2.48e-314, i.e. denormalized near-zero doubles read from uninitialized FP registers). The fix inspects the actual field types before choosing the filler. If all fields are composed entirely of floating-point types (including nested structs/arrays of floats/doubles), a floating-point filler type is selected. Otherwise, the existing integer filler behavior is preserved.
... to int, mixed and float types.
28787a8 to
7991602
Compare
|
Thank you @cfis for adding the tests! I extended the old tests a bit more and found a failing test when writing to this union: union union_float_test {
float f;
double d;
};I also rebased the PR due to changes on the master branch and simplified the union type finding. But the tests fail with and without the simplification. Maybe I'm just too tired for today. But they succeed with using ints as |
|
Good catch, you are right. The ARM logic requires the same floating point type, not mixed floating point types. I pushed a new commit that fixes that and also fixes the test failures on the additional tests you added. |
…ruby ... to fix test failure on Truffleruby.
|
I pushed an update for the msvc failures at https://github.com/ffi/ffi/actions/runs/22807005157/job/66157649724?pr=1178. Not sure about JRuby, is that a different code path someplace? |
Fixes #1177.
On ARM64 (aarch64), the calling convention for passing and returning structs by value depends on whether the type is a Homogeneous Floating-point Aggregate (HFA). HFAs are passed in floating-point registers (d0-d3), while non-HFAs use integer registers or memory.
The __union! method replaces the actual union field types with a repeated "filler" type that matches the union's alignment. It searches an array of candidate types and picks the first one whose alignment matches. Since ffi_type_sint64 (alignment 8) appears before ffi_type_double (alignment 8) in the array, unions of doubles get described to libffi as [sint64, sint64, sint64, sint64] instead of [double, double, double, double].
On x86_64 this is harmless because both type classes use the same register/stack passing mechanism. On ARM64 it causes a silent ABI mismatch: libffi passes the union in integer registers while the C function expects it in floating-point registers, resulting in garbage values (typically ~2.48e-314, i.e. denormalized near-zero doubles read from uninitialized FP registers).
The fix inspects the actual field types before choosing the filler. If all fields are composed entirely of floating-point types (including nested structs/arrays of floats/doubles), a floating-point filler type is selected. Otherwise, the existing integer filler behavior is preserved.
This bug is triggered by any FFI::Union containing only float/double members that is passed or returned by_value on ARM64. For example, the PROJ library's PJ_COORD union (4 doubles) used via ruby-ffi (see ticket for more details)
I verified the fix works on MacBook Pro.