RFR (S) : 8004967 - Default method cause java.lang.VerifyError: Illegal use of nonvirtual function call

[Bharadwaj Yadavalli]

Thanks for your review, Karen.

As I said in an earlier mail to the mailing list, it now appears that 
this fix for the three failures of Lambda tests does not really address 
the the bug whose number is in the subject line. I am in the process of 
addressing them separately with separate code changes, bug reports and 
corresponding review requests.

I will incorporate your suggestions in the new code review request I 
will be sending later.

Thanks,

Bharadwaj

On 1/3/2013 10:52 AM, Karen Kinnear wrote:
> Bharadwaj,
>
> Thank you for sending this for review and for the fixes.
>
> A small note:
> While I agree that the changes in the checking for the interface method modifiers matches the specification,
> there are changes which are actually more restrictive than they used to be, so you want to back them out:
>
> 1. For<  jdk8 classfile, if is_protected or is_private is set - we used to ignore it. I'm sure javac enforces
> that you can't have is_public as well as either is_protected or is_private, however for customers who
> generate their own classfiles or for other static compilers, we don't want to throw a verifier error now
> for classfiles that have worked in the past.
>
> 2. Same with is_bridge, is_varargs and is_synthetic - you don't want to check those for<  1.5
>
> Looks like we want some additional test cases for those for older classfile versions, as well
> as test cases for classfile version 52 and is_synchronized and is_strict.
>
> Please also run the vm.quick.testlist tests and check if there are any other vmsqe tests that
> are verifier/classfile parser specific.
>
> thanks,
> Karen
>
> On Jan 2, 2013, at 10:54 AM, Bharadwaj Yadavalli wrote:
>
>> Updated the webrev at http://cr.openjdk.java.net/~bharadwaj/8004967/hotspot/webrev/.
>>
>> Please review the changes.
>>
>> I re-ran JCK tests and runThese. No new failures.
>>
>> Thanks,
>>
>> Bharadwaj
>>
>> On 12/27/2012 12:49 PM, Vladimir Kozlov wrote:
>>> Bharadwaj,
>>>
>>> I would check for<  jdk8 instead of !>=, it is more clear :
>>>
>>> +       if (! (_major_version>= JAVA_8_VERSION)) {
>>>
>>> Next should be one line "} else {":
>>>
>>> +     }
>>> +     else {
>>>
>>> I would move the legality table comment inside "if (is_interface)". Why is_public and is_abstract in your table are illegal before jdk5? The conditions check the opposite.
>>>
>>> Should you also check for (is_protected) for versions before jdk8? It may be not defined for those versions but for condition completeness we should check. Also I don't see checks for is_bridge, is_varargs, is_synthetic. And what about pre jdk5? I would split>=jdk5 and pre versions to follow the table:
>>>
>>> if (major_gte_8) {
>>>   ...
>>> } else if (major_gte_15) {
>>>   ...
>>> } else {
>>>   ...
>>> }
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 12/24/12 7:44 AM, Bharadwaj Yadavalli wrote:
>>>> I updated legality verification of default methods of a Java 8
>>>> interfaces. This update
>>>> fixes JDK-8004967 and three other failures in Lambda's test-ng tests.
>>>>
>>>> I also cleaned up the legality verification of interface methods of
>>>> pre-Java 8 interface
>>>> methods. I refactored the code by separating the checks depending on the
>>>> version
>>>> being pre-Java 8 or Java 8 and later.
>>>>
>>>> Please review the changes at
>>>> http://cr.openjdk.java.net/~bharadwaj/8004967/hotspot/webrev/
>>>>
>>>> Ran api, lang and vm JCK tests; no new failures.
>>>>
>>>> Thanks,
>>>>
>>>> Bharadwaj
>>>>