8313733: CDS is disabled when java.base module is patched

make/Images.gmk

@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2014, 2023, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2014, 2024, Oracle and/or its affiliates. All rights reserved.
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # This code is free software; you can redistribute it and/or modify it
@@ -131,54 +131,57 @@ CDS_DUMP_FLAGS = -Xmx128M -Xms128M
 #
 # Param1 - VM variant (e.g., server, client, zero, ...)
 # Param2 - _nocoops, or empty
+# Param3 - _valhalla, or empty
 define CreateCDSArchive
-  $1_$2_DUMP_EXTRA_ARG := $(if $(filter _nocoops, $2),-XX:-UseCompressedOops,)
-  $1_$2_DUMP_TYPE      := $(if $(filter _nocoops, $2),-NOCOOPS,)
+  $1_$2_$3_DUMP_EXTRA_ARG := $(if $(filter _nocoops, $2),-XX:-UseCompressedOops,) $(if $(filter _valhalla, $3),--enable-preview,)
+  $1_$2_$3_DUMP_TYPE      := $(if $(filter _nocoops, $2),-NOCOOPS,)$(if $(filter _valhalla, $3),-VALHALLA,)
 
   # Only G1 supports dumping the shared heap, so explicitly use G1 if the JVM supports it.
-  $1_$2_CDS_DUMP_FLAGS := $(CDS_DUMP_FLAGS) $(if $(filter g1gc, $(JVM_FEATURES_$1)),-XX:+UseG1GC)
+  $1_$2_$3_CDS_DUMP_FLAGS := $(CDS_DUMP_FLAGS) $(if $(filter g1gc, $(JVM_FEATURES_$1)),-XX:+UseG1GC)
 
   ifeq ($(OPENJDK_TARGET_OS), windows)
-    $1_$2_CDS_ARCHIVE := bin/$1/classes$2.jsa
+    $1_$2_$3_CDS_ARCHIVE := bin/$1/classes$2$3.jsa
   else
-    $1_$2_CDS_ARCHIVE := lib/$1/classes$2.jsa
+    $1_$2_$3_CDS_ARCHIVE := lib/$1/classes$2$3.jsa
   endif
 
-  $$(eval $$(call SetupExecute, $1_$2_gen_cds_archive_jdk, \
-      WARN := Creating CDS$$($1_$2_DUMP_TYPE) archive for jdk image for $1, \
-      INFO := Using CDS flags for $1: $$($1_$2_CDS_DUMP_FLAGS), \
+  $$(eval $$(call SetupExecute, $1_$2_$3_gen_cds_archive_jdk, \
+      WARN := Creating CDS$$($1_$2_$3_DUMP_TYPE) archive for jdk image for $1, \
+      INFO := Using CDS flags for $1: $$($1_$2_$3_CDS_DUMP_FLAGS), \
       DEPS := $$(jlink_jdk), \
-      OUTPUT_FILE := $$(JDK_IMAGE_DIR)/$$($1_$2_CDS_ARCHIVE), \
+      OUTPUT_FILE := $$(JDK_IMAGE_DIR)/$$($1_$2_$3_CDS_ARCHIVE), \
       SUPPORT_DIR := $$(JDK_IMAGE_SUPPORT_DIR), \
       COMMAND := $$(FIXPATH) $$(JDK_IMAGE_DIR)/bin/java -Xshare:dump \
-          -XX:SharedArchiveFile=$$(JDK_IMAGE_DIR)/$$($1_$2_CDS_ARCHIVE) \
-          -$1 $$($1_$2_DUMP_EXTRA_ARG) $$($1_$2_CDS_DUMP_FLAGS) $$(LOG_INFO), \
+          -XX:SharedArchiveFile=$$(JDK_IMAGE_DIR)/$$($1_$2_$3_CDS_ARCHIVE) \
+          -$1 $$($1_$2_$3_DUMP_EXTRA_ARG) $$($1_$2_$3_CDS_DUMP_FLAGS) $$(LOG_INFO), \
   ))
 
-  JDK_TARGETS += $$($1_$2_gen_cds_archive_jdk)
+  JDK_TARGETS += $$($1_$2_$3_gen_cds_archive_jdk)
 
-  $$(eval $$(call SetupExecute, $1_$2_gen_cds_archive_jre, \
-      WARN := Creating CDS$$($1_$2_DUMP_TYPE) archive for jre image for $1, \
-      INFO := Using CDS flags for $1: $$($1_$2_CDS_DUMP_FLAGS), \
+  $$(eval $$(call SetupExecute, $1_$2_$3_gen_cds_archive_jre, \
+      WARN := Creating CDS$$($1_$2_$3_DUMP_TYPE) archive for jre image for $1, \
+      INFO := Using CDS flags for $1: $$($1_$2_$3_CDS_DUMP_FLAGS), \
       DEPS := $$(jlink_jre), \
-      OUTPUT_FILE := $$(JRE_IMAGE_DIR)/$$($1_$2_CDS_ARCHIVE), \
+      OUTPUT_FILE := $$(JRE_IMAGE_DIR)/$$($1_$2_$3_CDS_ARCHIVE), \
       SUPPORT_DIR := $$(JRE_IMAGE_SUPPORT_DIR), \
       COMMAND := $$(FIXPATH) $$(JRE_IMAGE_DIR)/bin/java -Xshare:dump \
-          -XX:SharedArchiveFile=$$(JRE_IMAGE_DIR)/$$($1_$2_CDS_ARCHIVE) \
-          -$1 $$($1_$2_DUMP_EXTRA_ARG) $$($1_$2_CDS_DUMP_FLAGS) $$(LOG_INFO), \
+          -XX:SharedArchiveFile=$$(JRE_IMAGE_DIR)/$$($1_$2_$3_CDS_ARCHIVE) \
+          -$1 $$($1_$2_$3_DUMP_EXTRA_ARG) $$($1_$2_$3_CDS_DUMP_FLAGS) $$(LOG_INFO), \
   ))
 
-  JRE_TARGETS += $$($1_$2_gen_cds_archive_jre)
+  JRE_TARGETS += $$($1_$2_$3_gen_cds_archive_jre)
 endef
 
 ifeq ($(BUILD_CDS_ARCHIVE), true)
   $(foreach v, $(JVM_VARIANTS), \
-    $(eval $(call CreateCDSArchive,$v,)) \
+    $(eval $(call CreateCDSArchive,$v,,)) \
+    $(eval $(call CreateCDSArchive,$v,,_valhalla)) \
   )
 
   ifeq ($(call isTargetCpuBits, 64), true)
     $(foreach v, $(JVM_VARIANTS), \
-      $(eval $(call CreateCDSArchive,$v,_nocoops)) \
+      $(eval $(call CreateCDSArchive,$v,_nocoops,)) \
+      $(eval $(call CreateCDSArchive,$v,_nocoops,_valhalla)) \
     )
   endif
 endif

src/hotspot/share/cds/filemap.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -279,7 +279,7 @@ void FileMapHeader::populate(FileMapInfo *info, size_t core_region_alignment,
   _max_heap_size = MaxHeapSize;
   _use_optimized_module_handling = MetaspaceShared::use_optimized_module_handling();
   _has_full_module_graph = CDSConfig::is_dumping_full_module_graph();
-
+  _has_valhalla_patched_classes = Arguments::enable_preview() && EnableValhalla;
   // The following fields are for sanity checks for whether this archive
   // will function correctly with this JVM and the bootclasspath it's
   // invoked with.
@@ -358,6 +358,7 @@ void FileMapHeader::print(outputStream* st) {
   st->print_cr("- allow_archiving_with_java_agent:%d", _allow_archiving_with_java_agent);
   st->print_cr("- use_optimized_module_handling:  %d", _use_optimized_module_handling);
   st->print_cr("- has_full_module_graph           %d", _has_full_module_graph);
+  st->print_cr("- has_valhalla_patched_classes    %d", _has_valhalla_patched_classes);
   st->print_cr("- ptrmap_size_in_bits:            " SIZE_FORMAT, _ptrmap_size_in_bits);
   _must_match.print(st);
 }
@@ -2453,6 +2454,24 @@ bool FileMapHeader::validate() {
     return false;
   }
 
+  if (is_static()) {
+    const char* err = nullptr;
+    if (Arguments::enable_preview() && EnableValhalla) {
+      if (!_has_valhalla_patched_classes) {
+        err = "not created";
+      }
+    } else {
+      if (_has_valhalla_patched_classes) {
+        err = "created";
+      }
+    }
+    if (err != nullptr) {
+      log_warning(cds)("This archive was %s with --enable-preview -XX:+EnableValhalla. It is "
+                         "incompatible with the current JVM setting", err);
+      return false;
+    }
+  }
+
   if (!_use_optimized_module_handling) {
     MetaspaceShared::disable_optimized_module_handling();
     log_info(cds)("optimized module handling: disabled because archive was created without optimized module handling");

src/hotspot/share/cds/filemap.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -253,6 +253,7 @@ class FileMapHeader: private CDSFileMapHeaderBase {
   bool   _use_optimized_module_handling;// No module-relation VM options were specified, so we can skip
                                         // some expensive operations.
   bool   _has_full_module_graph;        // Does this CDS archive contain the full archived module graph?
+  bool   _has_valhalla_patched_classes; // Is this archived dumped with --enable-preview -XX:+EnableValhalla?
   size_t _ptrmap_size_in_bits;          // Size of pointer relocation bitmap
   CDSMustMatchFlags _must_match;        // These flags must be the same between dumptime and runtime
   size_t _heap_roots_offset;            // Offset of the HeapShared::roots() object, from the bottom

src/hotspot/share/cds/heapShared.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1605,6 +1605,13 @@ void HeapShared::archive_object_subgraphs(ArchivableStaticFieldInfo fields[],
   for (int i = 0; fields[i].valid(); ) {
     ArchivableStaticFieldInfo* info = &fields[i];
     const char* klass_name = info->klass_name;
+
+    if (Arguments::enable_preview() && EnableValhalla && strcmp(klass_name, "jdk/internal/module/ArchivedModuleGraph") == 0) {
+      // FIXME -- ArchivedModuleGraph doesn't work when java.base is patched with valhalla classes.
+      i++;
+      continue;
+    }
+
     start_recording_subgraph(info->klass, klass_name, is_full_module_graph);
 
     // If you have specified consecutive fields of the same klass in

src/hotspot/share/classfile/classLoader.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1125,10 +1125,11 @@ InstanceKlass* ClassLoader::load_class(Symbol* name, bool search_append_only, TR
     // appear in the _patch_mod_entries. The runtime shared class visibility
     // check will determine if a shared class is visible based on the runtime
     // environment, including the runtime --patch-module setting.
-    //
-    // Dynamic dumping requires UseSharedSpaces to be enabled. Since --patch-module
-    // is not supported with UseSharedSpaces, we can never come here during dynamic dumping.
-    assert(!CDSConfig::is_dumping_dynamic_archive(), "sanity");
+    if (!Arguments::enable_preview() && EnableValhalla) {
+      // Dynamic dumping requires UseSharedSpaces to be enabled. Since --patch-module
+      // is not supported with UseSharedSpaces, we can never come here during dynamic dumping.
+      assert(!CDSConfig::is_dumping_dynamic_archive(), "sanity");
+    }
     if (!CDSConfig::is_dumping_static_archive()) {
       stream = search_module_entries(THREAD, _patch_mod_entries, class_name, file_name);
     }

src/hotspot/share/classfile/systemDictionary.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -940,8 +940,7 @@ bool SystemDictionary::is_shared_class_visible(Symbol* class_name,
                                                InstanceKlass* ik,
                                                PackageEntry* pkg_entry,
                                                Handle class_loader) {
-  assert(!ModuleEntryTable::javabase_moduleEntry()->is_patched(),
-         "Cannot use sharing if java.base is patched");
+  assert(!Arguments::module_patching_disables_cds(), "Cannot use CDS");
 
   // (1) Check if we are loading into the same loader as in dump time.
 
@@ -1017,7 +1016,7 @@ bool SystemDictionary::is_shared_class_visible_impl(Symbol* class_name,
       // 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");
+        assert(!Arguments::module_patching_disables_cds(), "Cannot use CDS");
       }
     } else {
       // During dump time, this class was in a named module, but at run time, this class should be

src/hotspot/share/oops/instanceKlass.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1006,6 +1006,7 @@ bool InstanceKlass::link_class_impl(TRAPS) {
     }
     // Aggressively preloading all classes from the Preload attribute
     if (preload_classes() != nullptr) {
+      HandleMark hm(THREAD);
       for (int i = 0; i < preload_classes()->length(); i++) {
         if (constants()->tag_at(preload_classes()->at(i)).is_klass()) continue;
         Symbol* class_name = constants()->klass_at_noresolve(preload_classes()->at(i));
@@ -2833,9 +2834,7 @@ void InstanceKlass::metaspace_pointers_do(MetaspaceClosure* it) {
   it->push(&_preload_classes);
   it->push(&_record_components);
 
-  if (has_inline_type_fields()) {
-    it->push(&_inline_type_field_klasses);
-  }
+  it->push(&_inline_type_field_klasses);
 }
 
 #if INCLUDE_CDS

src/hotspot/share/oops/method.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -396,7 +396,7 @@ Symbol* Method::klass_name() const {
 void Method::metaspace_pointers_do(MetaspaceClosure* it) {
   log_trace(cds)("Iter(Method): %p", this);
 
-  if (!method_holder()->is_rewritten()) {
+  if (!method_holder()->is_rewritten() || has_scalarized_args()) {
     it->push(&_constMethod, MetaspaceClosure::_writable);
   } else {
     it->push(&_constMethod);
@@ -1262,7 +1262,6 @@ void Method::link_method(const methodHandle& h_method, TRAPS) {
       !native_bind_event_is_interesting);
   }
   if (InlineTypeReturnedAsFields && returns_inline_type(THREAD) && !has_scalarized_return()) {
-    assert(!constMethod()->is_shared(), "Cannot update shared const objects");
     set_has_scalarized_return();
   }
 

src/hotspot/share/runtime/arguments.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -101,6 +101,7 @@ bool   Arguments::_ClipInlining                 = ClipInlining;
 size_t Arguments::_default_SharedBaseAddress    = SharedBaseAddress;
 
 bool   Arguments::_enable_preview               = false;
+bool   Arguments::_module_patching_disables_cds = false;
 
 char*  Arguments::_default_shared_archive_path  = nullptr;
 char*  Arguments::SharedArchivePath             = nullptr;
@@ -1331,12 +1332,9 @@ bool Arguments::add_property(const char* prop, PropertyWriteable writeable, Prop
 
 #if INCLUDE_CDS
 const char* unsupported_properties[] = { "jdk.module.limitmods",
-                                         "jdk.module.upgrade.path",
-                                         "jdk.module.patch.0" };
+                                         "jdk.module.upgrade.path"};
 const char* unsupported_options[] = { "--limit-modules",
-                                      "--upgrade-module-path",
-                                      "--patch-module"
-                                    };
+                                      "--upgrade-module-path"};
 void Arguments::check_unsupported_dumping_properties() {
   assert(CDSConfig::is_dumping_archive(),
          "this function is only used with CDS dump time");
@@ -1353,6 +1351,11 @@ void Arguments::check_unsupported_dumping_properties() {
     sp = sp->next();
   }
 
+  if (_module_patching_disables_cds) {
+    vm_exit_during_initialization(
+            "Cannot use the following option when dumping the shared archive", "--patch-module");
+  }
+
   // Check for an exploded module build in use with -Xshare:dump.
   if (!has_jimage()) {
     vm_exit_during_initialization("Dumping the shared archive is not supported with an exploded module build");
@@ -1379,6 +1382,16 @@ bool Arguments::check_unsupported_cds_runtime_properties() {
       return true;
     }
   }
+
+  if (_module_patching_disables_cds) {
+    if (RequireSharedSpaces) {
+      warning("CDS is disabled when the %s option is specified.", "--patch-module");
+    } else {
+      log_info(cds)("CDS is disabled when the %s option is specified.", "--patch-module");
+    }
+    return true;
+  }
+
   return false;
 }
 #endif
@@ -2177,7 +2190,7 @@ int Arguments::process_patch_mod_option(const char* patch_mod_tail) {
       memcpy(module_name, patch_mod_tail, module_len);
       *(module_name + module_len) = '\0';
       // The path piece begins one past the module_equal sign
-      add_patch_mod_prefix(module_name, module_equal + 1, false /* no append */);
+      add_patch_mod_prefix(module_name, module_equal + 1, false /* no append */, false /* no cds */);
       FREE_C_HEAP_ARRAY(char, module_name);
     } else {
       return JNI_ENOMEM;
@@ -2219,7 +2232,7 @@ int Arguments::finalize_patch_module() {
         module_name[len] = '\0';     // truncate to just module-name
 
         jio_snprintf(path, JVM_MAXPATHLEN, "%s%s", valueclasses_dir, &entry->d_name);
-        add_patch_mod_prefix(module_name, path, true /* append */);
+        add_patch_mod_prefix(module_name, path, true /* append */, true /* cds OK*/);
         log_info(class)("--enable-preview appending value classes for module %s: %s", module_name, entry->d_name);
       }
       FreeHeap(module_name);
@@ -2982,11 +2995,19 @@ bool match_module(void *module_name, ModulePatchPath *patch) {
   return (strcmp((char *)module_name, patch->module_name()) == 0);
 }
 
+static bool _java_base_module_patching_disables_cds = false;
 bool Arguments::patch_mod_javabase() {
-    return _patch_mod_prefix != nullptr && _patch_mod_prefix->find((void*)JAVA_BASE_NAME, match_module) >= 0;
+  return _java_base_module_patching_disables_cds;
 }
 
-void Arguments::add_patch_mod_prefix(const char* module_name, const char* path, bool allow_append) {
+void Arguments::add_patch_mod_prefix(const char* module_name, const char* path, bool allow_append, bool allow_cds) {
+  if (!allow_cds) {
+    _module_patching_disables_cds = true;
+    if (strcmp(module_name, JAVA_BASE_NAME) == 0) {
+      _java_base_module_patching_disables_cds = true;
+    }
+  }
+
   // Create GrowableArray lazily, only if --patch-module has been specified
   if (_patch_mod_prefix == nullptr) {
     _patch_mod_prefix = new (mtArguments) GrowableArray<ModulePatchPath*>(10, mtArguments);
@@ -3174,7 +3195,7 @@ jint Arguments::finalize_vm_init_args() {
     }
   }
 
-  if (UseSharedSpaces && patch_mod_javabase()) {
+  if (UseSharedSpaces && patch_mod_javabase() && _module_patching_disables_cds) {
     no_shared_spaces("CDS is disabled when " JAVA_BASE_NAME " module is patched.");
   }
   if (UseSharedSpaces && check_unsupported_cds_runtime_properties()) {
@@ -3486,11 +3507,15 @@ char* Arguments::get_default_shared_archive_path() {
     if (end != nullptr) *end = '\0';
     size_t jvm_path_len = strlen(jvm_path);
     size_t file_sep_len = strlen(os::file_separator());
-    const size_t len = jvm_path_len + file_sep_len + 20;
+    const size_t len = jvm_path_len + file_sep_len + strlen("classes_nocoops_valhalla.jsa") + 1;
     _default_shared_archive_path = NEW_C_HEAP_ARRAY(char, len, mtArguments);
-    jio_snprintf(_default_shared_archive_path, len,
-                LP64_ONLY(!UseCompressedOops ? "%s%sclasses_nocoops.jsa":) "%s%sclasses.jsa",
-                jvm_path, os::file_separator());
+    LP64_ONLY(bool nocoops = !UseCompressedOops);
+    NOT_LP64(bool nocoops = false);
+    bool valhalla = (enable_preview() && EnableValhalla);
+    jio_snprintf(_default_shared_archive_path, len, "%s%sclasses%s%s.jsa",
+                jvm_path, os::file_separator(),
+                 nocoops ? "_nocoops" : "",
+                 valhalla ? "_valhalla" : "");
   }
   return _default_shared_archive_path;
 }