8336729: C2: Div/Mod nodes without zero check could be split through iv phi of outer loop of long counted loop nest resulting in SIGFPE

Co-authored-by: Emanuel Peter <epeter@openjdk.org>
Reviewed-by: epeter, kvn, thartmann

src/hotspot/share/opto/loopnode.hpp

@@ -1578,7 +1578,7 @@ private:
   bool identical_backtoback_ifs(Node *n);
   bool can_split_if(Node *n_ctrl);
   bool cannot_split_division(const Node* n, const Node* region) const;
-  static bool is_divisor_counted_loop_phi(const Node* divisor, const Node* loop);
+  static bool is_divisor_loop_phi(const Node* divisor, const Node* loop);
   bool loop_phi_backedge_type_contains_zero(const Node* phi_divisor, const Type* zero) const;
 
   // Determine if a method is too big for a/another round of split-if, based on

src/hotspot/share/opto/loopopts.cpp

@@ -295,12 +295,12 @@ bool PhaseIdealLoop::cannot_split_division(const Node* n, const Node* region) co
   }
 
   Node* divisor = n->in(2);
-  return is_divisor_counted_loop_phi(divisor, region) &&
+  return is_divisor_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) {
+bool PhaseIdealLoop::is_divisor_loop_phi(const Node* divisor, const Node* loop) {
-  return loop->is_BaseCountedLoop() && divisor->is_Phi() && divisor->in(0) == loop;
+  return loop->is_Loop() && divisor->is_Phi() && divisor->in(0) == loop;
 }
 
 bool PhaseIdealLoop::loop_phi_backedge_type_contains_zero(const Node* phi_divisor, const Type* zero) const {

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2023, 2024, 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
@@ -24,7 +24,7 @@
 /**
 * @test
 * @key stress randomness
-* @bug 8299259
+* @bug 8299259 8336729
 * @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:StressSeed=884154126
@@ -35,7 +35,7 @@
 /**
 * @test
 * @key stress randomness
-* @bug 8299259
+* @bug 8299259 8336729
 * @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
@@ -43,6 +43,28 @@
 *                   compiler.splitif.TestSplitDivisionThroughPhi
 */
 
+/**
+ * @test
+ * @key stress randomness
+ * @bug 8336729
+ * @requires vm.compiler2.enabled
+ * @summary Test various cases of divisions/modulo which should not be split through iv phis.
+ * @run main/othervm -Xcomp -XX:+UnlockDiagnosticVMOptions -XX:LoopUnrollLimit=0 -XX:+StressGCM -XX:StressSeed=3434
+ *                   -XX:CompileCommand=compileonly,compiler.splitif.TestSplitDivisionThroughPhi::*
+ *                   compiler.splitif.TestSplitDivisionThroughPhi
+ */
+
+/**
+ * @test
+ * @key stress randomness
+ * @bug 8336729
+ * @requires vm.compiler2.enabled
+ * @summary Test various cases of divisions/modulo which should not be split through iv phis.
+ * @run main/othervm -Xcomp -XX:+UnlockDiagnosticVMOptions -XX:LoopUnrollLimit=0 -XX:+StressGCM
+ *                   -XX:CompileCommand=compileonly,compiler.splitif.TestSplitDivisionThroughPhi::*
+ *                   compiler.splitif.TestSplitDivisionThroughPhi
+ */
+
 package compiler.splitif;
 
 public class TestSplitDivisionThroughPhi {
@@ -61,6 +83,8 @@ public class TestSplitDivisionThroughPhi {
             testPushDivLThruPhiInChain();
             testPushModLThruPhi();
             testPushModLThruPhiInChain();
+            testPushDivLThruPhiForOuterLongLoop();
+            testPushModLThruPhiForOuterLongLoop();
         }
     }
 
@@ -78,6 +102,27 @@ public class TestSplitDivisionThroughPhi {
         }
     }
 
+    // Fixed with JDK-8336729.
+    static void testPushDivLThruPhiForOuterLongLoop() {
+        // This loop is first transformed into a LongCountedLoop in the first loop opts phase.
+        // In the second loop opts phase, the LongCountedLoop is split into an inner and an outer loop. Both get the
+        // same iv phi type which is [2..10]. Only the inner loop is transformed into a CountedLoopNode while the outer
+        // loop is still a LoopNode. We run into the same problem as described in testPushDivIThruPhi() when splitting
+        // the DivL node through the long iv phi of the outer LoopNode.
+        // The fix for JDK-8299259 only prevents this splitting for CountedLoopNodes. We now extend it to LoopNodes
+        // in general.
+        for (long i = 10; i > 1; i -= 2) {
+            lFld = 10 / i;
+        }
+    }
+
+    // Same as testPushDivLThruPhiForOuterLongLoop() but for ModL.
+    static void testPushModLThruPhiForOuterLongLoop() {
+        for (int i = 10; i > 1; i -= 2) {
+            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() {