[foreign-memaccess+abi] RFR: 8291873: Implement return value normalization in Java

[John R Rose]

On Wed, 14 Sep 2022 17:17:46 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

> This patch moves value normalization code from the downcall stub (where it was hard coded) to Java, and also adds it for upcall arguments where it was missing.
> 
> There is a slight change in behavior with this. The previous hard-coded conversion from a native value to Java `boolean` checked if the least significant byte was non-zero, i.e. `boolean result = value & 0xFF != 0`. While the new conversion only looks at the least significant bit, i.e. `boolean result = value & 0b1 != 0`. I think the new behavior is more correct. It means that the Linker will just do the "dumb" thing when mapping anything to `boolean`, and the native representation is really expected to be like a Java `boolean` i.e. only using the least significant bit. On the other hand I think it means that e.g. jextract will have to map the C `bool` as `byte` and then do a non-zero check to convert to `boolean`.

If this change is refactoring the logic that is intended to be a maximally compatible JNI replacement, and the encoded boolean appears as a return value from a JNI function, then we have to use the non-zero byte test (not the low-order bit test).

For other occurrences of unconditioned bytes, when they originate outside the JVM, we could in principle make other choices, but it seems safer to stick with the pre-existing JNI convention (non-zero byte test) rather than make a new one.

The pre-existing convention is arguably correct  from a C-language point of view, because a JNI boolean (`jboolean`, specifically) is an 8-bit type.  C tests that type as a boolean by performing a zero comparison, not a bit selection.  So the JVM does what C code would do *for that particular type*.  It's important to follow this model, because JNI wrapper code (written in C) will sometimes perform boolean tests on `jboolean` variables, using that non-zero byte test.

(For other types, C might test *more than 8 bits*, but JNI doesn't ever treat other types as booleans.  There is no `jboolean32` type, for example.  So the question doesn't come up with JNI of a more general test of other types as booleans.)

Meanwhile, the convention of testing the low-order bit (and clearing other bits) is enforced for the JVM's internal storage of booleans.  It's an internal matter within the JVM heap, fully disjoint from whatever contract JNI defines about `jboolean`.

It would be nice to use the bit-test other places, since it is efficient and immune to effects from word size.  But I don't think we have any occasion to do so, unless we are intentionally inventing another kind of boolean (not JNI's `jboolean`) to use as an interchange type between Panama and native code.

If we could prove that Panama's use of `JAVA_BOOLEAN` types is fully disjoint from JNI's use of `jboolean` then it would be a new, independent boolean type.   We could change the convention for testing the boolean value as originally suggested.  There might be some confusion as users converted their JNI code to Panama code.

In the end, I think it is safer to use the non-zero test for any value (of any size) that might have been in contact with C code "in the wild", since that C code might be throwing around non-zero values freely, and intending us to see them all as true values.  (The JVM's heap-management code is tightly controlled, so is not affected by this consideration.)

If Panama ever defines a direct implicit conversion from a multi-byte type (pointer, int, long) to boolean (JAVA_BOOLEAN), then I recommend aligning that conversion semantics with those of C, which is to test the whole value (not just its low byte) against zero (the all-zero-bits value).  The "take the low byte" part of JNI's treatment of booleans is just an artifact of `jboolean` being a single byte value, plus an assumption that any ABI-carried value of a single byte will be placed in the low byte of its register container.  (This is not actually true in all ABIs, IIRC some put bytes in the high part of the register.)

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

PR: https://git.openjdk.org/panama-foreign/pull/720