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

Wed Jan 6 23:26:01 UTC 2016

On 1/6/16 4:08 PM, Coleen Phillimore wrote:
>
>
> 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.

I don't think so... :-(

Dan


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