On Thu, 28 Nov 2024 12:06:13 GMT, Maurizio Cimadamore <mcimadam...@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
