RFR: 8145148: InterfaceMethod CP entry pointing to a class should cause ICCE

[Coleen Phillimore]

On 1/14/16 2:57 PM, Yumin Qi wrote:
> Hi, Vladimir
>
>   I will integrate the current version, filed a new bug to follow the 
> existing issue: asm should generate correct code for invoking an 
> interface static method handle.
>   https://bugs.openjdk.java.net/browse/JDK-8147419
>
>
>   Latest ASM version is 5.0.4, I could not see the problem is addressed.
>   http://asm.ow2.org/history.html
>
>   The testcases in defmethod will be kept. When this problem solved 
> in  ASM, we can remove the relax in hotspot under the new bug 8147419.

I don't understand this.  Will these testcases in defmeth fail like they 
did in https://bugs.openjdk.java.net/browse/JDK-8143320 (ie. 100s of tests?)

Or was the failure because of the check that you relax for 8147419 .

Obviously you can't check it in if you get 100s of test failures.

Is https://bugs.openjdk.java.net/browse/JDK-8143317 fix reviewed and 
will be checked in before this change?

This change still looks good to me but the test issues should be 
resolved before you check it in.
>
>   Coleen, how about this plan? Comment please.

That's what I think.

Coleen

>
>   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
>