Skip to content

Commit

Permalink
8270098: ZGC: ZBarrierSetC2::clone_at_expansion fails with "Guard aga…
Browse files Browse the repository at this point in the history 
…inst surprises" assert

Backport-of: 2384e128886ce8d60cb76e6cdae3a76ccebb9e89
  • Loading branch information
@TobiHartmann
TobiHartmann committed Aug 18, 2021
1 parent cb7e311 commit 52c3e7d
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 23 deletions.
27 changes: 9 additions & 18 deletions src/hotspot/share/gc/z/c2/zBarrierSetC2.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,24 +268,16 @@ static const TypeFunc* clone_type() {

void ZBarrierSetC2::clone_at_expansion(PhaseMacroExpand* phase, ArrayCopyNode* ac) const {
Node* const src = ac->in(ArrayCopyNode::Src);
const TypeAryPtr* ary_ptr = src->get_ptr_type()->isa_aryptr();

if (ac->is_clone_array()) {
const TypeAryPtr* ary_ptr = src->get_ptr_type()->isa_aryptr();
BasicType bt;
if (ary_ptr == NULL) {
// ary_ptr can be null iff we are running with StressReflectiveCode
// This code will be unreachable
assert(StressReflectiveCode, "Guard against surprises");
bt = T_LONG;
if (ac->is_clone_array() && ary_ptr != NULL) {
BasicType bt = ary_ptr->elem()->array_element_basic_type();
if (is_reference_type(bt)) {
// Clone object array
bt = T_OBJECT;
} else {
bt = ary_ptr->elem()->array_element_basic_type();
if (is_reference_type(bt)) {
// Clone object array
bt = T_OBJECT;
} else {
// Clone primitive array
bt = T_LONG;
}
// Clone primitive array
bt = T_LONG;
}

Node* ctrl = ac->in(TypeFunc::Control);
Expand Down Expand Up @@ -330,12 +322,11 @@ void ZBarrierSetC2::clone_at_expansion(PhaseMacroExpand* phase, ArrayCopyNode* a
Node* const dst = ac->in(ArrayCopyNode::Dest);
Node* const size = ac->in(ArrayCopyNode::Length);

assert(ac->is_clone_inst(), "Sanity check");
assert(size->bottom_type()->is_long(), "Should be long");

// The native clone we are calling here expects the instance size in words
// Add header/offset size to payload size to get instance size.
Node* const base_offset = phase->longcon(arraycopy_payload_base_offset(false) >> LogBytesPerLong);
Node* const base_offset = phase->longcon(arraycopy_payload_base_offset(ac->is_clone_array()) >> LogBytesPerLong);
Node* const full_size = phase->transform_later(new AddLNode(size, base_offset));

Node* const call = phase->make_leaf_call(ctrl,
Expand Down
91 changes: 86 additions & 5 deletions test/hotspot/jtreg/compiler/arraycopy/TestObjectArrayClone.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,33 @@

/*
* @test
* @bug 8155643 8268125 8270461
* @bug 8155643 8268125 8270461 8270098
* @summary Test Object.clone() intrinsic.
* @modules java.base/java.lang:+open
*
* @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:-ReduceInitialCardMarks
* -XX:CompileCommand=compileonly,compiler.arraycopy.TestObjectArrayClone::testClone*
* -XX:CompileCommand=compileonly,jdk.internal.reflect.GeneratedMethodAccessor*::invoke
* compiler.arraycopy.TestObjectArrayClone
* @run main/othervm -XX:CompileCommand=compileonly,compiler.arraycopy.TestObjectArrayClone::testClone*
* -XX:CompileCommand=compileonly,jdk.internal.reflect.GeneratedMethodAccessor*::invoke
* compiler.arraycopy.TestObjectArrayClone
* @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:-UseCompressedClassPointers -Xmx128m
* -XX:CompileCommand=compileonly,compiler.arraycopy.TestObjectArrayClone::testClone*
* -XX:CompileCommand=compileonly,jdk.internal.reflect.GeneratedMethodAccessor*::invoke
* compiler.arraycopy.TestObjectArrayClone
* @run main/othervm -Xbatch -XX:-UseTypeProfile
* -XX:CompileCommand=compileonly,compiler.arraycopy.TestObjectArrayClone::testClone*
* -XX:CompileCommand=compileonly,jdk.internal.reflect.GeneratedMethodAccessor*::invoke
* compiler.arraycopy.TestObjectArrayClone
*/

package compiler.arraycopy;

import java.lang.invoke.*;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;

class Payload implements Cloneable {
boolean b;
int i;
Expand All @@ -59,7 +71,7 @@ public Payload(boolean b, int i, char c, String str, short s, int i2) {
public Payload clonep() {
try {
return (Payload) super.clone();
} catch(CloneNotSupportedException e) {
} catch (CloneNotSupportedException e) {
return null;
}
}
Expand Down Expand Up @@ -136,6 +148,17 @@ public static String[] testCloneShortObjectArray() {
return arr.clone();
}

public static String[] testCloneShortObjectArray2(Method clone) throws Exception {
String[] arr = new String[5];
arr[0] = str1;
arr[1] = str2;
arr[2] = str3;
arr[3] = str4;
arr[4] = str5;
escape_arr = arr;
return (String[]) testCloneObject(clone, arr);
}

public static String[] testCloneShortObjectArrayCopy() {
String[] arr = new String[5];
arr[0] = str1;
Expand All @@ -161,7 +184,14 @@ public static Object testCloneOop2(Payload2 p) {
return p.clonep();
}

public static void main(String[] args) {
public static Object testCloneObject(Method clone, Object obj) throws Exception {
return clone.invoke(obj);
}

public static void main(String[] args) throws Exception {
Method clone = Object.class.getDeclaredMethod("clone");
clone.setAccessible(true);

String[] arr1 = new String[42];
for (int j = 0; j < arr1.length; j++) {
arr1[j] = new String(Integer.toString(j));
Expand All @@ -178,13 +208,30 @@ public static void main(String[] args) {
verifyStr(arr1, arr2);
}

for (int i = 0; i < 50_000; i++) {
for (int j = 0; j < arr1.length; j++) {
arr1[j] = new String(Integer.toString(j));
}
String[] arr2 = (String[]) testCloneObject(clone, arr1);
verifyStr(arr1, arr2);
String[] arr3 = (String[]) testCloneObject(clone, arr1);
verifyStr(arr1, arr3);
String[] arr4 = (String[]) testCloneObject(clone, arr1);
verifyStr(arr1, arr4);
verifyStr(arr1, arr3);
verifyStr(arr1, arr2);
}

for (int i = 0; i < 50_000; i++) {
String[] value = testCloneShortObjectArray();
verifyStr(value, escape_arr);
String[] value2 = testCloneShortObjectArray();
verifyStr(value2, escape_arr);
String[] value3 = testCloneShortObjectArray();
verifyStr(value3, escape_arr);
String[] value4 = testCloneShortObjectArray2(clone);
verifyStr(value4, escape_arr);
verifyStr(value, value4);
verifyStr(value, value3);
verifyStr(value, value2);
}
Expand All @@ -211,8 +258,21 @@ public static void main(String[] args) {
verifyStr(value, value2);
}

int[] arr2 = new int[42];
for (int i = 0; i < arr2.length; i++) {
arr2[i] = i;
}
for (int i = 0; i < 50_000; i++) {
testClonePrimitiveArray(new int[42]);
int[] res1 = testClonePrimitiveArray(arr2);
int[] res2 = (int[])testCloneObject(clone, arr2);
for (int j = 0; j < arr2.length; j++) {
if (res1[j] != j) {
throw new RuntimeException("Unexpected result: " + res1[j] + " != " + j);
}
if (res2[j] != j) {
throw new RuntimeException("Unexpected result: " + res2[j] + " != " + j);
}
}
}

Payload ref = new Payload(false, -1, 'c', str1, (short) 5, -1);
Expand All @@ -227,6 +287,17 @@ public static void main(String[] args) {
verifyPayload(p1, p3);
}

for (int i = 0; i < 50_000; i++) {
Payload p1 = (Payload) testCloneObject(clone, ref);
verifyPayload(ref, p1);
Payload p2 = (Payload) testCloneObject(clone, ref);
verifyPayload(ref, p2);
Payload p3 = (Payload) testCloneObject(clone, ref);
verifyPayload(ref, p3);
verifyPayload(p2, p3);
verifyPayload(p1, p3);
}

Payload2 ref2 = new Payload2(false, -1, 'c', str1, (short) 5, -1, false, 0, 'k', str2, (short)-1, 0);
for (int i = 0; i < 50_000; i++) {
Payload2 p1 = (Payload2) testCloneOop2(ref2);
Expand All @@ -238,6 +309,17 @@ public static void main(String[] args) {
verifyPayload2(p2, p3);
verifyPayload2(p1, p3);
}

for (int i = 0; i < 50_000; i++) {
Payload2 p1 = (Payload2) testCloneObject(clone, ref2);
verifyPayload2(ref2, p1);
Payload2 p2 = (Payload2) testCloneObject(clone, ref2);
verifyPayload2(ref2, p2);
Payload2 p3 = (Payload2) testCloneObject(clone, ref2);
verifyPayload2(ref2, p3);
verifyPayload2(p2, p3);
verifyPayload2(p1, p3);
}
}

public static void verifyPayload(Payload p1, Payload p2) {
Expand Down Expand Up @@ -323,7 +405,6 @@ public static void verifyStr(String[] arr1, String[] arr2) {
if (!arr1[i].equals(arr2[i])) {
throw new RuntimeException("Fail cloned element content not the same");
}

}
}
}
Expand Down

1 comment on commit 52c3e7d

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Review

Issues

Please sign in to comment.