Skip to content

Commit

Permalink
8299684: (bf) JNI direct buffer functions with large capacity behave …
Browse files Browse the repository at this point in the history
…unexpectedly

Reviewed-by: dholmes, alanb
  • Loading branch information
Brian Burkhalter committed Jan 23, 2023
1 parent 542bfe6 commit a56598f
Show file tree
Hide file tree
Showing 5 changed files with 218 additions and 9 deletions.
4 changes: 3 additions & 1 deletion 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
Expand Down Expand Up @@ -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
Expand Down
7 changes: 2 additions & 5 deletions src/hotspot/share/prims/jni.cpp
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
23 changes: 20 additions & 3 deletions 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
Expand Down Expand Up @@ -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
Expand Down
143 changes: 143 additions & 0 deletions 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);
}
50 changes: 50 additions & 0 deletions 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);
}

1 comment on commit a56598f

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.