Skip to content

FFI failures on Windows ARM64 #52644

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.

Sign up for GitHub

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

Jump to bottom
Closed
rmacnak-google opened this issue Jun 8, 2023 · 5 comments
Closed

FFI failures on Windows ARM64 #52644

rmacnak-google opened this issue Jun 8, 2023 · 5 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi os-windows

Comments

@rmacnak-google
Copy link
Contributor

The tests

ffi/function_callbacks_varargs_generated_test/0 RuntimeError (expected Pass)
ffi/function_callbacks_varargs_generated_test/1 RuntimeError (expected Pass)
ffi/function_callbacks_varargs_generated_test/2 RuntimeError (expected Pass)
ffi/function_callbacks_varargs_generated_test/3 RuntimeError (expected Pass)
ffi/function_varargs_generated_leaf_test/0 RuntimeError (expected Pass)
ffi/function_varargs_generated_leaf_test/1 RuntimeError (expected Pass)
ffi/function_varargs_generated_leaf_test/2 RuntimeError (expected Pass)
ffi/function_varargs_generated_leaf_test/3 RuntimeError (expected Pass)
ffi/function_varargs_generated_native_leaf_test/0 RuntimeError (expected Pass)
ffi/function_varargs_generated_native_leaf_test/1 RuntimeError (expected Pass)
ffi/function_varargs_generated_native_leaf_test/2 RuntimeError (expected Pass)
ffi/function_varargs_generated_native_leaf_test/3 RuntimeError (expected Pass)
ffi/function_varargs_generated_native_test/0 RuntimeError (expected Pass)
ffi/function_varargs_generated_native_test/1 RuntimeError (expected Pass)
ffi/function_varargs_generated_native_test/2 RuntimeError (expected Pass)
ffi/function_varargs_generated_native_test/3 RuntimeError (expected Pass)
ffi/function_varargs_generated_test/0 RuntimeError (expected Pass)
ffi/function_varargs_generated_test/1 RuntimeError (expected Pass)
ffi/function_varargs_generated_test/2 RuntimeError (expected Pass)
ffi/function_varargs_generated_test/3 RuntimeError (expected Pass)

are failing on configurations

vm-win-release-arm64

@dcharkes
@rmacnak-google rmacnak-google added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. os-windows library-ffi labels Jun 8, 2023
@dcharkes
Copy link
Contributor

dcharkes commented Jun 9, 2023

Link to failures?

Relevant CLs: https://dart-review.googlesource.com/c/sdk/+/306918 https://dart-review.googlesource.com/c/sdk/+/303424

From just the test names, maybe windows treats varargs differently on arm64 than the other OSes do.

@rmacnak-google
Copy link
Contributor Author

rmacnak-google commented Jun 12, 2023 •
edited
Loading

log

They all seem related to floats.

@liamappelbe liamappelbe mentioned this issue Aug 27, 2023
Closed
copybara-service bot pushed a commit that referenced this issue Oct 12, 2023
@rmacnak-google
[vm, ffi] Windows ARM64 varargs.
785f4d6 
- Don't use float registers
- Even for the fixed arguments before the variable arguments
- These changes apply only to the arguments not the result

TEST=ci
Bug: #52644
Change-Id: Ifa600a929f9a54b44e78a6f15eafe1cee7141466
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/329803
Reviewed-by: Daco Harkes <[email protected]>
Commit-Queue: Ryan Macnak <[email protected]>
copybara-service bot pushed a commit that referenced this issue Oct 17, 2023
@rmacnak-google
[vm, ffi] Round up checking for registers for small structs.
c0a9368 
This fixes an edge case on all ARM64 ABIs, when there is only one argument register remaining and the next argument is a 9-16 byte struct.

TEST=ci
Bug: #52644
Change-Id: I40d962e6d1b3484dbfcf91f5d6baca0bfec76056
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330161
Reviewed-by: Daco Harkes <[email protected]>
Commit-Queue: Ryan Macnak <[email protected]>
@rmacnak-google
Copy link
Contributor Author

The remaining bit seems to be handling

M(r7 int64, S+0 int64) Struct(size: 12) // float,float,float

on a callback, but not a callout.

@dcharkes
Copy link
Contributor

Thanks for fixing the small structs Ryan!

Hunch: It might be that we're trying to rely on the upper bits in the stack being zero-ed out but they are not? (That would be something that we provide on calls, but then C code doesn't care. And then we rely on in callbacks but C code doesn't provide it.) Could be something different though.

@rmacnak-google
Copy link
Contributor Author

I think it's Clang that's wrong. MSVC does the splitting when calling and being called. Clang only does it when being called.

copybara-service bot pushed a commit that referenced this issue Nov 14, 2023
@rmacnak-google
[vm, ffi] Don't read out-of-bounds when marshalling structs by value.
c5c578e 
Switch the Windows ARM64 builds to use MSVC. Clang disagrees with itself about handling of small structs in variadic functions, allowing splitting between the last argument register and the stack as the callee but not as the caller.

TEST=ci
Cq-Include-Trybots: luci.dart.try:vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try,vm-ffi-android-release-arm-try,vm-ffi-android-release-arm64c-try,vm-ffi-qemu-linux-release-arm-try,vm-linux-release-arm64-try,vm-mac-debug-arm64-try,vm-mac-release-arm64-try,vm-win-debug-arm64-try,vm-win-release-arm64-try,vm-ffi-qemu-linux-release-riscv64-try,vm-linux-debug-ia32-try,vm-linux-release-ia32-try,vm-win-release-ia32-try,vm-linux-debug-x64-try,vm-linux-release-x64-try,vm-mac-debug-x64-try,vm-mac-release-x64-try,vm-win-debug-x64-try,vm-win-release-x64-try
Bug: #52644
Bug: #53829
Change-Id: I2fd6c40620a885479f11bb8528ca1e9df3948a2f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/331209
Commit-Queue: Ryan Macnak <[email protected]>
Reviewed-by: Daco Harkes <[email protected]>
Reviewed-by: Siva Annamalai <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi os-windows
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dcharkes @rmacnak-google