Skip to content

Commit 2384e12

Browse files
TobiHartmannTobiHartmann
committed
8270098: ZGC: ZBarrierSetC2::clone_at_expansion fails with "Guard against surprises" assert
Reviewed-by: neliasso, kvn
1 parent d53d94b commit 2384e12

File tree

2 files changed

+95
-23
lines changed

2 files changed

+95
-23
lines changed

‎src/hotspot/share/gc/z/c2/zBarrierSetC2.cpp

+9-18
Original file line numberDiff line numberDiff line change
@@ -268,24 +268,16 @@ static const TypeFunc* clone_type() {
268268

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

272-
if (ac->is_clone_array()) {
273-
const TypeAryPtr* ary_ptr = src->get_ptr_type()->isa_aryptr();
274-
BasicType bt;
275-
if (ary_ptr == NULL) {
276-
// ary_ptr can be null iff we are running with StressReflectiveCode
277-
// This code will be unreachable
278-
assert(StressReflectiveCode, "Guard against surprises");
279-
bt = T_LONG;
273+
if (ac->is_clone_array() && ary_ptr != NULL) {
274+
BasicType bt = ary_ptr->elem()->array_element_basic_type();
275+
if (is_reference_type(bt)) {
276+
// Clone object array
277+
bt = T_OBJECT;
280278
} else {
281-
bt = ary_ptr->elem()->array_element_basic_type();
282-
if (is_reference_type(bt)) {
283-
// Clone object array
284-
bt = T_OBJECT;
285-
} else {
286-
// Clone primitive array
287-
bt = T_LONG;
288-
}
279+
// Clone primitive array
280+
bt = T_LONG;
289281
}
290282

291283
Node* ctrl = ac->in(TypeFunc::Control);
@@ -333,12 +325,11 @@ void ZBarrierSetC2::clone_at_expansion(PhaseMacroExpand* phase, ArrayCopyNode* a
333325
Node* const dst = ac->in(ArrayCopyNode::Dest);
334326
Node* const size = ac->in(ArrayCopyNode::Length);
335327

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

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

344335
Node* const call = phase->make_leaf_call(ctrl,

‎test/hotspot/jtreg/compiler/arraycopy/TestObjectArrayClone.java

+86-5
Original file line numberDiff line numberDiff line change
@@ -23,21 +23,33 @@
2323

2424
/*
2525
* @test
26-
* @bug 8155643 8268125 8270461
26+
* @bug 8155643 8268125 8270461 8270098
2727
* @summary Test Object.clone() intrinsic.
28+
* @modules java.base/java.lang:+open
2829
*
2930
* @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:-ReduceInitialCardMarks
3031
* -XX:CompileCommand=compileonly,compiler.arraycopy.TestObjectArrayClone::testClone*
32+
* -XX:CompileCommand=compileonly,jdk.internal.reflect.GeneratedMethodAccessor*::invoke
3133
* compiler.arraycopy.TestObjectArrayClone
3234
* @run main/othervm -XX:CompileCommand=compileonly,compiler.arraycopy.TestObjectArrayClone::testClone*
35+
* -XX:CompileCommand=compileonly,jdk.internal.reflect.GeneratedMethodAccessor*::invoke
3336
* compiler.arraycopy.TestObjectArrayClone
3437
* @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:-UseCompressedClassPointers -Xmx128m
3538
* -XX:CompileCommand=compileonly,compiler.arraycopy.TestObjectArrayClone::testClone*
39+
* -XX:CompileCommand=compileonly,jdk.internal.reflect.GeneratedMethodAccessor*::invoke
40+
* compiler.arraycopy.TestObjectArrayClone
41+
* @run main/othervm -Xbatch -XX:-UseTypeProfile
42+
* -XX:CompileCommand=compileonly,compiler.arraycopy.TestObjectArrayClone::testClone*
43+
* -XX:CompileCommand=compileonly,jdk.internal.reflect.GeneratedMethodAccessor*::invoke
3644
* compiler.arraycopy.TestObjectArrayClone
3745
*/
3846

3947
package compiler.arraycopy;
4048

49+
import java.lang.invoke.*;
50+
import java.lang.reflect.InvocationTargetException;
51+
import java.lang.reflect.Method;
52+
4153
class Payload implements Cloneable {
4254
boolean b;
4355
int i;
@@ -59,7 +71,7 @@ public Payload(boolean b, int i, char c, String str, short s, int i2) {
5971
public Payload clonep() {
6072
try {
6173
return (Payload) super.clone();
62-
} catch(CloneNotSupportedException e) {
74+
} catch (CloneNotSupportedException e) {
6375
return null;
6476
}
6577
}
@@ -136,6 +148,17 @@ public static String[] testCloneShortObjectArray() {
136148
return arr.clone();
137149
}
138150

151+
public static String[] testCloneShortObjectArray2(Method clone) throws Exception {
152+
String[] arr = new String[5];
153+
arr[0] = str1;
154+
arr[1] = str2;
155+
arr[2] = str3;
156+
arr[3] = str4;
157+
arr[4] = str5;
158+
escape_arr = arr;
159+
return (String[]) testCloneObject(clone, arr);
160+
}
161+
139162
public static String[] testCloneShortObjectArrayCopy() {
140163
String[] arr = new String[5];
141164
arr[0] = str1;
@@ -161,7 +184,14 @@ public static Object testCloneOop2(Payload2 p) {
161184
return p.clonep();
162185
}
163186

164-
public static void main(String[] args) {
187+
public static Object testCloneObject(Method clone, Object obj) throws Exception {
188+
return clone.invoke(obj);
189+
}
190+
191+
public static void main(String[] args) throws Exception {
192+
Method clone = Object.class.getDeclaredMethod("clone");
193+
clone.setAccessible(true);
194+
165195
String[] arr1 = new String[42];
166196
for (int j = 0; j < arr1.length; j++) {
167197
arr1[j] = new String(Integer.toString(j));
@@ -178,13 +208,30 @@ public static void main(String[] args) {
178208
verifyStr(arr1, arr2);
179209
}
180210

211+
for (int i = 0; i < 50_000; i++) {
212+
for (int j = 0; j < arr1.length; j++) {
213+
arr1[j] = new String(Integer.toString(j));
214+
}
215+
String[] arr2 = (String[]) testCloneObject(clone, arr1);
216+
verifyStr(arr1, arr2);
217+
String[] arr3 = (String[]) testCloneObject(clone, arr1);
218+
verifyStr(arr1, arr3);
219+
String[] arr4 = (String[]) testCloneObject(clone, arr1);
220+
verifyStr(arr1, arr4);
221+
verifyStr(arr1, arr3);
222+
verifyStr(arr1, arr2);
223+
}
224+
181225
for (int i = 0; i < 50_000; i++) {
182226
String[] value = testCloneShortObjectArray();
183227
verifyStr(value, escape_arr);
184228
String[] value2 = testCloneShortObjectArray();
185229
verifyStr(value2, escape_arr);
186230
String[] value3 = testCloneShortObjectArray();
187231
verifyStr(value3, escape_arr);
232+
String[] value4 = testCloneShortObjectArray2(clone);
233+
verifyStr(value4, escape_arr);
234+
verifyStr(value, value4);
188235
verifyStr(value, value3);
189236
verifyStr(value, value2);
190237
}
@@ -211,8 +258,21 @@ public static void main(String[] args) {
211258
verifyStr(value, value2);
212259
}
213260

261+
int[] arr2 = new int[42];
262+
for (int i = 0; i < arr2.length; i++) {
263+
arr2[i] = i;
264+
}
214265
for (int i = 0; i < 50_000; i++) {
215-
testClonePrimitiveArray(new int[42]);
266+
int[] res1 = testClonePrimitiveArray(arr2);
267+
int[] res2 = (int[])testCloneObject(clone, arr2);
268+
for (int j = 0; j < arr2.length; j++) {
269+
if (res1[j] != j) {
270+
throw new RuntimeException("Unexpected result: " + res1[j] + " != " + j);
271+
}
272+
if (res2[j] != j) {
273+
throw new RuntimeException("Unexpected result: " + res2[j] + " != " + j);
274+
}
275+
}
216276
}
217277

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

290+
for (int i = 0; i < 50_000; i++) {
291+
Payload p1 = (Payload) testCloneObject(clone, ref);
292+
verifyPayload(ref, p1);
293+
Payload p2 = (Payload) testCloneObject(clone, ref);
294+
verifyPayload(ref, p2);
295+
Payload p3 = (Payload) testCloneObject(clone, ref);
296+
verifyPayload(ref, p3);
297+
verifyPayload(p2, p3);
298+
verifyPayload(p1, p3);
299+
}
300+
230301
Payload2 ref2 = new Payload2(false, -1, 'c', str1, (short) 5, -1, false, 0, 'k', str2, (short)-1, 0);
231302
for (int i = 0; i < 50_000; i++) {
232303
Payload2 p1 = (Payload2) testCloneOop2(ref2);
@@ -238,6 +309,17 @@ public static void main(String[] args) {
238309
verifyPayload2(p2, p3);
239310
verifyPayload2(p1, p3);
240311
}
312+
313+
for (int i = 0; i < 50_000; i++) {
314+
Payload2 p1 = (Payload2) testCloneObject(clone, ref2);
315+
verifyPayload2(ref2, p1);
316+
Payload2 p2 = (Payload2) testCloneObject(clone, ref2);
317+
verifyPayload2(ref2, p2);
318+
Payload2 p3 = (Payload2) testCloneObject(clone, ref2);
319+
verifyPayload2(ref2, p3);
320+
verifyPayload2(p2, p3);
321+
verifyPayload2(p1, p3);
322+
}
241323
}
242324

243325
public static void verifyPayload(Payload p1, Payload p2) {
@@ -323,7 +405,6 @@ public static void verifyStr(String[] arr1, String[] arr2) {
323405
if (!arr1[i].equals(arr2[i])) {
324406
throw new RuntimeException("Fail cloned element content not the same");
325407
}
326-
327408
}
328409
}
329410
}

0 commit comments

Comments
 (0)