changeset 27637:cf68c0af6882

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
author zmajo
date Thu, 06 Nov 2014 09:40:58 +0100
parents 854b988388f3
children 1142a24d73eb
files hotspot/src/share/vm/opto/doCall.cpp hotspot/src/share/vm/opto/graphKit.cpp hotspot/src/share/vm/opto/library_call.cpp hotspot/src/share/vm/opto/macro.cpp hotspot/src/share/vm/opto/macroArrayCopy.cpp hotspot/src/share/vm/opto/memnode.cpp hotspot/src/share/vm/opto/memnode.hpp hotspot/src/share/vm/opto/parse1.cpp hotspot/src/share/vm/opto/parseHelper.cpp
diffstat 9 files changed, 69 insertions(+), 26 deletions(-) [+]
line wrap: on
line diff
--- 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);
 }