Skip to content

Commit 4b5be40

Browse files
author
Rahul Raghavan
committed
8238812: assert(false) failed: bad AD file
Reviewed-by: thartmann, chagedorn, roland
1 parent b2a2ddf commit 4b5be40

File tree

4 files changed

+208
-10
lines changed

4 files changed

+208
-10
lines changed

‎src/hotspot/share/opto/compile.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4161,10 +4161,10 @@ Node* Compile::conv_I2X_index(PhaseGVN* phase, Node* idx, const TypeInt* sizetyp
41614161
}
41624162

41634163
// Convert integer value to a narrowed long type dependent on ctrl (for example, a range check)
4164-
Node* Compile::constrained_convI2L(PhaseGVN* phase, Node* value, const TypeInt* itype, Node* ctrl) {
4164+
Node* Compile::constrained_convI2L(PhaseGVN* phase, Node* value, const TypeInt* itype, Node* ctrl, bool carry_dependency) {
41654165
if (ctrl != NULL) {
41664166
// Express control dependency by a CastII node with a narrow type.
4167-
value = new CastIINode(value, itype, false, true /* range check dependency */);
4167+
value = new CastIINode(value, itype, carry_dependency, true /* range check dependency */);
41684168
// Make the CastII node dependent on the control input to prevent the narrowed ConvI2L
41694169
// node from floating above the range check during loop optimizations. Otherwise, the
41704170
// ConvI2L node may be eliminated independently of the range check, causing the data path

‎src/hotspot/share/opto/compile.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1157,7 +1157,7 @@ class Compile : public Phase {
11571157
Node* ctrl = NULL);
11581158

11591159
// Convert integer value to a narrowed long type dependent on ctrl (for example, a range check)
1160-
static Node* constrained_convI2L(PhaseGVN* phase, Node* value, const TypeInt* itype, Node* ctrl);
1160+
static Node* constrained_convI2L(PhaseGVN* phase, Node* value, const TypeInt* itype, Node* ctrl, bool carry_dependency = false);
11611161

11621162
// Auxiliary methods for randomized fuzzing/stressing
11631163
int random();

‎src/hotspot/share/opto/parse2.cpp‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -863,10 +863,16 @@ bool Parse::create_jump_tables(Node* key_val, SwitchRange* lo, SwitchRange* hi)
863863

864864
// Clean the 32-bit int into a real 64-bit offset.
865865
// Otherwise, the jint value 0 might turn into an offset of 0x0800000000.
866-
const TypeInt* ikeytype = TypeInt::make(0, num_cases, Type::WidenMin);
867866
// Make I2L conversion control dependent to prevent it from
868867
// floating above the range check during loop optimizations.
869-
key_val = C->conv_I2X_index(&_gvn, key_val, ikeytype, control());
868+
// Do not use a narrow int type here to prevent the data path from dying
869+
// while the control path is not removed. This can happen if the type of key_val
870+
// is later known to be out of bounds of [0, num_cases] and therefore a narrow cast
871+
// would be replaced by TOP while C2 is not able to fold the corresponding range checks.
872+
// Set _carry_dependency for the cast to avoid being removed by IGVN.
873+
#ifdef _LP64
874+
key_val = C->constrained_convI2L(&_gvn, key_val, TypeInt::INT, control(), true /* carry_dependency */);
875+
#endif
870876

871877
// Shift the value by wordsize so we have an index into the table, rather
872878
// than a switch value

‎test/hotspot/jtreg/compiler/c2/TestJumpTable.java‎

Lines changed: 197 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -23,18 +23,24 @@
2323

2424
/**
2525
* @test
26-
* @bug 8229855
26+
* @bug 8229855 8238812
2727
* @summary Test jump table with key value that gets out of bounds after loop unrolling.
2828
* @run main/othervm -XX:CompileCommand=dontinline,compiler.c2.TestJumpTable::test*
2929
* -Xbatch -XX:+UnlockDiagnosticVMOptions -XX:-TieredCompilation -XX:-UseSwitchProfiling
3030
* compiler.c2.TestJumpTable
31+
* @run main/othervm -XX:CompileCommand=dontinline,compiler.c2.TestJumpTable::test*
32+
* -Xbatch -XX:-TieredCompilation -XX:-UseOnStackReplacement
33+
* compiler.c2.TestJumpTable
34+
* @run main/othervm -XX:CompileCommand=dontinline,compiler.c2.TestJumpTable::test*
35+
* -Xbatch -XX:+UnlockDiagnosticVMOptions -XX:-TieredCompilation -XX:+StressIGVN
36+
* compiler.c2.TestJumpTable
3137
*/
3238

3339
package compiler.c2;
3440

3541
public class TestJumpTable {
3642

37-
public static int test() {
43+
public static int test0() {
3844
int res = 0;
3945
for (int i = 10; i < 50; ++i) {
4046
switch (i * 5) {
@@ -53,9 +59,195 @@ public static int test() {
5359
return res;
5460
}
5561

62+
static int field;
63+
64+
// Original (slightly simplified) fuzzer generated test
65+
public static void test1() {
66+
int i4, i5 = 99, i6, i9 = 89;
67+
for (i4 = 12; i4 < 365; i4++) {
68+
for (i6 = 5; i6 > 1; i6--) {
69+
switch ((i6 * 5) + 11) {
70+
case 13:
71+
case 19:
72+
case 26:
73+
case 31:
74+
case 35:
75+
case 41:
76+
case 43:
77+
case 61:
78+
case 71:
79+
case 83:
80+
case 314:
81+
i9 = i5;
82+
break;
83+
}
84+
}
85+
}
86+
}
87+
88+
// This generates the following subgraph:
89+
/*
90+
// i: -10..4
91+
if ((i+min_jint) u<= max_jint) { <- This is always true but not folded by C2
92+
...
93+
} else {
94+
...
95+
CastII(i-5, 0..45) <- Replaced by TOP because i-5 range is -15..-1 but still considered reachable by C2 although it is dead code
96+
...
97+
}
98+
*/
99+
public static void test2() {
100+
for (int i = 5; i > -10; i--) {
101+
switch (i) {
102+
case 0:
103+
case 4:
104+
case 10:
105+
case 20:
106+
case 30:
107+
case 40:
108+
case 50:
109+
case 100:
110+
field = 42;
111+
break;
112+
}
113+
}
114+
}
115+
116+
// This generates the following subgraph:
117+
/*
118+
// i: -20..0
119+
if (i != 0) {
120+
// i: -20..-1
121+
if (i < 0) { <- This is always true but not folded by C2
122+
// Fall through
123+
} else {
124+
...
125+
CastII(i-1, 0..4) <- Replaced by TOP because i-1 range is -21..-1 but still considered reachable by C2 although it is dead code
126+
...
127+
}
128+
} else {
129+
StoreI <- Due to this additional store on, IfNode::has_shared_region returns false and the fold compares optimization does not kick in
130+
}
131+
*/
132+
public static void test3() {
133+
for (int i = 5; i > -20; i -= 5) {
134+
switch (i) {
135+
case 0:
136+
case 10:
137+
case 20:
138+
case 30:
139+
case 40:
140+
case 50:
141+
case 60:
142+
case 100:
143+
field = 42;
144+
break;
145+
}
146+
}
147+
}
148+
149+
// This generates the following subgraph:
150+
/*
151+
// i: -20..0
152+
if (i != 0) {
153+
// i: -20..-1
154+
if (i u< 101) { <- This is always false but not folded by C2 because CmpU is not handled
155+
CastII(i-1, 0..49) <- Replaced by TOP because i-1 range is -21..-1 but still considered reachable by C2 although it is dead code
156+
} else {
157+
...
158+
}
159+
} else {
160+
...
161+
}
162+
*/
163+
public static void test4() {
164+
int local = 0;
165+
for (int i = 5; i > -20; i -= 5) {
166+
switch (i) {
167+
case 0:
168+
case 10:
169+
case 20:
170+
case 30:
171+
case 40:
172+
case 50:
173+
case 100:
174+
local = 42;
175+
break;
176+
}
177+
}
178+
}
179+
180+
// This generates the following subgraph:
181+
/*
182+
// i: 0..20
183+
if (i != 20) {
184+
// i: 0..19
185+
if ((i-20) u< 281) { <- This is always false but not folded by C2 because the two ifs compare different values
186+
CastII(i-21, 0..49) <- Replaced by TOP because i-21 range is -21..-1 but still considered reachable by C2 although it is dead code
187+
} else {
188+
...
189+
}
190+
} else {
191+
...
192+
}
193+
*/
194+
public static void test5() {
195+
int local;
196+
for (int i = 25; i > 0; i -= 5) {
197+
switch (i) {
198+
case 20:
199+
case 30:
200+
case 40:
201+
case 50:
202+
case 60:
203+
case 70:
204+
case 300:
205+
local = 42;
206+
break;
207+
}
208+
}
209+
}
210+
211+
// This generates the following subgraph:
212+
/*
213+
// i: 0..20
214+
if ((i+10) != 30) {
215+
// i: 0..19
216+
if ((i-20) u< 271) { <- This is always false but not folded by C2 because the two ifs compare different values
217+
CastII(i-21, 0..4) <- Replaced by TOP because i-21 range is -21..-1 but still considered reachable by C2 although it is dead code
218+
} else {
219+
...
220+
}
221+
} else {
222+
...
223+
}
224+
*/
225+
public static void test6() {
226+
int local;
227+
for (int i = 25; i > 0; i -= 5) {
228+
switch (i + 10) {
229+
case 30:
230+
case 40:
231+
case 50:
232+
case 60:
233+
case 70:
234+
case 80:
235+
case 300:
236+
local = 42;
237+
break;
238+
}
239+
}
240+
}
241+
56242
public static void main(String[] args) {
57-
for (int i = 0; i < 20_000; ++i) {
58-
test();
243+
for (int i = 0; i < 50_000; ++i) {
244+
test0();
245+
test1();
246+
test2();
247+
test3();
248+
test4();
249+
test5();
250+
test6();
59251
}
60252
}
61253
}

0 commit comments

Comments
 (0)