[lworld] RFR: 8248003: [lworld] [lw3] VM crashes when classes with inline type fields are loaded from CDS archive

[Frederic Parain]

Ioi,

Thank you for reviewing these changes.
Line 1497 was added when I was trying to be more selective in 
InstanceKlass::remove_unshareable_info() but it didn’t work out.
After unconditional reset at instanceKlass.cpp:2669, then
line 1497 in systemDictionary.cpp is not needed anymore. Thank you
for spotting this.

I’m looking at RewriteBytecodesTest.java.

Fred


> On Jun 22, 2020, at 14:25, Ioi Lam <ioi.lam at oracle.com> wrote:
> 
> Hi Fred,
> 
> The changesets look OK to me. It is mixed with both CDS changes and function renaming. I reviewed only the CDS changes.
> 
> instanceKlass.cpp
> 
> 2666   if (has_inline_type_fields()) {
> 2667     for (AllFieldStream fs(fields(), constants()); !fs.done(); fs.next()) {
> 2668       if (Signature::basic_type(fs.signature()) == T_VALUETYPE) {
> 2669         reset_inline_type_field_klass(fs.index());
> 2670       }
> 2671     }
> 2672   }
> 
> systemDictionary.cpp:
> 
> 1484   if (ik->has_inline_type_fields()) {
> 1485     for (AllFieldStream fs(ik->fields(), ik->constants()); !fs.done(); fs.next()) {
> 1486       if (Signature::basic_type(fs.signature()) == T_VALUETYPE) {
> 1487         if (!fs.access_flags().is_static()) {
> 1488           // Pre-load inline class
> 1489           Klass* real_k = SystemDictionary::resolve_inline_type_field_or_fail(&fs,
> 1490             class_loader, protection_domain, true, CHECK_NULL);
> 1491           Klass* k = ik->get_inline_type_field_klass_or_null(fs.index());
> 1492           if (real_k != k) {
> 1493             // oops, the app has substituted a different version of k!
> 1494             return NULL;
> 1495           }
> 1496         } else {
> 1497          ik->reset_inline_type_field_klass(fs.index());
> 1498         }
> 
> [1] Is line 1497 still necessary?
> [2] For testing line 1494, I would recommend writing a new test case that dynamically redefines a "Q" class. You can see an example:
> 
> test/hotspot/jtreg/runtime/cds/appcds/RewriteBytecodesTest.java
> 
> Thanks
> - Ioi
> 
> On 6/22/20 5:56 AM, Frederic Parain wrote:
>> Please review this changeset fixing several issues in CDS related to inline type metadata removal and restoration.
>> 
>> Thank you,
>> 
>> Fred
>> 
>> -------------
>> 
>> Commit messages:
>>  - Fix inline type support in CDS
>> 
>> Changes: https://git.openjdk.java.net/valhalla/pull/95/files
>>  Webrev: https://webrevs.openjdk.java.net/valhalla/95/webrev.00
>>   Issue: https://bugs.openjdk.java.net/browse/JDK-8248003
>>   Stats: 151 lines in 22 files changed: 93 ins; 5 del; 53 mod
>>   Patch: https://git.openjdk.java.net/valhalla/pull/95.diff
>>   Fetch: git fetch https://git.openjdk.java.net/valhalla pull/95/head:pull/95
>> 
>> PR: https://git.openjdk.java.net/valhalla/pull/95
>