Request for reviews (S): 7092712: JSR 292: unloaded invokedynamic call sites can lead to a crash with signature types not on BCP

[Christian Thalinger]

On Oct 6, 2011, at 5:23 PM, Christian Thalinger wrote:

> 
> On Sep 29, 2011, at 3:44 AM, John Rose wrote:
> 
>> On Sep 22, 2011, at 3:23 AM, Christian Thalinger wrote:
>> 
>>> http://cr.openjdk.java.net/~twisti/7092712/
>>> 
>>> ...
>>> 
>>> The fix is to always pass an accessor to get_unloaded_method and
>>> subsequently the ciMethod constructor.
>> 
>> There's still a bug here:  If you call get_unloaded_method twice on the same parameters, it will return the same ciMethod, which is good.
>> 
>> But, if you pass a different accessor (along with the same other values) you will get the same ciMethod.  If the two accessors are different enough, they can imply different resolved signature types, so this can potentially cause 7092712 to reoccur.
> 
> I agree.
> 
>> 
>> I think you need to match the accessor argument in the matching loop of get_unloaded_method.
>> 
>> Option 1.  In the matching loop, after checking the symbolic signature, also check the resolved signature.  You can do this by making a ciSignature (which resolves types relative to the accessor).  Add an equals method to ciSignature.  Consider adding equals(ciSymbol* sig, ciKlass* accessor), so that the second ciSignature does not need to be created.
>> 
>> Option 2.  Check the accessor argument against the signature._accessing_klass in the matching loop.  To preserve existing behavior (for non-MH calls) either use null accessors for non-MH lookups and treat them specially (as wildcards) or else keep separate lists (of MH and non-MH unloaded methods).
>> 
>> I think Option 1 is more precise, but Option 2 might be a more conservative change.
> 
> Option 1 sounds like a clean way.  But to compare the resolved signature we have to resolve the types anyway so maybe it's better to create the second ciSignature lazily when we need it.  I updated the webrev.

John, can you re-review the changes please?

-- Chris

> 
> -- Chris
> 
>> 
>> (This unloaded method concept is very tricky, even kludgey.)
>> 
>> -- John
>