On Fri, 22 Nov 2024 14:31:23 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
> Allow `captureCallState` and `critical(true)` linker options to be combined.
> This allows passing a Java array to capture call state.
>
> One caveat is that the linker expects the memory to be aligned, which means
> that at least an `int[]` has to be used (i.e. `byte[]` will no work).
>
> This patch contains two implementations: one for the linkers that use
> `CallingSequenceBuilder`. That one is quite straight-forward, as we can just
> mimic what we already do for other memory segment arguments, but also for the
> capture state segment. i.e. split it into base and offset, and pass that down
> to our downcall stub. The stub will then add the offset and oop together, and
> pass use the resulting address to write to.
>
> The other implementation is for the fallback linker. This handles the capture
> state a little differently, but essentially currently just passes the native
> address to the back end for the native code to write the captured state into.
> I've just added another heap base parameter for that capture state segment to
> the back end, which is then turned into a native address using JNI's
> `GetPrimitiveArrayCritical`, similarly to what we do for other heap segments.
>
> Testing: `jdk_foreign` test suite.
src/java.base/share/classes/jdk/internal/foreign/abi/fallback/FallbackLinker.java
line 168:
> 166: capturedState =
> SharedUtils.checkCaptureSegment((MemorySegment) args[argStart++]);
> 167: if (!invData.allowsHeapAccess) {
> 168: SharedUtils.checkNative(capturedState);
I noticed that this check was missing in the fallback linker, and we were
actually crashing when a heap segment was passed as the capture state segment.
I've added a new test case that checks that this works as well.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22327#discussion_r1860473312
