Skip to content
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.

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

[CP] [vm/ffi] Fix variadic arguments on MacOS Arm64 #55943

Closed
dcharkes opened this issue Jun 5, 2024 · 2 comments
Closed

[CP] [vm/ffi] Fix variadic arguments on MacOS Arm64 #55943

dcharkes opened this issue Jun 5, 2024 · 2 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve

Comments

@dcharkes
Copy link
Contributor

dcharkes commented Jun 5, 2024

Commit(s) to merge

Target

stable

Prepared changelist for beta/stable

https://dart-review.googlesource.com/c/sdk/+/369822

Issue Description

Variadic arguments on Mac Arm64 weren't working correctly due to misalingment.

What is the fix

Distinguishing stack alignment of FFI call arguments between varargs and non-varargs.

Why cherry-pick

To prevent JNIgen having to target the 3.5 master branch for Flutter/Dart.

Risk

Small: It's a couple of lines of code CL (with many lines of code tests).

Issue link(s)

#55471

Extra Info

No response

@dcharkes dcharkes added the cherry-pick-review Issue that need cherry pick triage to approve label Jun 5, 2024
@mit-mit
Copy link
Member

mit-mit commented Jun 11, 2024

sgtm

copybara-service bot pushed a commit that referenced this issue Jun 11, 2024
Non-variadic arguments on the stack on MacOS Arm64 are aligned to
the value size (which can be smaller than word size). However, for
varargs, the arguments on the stack seem to be aligned to the word
size.

This CL introduces an alignment strategy constant for primitives on
the stack in varargs and uses it in the native calling convention
calculation.

TEST=runtime/vm/compiler/ffi/native_calling_convention_test.cc with
runtime/vm/compiler/ffi/unit_tests/variadic_less_than_word/arm64_macos.expect

TEST=tests/ffi/function_varargs_generated_native_leaf_test.dart

Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/362762
Cherry-pick-request: #55943
Change-Id: I621b89d8554a5507c25d9fd24ca64cd954f37388
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/369822
Reviewed-by: Martin Kustermann <kustermann@google.com>
@athomas athomas added the cherry-pick-approved Label for approved cherrypick request label Jun 12, 2024
@devoncarew devoncarew added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Jun 12, 2024
@athomas
Copy link
Member

athomas commented Jun 13, 2024

Released in 3.4.4.

@athomas athomas closed this as completed Jun 13, 2024
christopherfujino added a commit to flutter/engine that referenced this issue Jul 12, 2024
# Flutter stable 3.22.3 Engine

## Scheduled Cherrypicks

- Roll dart revision: dart-lang/sdk@604651494
### Dart
- dart-lang/sdk#55979
- dart-lang/sdk#55943
### Engine
- flutter/flutter#149700
- flutter/flutter#149701
- flutter/flutter#149702
- flutter/flutter#149704
- flutter/flutter#149745
- flutter/flutter#149771
- #53183

---------

Co-authored-by: Martin Kustermann <kustermann@google.com>
Co-authored-by: Jackson Gardner <eyebrowsoffire@gmail.com>
Co-authored-by: Christopher Fujino <christopherfujino@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve
Projects
None yet
Development

No branches or pull requests

6 participants