Skip to content
Permalink
Browse files
8248552: C2 crashes with SIGFPE due to division by zero
Bail out in PhaseIdealLoop:split_thru_phi when trying to split a Div or ModNode iv phi whose zero check was removed but could potentially still be zero based on type information.

Backport-of: 417e8e449df569f593b82ecd640fa01b6b762832
  • Loading branch information
Olga Mikhaltsova authored and Yuri Nesterenko committed Apr 9, 2021
1 parent 6cf1c0d commit 737889212d5b0434ea3c9b0633d25513001dcb0e
@@ -46,7 +46,7 @@
//=============================================================================
//------------------------------split_thru_phi---------------------------------
// Split Node 'n' through merge point if there is enough win.
Node *PhaseIdealLoop::split_thru_phi( Node *n, Node *region, int policy ) {
Node* PhaseIdealLoop::split_thru_phi(Node* n, Node* region, int policy) {
if (n->Opcode() == Op_ConvI2L && n->bottom_type() != TypeLong::LONG) {
// ConvI2L may have type information on it which is unsafe to push up
// so disable this for now
@@ -61,13 +61,28 @@ Node *PhaseIdealLoop::split_thru_phi( Node *n, Node *region, int policy ) {
return NULL;
}

// Bail out if 'n' is a Div or Mod node whose zero check was removed earlier (i.e. control is NULL) and its divisor is an induction variable
// phi p of a trip-counted (integer) loop whose inputs could be zero (include zero in their type range). p could have a more precise type
// range that does not necessarily include all values of its inputs. Since each of these inputs will be a divisor of the newly cloned nodes
// of 'n', we need to bail out of one of these divisors could be zero (zero in its type range).
if ((n->Opcode() == Op_DivI || n->Opcode() == Op_ModI) && n->in(0) == NULL
&& region->is_CountedLoop() && n->in(2) == region->as_CountedLoop()->phi()) {
Node* phi = region->as_CountedLoop()->phi();
for (uint i = 1; i < phi->req(); i++) {
if (_igvn.type(phi->in(i))->filter_speculative(TypeInt::ZERO) != Type::TOP) {
// Zero could be a possible value but we already removed the zero check. Bail out to avoid a possible division by zero at a later point.
return NULL;
}
}
}

int wins = 0;
assert(!n->is_CFG(), "");
assert(region->is_Region(), "");

const Type* type = n->bottom_type();
const TypeOopPtr *t_oop = _igvn.type(n)->isa_oopptr();
Node *phi;
const TypeOopPtr* t_oop = _igvn.type(n)->isa_oopptr();
Node* phi;
if (t_oop != NULL && t_oop->is_known_instance_field()) {
int iid = t_oop->instance_id();
int index = C->get_alias_index(t_oop);
@@ -78,7 +93,7 @@ Node *PhaseIdealLoop::split_thru_phi( Node *n, Node *region, int policy ) {
}
uint old_unique = C->unique();
for (uint i = 1; i < region->req(); i++) {
Node *x;
Node* x;
Node* the_clone = NULL;
if (region->in(i) == C->top()) {
x = C->top(); // Dead path? Use a dead data op
@@ -89,13 +104,13 @@ Node *PhaseIdealLoop::split_thru_phi( Node *n, Node *region, int policy ) {
if (n->in(0) == region)
x->set_req( 0, region->in(i) );
for (uint j = 1; j < n->req(); j++) {
Node *in = n->in(j);
Node* in = n->in(j);
if (in->is_Phi() && in->in(0) == region)
x->set_req( j, in->in(i) ); // Use pre-Phi input for the clone
x->set_req(j, in->in(i)); // Use pre-Phi input for the clone
}
}
// Check for a 'win' on some paths
const Type *t = x->Value(&_igvn);
const Type* t = x->Value(&_igvn);

bool singleton = t->singleton();

@@ -0,0 +1,95 @@
/*
* Copyright (c) 2020, 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
* @bug 8248552
* @summary A Division/modulo node whose zero check was removed is split through an induction variable phi and executed before
* the loop limit check resulting in a SIGFPE because the divisor is zero.
*
* @run main/othervm -XX:CompileCommand=dontinline,compiler.c2.loopopts.TestSplitThruPhiDivMod::test* compiler.c2.loopopts.TestSplitThruPhiDivMod
*/
package compiler.c2.loopopts;

public class TestSplitThruPhiDivMod {

int x;

public int testMod() {
int i1 = 2;
for (int i = 5; i < 25; i++) {
for (int j = 50; j > 1; j -= 2) {
/*
* Zero check is removed based on the type of the induction variable phi (variable j) since its always between 1 and 50.
* However, when splitting the modulo node through the phi, it can be executed right after the subtraction j-2 which can be
* 0 before evaluation the loop limit condition in the last iteration when j is 2: j-2 = 2-2 = 0. This results in a SIGFPE.
* The fix is to not split a division or modulo node 'n' through the induction variable phi if the zero check was removed
* earlier and the new inputs of the clones of 'n' after the split could be zero (i.e. the type of the clones of 'n' include 0).
*/
x = (20 % j); // Problematic division as part of modulo. Results in a SIGFPE, even though j is always non-zero.
i1 = (i1 / i);
for (int k = 3; k > 1; k--) {
switch ((i % 4) + 22) {
case 22:
switch (j % 10) {
case 83:
x += 5;
break;
}
}
}
}
}
return i1;
}

public int testDiv() {
int i1 = 2;
for (int i = 5; i < 25; i++) {
for (int j = 50; j > 1; j -= 2) {
// Same issue as above but with a division node. See explanation above.
x = (20 / j); // Problematic division. Results in a SIGFPE, even though j is always non-zero.
i1 = (i1 / i);
for (int k = 3; k > 1; k--) {
switch ((i % 4) + 22) {
case 22:
switch (j % 10) {
case 83:
x += 5;
break;
}
}
}
}
}
return i1;
}

public static void main(String[] strArr) {
TestSplitThruPhiDivMod t = new TestSplitThruPhiDivMod();
for (int i = 0; i < 10000; i++) {
t.testDiv();
t.testMod();
}
}
}

1 comment on commit 7378892

@openjdk-notifier

This comment has been minimized.

Copy link

@openjdk-notifier openjdk-notifier bot commented on 7378892 Apr 9, 2021

Please sign in to comment.