RFR: 8336768: Allow captureCallState and critical linker options to be combined

[Jorn Vernee]

On Thu, 28 Nov 2024 12:06:13 GMT, Maurizio Cimadamore <mcimadamore at 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/LibFallback.java line 93:
> 
>> 91:      */
>> 92:     static void doDowncall(MemorySegment cif, MemorySegment target, MemorySegment retPtr, MemorySegment argPtrs,
>> 93:                            Object captureStateHeapBase, MemorySegment capturedState, int capturedStateMask,
> 
> I find it a bit weird that this method is receiving both the captured segment base and the segment itself. It almost seems as if the checks we do in `FallbackLinker` should be done here?

I see what you mean, and I agree it looks a bit strange. but we also pass the array of heap bases separately here (which is more or less a forced move due to how the libffi API works). So, I think the current code is more inline with that. The `LibFallback` class is only supposed to be a thin wrapper around the native methods in `fallbackLinker.c`. The actual logic is all in `FallbackLinker`.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/22327#discussion_r1862190619