From acb1f5d2536712ced9baf0abbeda2e649da3a6d1 Mon Sep 17 00:00:00 2001 From: Calvin Cheung Date: Wed, 16 Apr 2025 18:07:40 +0000 Subject: [PATCH 1/8] 8352003: Support --add-opens with -XX:+AOTClassLinking --- src/hotspot/share/classfile/modules.cpp | 5 +- src/hotspot/share/runtime/arguments.cpp | 2 +- .../jdk/internal/module/ModuleBootstrap.java | 3 +- .../cds/appcds/jigsaw/ExactOptionMatch.java | 4 + .../jigsaw/addopens/AddopensOption.java | 158 ++++++++++++++++++ 5 files changed, 167 insertions(+), 5 deletions(-) create mode 100644 test/hotspot/jtreg/runtime/cds/appcds/jigsaw/addopens/AddopensOption.java diff --git a/src/hotspot/share/classfile/modules.cpp b/src/hotspot/share/classfile/modules.cpp index 3f2ff90ccab31..05be2df52af08 100644 --- a/src/hotspot/share/classfile/modules.cpp +++ b/src/hotspot/share/classfile/modules.cpp @@ -580,13 +580,14 @@ class Modules::ArchivedProperty { }; Modules::ArchivedProperty Modules::_archived_props[] = { - // numbered + // non-numbered {"jdk.module.main", false}, - // non-numbered + // numbered {"jdk.module.addexports", true}, // --add-exports {"jdk.module.addmods", true}, // --add-modules {"jdk.module.enable.native.access", true}, // --enable-native-access + {"jdk.module.addopens", true}, // --add-opens }; constexpr size_t Modules::num_archived_props() { diff --git a/src/hotspot/share/runtime/arguments.cpp b/src/hotspot/share/runtime/arguments.cpp index ef7f07656b5e8..9260f7b57e8eb 100644 --- a/src/hotspot/share/runtime/arguments.cpp +++ b/src/hotspot/share/runtime/arguments.cpp @@ -332,7 +332,6 @@ bool Arguments::internal_module_property_helper(const char* property, bool check if (strncmp(property, MODULE_PROPERTY_PREFIX, MODULE_PROPERTY_PREFIX_LEN) == 0) { const char* property_suffix = property + MODULE_PROPERTY_PREFIX_LEN; if (matches_property_suffix(property_suffix, ADDREADS, ADDREADS_LEN) || - matches_property_suffix(property_suffix, ADDOPENS, ADDOPENS_LEN) || matches_property_suffix(property_suffix, PATCH, PATCH_LEN) || matches_property_suffix(property_suffix, LIMITMODS, LIMITMODS_LEN) || matches_property_suffix(property_suffix, UPGRADE_PATH, UPGRADE_PATH_LEN) || @@ -343,6 +342,7 @@ bool Arguments::internal_module_property_helper(const char* property, bool check if (!check_for_cds) { // CDS notes: these properties are supported by CDS archived full module graph. if (matches_property_suffix(property_suffix, ADDEXPORTS, ADDEXPORTS_LEN) || + matches_property_suffix(property_suffix, ADDOPENS, ADDOPENS_LEN) || matches_property_suffix(property_suffix, PATH, PATH_LEN) || matches_property_suffix(property_suffix, ADDMODS, ADDMODS_LEN) || matches_property_suffix(property_suffix, ENABLE_NATIVE_ACCESS, ENABLE_NATIVE_ACCESS_LEN)) { diff --git a/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java b/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java index 0a9044178a443..52bf4efe37717 100644 --- a/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java +++ b/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java @@ -142,8 +142,7 @@ private static boolean canUseArchivedBootLayer() { return getProperty("jdk.module.upgrade.path") == null && getProperty("jdk.module.patch.0") == null && // --patch-module getProperty("jdk.module.limitmods") == null && // --limit-modules - getProperty("jdk.module.addreads.0") == null && // --add-reads - getProperty("jdk.module.addopens.0") == null; // --add-opens + getProperty("jdk.module.addreads.0") == null; // --add-reads } /** diff --git a/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/ExactOptionMatch.java b/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/ExactOptionMatch.java index e3fb6094c1d9b..dd8f186af8052 100644 --- a/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/ExactOptionMatch.java +++ b/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/ExactOptionMatch.java @@ -39,6 +39,10 @@ public class ExactOptionMatch { record Option(String cmdLine, String property, String valueA, String valueB) {} static Option[] allOptions = new Option[] { + new Option("--add-opens", + "jdk.module.addopens", + "java.base/java.util.concurrent.regex=ALL-UNNAMED", + "java.base/sun.security.x509=ALL-UNNAMED"), new Option("--add-exports", "jdk.module.addexports", "java.base/jdk.internal.misc=ALL-UNNAMED", diff --git a/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/addopens/AddopensOption.java b/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/addopens/AddopensOption.java new file mode 100644 index 0000000000000..ac7c3b55359d7 --- /dev/null +++ b/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/addopens/AddopensOption.java @@ -0,0 +1,158 @@ +/* + * Copyright (c) 2025, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * @test + * @bug 8352003 + * @summary Test handling of the --add-opens option. + * @requires vm.cds.write.archived.java.heap + * @requires vm.flagless + * @library /test/lib /test/hotspot/jtreg/runtime/cds/appcds + * @build jdk.test.whitebox.WhiteBox + * @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox + * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI AddopensOption + */ + +import jdk.test.lib.process.OutputAnalyzer; +import jdk.test.whitebox.code.Compiler; + +public class AddopensOption { + + public static void main(String[] args) throws Exception { + final String moduleOption = "jdk.httpserver/sun.net.httpserver.simpleserver.Main"; + final String addOpensNio = "java.base/java.nio=ALL-UNNAMED"; + final String addOpensTimeFormat = "java.base/java.time.format=ALL-UNNAMED"; + final String loggingOption = "-Xlog:cds=debug,cds+module=debug,cds+heap=info,module=trace"; + final String versionPattern = "java.[0-9][0-9].*"; + final String subgraphCannotBeUsed = "subgraph jdk.internal.module.ArchivedBootLayer cannot be used because full module graph is disabled"; + final String warningIncubator = "WARNING: Using incubator modules: jdk.incubator.vector"; + String archiveName = TestCommon.getNewArchiveName("addopens-option"); + TestCommon.setCurrentArchiveName(archiveName); + + // dump a base archive with --add-opens jdk.jconsole -m jdk.httpserver + OutputAnalyzer oa = TestCommon.dumpBaseArchive( + archiveName, + loggingOption, + "--add-opens", addOpensTimeFormat, + "-m", moduleOption, + "-version"); + oa.shouldHaveExitValue(0); + + // same modules specified during runtime + oa = TestCommon.execCommon( + loggingOption, + "--add-opens", addOpensTimeFormat, + "-m", moduleOption, + "-version"); + oa.shouldHaveExitValue(0) + // version of the jdk.httpserver module, e.g. java 22-ea + .shouldMatch(versionPattern) + //.shouldMatch("cds,module.*Restored from archive: entry.0x.*name jdk.jconsole") + .shouldMatch("cds,module.*Restored from archive: entry.0x.*name jdk.httpserver"); + + // different --add-opens specified during runtime + oa = TestCommon.execCommon( + loggingOption, + "--add-opens", addOpensNio, + "-m", moduleOption, + "-version"); + oa.shouldHaveExitValue(0) + .shouldContain("Mismatched values for property jdk.module.addopens") + .shouldContain("runtime java.base/java.nio=ALL-UNNAMED dump time java.base/java.time.format=ALL-UNNAMED") + .shouldContain(subgraphCannotBeUsed); + + // no module specified during runtime + oa = TestCommon.execCommon( + loggingOption, + "-version"); + oa.shouldHaveExitValue(0) + .shouldContain("jdk.httpserver specified during dump time but not during runtime") + .shouldContain(subgraphCannotBeUsed); + + // dump an archive without the --add-opens option + archiveName = TestCommon.getNewArchiveName("no-addopens-option"); + TestCommon.setCurrentArchiveName(archiveName); + oa = TestCommon.dumpBaseArchive( + archiveName, + loggingOption, + "-m", moduleOption, + "-version"); + oa.shouldHaveExitValue(0); + + // run with --add-opens option + oa = TestCommon.execCommon( + loggingOption, + "--add-opens", addOpensTimeFormat, + "-m", moduleOption, + "-version"); + oa.shouldHaveExitValue(0) + .shouldContain("java.base/java.time.format=ALL-UNNAMED specified during runtime but not during dump time") + // version of the jdk.httpserver module, e.g. java 22-ea + .shouldMatch(versionPattern) + .shouldContain(subgraphCannotBeUsed); + + // dump an archive with -add-opens java.base/java.nio=ALL-UNNAMED + archiveName = TestCommon.getNewArchiveName("addopens-java-nio"); + TestCommon.setCurrentArchiveName(archiveName); + oa = TestCommon.dumpBaseArchive( + archiveName, + loggingOption, + "--add-opens", addOpensNio, + "-m", moduleOption, + "-version"); + oa.shouldHaveExitValue(0) + .shouldContain("Full module graph = enabled"); + + // run with the same --add-opens + oa = TestCommon.execCommon( + loggingOption, + "--add-opens", addOpensNio, + "-m", moduleOption, + "-version"); + oa.shouldContain("optimized module handling: enabled") + .shouldHaveExitValue(0); + + // dump an archive with multiple --add-modules args + archiveName = TestCommon.getNewArchiveName("muti-addopens"); + TestCommon.setCurrentArchiveName(archiveName); + oa = TestCommon.dumpBaseArchive( + archiveName, + loggingOption, + "--add-opens", addOpensNio, + "--add-opens", addOpensTimeFormat, + "-m", moduleOption, + "-version"); + oa.shouldHaveExitValue(0); + + // run with the same multiple --add-modules args with a duplicate --add-opens entry + oa = TestCommon.execCommon( + loggingOption, + "--add-opens", addOpensTimeFormat, + "--add-opens", addOpensNio, + "--add-opens", addOpensTimeFormat, + "-m", moduleOption, + "-version"); + oa.shouldHaveExitValue(0) + .shouldMatch("cds,module.*Restored from archive: entry.0x.*name jdk.compiler"); + } +} From 4fa5f8fbef9d2075a0f7e835d57e319da611219f Mon Sep 17 00:00:00 2001 From: Calvin Cheung Date: Wed, 16 Apr 2025 21:04:55 +0000 Subject: [PATCH 2/8] trailing whitespace --- .../runtime/cds/appcds/jigsaw/addopens/AddopensOption.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/addopens/AddopensOption.java b/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/addopens/AddopensOption.java index ac7c3b55359d7..2f3768832cb6a 100644 --- a/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/addopens/AddopensOption.java +++ b/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/addopens/AddopensOption.java @@ -111,7 +111,7 @@ public static void main(String[] args) throws Exception { .shouldMatch(versionPattern) .shouldContain(subgraphCannotBeUsed); - // dump an archive with -add-opens java.base/java.nio=ALL-UNNAMED + // dump an archive with -add-opens java.base/java.nio=ALL-UNNAMED archiveName = TestCommon.getNewArchiveName("addopens-java-nio"); TestCommon.setCurrentArchiveName(archiveName); oa = TestCommon.dumpBaseArchive( From 658a895bb3cc88889d93a09ff8cba00d2296622a Mon Sep 17 00:00:00 2001 From: Calvin Cheung Date: Thu, 17 Apr 2025 22:56:31 +0000 Subject: [PATCH 3/8] cleanup testcase --- .../runtime/cds/appcds/jigsaw/addopens/AddopensOption.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/addopens/AddopensOption.java b/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/addopens/AddopensOption.java index 2f3768832cb6a..81b24d15f9eae 100644 --- a/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/addopens/AddopensOption.java +++ b/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/addopens/AddopensOption.java @@ -34,7 +34,6 @@ */ import jdk.test.lib.process.OutputAnalyzer; -import jdk.test.whitebox.code.Compiler; public class AddopensOption { @@ -49,7 +48,7 @@ public static void main(String[] args) throws Exception { String archiveName = TestCommon.getNewArchiveName("addopens-option"); TestCommon.setCurrentArchiveName(archiveName); - // dump a base archive with --add-opens jdk.jconsole -m jdk.httpserver + // dump a base archive with --add-opens jdk.java.base/java.time.format -m jdk.httpserver OutputAnalyzer oa = TestCommon.dumpBaseArchive( archiveName, loggingOption, @@ -67,7 +66,6 @@ public static void main(String[] args) throws Exception { oa.shouldHaveExitValue(0) // version of the jdk.httpserver module, e.g. java 22-ea .shouldMatch(versionPattern) - //.shouldMatch("cds,module.*Restored from archive: entry.0x.*name jdk.jconsole") .shouldMatch("cds,module.*Restored from archive: entry.0x.*name jdk.httpserver"); // different --add-opens specified during runtime @@ -153,6 +151,6 @@ public static void main(String[] args) throws Exception { "-m", moduleOption, "-version"); oa.shouldHaveExitValue(0) - .shouldMatch("cds,module.*Restored from archive: entry.0x.*name jdk.compiler"); + .shouldMatch("cds,module.*Restored from archive: entry.0x.*name jdk.httpserver"); } } From 2ae685f8e45800dddd9a114629b974310e063265 Mon Sep 17 00:00:00 2001 From: Calvin Cheung Date: Mon, 21 Apr 2025 06:16:00 +0000 Subject: [PATCH 4/8] @iklam comment --- .../jdk/internal/module/ModuleBootstrap.java | 1 + .../appcds/jigsaw/modulepath/AddOpens.java | 56 +++++++++++-------- 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java b/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java index 52bf4efe37717..d6cbd41c440ad 100644 --- a/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java +++ b/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java @@ -162,6 +162,7 @@ public static ModuleLayer boot() { bootLayer = archivedBootLayer.bootLayer(); BootLoader.getUnnamedModule(); // trigger of BootLoader. CDS.defineArchivedModules(ClassLoaders.platformClassLoader(), ClassLoaders.appClassLoader()); + boolean extraExportsOrOpens = addExtraExportsAndOpens(bootLayer); // assume boot layer has at least one module providing a service // that is mapped to the application class loader. diff --git a/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/modulepath/AddOpens.java b/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/modulepath/AddOpens.java index fbb9b723a902e..aa6abc2723b35 100644 --- a/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/modulepath/AddOpens.java +++ b/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/modulepath/AddOpens.java @@ -57,6 +57,11 @@ public class AddOpens { private static Path moduleDir2 = null; private static Path destJar = null; + private static String addOpensArg = "java.base/java.lang=" + TEST_MODULE1; + private static String extraOpts[][] = + {{"-Xlog:cds", "-Xlog:cds"}, + {"--add-opens", addOpensArg}}; + public static void buildTestModule() throws Exception { // javac -d mods/$TESTMODULE --module-path MOD_DIR src/$TESTMODULE/** @@ -78,29 +83,34 @@ public static void buildTestModule() throws Exception { public static void main(String... args) throws Exception { // compile the modules and create the modular jar files buildTestModule(); - String appClasses[] = {MAIN_CLASS}; - // create an archive with both -cp and --module-path in the command line. - // Only the class in the modular jar in the --module-path will be archived; - // the class in the modular jar in the -cp won't be archived. - OutputAnalyzer output = TestCommon.createArchive( - destJar.toString(), appClasses, - "--module-path", moduleDir.toString(), - "-m", TEST_MODULE1); - TestCommon.checkDump(output); - - // run with the archive using the same command line as in dump time - // plus the "--add-opens java.base/java.lang=com.simple" option. - // The main class should be loaded from the archive. - // The setaccessible(true) on the ClassLoader.defineClass method should - // be successful. - TestCommon.run( "-Xlog:class+load=trace", - "-cp", destJar.toString(), - "--add-opens", "java.base/java.lang=" + TEST_MODULE1, - "--module-path", moduleDir.toString(), - "-m", TEST_MODULE1, "with_add_opens") - .assertNormalExit( - "[class,load] com.simple.Main source: shared objects file", - "method.setAccessible succeeded!"); + String appClasses[] = {MAIN_CLASS, "java/lang/ClassLoader"}; + + for (int i = 0; i < 2; i++) { + // create an archive with both -cp and --module-path, and with the + // --add-opens option if i == 1, in the command line. + // Only the class in the modular jar in the --module-path will be archived; + // the class in the modular jar in the -cp won't be archived. + OutputAnalyzer output = TestCommon.createArchive( + destJar.toString(), appClasses, + extraOpts[i][0], extraOpts[i][1], + "--module-path", moduleDir.toString(), + "-m", TEST_MODULE1); + TestCommon.checkDump(output); + + // run with the archive using the same command line as in dump time + // plus the "--add-opens java.base/java.lang=com.simple" option. + // The main class should be loaded from the archive. + // The setaccessible(true) on the ClassLoader.defineClass method should + // be successful. + TestCommon.run( "-Xlog:class+load=trace", + "-cp", destJar.toString(), + "--add-opens", addOpensArg, + "--module-path", moduleDir.toString(), + "-m", TEST_MODULE1, "with_add_opens") + .assertNormalExit( + "[class,load] com.simple.Main source: shared objects file", + "method.setAccessible succeeded!"); + } } } From 95f2e9c6b3c1c7895075200bf538babd9cfb5681 Mon Sep 17 00:00:00 2001 From: Calvin Cheung Date: Mon, 21 Apr 2025 22:51:57 +0000 Subject: [PATCH 5/8] @iklam comment --- .../jdk/internal/module/ModuleBootstrap.java | 10 ++---- .../appcds/jigsaw/modulepath/AddOpens.java | 36 +++++++++++++------ 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java b/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java index d6cbd41c440ad..3601d2059a08c 100644 --- a/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java +++ b/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java @@ -162,7 +162,7 @@ public static ModuleLayer boot() { bootLayer = archivedBootLayer.bootLayer(); BootLoader.getUnnamedModule(); // trigger of BootLoader. CDS.defineArchivedModules(ClassLoaders.platformClassLoader(), ClassLoaders.appClassLoader()); - boolean extraExportsOrOpens = addExtraExportsAndOpens(bootLayer); + addExtraExportsAndOpens(bootLayer); // assume boot layer has at least one module providing a service // that is mapped to the application class loader. @@ -452,7 +452,7 @@ private static ModuleLayer boot2() { // --add-reads, --add-exports/--add-opens addExtraReads(bootLayer); - boolean extraExportsOrOpens = addExtraExportsAndOpens(bootLayer); + addExtraExportsAndOpens(bootLayer); // add enable native access addEnableNativeAccess(bootLayer); @@ -722,15 +722,13 @@ private static void addExtraReads(ModuleLayer bootLayer) { * Process the --add-exports and --add-opens options to export/open * additional packages specified on the command-line. */ - private static boolean addExtraExportsAndOpens(ModuleLayer bootLayer) { - boolean extraExportsOrOpens = false; + private static void addExtraExportsAndOpens(ModuleLayer bootLayer) { // --add-exports String prefix = "jdk.module.addexports."; Map> extraExports = decode(prefix); if (!extraExports.isEmpty()) { addExtraExportsOrOpens(bootLayer, extraExports, false); - extraExportsOrOpens = true; } @@ -739,10 +737,8 @@ private static boolean addExtraExportsAndOpens(ModuleLayer bootLayer) { Map> extraOpens = decode(prefix); if (!extraOpens.isEmpty()) { addExtraExportsOrOpens(bootLayer, extraOpens, true); - extraExportsOrOpens = true; } - return extraExportsOrOpens; } private static void addExtraExportsOrOpens(ModuleLayer bootLayer, diff --git a/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/modulepath/AddOpens.java b/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/modulepath/AddOpens.java index aa6abc2723b35..e539f29367da6 100644 --- a/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/modulepath/AddOpens.java +++ b/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/modulepath/AddOpens.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2018, 2025, 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 @@ -58,9 +58,13 @@ public class AddOpens { private static Path destJar = null; private static String addOpensArg = "java.base/java.lang=" + TEST_MODULE1; + private static String addOpensAllUnnamed = "java.base/java.lang=ALL-UNNAMED"; private static String extraOpts[][] = {{"-Xlog:cds", "-Xlog:cds"}, {"--add-opens", addOpensArg}}; + private static String expectedOutput[] = + { "[class,load] com.simple.Main source: shared objects file", + "method.setAccessible succeeded!"}; public static void buildTestModule() throws Exception { @@ -83,18 +87,19 @@ public static void buildTestModule() throws Exception { public static void main(String... args) throws Exception { // compile the modules and create the modular jar files buildTestModule(); - String appClasses[] = {MAIN_CLASS, "java/lang/ClassLoader"}; + String appClasses[] = {MAIN_CLASS}; + OutputAnalyzer output; for (int i = 0; i < 2; i++) { // create an archive with both -cp and --module-path, and with the // --add-opens option if i == 1, in the command line. // Only the class in the modular jar in the --module-path will be archived; // the class in the modular jar in the -cp won't be archived. - OutputAnalyzer output = TestCommon.createArchive( - destJar.toString(), appClasses, - extraOpts[i][0], extraOpts[i][1], - "--module-path", moduleDir.toString(), - "-m", TEST_MODULE1); + output = TestCommon.createArchive( + destJar.toString(), appClasses, + extraOpts[i][0], extraOpts[i][1], + "--module-path", moduleDir.toString(), + "-m", TEST_MODULE1); TestCommon.checkDump(output); // run with the archive using the same command line as in dump time @@ -107,10 +112,21 @@ public static void main(String... args) throws Exception { "--add-opens", addOpensArg, "--module-path", moduleDir.toString(), "-m", TEST_MODULE1, "with_add_opens") - .assertNormalExit( - "[class,load] com.simple.Main source: shared objects file", - "method.setAccessible succeeded!"); + .assertNormalExit(expectedOutput[0], expectedOutput[1]); } + // Test --add-opens to ALL-UNNAMED modules + output = TestCommon.createArchive( + destJar.toString(), appClasses, + "--add-opens", addOpensAllUnnamed, + MAIN_CLASS); + TestCommon.checkDump(output); + + TestCommon.run( "-Xlog:class+load=trace", + "-cp", destJar.toString(), + "--add-opens", addOpensAllUnnamed, + MAIN_CLASS, "with_add_opens") + .assertNormalExit(expectedOutput[0], expectedOutput[1]); + } } From 465261f720d94d8dd631a261df0616fce63bd35e Mon Sep 17 00:00:00 2001 From: Calvin Cheung Date: Tue, 22 Apr 2025 05:26:34 +0000 Subject: [PATCH 6/8] trailing whitespace --- .../jtreg/runtime/cds/appcds/jigsaw/modulepath/AddOpens.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/modulepath/AddOpens.java b/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/modulepath/AddOpens.java index e539f29367da6..b181d23f64cec 100644 --- a/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/modulepath/AddOpens.java +++ b/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/modulepath/AddOpens.java @@ -61,7 +61,7 @@ public class AddOpens { private static String addOpensAllUnnamed = "java.base/java.lang=ALL-UNNAMED"; private static String extraOpts[][] = {{"-Xlog:cds", "-Xlog:cds"}, - {"--add-opens", addOpensArg}}; + {"--add-opens", addOpensArg}}; private static String expectedOutput[] = { "[class,load] com.simple.Main source: shared objects file", "method.setAccessible succeeded!"}; From 0f28e07b1be8b83fca89949845c6e1f37e25bfe7 Mon Sep 17 00:00:00 2001 From: Calvin Cheung Date: Tue, 22 Apr 2025 23:37:01 +0000 Subject: [PATCH 7/8] @AlanBateman suggestion --- .../share/classes/java/lang/Module.java | 90 ++++++++----------- .../share/classes/java/lang/System.java | 3 - .../jdk/internal/access/JavaLangAccess.java | 5 -- 3 files changed, 38 insertions(+), 60 deletions(-) diff --git a/src/java.base/share/classes/java/lang/Module.java b/src/java.base/share/classes/java/lang/Module.java index dcc92d012de59..065e1ac46208e 100644 --- a/src/java.base/share/classes/java/lang/Module.java +++ b/src/java.base/share/classes/java/lang/Module.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 2025, 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 @@ -40,7 +40,6 @@ import java.net.URI; import java.net.URL; import java.security.CodeSource; -import java.security.ProtectionDomain; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -688,7 +687,6 @@ public boolean isOpen(String pn) { return implIsExportedOrOpen(pn, EVERYONE_MODULE, /*open*/true); } - /** * Returns {@code true} if this module exports or opens the given package * to the given module. If the other module is {@code EVERYONE_MODULE} then @@ -707,12 +705,12 @@ private boolean implIsExportedOrOpen(String pn, Module other, boolean open) { if (descriptor.isOpen() || descriptor.isAutomatic()) return descriptor.packages().contains(pn); - // exported/opened via module declaration/descriptor - if (isStaticallyExportedOrOpen(pn, other, open)) + // exported/opened via module declaration/descriptor or CLI options + if (isExplicitlyExportedOrOpened(pn, other, open)) return true; // exported via addExports/addOpens - if (isReflectivelyExportedOrOpen(pn, other, open)) + if (isReflectivelyExportedOrOpened(pn, other, open)) return true; // not exported or open to other @@ -723,7 +721,7 @@ private boolean implIsExportedOrOpen(String pn, Module other, boolean open) { * Returns {@code true} if this module exports or opens a package to * the given module via its module declaration or CLI options. */ - private boolean isStaticallyExportedOrOpen(String pn, Module other, boolean open) { + private boolean isExplicitlyExportedOrOpened(String pn, Module other, boolean open) { // test if package is open to everyone or Map> openPackages = this.openPackages; if (openPackages != null && allows(openPackages.get(pn), other)) { @@ -764,7 +762,7 @@ private boolean allows(Set targets, Module module) { * Returns {@code true} if this module reflectively exports or opens the * given package to the given module. */ - private boolean isReflectivelyExportedOrOpen(String pn, Module other, boolean open) { + private boolean isReflectivelyExportedOrOpened(String pn, Module other, boolean open) { // exported or open to all modules Map exports = ReflectionData.exports.get(this, EVERYONE_MODULE); if (exports != null) { @@ -809,7 +807,7 @@ private boolean isReflectivelyExportedOrOpen(String pn, Module other, boolean op * given package to the given module. */ boolean isReflectivelyExported(String pn, Module other) { - return isReflectivelyExportedOrOpen(pn, other, false); + return isReflectivelyExportedOrOpened(pn, other, false); } /** @@ -817,7 +815,7 @@ boolean isReflectivelyExported(String pn, Module other) { * given package to the given module. */ boolean isReflectivelyOpened(String pn, Module other) { - return isReflectivelyExportedOrOpen(pn, other, true); + return isReflectivelyExportedOrOpened(pn, other, true); } @@ -1033,50 +1031,38 @@ private void implAddExportsOrOpens(String pn, } } - // add package name to exports if absent - Map map = ReflectionData.exports - .computeIfAbsent(this, other, - (m1, m2) -> new ConcurrentHashMap<>()); - if (open) { - map.put(pn, Boolean.TRUE); // may need to promote from FALSE to TRUE - } else { - map.putIfAbsent(pn, Boolean.FALSE); - } - } - - /** - * Updates a module to open all packages in the given sets to all unnamed - * modules. - * - * @apiNote Used during startup to open packages for illegal access. - */ - void implAddOpensToAllUnnamed(Set concealedPkgs, Set exportedPkgs) { - if (jdk.internal.misc.VM.isModuleSystemInited()) { - throw new IllegalStateException("Module system already initialized"); - } - - // replace this module's openPackages map with a new map that opens - // the packages to all unnamed modules. - Map> openPackages = this.openPackages; - if (openPackages == null) { - openPackages = HashMap.newHashMap(concealedPkgs.size() + exportedPkgs.size()); + if (VM.isBooted()) { + // add package name to ReflectionData.exports if absent + Map map = ReflectionData.exports + .computeIfAbsent(this, other, + (m1, m2) -> new ConcurrentHashMap<>()); + if (open) { + map.put(pn, Boolean.TRUE); // may need to promote from FALSE to TRUE + } else { + map.putIfAbsent(pn, Boolean.FALSE); + } } else { - openPackages = new HashMap<>(openPackages); - } - implAddOpensToAllUnnamed(concealedPkgs, openPackages); - implAddOpensToAllUnnamed(exportedPkgs, openPackages); - this.openPackages = openPackages; - } - - private void implAddOpensToAllUnnamed(Set pkgs, Map> openPackages) { - for (String pn : pkgs) { - Set prev = openPackages.putIfAbsent(pn, ALL_UNNAMED_MODULE_SET); - if (prev != null) { - prev.add(ALL_UNNAMED_MODULE); + // export/open packages during startup (--add-exports and --add-opens) + Map> packageToTargets = (open) ? openPackages : exportedPackages; + if (packageToTargets != null) { + // copy existing map + packageToTargets = new HashMap<>(packageToTargets); + packageToTargets.compute(pn, (_, values) -> { + var targets = new HashSet(); + if (values != null) { + targets.addAll(values); + } + targets.add(other); + return targets; + }); + } else { + packageToTargets = Map.of(pn, Set.of(other)); + } + if (open) { + this.openPackages = packageToTargets; + } else { + this.exportedPackages = packageToTargets; } - - // update VM to export the package - addExportsToAllUnnamed0(this, pn); } } diff --git a/src/java.base/share/classes/java/lang/System.java b/src/java.base/share/classes/java/lang/System.java index 903f6e34e2ee1..f9b4698854dc6 100644 --- a/src/java.base/share/classes/java/lang/System.java +++ b/src/java.base/share/classes/java/lang/System.java @@ -2059,9 +2059,6 @@ public void addOpens(Module m, String pn, Module other) { public void addOpensToAllUnnamed(Module m, String pn) { m.implAddOpensToAllUnnamed(pn); } - public void addOpensToAllUnnamed(Module m, Set concealedPackages, Set exportedPackages) { - m.implAddOpensToAllUnnamed(concealedPackages, exportedPackages); - } public void addUses(Module m, Class service) { m.implAddUses(service); } diff --git a/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java b/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java index 8edeb731cee42..9e11bd7586fd0 100644 --- a/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java +++ b/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java @@ -230,11 +230,6 @@ public interface JavaLangAccess { */ void addOpensToAllUnnamed(Module m, String pkg); - /** - * Updates module m to open all packages in the given sets. - */ - void addOpensToAllUnnamed(Module m, Set concealedPkgs, Set exportedPkgs); - /** * Updates module m to use a service. */ From 4b4d4f505552baa8824349a1e10ef43af3fa213a Mon Sep 17 00:00:00 2001 From: Calvin Cheung Date: Fri, 25 Apr 2025 04:20:16 +0000 Subject: [PATCH 8/8] remove call to addExtraExportsAndOpens() in ModuleBootstrap.java --- .../share/classes/jdk/internal/module/ModuleBootstrap.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java b/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java index 3601d2059a08c..39c52f2551476 100644 --- a/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java +++ b/src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java @@ -162,7 +162,6 @@ public static ModuleLayer boot() { bootLayer = archivedBootLayer.bootLayer(); BootLoader.getUnnamedModule(); // trigger of BootLoader. CDS.defineArchivedModules(ClassLoaders.platformClassLoader(), ClassLoaders.appClassLoader()); - addExtraExportsAndOpens(bootLayer); // assume boot layer has at least one module providing a service // that is mapped to the application class loader.