8276787: Improve warning messages for -XX:+RecordDynamicDumpInfo

Reviewed-by: ccheung, stuefe

src/hotspot/share/cds/dynamicArchive.cpp

@@ -50,7 +50,9 @@
 
 
 class DynamicArchiveBuilder : public ArchiveBuilder {
+  const char* _archive_name;
 public:
+  DynamicArchiveBuilder(const char* archive_name) : _archive_name(archive_name) {}
   void mark_pointer(address* ptr_loc) {
     ArchivePtrMarker::mark_pointer(ptr_loc);
   }
@@ -318,7 +320,7 @@ void DynamicArchiveBuilder::write_archive(char* serialized_data) {
   FileMapInfo* dynamic_info = FileMapInfo::dynamic_info();
   assert(dynamic_info != NULL, "Sanity");
 
-  dynamic_info->open_for_write(Arguments::GetSharedDynamicArchivePath());
+  dynamic_info->open_for_write(_archive_name);
   ArchiveBuilder::write_archive(dynamic_info, NULL, NULL, NULL, NULL);
 
   address base = _requested_dynamic_archive_bottom;
@@ -333,9 +335,10 @@ void DynamicArchiveBuilder::write_archive(char* serialized_data) {
 }
 
 class VM_PopulateDynamicDumpSharedSpace: public VM_GC_Sync_Operation {
-  DynamicArchiveBuilder builder;
+  DynamicArchiveBuilder _builder;
 public:
-  VM_PopulateDynamicDumpSharedSpace() : VM_GC_Sync_Operation() {}
+  VM_PopulateDynamicDumpSharedSpace(const char* archive_name)
+  : VM_GC_Sync_Operation(), _builder(archive_name) {}
   VMOp_Type type() const { return VMOp_PopulateDumpSharedSpace; }
   void doit() {
     ResourceMark rm;
@@ -349,11 +352,30 @@ class VM_PopulateDynamicDumpSharedSpace: public VM_GC_Sync_Operation {
     }
     FileMapInfo::check_nonempty_dir_in_shared_path_table();
 
-    builder.doit();
+    _builder.doit();
   }
 };
 
-void DynamicArchive::prepare_for_dynamic_dumping() {
+void DynamicArchive::check_for_dynamic_dump() {
+  if (DynamicDumpSharedSpaces && !UseSharedSpaces) {
+    // This could happen if SharedArchiveFile has failed to load:
+    // - -Xshare:off was specified
+    // - SharedArchiveFile points to an non-existent file.
+    // - SharedArchiveFile points to an archive that has failed CRC check
+    // - SharedArchiveFile is not specified and the VM doesn't have a compatible default archive
+
+#define __THEMSG " is unsupported when base CDS archive is not loaded. Run with -Xlog:cds for more info."
+    if (RecordDynamicDumpInfo) {
+      vm_exit_during_initialization("-XX:+RecordDynamicDumpInfo" __THEMSG, NULL);
+    } else {
+      assert(ArchiveClassesAtExit != nullptr, "sanity");
+      vm_exit_during_initialization("-XX:ArchiveClassesAtExit" __THEMSG, NULL);
+#undef __THEMSG
+    }
+  }
+}
+
+void DynamicArchive::prepare_for_dump_at_exit() {
   EXCEPTION_MARK;
   ResourceMark rm(THREAD);
   MetaspaceShared::link_shared_classes(THREAD);
@@ -367,41 +389,27 @@ void DynamicArchive::prepare_for_dynamic_dumping() {
   }
 }
 
-void DynamicArchive::dump(const char* archive_name, TRAPS) {
-  assert(UseSharedSpaces && RecordDynamicDumpInfo, "already checked in arguments.cpp?");
-  assert(ArchiveClassesAtExit == nullptr, "already checked in arguments.cpp?");
-  ArchiveClassesAtExit = archive_name;
-  if (Arguments::init_shared_archive_paths()) {
-    prepare_for_dynamic_dumping();
-    if (DynamicDumpSharedSpaces) {
-      dump(CHECK);
-    }
-  } else {
-    ArchiveClassesAtExit = nullptr;
-    THROW_MSG(vmSymbols::java_lang_RuntimeException(),
-              "Could not setup SharedDynamicArchivePath");
-  }
-  // prevent do dynamic dump at exit.
-  ArchiveClassesAtExit = nullptr;
-  if (!Arguments::init_shared_archive_paths()) {
-    THROW_MSG(vmSymbols::java_lang_RuntimeException(),
-              "Could not restore SharedDynamicArchivePath");
-  }
+// This is called by "jcmd VM.cds dynamic_dump"
+void DynamicArchive::dump_for_jcmd(const char* archive_name, TRAPS) {
+  assert(UseSharedSpaces && RecordDynamicDumpInfo, "already checked in arguments.cpp");
+  assert(ArchiveClassesAtExit == nullptr, "already checked in arguments.cpp");
+  assert(DynamicDumpSharedSpaces, "already checked by check_for_dynamic_dump() during VM startup");
+  MetaspaceShared::link_shared_classes(CHECK);
+  dump(archive_name, THREAD);
 }
 
-void DynamicArchive::dump(TRAPS) {
-  if (Arguments::GetSharedDynamicArchivePath() == NULL) {
-    log_warning(cds, dynamic)("SharedDynamicArchivePath is not specified");
-    return;
-  }
-
+void DynamicArchive::dump(const char* archive_name, TRAPS) {
   // copy shared path table to saved.
   FileMapInfo::clone_shared_path_table(CHECK);
 
-  VM_PopulateDynamicDumpSharedSpace op;
+  VM_PopulateDynamicDumpSharedSpace op(archive_name);
   VMThread::execute(&op);
 }
 
+bool DynamicArchive::should_dump_at_vm_exit() {
+  return DynamicDumpSharedSpaces && (ArchiveClassesAtExit != nullptr);
+}
+
 bool DynamicArchive::validate(FileMapInfo* dynamic_info) {
   assert(!dynamic_info->is_static(), "must be");
   // Check if the recorded base archive matches with the current one

src/hotspot/share/cds/dynamicArchive.hpp

@@ -59,9 +59,11 @@ class DynamicArchiveHeader : public FileMapHeader {
 
 class DynamicArchive : AllStatic {
 public:
-  static void prepare_for_dynamic_dumping();
+  static void check_for_dynamic_dump();
+  static bool should_dump_at_vm_exit();
+  static void prepare_for_dump_at_exit();
+  static void dump_for_jcmd(const char* archive_name, TRAPS);
   static void dump(const char* archive_name, TRAPS);
-  static void dump(TRAPS);
   static bool is_mapped() { return FileMapInfo::dynamic_info() != NULL; }
   static bool validate(FileMapInfo* dynamic_info);
 };

src/hotspot/share/memory/metaspace.cpp

@@ -749,10 +749,6 @@ void Metaspace::global_initialize() {
     // If any of the archived space fails to map, UseSharedSpaces
     // is reset to false.
   }
-
-  if (DynamicDumpSharedSpaces && !UseSharedSpaces) {
-    vm_exit_during_initialization("DynamicDumpSharedSpaces is unsupported when base CDS archive is not loaded", NULL);
-  }
 #endif // INCLUDE_CDS
 
 #ifdef _LP64

src/hotspot/share/memory/universe.cpp

@@ -23,6 +23,7 @@
  */
 
 #include "precompiled.hpp"
+#include "cds/dynamicArchive.hpp"
 #include "cds/heapShared.hpp"
 #include "cds/metaspaceShared.hpp"
 #include "classfile/classLoader.hpp"
@@ -765,6 +766,7 @@ jint universe_init() {
   Universe::_do_stack_walk_cache = new LatestMethodCache();
 
 #if INCLUDE_CDS
+  DynamicArchive::check_for_dynamic_dump();
   if (UseSharedSpaces) {
     // Read the data structures supporting the shared spaces (shared
     // system dictionary, symbol table, etc.).  After that, access to

src/hotspot/share/prims/jvm.cpp

@@ -426,8 +426,8 @@ extern volatile jint vm_created;
 JVM_ENTRY_NO_ENV(void, JVM_BeforeHalt())
 #if INCLUDE_CDS
   // Link all classes for dynamic CDS dumping before vm exit.
-  if (DynamicDumpSharedSpaces) {
-    DynamicArchive::prepare_for_dynamic_dumping();
+  if (DynamicArchive::should_dump_at_vm_exit()) {
+    DynamicArchive::prepare_for_dump_at_exit();
   }
 #endif
   EventShutdown event;
@@ -3706,7 +3706,7 @@ JVM_ENTRY(void, JVM_DumpDynamicArchive(JNIEnv *env, jstring archiveName))
   ResourceMark rm(THREAD);
   Handle file_handle(THREAD, JNIHandles::resolve_non_null(archiveName));
   char* archive_name  = java_lang_String::as_utf8_string(file_handle());
-  DynamicArchive::dump(archive_name, CHECK);
+  DynamicArchive::dump_for_jcmd(archive_name, CHECK);
 #endif // INCLUDE_CDS
 JVM_END
 

src/hotspot/share/runtime/arguments.cpp

@@ -1439,6 +1439,8 @@ bool Arguments::check_unsupported_cds_runtime_properties() {
     if (get_property(unsupported_properties[i]) != NULL) {
       if (RequireSharedSpaces) {
         warning("CDS is disabled when the %s option is specified.", unsupported_options[i]);
+      } else {
+        log_info(cds)("CDS is disabled when the %s option is specified.", unsupported_options[i]);
       }
       return true;
     }
@@ -3117,17 +3119,11 @@ jint Arguments::finalize_vm_init_args(bool patch_mod_javabase) {
     // TODO: revisit the following for the static archive case.
     set_mode_flags(_int);
   }
-  if (DumpSharedSpaces || ArchiveClassesAtExit != NULL) {
-    // Always verify non-system classes during CDS dump
-    if (!BytecodeVerificationRemote) {
-      BytecodeVerificationRemote = true;
-      log_info(cds)("All non-system classes will be verified (-Xverify:remote) during CDS dump time.");
-    }
-  }
 
   // RecordDynamicDumpInfo is not compatible with ArchiveClassesAtExit
   if (ArchiveClassesAtExit != NULL && RecordDynamicDumpInfo) {
-    log_info(cds)("RecordDynamicDumpInfo is for jcmd only, could not set with -XX:ArchiveClassesAtExit.");
+    jio_fprintf(defaultStream::output_stream(),
+                "-XX:+RecordDynamicDumpInfo cannot be used with -XX:ArchiveClassesAtExit.\n");
     return JNI_ERR;
   }
 
@@ -3143,6 +3139,14 @@ jint Arguments::finalize_vm_init_args(bool patch_mod_javabase) {
   if (UseSharedSpaces && !DumpSharedSpaces && check_unsupported_cds_runtime_properties()) {
     FLAG_SET_DEFAULT(UseSharedSpaces, false);
   }
+
+  if (DumpSharedSpaces || DynamicDumpSharedSpaces) {
+    // Always verify non-system classes during CDS dump
+    if (!BytecodeVerificationRemote) {
+      BytecodeVerificationRemote = true;
+      log_info(cds)("All non-system classes will be verified (-Xverify:remote) during CDS dump time.");
+    }
+  }
 #endif
 
 #ifndef CAN_SHOW_REGISTERS_ON_ASSERT
@@ -3422,9 +3426,7 @@ jint Arguments::set_shared_spaces_flags_and_archive_paths() {
 #if INCLUDE_CDS
   // Initialize shared archive paths which could include both base and dynamic archive paths
   // This must be after set_ergonomics_flags() called so flag UseCompressedOops is set properly.
-  if (!init_shared_archive_paths()) {
-    return JNI_ENOMEM;
-  }
+  init_shared_archive_paths();
 #endif  // INCLUDE_CDS
   return JNI_OK;
 }
@@ -3487,45 +3489,45 @@ void Arguments::extract_shared_archive_paths(const char* archive_path,
   len = end_ptr - begin_ptr;
   cur_path = NEW_C_HEAP_ARRAY(char, len + 1, mtInternal);
   strncpy(cur_path, begin_ptr, len + 1);
-  //cur_path[len] = '\0';
   FileMapInfo::check_archive((const char*)cur_path, false /*is_static*/);
   *top_archive_path = cur_path;
 }
 
-bool Arguments::init_shared_archive_paths() {
-  if (ArchiveClassesAtExit != NULL) {
+void Arguments::init_shared_archive_paths() {
+  if (ArchiveClassesAtExit != nullptr) {
+    assert(!RecordDynamicDumpInfo, "already checked");
     if (DumpSharedSpaces) {
       vm_exit_during_initialization("-XX:ArchiveClassesAtExit cannot be used with -Xshare:dump");
     }
-    if (FLAG_SET_CMDLINE(DynamicDumpSharedSpaces, true) != JVMFlag::SUCCESS) {
-      return false;
-    }
     check_unsupported_dumping_properties();
-    SharedDynamicArchivePath = os::strdup_check_oom(ArchiveClassesAtExit, mtArguments);
-  } else {
-    if (SharedDynamicArchivePath != nullptr) {
-      os::free(SharedDynamicArchivePath);
-      SharedDynamicArchivePath = nullptr;
-    }
   }
-  if (SharedArchiveFile == NULL) {
+
+  if (SharedArchiveFile == nullptr) {
     SharedArchivePath = get_default_shared_archive_path();
   } else {
     int archives = num_archives(SharedArchiveFile);
-    if (is_dumping_archive()) {
-      if (archives > 1) {
-        vm_exit_during_initialization(
-          "Cannot have more than 1 archive file specified in -XX:SharedArchiveFile during CDS dumping");
-      }
-      if (DynamicDumpSharedSpaces) {
-        if (os::same_files(SharedArchiveFile, ArchiveClassesAtExit)) {
-          vm_exit_during_initialization(
-            "Cannot have the same archive file specified for -XX:SharedArchiveFile and -XX:ArchiveClassesAtExit",
-            SharedArchiveFile);
-        }
-      }
+    assert(archives > 0, "must be");
+
+    if (is_dumping_archive() && archives > 1) {
+      vm_exit_during_initialization(
+        "Cannot have more than 1 archive file specified in -XX:SharedArchiveFile during CDS dumping");
     }
-    if (!is_dumping_archive()){
+
+    if (DumpSharedSpaces) {
+      assert(archives == 1, "must be");
+      // Static dump is simple: only one archive is allowed in SharedArchiveFile. This file
+      // will be overwritten no matter regardless of its contents
+      SharedArchivePath = os::strdup_check_oom(SharedArchiveFile, mtArguments);
+    } else {
+      // SharedArchiveFile may specify one or two files. In case (c), the path for base.jsa
+      // is read from top.jsa
+      //    (a) 1 file:  -XX:SharedArchiveFile=base.jsa
+      //    (b) 2 files: -XX:SharedArchiveFile=base.jsa:top.jsa
+      //    (c) 2 files: -XX:SharedArchiveFile=top.jsa
+      //
+      // However, if either RecordDynamicDumpInfo or ArchiveClassesAtExit is used, we do not
+      // allow cases (b) and (c). Case (b) is already checked above.
+
       if (archives > 2) {
         vm_exit_during_initialization(
           "Cannot have more than 2 archive files specified in the -XX:SharedArchiveFile option");
@@ -3543,11 +3545,26 @@ bool Arguments::init_shared_archive_paths() {
         extract_shared_archive_paths((const char*)SharedArchiveFile,
                                       &SharedArchivePath, &SharedDynamicArchivePath);
       }
-    } else { // CDS dumping
-      SharedArchivePath = os::strdup_check_oom(SharedArchiveFile, mtArguments);
+
+      if (SharedDynamicArchivePath != nullptr) {
+        // Check for case (c)
+        if (RecordDynamicDumpInfo) {
+          vm_exit_during_initialization("-XX:+RecordDynamicDumpInfo is unsupported when a dynamic CDS archive is specified in -XX:SharedArchiveFile",
+                                        SharedArchiveFile);
+        }
+        if (ArchiveClassesAtExit != nullptr) {
+          vm_exit_during_initialization("-XX:ArchiveClassesAtExit is unsupported when a dynamic CDS archive is specified in -XX:SharedArchiveFile",
+                                        SharedArchiveFile);
+        }
+      }
+
+      if (ArchiveClassesAtExit != nullptr && os::same_files(SharedArchiveFile, ArchiveClassesAtExit)) {
+          vm_exit_during_initialization(
+            "Cannot have the same archive file specified for -XX:SharedArchiveFile and -XX:ArchiveClassesAtExit",
+            SharedArchiveFile);
+      }
     }
   }
-  return (SharedArchivePath != NULL);
 }
 #endif // INCLUDE_CDS
 

src/hotspot/share/runtime/arguments.hpp

@@ -618,7 +618,7 @@ class Arguments : AllStatic {
   static void  fix_appclasspath();
 
   static char* get_default_shared_archive_path() NOT_CDS_RETURN_(NULL);
-  static bool  init_shared_archive_paths() NOT_CDS_RETURN_(false);
+  static void  init_shared_archive_paths() NOT_CDS_RETURN;
 
   // Operation modi
   static Mode mode()                { return _mode;           }

src/hotspot/share/runtime/java.cpp

@@ -503,9 +503,10 @@ void before_exit(JavaThread* thread) {
   os::terminate_signal_thread();
 
 #if INCLUDE_CDS
-  if (DynamicDumpSharedSpaces) {
+  if (DynamicArchive::should_dump_at_vm_exit()) {
+    assert(ArchiveClassesAtExit != NULL, "Must be already set");
     ExceptionMark em(thread);
-    DynamicArchive::dump(thread);
+    DynamicArchive::dump(ArchiveClassesAtExit, thread);
     if (thread->has_pending_exception()) {
       ResourceMark rm(thread);
       oop pending_exception = thread->pending_exception();

src/hotspot/share/runtime/thread.cpp

@@ -3303,8 +3303,8 @@ void JavaThread::invoke_shutdown_hooks() {
   // Link all classes for dynamic CDS dumping before vm exit.
   // Same operation is being done in JVM_BeforeHalt for handling the
   // case where the application calls System.exit().
-  if (DynamicDumpSharedSpaces) {
-    DynamicArchive::prepare_for_dynamic_dumping();
+  if (DynamicArchive::should_dump_at_vm_exit()) {
+    DynamicArchive::prepare_for_dump_at_exit();
   }
 #endif