RFR: Lambda: 8026066 Invokeinterface throw ICCE for static methods
karen.kinnear at oracle.com
Mon Dec 2 19:27:12 PST 2013
Thank you so much for the quick review.
On Dec 2, 2013, at 7:24 PM, Coleen Phillimore wrote:
> On 12/02/2013 06:35 PM, Coleen Phillimore wrote:
>> Karen, This looks good. It's subtle that you only have to look in the immediate class and not up the hierarchy for resolve_interface_method because the incoming class is always an interface now, but I like it.
> Actually, I misread this. Both LinkResolver::lookup_method_in_klasses() and LinkResolver::lookup_instance_method_in_klasses() look up the class hierarchy in InstanceKlass::uncached_lookup_method(). The latter does a re-lookup if it gets a static method as a result.
Yes. Thanks for double-checking.
>> For resolve_method() is it true that the incoming class is never an interface? I think then you can remove the check and require_methodref in that case. That would be a good change.
If you run with the verifier on, it is true. If you don't, this will catch you. Run PrivateMethodsTest testPrivateInvokeVirtual
-Xverify:none and this catches you.
>> I have two nits. Can you realign the parameters in the calls where you added a parameter so that the lines aren't so long? The second is that the names nostatics is odd. I think it would be more discriptive as disallow_static_methods or something like that. The other boolean checkpolymorphism should be check_polymorphism (this is a nit but it's the coding style and more readable). I don't know why it's false for the interface method case.
> Can you also fix the alignment of the parameters to lookup_method_in_klasses since you added a parameter. This source file has all these unreadable long parameter lines that we should fix as we change (or add) parameters.
Let me get back to you on the "nits".
>> The other thing is can you add "@ignore until bug 8028741 is fixed" in test test/runtime/8026365/InvokeSpecialAnonTest.java with this putback? It'll keep us at 0 failures until the other bug is fixed.
>> On 11/30/2013 05:39 PM, Karen Kinnear wrote:
>>> Please review:
>>> webrev: http://cr.openjdk.java.net/~acorn/8026066.3/webrev
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8026066
>>> Fix invokeinterface, invokespecial and invokestatic to all use common interface method
>>> resolution and invokeinterface and invokespecial to throw ICCE if a static method is found.
>>> Thanks to Harold for the fix for the linkResolver lookup_instance_method_in_klasses. This
>>> fix passes here and is specifically tested by 8028438 coming next for review.
>>> Note: causes hotspot/test/runtime/8026365/InvokeSpecialAnonTest.java to fail - should
>>> be fixed by Lois with 8028741 - skip static and non-public methods in j.l.Object.
>>> I added this failure comment to that bug, hopefully in the correct syntax to show up as a known bug
>>> and so Lois could use that as a partial test.
>>> Tests run:
>>> jck.lang, jck.vm
>>> jtreg java.util, java.lang, lambda
>>> jtreg langtools/test/lambdaShapes/.../test/vm
>>> 2009 invoke* tests
>>> hotspot jtreg: runtime, compiler
>>> nsk vm.quick, vm.mlvm
More information about the hotspot-runtime-dev