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

[David Holmes]

On Fri, 6 Jan 2023 23:56:39 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: Add test for out of range capacity passed to NewDirectBuffer

Changes requested by dholmes (Reviewer).

make/test/JtregNativeJdk.gmk line 81:

> 79: 
> 80:   BUILD_JDK_JTREG_LIBRARIES_LIBS_libTracePinnedThreads := jvm.lib
> 81:   BUILD_JDK_JTREG_LIBRARIES_LIBS_libNewDirectByteBuffer := -ljava -lc

This is not correct for Windows

make/test/JtregNativeJdk.gmk line 85:

> 83:   BUILD_JDK_JTREG_LIBRARIES_LIBS_libstringPlatformChars := -ljava
> 84:   BUILD_JDK_JTREG_LIBRARIES_LIBS_libDirectIO := -ljava
> 85:   BUILD_JDK_JTREG_LIBRARIES_LIBS_libNewDirectByteBuffer := -ljava -lc

Why do we need `-lc`?

src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template line 245:

> 243:     // Constrain the capacity to int range throwing IAE if not possible
> 244:     //
> 245:     private static int clampCapacity(long capacity) {

I think `checkCapacity` would be better - clamp suggests the value gets clamped rather than throwing an exception.

src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template line 247:

> 245:     private static int clampCapacity(long capacity) {
> 246:         try {
> 247:             return Math.toIntExact(capacity);

Why not the more direct and obvious:

if (capacity < 0) {
  throw ...
} else if (capacity > Integer.MAX_VALUE) {
  throw ...
}

src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template line 251:

> 249:             String msg = "JNI NewDirectByteBuffer passed illegal capacity: "
> 250:                 + capacity + (capacity < Integer.MIN_VALUE
> 251:                 ? " < Integer.MIN_VALUE" : " > Integer.MAX_VALUE");

Why the check against `MIN_VALUE`?

test/jdk/java/nio/jni/NewDirectByteBuffer.java line 49:

> 47:         1L,
> 48:         (long)Integer.MAX_VALUE - 1,
> 49:         (long)Integer.MAX_VALUE

Won't such large capacities trigger OOME?

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

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