RFR: 8266330: itableMethodEntry::initialize() asserts with archived old classes

[calvin.cheung at oracle.com]

On 5/9/21 8:19 PM, David Holmes wrote:
> On 8/05/2021 11:03 pm, Coleen Phillimore wrote:
>> On Fri, 7 May 2021 22:32:33 GMT, Calvin Cheung <ccheung at openjdk.org> 
>> wrote:
>>
>>> #ifdef ASSERT
>>>    if (MetaspaceShared::is_in_shared_metaspace((void*)&_method) &&
>>>       !MetaspaceShared::remapped_readwrite() &&
>>>       !MetaspaceShared::is_old_class(m->method_holder())) {
>>>      // At runtime initialize_itable is rerun as part of 
>>> link_class_impl()
>>>      // for a shared class loaded by the non-boot loader.
>>>      // The dumptime itable method entry should be the same as the 
>>> runtime entry.
>>>      assert(_method == m, "sanity");
>>>    }
>>> #endif
>>>
>>>
>>> The above includes the check for 
>>> !MetaspaceShared::is_old_class(m->method_holder()).
>>> While the method_holder() is not an old class, the _klass in the 
>>> klassITable could be an old class.
>>> The proposed fix is to have the caller of the above function pass in 
>>> the _klass and add another check
>>> making sure the _klass is not an old class before the assert.
>>>
>>> Testing:
>>> - [x] tiers 1 and 2 (including the new test)
>>
>> Hi, the concept of 'old' class conflicts with the concept of 'old' 
>> methods which were ones that were redefined.  I looked at this to see 
>> how redefinition could have broken this. One of these should change 
>> their name.
>> In a future RFE, can you rename is_old_class to 
>> has_old_class_version(klass) ?
>
> Or even better define what "old" actually means in this API. It is 
> defined as a classfile version < 50, but it isn't clear why that is 
> significant to CDS. ??

I've filed https://bugs.openjdk.java.net/browse/JDK-8266822 for the above.

thanks,
Calvin
>
> Thanks,
> David
>
>> -------------
>>
>> PR: https://git.openjdk.java.net/jdk/pull/3930
>>