Added CDSConfig::preserve_all_dumptime_verification_states() to control whether classes verified by the old verifier can be archived

Author: iklam

src/hotspot/share/cds/cdsConfig.cpp

@@ -29,6 +29,7 @@
 #include "cds/heapShared.hpp"
 #include "cds/metaspaceShared.hpp"
 #include "classfile/classLoaderDataShared.hpp"
+#include "classfile/systemDictionaryShared.hpp"
 #include "logging/log.hpp"
 #include "prims/jvmtiExport.hpp"
 
@@ -75,6 +76,16 @@ bool CDSConfig::is_dumping_regenerated_lambdaform_invokers() {
   }
 }
 
+// Preserve all states that were examined used during dumptime verification, such
+// that the verification result (pass or fail) cannot be changed at runtime.
+//
+// For example, if the verification of ik requires that class A must be a subtype of B,
+// then this relationship between A and B cannot be changed at runtime. I.e., the app
+// cannot load alternative versions of A and B such that A is not a subtype of B.
+bool CDSConfig::preserve_all_dumptime_verification_states(const InstanceKlass* ik) {
+  return PreloadSharedClasses && SystemDictionaryShared::is_builtin(ik);
+}
+
 #if INCLUDE_CDS_JAVA_HEAP
 bool CDSConfig::is_dumping_heap() {
   return is_dumping_static_archive() && !is_dumping_preimage_static_archive()

src/hotspot/share/cds/cdsConfig.hpp

@@ -28,6 +28,8 @@
 #include "memory/allStatic.hpp"
 #include "utilities/macros.hpp"
 
+class InstanceKlass;
+
 class CDSConfig : public AllStatic {
 #if INCLUDE_CDS
   static bool _is_dumping_dynamic_archive;
@@ -49,6 +51,9 @@ class CDSConfig : public AllStatic {
   static void      set_has_preloaded_classes()               { CDS_ONLY(_has_preloaded_classes = true); }
   static bool      is_dumping_regenerated_lambdaform_invokers() NOT_CDS_RETURN_(false);
 
+  // Misc CDS features
+  static bool      preserve_all_dumptime_verification_states(const InstanceKlass* ik);
+
   // CDS archived heap
   static bool      is_dumping_heap()                         NOT_CDS_JAVA_HEAP_RETURN_(false);
   static bool      is_loading_heap()                         NOT_CDS_JAVA_HEAP_RETURN_(false);

src/hotspot/share/cds/dumpTimeClassInfo.cpp

@@ -47,9 +47,22 @@ bool DumpTimeClassInfo::is_excluded() {
   if (_excluded) {
     return true;
   }
-  if ((!PreloadSharedClasses || !ArchiveReflectionData || !SystemDictionaryShared::is_builtin(_klass))
-      && _failed_verification) {
-    return true;
+  if (_failed_verification) {
+    if (CDSConfig::preserve_all_dumptime_verification_states(_klass)) {
+      assert(PreloadSharedClasses, "sanity");
+      // If the verification states are preserved, _klass will be archived in unlinked state. This is
+      // necessary to support the following scenario, where the verification of X requires that
+      // A be a subclass of B:
+      //     class X {
+      //        B getB() { return new A(); }
+      //     }
+      // If X and B can be verified, but A fails verification, we still archive A (in a preloaded
+      // SystemDictionary) so that at runtime we cannot subvert the verification of X by replacing
+      // A with a version that is not a subtype of B.
+    } else {
+      // Don't archive this class. At runtime, load it from classfile and rerun verification.
+      return true;
+    }
   }
   return false;
 }

src/hotspot/share/cds/metaspaceShared.cpp

@@ -627,12 +627,16 @@ class CollectCLDClosure : public CLDClosure {
 // Check if we can eagerly link this class at dump time, so we can avoid the
 // runtime linking overhead (especially verification)
 bool MetaspaceShared::may_be_eagerly_linked(InstanceKlass* ik) {
+  if (CDSConfig::preserve_all_dumptime_verification_states(ik)) {
+    assert(ik->can_be_verified_at_dumptime(), "sanity");
+  }
   if (!ik->can_be_verified_at_dumptime()) {
     // For old classes, try to leave them in the unlinked state, so
     // we can still store them in the archive. They must be
     // linked/verified at runtime.
     return false;
   }
+
   if (CDSConfig::is_dumping_dynamic_archive() && ik->is_shared_unregistered_class()) {
     // Linking of unregistered classes at this stage may cause more
     // classes to be resolved, resulting in calls to ClassLoader.loadClass()
@@ -875,7 +879,6 @@ void MetaspaceShared::preload_and_dump_impl(StaticArchiveBuilder& builder, TRAPS
 
 #if INCLUDE_CDS_JAVA_HEAP
   if (CDSConfig::is_dumping_heap()) {
-    StringTable::allocate_shared_strings_array(CHECK);
     if (!HeapShared::is_archived_boot_layer_available(THREAD)) {
       log_info(cds)("archivedBootLayer not available, disabling full module graph");
       CDSConfig::disable_dumping_full_module_graph();
@@ -885,6 +888,10 @@ void MetaspaceShared::preload_and_dump_impl(StaticArchiveBuilder& builder, TRAPS
     if (CDSConfig::is_dumping_full_module_graph()) {
       HeapShared::reset_archived_object_states(CHECK);
     }
+
+    // Do this at the very end, when no Java code will be executed. Otherwise
+    // some new strings may be added to the intern table.
+    StringTable::allocate_shared_strings_array(CHECK);
   }
 #endif
 

src/hotspot/share/classfile/stringTable.cpp

@@ -806,7 +806,9 @@ oop StringTable::lookup_shared(const jchar* name, int len) {
 // This should be called when we know no more strings will be added (which will be easy
 // to guarantee because CDS runs with a single Java thread. See JDK-8253495.)
 void StringTable::allocate_shared_strings_array(TRAPS) {
-  assert(CDSConfig::is_dumping_heap(), "must be");
+  if (!CDSConfig::is_dumping_heap()) {
+    return;
+  }
   if (_items_count > (size_t)max_jint) {
     fatal("Too many strings to be archived: %zu", _items_count);
   }

src/hotspot/share/classfile/systemDictionaryShared.cpp

@@ -315,7 +315,7 @@ bool SystemDictionaryShared::check_for_exclusion_impl(InstanceKlass* k) {
     }
   }
 
-  if (!PreloadSharedClasses || !ArchiveReflectionData || !is_builtin(k)) {
+  if (!CDSConfig::preserve_all_dumptime_verification_states(k)) {
     if (!k->is_linked()) {
       if (has_class_failed_verification(k)) {
         return warn_excluded(k, "Failed verification");
@@ -337,7 +337,7 @@ bool SystemDictionaryShared::check_for_exclusion_impl(InstanceKlass* k) {
     if (ArchiveInvokeDynamic && HeapShared::is_archivable_hidden_klass(k)) {
       // Allow Lambda Proxy and LambdaForm classes, for ArchiveInvokeDynamic only
     } else {
-      log_debug(cds)("Skipping %s: Hidden class", k->name()->as_C_string());
+      log_info(cds)("Skipping %s: Hidden class", k->name()->as_C_string());
       return true;
     }
   }

src/hotspot/share/oops/instanceKlass.cpp

@@ -2966,13 +2966,8 @@ void InstanceKlass::restore_unshareable_info(ClassLoaderData* loader_data, Handl
   }
 }
 
-// Check if a class or any of its supertypes has a version older than 50.
-// CDS will not perform verification of old classes during dump time because
-// without changing the old verifier, the verification constraint cannot be
-// retrieved during dump time.
-// Verification of archived old classes will be performed during run time.
 bool InstanceKlass::can_be_verified_at_dumptime() const {
-  if (PreloadSharedClasses && SystemDictionaryShared::is_builtin(this)) {
+  if (CDSConfig::preserve_all_dumptime_verification_states(this)) {
     return true;
   }
 
@@ -2987,6 +2982,11 @@ bool InstanceKlass::can_be_verified_at_dumptime() const {
     return true;
   }
 
+  // Check if a class or any of its supertypes has a version older than 50.
+  // CDS will not perform verification of old classes during dump time because
+  // without changing the old verifier, the verification constraint cannot be
+  // retrieved during dump time.
+  // Verification of archived old classes will be performed during run time.
   if (major_version() < 50 /*JAVA_6_VERSION*/) {
     return false;
   }

src/hotspot/share/runtime/arguments.cpp

@@ -3185,7 +3185,13 @@ jint Arguments::finalize_vm_init_args(bool patch_mod_javabase) {
   }
 
   if (!CDSConfig::is_dumping_static_archive() || !PreloadSharedClasses) {
+    // FIXME -- CDSConfig::is_dumping_heap() is not yet callable from here, as UseG1GC is not yet set by ergo!
+    //
+    //
+    // These optimizations require heap dumping and PreloadSharedClasses, or else
+    // the classes of some archived heap objects may be replaced at runtime.
     ArchiveInvokeDynamic = false;
+    ArchiveReflectionData = false;
   }
 #endif
 

test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/OldClassVerifierTrouble.java

@@ -65,7 +65,8 @@ private static void doTestCustomBase(String topArchiveName) throws Exception {
         // create a custom base archive containing an old class
         OutputAnalyzer output = TestCommon.dump(appJar,
             TestCommon.list("VerifierTroubleApp", "VerifierTroublev49", "ChildOldSuper"),
-            "-Xlog:class+load,cds+class=debug");
+            "-Xlog:class+load,cds+class=debug",
+            "-XX:-PreloadSharedClasses");
         TestCommon.checkDump(output);
         // Check the ChildOldSuper and VerifierTroublev49 are being dumped into the base archive.
         output.shouldMatch(".cds.class.*klass.*0x.*app.*ChildOldSuper.*unlinked")
@@ -79,6 +80,7 @@ private static void doTestCustomBase(String topArchiveName) throws Exception {
         // Linking VerifierTroublev49 would result in java.lang.VerifyError.
         dump2(baseArchiveName, topArchiveName,
               "-Xlog:cds,cds+dynamic,class+load,cds+class=debug",
+              "-XX:-PreloadSharedClasses",
               "-cp", appJar,
               appClass)
             .assertAbnormalExit(out -> {

test/hotspot/jtreg/runtime/cds/appcds/preloadedClasses/PreloadedClassVerification.java

@@ -69,7 +69,8 @@ public static void main(String[] args) throws Exception {
                                                      "BadNewClass",
                                                      "BadNewClass2",
                                                      "GoodOldClass"),
-                            bootAppendWhiteBox);
+                            bootAppendWhiteBox,
+                            "-Xlog:cds+class=debug");
 
         TestCommon.run("-cp", app1Jar + File.pathSeparator + app2Jar,
                        "-XX:+UnlockDiagnosticVMOptions",