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