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

Add SVM AOT support for unresolved invokeHandle/invokeDynamic dispatch #20272

Merged
merged 4 commits into from
Oct 11, 2024

Conversation

dsouzai
Copy link
Contributor

@dsouzai dsouzai commented Oct 1, 2024

Support unresolved dispatch of invokeHandle/invokeDynamic for SVM AOT
compilations. No additional relocation/validation records are needed as
the method type table entry / callsite entry addresses are not emitted
in the generated code. Additionally, the polymorphic signature of the
dummy linkToStatic resolved method is entirely determined by the callee
and thus the combination of the callee's SVM ID and Class Chain
validations are sufficient to validate the polymorphic signature.

Depends on eclipse-omr/omr#7475

The methodName parameter of TR_J9VMBase::getMethodFromClass is of type
const char *. However, the TR_J9SharedCacheVM override did not; this
resulted in the overridden method not being called during an AOT
compilation, resulting in a missing validation record.

This commit fixes this by making the signatures consistent.

Signed-off-by: Irwin D'Souza <[email protected]>
@dsouzai dsouzai added comp:jit depends:omr Pull request is dependent on a corresponding change in OMR comp:jit:aot labels Oct 1, 2024
@jdmpapin jdmpapin self-requested a review October 2, 2024 14:56
@jdmpapin jdmpapin self-assigned this Oct 2, 2024
Support unresolved dispatch of invokeHandle/invokeDynamic for SVM AOT
compilations. No additional relocation/validation records are needed as
the method type table entry / callsite entry addresses are not emitted
in the generated code. Additionally, the polymorphic signature of the
dummy linkToStatic resolved method is entirely determined by the callee
and thus the combination of the callee's SVM ID and Class Chain
validations are sufficient to validate the polymorphic signature.

Signed-off-by: Irwin D'Souza <[email protected]>
Signed-off-by: Irwin D'Souza <[email protected]>
@jdmpapin
Copy link
Contributor

jdmpapin commented Oct 9, 2024

Jenkins test sanity all jdk11,jdk17,jdk21 depends eclipse/omr#master

@jdmpapin
Copy link
Contributor

jdmpapin commented Oct 9, 2024

The AArch64 Linux JDK11 sanity.openjdk failure is #13756

@jdmpapin
Copy link
Contributor

jdmpapin commented Oct 9, 2024

z Linux JDK17 sanity.functional failed a couple of tests in cmdLineTester_criu_jitserverPostRestore_1

"Test SSL Failure Case with mismatched certificate" failed with #14706

"Check SSL Verbose Log for connection failure with mismatched certificate" also failed, but it doesn't really show any indication of what went wrong. Instead, all of the expected files and successful/failing outputs are just mysteriously missing:

Output from test:
 [OUT] start running script
 [OUT] vlog sslVlog2 does not exist
 [OUT] 
 [OUT] Outputting previous test output
 [OUT] 
 [OUT] Removed test output files
 [OUT] finished script
 [ERR] cat: testOutput: No such file or directory
 [ERR] cat: criuOutput: No such file or directory
 [ERR] grep: testOutput: No such file or directory
 [ERR] grep: criuOutput: No such file or directory
>> Success condition was not found: [Output match: CHECKPOINT RESTORE: Ready for restore]
>> Success condition was not found: [Output match: JITServer::StreamFailure: Failed to SSL_connect]
>> Success condition was not found: [Output match: Could not connect to a server]
>> Failure condition was not found: [Output match: Connected to a server]
>> Success condition was not found: [Output match: CAT VLOG FORCE PASS]

I couldn't find an open issue for this. I could swear I've already seen it somewhere, but I don't know where. Either way, I'm confident this is unrelated to the changes in this PR

@dsouzai
Copy link
Contributor Author

dsouzai commented Oct 10, 2024

"Check SSL Verbose Log for connection failure with mismatched certificate" also failed, but it doesn't really show any indication of what went wrong.

This is one of those tests that depends on the output of the previous test. I guess because the previous test (i.e., Test SSL Failure Case with mismatched certificate) failed, this one has no output to work with, so it too failed.

@jdmpapin
Copy link
Contributor

jdmpapin commented Oct 10, 2024

The AIX JDK17 sanity.openjdk failure is #19962

Xenkins test sanity.functional+aot all jdk11,jdk17

(Edited to specify which AIX failure. Changed "Jenkins" just to be sure that my edit won't relaunch the build)

@jdmpapin
Copy link
Contributor

Jenkins test sanity.functional+aot all jdk11,jdk17 depends eclipse/omr#master

@jdmpapin
Copy link
Contributor

Jenkins test sanity.functional+aot alinux jdk17 depends eclipse/omr#master

@jdmpapin
Copy link
Contributor

The AIX JDK11 sanity.functional failure is #17396

@jdmpapin
Copy link
Contributor

The AIX JDK17 sanity.functional failure is also #17396

@jdmpapin
Copy link
Contributor

All other tests have passed, and eclipse-omr/omr#7475 has promoted

@jdmpapin jdmpapin merged commit 80542c0 into eclipse-openj9:master Oct 11, 2024
62 of 66 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:jit:aot comp:jit depends:omr Pull request is dependent on a corresponding change in OMR
Projects
Development

Successfully merging this pull request may close these issues.

2 participants