RFR: 8145148: InterfaceMethod CP entry pointing to a class should cause ICCE
Vladimir Ivanov
vladimir.x.ivanov at oracle.com
Fri Jan 29 09:44:09 UTC 2016
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