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

[Lois Foltan]

Looks good Karen.  Thank you for the mixed class file version test!
Lois

On 8/12/2016 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/
>
> thanks,
> Karen
>
>> On Aug 12, 2016, at 1:46 PM, Coleen Phillimore <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 <https://bugs.openjdk.java.net/browse/JDK-8163808>
>>>>>
>>>>> http://cr.openjdk.java.net/~acorn/8163808.hs/webrev <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