Request for reviews (S): 7092712: JSR 292: unloaded invokedynamic call sites can lead to a crash with signature types not on BCP
christian.thalinger at oracle.com
Wed Oct 12 04:39:35 PDT 2011
On Oct 11, 2011, at 8:29 PM, John Rose wrote:
> 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?
Type resolving happens in the ciSignature constructor. So when we are in equals everything is already resolved.
But as we talked on IM yesterday we can short-circuit the ciSignature creation in ciObjectFactory::get_unloaded_method
. I added some debug output to see how often the short-circuiting triggers and it triggers much more often than creating the ciSignature.
I removed the accessing class check in ciSignature::equals (see the method comment) but added a short-cut bailout on a signature pointer-compare.
Additionally I removed two debug output statements in src/share/vm/prims/methodHandleWalk.cpp that we missed.
Webrev is updated:
> -- John
More information about the hotspot-compiler-dev