8299259: C2: Div/Mod nodes without zero check could be split through …

…iv phi of loop resulting in SIGFPE

Reviewed-by: roland
Backport-of: 8b0133f2760f67cd968528c041a600408cc26978

src/hotspot/share/opto/loopnode.hpp

@@ -1344,6 +1344,9 @@ SHENANDOAHGC_ONLY(public:)
   bool identical_backtoback_ifs(Node *n);
   bool can_split_if(Node *n_ctrl);
 SHENANDOAHGC_ONLY(private:)
+  bool cannot_split_division(const Node* n, const Node* region) const;
+  static bool is_divisor_counted_loop_phi(const Node* divisor, const Node* loop);
+  bool loop_phi_backedge_type_contains_zero(const Node* phi_divisor, const Type* zero) const;
 
   // Clone loop predicates to slow and fast loop when unswitching a loop
   void clone_predicates_to_unswitched_loop(IdealLoopTree* loop, const Node_List& old_new, ProjNode*& iffast_pred, ProjNode*& ifslow_pred);

src/hotspot/share/opto/loopopts.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2023, 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
@@ -63,19 +63,8 @@ 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;
-      }
-    }
+  if (cannot_split_division(n, region)) {
+    return NULL;
   }
 
   int wins = 0;
@@ -227,6 +216,42 @@ Node* PhaseIdealLoop::split_thru_phi(Node* n, Node* region, int policy) {
   return phi;
 }
 
+// Return true if 'n' is a Div or Mod node (without zero check If node which was removed earlier) with a loop phi divisor
+// of a trip-counted (integer or long) loop with a backedge input that could be zero (include zero in its type range). In
+// this case, we cannot split the division to the backedge as it could freely float above the loop exit check resulting in
+// a division by zero. This situation is possible because the type of an increment node of an iv phi (trip-counter) could
+// include zero while the iv phi does not (see PhiNode::Value() for trip-counted loops where we improve types of iv phis).
+// We also need to check other loop phis as they could have been created in the same split-if pass when applying
+// PhaseIdealLoop::split_thru_phi() to split nodes through an iv phi.
+bool PhaseIdealLoop::cannot_split_division(const Node* n, const Node* region) const {
+  const Type* zero;
+  switch (n->Opcode()) {
+    case Op_DivI:
+    case Op_ModI:
+      zero = TypeInt::ZERO;
+      break;
+    case Op_DivL:
+    case Op_ModL:
+      zero = TypeLong::ZERO;
+      break;
+    default:
+      return false;
+  }
+
+  assert(n->in(0) == NULL, "divisions with zero check should already have bailed out earlier in split-if");
+  Node* divisor = n->in(2);
+  return is_divisor_counted_loop_phi(divisor, region) &&
+         loop_phi_backedge_type_contains_zero(divisor, zero);
+}
+
+bool PhaseIdealLoop::is_divisor_counted_loop_phi(const Node* divisor, const Node* loop) {
+  return loop->is_CountedLoop() && divisor->is_Phi() && divisor->in(0) == loop;
+}
+
+bool PhaseIdealLoop::loop_phi_backedge_type_contains_zero(const Node* phi_divisor, const Type* zero) const {
+    return _igvn.type(phi_divisor->in(LoopNode::LoopBackControl))->filter_speculative(zero) != Type::TOP;
+}
+
 //------------------------------dominated_by------------------------------------
 // Replace the dominated test with an obvious true or false.  Place it on the
 // IGVN worklist for later cleanup.  Move control-dependent data Nodes on the

test/hotspot/jtreg/compiler/splitif/TestSplitDivisionThroughPhi.java

@@ -0,0 +1,149 @@
+/*
+ * Copyright (c) 2023, 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
+* @bug 8299259
+* @requires vm.compiler2.enabled
+* @summary Test various cases of divisions/modulo which should not be split through iv phis.
+* @run main/othervm -Xbatch -XX:+UnlockDiagnosticVMOptions -XX:LoopUnrollLimit=0 -XX:+StressGCM
+*                   -XX:CompileCommand=compileonly,compiler.splitif.TestSplitDivisionThroughPhi::*
+*                   compiler.splitif.TestSplitDivisionThroughPhi
+*/
+
+package compiler.splitif;
+
+public class TestSplitDivisionThroughPhi {
+    static int iFld;
+    static long lFld;
+    static boolean flag;
+
+
+    public static void main(String[] strArr) {
+        for (int i = 0; i < 5000; i++) {
+            testPushDivIThruPhi();
+            testPushDivIThruPhiInChain();
+            testPushModIThruPhi();
+            testPushModIThruPhiInChain();
+            testPushDivLThruPhi();
+            testPushDivLThruPhiInChain();
+            testPushModLThruPhi();
+            testPushModLThruPhiInChain();
+        }
+    }
+
+    // Already fixed by JDK-8248552.
+    static void testPushDivIThruPhi() {
+        for (int i = 10; i > 1; i -= 2) {
+            // The Div node is only split in later loop opts phase because the zero divisor check is only removed
+            // in IGVN after the first loop opts phase.
+            //
+            // iv phi i type: [2..10]
+            // When splitting the DivI through the iv phi, it ends up on the back edge with the trip count decrement
+            // as input which has type [0..8]. We end up executing a division by zero on the last iteration because
+            // the DivI it is not pinned to the loop exit test and can freely float above the loop exit check.
+            iFld = 10 / i;
+        }
+    }
+
+   // Same as above but with an additional Mul node between the iv phi and the Div node. Both nodes are split through
+   // the iv phi in one pass of Split If.
+    static void testPushDivIThruPhiInChain() {
+        for (int i = 10; i > 1; i -= 2) {
+            // Empty one iteration loop which is only removed after split if in first loop opts phase. This prevents
+            // that the Mul node is already split through the iv phi while the Div node cannot be split yet due to
+            // the zero divisor check which can only be removed in the IGVN after the first loop opts pass.
+            for (int j = 0; j < 1; j++) {
+            }
+            iFld = 10 / (i * 100);
+        }
+    }
+
+    // Already fixed by JDK-8248552.
+    static void testPushModIThruPhi() {
+        for (int i = 10; i > 1; i -= 2) {
+            iFld = 10 / i;
+        }
+    }
+
+    // Same as above but with ModI.
+    static void testPushModIThruPhiInChain() {
+        for (int i = 10; i > 1; i -= 2) {
+            for (int j = 0; j < 1; j++) {
+            }
+            iFld = 10 / (i * 100);
+        }
+    }
+
+    // Long cases only trigger since JDK-8256655.
+
+    // Same as above but with DivL.
+    static void testPushDivLThruPhi() {
+        for (long i = 10; i > 1; i -= 2) {
+            lFld = 10L / i;
+
+            // Loop that is not removed such that we do not transform the outer LongCountedLoop (only done if innermost)
+            for (int j = 0; j < 10; j++) {
+                flag = !flag;
+            }
+        }
+    }
+
+    // Same as above but with DivL.
+    static void testPushDivLThruPhiInChain() {
+        for (long i = 10; i > 1; i -= 2) {
+            for (int j = 0; j < 1; j++) {
+            }
+            lFld = 10L / (i * 100L);
+
+            for (int j = 0; j < 10; j++) {
+                flag = !flag;
+            }
+        }
+    }
+
+    // Same as above but with ModL
+    static void testPushModLThruPhi() {
+        for (long i = 10; i > 1; i -= 2) {
+            lFld = 10L % i;
+
+            for (int j = 0; j < 10; j++) {
+                flag = !flag;
+            }
+        }
+    }
+
+    // Same as above but with ModL
+    static void testPushModLThruPhiInChain() {
+        for (long i = 10; i > 1; i -= 2) {
+            for (int j = 0; j < 1; j++) {
+            }
+            lFld = 10L % (i * 100L);
+
+            for (int j = 0; j < 10; j++) {
+                flag = !flag;
+            }
+        }
+    }
+}