From c29fd2470626506e8801414e028230665d502c85 Mon Sep 17 00:00:00 2001 From: Christian Hagedorn Date: Mon, 5 Jul 2021 14:23:48 +0200 Subject: [PATCH 1/2] 8269795: C2: Out of bounds array load floats above its range check in loop peeling resulting in SEGV --- src/hotspot/share/opto/loopTransform.cpp | 17 +++-- .../TestPeelingRemoveDominatedTest.java | 67 +++++++++++++++++++ 2 files changed, 75 insertions(+), 9 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/loopopts/TestPeelingRemoveDominatedTest.java diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp index 4cebd27c0c9..e0c07dceebd 100644 --- a/src/hotspot/share/opto/loopTransform.cpp +++ b/src/hotspot/share/opto/loopTransform.cpp @@ -507,24 +507,24 @@ uint IdealLoopTree::estimate_peeling(PhaseIdealLoop *phase) { // If we got the effect of peeling, either by actually peeling or by making // a pre-loop which must execute at least once, we can remove all // loop-invariant dominated tests in the main body. -void PhaseIdealLoop::peeled_dom_test_elim(IdealLoopTree *loop, Node_List &old_new) { +void PhaseIdealLoop::peeled_dom_test_elim(IdealLoopTree* loop, Node_List& old_new) { bool progress = true; while (progress) { - progress = false; // Reset for next iteration - Node *prev = loop->_head->in(LoopNode::LoopBackControl);//loop->tail(); - Node *test = prev->in(0); + progress = false; // Reset for next iteration + Node* prev = loop->_head->in(LoopNode::LoopBackControl); // loop->tail(); + Node* test = prev->in(0); while (test != loop->_head) { // Scan till run off top of loop - int p_op = prev->Opcode(); if ((p_op == Op_IfFalse || p_op == Op_IfTrue) && - test->is_If() && // Test? + test->is_If() && // Test? !test->in(1)->is_Con() && // And not already obvious? // Condition is not a member of this loop? !loop->is_member(get_loop(get_ctrl(test->in(1))))){ // Walk loop body looking for instances of this test + Node* test_cond = test->in(1); for (uint i = 0; i < loop->_body.size(); i++) { - Node *n = loop->_body.at(i); - if (n->is_If() && n->in(1) == test->in(1) /*&& n != loop->tail()->in(0)*/) { + Node* n = loop->_body.at(i); + if (n->is_If() && n->in(1) == test_cond) { // IfNode was dominated by version in peeled loop body progress = true; dominated_by(old_new[prev->_idx], n); @@ -534,7 +534,6 @@ void PhaseIdealLoop::peeled_dom_test_elim(IdealLoopTree *loop, Node_List &old_ne prev = test; test = idom(test); } // End of scan tests in loop - } // End of while (progress) } diff --git a/test/hotspot/jtreg/compiler/loopopts/TestPeelingRemoveDominatedTest.java b/test/hotspot/jtreg/compiler/loopopts/TestPeelingRemoveDominatedTest.java new file mode 100644 index 00000000000..800adb39caa --- /dev/null +++ b/test/hotspot/jtreg/compiler/loopopts/TestPeelingRemoveDominatedTest.java @@ -0,0 +1,67 @@ +/* + * Copyright (c) 2021, Oracle and/or its affiliates. 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 + * @key stress randomness + * @requires vm.compiler2.enabled + * @bug 8269795 + * @summary PhaseIdealLoop::peeled_dom_test_elim wrongly moves a non-dominated test out of a loop together with control dependent data nodes. + * This results in a crash due to an out of bounds read of an array. + * @run main/othervm -XX:+UnlockDiagnosticVMOptions -Xcomp -XX:-TieredCompilation -XX:+StressGCM + * -XX:CompileCommand=compileonly,compiler.loopopts.TestPeelingRemoveDominatedTest compiler.loopopts.TestPeelingRemoveDominatedTest + */ + +package compiler.loopopts; + +public class TestPeelingRemoveDominatedTest { + public static int N = 400; + static boolean bFld = true; + static int iArrFld[] = new int[N]; + + public static void main(String[] strArr) { + TestPeelingRemoveDominatedTest _instance = new TestPeelingRemoveDominatedTest(); + for (int i = 0; i < 10; i++) { + _instance.mainTest(); + } + } + + public void mainTest() { + vMeth(); + } + + + static void vMeth() { + iArrFld[1] = 2; + int i6 = 2; + while (--i6 > 0) { + try { + int i3 = (iArrFld[i6 - 1] / 56); + iArrFld[1] = (-139 % i3); + } catch (ArithmeticException a_e) { + } + if (bFld) { + } + } + } +} From bd9210369848047419bd794db755b107b8aeaf83 Mon Sep 17 00:00:00 2001 From: Christian Hagedorn Date: Mon, 12 Jul 2021 15:33:05 +0200 Subject: [PATCH 2/2] review comments by Vladimir K --- src/hotspot/share/opto/loopTransform.cpp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/hotspot/share/opto/loopTransform.cpp b/src/hotspot/share/opto/loopTransform.cpp index e0c07dceebd..9a6223de052 100644 --- a/src/hotspot/share/opto/loopTransform.cpp +++ b/src/hotspot/share/opto/loopTransform.cpp @@ -515,15 +515,20 @@ void PhaseIdealLoop::peeled_dom_test_elim(IdealLoopTree* loop, Node_List& old_ne Node* test = prev->in(0); while (test != loop->_head) { // Scan till run off top of loop int p_op = prev->Opcode(); - if ((p_op == Op_IfFalse || p_op == Op_IfTrue) && - test->is_If() && // Test? - !test->in(1)->is_Con() && // And not already obvious? - // Condition is not a member of this loop? - !loop->is_member(get_loop(get_ctrl(test->in(1))))){ + assert(test != NULL, "test cannot be NULL"); + Node* test_cond = NULL; + if ((p_op == Op_IfFalse || p_op == Op_IfTrue) && test->is_If()) { + test_cond = test->in(1); + } + if (test_cond != NULL && // Test? + !test_cond->is_Con() && // And not already obvious? + // And condition is not a member of this loop? + !loop->is_member(get_loop(get_ctrl(test_cond)))) { // Walk loop body looking for instances of this test - Node* test_cond = test->in(1); for (uint i = 0; i < loop->_body.size(); i++) { Node* n = loop->_body.at(i); + // Check against cached test condition because dominated_by() + // replaces the test condition with a constant. if (n->is_If() && n->in(1) == test_cond) { // IfNode was dominated by version in peeled loop body progress = true;