On Wed, 27 Nov 2024 19:21:30 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/NativeEntryPoint.java > line 91: > >> 89: int checkIdx = 1; >> 90: if ((needsReturnBuffer && methodType.parameterType(checkIdx++) >> != long.class) >> 91: || (savedValueMask != 0 && > > Maybe this routine is getting too complex and it would be better to split the > checks (and add some comments) ? E.g. we need to check that if there's a > return buffer, a certain low-level argument is `long` and, if there's need to > capture state, we either have `long`, or an `Object,long` pair. Perhaps, even throwing different assertion error might help ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22327#discussion_r1861153722
