# HG changeset patch # User kvn # Date 1240444998 25200 # Node ID 04fa5affa478c9cfd574efc7515c64b43b47af3f # Parent 85656c8fa13f0f571fae2d0423965ff1213d7db7 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 diff -r 85656c8fa13f -r 04fa5affa478 src/share/vm/opto/buildOopMap.cpp --- 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; } diff -r 85656c8fa13f -r 04fa5affa478 src/share/vm/opto/chaitin.cpp --- 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 ); diff -r 85656c8fa13f -r 04fa5affa478 src/share/vm/opto/matcher.cpp --- 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 diff -r 85656c8fa13f -r 04fa5affa478 src/share/vm/opto/matcher.hpp --- 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; }