--- old/src/hotspot/share/oops/instanceKlass.cpp 2020-03-06 00:25:55.530534180 -0500 +++ new/src/hotspot/share/oops/instanceKlass.cpp 2020-03-06 00:25:54.388521726 -0500 @@ -153,7 +153,9 @@ return false; } -// called to verify that k is a member of this nest +// private: called to verify that k is a static member of this nest. +// We know that k is an instance class in the same package and hence the +// same classloader. bool InstanceKlass::has_nest_member(InstanceKlass* k, TRAPS) const { assert(!is_hidden(), "unexpected hidden class"); if (_nest_members == NULL || _nest_members == Universe::the_empty_short_array()) { @@ -176,7 +178,8 @@ for (int i = 0; i < _nest_members->length(); i++) { int cp_index = _nest_members->at(i); if (_constants->tag_at(cp_index).is_klass()) { - Klass* k2 = _constants->klass_at(cp_index, CHECK_false); + Klass* k2 = _constants->klass_at(cp_index, THREAD); + assert(!HAS_PENDING_EXCEPTION, "Exceptions should not be possible here"); if (k2 == k) { log_trace(class, nestmates)("- class is listed at nest_members[%d] => cp[%d]", i, cp_index); return true; @@ -187,15 +190,13 @@ if (name == k->name()) { log_trace(class, nestmates)("- Found it at nest_members[%d] => cp[%d]", i, cp_index); - // Names match so check actual klass - this may trigger class loading if - // it doesn't match (though that should be impossible). But to be safe we - // have to check for a compiler thread executing here. - if (!THREAD->can_call_java() && !_constants->tag_at(cp_index).is_klass()) { - log_trace(class, nestmates)("- validation required resolution in an unsuitable thread"); - return false; - } - - Klass* k2 = _constants->klass_at(cp_index, CHECK_false); + // Names match so check actual klass. This may trigger class loading if + // it doesn't match though that should be impossible as it means one classloader + // has defined two different classes with the same name! A compiler thread won't be + // able to perform that loading but we can't exclude the compiler threads from + // executing this logic. But it should actually be impossible to trigger loading here. + Klass* k2 = _constants->klass_at(cp_index, THREAD); + assert(!HAS_PENDING_EXCEPTION, "Exceptions should not be possible here"); if (k2 == k) { log_trace(class, nestmates)("- class is listed as a nest member"); return true; @@ -213,148 +214,126 @@ return false; } -InstanceKlass* InstanceKlass::runtime_nest_host(TRAPS) { - // TODO: nest_host returns NULL if validation fails. Need to follow up - // the specification when to evaluate the runtime nest host. Right now - // it's only determined when a dynamic nestmate is added. - InstanceKlass* nest_host_k = nest_host(NULL, CHECK_NULL); - if (nest_host_k == NULL) { - assert(_nest_host == NULL, "should fail to validate NestNost"); - // drop the static nest information; set dynamic nest to this class - if (log_is_enabled(Trace, class, nestmates)) { - ResourceMark rm(THREAD); - log_trace(class, nestmates)("Fail to validate static nest host of %s: setting nest-host to self", - this->external_name()); - } - _nest_host = nest_host_k = this; - } - return nest_host_k; -} - // Return nest-host class, resolving, validating and saving it if needed. -// In cases where this is called from a thread that can not do classloading +// In cases where this is called from a thread that cannot do classloading // (such as a native JIT thread) then we simply return NULL, which in turn // causes the access check to return false. Such code will retry the access -// from a more suitable environment later. -InstanceKlass* InstanceKlass::nest_host(Symbol* validationException, TRAPS) { +// from a more suitable environment later. Otherwise the _nest_host is always +// set once this method returns. +// Any errors from nest-host resolution must be preserved so they can be queried +// from higher-level access checking code, and reported as part of access checking +// exceptions. +InstanceKlass* InstanceKlass::nest_host(TRAPS) { InstanceKlass* nest_host_k = _nest_host; - if (nest_host_k == NULL) { - // need to resolve and save our nest-host class. This could be attempted - // concurrently but as the result is idempotent and we don't use the class - // then we do not need any synchronization beyond what is implicitly used - // during class loading. - if (_nest_host_index != 0) { // we have a real nest_host - // Before trying to resolve check if we're in a suitable context - if (!THREAD->can_call_java() && !_constants->tag_at(_nest_host_index).is_klass()) { - if (log_is_enabled(Trace, class, nestmates)) { - ResourceMark rm(THREAD); - log_trace(class, nestmates)("Rejected resolution of nest-host of %s in unsuitable thread", - this->external_name()); - } - return NULL; - } + if (nest_host_k != NULL) { + return nest_host_k; + } - if (log_is_enabled(Trace, class, nestmates)) { + const bool doLog = log_is_enabled(Trace, class, nestmates); + + // need to resolve and save our nest-host class. This could be attempted + // concurrently but as the result is idempotent and we don't use the class + // then we do not need any synchronization beyond what is implicitly used + // during class loading. + if (_nest_host_index != 0) { // we have a real nest_host + // Before trying to resolve check if we're in a suitable context + if (!THREAD->can_call_java() && !_constants->tag_at(_nest_host_index).is_klass()) { + if (doLog) { ResourceMark rm(THREAD); - log_trace(class, nestmates)("Resolving nest-host of %s using cp entry for %s", - this->external_name(), - _constants->klass_name_at(_nest_host_index)->as_C_string()); + log_trace(class, nestmates)("Rejected resolution of nest-host of %s in unsuitable thread", + this->external_name()); } + return NULL; // sentinel to say "try again from a different context" + } - Klass* k = _constants->klass_at(_nest_host_index, THREAD); - if (HAS_PENDING_EXCEPTION) { - Handle exc_h = Handle(THREAD, PENDING_EXCEPTION); - if (validationException == NULL && exc_h->is_a(SystemDictionary::LinkageError_klass())) { - // clear exception if fails to resolve the nest host class - CLEAR_PENDING_EXCEPTION; - } - // throw a new NCDFE with the original as its cause, and a clear msg - if (exc_h->is_a(SystemDictionary::NoClassDefFoundError_klass()) && validationException != NULL) { - // throw a new NCDFE with the original as its cause, and a clear msg - ResourceMark rm(THREAD); - char buf[200]; - CLEAR_PENDING_EXCEPTION; - jio_snprintf(buf, sizeof(buf), - "Unable to load nest-host class (%s) of %s", - _constants->klass_name_at(_nest_host_index)->as_C_string(), - this->external_name()); - log_trace(class, nestmates)("%s - NoClassDefFoundError", buf); - THROW_MSG_CAUSE_NULL(vmSymbols::java_lang_NoClassDefFoundError(), buf, exc_h); - } - // All other exceptions pass through (OOME, StackOverflowError, LinkageErrors etc). - return NULL; - } + if (doLog) { + ResourceMark rm(THREAD); + log_trace(class, nestmates)("Resolving nest-host of %s using cp entry for %s", + this->external_name(), + _constants->klass_name_at(_nest_host_index)->as_C_string()); + } + Klass* k = _constants->klass_at(_nest_host_index, THREAD); + if (HAS_PENDING_EXCEPTION) { + ResourceMark rm(THREAD); + stringStream ss; + char* target_host_class = _constants->klass_name_at(_nest_host_index)->as_C_string(); + ss.print("Nest host resolution of %s with host %s failed: ", + this->external_name(), target_host_class); + java_lang_Throwable::print(PENDING_EXCEPTION, &ss); + _nest_host_res_error = ss.as_string(true /* on C-heap */); + CLEAR_PENDING_EXCEPTION; + if (doLog) { + log_trace(class, nestmates)("%s", _nest_host_res_error); + } + } else { // A valid nest-host is an instance class in the current package that lists this - // class as a nest member. If any of these conditions are not met we post the - // requested exception type (if any) and return NULL - + // class as a nest member. If any of these conditions are not met the class is + // its own nest-host. const char* error = NULL; // JVMS 5.4.4 indicates package check comes first if (is_same_class_package(k)) { - // Now check actual membership. We can't be a member if our "host" is // not an instance class. if (k->is_instance_klass()) { nest_host_k = InstanceKlass::cast(k); - - bool is_member = nest_host_k->has_nest_member(this, CHECK_NULL); - if (is_member) { - // save resolved nest-host value - _nest_host = nest_host_k; - - if (log_is_enabled(Trace, class, nestmates)) { - ResourceMark rm(THREAD); - log_trace(class, nestmates)("Resolved nest-host of %s to %s", - this->external_name(), k->external_name()); + bool is_member = nest_host_k->has_nest_member(this, THREAD); + // exception is rare, perhaps impossible + if (!HAS_PENDING_EXCEPTION) { + if (is_member) { + _nest_host = nest_host_k; // save resolved nest-host value + if (doLog) { + ResourceMark rm(THREAD); + log_trace(class, nestmates)("Resolved nest-host of %s to %s", + this->external_name(), k->external_name()); + } + return nest_host_k; + } else { + error = "current type is not listed as a nest member"; } - return nest_host_k; + } else { + stringStream ss; + ss.print("exception on member check: "); + java_lang_Throwable::print(PENDING_EXCEPTION, &ss); + error = ss.as_string(); } + } else { + error = "host is not an instance class"; } - error = "current type is not listed as a nest member"; } else { error = "types are in different packages"; } - if (log_is_enabled(Trace, class, nestmates)) { + // something went wrong, so record what and log it + { ResourceMark rm(THREAD); - log_trace(class, nestmates) - ("Type %s (loader: %s) is not a nest member of " - "resolved type %s (loader: %s): %s", - this->external_name(), - this->class_loader_data()->loader_name_and_id(), - k->external_name(), - k->class_loader_data()->loader_name_and_id(), - error); - } + stringStream ss; + ss.print("Type %s (loader: %s) is not a nest member of type %s (loader: %s): %s", + this->external_name(), + this->class_loader_data()->loader_name_and_id(), + k->external_name(), + k->class_loader_data()->loader_name_and_id(), + error); + _nest_host_res_error = ss.as_string(true /* on C-heap */); - if (validationException != NULL && THREAD->can_call_java()) { - ResourceMark rm(THREAD); - Exceptions::fthrow(THREAD_AND_LOCATION, - validationException, - "Type %s (loader: %s) is not a nest member of %s (loader: %s): %s", - this->external_name(), - this->class_loader_data()->loader_name_and_id(), - k->external_name(), - k->class_loader_data()->loader_name_and_id(), - error - ); - } - return NULL; - } else { - if (log_is_enabled(Trace, class, nestmates)) { - ResourceMark rm(THREAD); - log_trace(class, nestmates)("Type %s is not part of a nest: setting nest-host to self", - this->external_name()); + if (doLog) { + log_trace(class, nestmates)("%s", _nest_host_res_error); + } } - // save resolved nest-host value - return (_nest_host = this); + } + } else { + if (doLog) { + ResourceMark rm(THREAD); + log_trace(class, nestmates)("Type %s is not part of a nest: setting nest-host to self", + this->external_name()); } } - return nest_host_k; -} + // Either not in an explicit nest, or else an error occurred, so + // the nest-host is set to `this`. + return (_nest_host = this); +} // Dynamic nest member support: set this class's nest host to the given class. // This occurs as part of the class definition, as soon as the instanceKlass @@ -369,6 +348,8 @@ assert(is_hidden(), "must be a hidden class"); assert(host != NULL, "NULL nest host specified"); assert(_nest_host == NULL, "current class has resolved nest-host"); + assert(_nest_host_res_error == NULL, "unexpected nest host resolution error exists: %s", + _nest_host_res_error); assert((host->_nest_host == NULL && host->_nest_host_index == 0) || (host->_nest_host == host), "proposed host is not a valid nest-host"); // Can't assert this as package is not set yet: @@ -376,16 +357,17 @@ if (log_is_enabled(Trace, class, nestmates)) { ResourceMark rm(THREAD); + const char* msg = ""; // a hidden class does not expect a statically defined nest-host if (_nest_host_index > 0) { - log_trace(class, nestmates)("Type %s is a dynamic nest member of %s: the NestHost attribute in the current class is ignored", - this->external_name(), - host->external_name()); + msg = "(the NestHost attribute in the current class is ignored)"; } else if (_nest_members != NULL && _nest_members != Universe::the_empty_short_array()) { - log_trace(class, nestmates)("Type %s is a dynamic nest member of %s: the NestMembers attribute in the current class is ignored", - this->external_name(), - host->external_name()); + msg = "(the NestMembers attribute in the current class is ignored)"; } + log_trace(class, nestmates)("Injected type %s into the nest of %s %s", + this->external_name(), + host->external_name(), + msg); } // set dynamic nest host _nest_host = host; @@ -399,17 +381,14 @@ assert(this != k, "this should be handled by higher-level code"); // Per JVMS 5.4.4 we first resolve and validate the current class, then - // the target class k. Resolution exceptions will be passed on by upper - // layers. IncompatibleClassChangeErrors from membership validation failures - // will also be passed through. + // the target class k. - Symbol* icce = vmSymbols::java_lang_IncompatibleClassChangeError(); - InstanceKlass* cur_host = nest_host(icce, CHECK_false); + InstanceKlass* cur_host = nest_host(THREAD); if (cur_host == NULL) { return false; } - Klass* k_nest_host = k->nest_host(icce, CHECK_false); + Klass* k_nest_host = k->nest_host(THREAD); if (k_nest_host == NULL) { return false; } @@ -497,6 +476,7 @@ _nest_members(NULL), _nest_host_index(0), _nest_host(NULL), + _nest_host_res_error(NULL), _record_components(NULL), _static_field_size(parser.static_field_size()), _nonstatic_oop_map_size(nonstatic_oop_map_size(parser.total_oop_map_count())), @@ -2482,6 +2462,7 @@ _oop_map_cache = NULL; // clear _nest_host to ensure re-load at runtime _nest_host = NULL; + _nest_host_res_error = NULL; _package_entry = NULL; _dep_context_last_cleaned = 0; } @@ -2647,6 +2628,12 @@ // class can't be referenced anymore). if (_array_name != NULL) _array_name->decrement_refcount(); FREE_C_HEAP_ARRAY(char, _source_debug_extension); + + // deallocate memoized nest-host resolution error + if (_nest_host_res_error != NULL) { + FREE_C_HEAP_ARRAY(char, _nest_host_res_error); + _nest_host_res_error = NULL; + } } void InstanceKlass::set_source_debug_extension(const char* array, int length) {