RFR: Lambda: 8026066 Invokeinterface throw ICCE for static methods

[Karen Kinnear]

Coleen,

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.
> Ok.
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".

thanks,
Karen
> 
> Thanks,
> Coleen
> 
>> 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.
>> 
>> Coleen
>> 
>> 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:
>>> defmeth
>>> 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
>>> itabletests/intfbug
>>> 
>>> thanks,
>>> KAren
>> 
>