8229496: SIGFPE (division by zero) in C2 OSR compiled method
authorthartmann
Thu, 05 Sep 2019 13:56:17 +0200
changeset 58019 86b95fc6ca32
parent 58018 a3c63a9dfb2c
child 58021 5f5ca2e02f6e
8229496: SIGFPE (division by zero) in C2 OSR compiled method Summary: Adding a CastNode to keep the dependency between the div/mod operation and the zero check. Reviewed-by: roland, mdoerr
src/hotspot/cpu/aarch64/aarch64.ad
src/hotspot/cpu/arm/arm.ad
src/hotspot/cpu/ppc/ppc.ad
src/hotspot/cpu/s390/s390.ad
src/hotspot/cpu/sparc/sparc.ad
src/hotspot/cpu/x86/x86_32.ad
src/hotspot/cpu/x86/x86_64.ad
src/hotspot/share/opto/castnode.cpp
src/hotspot/share/opto/castnode.hpp
src/hotspot/share/opto/cfgnode.cpp
src/hotspot/share/opto/classes.hpp
src/hotspot/share/opto/graphKit.cpp
src/hotspot/share/opto/node.hpp
src/hotspot/share/runtime/vmStructs.cpp
test/hotspot/jtreg/compiler/loopopts/TestDivZeroCheckControl.java
--- a/src/hotspot/cpu/aarch64/aarch64.ad	Thu Sep 05 12:39:48 2019 +0200
+++ b/src/hotspot/cpu/aarch64/aarch64.ad	Thu Sep 05 13:56:17 2019 +0200
@@ -8354,6 +8354,17 @@
   ins_pipe(pipe_class_empty);
 %}
 
+instruct castLL(iRegL dst)
+%{
+  match(Set dst (CastLL dst));
+
+  size(0);
+  format %{ "# castLL of $dst" %}
+  ins_encode(/* empty encoding */);
+  ins_cost(0);
+  ins_pipe(pipe_class_empty);
+%}
+
 // ============================================================================
 // Atomic operation instructions
 //
--- a/src/hotspot/cpu/arm/arm.ad	Thu Sep 05 12:39:48 2019 +0200
+++ b/src/hotspot/cpu/arm/arm.ad	Thu Sep 05 13:56:17 2019 +0200
@@ -5276,6 +5276,14 @@
   ins_pipe(empty);
 %}
 
+instruct castLL( iRegL dst ) %{
+  match(Set dst (CastLL dst));
+  format %{ "! castLL of $dst" %}
+  ins_encode( /*empty encoding*/ );
+  ins_cost(0);
+  ins_pipe(empty);
+%}
+
 //----------Arithmetic Instructions--------------------------------------------
 // Addition Instructions
 // Register Addition
--- a/src/hotspot/cpu/ppc/ppc.ad	Thu Sep 05 12:39:48 2019 +0200
+++ b/src/hotspot/cpu/ppc/ppc.ad	Thu Sep 05 13:56:17 2019 +0200
@@ -10867,6 +10867,14 @@
   ins_pipe(pipe_class_default);
 %}
 
+instruct castLL(iRegLdst dst) %{
+  match(Set dst (CastLL dst));
+  format %{ " -- \t// castLL of $dst" %}
+  size(0);
+  ins_encode( /*empty*/ );
+  ins_pipe(pipe_class_default);
+%}
+
 instruct checkCastPP(iRegPdst dst) %{
   match(Set dst (CheckCastPP dst));
   format %{ " -- \t// checkcastPP of $dst" %}
--- a/src/hotspot/cpu/s390/s390.ad	Thu Sep 05 12:39:48 2019 +0200
+++ b/src/hotspot/cpu/s390/s390.ad	Thu Sep 05 13:56:17 2019 +0200
@@ -5409,6 +5409,14 @@
   ins_pipe(pipe_class_dummy);
 %}
 
+instruct castLL(iRegL dst) %{
+  match(Set dst (CastLL dst));
+  size(0);
+  format %{ "# castLL of $dst" %}
+  ins_encode(/*empty*/);
+  ins_pipe(pipe_class_dummy);
+%}
+
 
 //----------Conditional_store--------------------------------------------------
 // Conditional-store of the updated heap-top.
--- a/src/hotspot/cpu/sparc/sparc.ad	Thu Sep 05 12:39:48 2019 +0200
+++ b/src/hotspot/cpu/sparc/sparc.ad	Thu Sep 05 13:56:17 2019 +0200
@@ -6813,6 +6813,14 @@
   ins_pipe(empty);
 %}
 
+instruct castLL( iRegL dst ) %{
+  match(Set dst (CastLL dst));
+  format %{ "# castLL of $dst" %}
+  ins_encode( /*empty encoding*/ );
+  ins_cost(0);
+  ins_pipe(empty);
+%}
+
 //----------Arithmetic Instructions--------------------------------------------
 // Addition Instructions
 // Register Addition
--- a/src/hotspot/cpu/x86/x86_32.ad	Thu Sep 05 12:39:48 2019 +0200
+++ b/src/hotspot/cpu/x86/x86_32.ad	Thu Sep 05 13:56:17 2019 +0200
@@ -7315,6 +7315,14 @@
   ins_pipe( empty );
 %}
 
+instruct castLL( rRegL dst ) %{
+  match(Set dst (CastLL dst));
+  format %{ "#castLL of $dst" %}
+  ins_encode( /*empty encoding*/ );
+  ins_cost(0);
+  ins_pipe( empty );
+%}
+
 
 // Load-locked - same as a regular pointer load when used with compare-swap
 instruct loadPLocked(eRegP dst, memory mem) %{
--- a/src/hotspot/cpu/x86/x86_64.ad	Thu Sep 05 12:39:48 2019 +0200
+++ b/src/hotspot/cpu/x86/x86_64.ad	Thu Sep 05 13:56:17 2019 +0200
@@ -7763,6 +7763,17 @@
   ins_pipe(empty);
 %}
 
+instruct castLL(rRegL dst)
+%{
+  match(Set dst (CastLL dst));
+
+  size(0);
+  format %{ "# castLL of $dst" %}
+  ins_encode(/* empty encoding */);
+  ins_cost(0);
+  ins_pipe(empty);
+%}
+
 // LoadP-locked same as a regular LoadP when used with compare-swap
 instruct loadPLocked(rRegP dst, memory mem)
 %{
--- a/src/hotspot/share/opto/castnode.cpp	Thu Sep 05 12:39:48 2019 +0200
+++ b/src/hotspot/share/opto/castnode.cpp	Thu Sep 05 13:56:17 2019 +0200
@@ -63,6 +63,14 @@
       if (rt->empty())       assert(ft == Type::TOP, "special case #2");
       break;
     }
+    case Op_CastLL:
+    {
+      const Type* t1 = phase->type(in(1));
+      if (t1 == Type::TOP)   assert(ft == Type::TOP, "special case #1");
+      const Type* rt = t1->join_speculative(_type);
+      if (rt->empty())       assert(ft == Type::TOP, "special case #2");
+      break;
+    }
     case Op_CastPP:
     if (phase->type(in(1)) == TypePtr::NULL_PTR &&
         _type->isa_ptr() && _type->is_ptr()->_ptr == TypePtr::NotNull)
@@ -96,6 +104,11 @@
     cast->set_req(0, c);
     return cast;
   }
+  case Op_CastLL: {
+    Node* cast = new CastLLNode(n, t, carry_dependency);
+    cast->set_req(0, c);
+    return cast;
+  }
   case Op_CastPP: {
     Node* cast = new CastPPNode(n, t, carry_dependency);
     cast->set_req(0, c);
@@ -279,6 +292,45 @@
 }
 #endif
 
+Node* CastLLNode::Ideal(PhaseGVN* phase, bool can_reshape) {
+  Node* progress = ConstraintCastNode::Ideal(phase, can_reshape);
+  if (progress != NULL) {
+    return progress;
+  }
+
+  // Same as in CastIINode::Ideal but for TypeLong instead of TypeInt
+  if (can_reshape && !phase->C->major_progress()) {
+    const TypeLong* this_type = this->type()->is_long();
+    const TypeLong* in_type = phase->type(in(1))->isa_long();
+    if (in_type != NULL && this_type != NULL &&
+        (in_type->_lo != this_type->_lo ||
+         in_type->_hi != this_type->_hi)) {
+      jlong lo1 = this_type->_lo;
+      jlong hi1 = this_type->_hi;
+      int w1  = this_type->_widen;
+
+      if (lo1 >= 0) {
+        // Keep a range assertion of >=0.
+        lo1 = 0;         hi1 = max_jlong;
+      } else if (hi1 < 0) {
+        // Keep a range assertion of <0.
+        lo1 = min_jlong; hi1 = -1;
+      } else {
+        lo1 = min_jlong; hi1 = max_jlong;
+      }
+      const TypeLong* wtype = TypeLong::make(MAX2(in_type->_lo, lo1),
+                                             MIN2(in_type->_hi, hi1),
+                                             MAX2((int)in_type->_widen, w1));
+      if (wtype != type()) {
+        set_type(wtype);
+        return this;
+      }
+    }
+  }
+  return NULL;
+}
+
+
 //=============================================================================
 //------------------------------Identity---------------------------------------
 // If input is already higher or equal to cast type, then this is an identity.
--- a/src/hotspot/share/opto/castnode.hpp	Thu Sep 05 12:39:48 2019 +0200
+++ b/src/hotspot/share/opto/castnode.hpp	Thu Sep 05 13:56:17 2019 +0200
@@ -91,6 +91,19 @@
 #endif
 };
 
+//------------------------------CastLLNode-------------------------------------
+// cast long to long (different range)
+class CastLLNode: public ConstraintCastNode {
+  public:
+  CastLLNode(Node* n, const Type* t, bool carry_dependency = false)
+    : ConstraintCastNode(n, t, carry_dependency) {
+    init_class_id(Class_CastLL);
+  }
+  virtual int Opcode() const;
+  virtual uint ideal_reg() const { return Op_RegL; }
+  virtual Node* Ideal(PhaseGVN* phase, bool can_reshape);
+};
+
 //------------------------------CastPPNode-------------------------------------
 // cast pointer to pointer (different type)
 class CastPPNode: public ConstraintCastNode {
--- a/src/hotspot/share/opto/cfgnode.cpp	Thu Sep 05 12:39:48 2019 +0200
+++ b/src/hotspot/share/opto/cfgnode.cpp	Thu Sep 05 13:56:17 2019 +0200
@@ -1878,12 +1878,13 @@
       // Wait until after parsing for the type information to propagate from the casts.
       assert(can_reshape, "Invalid during parsing");
       const Type* phi_type = bottom_type();
-      assert(phi_type->isa_int() || phi_type->isa_ptr(), "bad phi type");
-      // Add casts to carry the control dependency of the Phi that is
-      // going away
+      assert(phi_type->isa_int() || phi_type->isa_long() || phi_type->isa_ptr(), "bad phi type");
+      // Add casts to carry the control dependency of the Phi that is going away
       Node* cast = NULL;
       if (phi_type->isa_int()) {
         cast = ConstraintCastNode::make_cast(Op_CastII, r, uin, phi_type, true);
+      } else if (phi_type->isa_long()) {
+        cast = ConstraintCastNode::make_cast(Op_CastLL, r, uin, phi_type, true);
       } else {
         const Type* uin_type = phase->type(uin);
         if (!phi_type->isa_oopptr() && !uin_type->isa_oopptr()) {
--- a/src/hotspot/share/opto/classes.hpp	Thu Sep 05 12:39:48 2019 +0200
+++ b/src/hotspot/share/opto/classes.hpp	Thu Sep 05 13:56:17 2019 +0200
@@ -61,6 +61,7 @@
 macro(CallRuntime)
 macro(CallStaticJava)
 macro(CastII)
+macro(CastLL)
 macro(CastX2P)
 macro(CastP2X)
 macro(CastPP)
--- a/src/hotspot/share/opto/graphKit.cpp	Thu Sep 05 12:39:48 2019 +0200
+++ b/src/hotspot/share/opto/graphKit.cpp	Thu Sep 05 13:56:17 2019 +0200
@@ -1362,35 +1362,37 @@
 
   // Cast obj to not-null on this path, if there is no null_control.
   // (If there is a null_control, a non-null value may come back to haunt us.)
-  if (type == T_OBJECT) {
-    Node* cast = cast_not_null(value, false);
-    if (null_control == NULL || (*null_control) == top())
-      replace_in_map(value, cast);
-    value = cast;
-  }
-
-  return value;
+  return cast_not_null(value, (null_control == NULL || (*null_control) == top()));
 }
 
 
 //------------------------------cast_not_null----------------------------------
 // Cast obj to not-null on this path
 Node* GraphKit::cast_not_null(Node* obj, bool do_replace_in_map) {
-  const Type *t = _gvn.type(obj);
-  const Type *t_not_null = t->join_speculative(TypePtr::NOTNULL);
-  // Object is already not-null?
-  if( t == t_not_null ) return obj;
-
-  Node *cast = new CastPPNode(obj,t_not_null);
-  cast->init_req(0, control());
-  cast = _gvn.transform( cast );
+  Node* cast = NULL;
+  const Type* t = _gvn.type(obj);
+  if (t->make_ptr() != NULL) {
+    const Type* t_not_null = t->join_speculative(TypePtr::NOTNULL);
+    // Object is already not-null?
+    if (t == t_not_null) {
+      return obj;
+    }
+    cast = ConstraintCastNode::make_cast(Op_CastPP, control(), obj, t_not_null, false);
+  } else if (t->isa_int() != NULL) {
+    cast = ConstraintCastNode::make_cast(Op_CastII, control(), obj, TypeInt::INT, true);
+  } else if (t->isa_long() != NULL) {
+    cast = ConstraintCastNode::make_cast(Op_CastLL, control(), obj, TypeLong::LONG, true);
+  } else {
+    fatal("unexpected type: %s", type2name(t->basic_type()));
+  }
+  cast = _gvn.transform(cast);
 
   // Scan for instances of 'obj' in the current JVM mapping.
   // These instances are known to be not-null after the test.
-  if (do_replace_in_map)
+  if (do_replace_in_map) {
     replace_in_map(obj, cast);
-
-  return cast;                  // Return casted value
+  }
+  return cast;
 }
 
 // Sometimes in intrinsics, we implicitly know an object is not null
--- a/src/hotspot/share/opto/node.hpp	Thu Sep 05 12:39:48 2019 +0200
+++ b/src/hotspot/share/opto/node.hpp	Thu Sep 05 13:56:17 2019 +0200
@@ -52,6 +52,7 @@
 class CallRuntimeNode;
 class CallStaticJavaNode;
 class CastIINode;
+class CastLLNode;
 class CatchNode;
 class CatchProjNode;
 class CheckCastPPNode;
@@ -666,7 +667,8 @@
       DEFINE_CLASS_ID(Phi,   Type, 0)
       DEFINE_CLASS_ID(ConstraintCast, Type, 1)
         DEFINE_CLASS_ID(CastII, ConstraintCast, 0)
-        DEFINE_CLASS_ID(CheckCastPP, ConstraintCast, 1)
+        DEFINE_CLASS_ID(CastLL, ConstraintCast, 1)
+        DEFINE_CLASS_ID(CheckCastPP, ConstraintCast, 2)
       DEFINE_CLASS_ID(CMove, Type, 3)
       DEFINE_CLASS_ID(SafePointScalarObject, Type, 4)
       DEFINE_CLASS_ID(DecodeNarrowPtr, Type, 5)
@@ -805,6 +807,7 @@
   DEFINE_CLASS_QUERY(CatchProj)
   DEFINE_CLASS_QUERY(CheckCastPP)
   DEFINE_CLASS_QUERY(CastII)
+  DEFINE_CLASS_QUERY(CastLL)
   DEFINE_CLASS_QUERY(ConstraintCast)
   DEFINE_CLASS_QUERY(ClearArray)
   DEFINE_CLASS_QUERY(CMove)
--- a/src/hotspot/share/runtime/vmStructs.cpp	Thu Sep 05 12:39:48 2019 +0200
+++ b/src/hotspot/share/runtime/vmStructs.cpp	Thu Sep 05 13:56:17 2019 +0200
@@ -1592,6 +1592,7 @@
   declare_c2_type(DecodeNKlassNode, TypeNode)                             \
   declare_c2_type(ConstraintCastNode, TypeNode)                           \
   declare_c2_type(CastIINode, ConstraintCastNode)                         \
+  declare_c2_type(CastLLNode, ConstraintCastNode)                         \
   declare_c2_type(CastPPNode, ConstraintCastNode)                         \
   declare_c2_type(CheckCastPPNode, TypeNode)                              \
   declare_c2_type(Conv2BNode, Node)                                       \
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/compiler/loopopts/TestDivZeroCheckControl.java	Thu Sep 05 13:56:17 2019 +0200
@@ -0,0 +1,106 @@
+/*
+ * Copyright (c) 2019, 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 8229496
+ * @summary Verify that zero check is executed before division/modulo operation.
+ * @run main/othervm -Xbatch -XX:LoopUnrollLimit=0
+ *                   -XX:CompileCommand=dontinline,compiler.loopopts.TestDivZeroCheckControl::test*
+ *                   compiler.loopopts.TestDivZeroCheckControl
+ */
+
+package compiler.loopopts;
+
+public class TestDivZeroCheckControl {
+
+    public static int test1(int div, int array[]) {
+        int res = 0;
+        for (int i = 0; i < 256; i++) {
+            int j = 0;
+            do {
+                array[i] = i;
+                try {
+                    res = 1 % div;
+                } catch (ArithmeticException ex) { }
+            } while (++j < 9);
+        }
+        return res;
+    }
+
+    // Same as test1 but with division instead of modulo
+    public static int test2(int div, int array[]) {
+        int res = 0;
+        for (int i = 0; i < 256; i++) {
+            int j = 0;
+            do {
+                array[i] = i;
+                try {
+                    res = 1 / div;
+                } catch (ArithmeticException ex) { }
+            } while (++j < 9);
+        }
+        return res;
+    }
+
+    // Same as test1 but with long
+    public static long test3(long div, int array[]) {
+        long res = 0;
+        for (int i = 0; i < 256; i++) {
+            int j = 0;
+            do {
+                array[i] = i;
+                try {
+                    res = 1L % div;
+                } catch (ArithmeticException ex) { }
+            } while (++j < 9);
+        }
+        return res;
+    }
+
+    // Same as test2 but with long
+    public static long test4(long div, int array[]) {
+        long res = 0;
+        for (int i = 0; i < 256; i++) {
+            int j = 0;
+            do {
+                array[i] = i;
+                try {
+                    res = 1L / div;
+                } catch (ArithmeticException ex) { }
+            } while (++j < 9);
+        }
+        return res;
+    }
+
+    public static void main(String[] args) {
+        int array[] = new int[256];
+        for (int i = 0; i < 50_000; ++i) {
+            test1(0, array);
+            test2(0, array);
+            test3(0, array);
+            test4(0, array);
+        }
+    }
+}