changeset 729:04fa5affa478

6709742: find_base_for_derived's use of Ideal NULL is unsafe causing crashes during register allocation Summary: Create a mach node corresponding to ideal node ConP #NULL specifically for derived pointers. Reviewed-by: never
author kvn
date Wed, 22 Apr 2009 17:03:18 -0700
parents 85656c8fa13f
children 9c6be3edf0dc
files src/share/vm/opto/buildOopMap.cpp src/share/vm/opto/chaitin.cpp src/share/vm/opto/matcher.cpp src/share/vm/opto/matcher.hpp
diffstat 4 files changed, 62 insertions(+), 6 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/vm/opto/buildOopMap.cpp	Wed Apr 22 06:09:24 2009 -0700
+++ b/src/share/vm/opto/buildOopMap.cpp	Wed Apr 22 17:03:18 2009 -0700
@@ -363,6 +363,20 @@
   */
 #endif
 
+#ifdef ASSERT
+  for( OopMapStream oms1(omap, OopMapValue::derived_oop_value); !oms1.is_done(); oms1.next()) {
+    OopMapValue omv1 = oms1.current();
+    bool found = false;
+    for( OopMapStream oms2(omap,OopMapValue::oop_value); !oms2.is_done(); oms2.next()) {
+      if( omv1.content_reg() == oms2.current().reg() ) {
+        found = true;
+        break;
+      }
+    }
+    assert( found, "derived with no base in oopmap" );
+  }
+#endif
+
   return omap;
 }
 
--- a/src/share/vm/opto/chaitin.cpp	Wed Apr 22 06:09:24 2009 -0700
+++ b/src/share/vm/opto/chaitin.cpp	Wed Apr 22 17:03:18 2009 -0700
@@ -1423,17 +1423,33 @@
   // pointers derived from NULL!  These are always along paths that
   // can't happen at run-time but the optimizer cannot deduce it so
   // we have to handle it gracefully.
+  assert(!derived->bottom_type()->isa_narrowoop() ||
+          derived->bottom_type()->make_ptr()->is_ptr()->_offset == 0, "sanity");
   const TypePtr *tj = derived->bottom_type()->isa_ptr();
   // If its an OOP with a non-zero offset, then it is derived.
-  if( tj->_offset == 0 ) {
+  if( tj == NULL || tj->_offset == 0 ) {
     derived_base_map[derived->_idx] = derived;
     return derived;
   }
   // Derived is NULL+offset?  Base is NULL!
   if( derived->is_Con() ) {
-    Node *base = new (C, 1) ConPNode( TypePtr::NULL_PTR );
-    uint no_lidx = 0;  // an unmatched constant in debug info has no LRG
-    _names.extend(base->_idx, no_lidx);
+    Node *base = _matcher.mach_null();
+    assert(base != NULL, "sanity");
+    if (base->in(0) == NULL) {
+      // Initialize it once and make it shared:
+      // set control to _root and place it into Start block
+      // (where top() node is placed).
+      base->init_req(0, _cfg._root);
+      Block *startb = _cfg._bbs[C->top()->_idx];
+      startb->_nodes.insert(startb->find_node(C->top()), base );
+      _cfg._bbs.map( base->_idx, startb );
+      assert (n2lidx(base) == 0, "should not have LRG yet");
+    }
+    if (n2lidx(base) == 0) {
+      new_lrg(base, maxlrg++);
+    }
+    assert(base->in(0) == _cfg._root &&
+           _cfg._bbs[base->_idx] == _cfg._bbs[C->top()->_idx], "base NULL should be shared");
     derived_base_map[derived->_idx] = base;
     return base;
   }
@@ -1460,9 +1476,13 @@
   }
 
   // Now we see we need a base-Phi here to merge the bases
-  base = new (C, derived->req()) PhiNode( derived->in(0), base->bottom_type() );
-  for( i = 1; i < derived->req(); i++ )
+  const Type *t = base->bottom_type();
+  base = new (C, derived->req()) PhiNode( derived->in(0), t );
+  for( i = 1; i < derived->req(); i++ ) {
     base->init_req(i, find_base_for_derived(derived_base_map, derived->in(i), maxlrg));
+    t = t->meet(base->in(i)->bottom_type());
+  }
+  base->as_Phi()->set_type(t);
 
   // Search the current block for an existing base-Phi
   Block *b = _cfg._bbs[derived->_idx];
@@ -1560,6 +1580,8 @@
           // This works because we are still in SSA during this call.
           Node *derived = lrgs(neighbor)._def;
           const TypePtr *tj = derived->bottom_type()->isa_ptr();
+          assert(!derived->bottom_type()->isa_narrowoop() ||
+                  derived->bottom_type()->make_ptr()->is_ptr()->_offset == 0, "sanity");
           // If its an OOP with a non-zero offset, then it is derived.
           if( tj && tj->_offset != 0 && tj->isa_oop_ptr() ) {
             Node *base = find_base_for_derived( derived_base_map, derived, maxlrg );
--- a/src/share/vm/opto/matcher.cpp	Wed Apr 22 06:09:24 2009 -0700
+++ b/src/share/vm/opto/matcher.cpp	Wed Apr 22 17:03:18 2009 -0700
@@ -275,6 +275,12 @@
 
   C->print_method("Before Matching");
 
+  // Create new ideal node ConP #NULL even if it does exist in old space
+  // to avoid false sharing if the corresponding mach node is not used.
+  // The corresponding mach node is only used in rare cases for derived
+  // pointers.
+  Node* new_ideal_null = ConNode::make(C, TypePtr::NULL_PTR);
+
   // Swap out to old-space; emptying new-space
   Arena *old = C->node_arena()->move_contents(C->old_arena());
 
@@ -316,7 +322,16 @@
         }
       }
 
+      // Generate new mach node for ConP #NULL
+      assert(new_ideal_null != NULL, "sanity");
+      _mach_null = match_tree(new_ideal_null);
+      // Don't set control, it will confuse GCM since there are no uses.
+      // The control will be set when this node is used first time
+      // in find_base_for_derived().
+      assert(_mach_null != NULL, "");
+
       C->set_root(xroot->is_Root() ? xroot->as_Root() : NULL);
+
 #ifdef ASSERT
       verify_new_nodes_only(xroot);
 #endif
--- a/src/share/vm/opto/matcher.hpp	Wed Apr 22 06:09:24 2009 -0700
+++ b/src/share/vm/opto/matcher.hpp	Wed Apr 22 17:03:18 2009 -0700
@@ -109,6 +109,9 @@
   Node* _mem_node;   // Ideal memory node consumed by mach node
 #endif
 
+  // Mach node for ConP #NULL
+  MachNode* _mach_null;
+
 public:
   int LabelRootDepth;
   static const int base2reg[];        // Map Types to machine register types
@@ -122,6 +125,8 @@
   static RegMask mreg2regmask[];
   static RegMask STACK_ONLY_mask;
 
+  MachNode* mach_null() const { return _mach_null; }
+
   bool    is_shared( Node *n ) { return _shared.test(n->_idx) != 0; }
   void   set_shared( Node *n ) {  _shared.set(n->_idx); }
   bool   is_visited( Node *n ) { return _visited.test(n->_idx) != 0; }