Skip to content
Permalink
Browse files
8240481: Remove CDS usage of InstanceKlass::is_in_error_state
Track the classes which fail verification during CDS dumping in DumpTimeSharedClassInfo.

Reviewed-by: iklam, minqi
  • Loading branch information
calvinccheung committed Mar 4, 2020
1 parent edb59b5 commit 5229896f4fdb6313775ccc98f2661782dafe18c1
@@ -76,6 +76,7 @@ class DumpTimeSharedClassInfo: public CHeapObj<mtClass> {
};

InstanceKlass* _klass;
bool _failed_verification;
int _id;
int _clsfile_size;
int _clsfile_crc32;
@@ -84,6 +85,7 @@ class DumpTimeSharedClassInfo: public CHeapObj<mtClass> {

DumpTimeSharedClassInfo() {
_klass = NULL;
_failed_verification = false;
_id = -1;
_clsfile_size = -1;
_clsfile_crc32 = -1;
@@ -124,7 +126,15 @@ class DumpTimeSharedClassInfo: public CHeapObj<mtClass> {

bool is_excluded() {
// _klass may become NULL due to DynamicArchiveBuilder::set_to_null
return _excluded || _klass == NULL;
return _excluded || _failed_verification || _klass == NULL;
}

void set_failed_verification() {
_failed_verification = true;
}

bool failed_verification() {
return _failed_verification;
}
};

@@ -1108,17 +1118,21 @@ bool SystemDictionaryShared::should_be_excluded(InstanceKlass* k) {
return true;
}
if (k->init_state() < InstanceKlass::linked) {
// In static dumping, we will attempt to link all classes. Those that fail to link will
// be marked as in error state.
assert(DynamicDumpSharedSpaces, "must be");
// In CDS dumping, we will attempt to link all classes. Those that fail to link will
// be recorded in DumpTimeSharedClassInfo.
Arguments::assert_is_dumping_archive();

// TODO -- rethink how this can be handled.
// We should try to link ik, however, we can't do it here because
// 1. We are at VM exit
// 2. linking a class may cause other classes to be loaded, which means
// a custom ClassLoader.loadClass() may be called, at a point where the
// class loader doesn't expect it.
warn_excluded(k, "Not linked");
if (has_class_failed_verification(k)) {
warn_excluded(k, "Failed verification");
} else {
warn_excluded(k, "Not linked");
}
return true;
}
if (k->major_version() < 50 /*JAVA_6_VERSION*/) {
@@ -1187,6 +1201,22 @@ bool SystemDictionaryShared::is_excluded_class(InstanceKlass* k) {
return find_or_allocate_info_for(k)->is_excluded();
}

void SystemDictionaryShared::set_class_has_failed_verification(InstanceKlass* ik) {
Arguments::assert_is_dumping_archive();
find_or_allocate_info_for(ik)->set_failed_verification();
}

bool SystemDictionaryShared::has_class_failed_verification(InstanceKlass* ik) {
Arguments::assert_is_dumping_archive();
if (_dumptime_table == NULL) {
assert(DynamicDumpSharedSpaces, "sanity");
assert(ik->is_shared(), "must be a shared class in the static archive");
return false;
}
DumpTimeSharedClassInfo* p = _dumptime_table->get(ik);
return (p == NULL) ? false : p->failed_verification();
}

class IterateDumpTimeSharedClassTable : StackObj {
MetaspaceClosure *_it;
public:
@@ -292,6 +292,8 @@ class SystemDictionaryShared: public SystemDictionary {
bool from_is_array, bool from_is_object) NOT_CDS_RETURN_(false);
static void check_verification_constraints(InstanceKlass* klass,
TRAPS) NOT_CDS_RETURN;
static void set_class_has_failed_verification(InstanceKlass* ik) NOT_CDS_RETURN;
static bool has_class_failed_verification(InstanceKlass* ik) NOT_CDS_RETURN_(false);
static bool is_builtin(InstanceKlass* k) {
return (k->shared_classpath_index() != UNREGISTERED_INDEX);
}
@@ -1713,20 +1713,6 @@ class LinkSharedClassesClosure : public KlassClosure {
}
};

class CheckSharedClassesClosure : public KlassClosure {
bool _made_progress;
public:
CheckSharedClassesClosure() : _made_progress(false) {}

void reset() { _made_progress = false; }
bool made_progress() const { return _made_progress; }
void do_klass(Klass* k) {
if (k->is_instance_klass() && InstanceKlass::cast(k)->check_sharing_error_state()) {
_made_progress = true;
}
}
};

void MetaspaceShared::link_and_cleanup_shared_classes(TRAPS) {
// We need to iterate because verification may cause additional classes
// to be loaded.
@@ -1736,18 +1722,6 @@ void MetaspaceShared::link_and_cleanup_shared_classes(TRAPS) {
ClassLoaderDataGraph::unlocked_loaded_classes_do(&link_closure);
guarantee(!HAS_PENDING_EXCEPTION, "exception in link_class");
} while (link_closure.made_progress());

if (_has_error_classes) {
// Mark all classes whose super class or interfaces failed verification.
CheckSharedClassesClosure check_closure;
do {
// Not completely sure if we need to do this iteratively. Anyway,
// we should come here only if there are unverifiable classes, which
// shouldn't happen in normal cases. So better safe than sorry.
check_closure.reset();
ClassLoaderDataGraph::unlocked_loaded_classes_do(&check_closure);
} while (check_closure.made_progress());
}
}

void MetaspaceShared::prepare_for_dumping() {
@@ -1875,7 +1849,8 @@ int MetaspaceShared::preload_classes(const char* class_list_path, TRAPS) {
// Returns true if the class's status has changed
bool MetaspaceShared::try_link_class(InstanceKlass* ik, TRAPS) {
assert(DumpSharedSpaces, "should only be called during dumping");
if (ik->init_state() < InstanceKlass::linked) {
if (ik->init_state() < InstanceKlass::linked &&
!SystemDictionaryShared::has_class_failed_verification(ik)) {
bool saved = BytecodeVerificationLocal;
if (ik->loader_type() == 0 && ik->class_loader() == NULL) {
// The verification decision is based on BytecodeVerificationRemote
@@ -1893,7 +1868,7 @@ bool MetaspaceShared::try_link_class(InstanceKlass* ik, TRAPS) {
log_warning(cds)("Preload Warning: Verification failed for %s",
ik->external_name());
CLEAR_PENDING_EXCEPTION;
ik->set_in_error_state();
SystemDictionaryShared::set_class_has_failed_verification(ik);
_has_error_classes = true;
}
BytecodeVerificationLocal = saved;
@@ -763,7 +763,7 @@ bool InstanceKlass::link_class_or_fail(TRAPS) {
}

bool InstanceKlass::link_class_impl(TRAPS) {
if (DumpSharedSpaces && is_in_error_state()) {
if (DumpSharedSpaces && SystemDictionaryShared::has_class_failed_verification(this)) {
// This is for CDS dumping phase only -- we use the in_error_state to indicate that
// the class has failed verification. Throwing the NoClassDefFoundError here is just
// a convenient way to stop repeat attempts to verify the same (bad) class.
@@ -2373,12 +2373,10 @@ void InstanceKlass::metaspace_pointers_do(MetaspaceClosure* it) {
void InstanceKlass::remove_unshareable_info() {
Klass::remove_unshareable_info();

if (is_in_error_state()) {
if (SystemDictionaryShared::has_class_failed_verification(this)) {
// Classes are attempted to link during dumping and may fail,
// but these classes are still in the dictionary and class list in CLD.
// Check in_error state first because in_error is > linked state, so
// is_linked() is true.
// If there's a linking error, there is nothing else to remove.
// If the class has failed verification, there is nothing else to remove.
return;
}

@@ -2473,38 +2471,6 @@ void InstanceKlass::restore_unshareable_info(ClassLoaderData* loader_data, Handl
}
}

// returns true IFF is_in_error_state() has been changed as a result of this call.
bool InstanceKlass::check_sharing_error_state() {
assert(DumpSharedSpaces, "should only be called during dumping");
bool old_state = is_in_error_state();

if (!is_in_error_state()) {
bool bad = false;
for (InstanceKlass* sup = java_super(); sup; sup = sup->java_super()) {
if (sup->is_in_error_state()) {
bad = true;
break;
}
}
if (!bad) {
Array<InstanceKlass*>* interfaces = transitive_interfaces();
for (int i = 0; i < interfaces->length(); i++) {
InstanceKlass* iface = interfaces->at(i);
if (iface->is_in_error_state()) {
bad = true;
break;
}
}
}

if (bad) {
set_in_error_state();
}
}

return (old_state != is_in_error_state());
}

void InstanceKlass::set_class_loader_type(s2 loader_type) {
switch (loader_type) {
case ClassLoader::BOOT_LOADER:
@@ -1268,7 +1268,6 @@ class InstanceKlass: public Klass {
assert(DumpSharedSpaces, "only call this when dumping archive");
_init_state = initialization_error;
}
bool check_sharing_error_state();

private:
// initialization state

0 comments on commit 5229896

Please sign in to comment.