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

[GR-60395] Allow combination of captureCallState and critical(true). #10301

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

graalvmbot
Copy link
Collaborator

@graalvmbot graalvmbot commented Dec 12, 2024

JDK-24+27 again introduced several to the FFM API:

  1. 8336768: Allow captureCallState and critical linker options to be combined openjdk/jdk#22327
  2. 8345120: A likely bug in StringSupport::chunkedStrlenShort openjdk/jdk#22451
  3. 8344989: Test java/foreign/TestLinker.java failed with zero: IllegalStateException: libffi call failed with status: FFI_BAD_TYPEDEF openjdk/jdk#22417

(2) and (3) only affect our imported tests which we need to update.
However, (1) is a significant change where we also need to adapt our internal implementation.
In (1), they allow the combination of Linker.Option.captureCallState(...) with Linker.Option.critical(true).
To give some background: When using captureCallState(...), the created downcall handle then expects an additional MemorySegment argument immediately following the target address.
Until now, it was only possible to provide a native memory segment to specify the capture state buffer.
Our adaptations then extracted the native address from this single parameter.
Now, it is possible to specify a heap memory segment which will be unpacked into an object+offset pair. Therefore, the downcall stub then receives two parameters that specify the capture state buffer's address.
So far, we did not support this.
(1) contains one more breaking change: it adds an argument to NativeEntryPoint.make where we have a substitution for.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Dec 12, 2024
@graalvmbot graalvmbot closed this Dec 13, 2024
@graalvmbot graalvmbot deleted the fa/GR-60395/captureStateMask_critical branch December 13, 2024 09:31
@graalvmbot graalvmbot merged commit f9078d7 into master Dec 13, 2024
13 checks passed
@@ -58,7 +58,8 @@ public static Target_jdk_internal_foreign_abi_NativeEntryPoint make(ABIDescripto
MethodType methodType,
boolean needsReturnBuffer,
int capturedStateMask,
boolean needsTransition) {
boolean needsTransition,
boolean usingAddressPairs) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fangerer is the lack of a @BasedOnJDKFile annotation on this method intentional?

Copy link
Member

@fangerer fangerer Dec 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, since the substitution class is annotated with @TargetClass, we track changes in the original class anyway.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good, thank you for the explanation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants