From 70ab0931f6f0154b196efb1ae630aa5151b7243f Mon Sep 17 00:00:00 2001 From: kvn Date: Wed, 22 Apr 2009 17:03:18 -0700 Subject: [PATCH] 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 --- src/share/vm/opto/buildOopMap.cpp | 14 +++++++++++++ src/share/vm/opto/chaitin.cpp | 34 +++++++++++++++++++++++++------ src/share/vm/opto/matcher.cpp | 15 ++++++++++++++ src/share/vm/opto/matcher.hpp | 5 +++++ 4 files changed, 62 insertions(+), 6 deletions(-) diff --git a/src/share/vm/opto/buildOopMap.cpp b/src/share/vm/opto/buildOopMap.cpp index 4a8612687..9401e024a 100644 --- a/src/share/vm/opto/buildOopMap.cpp +++ b/src/share/vm/opto/buildOopMap.cpp @@ -363,6 +363,20 @@ OopMap *OopFlow::build_oop_map( Node *n, int max_reg, PhaseRegAlloc *regalloc, i */ #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 --git a/src/share/vm/opto/chaitin.cpp b/src/share/vm/opto/chaitin.cpp index 4ad220d41..eb177c8f5 100644 --- a/src/share/vm/opto/chaitin.cpp +++ b/src/share/vm/opto/chaitin.cpp @@ -1423,17 +1423,33 @@ Node *PhaseChaitin::find_base_for_derived( Node **derived_base_map, Node *derive // 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 @@ Node *PhaseChaitin::find_base_for_derived( Node **derived_base_map, Node *derive } // 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 @@ bool PhaseChaitin::stretch_base_pointer_live_ranges( ResourceArea *a ) { // 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 --git a/src/share/vm/opto/matcher.cpp b/src/share/vm/opto/matcher.cpp index e230480b2..2723a32fd 100644 --- a/src/share/vm/opto/matcher.cpp +++ b/src/share/vm/opto/matcher.cpp @@ -275,6 +275,12 @@ void Matcher::match( ) { 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 @@ void Matcher::match( ) { } } + // 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 --git a/src/share/vm/opto/matcher.hpp b/src/share/vm/opto/matcher.hpp index a042c04b0..9f79dc372 100644 --- a/src/share/vm/opto/matcher.hpp +++ b/src/share/vm/opto/matcher.hpp @@ -109,6 +109,9 @@ class Matcher : public PhaseTransform { 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 @@ public: 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; } -- GitLab