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

AArch64: Fix getArgPointer() for FFI upcall #16332

Merged
merged 1 commit into from Nov 18, 2022

Conversation

knn-k
Copy link
Contributor

@knn-k knn-k commented Nov 16, 2022

This commit fixes getArgPointer() for AArch64 for FFI upcall.

The function needs to maintain two offsets in the stack: one for the arguments stored by the upcall thunk, another one for the arguments stored in the stack by the caller.

Fixes: #16237

Signed-off-by: KONNO Kazuhiro konno@jp.ibm.com

@knn-k knn-k added comp:vm project:panama Used to track Project Panama related work arch:aarch64 labels Nov 16, 2022
@knn-k
Copy link
Contributor Author

knn-k commented Nov 16, 2022

Jenkins test sanity alinux64,amac jdk19

@knn-k
Copy link
Contributor Author

knn-k commented Nov 16, 2022

Jenkins test sanity alinux64,amac jdk19

@knn-k knn-k marked this pull request as ready for review November 16, 2022 10:54
@knn-k
Copy link
Contributor Author

knn-k commented Nov 16, 2022

This PR fixes the failures with TestUpcallHighArity.java on AArch64 in #16237.

@knn-k
Copy link
Contributor Author

knn-k commented Nov 17, 2022

FYI. @ChengJin01

This commit fixes getArgPointer() for AArch64 for FFI upcall.

The function needs to maintain two offsets in the stack: one for the
arguments stored by the upcall thunk, another one for the arguments
stored in the stack by the caller.

Signed-off-by: KONNO Kazuhiro <konno@jp.ibm.com>
@knn-k
Copy link
Contributor Author

knn-k commented Nov 17, 2022

Jenkins test sanity alinux64,amac jdk19

@ChengJin01
Copy link
Contributor

FYI: @zl-wang, @0xdaryl

@knn-k
Copy link
Contributor Author

knn-k commented Nov 18, 2022

I ran the following tests with this fix on Linux and macOS manually.

TestUpcallAsync.java
TestUpcallHighArity.java
TestUpcallScope.java
TestUpcallStack.java
Jep424Tests_testLinkerFfi_UpCall (in functional test)

TestUpcallStack.java failed only on macOS, and #16336 tracks it. All other tests were successful.

@zl-wang
Copy link
Contributor

zl-wang commented Nov 18, 2022

maintaining two stack-pointer(s) sounds more complicated than necessary. it can be done relatively simply: saving-frame is of fixed size because you can know the max stack saving area needed for all possibly in-register args. at the same time, original passed-in-stack arg pointer can be calculated according to that fixed size and arg's offset together. z (and to a lesser degree, p where the ABI dictates the stack saving area either none or all) implemented this way.

@zl-wang
Copy link
Contributor

zl-wang commented Nov 18, 2022

your fix did implement that way. implementation details are slightly different. LGTM ... approved!

@zl-wang zl-wang merged commit 5e4baa7 into eclipse-openj9:master Nov 18, 2022
@knn-k knn-k deleted the aarch64upcallFix2 branch November 18, 2022 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch:aarch64 comp:vm project:panama Used to track Project Panama related work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Jtreg/FFI]Crash in getArgPointer() in upcall on Aarch64
3 participants