Skip to content

Commit

Permalink
8296656: java.lang.NoClassDefFoundError exception on running fully le…
Browse files Browse the repository at this point in the history
…gitimate code

8287885: Local classes cause ClassLoader error if the type names are similar but not same

Reviewed-by: vromero
  • Loading branch information
archiecobbs authored and Vicente Romero committed Mar 27, 2023
1 parent 80e2d52 commit 14b970d
Show file tree
Hide file tree
Showing 10 changed files with 231 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,11 @@ public enum LintCategory {
*/
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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
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;
Expand All @@ -54,10 +56,12 @@
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;
Expand Down Expand Up @@ -87,9 +91,12 @@ public void setContext(Context context) {
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.
Expand Down Expand Up @@ -133,6 +140,9 @@ protected Locations createLocations() {

protected final Locations locations;

// This is non-null when output file clash detection is enabled
private HashSet<Path> outputFilesWritten;

/**
* A flag for clients to use to indicate that this file manager should
* be closed when it is no longer required.
Expand Down Expand Up @@ -467,6 +477,11 @@ public void flushCache(JavaFileObject file) {
contentCache.remove(file);
}

public synchronized void resetOutputFilesWritten() {
if (outputFilesWritten != null)
outputFilesWritten.clear();
}

protected final Map<JavaFileObject, ContentCacheEntry> contentCache = new HashMap<>();

protected static class ContentCacheEntry {
Expand Down Expand Up @@ -512,4 +527,29 @@ protected static <T> Collection<T> nullCheck(Collection<T> it) {
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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,7 @@ public void close() throws IOException {
pathsAndContainersByLocationAndRelativeDirectory.clear();
nonIndexingContainersByLocation.clear();
contentCache.clear();
resetOutputFilesWritten();
}

@Override @DefinedBy(Api.COMPILER)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,9 @@ public OutputStream openOutputStream() throws IOException {
fileManager.updateLastUsedTime();
fileManager.flushCache(this);
ensureParentDirectoriesExist();
return Files.newOutputStream(path);
OutputStream output = Files.newOutputStream(path);
fileManager.newOutputToPath(path);
return output;
}

@Override @DefinedBy(Api.COMPILER)
Expand Down Expand Up @@ -483,7 +485,9 @@ public Writer openWriter() throws IOException {
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
3 changes: 3 additions & 0 deletions src/jdk.compiler/share/man/javac.1
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions test/langtools/tools/javac/diags/examples.not-yet.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
139 changes: 139 additions & 0 deletions test/langtools/tools/javac/file/OutputFileClashTest.java
Original file line number Diff line number Diff line change
@@ -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<String> 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();
}
}
27 changes: 27 additions & 0 deletions test/langtools/tools/lib/toolbox/JavacTask.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public class JavacTask extends AbstractTask<JavacTask> {
private List<Path> classpath;
private List<Path> sourcepath;
private Path outdir;
private Path headerdir;
private List<String> options;
private List<String> classes;
private List<String> files;
Expand Down Expand Up @@ -169,6 +170,26 @@ public JavacTask outdir(Path outdir) {
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
Expand Down Expand Up @@ -353,6 +374,8 @@ private int runAPI(PrintWriter pw) throws IOException {
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)
Expand Down Expand Up @@ -422,6 +445,10 @@ private List<String> getAllArgs() {
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));
Expand Down

1 comment on commit 14b970d

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.