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

[Coleen Phillimore]

On 1/6/16 6:05 PM, Daniel D. Daugherty wrote:
> On 1/5/16 4:37 PM, Yumin Qi wrote:
>> On 1/5/2016 3:28 PM, Daniel D. Daugherty wrote:
>>> On 12/21/15 11:52 AM, 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.
>>>
>>> Are the failing tests quarantined? When this fix is pushed,
>>> neither the "new" or "known" failures counts should increase.
>>>
>> Do you mean 'RULE' out in bugs? No.
>
> No I do not mean 'RULE' entries. A 'RULE' entry will make a test
> failure show up as "known" and that is not what should happen here.
>
> You are changing product code in such a way that existing tests
> will fail. Yes, the tests need to be fixed, but we should not
> bother to run those tests until they are fixed. This is what
> 'quarantine' is all about.
>
> For JavaTest/JTREG tests, tests are quarantined by adding an
> "@ignore NNNNNNN" entry to the test. For Tonga tests, I believe
> the test list has to updated to include the quarantine keyword,
> but that could have changed.
>
> Please coordinate with one of our SQE folks to make sure that
> the affected tests are quarantined before you push this fix.
>
> 8143320 covers vm.defmeth tests so that's Tonga.
> 8143317 covers tests in the JDK repo so that's JavaTest/JTREG.
>
> So it looks like you need to work with SQE to get both Tonga
> and JavaTest/JTREG tests quarantined.

I thought Yumin had a fix for the tonga tests and was in the process of 
pushing it, and he should fix the JDK tests before pushing this.

Coleen

>
> Dan
>
>
>> So after the fix pushed, it still fails on the tests --- but will 
>> pass after 8143320 pushed to vm repo. It is not clear for me:
>>
>> after 8145148(this) pushed to hs-rt, the tests will fail as before.
>> after 8143320 pushed to vm (sqe), hs-rt will pass, but hs-comp will 
>> fail since I changed defmeth!
>>
>> As for 8143317, so far not assigned --- in jdk group.
>>
>> Thanks
>> Yumin
>>
>>> Dan
>>>
>>>
>>>>
>>>>
>>>> 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
>>>
>>
>