RFR: 8299684: (bf) JNI direct buffer functions with large capacity behave unexpectedly [v2]

[Alan Bateman]

On Thu, 5 Jan 2023 23:42:11 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

>> Remove cast in `JNI::NewDirectByteBuffer`of `long` capacity to `int`, modify the constructor in question to accept a `long` capacity, and verify in the constructor that the  capacity does not overflow `int` range, throwing IAE If it does.
>
> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8299684: Enhance message in IAE caused by NewDirectByteBuffer passing up a capacity which overflows int range

I checked ancient history and the issue goes back to JDK 1.4 (when JSR-51 defined this API). It's surprising that it hasn't been reported/noticed before now.

Yes, this will need an update to the JNI spec to say that it throws if capacity is negative or greater than Integer.MAX_VALUE.  It will look a bit strange in the spec when the parameter is jlong but we can't change it. As regards what to throw then the current proposal to throw IAE seems okay as the function already throws it for negative values that are >= Integer.MIN_VALUE, e.g.


Exception in thread "main" java.lang.IllegalArgumentException: capacity < 0: (-1 < 0)
	at java.base/java.nio.Buffer.createCapacityException(Buffer.java:282)
	at java.base/java.nio.Buffer.<init>(Buffer.java:245)
	at java.base/java.nio.ByteBuffer.<init>(ByteBuffer.java:298)
	at java.base/java.nio.ByteBuffer.<init>(ByteBuffer.java:306)
	at java.base/java.nio.MappedByteBuffer.<init>(MappedByteBuffer.java:113)
	at java.base/java.nio.DirectByteBuffer.<init>(DirectByteBuffer.java:177)
	at Test.newDirectByteBuffer(Native Method)
	at Test.main(Test.java:7)


The compatibility impact of the change should be minimal, at least I can't imagine anyone depending on the current broken behavior.

For the change, clampCapacity only needs to be called once, not twice. The exception message is a bit strange. If cap is < Integer.MIN_VALUE then it would be better to have it consistent with the existing message above. Also "in upcall from JNI NewDirectByteBuffer" should be better if it didn't include "in upcall".

We will need to add a test for this and decide whether to place it. It seems there aren't any tests for these JNI functions in hotspot//jtreg/runtime/jni, maybe we start a new locations for tests in test/jdk/java/nio/jni.

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

PR: https://git.openjdk.org/jdk/pull/11873