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

[Daniel D. Daugherty]

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.

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