From ed67de0ec9b0ac578537cc0038dcadffa3e73985 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonathan=20Lamp=C3=A9rth?= Date: Thu, 28 Nov 2024 11:01:11 +0000 Subject: [PATCH] feat: don't display LNT and LVT without -c/-v with warning --- .../com/sun/tools/javap/ClassWriter.java | 6 +- .../com/sun/tools/javap/CodeWriter.java | 7 +- .../com/sun/tools/javap/JavapTask.java | 4 + .../tools/javap/resources/javap.properties | 2 +- .../ClassWriterNoLineVariableTableTest.java | 101 ++++++++++++++++++ .../javap/ClassWriterTableIndentTest.java | 2 +- test/langtools/tools/javap/T4459541.java | 2 +- test/langtools/tools/javap/T8032814.java | 2 +- 8 files changed, 113 insertions(+), 13 deletions(-) create mode 100644 test/langtools/tools/javap/ClassWriterNoLineVariableTableTest.java diff --git a/src/jdk.jdeps/share/classes/com/sun/tools/javap/ClassWriter.java b/src/jdk.jdeps/share/classes/com/sun/tools/javap/ClassWriter.java index 2dbc411dd6ee0..3ac3d35b787c9 100644 --- a/src/jdk.jdeps/share/classes/com/sun/tools/javap/ClassWriter.java +++ b/src/jdk.jdeps/share/classes/com/sun/tools/javap/ClassWriter.java @@ -574,10 +574,8 @@ protected void writeMethod(MethodModel m) { if (options.showAllAttrs) { attrWriter.write(m.attributes()); - } else if (code != null) { - if (options.showDisassembled || options.showLineAndLocalVariableTables) { - codeWriter.writeMinimal(code); - } + } else if (code != null && options.showDisassembled) { + codeWriter.writeMinimal(code); } indent(-1); diff --git a/src/jdk.jdeps/share/classes/com/sun/tools/javap/CodeWriter.java b/src/jdk.jdeps/share/classes/com/sun/tools/javap/CodeWriter.java index 99d7e2c2686e6..9c884fff7ee2c 100644 --- a/src/jdk.jdeps/share/classes/com/sun/tools/javap/CodeWriter.java +++ b/src/jdk.jdeps/share/classes/com/sun/tools/javap/CodeWriter.java @@ -266,11 +266,8 @@ private void writeInternal(CodeAttribute attr, boolean minimal) { } private void writeMinimalMode(CodeAttribute attr) { - if (options.showDisassembled) { - writeInstrs(attr); - writeExceptionTable(attr); - } - + writeInstrs(attr); + writeExceptionTable(attr); if (options.showLineAndLocalVariableTables) { writeLineAndLocalVariableTables(attr); } diff --git a/src/jdk.jdeps/share/classes/com/sun/tools/javap/JavapTask.java b/src/jdk.jdeps/share/classes/com/sun/tools/javap/JavapTask.java index b797ef73522c2..138a76b6f2ecf 100644 --- a/src/jdk.jdeps/share/classes/com/sun/tools/javap/JavapTask.java +++ b/src/jdk.jdeps/share/classes/com/sun/tools/javap/JavapTask.java @@ -549,6 +549,10 @@ else if (allowClasses) { throw new BadArgs("err.incompatible.options", sb); } + if (!options.showDisassembled && !options.verbose && options.showLineAndLocalVariableTables) { + reportWarning("err.incompatible.options", "-l without -c, line number and local variable tables will not be printed"); + } + if ((classes == null || classes.size() == 0) && !(noArgs || options.help || options.version || options.fullVersion)) { throw new BadArgs("err.no.classes.specified"); diff --git a/src/jdk.jdeps/share/classes/com/sun/tools/javap/resources/javap.properties b/src/jdk.jdeps/share/classes/com/sun/tools/javap/resources/javap.properties index 5a50662afa0b9..02e277e2235dd 100644 --- a/src/jdk.jdeps/share/classes/com/sun/tools/javap/resources/javap.properties +++ b/src/jdk.jdeps/share/classes/com/sun/tools/javap/resources/javap.properties @@ -74,7 +74,7 @@ main.opt.v=\ \ -v -verbose Print additional information main.opt.l=\ -\ -l Print line number and local variable tables +\ -l Print line number and local variable tables, works in combination with -c main.opt.public=\ \ -public Show only public classes and members diff --git a/test/langtools/tools/javap/ClassWriterNoLineVariableTableTest.java b/test/langtools/tools/javap/ClassWriterNoLineVariableTableTest.java new file mode 100644 index 0000000000000..102c3c1bde9eb --- /dev/null +++ b/test/langtools/tools/javap/ClassWriterNoLineVariableTableTest.java @@ -0,0 +1,101 @@ +/* + * Copyright (c) 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 + * 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 8345145 + * @summary javap should not print LineNumberTable/LocalVariableTable (-l) without disassembled code (-c). + * @compile -g ClassWriterNoLineVariableTableTest.java + * @run junit ClassWriterNoLineVariableTableTest + * @modules jdk.jdeps/com.sun.tools.javap + */ + +import java.io.PrintWriter; +import java.io.StringWriter; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.junit.jupiter.api.Assertions.*; + +public class ClassWriterNoLineVariableTableTest { + String expectedErrorOutput = "Warning: bad combination of options: -l without -c, line number and local variable tables will not be printed"; + + @Test + public void testJavapWithoutCodeAttribute() { + String output = javap("-l"); + assertContains(output, expectedErrorOutput, + "javap should throw warning, when -l used without -c or -v"); + assertNotContains(output, "LineNumberTable", + "There should be no LineNumberTable output when javap is provided l without -c or -v"); + assertNotContains(output, "LocalVariableTable", + "There should be no LineNumberTable output when javap is provided l without -c or -v"); + } + + @ParameterizedTest(name = "Test javap with fixed option -l and varying option: {0}") + @ValueSource(strings = {"-v", "-c"}) + public void testJavapWithCodeAttribute(String addedOption) { + String output = javap("-l", addedOption); + assertNotContains(output, expectedErrorOutput, + "There should be no warning when javap is provided -l and " + addedOption); + assertContains(output, "LineNumberTable", + "There should be LineNumberTable output when javap is provided -l and " + addedOption); + assertContains(output, "LocalVariableTable", + "There should be LocalVariableTable output when javap is provided -l and " + addedOption); + } + + private static void assertContains(String actual, String expectedSubstring, String message) { + assertTrue(actual.contains(expectedSubstring), + message + " - Expected '" + actual + "' to contain '" + expectedSubstring + "'"); + } + + private static void assertNotContains(String actual, String expectedSubstring, String message) { + assertFalse(actual.contains(expectedSubstring), + message + " - Expected '" + actual + "' not to contain '" + expectedSubstring + "'"); + } + + private String javap(String... args) { + StringWriter sw = new StringWriter(); + PrintWriter out = new PrintWriter(sw); + + String[] fullArgs = new String[args.length + 1]; + System.arraycopy(args, 0, fullArgs, 0, args.length); + fullArgs[args.length] = System.getProperty("test.classes") + "/RandomLoop8345145.class"; + + int rc = com.sun.tools.javap.Main.run(fullArgs, out); + if (rc != 0) + throw new Error("javap failed. rc=" + rc); + out.close(); + System.out.println(sw); + return sw.toString(); + } +} + +class RandomLoop8345145 { + public void randomLoop() { + int x = 5; + for (int i = 0; i < 10; i++) { + x*=2; + } + } +} \ No newline at end of file diff --git a/test/langtools/tools/javap/ClassWriterTableIndentTest.java b/test/langtools/tools/javap/ClassWriterTableIndentTest.java index f4f4c23c82611..ccdfd9d69e72f 100644 --- a/test/langtools/tools/javap/ClassWriterTableIndentTest.java +++ b/test/langtools/tools/javap/ClassWriterTableIndentTest.java @@ -52,7 +52,7 @@ public void run() { * line 145: 14 * ... */ - List runArgsList = List.of(new String[]{"-c", "-l"}, new String[]{"-v"}, new String[]{"-l"}); + List runArgsList = List.of(new String[]{"-c", "-l"}, new String[]{"-v"}); for (String[] runArgs : runArgsList) { String output = javap(runArgs); int methodIntent = findNthMatchPrecedingSpaces(output, "public void emptyLoop();", 0); diff --git a/test/langtools/tools/javap/T4459541.java b/test/langtools/tools/javap/T4459541.java index 10887ad709fc5..7a05e9fc64c8b 100644 --- a/test/langtools/tools/javap/T4459541.java +++ b/test/langtools/tools/javap/T4459541.java @@ -90,7 +90,7 @@ File compileTestFile(File f) { String javap(File f) { StringWriter sw = new StringWriter(); PrintWriter out = new PrintWriter(sw); - int rc = com.sun.tools.javap.Main.run(new String[] { "-l", f.getPath() }, out); + int rc = com.sun.tools.javap.Main.run(new String[] { "-l", "-c", f.getPath() }, out); if (rc != 0) throw new Error("javap failed. rc=" + rc); out.close(); diff --git a/test/langtools/tools/javap/T8032814.java b/test/langtools/tools/javap/T8032814.java index 2c83fd3b71ec4..9472ff235dc6c 100644 --- a/test/langtools/tools/javap/T8032814.java +++ b/test/langtools/tools/javap/T8032814.java @@ -45,7 +45,7 @@ void run() throws Exception { + clazz.getDeclaredMethods().length; test(clazz, 0); test(clazz, count, "-v"); - test(clazz, count, "-l"); + test(clazz, count, "-c", "-l"); test(clazz, count, "-v", "-l"); if (errors > 0)