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

Thu Jan 14 23:52:10 UTC 2016

Coleen,

   8143317: jdk/test... this will be integrated first. Now it gets 
reviewed by Paul Sandoz, waiting for one more reviewer.
   8143320: this is the 100s failures(in fact not that many, it's 
testing against 4 versions, 49, 50, 51, 52. so about 25 kinds of 
failures). The test cases should be modified after 8145148 fix. This is 
not open issue, I will send you another email about it.

   After investigation of those failures, the result is
   1) some tests will be modified since after 8145148, the expected 
Error or Exception changed.
   2) some tests failed due to the reason in my last email:

    ASM could not generate a correct handle invoke to an interface 
static default method.
    In the new webrev of 8145148, I relax the check for invokestatic on 
an interface which should with an InterfaceMethodref tag, but now has to 
be with a Methodref. With this addressed in ASM, the relaxing check is 
no longer needed(8147419).

    So, with above steps, all the failures are about to be fixed.

Thanks
Yumin

On 1/14/2016 1:55 PM, Coleen Phillimore wrote:
>
>
> 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
>>
>