From 14b970dc9e8d0fe1173039c01cced8a9422ec1ae Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Mon, 27 Mar 2023 21:33:01 +0000 Subject: [PATCH] 8296656: java.lang.NoClassDefFoundError exception on running fully legitimate code 8287885: Local classes cause ClassLoader error if the type names are similar but not same Reviewed-by: vromero --- .../com/sun/tools/javac/code/Lint.java | 5 + .../sun/tools/javac/file/BaseFileManager.java | 42 +++++- .../tools/javac/file/JavacFileManager.java | 1 + .../sun/tools/javac/file/PathFileObject.java | 8 +- .../tools/javac/resources/compiler.properties | 4 + .../tools/javac/resources/javac.properties | 4 + src/jdk.compiler/share/man/javac.1 | 3 + .../tools/javac/diags/examples.not-yet.txt | 1 + .../tools/javac/file/OutputFileClashTest.java | 139 ++++++++++++++++++ .../tools/lib/toolbox/JavacTask.java | 27 ++++ 10 files changed, 231 insertions(+), 3 deletions(-) create mode 100644 test/langtools/tools/javac/file/OutputFileClashTest.java diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java index 137c46c6439..0ed37e98b8a 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java @@ -240,6 +240,11 @@ public class Lint */ OPTIONS("options"), + /** + * Warn when any output file is written to more than once. + */ + OUTPUT_FILE_CLASH("output-file-clash"), + /** * Warn about issues regarding method overloads. */ diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/file/BaseFileManager.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/file/BaseFileManager.java index bbc684404b8..fbd4506ed53 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/file/BaseFileManager.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/file/BaseFileManager.java @@ -41,6 +41,8 @@ import java.nio.charset.CoderResult; import java.nio.charset.CodingErrorAction; import java.nio.charset.IllegalCharsetNameException; import java.nio.charset.UnsupportedCharsetException; +import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.util.Collection; import java.util.HashMap; @@ -54,10 +56,12 @@ import javax.tools.JavaFileManager; import javax.tools.JavaFileObject; import javax.tools.JavaFileObject.Kind; +import com.sun.tools.javac.code.Lint.LintCategory; import com.sun.tools.javac.main.Option; import com.sun.tools.javac.main.OptionHelper; import com.sun.tools.javac.main.OptionHelper.GrumpyHelper; import com.sun.tools.javac.resources.CompilerProperties.Errors; +import com.sun.tools.javac.resources.CompilerProperties.Warnings; import com.sun.tools.javac.util.Abort; import com.sun.tools.javac.util.Context; import com.sun.tools.javac.util.DefinedBy; @@ -87,9 +91,12 @@ public abstract class BaseFileManager implements JavaFileManager { options = Options.instance(context); classLoaderClass = options.get("procloader"); - // Avoid initializing Lint + // Detect Lint options, but use Options.isLintSet() to avoid initializing the Lint class boolean warn = options.isLintSet("path"); locations.update(log, warn, FSInfo.instance(context)); + synchronized (this) { + outputFilesWritten = options.isLintSet("output-file-clash") ? new HashSet<>() : null; + } // Setting this option is an indication that close() should defer actually closing // the file manager until after a specified period of inactivity. @@ -133,6 +140,9 @@ public abstract class BaseFileManager implements JavaFileManager { protected final Locations locations; + // This is non-null when output file clash detection is enabled + private HashSet outputFilesWritten; + /** * A flag for clients to use to indicate that this file manager should * be closed when it is no longer required. @@ -467,6 +477,11 @@ public abstract class BaseFileManager implements JavaFileManager { contentCache.remove(file); } + public synchronized void resetOutputFilesWritten() { + if (outputFilesWritten != null) + outputFilesWritten.clear(); + } + protected final Map contentCache = new HashMap<>(); protected static class ContentCacheEntry { @@ -512,4 +527,29 @@ public abstract class BaseFileManager implements JavaFileManager { Objects.requireNonNull(t); return it; } + +// Output File Clash Detection + + /** Record the fact that we have started writing to an output file. + */ + // Note: individual files can be accessed concurrently, so we synchronize here + synchronized void newOutputToPath(Path path) throws IOException { + + // Is output file clash detection enabled? + if (outputFilesWritten == null) + return; + + // Get the "canonical" version of the file's path; we are assuming + // here that two clashing files will resolve to the same real path. + Path realPath; + try { + realPath = path.toRealPath(); + } catch (NoSuchFileException e) { + return; // should never happen except on broken filesystems + } + + // Check whether we've already opened this file for output + if (!outputFilesWritten.add(realPath)) + log.warning(LintCategory.OUTPUT_FILE_CLASH, Warnings.OutputFileClash(path)); + } } diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/file/JavacFileManager.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/file/JavacFileManager.java index 5c1bc5101f0..44951ac83f6 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/file/JavacFileManager.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/file/JavacFileManager.java @@ -738,6 +738,7 @@ public class JavacFileManager extends BaseFileManager implements StandardJavaFil pathsAndContainersByLocationAndRelativeDirectory.clear(); nonIndexingContainersByLocation.clear(); contentCache.clear(); + resetOutputFilesWritten(); } @Override @DefinedBy(Api.COMPILER) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/file/PathFileObject.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/file/PathFileObject.java index adc4f813c60..9ca2aee71ad 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/file/PathFileObject.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/file/PathFileObject.java @@ -448,7 +448,9 @@ public abstract class PathFileObject implements JavaFileObject { fileManager.updateLastUsedTime(); fileManager.flushCache(this); ensureParentDirectoriesExist(); - return Files.newOutputStream(path); + OutputStream output = Files.newOutputStream(path); + fileManager.newOutputToPath(path); + return output; } @Override @DefinedBy(Api.COMPILER) @@ -483,7 +485,9 @@ public abstract class PathFileObject implements JavaFileObject { fileManager.updateLastUsedTime(); fileManager.flushCache(this); ensureParentDirectoriesExist(); - return new OutputStreamWriter(Files.newOutputStream(path), fileManager.getEncodingName()); + Writer writer = new OutputStreamWriter(Files.newOutputStream(path), fileManager.getEncodingName()); + fileManager.newOutputToPath(path); + return writer; } @Override @DefinedBy(Api.COMPILER) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties index 427c79c13ca..3edf0f8aea7 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties @@ -1618,6 +1618,10 @@ compiler.misc.x.print.rounds=\ compiler.warn.file.from.future=\ Modification date is in the future for file {0} +# 0: path +compiler.warn.output.file.clash=\ + output file written more than once: {0} + ##### ## The following string will appear before all messages keyed as: diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties index 5e388b65627..d63be22001c 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties @@ -221,6 +221,10 @@ javac.opt.Xlint.desc.opens=\ javac.opt.Xlint.desc.options=\ Warn about issues relating to use of command line options. +javac.opt.Xlint.desc.output-file-clash=\ + Warn when an output file is overwritten during compilation. This can occur, for example,\n\ +\ on case-insensitive filesystems. Covers class files, native header files, and source files. + javac.opt.Xlint.desc.overloads=\ Warn about issues regarding method overloads. diff --git a/src/jdk.compiler/share/man/javac.1 b/src/jdk.compiler/share/man/javac.1 index c9fcdb7d05c..b28938d255a 100644 --- a/src/jdk.compiler/share/man/javac.1 +++ b/src/jdk.compiler/share/man/javac.1 @@ -736,6 +736,9 @@ constructors in public and protected classes in exported packages. \f[V]options\f[R]: Warns about the issues relating to use of command line options. .IP \[bu] 2 +\f[V]output-file-clash\f[R]: Warns if any output file is overwritten during compilation. +This can occur, for example, on case-insensitive filesystems. +.IP \[bu] 2 \f[V]overloads\f[R]: Warns about the issues related to method overloads. .IP \[bu] 2 \f[V]overrides\f[R]: Warns about the issues related to method overrides. diff --git a/test/langtools/tools/javac/diags/examples.not-yet.txt b/test/langtools/tools/javac/diags/examples.not-yet.txt index 395acd426f9..bb1ab466c55 100644 --- a/test/langtools/tools/javac/diags/examples.not-yet.txt +++ b/test/langtools/tools/javac/diags/examples.not-yet.txt @@ -118,6 +118,7 @@ compiler.warn.incubating.modules # requires adjusted clas compiler.warn.invalid.archive.file compiler.warn.is.preview # difficult to produce reliably despite future changes to java.base compiler.warn.is.preview.reflective # difficult to produce reliably despite future changes to java.base +compiler.warn.output.file.clash # this warning is not generated on Linux compiler.warn.override.bridge compiler.warn.position.overflow # CRTable: caused by files with long lines >= 1024 chars compiler.warn.proc.type.already.exists # JavacFiler: just mentioned in TODO diff --git a/test/langtools/tools/javac/file/OutputFileClashTest.java b/test/langtools/tools/javac/file/OutputFileClashTest.java new file mode 100644 index 00000000000..68714a863cd --- /dev/null +++ b/test/langtools/tools/javac/file/OutputFileClashTest.java @@ -0,0 +1,139 @@ +/* + * Copyright (c) 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 + * 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 8287885 8296656 7016187 + * @summary Verify proper function of the "output-file-clash" lint flag + * @requires os.family == "mac" + * @library /tools/lib + * @modules jdk.compiler/com.sun.tools.javac.api + * jdk.compiler/com.sun.tools.javac.main + * jdk.compiler/com.sun.tools.javac.util + * @build toolbox.ToolBox toolbox.JavacTask + * @run main OutputFileClashTest +*/ + +import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.List; +import java.util.regex.Pattern; + +import toolbox.TestRunner; +import toolbox.ToolBox; +import toolbox.JavacTask; +import toolbox.Task; + +public class OutputFileClashTest extends TestRunner { + + protected ToolBox tb; + + public OutputFileClashTest() { + super(System.err); + tb = new ToolBox(); + } + + protected void runTests() throws Exception { + runTests(m -> new Object[] { Paths.get(m.getName()) }); + } + + Path[] findJavaFiles(Path... paths) throws IOException { + return tb.findJavaFiles(paths); + } + +// Note: in these tests, it's indeterminate which output file gets written first. +// So we compare the log output to a regex that matches the error either way. + + @Test + public void testBug8287885(Path base) throws Exception { + testClash(base, + """ + public class Test { + void method1() { + enum ABC { A, B, C; }; // becomes "Test$1ABC.class" + } + void method2() { + enum Abc { A, B, C; }; // becomes "Test$1Abc.class" + } + } + """, + "- compiler.warn.output.file.clash: .*Test\\$1(ABC|Abc)\\.class"); + } + + @Test + public void testBug8296656(Path base) throws Exception { + testClash(base, + """ + public class Test { + @interface Annotation { + interface foo { } + @interface Foo { } + } + } + """, + "- compiler.warn.output.file.clash: .*Test\\$Annotation\\$(foo|Foo)\\.class"); + } + + @Test + public void testCombiningAcuteAccent(Path base) throws Exception { + testClash(base, + """ + public class Test { + interface Cafe\u0301 { // macos normalizes "e" + U0301 -> U00e9 + } + interface Caf\u00e9 { + } + } + """, + "- compiler.warn.output.file.clash: .*Test\\$Caf.*\\.class"); + } + + private void testClash(Path base, String javaSource, String regex) throws Exception { + + // Compile source + Path src = base.resolve("src"); + tb.writeJavaFiles(src, javaSource); + Path classes = base.resolve("classes"); + tb.createDirectories(classes); + Path headers = base.resolve("headers"); + tb.createDirectories(headers); + List log = new JavacTask(tb, Task.Mode.CMDLINE) + .options("-XDrawDiagnostics", "-Werror", "-Xlint:output-file-clash") + .outdir(classes) + .headerdir(headers) + .files(findJavaFiles(src)) + .run(Task.Expect.FAIL) + .writeAll() + .getOutputLines(Task.OutputKind.DIRECT); + + // Find expected error line + Pattern pattern = Pattern.compile(regex); + if (!log.stream().anyMatch(line -> pattern.matcher(line).matches())) + throw new Exception("expected error not found: \"" + regex + "\""); + } + + public static void main(String... args) throws Exception { + new OutputFileClashTest().runTests(); + } +} diff --git a/test/langtools/tools/lib/toolbox/JavacTask.java b/test/langtools/tools/lib/toolbox/JavacTask.java index 9c188b7fe0e..b4f596b0a79 100644 --- a/test/langtools/tools/lib/toolbox/JavacTask.java +++ b/test/langtools/tools/lib/toolbox/JavacTask.java @@ -55,6 +55,7 @@ public class JavacTask extends AbstractTask { private List classpath; private List sourcepath; private Path outdir; + private Path headerdir; private List options; private List classes; private List files; @@ -169,6 +170,26 @@ public class JavacTask extends AbstractTask { return this; } + /** + * Sets the native header output directory. + * @param headerdir the native header output directory + * @return this task object + */ + public JavacTask headerdir(String headerdir) { + this.headerdir = Paths.get(headerdir); + return this; + } + + /** + * Sets the native header output directory. + * @param headerdir the native header output directory + * @return this task object + */ + public JavacTask headerdir(Path headerdir) { + this.headerdir = headerdir; + return this; + } + /** * Sets the options. * @param options the options @@ -353,6 +374,8 @@ public class JavacTask extends AbstractTask { fileManager = internalFileManager = compiler.getStandardFileManager(null, null, null); if (outdir != null) setLocationFromPaths(StandardLocation.CLASS_OUTPUT, Collections.singletonList(outdir)); + if (headerdir != null) + setLocationFromPaths(StandardLocation.NATIVE_HEADER_OUTPUT, Collections.singletonList(headerdir)); if (classpath != null) setLocationFromPaths(StandardLocation.CLASS_PATH, classpath); if (sourcepath != null) @@ -422,6 +445,10 @@ public class JavacTask extends AbstractTask { args.add("-d"); args.add(outdir.toString()); } + if (headerdir != null) { + args.add("-h"); + args.add(headerdir.toString()); + } if (classpath != null) { args.add("-classpath"); args.add(toSearchPath(classpath));