RFR: 8278602: CDS dynamic dump may access unloaded classes

[Ioi Lam]

On Thu, 16 Dec 2021 17:50:21 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

> Could you also add your unloads a lot test even though it doesn't reproduce this particular error without the ZGC change? It might find a similar bug under stress conditions.

OK, I'll add the test case.

> src/hotspot/share/cds/dumpTimeClassInfo.inline.hpp line 53:
> 
>> 51:     assert_lock_strong(DumpTimeTable_lock);
>> 52:     if (k->is_loader_alive()) {
>> 53:       assert(k->is_loader_alive(), "must be");
> 
> This does seem a bit paranoid and redundant here.

Oops, that's was left over code. I'll remove it.

> src/hotspot/share/cds/dumpTimeClassInfo.inline.hpp line 58:
> 
>> 56:       return result;
>> 57:     } else {
>> 58:       if (!SystemDictionaryShared::is_excluded_class(k)) {
> 
> I thought this was the original bug?  is_excluded_class() looks at mirror->signers() which if the class isn't alive, mirror->signers() will crash.  This has to be in the k->is_loader_alive() too.

is_excluded_class() only checks the DumpTimeClassInfo::_is_excluded field. It doesn't examine the mirror->signers(). The crash happened with SystemDictionaryShared::check_excluded_classes(), which does examine the signers.


bool SystemDictionaryShared::is_excluded_class(InstanceKlass* k) {
  assert(_no_class_loading_should_happen, "sanity");
  assert_lock_strong(DumpTimeTable_lock);
  Arguments::assert_is_dumping_archive();
  DumpTimeClassInfo* p = find_or_allocate_info_for_locked(k);
  return (p == NULL) ? true : p->is_excluded();
}

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

PR: https://git.openjdk.java.net/jdk/pull/6859