8057622: java/util/stream/test/org/openjdk/tests/java/util/stream/InfiniteStreamWithLimitOpTest: SEGV inside compiled code (sparc)
Summary: In Parse::array_store_check(), add control edge FROM IfTrue branch of runtime type check of the destination array TO loading _element_klass from destination array.
Reviewed-by: kvn, roland, anoll
--- a/hotspot/src/share/vm/opto/doCall.cpp Tue Nov 04 07:09:34 2014 -1000
+++ b/hotspot/src/share/vm/opto/doCall.cpp Thu Nov 06 09:40:58 2014 +0100
@@ -794,7 +794,7 @@
Node* ex_klass_node = NULL;
if (has_ex_handler() && !ex_type->klass_is_exact()) {
Node* p = basic_plus_adr( ex_node, ex_node, oopDesc::klass_offset_in_bytes());
- ex_klass_node = _gvn.transform( LoadKlassNode::make(_gvn, immutable_memory(), p, TypeInstPtr::KLASS, TypeKlassPtr::OBJECT) );
+ ex_klass_node = _gvn.transform(LoadKlassNode::make(_gvn, NULL, immutable_memory(), p, TypeInstPtr::KLASS, TypeKlassPtr::OBJECT));
// Compute the exception klass a little more cleverly.
// Obvious solution is to simple do a LoadKlass from the 'ex_node'.
@@ -812,7 +812,7 @@
continue;
}
Node* p = basic_plus_adr(ex_in, ex_in, oopDesc::klass_offset_in_bytes());
- Node* k = _gvn.transform( LoadKlassNode::make(_gvn, immutable_memory(), p, TypeInstPtr::KLASS, TypeKlassPtr::OBJECT) );
+ Node* k = _gvn.transform( LoadKlassNode::make(_gvn, NULL, immutable_memory(), p, TypeInstPtr::KLASS, TypeKlassPtr::OBJECT));
ex_klass_node->init_req( i, k );
}
_gvn.set_type(ex_klass_node, TypeKlassPtr::OBJECT);
--- a/hotspot/src/share/vm/opto/graphKit.cpp Tue Nov 04 07:09:34 2014 -1000
+++ b/hotspot/src/share/vm/opto/graphKit.cpp Thu Nov 06 09:40:58 2014 +0100
@@ -1154,7 +1154,7 @@
Node* akls = AllocateNode::Ideal_klass(obj, &_gvn);
if (akls != NULL) return akls;
Node* k_adr = basic_plus_adr(obj, oopDesc::klass_offset_in_bytes());
- return _gvn.transform( LoadKlassNode::make(_gvn, immutable_memory(), k_adr, TypeInstPtr::KLASS) );
+ return _gvn.transform(LoadKlassNode::make(_gvn, NULL, immutable_memory(), k_adr, TypeInstPtr::KLASS));
}
//-------------------------load_array_length-----------------------------------
@@ -2615,7 +2615,7 @@
// types load from the super-class display table which is immutable.
m = mem->memory_at(C->get_alias_index(gvn->type(p2)->is_ptr()));
Node *kmem = might_be_cache ? m : C->immutable_memory();
- Node *nkls = gvn->transform(LoadKlassNode::make(*gvn, kmem, p2, gvn->type(p2)->is_ptr(), TypeKlassPtr::OBJECT_OR_NULL));
+ Node *nkls = gvn->transform(LoadKlassNode::make(*gvn, NULL, kmem, p2, gvn->type(p2)->is_ptr(), TypeKlassPtr::OBJECT_OR_NULL));
// Compile speed common case: ARE a subtype and we canNOT fail
if( superklass == nkls )
--- a/hotspot/src/share/vm/opto/library_call.cpp Tue Nov 04 07:09:34 2014 -1000
+++ b/hotspot/src/share/vm/opto/library_call.cpp Thu Nov 06 09:40:58 2014 +0100
@@ -3345,7 +3345,7 @@
if (region == NULL) never_see_null = true;
Node* p = basic_plus_adr(mirror, offset);
const TypeKlassPtr* kls_type = TypeKlassPtr::OBJECT_OR_NULL;
- Node* kls = _gvn.transform( LoadKlassNode::make(_gvn, immutable_memory(), p, TypeRawPtr::BOTTOM, kls_type));
+ Node* kls = _gvn.transform(LoadKlassNode::make(_gvn, NULL, immutable_memory(), p, TypeRawPtr::BOTTOM, kls_type));
Node* null_ctl = top();
kls = null_check_oop(kls, &null_ctl, never_see_null);
if (region != NULL) {
@@ -3517,7 +3517,7 @@
phi->add_req(makecon(TypeInstPtr::make(env()->Object_klass()->java_mirror())));
// If we fall through, it's a plain class. Get its _super.
p = basic_plus_adr(kls, in_bytes(Klass::super_offset()));
- kls = _gvn.transform( LoadKlassNode::make(_gvn, immutable_memory(), p, TypeRawPtr::BOTTOM, TypeKlassPtr::OBJECT_OR_NULL));
+ kls = _gvn.transform(LoadKlassNode::make(_gvn, NULL, immutable_memory(), p, TypeRawPtr::BOTTOM, TypeKlassPtr::OBJECT_OR_NULL));
null_ctl = top();
kls = null_check_oop(kls, &null_ctl);
if (null_ctl != top()) {
@@ -3671,7 +3671,7 @@
args[which_arg] = arg;
Node* p = basic_plus_adr(arg, class_klass_offset);
- Node* kls = LoadKlassNode::make(_gvn, immutable_memory(), p, adr_type, kls_type);
+ Node* kls = LoadKlassNode::make(_gvn, NULL, immutable_memory(), p, adr_type, kls_type);
klasses[which_arg] = _gvn.transform(kls);
}
--- a/hotspot/src/share/vm/opto/macro.cpp Tue Nov 04 07:09:34 2014 -1000
+++ b/hotspot/src/share/vm/opto/macro.cpp Thu Nov 06 09:40:58 2014 +0100
@@ -2202,7 +2202,7 @@
Node* klass_node = AllocateNode::Ideal_klass(obj, &_igvn);
if (klass_node == NULL) {
Node* k_adr = basic_plus_adr(obj, oopDesc::klass_offset_in_bytes());
- klass_node = transform_later( LoadKlassNode::make(_igvn, mem, k_adr, _igvn.type(k_adr)->is_ptr()) );
+ klass_node = transform_later(LoadKlassNode::make(_igvn, NULL, mem, k_adr, _igvn.type(k_adr)->is_ptr()));
#ifdef _LP64
if (UseCompressedClassPointers && klass_node->is_DecodeNKlass()) {
assert(klass_node->in(1)->Opcode() == Op_LoadNKlass, "sanity");
--- a/hotspot/src/share/vm/opto/macroArrayCopy.cpp Tue Nov 04 07:09:34 2014 -1000
+++ b/hotspot/src/share/vm/opto/macroArrayCopy.cpp Thu Nov 06 09:40:58 2014 +0100
@@ -529,7 +529,7 @@
// (At this point we can assume disjoint_bases, since types differ.)
int ek_offset = in_bytes(ObjArrayKlass::element_klass_offset());
Node* p1 = basic_plus_adr(dest_klass, ek_offset);
- Node* n1 = LoadKlassNode::make(_igvn, C->immutable_memory(), p1, TypeRawPtr::BOTTOM);
+ Node* n1 = LoadKlassNode::make(_igvn, NULL, C->immutable_memory(), p1, TypeRawPtr::BOTTOM);
Node* dest_elem_klass = transform_later(n1);
Node* cv = generate_checkcast_arraycopy(&local_ctrl, &local_mem,
adr_type,
--- a/hotspot/src/share/vm/opto/memnode.cpp Tue Nov 04 07:09:34 2014 -1000
+++ b/hotspot/src/share/vm/opto/memnode.cpp Thu Nov 06 09:40:58 2014 +0100
@@ -861,6 +861,10 @@
//=============================================================================
+// Should LoadNode::Ideal() attempt to remove control edges?
+bool LoadNode::can_remove_control() const {
+ return true;
+}
uint LoadNode::size_of() const { return sizeof(*this); }
uint LoadNode::cmp( const Node &n ) const
{ return !Type::cmp( _type, ((LoadNode&)n)._type ); }
@@ -1471,7 +1475,7 @@
}
//------------------------------Ideal------------------------------------------
-// If the load is from Field memory and the pointer is non-null, we can
+// If the load is from Field memory and the pointer is non-null, it might be possible to
// zero out the control input.
// If the offset is constant and the base is an object allocation,
// try to hook me up to the exact initializing store.
@@ -1498,6 +1502,7 @@
&& phase->C->get_alias_index(phase->type(address)->is_ptr()) != Compile::AliasIdxRaw) {
// Check for useless control edge in some common special cases
if (in(MemNode::Control) != NULL
+ && can_remove_control()
&& phase->type(base)->higher_equal(TypePtr::NOTNULL)
&& all_controls_dominate(base, phase->C->start())) {
// A method-invariant, non-null address (constant or 'this' argument).
@@ -2019,8 +2024,7 @@
//=============================================================================
//----------------------------LoadKlassNode::make------------------------------
// Polymorphic factory method:
-Node *LoadKlassNode::make( PhaseGVN& gvn, Node *mem, Node *adr, const TypePtr* at, const TypeKlassPtr *tk ) {
- Node *ctl = NULL;
+Node* LoadKlassNode::make(PhaseGVN& gvn, Node* ctl, Node* mem, Node* adr, const TypePtr* at, const TypeKlassPtr* tk) {
// sanity check the alias category against the created node type
const TypePtr *adr_type = adr->bottom_type()->isa_ptr();
assert(adr_type != NULL, "expecting TypeKlassPtr");
@@ -2040,6 +2044,12 @@
return klass_value_common(phase);
}
+// In most cases, LoadKlassNode does not have the control input set. If the control
+// input is set, it must not be removed (by LoadNode::Ideal()).
+bool LoadKlassNode::can_remove_control() const {
+ return false;
+}
+
const Type *LoadNode::klass_value_common( PhaseTransform *phase ) const {
// Either input is TOP ==> the result is TOP
const Type *t1 = phase->type( in(MemNode::Memory) );
--- a/hotspot/src/share/vm/opto/memnode.hpp Tue Nov 04 07:09:34 2014 -1000
+++ b/hotspot/src/share/vm/opto/memnode.hpp Thu Nov 06 09:40:58 2014 +0100
@@ -148,6 +148,8 @@
protected:
virtual uint cmp(const Node &n) const;
virtual uint size_of() const; // Size is bigger
+ // Should LoadNode::Ideal() attempt to remove control edges?
+ virtual bool can_remove_control() const;
const Type* const _type; // What kind of value is loaded?
public:
@@ -171,8 +173,10 @@
// we are equivalent to. We look for Load of a Store.
virtual Node *Identity( PhaseTransform *phase );
- // If the load is from Field memory and the pointer is non-null, we can
+ // If the load is from Field memory and the pointer is non-null, it might be possible to
// zero out the control input.
+ // If the offset is constant and the base is an object allocation,
+ // try to hook me up to the exact initializing store.
virtual Node *Ideal(PhaseGVN *phase, bool can_reshape);
// Split instance field load through Phi.
@@ -431,6 +435,10 @@
//------------------------------LoadKlassNode----------------------------------
// Load a Klass from an object
class LoadKlassNode : public LoadPNode {
+protected:
+ // In most cases, LoadKlassNode does not have the control input set. If the control
+ // input is set, it must not be removed (by LoadNode::Ideal()).
+ virtual bool can_remove_control() const;
public:
LoadKlassNode(Node *c, Node *mem, Node *adr, const TypePtr *at, const TypeKlassPtr *tk, MemOrd mo)
: LoadPNode(c, mem, adr, at, tk, mo) {}
@@ -440,8 +448,8 @@
virtual bool depends_only_on_test() const { return true; }
// Polymorphic factory method:
- static Node* make( PhaseGVN& gvn, Node *mem, Node *adr, const TypePtr* at,
- const TypeKlassPtr *tk = TypeKlassPtr::OBJECT );
+ static Node* make(PhaseGVN& gvn, Node* ctl, Node* mem, Node* adr, const TypePtr* at,
+ const TypeKlassPtr* tk = TypeKlassPtr::OBJECT);
};
//------------------------------LoadNKlassNode---------------------------------
--- a/hotspot/src/share/vm/opto/parse1.cpp Tue Nov 04 07:09:34 2014 -1000
+++ b/hotspot/src/share/vm/opto/parse1.cpp Thu Nov 06 09:40:58 2014 +0100
@@ -1987,7 +1987,7 @@
// finalization. In general this will fold up since the concrete
// class is often visible so the access flags are constant.
Node* klass_addr = basic_plus_adr( receiver, receiver, oopDesc::klass_offset_in_bytes() );
- Node* klass = _gvn.transform( LoadKlassNode::make(_gvn, immutable_memory(), klass_addr, TypeInstPtr::KLASS) );
+ Node* klass = _gvn.transform(LoadKlassNode::make(_gvn, NULL, immutable_memory(), klass_addr, TypeInstPtr::KLASS));
Node* access_flags_addr = basic_plus_adr(klass, klass, in_bytes(Klass::access_flags_offset()));
Node* access_flags = make_load(NULL, access_flags_addr, TypeInt::INT, T_INT, MemNode::unordered);
--- a/hotspot/src/share/vm/opto/parseHelper.cpp Tue Nov 04 07:09:34 2014 -1000
+++ b/hotspot/src/share/vm/opto/parseHelper.cpp Thu Nov 06 09:40:58 2014 +0100
@@ -156,22 +156,43 @@
int klass_offset = oopDesc::klass_offset_in_bytes();
Node* p = basic_plus_adr( ary, ary, klass_offset );
// p's type is array-of-OOPS plus klass_offset
- Node* array_klass = _gvn.transform( LoadKlassNode::make(_gvn, immutable_memory(), p, TypeInstPtr::KLASS) );
+ Node* array_klass = _gvn.transform(LoadKlassNode::make(_gvn, NULL, immutable_memory(), p, TypeInstPtr::KLASS));
// Get the array klass
const TypeKlassPtr *tak = _gvn.type(array_klass)->is_klassptr();
- // array_klass's type is generally INexact array-of-oop. Heroically
- // cast the array klass to EXACT array and uncommon-trap if the cast
- // fails.
+ // The type of array_klass is usually INexact array-of-oop. Heroically
+ // cast array_klass to EXACT array and uncommon-trap if the cast fails.
+ // Make constant out of the inexact array klass, but use it only if the cast
+ // succeeds.
bool always_see_exact_class = false;
if (MonomorphicArrayCheck
- && !too_many_traps(Deoptimization::Reason_array_check)) {
+ && !too_many_traps(Deoptimization::Reason_array_check)
+ && !tak->klass_is_exact()
+ && tak != TypeKlassPtr::OBJECT) {
+ // Regarding the fourth condition in the if-statement from above:
+ //
+ // If the compiler has determined that the type of array 'ary' (represented
+ // by 'array_klass') is java/lang/Object, the compiler must not assume that
+ // the array 'ary' is monomorphic.
+ //
+ // If 'ary' were of type java/lang/Object, this arraystore would have to fail,
+ // because it is not possible to perform a arraystore into an object that is not
+ // a "proper" array.
+ //
+ // Therefore, let's obtain at runtime the type of 'ary' and check if we can still
+ // successfully perform the store.
+ //
+ // The implementation reasons for the condition are the following:
+ //
+ // java/lang/Object is the superclass of all arrays, but it is represented by the VM
+ // as an InstanceKlass. The checks generated by gen_checkcast() (see below) expect
+ // 'array_klass' to be ObjArrayKlass, which can result in invalid memory accesses.
+ //
+ // See issue JDK-8057622 for details.
+
always_see_exact_class = true;
// (If no MDO at all, hope for the best, until a trap actually occurs.)
- }
- // Is the array klass is exactly its defined type?
- if (always_see_exact_class && !tak->klass_is_exact()) {
// Make a constant out of the inexact array klass
const TypeKlassPtr *extak = tak->cast_to_exactness(true)->is_klassptr();
Node* con = makecon(extak);
@@ -202,11 +223,15 @@
// Extract the array element class
int element_klass_offset = in_bytes(ObjArrayKlass::element_klass_offset());
Node *p2 = basic_plus_adr(array_klass, array_klass, element_klass_offset);
- Node *a_e_klass = _gvn.transform( LoadKlassNode::make(_gvn, immutable_memory(), p2, tak) );
+ // We are allowed to use the constant type only if cast succeeded. If always_see_exact_class is true,
+ // we must set a control edge from the IfTrue node created by the uncommon_trap above to the
+ // LoadKlassNode.
+ Node* a_e_klass = _gvn.transform(LoadKlassNode::make(_gvn, always_see_exact_class ? control() : NULL,
+ immutable_memory(), p2, tak));
// Check (the hard way) and throw if not a subklass.
// Result is ignored, we just need the CFG effects.
- gen_checkcast( obj, a_e_klass );
+ gen_checkcast(obj, a_e_klass);
}