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

[Karen Kinnear]

Rachel,

Thank you - I was very much hoping you would look this over, particularly the
logging changes.

thanks,
Karen

> On Aug 12, 2016, at 1:35 PM, Rachel Protacio <rachel.protacio at oracle.com> wrote:
> 
> Looks good to me!
> 
> Rachel
> 
> 
> On 8/12/2016 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");
>>   }
>> 
>> Thanks!
>> Karen
>> 
>>> On Aug 12, 2016, at 10:47 AM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
>>> 
>>> 
>>> http://cr.openjdk.java.net/~acorn/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
>