From 5594617b8f59e86767f773e00c0e191d0869ac13 Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Mon, 29 Sep 2025 10:38:56 -0700 Subject: [PATCH 1/3] 8347007: --strip-debug removes parameter names included with -parameters --- .../jdk/tools/jlink/internal/TaskHelper.java | 13 + .../plugins/DefaultStripDebugPlugin.java | 15 +- .../StripJavaDebugAttributesPlugin.java | 53 +++- .../tools/jlink/resources/plugins.properties | 12 +- .../plugins/DefaultStripDebugPluginTest.java | 34 +++ .../plugins/StripParameterNamesTest.java | 257 ++++++++++++++++++ 6 files changed, 373 insertions(+), 11 deletions(-) create mode 100644 test/jdk/tools/jlink/plugins/StripParameterNamesTest.java diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java index 689c8b24743e9..3d930fd834fc6 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java @@ -51,6 +51,7 @@ import jdk.tools.jlink.internal.plugins.DefaultStripDebugPlugin; import jdk.tools.jlink.internal.plugins.ExcludeJmodSectionPlugin; import jdk.tools.jlink.internal.plugins.PluginsResourceBundle; +import jdk.tools.jlink.internal.plugins.StripJavaDebugAttributesPlugin; import jdk.tools.jlink.plugin.Plugin; import jdk.tools.jlink.plugin.Plugin.Category; @@ -418,6 +419,9 @@ private PluginsConfiguration getPluginsConfig(Path output, Map l List pluginsList = new ArrayList<>(); Set seenPlugins = new HashSet<>(); + // reference to the enabled DefaultStripDebugPlugin + DefaultStripDebugPlugin defaultStripDebugPlugin = null; + for (Entry>> entry : pluginToMaps.entrySet()) { Plugin plugin = entry.getKey(); List> argsMaps = entry.getValue(); @@ -438,6 +442,10 @@ private PluginsConfiguration getPluginsConfig(Path output, Map l } if (!Utils.isDisabled(plugin)) { + if (plugin instanceof DefaultStripDebugPlugin) { + defaultStripDebugPlugin = (DefaultStripDebugPlugin) plugin; + } + // make sure that --strip-debug and --strip-native-debug-symbols // aren't being used at the same time. --strip-debug invokes --strip-native-debug-symbols on // platforms that support it, so it makes little sense to allow both at the same time. @@ -452,6 +460,11 @@ private PluginsConfiguration getPluginsConfig(Path output, Map l } } + // disable StripJavaDebugAttributesPlugin within DefaultStripDebug plugin if both enabled + if (seenPlugins.contains(StripJavaDebugAttributesPlugin.NAME) && defaultStripDebugPlugin != null) { + defaultStripDebugPlugin.enableJavaStripPlugin(false); + } + // recreate or postprocessing don't require an output directory. ImageBuilder builder = null; if (output != null) { diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/DefaultStripDebugPlugin.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/DefaultStripDebugPlugin.java index e497083cc949a..b3644bdde857f 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/DefaultStripDebugPlugin.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/DefaultStripDebugPlugin.java @@ -47,6 +47,8 @@ public final class DefaultStripDebugPlugin extends AbstractPlugin { private final Plugin javaStripPlugin; private final NativePluginFactory stripNativePluginFactory; + private boolean isJavaStripPluginEnabled = true; + public DefaultStripDebugPlugin() { this(new StripJavaDebugAttributesPlugin(), new DefaultNativePluginFactory()); @@ -59,6 +61,10 @@ public DefaultStripDebugPlugin(Plugin javaStripPlugin, this.stripNativePluginFactory = nativeStripPluginFact; } + public void enableJavaStripPlugin(boolean enableJavaStripPlugin) { + isJavaStripPluginEnabled = enableJavaStripPlugin; + } + @Override public ResourcePool transform(ResourcePool in, ResourcePoolBuilder out) { Plugin stripNativePlugin = stripNativePluginFactory.create(); @@ -66,14 +72,21 @@ public ResourcePool transform(ResourcePool in, ResourcePoolBuilder out) { Map stripNativeConfig = Map.of( STRIP_NATIVE_DEBUG_PLUGIN, EXCLUDE_DEBUGINFO); stripNativePlugin.configure(stripNativeConfig); + + if (!isJavaStripPluginEnabled) { + return stripNativePlugin.transform(in, out); + } + ResourcePoolManager outRes = new ResourcePoolManager(in.byteOrder(), ((ResourcePoolImpl)in).getStringTable()); ResourcePool strippedJava = javaStripPlugin.transform(in, outRes.resourcePoolBuilder()); return stripNativePlugin.transform(strippedJava, out); - } else { + } else if (isJavaStripPluginEnabled) { return javaStripPlugin.transform(in, out); + } else { + return in; } } diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/StripJavaDebugAttributesPlugin.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/StripJavaDebugAttributesPlugin.java index fff585bb2dd31..a1b47e4f874da 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/StripJavaDebugAttributesPlugin.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/StripJavaDebugAttributesPlugin.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 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 @@ -25,6 +25,7 @@ package jdk.tools.jlink.internal.plugins; import java.util.function.Predicate; +import java.lang.IllegalArgumentException; import java.lang.classfile.ClassFile; import java.lang.classfile.ClassTransform; import java.lang.classfile.CodeTransform; @@ -32,6 +33,7 @@ import java.lang.classfile.attribute.MethodParametersAttribute; import java.lang.classfile.attribute.SourceFileAttribute; import java.lang.classfile.attribute.SourceDebugExtensionAttribute; +import java.util.Map; import jdk.tools.jlink.plugin.ResourcePool; import jdk.tools.jlink.plugin.ResourcePoolBuilder; @@ -40,17 +42,50 @@ /** * * Strip java debug attributes plugin + * + * Usage: --strip-java-debug-attributes(=+parameter_names) */ public final class StripJavaDebugAttributesPlugin extends AbstractPlugin { + public static final String NAME = "strip-java-debug-attributes"; + public static final String DROP_METHOD_PARAMETER_NAMES = "+parameter-names"; + private final Predicate predicate; + private boolean isDroppingMethodNames; public StripJavaDebugAttributesPlugin() { this((path) -> false); } StripJavaDebugAttributesPlugin(Predicate predicate) { - super("strip-java-debug-attributes"); + super(NAME); this.predicate = predicate; + isDroppingMethodNames = false; + } + + @Override + public boolean hasArguments() { + return true; + } + + @Override + public boolean hasRawArgument() { + return true; + } + + @Override + public void configure(Map config) { + var rawArg = config.get(NAME); + if (rawArg != null) { + if (rawArg.isEmpty()) { + return; + } else if (rawArg.equals(DROP_METHOD_PARAMETER_NAMES)) { + isDroppingMethodNames = true; + } else { + // We only support one value for now, other values is illegal. + throw new IllegalArgumentException( + PluginsResourceBundle.getMessage("err.illegal.argument", rawArg)); + } + } } @Override @@ -67,13 +102,19 @@ public ResourcePool transform(ResourcePool in, ResourcePoolBuilder out) { var clm = newClassReader(path, resource, ClassFile.DebugElementsOption.DROP_DEBUG, ClassFile.LineNumbersOption.DROP_LINE_NUMBERS); + + MethodTransform mt; + if (isDroppingMethodNames) { + mt = MethodTransform.dropping(me -> me instanceof MethodParametersAttribute) + .andThen(MethodTransform.transformingCode(CodeTransform.ACCEPT_ALL)); + } else { + mt = MethodTransform.transformingCode(CodeTransform.ACCEPT_ALL); + } + byte[] content = ClassFile.of().transformClass(clm, ClassTransform .dropping(cle -> cle instanceof SourceFileAttribute || cle instanceof SourceDebugExtensionAttribute) - .andThen(ClassTransform.transformingMethods(MethodTransform - .dropping(me -> me instanceof MethodParametersAttribute) - .andThen(MethodTransform - .transformingCode(CodeTransform.ACCEPT_ALL))))); + .andThen(ClassTransform.transformingMethods(mt))); res = resource.copyWithContent(content); } } diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties b/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties index a4b780a15c340..9262e79213ce8 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties @@ -1,5 +1,5 @@ # -# Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2015, 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 @@ -192,13 +192,17 @@ Strip debug information from the output image strip-debug.usage=\ \ --strip-debug Strip debug information from the output image +strip-java-debug-attributes.argument=[+parameter-names] + strip-java-debug-attributes.description=\ Strip Java debug attributes from classes in the output image strip-java-debug-attributes.usage=\ -\ --strip-java-debug-attributes \n\ +\ --strip-java-debug-attributes [+parameter-names]\n\ \ Strip Java debug attributes from\n\ -\ classes in the output image +\ classes in the output image. +\ Use +parameter-names to strip method parameters +\ name. strip-native-commands.description=\ Exclude native commands (such as java/java.exe) from the image @@ -367,7 +371,7 @@ err.provider.not.functional=The provider {0} is not functional. err.plugin.multiple.options=More than one plugin enabled by {0} option err.plugin.conflicts={0} ({1}) conflicts with {2}. Please use one or the other, but not both. err.provider.additional.arg.error=Error in additional argument specification in {0} option: {1} - +err.illegal.argument=Illegal argument: \"{0}\" err.no.plugins.path=No plugins path argument. err.dir.already.exits=directory already exists: {0} diff --git a/test/jdk/tools/jlink/plugins/DefaultStripDebugPluginTest.java b/test/jdk/tools/jlink/plugins/DefaultStripDebugPluginTest.java index 10d534348fab8..4b131484d9d77 100644 --- a/test/jdk/tools/jlink/plugins/DefaultStripDebugPluginTest.java +++ b/test/jdk/tools/jlink/plugins/DefaultStripDebugPluginTest.java @@ -73,6 +73,40 @@ public void testNoNativeStripPluginPresent() { } } + public void testOnlyNativePlugin() { + MockStripPlugin javaPlugin = new MockStripPlugin(false); + MockStripPlugin nativePlugin = new MockStripPlugin(true); + TestNativeStripPluginFactory nativeFactory = + new TestNativeStripPluginFactory(nativePlugin); + DefaultStripDebugPlugin plugin = new DefaultStripDebugPlugin(javaPlugin, + nativeFactory); + plugin.enableJavaStripPlugin(false); + + ResourcePoolManager inManager = new ResourcePoolManager(); + ResourcePool pool = plugin.transform(inManager.resourcePool(), + inManager.resourcePoolBuilder()); + if (pool.findEntry(MockStripPlugin.JAVA_PATH).isPresent() || + !pool.findEntry(MockStripPlugin.NATIVE_PATH).isPresent()) { + throw new AssertionError("Expected only native to get called"); + } + } + + public void testNoOperation() { + MockStripPlugin javaPlugin = new MockStripPlugin(false); + TestNativeStripPluginFactory nativeFactory = + new TestNativeStripPluginFactory(null); + DefaultStripDebugPlugin plugin = new DefaultStripDebugPlugin(javaPlugin, + nativeFactory); + plugin.enableJavaStripPlugin(false); + ResourcePoolManager inManager = new ResourcePoolManager(); + ResourcePool pool = plugin.transform(inManager.resourcePool(), + inManager.resourcePoolBuilder()); + if (pool.findEntry(MockStripPlugin.JAVA_PATH).isPresent() || + pool.findEntry(MockStripPlugin.NATIVE_PATH).isPresent()) { + throw new AssertionError("Expected both native and java not called"); + } + } + public static void main(String[] args) { DefaultStripDebugPluginTest test = new DefaultStripDebugPluginTest(); test.testNoNativeStripPluginPresent(); diff --git a/test/jdk/tools/jlink/plugins/StripParameterNamesTest.java b/test/jdk/tools/jlink/plugins/StripParameterNamesTest.java new file mode 100644 index 0000000000000..5e17803dc3ad9 --- /dev/null +++ b/test/jdk/tools/jlink/plugins/StripParameterNamesTest.java @@ -0,0 +1,257 @@ +/* + * 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. + */ + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.FieldSource; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.io.IOException; +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.List; +import java.util.stream.Stream; +import java.util.spi.ToolProvider; + +import tests.JImageGenerator; +import tests.JImageGenerator.InMemorySourceFile; +import tests.Result; + +/* + * @test + * @summary Test jlink strip debug plugins handle method parameter names. + * @bug 8347007 + * @library ../../lib + * @modules java.base/jdk.internal.jimage + * jdk.jlink/jdk.tools.jlink.internal + * jdk.jlink/jdk.tools.jlink.plugin + * jdk.jlink/jdk.tools.jmod + * jdk.jlink/jdk.tools.jimage + * jdk.compiler + * @build tests.* + * @run junit/othervm StripParameterNamesTest + */ +public class StripParameterNamesTest { + private static final ToolProvider JAVAC_TOOL = ToolProvider.findFirst("javac") + .orElseThrow(() -> new RuntimeException("javac tool not found")); + + private static Path src = Paths.get("src").toAbsolutePath(); + private static List testJmods = new ArrayList<>(); + + record Jmod(Path moduleDir, boolean withDebugInfo, boolean withParameterNames) {} + + @BeforeAll + public static void setup() throws IOException { + Files.createDirectory(src); + var mainClassSource = new InMemorySourceFile("test", "InspectParameterNames", """ + package test; + + public class InspectParameterNames { + int add(int a, int b) { + return a + b; + } + + public static boolean hasParameterNames() throws NoSuchMethodException { + // Get add method in the class + var method = InspectParameterNames.class.getDeclaredMethod("add", int.class, int.class); + + // Get method parameters + var parameters = method.getParameters(); + + // validate parameter names + return parameters[0].getName().equals("a") && parameters[1].getName().equals("b"); + } + + public static void main(String[] args) throws NoSuchMethodException { + System.out.println(hasParameterNames()); + } + } + """); + var moduleDir = JImageGenerator.generateSources(src, "bug8347007x", List.of(mainClassSource)); + JImageGenerator.generateModuleInfo(moduleDir, List.of("test")); + testJmods.add(buildJmod(true, true)); + testJmods.add(buildJmod(true, false)); + testJmods.add(buildJmod(false, true)); + testJmods.add(buildJmod(false, false)); + } + + @AfterEach + public void cleanup() throws IOException { + rmdir(Paths.get("img")); + } + + static void report(String command, List args) { + System.out.println(command + " " + String.join(" ", args)); + } + + static void javac(List args) { + report("javac", args); + JAVAC_TOOL.run(System.out, System.err, args.toArray(new String[0])); + } + /** + * Recursively remove a Directory + * + * @param dir Directory to delete + * @throws IOException If an error occurs + */ + static void rmdir(Path dir) throws IOException { + // Nothing to do if the file does not exist + if (!Files.exists(dir)) { + return; + } + try (Stream walk = Files.walk(dir)) { + walk.sorted(Comparator.reverseOrder()) + .map(Path::toFile) + .peek(System.out::println) + .forEach(File::delete); + } + } + + /** + * Build jmods from the module source path + */ + static Jmod buildJmod(boolean withDebugInfo, boolean withParameterNames) { + String dirName = "jmods"; + List options = new ArrayList<>(); + + if (withDebugInfo) { + options.add("-g"); + dirName += "g"; + } + + if (withParameterNames) { + options.add("-parameters"); + dirName += "p"; + } + + Path moduleDir = Paths.get(dirName).toAbsolutePath(); + + options.add("-d"); + options.add(moduleDir.toString()); + options.add("--module-source-path"); + options.add(src.toString()); + options.add("--module"); + options.add("bug8347007x"); + + javac(options); + return new Jmod(moduleDir, withDebugInfo, withParameterNames); + } + + Result buildImage(Path modulePath, Path imageDir, String... options) { + var jlinkTask = JImageGenerator.getJLinkTask() + .modulePath(modulePath.toString()) + .output(imageDir); + + for (var option: options) { + jlinkTask.option(option); + } + + return jlinkTask.addMods("bug8347007x") + .call(); + } + + void assertHasParameterNames(Path imageDir, boolean expected) throws IOException, InterruptedException { + Path binDir = imageDir.resolve("bin").toAbsolutePath(); + Path bin = binDir.resolve("java"); + + ProcessBuilder processBuilder = new ProcessBuilder(bin.toString(), + "-XX:+UnlockDiagnosticVMOptions", + "-XX:+BytecodeVerificationLocal", + "-m", "bug8347007x/test.InspectParameterNames"); + processBuilder.directory(binDir.toFile()); + Process process = processBuilder.start(); + int exitCode = process.waitFor(); + var output = process.inputReader().readLine(); + System.out.println(output); + assertEquals(expected, Boolean.parseBoolean(output)); + } + + Stream provideTestJmods() { + return testJmods.stream(); + } + + @ParameterizedTest + @FieldSource("testJmods") + public void testDefaultBehavior(Jmod jmod) throws Exception { + var imageDir = Paths.get("img"); + buildImage(jmod.moduleDir(), imageDir) + .assertSuccess(); + var hasParameter = jmod.withParameterNames(); + assertHasParameterNames(imageDir, hasParameter); + } + + @ParameterizedTest + @FieldSource("testJmods") + public void testStripDebug(Jmod jmod) throws Exception { + var imageDir = Paths.get("img"); + buildImage(jmod.moduleDir(), imageDir, + "--strip-debug") + .assertSuccess(); + var hasParameter = jmod.withParameterNames(); + assertHasParameterNames(imageDir, hasParameter); + } + + @ParameterizedTest + @FieldSource("testJmods") + public void testStripParameterNames(Jmod jmod) throws Exception { + var imageDir = Paths.get("img"); + buildImage(jmod.moduleDir(), imageDir, + "--strip-debug", "--strip-java-debug-attributes=+parameter-names") + .assertSuccess(); + assertHasParameterNames(imageDir, false); + } + + @Test + public void testWithoutNative() throws Exception { + var imageDir = Paths.get("img"); + var jmod = testJmods.get(0); + buildImage(jmod.moduleDir(), imageDir, + "--strip-java-debug-attributes=+parameter-names") + .assertSuccess(); + assertHasParameterNames(imageDir, false); + } + + @Test + public void testAlternativeFormOfArgument() throws Exception { + var imageDir = Paths.get("img"); + var jmod = testJmods.get(0); + buildImage(jmod.moduleDir(), imageDir, + "--strip-debug", "--strip-java-debug-attributes", "+parameter-names") + .assertSuccess(); + assertHasParameterNames(imageDir, false); + } + @Test + public void testInvalidArgument() throws Exception { + var imageDir = Paths.get("img"); + var jmod = testJmods.get(0); + buildImage(jmod.moduleDir(), imageDir, + "--strip-debug", "--strip-java-debug-attributes=+parameter_names") + .assertFailure("Illegal argument: \"+parameter_names\""); + } +} \ No newline at end of file From 8b5a6c12a5a6735c02fa720c32dcd678ae69f3ca Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Mon, 29 Sep 2025 19:46:35 -0700 Subject: [PATCH 2/3] Support optional argument for PluginOptions --- .../jdk/tools/jlink/internal/TaskHelper.java | 27 ++++++++++++++----- .../StripJavaDebugAttributesPlugin.java | 5 ++++ .../jdk/tools/jlink/plugin/Plugin.java | 4 +++ .../plugins/StripParameterNamesTest.java | 12 ++++++++- 4 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java index 3d930fd834fc6..4fd4c13d390b6 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java @@ -91,6 +91,7 @@ public interface Processing { } final boolean hasArg; + final boolean isArgumentOptional; final Processing processing; final boolean hidden; final String name; @@ -99,6 +100,7 @@ public interface Processing { final boolean terminalOption; public Option(boolean hasArg, + boolean isArgumentOptional, Processing processing, boolean hidden, String name, @@ -114,6 +116,7 @@ public Option(boolean hasArg, } this.hasArg = hasArg; + this.isArgumentOptional = isArgumentOptional; this.processing = processing; this.hidden = hidden; this.name = name; @@ -128,29 +131,33 @@ public Option(boolean hasArg, String shortname, boolean isTerminal) { - this(hasArg, processing, hidden, name, shortname, "", isTerminal); + this(hasArg, false, processing, hidden, name, shortname, "", isTerminal); } public Option(boolean hasArg, Processing processing, String name, String shortname, boolean isTerminal) { - this(hasArg, processing, false, name, shortname, "", isTerminal); + this(hasArg, false, processing, false, name, shortname, "", isTerminal); } public Option(boolean hasArg, Processing processing, String name, String shortname, String shortname2) { - this(hasArg, processing, false, name, shortname, shortname2, false); + this(hasArg, false, processing, false, name, shortname, shortname2, false); } public Option(boolean hasArg, Processing processing, String name, String shortname) { - this(hasArg, processing, false, name, shortname, "", false); + this(hasArg, false, processing, false, name, shortname, "", false); } public Option(boolean hasArg, Processing processing, boolean hidden, String name) { - this(hasArg, processing, hidden, name, "", "", false); + this(hasArg, false, processing, hidden, name, "", "", false); } public Option(boolean hasArg, Processing processing, String name) { this(hasArg, processing, false, name, "", false); } + public boolean isArgumentOptional() { + return isArgumentOptional; + } + public boolean isHidden() { return hidden; } @@ -209,6 +216,12 @@ public PluginOption(boolean hasArg, Processing processing, super(hasArg, processing, hidden, name, shortname, false); } + public PluginOption(boolean hasArg, boolean isArgumentOptional, + Processing processing, + boolean hidden, String name) { + super(hasArg, isArgumentOptional, processing, hidden, name, "", "", false); + } + public PluginOption(boolean hasArg, Processing processing, boolean hidden, String name) { super(hasArg, processing, hidden, name, "", false); @@ -302,7 +315,7 @@ private void addOrderedPluginOptions(Plugin plugin, optionsSeen.add(option); PluginOption plugOption - = new PluginOption(plugin.hasArguments(), + = new PluginOption(plugin.hasArguments(), plugin.isArgumentOptional(), (task, opt, arg) -> { if (!Utils.isFunctional(plugin)) { throw newBadArgs("err.provider.not.functional", @@ -557,7 +570,7 @@ public List handleOptions(T task, String[] args) throws BadArgs { potentiallyGnuOption = true; param = args[++i]; } - if (param == null || param.isEmpty()) { + if (!opt.isArgumentOptional() && (param == null || param.isEmpty())) { throw new BadArgs("err.missing.arg", name).showUsage(true); } if (potentiallyGnuOption && param.length() >= 2 && diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/StripJavaDebugAttributesPlugin.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/StripJavaDebugAttributesPlugin.java index a1b47e4f874da..efc431d85434f 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/StripJavaDebugAttributesPlugin.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/StripJavaDebugAttributesPlugin.java @@ -72,6 +72,11 @@ public boolean hasRawArgument() { return true; } + @Override + public boolean isArgumentOptional() { + return true; + } + @Override public void configure(Map config) { var rawArg = config.get(NAME); diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/plugin/Plugin.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/plugin/Plugin.java index 11a5d63005143..b153c54c0d8c6 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/plugin/Plugin.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/plugin/Plugin.java @@ -166,6 +166,10 @@ public default boolean hasRawArgument() { return false; } + public default boolean isArgumentOptional() { + return false; + } + /** * The plugin argument(s) description. * @return The argument(s) description. diff --git a/test/jdk/tools/jlink/plugins/StripParameterNamesTest.java b/test/jdk/tools/jlink/plugins/StripParameterNamesTest.java index 5e17803dc3ad9..fdd6e98ecbbad 100644 --- a/test/jdk/tools/jlink/plugins/StripParameterNamesTest.java +++ b/test/jdk/tools/jlink/plugins/StripParameterNamesTest.java @@ -128,7 +128,6 @@ static void rmdir(Path dir) throws IOException { try (Stream walk = Files.walk(dir)) { walk.sorted(Comparator.reverseOrder()) .map(Path::toFile) - .peek(System.out::println) .forEach(File::delete); } } @@ -246,6 +245,17 @@ public void testAlternativeFormOfArgument() throws Exception { .assertSuccess(); assertHasParameterNames(imageDir, false); } + + @Test + public void testWithoutStripParameterName() throws Exception { + var imageDir = Paths.get("img"); + var jmod = testJmods.get(0); + buildImage(jmod.moduleDir(), imageDir, + "--strip-debug", "--strip-java-debug-attributes") + .assertSuccess(); + assertHasParameterNames(imageDir, jmod.withParameterNames()); + } + @Test public void testInvalidArgument() throws Exception { var imageDir = Paths.get("img"); From 766dc01e7e02267c929f96441b65ca4e888c8e1f Mon Sep 17 00:00:00 2001 From: Henry Jen Date: Wed, 1 Oct 2025 22:34:30 -0700 Subject: [PATCH 3/3] MethodParameters attribute should not be removed --- .../jdk/tools/jlink/internal/TaskHelper.java | 31 +++------- .../StripJavaDebugAttributesPlugin.java | 57 ++----------------- .../jdk/tools/jlink/plugin/Plugin.java | 4 -- .../tools/jlink/resources/plugins.properties | 12 ++-- .../plugins/StripParameterNamesTest.java | 39 ------------- 5 files changed, 18 insertions(+), 125 deletions(-) diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java index 4fd4c13d390b6..d53589dc388e3 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java @@ -91,7 +91,6 @@ public interface Processing { } final boolean hasArg; - final boolean isArgumentOptional; final Processing processing; final boolean hidden; final String name; @@ -100,7 +99,6 @@ public interface Processing { final boolean terminalOption; public Option(boolean hasArg, - boolean isArgumentOptional, Processing processing, boolean hidden, String name, @@ -116,7 +114,6 @@ public Option(boolean hasArg, } this.hasArg = hasArg; - this.isArgumentOptional = isArgumentOptional; this.processing = processing; this.hidden = hidden; this.name = name; @@ -131,33 +128,29 @@ public Option(boolean hasArg, String shortname, boolean isTerminal) { - this(hasArg, false, processing, hidden, name, shortname, "", isTerminal); + this(hasArg, processing, hidden, name, shortname, "", isTerminal); } public Option(boolean hasArg, Processing processing, String name, String shortname, boolean isTerminal) { - this(hasArg, false, processing, false, name, shortname, "", isTerminal); + this(hasArg, processing, false, name, shortname, "", isTerminal); } public Option(boolean hasArg, Processing processing, String name, String shortname, String shortname2) { - this(hasArg, false, processing, false, name, shortname, shortname2, false); + this(hasArg, processing, false, name, shortname, shortname2, false); } public Option(boolean hasArg, Processing processing, String name, String shortname) { - this(hasArg, false, processing, false, name, shortname, "", false); + this(hasArg, processing, false, name, shortname, "", false); } public Option(boolean hasArg, Processing processing, boolean hidden, String name) { - this(hasArg, false, processing, hidden, name, "", "", false); + this(hasArg, processing, hidden, name, "", "", false); } public Option(boolean hasArg, Processing processing, String name) { this(hasArg, processing, false, name, "", false); } - public boolean isArgumentOptional() { - return isArgumentOptional; - } - public boolean isHidden() { return hidden; } @@ -216,12 +209,6 @@ public PluginOption(boolean hasArg, Processing processing, super(hasArg, processing, hidden, name, shortname, false); } - public PluginOption(boolean hasArg, boolean isArgumentOptional, - Processing processing, - boolean hidden, String name) { - super(hasArg, isArgumentOptional, processing, hidden, name, "", "", false); - } - public PluginOption(boolean hasArg, Processing processing, boolean hidden, String name) { super(hasArg, processing, hidden, name, "", false); @@ -315,7 +302,7 @@ private void addOrderedPluginOptions(Plugin plugin, optionsSeen.add(option); PluginOption plugOption - = new PluginOption(plugin.hasArguments(), plugin.isArgumentOptional(), + = new PluginOption(plugin.hasArguments(), (task, opt, arg) -> { if (!Utils.isFunctional(plugin)) { throw newBadArgs("err.provider.not.functional", @@ -455,8 +442,8 @@ private PluginsConfiguration getPluginsConfig(Path output, Map l } if (!Utils.isDisabled(plugin)) { - if (plugin instanceof DefaultStripDebugPlugin) { - defaultStripDebugPlugin = (DefaultStripDebugPlugin) plugin; + if (plugin instanceof DefaultStripDebugPlugin p) { + defaultStripDebugPlugin = p; } // make sure that --strip-debug and --strip-native-debug-symbols @@ -570,7 +557,7 @@ public List handleOptions(T task, String[] args) throws BadArgs { potentiallyGnuOption = true; param = args[++i]; } - if (!opt.isArgumentOptional() && (param == null || param.isEmpty())) { + if (param == null || param.isEmpty()) { throw new BadArgs("err.missing.arg", name).showUsage(true); } if (potentiallyGnuOption && param.length() >= 2 && diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/StripJavaDebugAttributesPlugin.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/StripJavaDebugAttributesPlugin.java index efc431d85434f..4a6b54a3b6c83 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/StripJavaDebugAttributesPlugin.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/StripJavaDebugAttributesPlugin.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 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 @@ -25,7 +25,6 @@ package jdk.tools.jlink.internal.plugins; import java.util.function.Predicate; -import java.lang.IllegalArgumentException; import java.lang.classfile.ClassFile; import java.lang.classfile.ClassTransform; import java.lang.classfile.CodeTransform; @@ -33,7 +32,6 @@ import java.lang.classfile.attribute.MethodParametersAttribute; import java.lang.classfile.attribute.SourceFileAttribute; import java.lang.classfile.attribute.SourceDebugExtensionAttribute; -import java.util.Map; import jdk.tools.jlink.plugin.ResourcePool; import jdk.tools.jlink.plugin.ResourcePoolBuilder; @@ -42,15 +40,10 @@ /** * * Strip java debug attributes plugin - * - * Usage: --strip-java-debug-attributes(=+parameter_names) */ public final class StripJavaDebugAttributesPlugin extends AbstractPlugin { - public static final String NAME = "strip-java-debug-attributes"; - public static final String DROP_METHOD_PARAMETER_NAMES = "+parameter-names"; - private final Predicate predicate; - private boolean isDroppingMethodNames; + public static final String NAME = "strip-java-debug-attributes"; public StripJavaDebugAttributesPlugin() { this((path) -> false); @@ -59,38 +52,6 @@ public StripJavaDebugAttributesPlugin() { StripJavaDebugAttributesPlugin(Predicate predicate) { super(NAME); this.predicate = predicate; - isDroppingMethodNames = false; - } - - @Override - public boolean hasArguments() { - return true; - } - - @Override - public boolean hasRawArgument() { - return true; - } - - @Override - public boolean isArgumentOptional() { - return true; - } - - @Override - public void configure(Map config) { - var rawArg = config.get(NAME); - if (rawArg != null) { - if (rawArg.isEmpty()) { - return; - } else if (rawArg.equals(DROP_METHOD_PARAMETER_NAMES)) { - isDroppingMethodNames = true; - } else { - // We only support one value for now, other values is illegal. - throw new IllegalArgumentException( - PluginsResourceBundle.getMessage("err.illegal.argument", rawArg)); - } - } } @Override @@ -107,19 +68,11 @@ public ResourcePool transform(ResourcePool in, ResourcePoolBuilder out) { var clm = newClassReader(path, resource, ClassFile.DebugElementsOption.DROP_DEBUG, ClassFile.LineNumbersOption.DROP_LINE_NUMBERS); - - MethodTransform mt; - if (isDroppingMethodNames) { - mt = MethodTransform.dropping(me -> me instanceof MethodParametersAttribute) - .andThen(MethodTransform.transformingCode(CodeTransform.ACCEPT_ALL)); - } else { - mt = MethodTransform.transformingCode(CodeTransform.ACCEPT_ALL); - } - byte[] content = ClassFile.of().transformClass(clm, ClassTransform .dropping(cle -> cle instanceof SourceFileAttribute - || cle instanceof SourceDebugExtensionAttribute) - .andThen(ClassTransform.transformingMethods(mt))); + || cle instanceof SourceDebugExtensionAttribute) + .andThen(ClassTransform.transformingMethods(MethodTransform + .transformingCode(CodeTransform.ACCEPT_ALL)))); res = resource.copyWithContent(content); } } diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/plugin/Plugin.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/plugin/Plugin.java index b153c54c0d8c6..11a5d63005143 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/plugin/Plugin.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/plugin/Plugin.java @@ -166,10 +166,6 @@ public default boolean hasRawArgument() { return false; } - public default boolean isArgumentOptional() { - return false; - } - /** * The plugin argument(s) description. * @return The argument(s) description. diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties b/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties index 9262e79213ce8..a4b780a15c340 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins.properties @@ -1,5 +1,5 @@ # -# Copyright (c) 2015, 2025, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2015, 2023, 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 @@ -192,17 +192,13 @@ Strip debug information from the output image strip-debug.usage=\ \ --strip-debug Strip debug information from the output image -strip-java-debug-attributes.argument=[+parameter-names] - strip-java-debug-attributes.description=\ Strip Java debug attributes from classes in the output image strip-java-debug-attributes.usage=\ -\ --strip-java-debug-attributes [+parameter-names]\n\ +\ --strip-java-debug-attributes \n\ \ Strip Java debug attributes from\n\ -\ classes in the output image. -\ Use +parameter-names to strip method parameters -\ name. +\ classes in the output image strip-native-commands.description=\ Exclude native commands (such as java/java.exe) from the image @@ -371,7 +367,7 @@ err.provider.not.functional=The provider {0} is not functional. err.plugin.multiple.options=More than one plugin enabled by {0} option err.plugin.conflicts={0} ({1}) conflicts with {2}. Please use one or the other, but not both. err.provider.additional.arg.error=Error in additional argument specification in {0} option: {1} -err.illegal.argument=Illegal argument: \"{0}\" + err.no.plugins.path=No plugins path argument. err.dir.already.exits=directory already exists: {0} diff --git a/test/jdk/tools/jlink/plugins/StripParameterNamesTest.java b/test/jdk/tools/jlink/plugins/StripParameterNamesTest.java index fdd6e98ecbbad..10e58f25997b6 100644 --- a/test/jdk/tools/jlink/plugins/StripParameterNamesTest.java +++ b/test/jdk/tools/jlink/plugins/StripParameterNamesTest.java @@ -216,36 +216,6 @@ public void testStripDebug(Jmod jmod) throws Exception { assertHasParameterNames(imageDir, hasParameter); } - @ParameterizedTest - @FieldSource("testJmods") - public void testStripParameterNames(Jmod jmod) throws Exception { - var imageDir = Paths.get("img"); - buildImage(jmod.moduleDir(), imageDir, - "--strip-debug", "--strip-java-debug-attributes=+parameter-names") - .assertSuccess(); - assertHasParameterNames(imageDir, false); - } - - @Test - public void testWithoutNative() throws Exception { - var imageDir = Paths.get("img"); - var jmod = testJmods.get(0); - buildImage(jmod.moduleDir(), imageDir, - "--strip-java-debug-attributes=+parameter-names") - .assertSuccess(); - assertHasParameterNames(imageDir, false); - } - - @Test - public void testAlternativeFormOfArgument() throws Exception { - var imageDir = Paths.get("img"); - var jmod = testJmods.get(0); - buildImage(jmod.moduleDir(), imageDir, - "--strip-debug", "--strip-java-debug-attributes", "+parameter-names") - .assertSuccess(); - assertHasParameterNames(imageDir, false); - } - @Test public void testWithoutStripParameterName() throws Exception { var imageDir = Paths.get("img"); @@ -255,13 +225,4 @@ public void testWithoutStripParameterName() throws Exception { .assertSuccess(); assertHasParameterNames(imageDir, jmod.withParameterNames()); } - - @Test - public void testInvalidArgument() throws Exception { - var imageDir = Paths.get("img"); - var jmod = testJmods.get(0); - buildImage(jmod.moduleDir(), imageDir, - "--strip-debug", "--strip-java-debug-attributes=+parameter_names") - .assertFailure("Illegal argument: \"+parameter_names\""); - } } \ No newline at end of file