RFR: JDK-8163808 fix vtable assertion and logging for older classfiles

[Coleen Phillimore]

http://cr.openjdk.java.net/~acorn/8163808.hs.1/webrev/test/runtime/TransitiveOverrideCFV50/TransitiveOverrideCFV50.java.html

One tiny thing in case you haven't pushed it yet.   The copyright dates 
should just say 2016, since it's a new test.

Also, I'm not sure if you need @compile directive since I think jtreg 
adds -Dignore.symbol.file for you.

Thanks,
Coleen

On 8/12/16 3:28 PM, Karen Kinnear wrote:
> Added a targeted test case for class files with different class file 
> versions in the
> inheritance hierarchy.
>
> http://cr.openjdk.java.net/~acorn/8163808.hs.1/webrev/ 
> <http://cr.openjdk.java.net/%7Eacorn/8163808.hs.1/webrev/>
>
> thanks,
> Karen
>
>> On Aug 12, 2016, at 1:46 PM, Coleen Phillimore 
>> <coleen.phillimore at oracle.com <mailto:coleen.phillimore at oracle.com>> 
>> wrote:
>>
>>
>>
>> On 8/12/16 1:33 PM, Karen Kinnear wrote:
>>> Coleen,
>>>
>>> Good catch - I will make that change.
>>> Today this code is not called for arrays, but I totally appreciate 
>>> you looking at the bigger picture
>>> and preparing for potential other uses.
>>>
>>>
>>> Here is the updated lines:
>>>   KlassHandle vtklass_h = vt->klass();
>>> Klass* vtklass = vtklass_h();
>>> if (vtklass->is_instance_klass() &&
>>> (InstanceKlass::cast(vtklass)->major_version() >= 
>>> klassVtable::VTABLE_TRANSITIVE_OVERRIDE_VERSION)) {
>>>   assert(method() != NULL, "must have set method");
>>> }
>>>
>> This looks good.
>> Thanks,
>> Coleen
>>
>>> Thanks!
>>> Karen
>>>
>>>> On Aug 12, 2016, at 10:47 AM, Coleen Phillimore 
>>>> <coleen.phillimore at oracle.com 
>>>> <mailto:coleen.phillimore at oracle.com>> wrote:
>>>>
>>>>
>>>> http://cr.openjdk.java.net/~acorn/8163808.hs/webrev/src/share/vm/oops/klassVtable.cpp.udiff.html 
>>>> <http://cr.openjdk.java.net/%7Eacorn/8163808.hs/webrev/src/share/vm/oops/klassVtable.cpp.udiff.html>
>>>>
>>>> void vtableEntry::verify(klassVtable* vt, outputStream* st) {
>>>>   NOT_PRODUCT(FlagSetting fs(IgnoreLockingAssertions, true));
>>>> + KlassHandle vtklass_h = vt->klass();
>>>> + Klass* vtklass = vtklass_h();
>>>> + if (InstanceKlass::cast(vtklass)->major_version() >= 
>>>> klassVtable::VTABLE_TRANSITIVE_OVERRIDE_VERSION) {
>>>>   assert(method() != NULL, "must have set method");
>>>> + }
>>>>
>>>> I might be wrong but the vtable->klass() can be an ArrayKlass, so I 
>>>> think you have to do:
>>>>
>>>> if (vtklass->oop_is_instance() && InstanceKlass::cast(vtklass) ...)
>>>>
>>>> InstanceKlass::cast makes this assertion.  Otherwise, the code 
>>>> looks good.
>>>>
>>>> Coleen
>>>>
>>>> On 8/11/16 5:07 PM, Karen Kinnear wrote:
>>>>> Please review:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8163808
>>>>>
>>>>> http://cr.openjdk.java.net/~acorn/8163808.hs/webrev
>>>>>
>>>>> Bug: For classfiles before class file version 51, JVMS did not 
>>>>> support transitive over-ride behavior.
>>>>> Implementation needed to check this in three places, not just one. 
>>>>> Vtable size calculation is only exact
>>>>> for later classfile versions.
>>>>>
>>>>> Also fixed vtable logging output - since the method name-and-sig 
>>>>> printing was changed to also print
>>>>> the holder’s class name, we do not need to print the holder’s 
>>>>> class name separately - it was printing twice.
>>>>>
>>>>> Testing: linux-x64-slowdebug
>>>>> rbt hs-nightly-runtime.js
>>>>> jck vm,lang, api.java.lang
>>>>> small invocation tests
>>>>>
>>>>> thanks,
>>>>> Karen
>>>>
>>>
>>
>