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

[Ioi Lam]

On Fri, 28 Jul 2023 21:20:56 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

> During CDS dynamic dump, VM crashes due to `assert(!k->is_shared()) failed` while linking an old class (class file version <= 49) loaded from the static archive. The fix mostly adjusts some assert statements based on static vs dynamic dumping. In `MetaspaceShared::try_link_class`, if linking of a shared class during dynamic dump has failed, the `shared_loading_failed` flag will be set to avoid linking the same class again.
> 
> Passed tiers 1 - 4 testing.

Changes requested by iklam (Reviewer).

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() &&

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

PR Review: https://git.openjdk.org/jdk/pull/15077#pullrequestreview-1555340522
PR Review Comment: https://git.openjdk.org/jdk/pull/15077#discussion_r1279604928