RFR: 8145148: InterfaceMethod CP entry pointing to a class should cause ICCE
Coleen Phillimore
coleen.phillimore at oracle.com
Fri Jan 29 14:51:07 UTC 2016
I agree. This is great news! The code still looks good. I tried to
update the links and believe it's blocked on 8143320 and 8147755.
Coleen
On 1/29/16 4:44 AM, Vladimir Ivanov wrote:
> That's awesome! My congratulations with success in this multi-project
> effort! :-)
>
> Best regards,
> Vladimir Ivanov
>
> On 1/29/16 7:35 AM, Yumin Qi wrote:
>> Vladimir and Coleen,
>>
>> Based on fix of 8143320 (which passed synced jdk from future ASM5.1),
>> I think the relax can be removed now.
>> New webrev (based on current hs-rt repo) remove relax check on
>> MethodHandle resolution and some copyright year to 2016.
>>
>> http://cr.openjdk.java.net/~minqi/8145148/webrev-02/
>>
>> Testing on defmeth (fix 8143320) with jdk (contains new version of
>> Handle from ASM5.1). Now the only issue is waiting for vm test
>> co-location project so it can be built with current jdk.
>>
>> on testlist/vm.defmeth.testlist:
>>
>> TOTAL TESTS IN RUN: 696
>> TEST PASS: 696; 100% rate
>> TEST FAIL: 0; 0% rate
>> TEST UNDEFINED: 0; 0% rate
>> TEST INCOMPLETE: 0; 0% rate
>> TESTS NOT RUN: 0
>>
>> TOTAL TEST IN TESTLIST: 696
>>
>>
>>
>> Thanks
>> Yumin
>>
>> On 1/20/2016 10:54 AM, Yumin Qi wrote:
>>> Hi, Vladimir
>>>
>>> I filed another bug 8147755 for ASM which now I have a fix for it:
>>> make asm create correct constant tag for a handle pointing to
>>> interface static method.
>>> https://bugs.openjdk.java.net/browse/JDK-8147755
>>>
>>> I have a simple test case showed it works to generate a callsite
>>> for static I.m:
>>>
>>> Constant pool:
>>> ....
>>> #16 = Utf8 Calling static I.m():
>>> #17 = String #16 // Calling static I.m():
>>> #18 = Utf8 java/io/PrintStream
>>> #19 = Class #18 // java/io/PrintStream
>>> #20 = Utf8 println
>>> #21 = Utf8 (Ljava/lang/String;)V
>>> #22 = NameAndType #20:#21 //
>>> println:(Ljava/lang/String;)V
>>> #23 = Methodref #19.#22 //
>>> java/io/PrintStream.println:(Ljava/lang/String;)V
>>> #24 = Utf8 I
>>> #25 = Class #24 // I
>>> #26 = Utf8 m
>>> #27 = NameAndType #26:#6 // m:()V
>>> #28 = InterfaceMethodref #25.#27 // I.m:()V
>>> #29 = MethodHandle #6:#28 // invokestatic I.m:()V
>>> #30 = Utf8 java/lang/invoke/MethodHandle
>>> ....
>>> {
>>> public static void staticCallIm();
>>> descriptor: ()V
>>> flags: ACC_PUBLIC, ACC_STATIC
>>> Code:
>>> stack=3, locals=1, args_size=0
>>> 0: getstatic #15 // Field
>>> java/lang/System.out:Ljava/io/PrintStream;
>>> 3: ldc #17 // String Calling static
>>> I.m():
>>> 5: invokevirtual #23 // Method
>>> java/io/PrintStream.println:(Ljava/lang/String;)V
>>> 8: ldc #29 // MethodHandle
>>> invokestatic I.m:()V
>>> 10: invokevirtual #34 // Method
>>> java/lang/invoke/MethodHandle.invoke:()V
>>> 13: return
>>> }
>>>
>>> So now keep the original fix of 8087223 as 8145148 fix and relax
>>> for the check in method handle method tag no longer needed with
>>> 8147755 solved.
>>>
>>> Still, I need to test defmeth again to see the failed cases --- and
>>> the fix will be in defmeth use the new added Handle.
>>>
>>> Stas and Dmitry:
>>> 8143320 fix may change if tests on new binary with 8145148/8147755
>>> shows defmeth needs changes too(I think so, since I add a new Handle
>>> constructor).
>>>
>>> The previous webrev now has conflict with the current repo so
>>> need make new webrev and send codereview again.
>>>
>>> Thanks
>>> Yumin
>>>
>>> On 1/14/2016 2:51 AM, Vladimir Ivanov wrote:
>>>> Yumin,
>>>>
>>>> Please, file a follow-up bug for the missing check (interface static
>>>> method referenced by a MethodHandle). We shouldn't leave it
>>>> half-fixed.
>>>>
>>>> I'd prefer to keep the original VM fix and quarantine affected tests
>>>> until ASM is fixed, but 2-phase fix is also fine with me.
>>>>
>>>> Not sure how to handle ASM problem. Is the problem present in the
>>>> latest ASM version?
>>>>
>>>> The best way would be to get it fixed upstream first and then sync
>>>> internal copy with upstream. defmeth tests use the library bundled
>>>> with the JDK.
>>>>
>>>> Best regards,
>>>> Vladimir Ivanov
>>>>
>>>> On 12/21/15 9:52 PM, Yumin Qi wrote:
>>>>> Please review:
>>>>>
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8145148
>>>>> webrev:http://cr.openjdk.java.net/~minqi/8145148/webrev-01/
>>>>>
>>>>> This is REDO for bug 8087223, which was backed out due to failed on
>>>>> 8143317 and 8143320.
>>>>> The defmeth encoutered new failures after the fix, which now has
>>>>> correct
>>>>> constant tags. The test bugs will be addressed in 8143320, which
>>>>> based
>>>>> on vm/runtime/defmeth and 8143317, which in jdk/test.
>>>>>
>>>>> One difference from the attached for this webrev is that in
>>>>> MethodHandle
>>>>> resolution, I relax the check for one specific case: Using
>>>>> MethodHandle
>>>>> to invoke static default interface method. In this case, we must use
>>>>> invokestatic (JVM_REF_invokeStatic), which in defmeth it will
>>>>> generate
>>>>> method tag for interface which violates JVMS-5.4.3.3 (it should
>>>>> create
>>>>> interface method tag!). ASM currently has no API to generate
>>>>> invokeinterface for a interface static default method through
>>>>> MethodHandle invocation.
>>>>>
>>>>> Tests: JPRT, runtime quick test list (the fixed version for 8143320).
>>>>> Note the fix still fails 8143320 and 8143317 which are not fixed
>>>>> at the
>>>>> momment.
>>>>>
>>>>>
>>>>> Thanks
>>>>> Yumin
>>>>>
>>>>>
>>>>> Original post:
>>>>> -----------------------------------------------------------------------------------------------------------------------------------------------------
>>>>>
>>>>>
>>>>>
>>>>> Please review:
>>>>>
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8087223
>>>>> webrev: http://cr.openjdk.java.net/~minqi/8087223/webrev-02/
>>>>>
>>>>> According to JVMS-8:
>>>>>
>>>>> JVMS-5.4.3.3 Method Resolution:
>>>>> " If C is an interface, method resolution throws an
>>>>> IncompatibleClassChangeError."
>>>>> JVMS-5.4.3.4 Interface Method Resolution:
>>>>> "If C is not an interface, interface method resolution throws an
>>>>> IncompatibleClassChangeError"
>>>>>
>>>>> When invoke a method with resolved to an interface method, or
>>>>> invoke a
>>>>> interface method with resolved to an instance method, ICCE should be
>>>>> thrown. The case usually happens when using tools like asmtools or
>>>>> jdk.internal.org.objectweb.asm to generate java bytecode.
>>>>>
>>>>> The fix is carrying the constantTag for the method at call and
>>>>> check if
>>>>> tag is consistent with the method called. Doing this by adding a
>>>>> member
>>>>> of constantTag, _tag, to LinkInfo, and check tag in resolve
>>>>> functions
>>>>> to see if tag matched with the correct method.
>>>>>
>>>>> The fix solved the problem when call is from interpreter and
>>>>> compiler,
>>>>> bug for MethodHandle invoke, which should be addressed in another
>>>>> bug,
>>>>> since the MethodHandle does not come with a byte stream and
>>>>> getting the
>>>>> constant pool index at the invoke is not possible. It will be
>>>>> addressed
>>>>> in another bug.
>>>>>
>>>>> Tests: test case (added, minor modified from bug), JPRT, rutime quick
>>>>> test list(in progress).
>>>>> manually tested: 1) -Xint
>>>>> 2) -Xcomp
>>>>> 3) -Xcomp -XX:-TieredCompiltion
>>>>> 4) -Xcomp -XX:+TieredCompilation
>>>>>
>>>>> Thanks to Coleen for helping fixed constant pool index and cleaned
>>>>> LinkInfo.
>>>>> ----------------------------------------------------------------------------------------------------------------------------------
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Second revision attached.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Thanks
>>>>> Yumin
>>>
>>
More information about the hotspot-runtime-dev
mailing list