RFR: 8312181: CDS dynamic dump crashes when verifying unlinked class from static archive [v2]

[Calvin Cheung]

On Mon, 31 Jul 2023 16:48:10 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Calvin Cheung has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits:
>> 
>>  - Merge branch 'master' into 8312181-dynamic-dump-crash
>>  - @iklam comments
>>  - 8312181: CDS dynamic dump crashes when verifying unlinked class from static archive
>
> src/hotspot/share/cds/dumpTimeClassInfo.cpp line 165:
> 
>> 163:            "not stored with SystemDictionaryShared::init_dumptime_info");
>> 164:     assert(p->_klass == k, "Sanity");
>> 165:   }
> 
> We have lots of code that looks like this:
> 
> 
> DumpTimeClassInfo* info = get_info(k);
> info->_clsfile_size  = cfs->length();
> 
> 
> The assumption is that `k` must have a `DumpTimeClassInfo`, which means that `k` was created by the `ClassFileParser` (and not loaded with `SystemDictionary::load_shared_class()`).
> 
> With the above change, `get_info()` could potentially return null, which means that the caller now need to check and handle for the null return. This will make the code more complicated.
> 
> I think a better fix would be to avoid calling `SystemDictionaryShared::set_class_has_failed_verification()` on classes that are shared. Maybe the condition in `MetaspaceShared::try_link_class()` can be changed like this?
> 
> 
> - if (ik->is_loaded() && !ik->is_linked() && ik->can_be_verified_at_dumptime() &&
> + if (!ik->is_shared() && ik->is_loaded() && !ik->is_linked() && ik->can_be_verified_at_dumptime() &&

With the above suggestions, it means that we don't try to link any shared classes which had failed linking before.
It seems I could revert the other changes too.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15077#discussion_r1279944795