8299684: (bf) JNI direct buffer functions with large capacity behave unexpectedly

Reviewed-by: dholmes, alanb

make/test/JtregNativeJdk.gmk

@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2015, 2022, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # This code is free software; you can redistribute it and/or modify it
@@ -78,9 +78,11 @@ ifeq ($(call isTargetOs, windows), true)
   BUILD_JDK_JTREG_LIBRARIES_CFLAGS_libAsyncInvokers := -I$(TEST_LIB_NATIVE_SRC)
 
   BUILD_JDK_JTREG_LIBRARIES_LIBS_libTracePinnedThreads := jvm.lib
+  BUILD_JDK_JTREG_LIBRARIES_LIBS_libNewDirectByteBuffer := $(WIN_LIB_JAVA)
 else
   BUILD_JDK_JTREG_LIBRARIES_LIBS_libstringPlatformChars := -ljava
   BUILD_JDK_JTREG_LIBRARIES_LIBS_libDirectIO := -ljava
+  BUILD_JDK_JTREG_LIBRARIES_LIBS_libNewDirectByteBuffer := -ljava
   BUILD_JDK_JTREG_LIBRARIES_LDFLAGS_libNativeThread := -pthread
 
   # java.lang.foreign tests

src/hotspot/share/prims/jni.cpp

@@ -2959,7 +2959,7 @@ static bool initializeDirectBufferSupport(JNIEnv* env, JavaThread* thread) {
     }
 
     // Get needed field and method IDs
-    directByteBufferConstructor = env->GetMethodID(directByteBufferClass, "<init>", "(JI)V");
+    directByteBufferConstructor = env->GetMethodID(directByteBufferClass, "<init>", "(JJ)V");
     if (env->ExceptionCheck()) {
       env->ExceptionClear();
       directBufferSupportInitializeFailed = 1;
@@ -3011,10 +3011,7 @@ extern "C" jobject JNICALL jni_NewDirectByteBuffer(JNIEnv *env, void* address, j
 
   // Being paranoid about accidental sign extension on address
   jlong addr = (jlong) ((uintptr_t) address);
-  // NOTE that package-private DirectByteBuffer constructor currently
-  // takes int capacity
-  jint  cap  = (jint)  capacity;
-  jobject ret = env->NewObject(directByteBufferClass, directByteBufferConstructor, addr, cap);
+  jobject ret = env->NewObject(directByteBufferClass, directByteBufferConstructor, addr, capacity);
   HOTSPOT_JNI_NEWDIRECTBYTEBUFFER_RETURN(ret);
   return ret;
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -178,14 +178,31 @@ class Direct$Type$Buffer$RW$$BO$
     }
 
     // Invoked only by JNI: NewDirectByteBuffer(void*, long)
+    // The long-valued capacity is restricted to int range.
     //
-    private Direct$Type$Buffer(long addr, int cap) {
-        super(-1, 0, cap, cap, null);
+    private Direct$Type$Buffer(long addr, long cap) {
+        super(-1, 0, checkCapacity(cap), (int)cap, null);
         address = addr;
         cleaner = null;
         att = null;
     }
 
+    // Throw an IllegalArgumentException if the capacity is not in
+    // the range [0, Integer.MAX_VALUE]
+    //
+    private static int checkCapacity(long capacity) {
+        if (capacity < 0) {
+            throw new IllegalArgumentException
+                ("JNI NewDirectByteBuffer passed capacity < 0: ("
+                + capacity + ")");
+        } else if (capacity > Integer.MAX_VALUE) {
+            throw new IllegalArgumentException
+                ("JNI NewDirectByteBuffer passed capacity > Integer.MAX_VALUE: ("
+                + capacity + ")");
+        }
+        return (int)capacity;
+    }
+
 #end[rw]
 
     // For memory-mapped buffers -- invoked by FileChannelImpl via reflection

test/jdk/java/nio/jni/NewDirectByteBuffer.java

@@ -0,0 +1,143 @@
+/*
+ * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+import java.nio.ByteBuffer;
+import java.util.concurrent.atomic.AtomicReference;
+
+import jdk.internal.misc.Unsafe;
+
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/*
+ * @test
+ * @bug 8299684
+ * @summary Unit test for the JNI function NewDirectByteBuffer
+ * @requires (sun.arch.data.model == "64" & os.maxMemory >= 8g)
+ * @modules java.base/jdk.internal.misc
+ * @run junit/othervm/native NewDirectByteBuffer
+ */
+public class NewDirectByteBuffer {
+    private static final Unsafe UNSAFE;
+    static {
+        System.loadLibrary("NewDirectByteBuffer");
+        UNSAFE = Unsafe.getUnsafe();
+    }
+
+    private static final void checkBuffer(ByteBuffer buf, long capacity) {
+        // Verify that the JNI function returns the correct capacity
+        assertEquals(capacity, getDirectByteBufferCapacity(buf),
+            "GetDirectBufferCapacity returned unexpected value");
+
+        // Verify that the initial state values are correct
+        assertTrue(buf.isDirect(), "Buffer is not direct");
+        assertFalse(buf.hasArray(), "Buffer has an array");
+        if (capacity > 0) {
+            assertTrue(buf.hasRemaining(), "Buffer has no remaining values");
+        }
+        assertFalse(buf.isReadOnly(), "Buffer s read-only");
+        assertEquals(capacity, buf.capacity(),
+            "Buffer::capacity returned unexpected value");
+        assertEquals(0L, buf.position(),
+            "Buffer::position returned unexpected value");
+        assertEquals(capacity, buf.limit(),
+            "Buffer::limit returned unexpected value");
+
+        // Verify that the various state mutators work correctly
+        int halfPos = buf.capacity()/2;
+        buf.position(halfPos);
+        assertEquals(halfPos, buf.position(),
+            "Position not set to halfPos");
+        assertEquals(buf.capacity() - halfPos, buf.remaining(),
+            "Remaining not capacity - halfPos");
+
+        buf.mark();
+
+        int twoThirdsPos = 2*(buf.capacity()/3);
+        buf.position(twoThirdsPos);
+        assertEquals(twoThirdsPos, buf.position(),
+            "Position not set to twoThirdsPos");
+        assertEquals(buf.capacity() - twoThirdsPos, buf.remaining(),
+            "Remaining != capacity - twoThirdsPos");
+
+        buf.reset();
+        assertEquals(halfPos, buf.position(),
+            "Buffer not reset to halfPos");
+
+        buf.limit(twoThirdsPos);
+        assertEquals(twoThirdsPos, buf.limit(),
+            "Limit not set to twoThirdsPos");
+        assertEquals(twoThirdsPos - halfPos, buf.remaining(),
+            "Remaining != twoThirdsPos - halfPos");
+
+        buf.position(twoThirdsPos);
+        assertFalse(buf.hasRemaining(), "Buffer has remaining values");
+    }
+
+    @ParameterizedTest
+    @ValueSource(longs = {0L, 1L, (long)Integer.MAX_VALUE/2,
+        (long)Integer.MAX_VALUE - 1, (long)Integer.MAX_VALUE})
+    void legalCapacities(long capacity) {
+        long addr;
+        try {
+            addr = UNSAFE.allocateMemory(capacity);
+        } catch (OutOfMemoryError ignore) {
+            System.err.println("legalCapacities( " + capacity
+                + ") test skipped due to insufficient memory");
+            return;
+        }
+        try {
+            ByteBuffer buf = newDirectByteBuffer(addr, capacity);
+            assertEquals(addr, getDirectBufferAddress(buf),
+                "GetDirectBufferAddress does not return supplied address");
+            checkBuffer(buf, capacity);
+        } finally {
+            UNSAFE.freeMemory(addr);
+        }
+    }
+
+    @ParameterizedTest
+    @ValueSource(longs = {Long.MIN_VALUE, (long)Integer.MIN_VALUE - 1L, -1L,
+        (long)Integer.MAX_VALUE + 1L, 3_000_000_000L, 5_000_000_000L,
+        Long.MAX_VALUE})
+    void illegalCapacities(long capacity) {
+        assertThrows(IllegalArgumentException.class, () -> {
+            long addr = UNSAFE.allocateMemory(1);
+            try {
+                ByteBuffer buf = newDirectByteBuffer(addr, capacity);
+            } finally {
+                UNSAFE.freeMemory(addr);
+            }
+        });
+    }
+
+    // See libNewDirectByteBuffer.c for implementations.
+    private static native ByteBuffer newDirectByteBuffer(long addr, long capacity);
+    private static native long getDirectByteBufferCapacity(ByteBuffer buf);
+    private static native long getDirectBufferAddress(ByteBuffer buf);
+}

test/jdk/java/nio/jni/libNewDirectByteBuffer.c

@@ -0,0 +1,50 @@
+/*
+ * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+#include <stdlib.h>
+#include "jni.h"
+
+// private static native ByteBuffer newDirectByteBuffer(long addr, long size)
+JNIEXPORT jobject JNICALL
+Java_NewDirectByteBuffer_newDirectByteBuffer
+    (JNIEnv *env, jclass cls, jlong addr, jlong size)
+{
+    // Create the direct byte buffer, freeing the native memory if an exception
+    // is thrown while constructing the buffer
+    return (*env)->NewDirectByteBuffer(env, (void*)addr, size);
+}
+
+// private static native long getDirectByteBufferCapacity(ByteBuffer buf)
+JNIEXPORT jlong JNICALL
+Java_NewDirectByteBuffer_getDirectByteBufferCapacity
+    (JNIEnv *env, jclass cls, jobject buf)
+{
+    return (*env)->GetDirectBufferCapacity(env, buf);
+}
+
+// private static native long getDirectBufferAddress(ByteBuffer buf)
+JNIEXPORT jlong JNICALL
+Java_NewDirectByteBuffer_getDirectBufferAddress
+    (JNIEnv *env, jclass cls, jobject buf)
+{
+    return (jlong)(*env)->GetDirectBufferAddress(env, buf);
+}