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

[Karen Kinnear]

Good timing. Fixed the copyright.

And you are right that I did not need the compiler directive - with jigsaw we don’t need the -Dignore.symbol.file anymore
because we handle this with module boundaries. jtreg added an -XaddExports:java.bae/jdk.internal.org.objectweb.asm=ALL_UNNAMED for me.

thanks,
Karen

> On Aug 16, 2016, at 10:41 AM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
> 
> 
> http://cr.openjdk.java.net/~acorn/8163808.hs.1/webrev/test/runtime/TransitiveOverrideCFV50/TransitiveOverrideCFV50.java.html <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 <https://bugs.openjdk.java.net/browse/JDK-8163808>
>>>>>> 
>>>>>> http://cr.openjdk.java.net/~acorn/8163808.hs/webrev <http://cr.openjdk.java.net/%7Eacorn/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
>>>>> 
>>>> 
>>> 
>> 
>