On Wed, 4 Mar 2026 17:48:32 GMT, Paul Hübner <[email protected]> wrote:
> Hi all, > > This change enhances the Java foreign functions and memory API to allow users > to inject thread-locals before a downcall. > > The lvalue `errno` is used for error handling in C. The C11 (draft) standard > gives the following use-case example: >> Thus, a program that uses errnofor error checking should set it to zero >> before a library function call, > then inspect it before a subsequent library function call. Of course, a > library function can save the > value of errno on entry and then set it to zero, as long as the original > value is restored if errno’s > value is still zero just before the return. > > In the present day, it is possible to retrieve `errno` by using the API > provided by `Linker.Option captureCallState`. This provides the Java-user a > memory segment for thread-local data, that is populated by the JVM. The > memory segment can be read using conventional FFM means, such as with a > `VarHandle`. > > However, the contents of this segment is not used to populate the thread > local data _before_ the downcall. As an example, in Java using the FFM API, > an end user calling `strtol` and checking for `ERANGE` would do the following > (in-line with the C standard footnote): set the captured call state buffer's > `errno` to zero, perform a downcall, and then read the captured call state > again to check if `errno` is still zero. While the memory segment sees > `errno=0`, this is not actually written to the thread before execution. Thus, > it is possible to observe stale or irrelevant error numbers. In this example, > one could observe `ENOENT` after the `strtol` call, which is incorrect > behaviour. > > This PR flushes the captured call state to the thread local variables before > the downcall, ensuring that procedures in accordance to the C standard are > possible. Since this was somewhat invasive, I ported it to non-Oracle > platforms as well. > > One thing worth noting is that the setting and retrieving of thread local > data is now coupled. Meaning, if a user wants to retrieve e.g. `errno` after > a downcall, it is their responsibility to ensure that `errno` is > appropriately set beforehand in order to make a meaningful comparison with > predetermined error ranges. > > Testing: tiers 1-4 on Linux (x64, AArch64), macOS (x64, AArch64) and Windows > (x64). I have built the JDK and run a [minimal but > representative](https://gist.github.com/Arraying/41db1b093dfa5cc4bc49f1213584d520) > test case on Oracle Linux 10 (x64) with Zero (native), s390x, powerpc64le > and riscv64 (all QEMU). I've smoke tested these platfo... Great work! I think the javadoc needs some changes, but the rest looks spot on. src/java.base/share/classes/java/lang/foreign/Linker.java line 819: > 817: * state immediately before or after calling a foreign > function associated > 818: * with a downcall method handle, before it can be > overwritten by the Java > 819: * runtime, or read through conventional means} I don't think the word 'load' is right here. Maybe 'initialize'. Also, since we can't save before calling a foreign function, I suggest splitting the sentence differently: Suggestion: * {@return a linker option used to initialize portions of the execution * state immediately before, and save portions of the execution * state immediately after calling a foreign function associated * with a downcall method handle, before it can be overwritten by the Java * runtime, or read through conventional means} src/java.base/share/classes/java/lang/foreign/Linker.java line 823: > 821: * Execution state is captured by a downcall method handle on > invocation, by > 822: * writing it to a native segment provided by the user to the > downcall method > 823: * handle. For this purpose, a downcall method handle linked > with this option I think this part also needs some changes. I don't like that this is talking about 'on invocation', since there is not a single point where we interact with the capture state segment anymore. I suggest changing this to the following: Suggestion: * Execution state is initialized from, or saved to a native segment provided by * the user to the downcall method handle. For this purpose, a downcall * method handle linked with this option This also ties directly back to the terminology in the previous paragraph (instead of using the word 'captured'). (and then reflow the rest of the lines) src/java.base/share/classes/java/lang/foreign/Linker.java line 827: > 825: * the target address, and optional {@link SegmentAllocator} > parameters. This > 826: * parameter, the <em>capture state segment</em>, represents the > native segment > 827: * into which the captured state is written. Suggestion: * parameter, the <em>capture state segment</em>, represents the native segment * from which the capture state is initialized, and into which the captured state is saved. src/java.base/share/classes/java/lang/foreign/Linker.java line 832: > 830: * handle is invoked, the contents of the <em>capture state > segment</em> is > 831: * copied into the foreign function's execution state. > 832: * <p> With the changes I suggested for the previous section, I don't think this section is needed. src/java.base/share/classes/java/lang/foreign/Linker.java line 838: > 836: * <p> > 837: * Captured state can be set or retrieved from the capture state > segment by > 838: * constructing var handles from the {@linkplain > #captureStateLayout capture state layout}. I suggest "stored in" as the opposite of "retrieved from". The current "set ... from the capture state segment" doesn't quite work grammatically. Suggestion: * Captured state can be stored in, or retrieved from the capture state segment by * constructing var handles from the {@linkplain #captureStateLayout capture state layout}. test/jdk/java/foreign/capturecallstate/TestCaptureCallState.java line 106: > 104: assertEquals(savedErrno, testValue); > 105: } else { > 106: assertNotEquals(savedErrno, testValue); Can't we test for `prevValue` here? Seems more precise Suggestion: assertEquals(savedErrno, prevValue); ------------- PR Review: https://git.openjdk.org/jdk/pull/30059#pullrequestreview-3896151496 PR Review Comment: https://git.openjdk.org/jdk/pull/30059#discussion_r2889746409 PR Review Comment: https://git.openjdk.org/jdk/pull/30059#discussion_r2889766220 PR Review Comment: https://git.openjdk.org/jdk/pull/30059#discussion_r2889770907 PR Review Comment: https://git.openjdk.org/jdk/pull/30059#discussion_r2889775103 PR Review Comment: https://git.openjdk.org/jdk/pull/30059#discussion_r2889713190 PR Review Comment: https://git.openjdk.org/jdk/pull/30059#discussion_r2889801144
