8246546: Simplify SystemDictionary::is_shared_class_visible

Reviewed-by: minqi, ccheung

src/hotspot/share/classfile/moduleEntry.cpp

@@ -29,6 +29,7 @@
 #include "classfile/javaClasses.inline.hpp"
 #include "classfile/moduleEntry.hpp"
 #include "logging/log.hpp"
+#include "memory/filemap.hpp"
 #include "memory/resourceArea.hpp"
 #include "memory/universe.hpp"
 #include "oops/oopHandle.inline.hpp"
@@ -55,6 +56,9 @@ void ModuleEntry::set_location(Symbol* location) {
 
   if (location != NULL) {
     location->increment_refcount();
+    CDS_ONLY(if (UseSharedSpaces) {
+        _shared_path_index = FileMapInfo::get_module_shared_path_index(location);
+      });
   }
 }
 

src/hotspot/share/classfile/moduleEntry.hpp

@@ -69,6 +69,7 @@ class ModuleEntry : public HashtableEntry<Symbol*, mtModule> {
   GrowableArray<ModuleEntry*>* _reads; // list of modules that are readable by this module
   Symbol* _version;                    // module version number
   Symbol* _location;                   // module location
+  CDS_ONLY(int _shared_path_index;)    // >=0 if classes in this module are in CDS archive
   bool _can_read_all_unnamed;
   bool _has_default_read_edges;        // JVMTI redefine/retransform support
   bool _must_walk_reads;               // walk module's reads list at GC safepoints to purge out dead modules
@@ -90,6 +91,7 @@ class ModuleEntry : public HashtableEntry<Symbol*, mtModule> {
     _must_walk_reads = false;
     _is_patched = false;
     _is_open = false;
+    CDS_ONLY(_shared_path_index = -1);
   }
 
   Symbol*          name() const                        { return literal(); }
@@ -154,6 +156,7 @@ class ModuleEntry : public HashtableEntry<Symbol*, mtModule> {
 
   void set_is_patched() {
       _is_patched = true;
+      CDS_ONLY(_shared_path_index = -1); // Mark all shared classes in this module invisible.
   }
   bool is_patched() {
       return _is_patched;
@@ -182,6 +185,8 @@ class ModuleEntry : public HashtableEntry<Symbol*, mtModule> {
   void print(outputStream* st = tty);
   void verify();
 
+  CDS_ONLY(int shared_path_index() { return _shared_path_index;})
+
   JFR_ONLY(DEFINE_TRACE_ID_METHODS;)
 };
 

src/hotspot/share/classfile/systemDictionary.cpp

@@ -1241,104 +1241,92 @@ InstanceKlass* SystemDictionary::load_shared_boot_class(Symbol* class_name,
   return NULL;
 }
 
-// Check if a shared class can be loaded by the specific classloader:
-//
-// NULL classloader:
-//   - Module class from "modules" jimage. ModuleEntry must be defined in the classloader.
-//   - Class from -Xbootclasspath/a. The class has no defined PackageEntry, or must
-//     be defined in an unnamed module.
+// Check if a shared class can be loaded by the specific classloader.
 bool SystemDictionary::is_shared_class_visible(Symbol* class_name,
                                                InstanceKlass* ik,
                                                PackageEntry* pkg_entry,
                                                Handle class_loader, TRAPS) {
   assert(!ModuleEntryTable::javabase_moduleEntry()->is_patched(),
          "Cannot use sharing if java.base is patched");
-  if (ik->shared_classpath_index() < 0) {
-    // path_index < 0 indicates that the class is intended for a custom loader
-    // and should not be loaded by boot/platform/app loaders
-    if (is_builtin_class_loader(class_loader())) {
+
+  // (1) Check if we are loading into the same loader as in dump time.
+
+  if (ik->is_shared_boot_class()) {
+    if (class_loader() != NULL) {
+      return false;
+    }
+  } else if (ik->is_shared_platform_class()) {
+    if (class_loader() != java_platform_loader()) {
+      return false;
+    }
+  } else if (ik->is_shared_app_class()) {
+    if (class_loader() != java_system_loader()) {
+      return false;
+    }
+  } else {
+    // ik was loaded by a custom loader during dump time
+    if (class_loader_data(class_loader)->is_builtin_class_loader_data()) {
       return false;
     } else {
       return true;
     }
   }
 
-  // skip class visibility check
+  // (2) Check if we are loading into the same module from the same location as in dump time.
+
   if (MetaspaceShared::use_optimized_module_handling()) {
-    assert(SystemDictionary::is_shared_class_visible_impl(class_name, ik, pkg_entry, class_loader, THREAD), "Optimizing module handling failed.");
+    // Class visibility has not changed between dump time and run time, so a class
+    // that was visible (and thus archived) during dump time is always visible during runtime.
+    assert(SystemDictionary::is_shared_class_visible_impl(class_name, ik, pkg_entry, class_loader, THREAD),
+           "visibility cannot change between dump time and runtime");
     return true;
   }
   return is_shared_class_visible_impl(class_name, ik, pkg_entry, class_loader, THREAD);
 }
 
 bool SystemDictionary::is_shared_class_visible_impl(Symbol* class_name,
-                                               InstanceKlass* ik,
-                                               PackageEntry* pkg_entry,
-                                               Handle class_loader, TRAPS) {
-  int path_index = ik->shared_classpath_index();
-  ClassLoaderData* loader_data = class_loader_data(class_loader);
-  SharedClassPathEntry* ent =
-            (SharedClassPathEntry*)FileMapInfo::shared_path(path_index);
+                                                    InstanceKlass* ik,
+                                                    PackageEntry* pkg_entry,
+                                                    Handle class_loader, TRAPS) {
+  int scp_index = ik->shared_classpath_index();
+  assert(!ik->is_shared_unregistered_class(), "this function should be called for built-in classes only");
+  assert(scp_index >= 0, "must be");
+  SharedClassPathEntry* scp_entry = FileMapInfo::shared_path(scp_index);
   if (!Universe::is_module_initialized()) {
-    assert(ent != NULL && ent->is_modules_image(),
+    assert(scp_entry != NULL && scp_entry->is_modules_image(),
            "Loading non-bootstrap classes before the module system is initialized");
     assert(class_loader.is_null(), "sanity");
     return true;
   }
-  // Get the pkg_entry from the classloader
-  ModuleEntry* mod_entry = NULL;
-  TempNewSymbol pkg_name = pkg_entry != NULL ? pkg_entry->name() :
-                                               ClassLoader::package_from_class_name(class_name);
-  if (pkg_name != NULL) {
-    if (loader_data != NULL) {
-      if (pkg_entry != NULL) {
-        mod_entry = pkg_entry->module();
-        // If the archived class is from a module that has been patched at runtime,
-        // the class cannot be loaded from the archive.
-        if (mod_entry != NULL && mod_entry->is_patched()) {
-          return false;
-        }
-      }
-    }
-  }
 
-  if (class_loader.is_null()) {
-    assert(ent != NULL, "Shared class for NULL classloader must have valid SharedClassPathEntry");
-    // The NULL classloader can load archived class originated from the
-    // "modules" jimage and the -Xbootclasspath/a. For class from the
-    // "modules" jimage, the PackageEntry/ModuleEntry must be defined
-    // by the NULL classloader.
-    if (mod_entry != NULL) {
-      // PackageEntry/ModuleEntry is found in the classloader. Check if the
-      // ModuleEntry's location agrees with the archived class' origination.
-      if (ent->is_modules_image() && mod_entry->location()->starts_with("jrt:")) {
-        return true; // Module class from the "module" jimage
-      }
-    }
+  ModuleEntry* mod_entry = (pkg_entry == NULL) ? NULL : pkg_entry->module();
+  bool should_be_in_named_module = (mod_entry != NULL && mod_entry->is_named());
+  bool was_archived_from_named_module = scp_entry->in_named_module();
+  bool visible;
 
-    // If the archived class is not from the "module" jimage, the class can be
-    // loaded by the NULL classloader if
-    //
-    // 1. the class is from the unamed package
-    // 2. or, the class is not from a module defined in the NULL classloader
-    // 3. or, the class is from an unamed module
-    if (!ent->is_modules_image() && ik->is_shared_boot_class()) {
-      // the class is from the -Xbootclasspath/a
-      if (pkg_name == NULL ||
-          pkg_entry == NULL ||
-          pkg_entry->in_unnamed_module()) {
-        assert(mod_entry == NULL ||
-               mod_entry == loader_data->unnamed_module(),
-               "the unnamed module is not defined in the classloader");
-        return true;
+  if (was_archived_from_named_module) {
+    if (should_be_in_named_module) {
+      // Is the module loaded from the same location as during dump time?
+      visible = mod_entry->shared_path_index() == scp_index;
+      if (visible) {
+        assert(!mod_entry->is_patched(), "cannot load archived classes for patched module");
       }
+    } else {
+      // During dump time, this class was in a named module, but at run time, this class should be
+      // in an unnamed module.
+      visible = false;
     }
-    return false;
   } else {
-    bool res = SystemDictionaryShared::is_shared_class_visible_for_classloader(
-              ik, class_loader, pkg_name, pkg_entry, mod_entry, CHECK_(false));
-    return res;
+    if (should_be_in_named_module) {
+      // During dump time, this class was in an unnamed, but at run time, this class should be
+      // in a named module.
+      visible = false;
+    } else {
+      visible = true;
+    }
   }
+
+  return visible;
 }
 
 bool SystemDictionary::check_shared_class_super_type(InstanceKlass* child, InstanceKlass* super_type,

src/hotspot/share/classfile/systemDictionaryShared.cpp

@@ -886,114 +886,6 @@ bool SystemDictionaryShared::is_sharing_possible(ClassLoaderData* loader_data) {
           SystemDictionary::is_platform_class_loader(class_loader));
 }
 
-// Currently AppCDS only archives classes from the run-time image, the
-// -Xbootclasspath/a path, the class path, and the module path.
-//
-// Check if a shared class can be loaded by the specific classloader. Following
-// are the "visible" archived classes for different classloaders.
-//
-// NULL classloader:
-//   - see SystemDictionary::is_shared_class_visible()
-// Platform classloader:
-//   - Module class from runtime image. ModuleEntry must be defined in the
-//     classloader.
-// App classloader:
-//   - Module Class from runtime image and module path. ModuleEntry must be defined in the
-//     classloader.
-//   - Class from -cp. The class must have no PackageEntry defined in any of the
-//     boot/platform/app classloader, or must be in the unnamed module defined in the
-//     AppClassLoader.
-bool SystemDictionaryShared::is_shared_class_visible_for_classloader(
-                                                     InstanceKlass* ik,
-                                                     Handle class_loader,
-                                                     Symbol* pkg_name,
-                                                     PackageEntry* pkg_entry,
-                                                     ModuleEntry* mod_entry,
-                                                     TRAPS) {
-  assert(class_loader.not_null(), "Class loader should not be NULL");
-  assert(Universe::is_module_initialized(), "Module system is not initialized");
-  ResourceMark rm(THREAD);
-
-  int path_index = ik->shared_classpath_index();
-  SharedClassPathEntry* ent =
-            (SharedClassPathEntry*)FileMapInfo::shared_path(path_index);
-
-  if (SystemDictionary::is_platform_class_loader(class_loader())) {
-    assert(ent != NULL, "shared class for PlatformClassLoader should have valid SharedClassPathEntry");
-    // The PlatformClassLoader can only load archived class originated from the
-    // run-time image. The class' PackageEntry/ModuleEntry must be
-    // defined by the PlatformClassLoader.
-    if (mod_entry != NULL) {
-      // PackageEntry/ModuleEntry is found in the classloader. Check if the
-      // ModuleEntry's location agrees with the archived class' origination.
-      if (ent->is_modules_image() && mod_entry->location()->starts_with("jrt:")) {
-        return true; // Module class from the runtime image
-      }
-    }
-  } else if (SystemDictionary::is_system_class_loader(class_loader())) {
-    assert(ent != NULL, "shared class for system loader should have valid SharedClassPathEntry");
-    if (pkg_name == NULL) {
-      // The archived class is in the unnamed package. Currently, the boot image
-      // does not contain any class in the unnamed package.
-      assert(!ent->is_modules_image(), "Class in the unnamed package must be from the classpath");
-      if (path_index >= ClassLoaderExt::app_class_paths_start_index()) {
-        assert(path_index < ClassLoaderExt::app_module_paths_start_index(), "invalid path_index");
-        return true;
-      }
-    } else {
-      // Check if this is from a PackageEntry/ModuleEntry defined in the AppClassloader.
-      if (pkg_entry == NULL) {
-        // It's not guaranteed that the class is from the classpath if the
-        // PackageEntry cannot be found from the AppClassloader. Need to check
-        // the boot and platform classloader as well.
-        ClassLoaderData* platform_loader_data =
-          ClassLoaderData::class_loader_data_or_null(SystemDictionary::java_platform_loader()); // can be NULL during bootstrap
-        if ((platform_loader_data == NULL ||
-             ClassLoader::get_package_entry(pkg_name, platform_loader_data) == NULL) &&
-             ClassLoader::get_package_entry(pkg_name, ClassLoaderData::the_null_class_loader_data()) == NULL) {
-          // The PackageEntry is not defined in any of the boot/platform/app classloaders.
-          // The archived class must from -cp path and not from the runtime image.
-          if (!ent->is_modules_image() && path_index >= ClassLoaderExt::app_class_paths_start_index() &&
-                                          path_index < ClassLoaderExt::app_module_paths_start_index()) {
-            return true;
-          }
-        }
-      } else if (mod_entry != NULL) {
-        // The package/module is defined in the AppClassLoader. We support
-        // archiving application module class from the runtime image or from
-        // a named module from a module path.
-        // Packages from the -cp path are in the unnamed_module.
-        if (ent->is_modules_image() && mod_entry->location()->starts_with("jrt:")) {
-          // shared module class from runtime image
-          return true;
-        } else if (pkg_entry->in_unnamed_module() && path_index >= ClassLoaderExt::app_class_paths_start_index() &&
-            path_index < ClassLoaderExt::app_module_paths_start_index()) {
-          // shared class from -cp
-          DEBUG_ONLY( \
-            ClassLoaderData* loader_data = class_loader_data(class_loader); \
-            assert(mod_entry == loader_data->unnamed_module(), "the unnamed module is not defined in the classloader");)
-          return true;
-        } else {
-          if(!pkg_entry->in_unnamed_module() &&
-              (path_index >= ClassLoaderExt::app_module_paths_start_index())&&
-              (path_index < FileMapInfo::get_number_of_shared_paths()) &&
-              (strcmp(ent->name(), ClassLoader::skip_uri_protocol(mod_entry->location()->as_C_string())) == 0)) {
-            // shared module class from module path
-            return true;
-          } else {
-            assert(path_index < FileMapInfo::get_number_of_shared_paths(), "invalid path_index");
-          }
-        }
-      }
-    }
-  } else {
-    // TEMP: if a shared class can be found by a custom loader, consider it visible now.
-    // FIXME: is this actually correct?
-    return true;
-  }
-  return false;
-}
-
 bool SystemDictionaryShared::has_platform_or_app_classes() {
   if (FileMapInfo::current_info()->has_platform_or_app_classes()) {
     return true;
@@ -1026,7 +918,7 @@ bool SystemDictionaryShared::has_platform_or_app_classes() {
 // [b] BuiltinClassLoader.loadClassOrNull() first calls findLoadedClass(name).
 // [c] At this point, if we can find the named class inside the
 //     shared_dictionary, we can perform further checks (see
-//     is_shared_class_visible_for_classloader() to ensure that this class
+//     SystemDictionary::is_shared_class_visible) to ensure that this class
 //     was loaded by the same class loader during dump time.
 //
 // Given these assumptions, we intercept the findLoadedClass() call to invoke

src/hotspot/share/classfile/systemDictionaryShared.hpp

@@ -242,12 +242,6 @@ class SystemDictionaryShared: public SystemDictionary {
 
   // Check if sharing is supported for the class loader.
   static bool is_sharing_possible(ClassLoaderData* loader_data);
-  static bool is_shared_class_visible_for_classloader(InstanceKlass* ik,
-                                                      Handle class_loader,
-                                                      Symbol* pkg_name,
-                                                      PackageEntry* pkg_entry,
-                                                      ModuleEntry* mod_entry,
-                                                      TRAPS);
 
   static bool add_unregistered_class(InstanceKlass* k, TRAPS);
   static InstanceKlass* dump_time_resolve_super_or_fail(Symbol* child_name,