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

Karen Kinnear karen.kinnear at oracle.com
Fri Aug 12 19:28:41 UTC 2016


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



More information about the hotspot-runtime-dev mailing list