8182450: javac aborts when generating ct.sym intermittently
Summary: Initialize the module system model even in presence of missing/broken module-infos; BadClassFiles should not immediatelly abort compilation anymore, but should be handled as if the classfile did not exist.
Reviewed-by: jjg
--- a/langtools/make/src/classes/build/tools/symbolgenerator/TransitiveDependencies.java Thu Jul 13 08:49:11 2017 +0200
+++ b/langtools/make/src/classes/build/tools/symbolgenerator/TransitiveDependencies.java Thu Jul 13 13:37:44 2017 +0200
@@ -59,13 +59,14 @@
JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
List<String> options = Arrays.asList("-source", "9",
"-target", "9",
+ "-proc:only",
"--system", "none",
"--module-source-path", args[0],
"--add-modules", Arrays.stream(args)
.skip(1)
.collect(Collectors.joining(",")));
List<String> jlObjectList = Arrays.asList("java.lang.Object");
- JavacTaskImpl task = (JavacTaskImpl) compiler.getTask(null, null, d -> {}, options, jlObjectList, null);
+ JavacTaskImpl task = (JavacTaskImpl) compiler.getTask(null, null, null, options, jlObjectList, null);
task.enter();
Elements elements = task.getElements();
List<String> todo = new LinkedList<>();
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/ClassFinder.java Thu Jul 13 08:49:11 2017 +0200
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/ClassFinder.java Thu Jul 13 13:37:44 2017 +0200
@@ -340,6 +340,8 @@
JavaFileObject classfile = c.classfile;
if (classfile != null) {
JavaFileObject previousClassFile = currentClassFile;
+ Symbol prevOwner = c.owner;
+ Name prevName = c.fullname;
try {
if (reader.filling) {
Assert.error("Filling " + classfile.toUri() + " during " + previousClassFile);
@@ -360,6 +362,21 @@
+ classfile.toUri());
}
}
+ } catch (BadClassFile cf) {
+ //the symbol may be partially initialized, purge it:
+ c.owner = prevOwner;
+ c.members_field.getSymbols(sym -> sym.kind == TYP).forEach(sym -> {
+ ClassSymbol csym = (ClassSymbol) sym;
+ csym.owner = sym.packge();
+ csym.owner.members().enter(sym);
+ csym.fullname = sym.flatName();
+ csym.name = Convert.shortName(sym.flatName());
+ csym.reset();
+ });
+ c.fullname = prevName;
+ c.name = Convert.shortName(prevName);
+ c.reset();
+ throw cf;
} finally {
currentClassFile = previousClassFile;
}
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java Thu Jul 13 08:49:11 2017 +0200
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java Thu Jul 13 13:37:44 2017 +0200
@@ -290,8 +290,7 @@
*/
public Type completionError(DiagnosticPosition pos, CompletionFailure ex) {
log.error(JCDiagnostic.DiagnosticFlag.NON_DEFERRABLE, pos, Errors.CantAccess(ex.sym, ex.getDetailValue()));
- if (ex instanceof ClassFinder.BadClassFile) throw new Abort();
- else return syms.errType;
+ return syms.errType;
}
/** Report an error that wrong type tag was found.
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java Thu Jul 13 08:49:11 2017 +0200
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java Thu Jul 13 13:37:44 2017 +0200
@@ -94,10 +94,8 @@
import com.sun.tools.javac.tree.JCTree.JCUses;
import com.sun.tools.javac.tree.JCTree.Tag;
import com.sun.tools.javac.tree.TreeInfo;
-import com.sun.tools.javac.util.Abort;
import com.sun.tools.javac.util.Assert;
import com.sun.tools.javac.util.Context;
-import com.sun.tools.javac.util.JCDiagnostic;
import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition;
import com.sun.tools.javac.util.List;
import com.sun.tools.javac.util.ListBuffer;
@@ -105,7 +103,6 @@
import com.sun.tools.javac.util.Name;
import com.sun.tools.javac.util.Names;
import com.sun.tools.javac.util.Options;
-import com.sun.tools.javac.util.Position;
import static com.sun.tools.javac.code.Flags.ABSTRACT;
import static com.sun.tools.javac.code.Flags.ENUM;
@@ -266,8 +263,7 @@
msym.complete();
}
} catch (CompletionFailure ex) {
- log.error(JCDiagnostic.DiagnosticFlag.NON_DEFERRABLE, Position.NOPOS, Errors.CantAccess(ex.sym, ex.getDetailValue()));
- if (ex instanceof ClassFinder.BadClassFile) throw new Abort();
+ chk.completionError(null, ex);
} finally {
depth--;
}
@@ -1220,10 +1216,6 @@
Predicate<ModuleSymbol> observablePred = sym ->
(observable == null) ? (moduleFinder.findModule(sym).kind != ERR) : observable.contains(sym);
Predicate<ModuleSymbol> systemModulePred = sym -> (sym.flags() & Flags.SYSTEM_MODULE) != 0;
- Predicate<ModuleSymbol> noIncubatorPred = sym -> {
- sym.complete();
- return !sym.resolutionFlags.contains(ModuleResolutionFlags.DO_NOT_RESOLVE_BY_DEFAULT);
- };
Set<ModuleSymbol> enabledRoot = new LinkedHashSet<>();
if (rootModules.contains(syms.unnamedModule)) {
@@ -1241,9 +1233,18 @@
jdkModulePred = sym -> true;
}
+ Predicate<ModuleSymbol> noIncubatorPred = sym -> {
+ sym.complete();
+ return !sym.resolutionFlags.contains(ModuleResolutionFlags.DO_NOT_RESOLVE_BY_DEFAULT);
+ };
+
for (ModuleSymbol sym : new HashSet<>(syms.getAllModules())) {
- if (systemModulePred.test(sym) && observablePred.test(sym) && jdkModulePred.test(sym) && noIncubatorPred.test(sym)) {
- enabledRoot.add(sym);
+ try {
+ if (systemModulePred.test(sym) && observablePred.test(sym) && jdkModulePred.test(sym) && noIncubatorPred.test(sym)) {
+ enabledRoot.add(sym);
+ }
+ } catch (CompletionFailure ex) {
+ chk.completionError(null, ex);
}
}
}
@@ -1341,32 +1342,36 @@
result.add(syms.java_base);
while (primaryTodo.nonEmpty() || secondaryTodo.nonEmpty()) {
- ModuleSymbol current;
- boolean isPrimaryTodo;
- if (primaryTodo.nonEmpty()) {
- current = primaryTodo.head;
- primaryTodo = primaryTodo.tail;
- isPrimaryTodo = true;
- } else {
- current = secondaryTodo.head;
- secondaryTodo = secondaryTodo.tail;
- isPrimaryTodo = false;
- }
- if (observable != null && !observable.contains(current))
- continue;
- if (!result.add(current) || current == syms.unnamedModule || ((current.flags_field & Flags.AUTOMATIC_MODULE) != 0))
- continue;
- current.complete();
- if (current.kind == ERR && (isPrimaryTodo || base.contains(current)) && warnedMissing.add(current)) {
- log.error(Errors.ModuleNotFound(current));
- }
- for (RequiresDirective rd : current.requires) {
- if (rd.module == syms.java_base) continue;
- if ((rd.isTransitive() && isPrimaryTodo) || rootModules.contains(current)) {
- primaryTodo = primaryTodo.prepend(rd.module);
+ try {
+ ModuleSymbol current;
+ boolean isPrimaryTodo;
+ if (primaryTodo.nonEmpty()) {
+ current = primaryTodo.head;
+ primaryTodo = primaryTodo.tail;
+ isPrimaryTodo = true;
} else {
- secondaryTodo = secondaryTodo.prepend(rd.module);
+ current = secondaryTodo.head;
+ secondaryTodo = secondaryTodo.tail;
+ isPrimaryTodo = false;
}
+ if (observable != null && !observable.contains(current))
+ continue;
+ if (!result.add(current) || current == syms.unnamedModule || ((current.flags_field & Flags.AUTOMATIC_MODULE) != 0))
+ continue;
+ current.complete();
+ if (current.kind == ERR && (isPrimaryTodo || base.contains(current)) && warnedMissing.add(current)) {
+ log.error(Errors.ModuleNotFound(current));
+ }
+ for (RequiresDirective rd : current.requires) {
+ if (rd.module == syms.java_base) continue;
+ if ((rd.isTransitive() && isPrimaryTodo) || rootModules.contains(current)) {
+ primaryTodo = primaryTodo.prepend(rd.module);
+ } else {
+ secondaryTodo = secondaryTodo.prepend(rd.module);
+ }
+ }
+ } catch (CompletionFailure ex) {
+ chk.completionError(null, ex);
}
}
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java Thu Jul 13 08:49:11 2017 +0200
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java Thu Jul 13 13:37:44 2017 +0200
@@ -1425,7 +1425,7 @@
ClassSymbol c = readClassSymbol(nextChar());
NameAndType nt = readNameAndType(nextChar());
- if (c.members_field == null)
+ if (c.members_field == null || c.kind != TYP)
throw badClassFile("bad.enclosing.class", self, c);
MethodSymbol m = findMethod(nt, c.members_field, self.flags());
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/main/JavaCompiler.java Thu Jul 13 08:49:11 2017 +0200
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/main/JavaCompiler.java Thu Jul 13 13:37:44 2017 +0200
@@ -400,8 +400,6 @@
} catch (CompletionFailure ex) {
// inlined Check.completionError as it is not initialized yet
log.error(Errors.CantAccess(ex.sym, ex.getDetailValue()));
- if (ex instanceof ClassFinder.BadClassFile)
- throw new Abort();
}
source = Source.instance(context);
attr = Attr.instance(context);
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/modules/BrokenModulesTest.java Thu Jul 13 13:37:44 2017 +0200
@@ -0,0 +1,110 @@
+/*
+ * Copyright (c) 2017, 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 8182450
+ * @summary Test model behavior when a completing a broken module-info.
+ * @library /tools/lib
+ * @modules
+ * jdk.compiler/com.sun.tools.javac.api
+ * jdk.compiler/com.sun.tools.javac.code
+ * jdk.compiler/com.sun.tools.javac.main
+ * jdk.compiler/com.sun.tools.javac.processing
+ * @build toolbox.ToolBox toolbox.JavacTask toolbox.ModuleBuilder ModuleTestBase
+ * @run main BrokenModulesTest
+ */
+
+import java.nio.file.Path;
+import java.util.List;
+
+import javax.tools.JavaCompiler;
+import javax.tools.ToolProvider;
+
+import com.sun.tools.javac.api.JavacTaskImpl;
+
+public class BrokenModulesTest extends ModuleTestBase {
+
+ public static void main(String... args) throws Exception {
+ new BrokenModulesTest().runTests();
+ }
+
+ List<String> jlObjectList = List.of("java.lang.Object");
+ JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
+
+ @Test
+ public void testBrokenModuleInfoModuleSourcePath(Path base) throws Exception {
+ Path msp = base.resolve("msp");
+ Path ma = msp.resolve("ma");
+ tb.writeJavaFiles(ma,
+ "module ma { requires not.available; exports api1; }",
+ "package api1; public interface Api { }");
+ Path out = base.resolve("out");
+ tb.createDirectories(out);
+
+ List<String> opts = List.of("--module-source-path", msp.toString(),
+ "--add-modules", "ma");
+ JavacTaskImpl task = (JavacTaskImpl) compiler.getTask(null, null, null, opts, jlObjectList, null);
+
+ task.enter();
+
+ task.getElements().getModuleElement("java.base");
+ }
+
+ @Test
+ public void testSystem(Path base) throws Exception {
+ Path jdkCompiler = base.resolve("jdk.compiler");
+ tb.writeJavaFiles(jdkCompiler,
+ "module jdk.compiler { requires not.available; }");
+ Path out = base.resolve("out");
+ tb.createDirectories(out);
+
+ List<String> opts = List.of("--patch-module", "jdk.compiler=" + jdkCompiler.toString(),
+ "--add-modules", "jdk.compiler");
+ JavacTaskImpl task = (JavacTaskImpl) compiler.getTask(null, null, null, opts, jlObjectList, null);
+
+ task.enter();
+
+ task.getElements().getModuleElement("java.base");
+ }
+
+ @Test
+ public void testParseError(Path base) throws Exception {
+ Path msp = base.resolve("msp");
+ Path ma = msp.resolve("ma");
+ tb.writeJavaFiles(ma,
+ "broken module ma { }",
+ "package api1; public interface Api { }");
+ Path out = base.resolve("out");
+ tb.createDirectories(out);
+
+ List<String> opts = List.of("--module-source-path", msp.toString(),
+ "--add-modules", "ma");
+ JavacTaskImpl task = (JavacTaskImpl) compiler.getTask(null, null, null, opts, jlObjectList, null);
+
+ task.analyze();
+
+ task.getElements().getModuleElement("java.base");
+ }
+
+}
--- a/langtools/test/tools/javac/modules/EdgeCases.java Thu Jul 13 08:49:11 2017 +0200
+++ b/langtools/test/tools/javac/modules/EdgeCases.java Thu Jul 13 13:37:44 2017 +0200
@@ -467,7 +467,8 @@
List<String> expected = Arrays.asList(
"- compiler.err.cant.access: m1x.module-info, (compiler.misc.bad.class.file.header: module-info.class, (compiler.misc.module.name.mismatch: other, m1x))",
- "1 error");
+ "module-info.java:1:1: compiler.err.module.not.found: m1x",
+ "2 errors");
if (!expected.equals(log)) {
throw new AssertionError("Unexpected output: " + log);
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/processing/model/completionfailure/NoAbortForBadClassFile.java Thu Jul 13 13:37:44 2017 +0200
@@ -0,0 +1,248 @@
+/*
+ * Copyright (c) 2017, 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 8182450
+ * @summary Bad classfiles should not abort compilations
+ * @library /tools/lib
+ * @modules
+ * jdk.compiler/com.sun.tools.javac.api
+ * jdk.compiler/com.sun.tools.javac.code
+ * jdk.compiler/com.sun.tools.javac.comp
+ * jdk.compiler/com.sun.tools.javac.jvm
+ * jdk.compiler/com.sun.tools.javac.main
+ * jdk.compiler/com.sun.tools.javac.processing
+ * jdk.compiler/com.sun.tools.javac.util
+ * @build toolbox.ToolBox toolbox.JavacTask
+ * @run main NoAbortForBadClassFile
+ */
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import com.sun.tools.javac.api.JavacTaskImpl;
+import com.sun.tools.javac.api.JavacTool;
+import com.sun.tools.javac.code.Flags;
+import com.sun.tools.javac.code.Symbol.ClassSymbol;
+import com.sun.tools.javac.code.Symbol.CompletionFailure;
+import com.sun.tools.javac.code.Symtab;
+import com.sun.tools.javac.jvm.ClassReader;
+import com.sun.tools.javac.util.Context;
+import com.sun.tools.javac.util.Context.Factory;
+import com.sun.tools.javac.util.Names;
+import com.sun.tools.javac.util.Options;
+import toolbox.Task;
+import toolbox.Task.Expect;
+
+import toolbox.TestRunner;
+import toolbox.ToolBox;
+
+public class NoAbortForBadClassFile extends TestRunner {
+
+ private ToolBox tb = new ToolBox();
+
+ public NoAbortForBadClassFile() {
+ super(System.out);
+ }
+
+ public static void main(String... args) throws Exception {
+ new NoAbortForBadClassFile().runTests(m -> new Object[] { Paths.get(m.getName()) });
+ }
+
+ @Test
+ public void testBrokenClassFile(Path base) throws Exception {
+ Path classes = base.resolve("classes");
+ Path brokenClassFile = classes.resolve("test").resolve("Broken.class");
+
+ Files.createDirectories(brokenClassFile.getParent());
+ Files.newOutputStream(brokenClassFile).close();
+
+ Path src = base.resolve("src");
+ tb.writeJavaFiles(src,
+ "package test; public class Test { private void test() { Broken b; String.unknown(); } }");
+ Path out = base.resolve("out");
+ tb.createDirectories(out);
+
+ List<String> log = new toolbox.JavacTask(tb)
+ .options("-classpath", classes.toString(),
+ "-XDrawDiagnostics")
+ .outdir(out)
+ .files(tb.findJavaFiles(src))
+ .run(Expect.FAIL)
+ .writeAll()
+ .getOutputLines(Task.OutputKind.DIRECT);
+
+ List<String> expectedOut = Arrays.asList(
+ "Test.java:1:57: compiler.err.cant.access: test.Broken, (compiler.misc.bad.class.file.header: Broken.class, (compiler.misc.class.file.wrong.class: java.lang.AutoCloseable))",
+ "Test.java:1:73: compiler.err.cant.resolve.location.args: kindname.method, unknown, , , (compiler.misc.location: kindname.class, java.lang.String, null)",
+ "2 errors"
+ );
+
+ if (!expectedOut.equals(log))
+ throw new Exception("expected output not found: " + log);
+ }
+
+ @Test
+ public void testLoading(Path base) throws Exception {
+ Path src = base.resolve("src");
+ tb.writeJavaFiles(src,
+ "public class Test { static { new Object() {}; } public static class I { public static class II { } } }");
+ Path out = base.resolve("out");
+ tb.createDirectories(out);
+
+ new toolbox.JavacTask(tb)
+ .outdir(out)
+ .files(tb.findJavaFiles(src))
+ .run(Expect.SUCCESS)
+ .writeAll()
+ .getOutputLines(Task.OutputKind.DIRECT);
+
+ List<Path> files;
+ try (Stream<Path> dir = Files.list(out)) {
+ files = dir.collect(Collectors.toList());
+ }
+
+ List<List<Path>> result = new ArrayList<>();
+
+ permutations(files, Collections.emptyList(), result);
+
+ for (List<Path> order : result) {
+ for (Path missing : order) {
+ Path test = base.resolve("test");
+
+ if (Files.exists(test)) {
+ tb.cleanDirectory(test);
+ } else {
+ tb.createDirectories(test);
+ }
+
+ for (Path p : order) {
+ Files.copy(p, test.resolve(p.getFileName()));
+ }
+
+ List<String> actual = complete(test, order, missing, true);
+
+ Files.delete(test.resolve(missing.getFileName()));
+
+ List<String> expected = complete(test, order, missing, false);
+
+ if (!actual.equals(expected)) {
+ throw new AssertionError("Unexpected state, actual=\n" + actual + "\nexpected=\n" + expected + "\norder=" + order + "\nmissing=" + missing);
+ }
+ }
+ }
+ }
+
+ private static void permutations(List<Path> todo, List<Path> currentList, List<List<Path>> result) {
+ if (todo.isEmpty()) {
+ result.add(currentList);
+ return ;
+ }
+
+ for (Path p : todo) {
+ List<Path> nextTODO = new ArrayList<>(todo);
+
+ nextTODO.remove(p);
+
+ List<Path> nextList = new ArrayList<>(currentList);
+
+ nextList.add(p);
+
+ permutations(nextTODO, nextList, result);
+ }
+ }
+
+ private List<String> complete(Path test, List<Path> order, Path missing, boolean badClassFile) {
+ Context context = new Context();
+ if (badClassFile) {
+ TestClassReader.preRegister(context);
+ }
+ JavacTool tool = JavacTool.create();
+ JavacTaskImpl task = (JavacTaskImpl) tool.getTask(null, null, null, List.of("-classpath", test.toString(), "-XDblockClass=" + flatName(missing)), null, null, context);
+ Symtab syms = Symtab.instance(context);
+ Names names = Names.instance(context);
+
+ task.getElements().getTypeElement("java.lang.Object");
+
+ if (!badClassFile) {
+ //to ensure the same paths taken in ClassFinder.completeEnclosing in case the file is missing:
+ syms.enterClass(syms.unnamedModule, names.fromString(flatName(missing)));
+ }
+
+ List<String> result = new ArrayList<>();
+
+ for (Path toCheck : order) {
+ ClassSymbol sym = syms.enterClass(syms.unnamedModule, names.fromString(flatName(toCheck)));
+
+ try {
+ sym.complete();
+ } catch (CompletionFailure ignore) {
+ }
+
+ long flags = sym.flags_field;
+
+ flags &= ~(Flags.CLASS_SEEN | Flags.SOURCE_SEEN);
+
+ result.add("sym: " + sym.flatname + ", " + sym.owner.flatName() +
+ ", " + sym.type + ", " + sym.members_field + ", " + flags);
+ }
+
+ return result;
+ }
+
+ private String flatName(Path p) {
+ return p.getFileName().toString().replace(".class", "");
+ }
+
+ private static class TestClassReader extends ClassReader {
+ public static void preRegister(Context ctx) {
+ ctx.put(classReaderKey, (Factory<ClassReader>) c -> new TestClassReader(ctx));
+ }
+
+ private final String block;
+
+ public TestClassReader(Context context) {
+ super(context);
+ block = Options.instance(context).get("blockClass");
+ }
+
+ @Override
+ public void readClassFile(ClassSymbol c) {
+ super.readClassFile(c);
+
+ if (c.flatname.contentEquals(block)) {
+ throw badClassFile("blocked");
+ }
+ }
+
+ }
+
+}