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

[David Holmes]

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

Thanks,
David

> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/3930
>