• Type: Enhancement
• Component: hotspot
• Sub-Component: runtime
• Affected Version: 11

• Priority: P4
• Status: Resolved
• Resolution: Fixed

• Submitted: 2018-05-15
• Updated: 2019-05-16
• Resolved: 2018-06-23

JDK 11	JDK 12
11 b20Fixed	12Fixed

They can be implementations of private interface methods, but they should have taken a different code path by this point. 05-06-2018 Thanks David! I see it in the bytecode too now: Compiled from "TestInterfaceMethodSelection.java" class TestInterfaceMethodSelection$PA_I implements TestInterfaceMethodSelection$I { TestInterfaceMethodSelection$PA_I(); Code: 0: aload_0 1: invokespecial #1 // Method java/lang/Object."<init>":()V 4: return private java.lang.String m(); Code: 0: ldc #2 // String PA_I::m 2: areturn public java.lang.String bad_m(); Code: 0: ldc #3 // String Should not see this 2: areturn } So the actual problem for the MH Zero case was that the target is null, because the implementation of m() is private which are skipped in the itable as they cannot be implementations of interface methods. Either way, throwing AME in this case is correct. 05-06-2018 The jcod file has: 20 Utf8 "m"; // #11 MODIFIED 21 Utf8 "()Ljava/lang/String;"; // #12 22 Utf8 "bad_m"; // #13 MODIFIED the original classfile has: 20 Utf8 "real_m"; 21 Utf8 "()Ljava/lang/String;"; // #12 22 Utf8 "m"; hence we have renamed the method m() to bad_m(), and real_m() to m().. Whether or not an interface is implemented is determined by the declarations on the class not on the set of methods the class contains. 04-06-2018 Posted this for review: http://mail.openjdk.java.net/pipermail/hotspot-dev/2018-June/032738.html 04-06-2018 Updated webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8203188/webrev.02/ This now passes this test set: test/jdk/java/lang/invoke/AccessControlTest.java test/jdk/java/lang/invoke/FinalVirtualCallFromInterface.java test/jdk/java/lang/invoke/PrivateInterfaceCall.java test/jdk/java/lang/invoke/SpecialInterfaceCall.java test/jdk/java/lang/reflect/Nestmates test/hotspot/jtreg/runtime/SelectionResolution/InvokeInterfaceICCE.java test/hotspot/jtreg/runtime/SelectionResolution/InvokeInterfaceSuccessTest.java test/hotspot/jtreg/runtime/Nestmates As for David's comment on the MH changes: Throwing AME when linking to the interface is correct. What I've changed is the comment which was misleading. The code-path triggering this AME is not PrivateInterfaceCall, but TestInterfaceMethodSelection.badMHInvoke(). For example it calls a MH with PA_I which supposed to implement I, but the jcod file, PA_I.jcod renames m->bad_m(). m() is then not implemented, thus throwing AME is correct. The comment in PA_I.jcod seems misleading as it does not seem to be renaming real_m to m. That being said, I'm not at all sure how jcod files work. 04-06-2018 Apologies Severin, I didn't keep the full patch updated - though none of the changes are going to affect the basic interpreter functionality. Anyway I have a full webrev generating into: http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.full.v4/ The TestInterfaceMethodSelection test does expect AME because it tries to find a method in an interface that was subsequently deleted - the method no longer exists hence AME. This is per JVMS selection and resolution rules (which MethodHandles still have to follow). The PrivateInterfaceCallTest is verifying that the receiver implements the interface that the invokeinterface is targeting. If not then JVMS requires IncompatibleClassChangeError be thrown (not a subclass thereof like AME!). Again MethodHandles must obey this. But I can see some wiggle room in the spec here that might allow an argument for throwing a more specific subclass of ICCE. One of the things I found when developing nestmates and working with existing selection/resolution tests was that in a number of cases tests were passing for the wrong reason, precisely because they checked for instanceof ICCE only, which allowed the test to "pass" whether it threw IllegalAccessError or AbstractMethodError or ..., which was masking bugs! The key tests to initially pass are all the new tests I added: - test/jdk/java/lang/invoke/PrivateInterfaceCall.java - test/jdk/java/lang/reflect/Nestmates - test/hotspot/jtreg/runtime/Nestmates Plus the modified SelectionResolution tests you listed. Thanks. 30-05-2018 Thanks, David! The reason why the MH change was done that way was to account for test/hotspot/jtreg/runtime/Nestmates/methodSelection/TestInterfaceMethodSelection.java which expects AME. I do see a failure in test/jdk/java/lang/invoke/PrivateInterfaceCall.java and am looking into it. I wonder what the latest full version of the Nestmates patch is. Some bits have been updated in place, some have incrementals on top. It's difficult for me to get the correct latest full patch. Similarly, it's not clear to me which set of tests must pass for this feature. Perhaps this list? test/jdk/java/lang/invoke/AccessControlTest.java test/jdk/java/lang/invoke/FinalVirtualCallFromInterface.java test/jdk/java/lang/invoke/PrivateInterfaceCall.java test/jdk/java/lang/invoke/SpecialInterfaceCall.java test/jdk/java/lang/reflect/Nestmates test/hotspot/jtreg/runtime/SelectionResolution/InvokeInterfaceICCE.java test/hotspot/jtreg/runtime/SelectionResolution/InvokeInterfaceSuccessTest.java test/hotspot/jtreg/runtime/Nestmates The preliminary webrev passed all *hotspot* Nestmates tests above. As mentioned above, there are failures in the jdk tests which I'll look into... 30-05-2018 The MH change should throw an actual IncompatibleClassChangeError not AbstractMethodError. The test: test/jdk/java/lang/invoke/PrivateInterfaceCall.java checks for ICCE, not a subclass. 30-05-2018 Thanks, David! 30-05-2018 Preliminary webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8203188/webrev.01/ 29-05-2018