8315130: java.lang.IllegalAccessError when processing classlist to create CDS archive

src/hotspot/share/cds/classListParser.cpp

@@ -397,17 +397,13 @@ bool ClassListParser::parse_uint_option(const char* option_name, int* value) {
   return false;
 }
 
-objArrayOop ClassListParser::get_specified_interfaces(TRAPS) {
+GrowableArray<InstanceKlass *> ClassListParser::get_specified_interfaces() {
   const int n = _interfaces->length();
-  if (n == 0) {
-    return nullptr;
-  } else {
-    objArrayOop array = oopFactory::new_objArray(vmClasses::Class_klass(), n, CHECK_NULL);
-    for (int i = 0; i < n; i++) {
-      array->obj_at_put(i, lookup_class_by_id(_interfaces->at(i))->java_mirror());
-    }
-    return array;
+  GrowableArray<InstanceKlass *> specified_interfaces(n);
+  for (int i = 0; i < n; i++) {
+    specified_interfaces.append(lookup_class_by_id(_interfaces->at(i)));
   }
+  return specified_interfaces;
 }
 
 void ClassListParser::print_specified_interfaces() {
@@ -469,6 +465,25 @@ void ClassListParser::error(const char* msg, ...) {
   va_end(ap);
 }
 
+// If an unregistered class U is specified to have a registered supertype S1
+// named SN but an unregistered class S2 also named SN has already been loaded
+// S2 will be incorrectly used as the supertype of U instead of S1 due to
+// limitations in the loading mechanism of unregistered classes.
+void ClassListParser::check_supertype_obstruction(int specified_supertype_id, const InstanceKlass* specified_supertype, TRAPS) {
+  if (specified_supertype->is_shared_unregistered_class()) {
+    return; // Only registered supertypes can be obstructed
+  }
+  const InstanceKlass* obstructor = SystemDictionaryShared::get_unregistered_class(specified_supertype->name());
+  if (obstructor == nullptr) {
+    return; // No unregistered types with the same name have been loaded, i.e. no obstruction
+  }
+  // 'specified_supertype' is S1, 'obstructor' is S2 from the explanation above
+  ResourceMark rm;
+  THROW_MSG(vmSymbols::java_lang_UnsupportedOperationException(),
+            err_msg("%s (id %d) has super-type %s (id %d) obstructed by another class with the same name",
+                    _class_name, _id, specified_supertype->external_name(), specified_supertype_id));
+}
+
 // This function is used for loading classes for customized class loaders
 // during archive dumping.
 InstanceKlass* ClassListParser::load_class_from_source(Symbol* class_name, TRAPS) {
@@ -493,13 +508,18 @@ InstanceKlass* ClassListParser::load_class_from_source(Symbol* class_name, TRAPS
   }
 
   ResourceMark rm;
-  char * source_path = os::strdup_check_oom(ClassLoader::uri_to_path(_source));
   InstanceKlass* specified_super = lookup_class_by_id(_super);
-  Handle super_class(THREAD, specified_super->java_mirror());
-  objArrayOop r = get_specified_interfaces(CHECK_NULL);
-  objArrayHandle interfaces(THREAD, r);
-  InstanceKlass* k = UnregisteredClasses::load_class(class_name, source_path,
-                                                     super_class, interfaces, CHECK_NULL);
+  GrowableArray<InstanceKlass*> specified_interfaces = get_specified_interfaces();
+  // Obstruction must be checked before the class loading attempt because it may
+  // cause class loading errors (JVMS 5.3.5.3-5.3.5.4)
+  check_supertype_obstruction(_super, specified_super, CHECK_NULL);
+  for (int i = 0; i < _interfaces->length(); i++) {
+    check_supertype_obstruction(_interfaces->at(i), specified_interfaces.at(i), CHECK_NULL);
+  }
+
+  const char* source_path = ClassLoader::uri_to_path(_source);
+  InstanceKlass* k = UnregisteredClasses::load_class(class_name, source_path, CHECK_NULL);
+
   if (k->java_super() != specified_super) {
     error("The specified super class %s (id %d) does not match actual super class %s",
           specified_super->external_name(), _super,
@@ -511,6 +531,15 @@ InstanceKlass* ClassListParser::load_class_from_source(Symbol* class_name, TRAPS
     error("The number of interfaces (%d) specified in class list does not match the class file (%d)",
           _interfaces->length(), k->local_interfaces()->length());
   }
+  for (int i = 0; i < _interfaces->length(); i++) {
+    InstanceKlass* specified_interface = specified_interfaces.at(i);
+    if (!k->local_interfaces()->contains(specified_interface)) {
+      print_specified_interfaces();
+      print_actual_interfaces(k);
+      error("Specified interface %s (id %d) is not directly implemented",
+            specified_interface->external_name(), _interfaces->at(i));
+      }
+  }
 
   assert(k->is_shared_unregistered_class(), "must be");
 

src/hotspot/share/cds/classListParser.hpp

@@ -134,7 +134,8 @@ class ClassListParser : public StackObj {
 
   ClassListParser(const char* file, ParseMode _parse_mode);
   ~ClassListParser();
-  objArrayOop get_specified_interfaces(TRAPS);
+  GrowableArray<InstanceKlass*> get_specified_interfaces();
+  void check_supertype_obstruction(int specified_supertype_id, const InstanceKlass* specified_supertype, TRAPS);
 
 public:
   static int parse_classlist(const char* classlist_path, ParseMode parse_mode, TRAPS);

src/hotspot/share/cds/unregisteredClasses.cpp

@@ -24,97 +24,83 @@
 
 #include "precompiled.hpp"
 #include "cds/unregisteredClasses.hpp"
-#include "classfile/classFileStream.hpp"
-#include "classfile/classLoader.inline.hpp"
-#include "classfile/classLoaderExt.hpp"
-#include "classfile/javaClasses.inline.hpp"
 #include "classfile/symbolTable.hpp"
 #include "classfile/systemDictionary.hpp"
 #include "classfile/vmSymbols.hpp"
-#include "memory/oopFactory.hpp"
-#include "memory/resourceArea.hpp"
 #include "oops/instanceKlass.hpp"
+#include "oops/oopHandle.hpp"
 #include "oops/oopHandle.inline.hpp"
+#include "runtime/handles.hpp"
 #include "runtime/handles.inline.hpp"
 #include "runtime/javaCalls.hpp"
 #include "services/threadService.hpp"
 
-InstanceKlass* UnregisteredClasses::_UnregisteredClassLoader_klass = nullptr;
+static InstanceKlass* _UnregisteredClassLoader_klass;
+static InstanceKlass* _UnregisteredClassLoader_Source_klass;
+static OopHandle _unregistered_class_loader;
 
 void UnregisteredClasses::initialize(TRAPS) {
-  if (_UnregisteredClassLoader_klass == nullptr) {
-    // no need for synchronization as this function is called single-threaded.
-    Symbol* klass_name = SymbolTable::new_symbol("jdk/internal/misc/CDS$UnregisteredClassLoader");
-    Klass* k = SystemDictionary::resolve_or_fail(klass_name, true, CHECK);
-    _UnregisteredClassLoader_klass = InstanceKlass::cast(k);
+  if (_UnregisteredClassLoader_klass != nullptr) {
+    return;
   }
+
+  Symbol* klass_name;
+  Klass* k;
+
+  // no need for synchronization as this function is called single-threaded.
+  klass_name = SymbolTable::new_symbol("jdk/internal/misc/CDS$UnregisteredClassLoader");
+  k = SystemDictionary::resolve_or_fail(klass_name, true, CHECK);
+  _UnregisteredClassLoader_klass = InstanceKlass::cast(k);
+
+  klass_name = SymbolTable::new_symbol("jdk/internal/misc/CDS$UnregisteredClassLoader$Source");
+  k = SystemDictionary::resolve_or_fail(klass_name, true, CHECK);
+  _UnregisteredClassLoader_Source_klass = InstanceKlass::cast(k);
+
+  precond(_unregistered_class_loader.is_empty());
+  HandleMark hm(THREAD);
+  const Handle cl = JavaCalls::construct_new_instance(_UnregisteredClassLoader_klass,
+                                                      vmSymbols::void_method_signature(), CHECK);
+  _unregistered_class_loader = OopHandle(Universe::vm_global(), cl());
 }
 
 // Load the class of the given name from the location given by path. The path is specified by
 // the "source:" in the class list file (see classListParser.cpp), and can be a directory or
 // a JAR file.
-InstanceKlass* UnregisteredClasses::load_class(Symbol* name, const char* path,
-                                               Handle super_class, objArrayHandle interfaces, TRAPS) {
+InstanceKlass* UnregisteredClasses::load_class(Symbol* name, const char* path, TRAPS) {
   assert(name != nullptr, "invariant");
   assert(DumpSharedSpaces, "this function is only used with -Xshare:dump");
 
   PerfClassTraceTime vmtimer(ClassLoader::perf_app_classload_time(),
                              THREAD->get_thread_stat()->perf_timers_addr(),
                              PerfClassTraceTime::CLASS_LOAD);
 
-  // Call CDS$UnregisteredClassLoader::load(String name, Class<?> superClass, Class<?>[] interfaces)
+  assert(!_unregistered_class_loader.is_empty(), "not initialized");
+  Handle classloader(THREAD, _unregistered_class_loader.resolve());
+
+  // Call CDS$UnregisteredClassLoader::load(String name, String source)
   Symbol* methodName = SymbolTable::new_symbol("load");
-  Symbol* methodSignature = SymbolTable::new_symbol("(Ljava/lang/String;Ljava/lang/Class;[Ljava/lang/Class;)Ljava/lang/Class;");
-  Symbol* path_symbol = SymbolTable::new_symbol(path);
-  Handle classloader = get_classloader(path_symbol, CHECK_NULL);
+  Symbol* methodSignature = SymbolTable::new_symbol("(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/Class;");
   Handle ext_class_name = java_lang_String::externalize_classname(name, CHECK_NULL);
+  Handle path_string = java_lang_String::create_from_str(path, CHECK_NULL);
 
   JavaValue result(T_OBJECT);
-  JavaCallArguments args(3);
-  args.set_receiver(classloader);
-  args.push_oop(ext_class_name);
-  args.push_oop(super_class);
-  args.push_oop(interfaces);
   JavaCalls::call_virtual(&result,
-                          UnregisteredClassLoader_klass(),
+                          classloader,
+                          _UnregisteredClassLoader_klass,
                           methodName,
                           methodSignature,
-                          &args,
+                          ext_class_name,
+                          path_string,
                           CHECK_NULL);
   assert(result.get_type() == T_OBJECT, "just checking");
-  oop obj = result.get_oop();
-  return InstanceKlass::cast(java_lang_Class::as_Klass(obj));
-}
-
-class UnregisteredClasses::ClassLoaderTable : public ResourceHashtable<
-  Symbol*, OopHandle,
-  137, // prime number
-  AnyObj::C_HEAP> {};
-
-static UnregisteredClasses::ClassLoaderTable* _classloader_table = nullptr;
 
-Handle UnregisteredClasses::create_classloader(Symbol* path, TRAPS) {
-  ResourceMark rm(THREAD);
-  JavaValue result(T_OBJECT);
-  Handle path_string = java_lang_String::create_from_str(path->as_C_string(), CHECK_NH);
-  Handle classloader = JavaCalls::construct_new_instance(
-                           UnregisteredClassLoader_klass(),
-                           vmSymbols::string_void_signature(),
-                           path_string, CHECK_NH);
-  return classloader;
+  return InstanceKlass::cast(java_lang_Class::as_Klass(result.get_oop()));
 }
 
-Handle UnregisteredClasses::get_classloader(Symbol* path, TRAPS) {
-  if (_classloader_table == nullptr) {
-    _classloader_table = new (mtClass)ClassLoaderTable();
-  }
-  OopHandle* classloader_ptr = _classloader_table->get(path);
-  if (classloader_ptr != nullptr) {
-    return Handle(THREAD, (*classloader_ptr).resolve());
-  } else {
-    Handle classloader = create_classloader(path, CHECK_NH);
-    _classloader_table->put(path, OopHandle(Universe::vm_global(), classloader()));
-    path->increment_refcount();
-    return classloader;
+bool UnregisteredClasses::check_for_exclusion(const InstanceKlass* k) {
+  if (_UnregisteredClassLoader_klass == nullptr) {
+    return false; // Uninitialized
   }
+  return k == _UnregisteredClassLoader_klass ||
+         k->implements_interface(_UnregisteredClassLoader_Source_klass);
 }

src/hotspot/share/cds/unregisteredClasses.hpp

@@ -33,22 +33,10 @@ class Symbol;
 
 class UnregisteredClasses: AllStatic {
 public:
-  static InstanceKlass* load_class(Symbol* h_name, const char* path,
-                                   Handle super_class, objArrayHandle interfaces,
-                                   TRAPS);
+  static InstanceKlass* load_class(Symbol* name, const char* path, TRAPS);
   static void initialize(TRAPS);
-  static InstanceKlass* UnregisteredClassLoader_klass() {
-    return _UnregisteredClassLoader_klass;
-  }
-
-  class ClassLoaderTable;
-
-private:
-  // Don't put this in vmClasses as it's used only with CDS dumping.
-  static InstanceKlass* _UnregisteredClassLoader_klass;
-
-  static Handle create_classloader(Symbol* path, TRAPS);
-  static Handle get_classloader(Symbol* path, TRAPS);
+  // Returns true if the class is loaded internally for dumping unregistered classes.
+  static bool check_for_exclusion(const InstanceKlass* k);
 };
 
 #endif // SHARE_CDS_UNREGISTEREDCLASSES_HPP

src/hotspot/share/classfile/systemDictionaryShared.cpp

@@ -314,6 +314,12 @@ bool SystemDictionaryShared::check_for_exclusion_impl(InstanceKlass* k) {
     return true;
   }
 
+  if (UnregisteredClasses::check_for_exclusion(k)) {
+    ResourceMark rm;
+    log_info(cds)("Skipping %s: used only when dumping CDS archive", k->name()->as_C_string());
+    return true;
+  }
+
   InstanceKlass* super = k->java_super();
   if (super != nullptr && check_for_exclusion(super, nullptr)) {
     ResourceMark rm;
@@ -332,12 +338,6 @@ bool SystemDictionaryShared::check_for_exclusion_impl(InstanceKlass* k) {
     }
   }
 
-  if (k == UnregisteredClasses::UnregisteredClassLoader_klass()) {
-    ResourceMark rm;
-    log_info(cds)("Skipping %s: used only when dumping CDS archive", k->name()->as_C_string());
-    return true;
-  }
-
   return false; // false == k should NOT be excluded
 }
 
@@ -456,6 +456,15 @@ bool SystemDictionaryShared::add_unregistered_class(Thread* current, InstanceKla
   return (klass == *v);
 }
 
+InstanceKlass* SystemDictionaryShared::get_unregistered_class(Symbol* name) {
+  assert(Arguments::is_dumping_archive() || ClassListWriter::is_enabled(), "sanity");
+  if (_unregistered_classes_table == nullptr) {
+    return nullptr;
+  }
+  InstanceKlass** k = _unregistered_classes_table->get(name);
+  return k != nullptr ? *k : nullptr;
+}
+
 void SystemDictionaryShared::set_shared_class_misc_info(InstanceKlass* k, ClassFileStream* cfs) {
   Arguments::assert_is_dumping_archive();
   assert(!is_builtin(k), "must be unregistered class");

src/hotspot/share/classfile/systemDictionaryShared.hpp

@@ -287,6 +287,7 @@ class SystemDictionaryShared: public SystemDictionary {
     return (k->shared_classpath_index() != UNREGISTERED_INDEX);
   }
   static bool add_unregistered_class(Thread* current, InstanceKlass* k);
+  static InstanceKlass* get_unregistered_class(Symbol* name);
 
   static void check_excluded_classes();
   static bool check_for_exclusion(InstanceKlass* k, DumpTimeClassInfo* info);