8332829: [BACKOUT] C2: crash in compiled code because of dependency on removed range check CastIIs

Reviewed-by: thartmann
This commit is contained in:
Roland Westrelin 2024-05-23 16:37:01 +00:00
parent 7fd9d6c760
commit c9a7b9772d
6 changed files with 26 additions and 539 deletions

View File

@ -213,7 +213,13 @@ const Type* CastIINode::Value(PhaseGVN* phase) const {
// Similar to ConvI2LNode::Value() for the same reasons // Similar to ConvI2LNode::Value() for the same reasons
// see if we can remove type assertion after loop opts // see if we can remove type assertion after loop opts
res = widen_type(phase, res, T_INT); // But here we have to pay extra attention:
// Do not narrow the type of range check dependent CastIINodes to
// avoid corruption of the graph if a CastII is replaced by TOP but
// the corresponding range check is not removed.
if (!_range_check_dependency) {
res = widen_type(phase, res, T_INT);
}
return res; return res;
} }
@ -233,11 +239,11 @@ Node *CastIINode::Ideal(PhaseGVN *phase, bool can_reshape) {
if (progress != nullptr) { if (progress != nullptr) {
return progress; return progress;
} }
if (can_reshape && !phase->C->post_loop_opts_phase()) { if (can_reshape && !_range_check_dependency && !phase->C->post_loop_opts_phase()) {
// makes sure we run ::Value to potentially remove type assertion after loop opts // makes sure we run ::Value to potentially remove type assertion after loop opts
phase->C->record_for_post_loop_opts_igvn(this); phase->C->record_for_post_loop_opts_igvn(this);
} }
if (!_type->is_int()->empty()) { if (!_range_check_dependency) {
return optimize_integer_cast(phase, T_INT); return optimize_integer_cast(phase, T_INT);
} }
return nullptr; return nullptr;
@ -248,6 +254,13 @@ Node* CastIINode::Identity(PhaseGVN* phase) {
if (progress != this) { if (progress != this) {
return progress; return progress;
} }
if (_range_check_dependency) {
if (phase->C->post_loop_opts_phase()) {
return this->in(1);
} else {
phase->C->record_for_post_loop_opts_igvn(this);
}
}
return this; return this;
} }

View File

@ -3464,10 +3464,6 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f
} }
break; break;
} }
case Op_CastII: {
remove_range_check_cast(n->as_CastII());
}
break;
#ifdef _LP64 #ifdef _LP64
case Op_CmpP: case Op_CmpP:
// Do this transformation here to preserve CmpPNode::sub() and // Do this transformation here to preserve CmpPNode::sub() and
@ -3619,6 +3615,16 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f
#endif #endif
#ifdef ASSERT
case Op_CastII:
// Verify that all range check dependent CastII nodes were removed.
if (n->isa_CastII()->has_range_check()) {
n->dump(3);
assert(false, "Range check dependent CastII node was not removed");
}
break;
#endif
case Op_ModI: case Op_ModI:
if (UseDivMod) { if (UseDivMod) {
// Check if a%b and a/b both exist // Check if a%b and a/b both exist
@ -3627,8 +3633,6 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f
// Replace them with a fused divmod if supported // Replace them with a fused divmod if supported
if (Matcher::has_match_rule(Op_DivModI)) { if (Matcher::has_match_rule(Op_DivModI)) {
DivModINode* divmod = DivModINode::make(n); DivModINode* divmod = DivModINode::make(n);
divmod->add_prec_from(n);
divmod->add_prec_from(d);
d->subsume_by(divmod->div_proj(), this); d->subsume_by(divmod->div_proj(), this);
n->subsume_by(divmod->mod_proj(), this); n->subsume_by(divmod->mod_proj(), this);
} else { } else {
@ -3649,8 +3653,6 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f
// Replace them with a fused divmod if supported // Replace them with a fused divmod if supported
if (Matcher::has_match_rule(Op_DivModL)) { if (Matcher::has_match_rule(Op_DivModL)) {
DivModLNode* divmod = DivModLNode::make(n); DivModLNode* divmod = DivModLNode::make(n);
divmod->add_prec_from(n);
divmod->add_prec_from(d);
d->subsume_by(divmod->div_proj(), this); d->subsume_by(divmod->div_proj(), this);
n->subsume_by(divmod->mod_proj(), this); n->subsume_by(divmod->mod_proj(), this);
} else { } else {
@ -3671,8 +3673,6 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f
// Replace them with a fused unsigned divmod if supported // Replace them with a fused unsigned divmod if supported
if (Matcher::has_match_rule(Op_UDivModI)) { if (Matcher::has_match_rule(Op_UDivModI)) {
UDivModINode* divmod = UDivModINode::make(n); UDivModINode* divmod = UDivModINode::make(n);
divmod->add_prec_from(n);
divmod->add_prec_from(d);
d->subsume_by(divmod->div_proj(), this); d->subsume_by(divmod->div_proj(), this);
n->subsume_by(divmod->mod_proj(), this); n->subsume_by(divmod->mod_proj(), this);
} else { } else {
@ -3693,8 +3693,6 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f
// Replace them with a fused unsigned divmod if supported // Replace them with a fused unsigned divmod if supported
if (Matcher::has_match_rule(Op_UDivModL)) { if (Matcher::has_match_rule(Op_UDivModL)) {
UDivModLNode* divmod = UDivModLNode::make(n); UDivModLNode* divmod = UDivModLNode::make(n);
divmod->add_prec_from(n);
divmod->add_prec_from(d);
d->subsume_by(divmod->div_proj(), this); d->subsume_by(divmod->div_proj(), this);
n->subsume_by(divmod->mod_proj(), this); n->subsume_by(divmod->mod_proj(), this);
} else { } else {
@ -3896,34 +3894,6 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f
} }
} }
void Compile::remove_range_check_cast(CastIINode* cast) {
if (cast->has_range_check()) {
// Range check CastII nodes feed into an address computation subgraph. Remove them to let that subgraph float freely.
// For memory access or integer divisions nodes that depend on the cast, record the dependency on the cast's control
// as a precedence edge, so they can't float above the cast in case that cast's narrowed type helped eliminate a
// range check or a null divisor check.
assert(cast->in(0) != nullptr, "All RangeCheck CastII must have a control dependency");
ResourceMark rm;
Unique_Node_List wq;
wq.push(cast);
for (uint next = 0; next < wq.size(); ++next) {
Node* m = wq.at(next);
for (DUIterator_Fast imax, i = m->fast_outs(imax); i < imax; i++) {
Node* use = m->fast_out(i);
if (use->is_Mem() || use->is_div_or_mod(T_INT) || use->is_div_or_mod(T_LONG)) {
use->ensure_control_or_add_prec(cast->in(0));
} else if (!use->is_CFG() && !use->is_Phi()) {
wq.push(use);
}
}
}
cast->subsume_by(cast->in(1), this);
if (cast->outcnt() == 0) {
cast->disconnect_inputs(this);
}
}
}
//------------------------------final_graph_reshaping_walk--------------------- //------------------------------final_graph_reshaping_walk---------------------
// Replacing Opaque nodes with their input in final_graph_reshaping_impl(), // Replacing Opaque nodes with their input in final_graph_reshaping_impl(),
// requires that the walk visits a node's inputs before visiting the node. // requires that the walk visits a node's inputs before visiting the node.

View File

@ -53,7 +53,6 @@ class Block;
class Bundle; class Bundle;
class CallGenerator; class CallGenerator;
class CallStaticJavaNode; class CallStaticJavaNode;
class CastIINode;
class CloneMap; class CloneMap;
class CompilationFailureInfo; class CompilationFailureInfo;
class ConnectionGraph; class ConnectionGraph;
@ -1315,8 +1314,6 @@ private:
BasicType out_bt, BasicType in_bt); BasicType out_bt, BasicType in_bt);
static Node* narrow_value(BasicType bt, Node* value, const Type* type, PhaseGVN* phase, bool transform_res); static Node* narrow_value(BasicType bt, Node* value, const Type* type, PhaseGVN* phase, bool transform_res);
void remove_range_check_cast(CastIINode* cast);
}; };
#endif // SHARE_OPTO_COMPILE_HPP #endif // SHARE_OPTO_COMPILE_HPP

View File

@ -2878,15 +2878,6 @@ void Node::ensure_control_or_add_prec(Node* c) {
} }
} }
void Node::add_prec_from(Node* n) {
for (uint i = n->req(); i < n->len(); i++) {
Node* prec = n->in(i);
if (prec != nullptr) {
add_prec(prec);
}
}
}
bool Node::is_dead_loop_safe() const { bool Node::is_dead_loop_safe() const {
if (is_Phi()) { if (is_Phi()) {
return true; return true;
@ -2910,9 +2901,6 @@ bool Node::is_dead_loop_safe() const {
return false; return false;
} }
bool Node::is_div_or_mod(BasicType bt) const { return Opcode() == Op_Div(bt) || Opcode() == Op_Mod(bt) ||
Opcode() == Op_UDiv(bt) || Opcode() == Op_UMod(bt); }
//============================================================================= //=============================================================================
//------------------------------yank------------------------------------------- //------------------------------yank-------------------------------------------
// Find and remove // Find and remove

View File

@ -1143,7 +1143,6 @@ public:
// Set control or add control as precedence edge // Set control or add control as precedence edge
void ensure_control_or_add_prec(Node* c); void ensure_control_or_add_prec(Node* c);
void add_prec_from(Node* n);
// Visit boundary uses of the node and apply a callback function for each. // Visit boundary uses of the node and apply a callback function for each.
// Recursively traverse uses, stopping and applying the callback when // Recursively traverse uses, stopping and applying the callback when
@ -1255,8 +1254,6 @@ public:
// Whether this is a memory phi node // Whether this is a memory phi node
bool is_memory_phi() const { return is_Phi() && bottom_type() == Type::MEMORY; } bool is_memory_phi() const { return is_Phi() && bottom_type() == Type::MEMORY; }
bool is_div_or_mod(BasicType bt) const;
//----------------- Printing, etc //----------------- Printing, etc
#ifndef PRODUCT #ifndef PRODUCT
public: public:
@ -2026,10 +2023,6 @@ Op_IL(URShift)
Op_IL(LShift) Op_IL(LShift)
Op_IL(Xor) Op_IL(Xor)
Op_IL(Cmp) Op_IL(Cmp)
Op_IL(Div)
Op_IL(Mod)
Op_IL(UDiv)
Op_IL(UMod)
inline int Op_ConIL(BasicType bt) { inline int Op_ConIL(BasicType bt) {
assert(bt == T_INT || bt == T_LONG, "only for int or longs"); assert(bt == T_INT || bt == T_LONG, "only for int or longs");

View File

@ -1,474 +0,0 @@
/*
* Copyright (c) 2024, Red Hat, Inc. 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.
*/
/**
* @test
* @bug 8324517
* @summary C2: out of bound array load because of dependency on removed range check CastIIs
*
* @run main/othervm -XX:-TieredCompilation -XX:-UseOnStackReplacement -XX:-BackgroundCompilation
* -XX:CompileCommand=dontinline,TestArrayAccessAboveRCAfterRCCastIIEliminated::notInlined
* TestArrayAccessAboveRCAfterRCCastIIEliminated
* @run main/othervm -XX:-TieredCompilation -XX:-UseOnStackReplacement -XX:-BackgroundCompilation
* -XX:CompileCommand=dontinline,TestArrayAccessAboveRCAfterRCCastIIEliminated::notInlined
* -XX:+UnlockDiagnosticVMOptions -XX:+StressGCM TestArrayAccessAboveRCAfterRCCastIIEliminated
* @run main/othervm TestArrayAccessAboveRCAfterRCCastIIEliminated
* @run main/othervm -XX:CompileCommand=dontinline,TestArrayAccessAboveRCAfterRCCastIIEliminated::notInlined
* TestArrayAccessAboveRCAfterRCCastIIEliminated
*
*/
public class TestArrayAccessAboveRCAfterRCCastIIEliminated {
private static int intField;
private static long longField;
private static volatile int volatileField;
public static void main(String[] args) {
int[] array = new int[100];
for (int i = 0; i < 20_000; i++) {
test1(9, 10, 1, true);
test1(9, 10, 1, false);
test2(9, 10, 1, true);
test2(9, 10, 1, false);
test3(9, 10, 1, true);
test3(9, 10, 1, false);
test4(9, 10, 1, true);
test4(9, 10, 1, false);
test5(9, 10, 1, true);
test5(9, 10, 1, false);
test6(9, 10, 1, true);
test6(9, 10, 1, false);
test7(9, 10, 1, true);
test7(9, 10, 1, false);
test8(9, 10, 1, true);
test8(9, 10, 1, false);
test9(9, 10, 1, true);
test9(9, 10, 1, false);
test10(9, 10, 1, true);
test10(9, 10, 1, false);
test11(9, 10, 1, true);
test11(9, 10, 1, false);
test12(9, 10, 1, true);
test12(9, 10, 1, false);
test13(9, 10, 1, true);
test13(9, 10, 1, false);
}
try {
test1(-1, 10, 1, true);
} catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) {
}
try {
test2(-1, 10, 1, true);
} catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) {
}
try {
test3(-1, 10, 1, true);
} catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) {
}
try {
test4(-1, 10, 1, true);
} catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) {
}
try {
test5(-1, 10, 1, true);
} catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) {
}
try {
test6(-1, 10, 1, true);
} catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) {
}
try {
test7(-1, 10, 1, true);
} catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) {
}
try {
test8(-1, 10, 1, true);
} catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) {
}
try {
test9(-1, 10, 1, true);
} catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) {
}
try {
test10(-1, 10, 1, true);
} catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) {
}
try {
test11(-1, 10, 1, true);
} catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) {
}
try {
test12(-1, 10, 1, true);
} catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) {
}
try {
test13(-1, 10, 1, true);
} catch (ArrayIndexOutOfBoundsException arrayIndexOutOfBoundsException) {
}
}
private static void test1(int i, int j, int flag, boolean flag2) {
i = Math.min(i, 9);
int[] array = new int[10];
notInlined(array);
if (flag == 0) {
}
if (flag2) {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
intField = array[otherArray.length];
} else {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
intField = array[otherArray.length];
}
for (int k = 0; k < 10; k++) {
}
}
private static void test2(int i, int j, int flag, boolean flag2) {
i = Math.min(i, 9);
int[] array = new int[10];
notInlined(array);
if (flag == 0) {
}
if (flag2) {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
intField = 1 / (otherArray.length + 1);
} else {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
intField = 1 / (otherArray.length + 1);
}
for (int k = 0; k < 10; k++) {
}
}
private static void test3(int i, int j, int flag, boolean flag2) {
i = Math.min(i, 9);
int[] array = new int[10];
notInlined(array);
if (flag == 0) {
}
if (flag2) {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
longField = 1L / (otherArray.length + 1);
} else {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
longField = 1L / (otherArray.length + 1);
}
for (int k = 0; k < 10; k++) {
}
}
private static void test4(int i, int j, int flag, boolean flag2) {
i = Math.min(i, 9);
int[] array = new int[10];
notInlined(array);
if (flag == 0) {
}
if (flag2) {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
intField = 1 % (otherArray.length + 1);
} else {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
intField = 1 % (otherArray.length + 1);
}
for (int k = 0; k < 10; k++) {
}
}
private static void test5(int i, int j, int flag, boolean flag2) {
i = Math.min(i, 9);
int[] array = new int[10];
notInlined(array);
if (flag == 0) {
}
if (flag2) {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
longField = 1L % (otherArray.length + 1);
} else {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
longField = 1L % (otherArray.length + 1);
}
for (int k = 0; k < 10; k++) {
}
}
private static void test6(int i, int j, int flag, boolean flag2) {
i = Math.min(i, 9);
int[] array = new int[10];
notInlined(array);
if (flag == 0) {
}
if (flag2) {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
intField = 1 % (otherArray.length + 1) + 1 / (otherArray.length + 1);
} else {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
intField = 1 % (otherArray.length + 1) + 1 / (otherArray.length + 1);
}
for (int k = 0; k < 10; k++) {
}
}
private static void test7(int i, int j, int flag, boolean flag2) {
i = Math.min(i, 9);
int[] array = new int[10];
notInlined(array);
if (flag == 0) {
}
if (flag2) {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
longField = 1L % (otherArray.length + 1) + 1L / (otherArray.length + 1);
} else {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
longField = 1L % (otherArray.length + 1) + 1L / (otherArray.length + 1);
}
for (int k = 0; k < 10; k++) {
}
}
private static void test8(int i, int j, int flag, boolean flag2) {
i = Math.min(i, 9);
int[] array = new int[10];
notInlined(array);
if (flag == 0) {
}
if (flag2) {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
intField = Integer.divideUnsigned(1, (otherArray.length + 1));
} else {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
intField = Integer.divideUnsigned(1, (otherArray.length + 1));
}
for (int k = 0; k < 10; k++) {
}
}
private static void test9(int i, int j, int flag, boolean flag2) {
i = Math.min(i, 9);
int[] array = new int[10];
notInlined(array);
if (flag == 0) {
}
if (flag2) {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
longField = Long.divideUnsigned(1L, (otherArray.length + 1));
} else {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
longField = Long.divideUnsigned(1L, (otherArray.length + 1));
}
for (int k = 0; k < 10; k++) {
}
}
private static void test10(int i, int j, int flag, boolean flag2) {
i = Math.min(i, 9);
int[] array = new int[10];
notInlined(array);
if (flag == 0) {
}
if (flag2) {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
intField = Integer.remainderUnsigned(1, (otherArray.length + 1));
} else {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
intField = Integer.remainderUnsigned(1, (otherArray.length + 1));
}
for (int k = 0; k < 10; k++) {
}
}
private static void test11(int i, int j, int flag, boolean flag2) {
i = Math.min(i, 9);
int[] array = new int[10];
notInlined(array);
if (flag == 0) {
}
if (flag2) {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
longField = Long.remainderUnsigned(1L, (otherArray.length + 1));
} else {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
longField = Long.remainderUnsigned(1L, (otherArray.length + 1));
}
for (int k = 0; k < 10; k++) {
}
}
private static void test12(int i, int j, int flag, boolean flag2) {
i = Math.min(i, 9);
int[] array = new int[10];
notInlined(array);
if (flag == 0) {
}
if (flag2) {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
intField = Integer.divideUnsigned(1, (otherArray.length + 1)) +
Integer.remainderUnsigned(1, (otherArray.length + 1));
} else {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
intField = Integer.divideUnsigned(1, (otherArray.length + 1)) +
Integer.remainderUnsigned(1, (otherArray.length + 1));
}
for (int k = 0; k < 10; k++) {
}
}
private static void test13(int i, int j, int flag, boolean flag2) {
i = Math.min(i, 9);
int[] array = new int[10];
notInlined(array);
if (flag == 0) {
}
if (flag2) {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
longField = Long.remainderUnsigned(1L, (otherArray.length + 1)) +
Long.divideUnsigned(1L, (otherArray.length + 1));
} else {
float[] newArray = new float[j];
newArray[i] = 42;
float[] otherArray = new float[i];
if (flag == 0) {
}
longField = Long.remainderUnsigned(1L, (otherArray.length + 1)) +
Long.divideUnsigned(1L, (otherArray.length + 1));
}
for (int k = 0; k < 10; k++) {
}
}
private static void notInlined(int[] array) {
}
}