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

[Karen Kinnear]

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