From a56598f5a534cc9223367e7faa8433ea38661db9 Mon Sep 17 00:00:00 2001 From: Brian Burkhalter Date: Mon, 23 Jan 2023 17:12:49 +0000 Subject: [PATCH] 8299684: (bf) JNI direct buffer functions with large capacity behave unexpectedly Reviewed-by: dholmes, alanb --- make/test/JtregNativeJdk.gmk | 4 +- src/hotspot/share/prims/jni.cpp | 7 +- .../java/nio/Direct-X-Buffer.java.template | 23 ++- .../jdk/java/nio/jni/NewDirectByteBuffer.java | 143 ++++++++++++++++++ .../jdk/java/nio/jni/libNewDirectByteBuffer.c | 50 ++++++ 5 files changed, 218 insertions(+), 9 deletions(-) create mode 100644 test/jdk/java/nio/jni/NewDirectByteBuffer.java create mode 100644 test/jdk/java/nio/jni/libNewDirectByteBuffer.c diff --git a/make/test/JtregNativeJdk.gmk b/make/test/JtregNativeJdk.gmk index 37b529e3d5a44..d1e2792975976 100644 --- a/make/test/JtregNativeJdk.gmk +++ b/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 diff --git a/src/hotspot/share/prims/jni.cpp b/src/hotspot/share/prims/jni.cpp index 08633a20788d2..1aa7eaad986e6 100644 --- a/src/hotspot/share/prims/jni.cpp +++ b/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, "", "(JI)V"); + directByteBufferConstructor = env->GetMethodID(directByteBufferClass, "", "(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; } diff --git a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template index b65306f6e21eb..69cce7fa7c48f 100644 --- a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template +++ b/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 diff --git a/test/jdk/java/nio/jni/NewDirectByteBuffer.java b/test/jdk/java/nio/jni/NewDirectByteBuffer.java new file mode 100644 index 0000000000000..8033dba13ec30 --- /dev/null +++ b/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); +} diff --git a/test/jdk/java/nio/jni/libNewDirectByteBuffer.c b/test/jdk/java/nio/jni/libNewDirectByteBuffer.c new file mode 100644 index 0000000000000..3aec0713ad1c4 --- /dev/null +++ b/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 +#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); +}