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

[John Rose]

On Oct 11, 2011, at 8:13 AM, Christian Thalinger wrote:

>>> 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?

In the ciSignature::equals method, why bother to resolve types when you first prove equality of accessing_klass?

The code in the webrev is (I think) equivalent to this->ac_kl == that->ac_kl && this->symbol == that->symbol.

Either drop the equality check on accessing_klass, or don't bother to resolve the types.

(And if you don't resolve the types, you don't really need the temporary ciSignature, since it's just a <sym, ac_kl> ordered pair check.)

Or am I missing something?

-- John
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20111011/c5cefe7f/attachment.html