Skip to content

8386719: [lworld] Remaining ValueClass argument verification issues #2586

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fparain wants to merge 1 commit into
base: lworld
Choose a base branch
from
+150 −34
Open

8386719: [lworld] Remaining ValueClass argument verification issues #2586

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 22 additions & 33 deletions src/hotspot/share/prims/jvm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,56 +430,45 @@ JVM_END
JVM_ENTRY(jarray, JVM_CopyOfSpecialArray(JNIEnv *env, jarray orig, jint from, jint to))
oop o = JNIHandles::resolve_non_null(orig);
assert(o->is_array(), "Must be");
oop array = nullptr;
objArrayOop dest = nullptr;
arrayOop org = (arrayOop)o;
arrayHandle oh(THREAD, org);
ObjArrayKlass* ak = ObjArrayKlass::cast(org->klass());
InlineKlass* vk = InlineKlass::cast(ak->element_klass());
int len = to - from; // length of the new array
int orig_length = org->length();
if (ak->is_null_free_array_klass()) {
if ((len != 0) && (from >= org->length() || to > org->length())) {
if ((len != 0) && (from >= orig_length || to > orig_length)) {
THROW_MSG_NULL(vmSymbols::java_lang_IllegalArgumentException(), "Copying of null-free array with uninitialized elements");
}
}

if (ak->is_flatArray_klass()) {
// The whole JVM_CopyOfSpecialArray is currently broken. Fix this in a separate bugfix.
int org_length = org->length();
int copy_len = MIN2(to, org_length) - MIN2(from, org_length);
FlatArrayKlass* const fak = FlatArrayKlass::cast(org->klass());
flatArrayOop dst = fak->allocate_instance(len, CHECK_NULL);
dest = fak->allocate_instance(len, CHECK_NULL);
} else {
const ArrayProperties props = ArrayProperties::Default().with_null_restricted(ak->is_null_free_array_klass());
dest = oopFactory::new_objArray(vk, len, props, CHECK_NULL);
}

arrayHandle dh(THREAD, dest);

int copy_len = MIN2(to, orig_length) - MIN2(from, orig_length);
if (copy_len != 0) {
assert(!ak->is_null_free_array_klass() || copy_len == len,
"Failed to throw the IllegalArgumentException");
if (copy_len != 0) {
int start = MIN2(from, org_length - 1);
FlatArrayPayload src_payload(flatArrayOop(oh()), start, fak);
FlatArrayPayload dst_payload(dst, 0, fak);
int end = to < oh()->length() ? to : oh()->length();
for (int i = from; i < end; i++) {
// Copy a value
src_payload.copy_to(dst_payload);

// Advance to the next element
src_payload.next_element();
dst_payload.next_element();
}
ak->copy_array(oh(), from, dh(), 0, copy_len, CHECK_NULL);
for (int i = copy_len; i < len; i++) {
((objArrayOop)dh())->obj_at_put(i, nullptr);
}
array = dst;
} else {
const ArrayProperties props = ArrayProperties::Default().with_null_restricted(ak->is_null_free_array_klass());

array = oopFactory::new_objArray(vk, len, props, CHECK_NULL);
int end = to < oh()->length() ? to : oh()->length();
for (int i = from; i < end; i++) {
if (i < ((objArrayOop)oh())->length()) {
oop val = ((objArrayOop)oh())->obj_at(i, CHECK_NULL);
((objArrayOop)array)->obj_at_put(i - from, val);
} else {
assert(!ak->is_null_free_array_klass(), "Must be a nullable array");
((objArrayOop)array)->obj_at_put(i - from, nullptr);
}
for (int i = 0; i < len; i++) {
((objArrayOop)dh())->obj_at_put(i, nullptr);
}
}
return (jarray) JNIHandles::make_local(THREAD, array);

return (jarray) JNIHandles::make_local(THREAD, dh());

JVM_END

#ifdef ASSERT
Expand Down
10 changes: 10 additions & 0 deletions src/java.base/share/classes/jdk/internal/value/ValueClass.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,14 +172,24 @@ public static boolean isFlatArray(Object[] array) {
@IntrinsicCandidate
private static native boolean isFlatArray0(Object[] array);

@ForceInline
private static void validateSpecialArray(Object[] array) {
Objects.requireNonNull(array);
if (!isConcreteValueClass(array.getClass().getComponentType())) {
throw new IllegalArgumentException("Element class is not a concrete value class");
}
}

public static Object[] copyOfSpecialArray(Object[] array, int newLength) {
validateSpecialArray(array);
if (newLength < 0) {
throw new NegativeArraySizeException("" + newLength);
}
return copyOfSpecialArray0(array, 0, newLength);
}

public static Object[] copyOfRangeSpecialArray(Object[] array, int from, int to) {
validateSpecialArray(array);
int length = array.length;
if (from < 0 || from > length) {
throw new ArrayIndexOutOfBoundsException("source index " + from + " out of bounds for object array[" + length + "]");
Expand Down
2 changes: 1 addition & 1 deletion test/hotspot/jtreg/runtime/valhalla/inlinetypes/InlineTypeArray.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ void testUtilArraysOnNullableAtomicArrays() {
expected3b[2] = null;
checkArrayElementsEqual(exceedingRangeCopy, expected3b);

// Range starting after the end of the original array, must suceed for nullable arrays
// Range starting after the end of the original array, must succeed for nullable arrays
MyInt[] farRangeCopy = (MyInt[]) Arrays.copyOfRange(myInts, myInts.length, myInts.length + 1);
MyInt[] expected3c = (MyInt[])ValueClass.newNullableAtomicArray(MyInt.class, 1);
expected3c[0] = null;
Expand Down
117 changes: 117 additions & 0 deletions test/hotspot/jtreg/runtime/valhalla/inlinetypes/TestArrayFactories.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,123 @@ static void test_25() {
} catch (NullPointerException e) { }
}

// Test arguments validation in copyOfSpecialArray()

static void test_26() {
try {
var a = ValueClass.copyOfSpecialArray(null, 0);
throw new RuntimeException("Missing NullPointerException");
} catch (NullPointerException e) {}
}

static void test_27() {
var array = ValueClass.newNullRestrictedNonAtomicArray(MyVal.class, 1, new MyVal());
try {
var a = ValueClass.copyOfSpecialArray(array, -1);
throw new RuntimeException("Missing NegativeArraySizeException");
} catch (NegativeArraySizeException e) {}
}

static void test_28() {
var array = new Object[1];
try {
var a = ValueClass.copyOfSpecialArray(array, 0);
throw new RuntimeException("Missing IllegalArgumentException");
} catch (IllegalArgumentException e) {}
}

static void test_29() {
var array = ValueClass.newNullRestrictedNonAtomicArray(MyVal.class, 2, new MyVal());
try {
var a = ValueClass.copyOfSpecialArray(array, 4);
throw new RuntimeException("Missing IllegalArgumentException");
} catch (IllegalArgumentException e) {}
}

static void test_30() {
var array = ValueClass.newNullRestrictedAtomicArray(MyVal.class, 2, new MyVal());
try {
var a = ValueClass.copyOfSpecialArray(array, 4);
throw new RuntimeException("Missing IllegalArgumentException");
} catch (IllegalArgumentException e) {}
}

static void test_31() {
var array = ValueClass.newNullableAtomicArray(MyVal.class, 2);
try {
var a = ValueClass.copyOfSpecialArray(array, 4);
Asserts.assertEquals(a.length, 4);
} catch (Throwable e) {
throw new RuntimeException("Unexpected exception " + e);
}
}

// Test arguments validation in copyOfRangeSpecialArray()

static void test_32() {
try {
var a = ValueClass.copyOfRangeSpecialArray(null, 0, 1);
throw new RuntimeException("Missing NullPointerException");
} catch (NullPointerException e) {}
}

static void test_33() {
var array = ValueClass.newNullRestrictedNonAtomicArray(MyVal.class, 2, new MyVal());
try {
var a = ValueClass.copyOfRangeSpecialArray(array, -1, 1);
throw new RuntimeException("Missing ArrayIndexOutOfBoundsException");
} catch (ArrayIndexOutOfBoundsException e) {}
}

static void test_34() {
var array = ValueClass.newNullRestrictedNonAtomicArray(MyVal.class, 2, new MyVal());
try {
var a = ValueClass.copyOfRangeSpecialArray(array, 3, 4);
throw new RuntimeException("Missing ArrayIndexOutOfBoundsException");
} catch (ArrayIndexOutOfBoundsException e) {}
}

static void test_35() {
var array = ValueClass.newNullRestrictedNonAtomicArray(MyVal.class, 2, new MyVal());
try {
var a = ValueClass.copyOfRangeSpecialArray(array, 1, 0);
throw new RuntimeException("Missing IllegalArgumentException");
} catch (IllegalArgumentException e) {}
}

static void test_36() {
var array = ValueClass.newNullRestrictedNonAtomicArray(MyVal.class, 2, new MyVal());
try {
var a = ValueClass.copyOfRangeSpecialArray(array, 2, 4);
throw new RuntimeException("Missing IllegalArgumentException");
} catch (IllegalArgumentException e) {}
}

static void test_37() {
var array = ValueClass.newNullRestrictedAtomicArray(MyVal.class, 2, new MyVal());
try {
var a = ValueClass.copyOfRangeSpecialArray(array, 2, 4);
throw new RuntimeException("Missing IllegalArgumentException");
} catch (IllegalArgumentException e) {}
}

static void test_38() {
var array = ValueClass.newNullableAtomicArray(MyVal.class, 2);
try {
var a = ValueClass.copyOfRangeSpecialArray(array, 2, 4);
} catch (Throwable e) {
throw new RuntimeException("Unexpected exception " + e);
}
}

static void test_39() {
var array = new Object[1];
try {
var a = ValueClass.copyOfRangeSpecialArray(array, 0, 0);
throw new RuntimeException("Missing IllegalArgumentException");
} catch (IllegalArgumentException e) {}
}

public static void main(String[] args) {
var test = new TestArrayFactories();
Class c = test.getClass();
Expand Down