OpenJDK / jdk / jdk
changeset 56184:86b95fc6ca32
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
author | thartmann |
---|---|
date | Thu, 05 Sep 2019 13:56:17 +0200 |
parents | a3c63a9dfb2c |
children | 5f5ca2e02f6e |
files | 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 |
diffstat | 15 files changed, 264 insertions(+), 23 deletions(-) [+] |
line wrap: on
line diff
--- 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); + } + } +}