RFR: 8145148: InterfaceMethod CP entry pointing to a class should cause ICCE
Yumin Qi
yumin.qi at oracle.com
Fri Jan 29 04:35:15 UTC 2016
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