Request for prereviews (S): 7047954: VM crashes with assert(is_Mem()) failed: invalid node class (original) (raw)

Tom Rodriguez tom.rodriguez at oracle.com
Thu May 26 17:49:38 PDT 2011

1.529 01/12/19 In flattenaliastype(), handle constant arrays more consistently. 883,884c887,893 < if( ptr == TypePtr::NotNull || ta->klassisexact() ) < tj = ta = TypeAryPtr::make(TypePtr::BotPTR,ta->constoop(),ta->ary(),ta->klass(),false,offset); --- > if( ptr == TypePtr::NotNull || ta->klassisexact() ) { > if (ta->constoop()) { > tj = ta = TypeAryPtr::make(TypePtr::Constant,ta->constoop(),ta->ary(),ta->klass(),false,offset); > } else { > tj = ta = TypeAryPtr::make(TypePtr::BotPTR,ta->ary(),ta->klass(),false,offset); > } > } As I understand the original code had problem that ptr was BotPTR but constoop was set. But the fix should be using other ctor which set constoop to NULL: tj = ta = TypeAryPtr::make(TypePtr::BotPTR,ta->ary(),ta->klass(),false,offset);

Anyway, the only concern I have is performance since with this fix (cast to bottom) accesses to different arrays will be ordered. But on other hand we already do that for constant instances so it could be fine. So do you agree that I should just cast constant array ptrs to bottom to fix this bug?

That seems the most consistent solution assuming it doesn't surface other problems.

tom

Thanks, Vlaidmir Tom Rodriguez wrote: On May 26, 2011, at 4:34 PM, Vladimir Kozlov wrote:

Tom Rodriguez wrote:

On May 26, 2011, at 3:27 PM, Vladimir Kozlov wrote:

http://cr.openjdk.java.net/~kvn/7047954/webrev

Fixed 7047954: VM crashes with assert(isMem()) failed: invalid node class It was exposed by isscavengable change. Final static array pointer was replaced with ConP. The main problem is a constant array pointer does not alias with other array pointers (see Compile::flattenaliastype()). Do you mean this logic: // Array pointers need some flattening const TypeAryPtr *ta = tj->isaaryptr(); if( ta && isknowninst ) { if ( offset != Type::OffsetBot && offset > arrayOopDesc::lengthoffsetinbytes() ) { offset = Type::OffsetBot; // Flatten constant access into array body only tj = ta = TypeAryPtr::make(ptr, ta->ary(), ta->klass(), true, offset, ta->instanceid()); } } No. This logic which does not convert constant array pointer to bottom array pointer (keep them separate): // Make sure the Bottom and NotNull variants alias the same. // Also, make sure exact and non-exact variants alias the same. if( ptr == TypePtr::NotNull || ta->klassisexact() ) { if (ta->constoop()) { tj = ta = TypeAryPtr::make(TypePtr::Constant,ta->constoop(),ta->ary(),ta->klass(),false,offset); } else { tj = ta = TypeAryPtr::make(TypePtr::BotPTR,ta->ary(),ta->klass(),false,offset); } } Ok. Why are arrays handled differently than instances? Those don't seem to do that. That seems odd. It was added as part of 6723160: Nightly failure: Error: meet not symmetric. Is instanceid set for constant objects? As result the address of a store has different memory slice (idx=4) than calculated memory slice affected by the store (idx=5): It seems like there should be an assert that checks that these agree? The check is in MemNode::adrtype() works fine but the problem is not there but in GraphKit::storetomemory() where created store attached to mergemem: setmemory(st, adridx); adridx is calculated from passed (general) adrtype and not from adr->bottomtype(). It seems like those should be required to agree though I suspect sometimes the passed in value is raw while adr->bottomtype is something else. tom Vladimir 75 StoreI === 61 46 73 20 [[ 95 81 ]] @int[int:>=0]:exact+any *, idx=5; Memory: @int[int:1000000]<ciTypeArray length=1000000 type= ident=696 address=0x8527328>+any *, idx=4; !jvms: ReadAfterWrite::main @ bci:16 It happened because when a store to array is created the type of affected memory is calculated using general array type TypeAryPtr::getarraybodytype(elemtype). So we either should allow a constant array pointer alias with other array pointers or check if an array ptr is constant and use its type instead of general one. I used second approach and fixed all places where getarraybodytype() was used. But I need your opinion on this. Based on previous Tom's comment we should not convert array pointers to constant (can we have arrays in Perm?). Yes we can. Previously the char[] of constant Strings were like that though we only read from those and only recently started treating their fields as constants and we also recently moved them out of perm. I don't think we embed any other constant perm arrays in compiled code though. But then there is invoke dynamic case and I am not sure that we will not get constant arrays from it. Going forward we're going to be seeing a lot more constant objects in generated code since it's easy to embed them now. 292 in particular is going to want this so we can fold away as much goo as possible. tom Thanks, Vladimir

More information about the hotspot-compiler-dev mailing list