8230847: Trees.getScope may crash when invoked for statement inside switch.
Summary: More thoroughly avoiding side-effects when attributing (to) for Trees.getScope.
Reviewed-by: mcimadamore
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/api/JavacTrees.java Wed Oct 30 14:52:27 2019 +0100
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/api/JavacTrees.java Mon Nov 04 10:58:14 2019 +0100
@@ -951,34 +951,20 @@
private Env<AttrContext> attribStatToTree(JCTree stat, Env<AttrContext>env,
JCTree tree, Map<JCClassDecl, JCClassDecl> copiedClasses) {
- JavaFileObject prev = log.useSource(env.toplevel.sourcefile);
- Log.DiagnosticHandler diagHandler = new Log.DiscardDiagnosticHandler(log);
- try {
- Env<AttrContext> result = attr.attribStatToTree(stat, env, tree);
+ Env<AttrContext> result = attr.attribStatToTree(stat, env, tree);
- enter.unenter(env.toplevel, stat);
- fixLocalClassNames(copiedClasses, env);
- return result;
- } finally {
- log.popDiagnosticHandler(diagHandler);
- log.useSource(prev);
- }
+ fixLocalClassNames(copiedClasses, env);
+
+ return result;
}
private Env<AttrContext> attribExprToTree(JCExpression expr, Env<AttrContext>env,
JCTree tree, Map<JCClassDecl, JCClassDecl> copiedClasses) {
- JavaFileObject prev = log.useSource(env.toplevel.sourcefile);
- Log.DiagnosticHandler diagHandler = new Log.DiscardDiagnosticHandler(log);
- try {
- Env<AttrContext> result = attr.attribExprToTree(expr, env, tree);
+ Env<AttrContext> result = attr.attribExprToTree(expr, env, tree);
- enter.unenter(env.toplevel, expr);
- fixLocalClassNames(copiedClasses, env);
- return result;
- } finally {
- log.popDiagnosticHandler(diagHandler);
- log.useSource(prev);
- }
+ fixLocalClassNames(copiedClasses, env);
+
+ return result;
}
/* Change the flatnames of the local and anonymous classes in the Scope to
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Analyzer.java Wed Oct 30 14:52:27 2019 +0100
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Analyzer.java Mon Nov 04 10:58:14 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2014, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2019, 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
@@ -564,7 +564,7 @@
//TODO: to further refine the analysis, try all rewriting combinations
deferredAttr.attribSpeculative(treeToAnalyze, rewriting.env, attr.statInfo, new TreeRewriter(rewriting),
- t -> rewriting.diagHandler(), AttributionMode.ANALYZER, argumentAttr.withLocalCacheContext());
+ () -> rewriting.diagHandler(), AttributionMode.ANALYZER, argumentAttr.withLocalCacheContext());
rewriting.analyzer.process(rewriting.oldTree, rewriting.replacement, rewriting.erroneous);
} catch (Throwable ex) {
Assert.error("Analyzer error when processing: " +
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Annotate.java Wed Oct 30 14:52:27 2019 +0100
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Annotate.java Mon Nov 04 10:58:14 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2003, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2019, 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
@@ -1377,4 +1377,31 @@
public void newRound() {
blockCount = 1;
}
+
+ public Queues setQueues(Queues nue) {
+ Queues stored = new Queues(q, validateQ, typesQ, afterTypesQ);
+ this.q = nue.q;
+ this.typesQ = nue.typesQ;
+ this.afterTypesQ = nue.afterTypesQ;
+ this.validateQ = nue.validateQ;
+ return stored;
+ }
+
+ static class Queues {
+ private final ListBuffer<Runnable> q;
+ private final ListBuffer<Runnable> validateQ;
+ private final ListBuffer<Runnable> typesQ;
+ private final ListBuffer<Runnable> afterTypesQ;
+
+ public Queues() {
+ this(new ListBuffer<Runnable>(), new ListBuffer<Runnable>(), new ListBuffer<Runnable>(), new ListBuffer<Runnable>());
+ }
+
+ public Queues(ListBuffer<Runnable> q, ListBuffer<Runnable> validateQ, ListBuffer<Runnable> typesQ, ListBuffer<Runnable> afterTypesQ) {
+ this.q = q;
+ this.validateQ = validateQ;
+ this.typesQ = typesQ;
+ this.afterTypesQ = afterTypesQ;
+ }
+ }
}
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java Wed Oct 30 14:52:27 2019 +0100
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java Mon Nov 04 10:58:14 2019 +0100
@@ -74,11 +74,8 @@
import static com.sun.tools.javac.code.Kinds.Kind.*;
import static com.sun.tools.javac.code.TypeTag.*;
import static com.sun.tools.javac.code.TypeTag.WILDCARD;
-import com.sun.tools.javac.comp.Analyzer.AnalyzerMode;
import static com.sun.tools.javac.tree.JCTree.Tag.*;
import com.sun.tools.javac.util.JCDiagnostic.DiagnosticFlag;
-import com.sun.tools.javac.util.Log.DeferredDiagnosticHandler;
-import com.sun.tools.javac.util.Log.DiscardDiagnosticHandler;
/** This is the main context-dependent analysis phase in GJC. It
* encompasses name resolution, type checking and constant folding as
@@ -394,12 +391,20 @@
}
public Env<AttrContext> attribExprToTree(JCTree expr, Env<AttrContext> env, JCTree tree) {
+ return attribToTree(expr, env, tree, unknownExprInfo);
+ }
+
+ public Env<AttrContext> attribStatToTree(JCTree stmt, Env<AttrContext> env, JCTree tree) {
+ return attribToTree(stmt, env, tree, statInfo);
+ }
+
+ private Env<AttrContext> attribToTree(JCTree root, Env<AttrContext> env, JCTree tree, ResultInfo resultInfo) {
breakTree = tree;
JavaFileObject prev = log.useSource(env.toplevel.sourcefile);
- EnumSet<AnalyzerMode> analyzerModes = EnumSet.copyOf(analyzer.analyzerModes);
try {
- analyzer.analyzerModes.clear();
- attribExpr(expr, env);
+ deferredAttr.attribSpeculative(root, env, resultInfo,
+ null, DeferredAttr.AttributionMode.ANALYZER,
+ argumentAttr.withLocalCacheContext());
} catch (BreakAttr b) {
return b.env;
} catch (AssertionError ae) {
@@ -411,30 +416,6 @@
} finally {
breakTree = null;
log.useSource(prev);
- analyzer.analyzerModes.addAll(analyzerModes);
- }
- return env;
- }
-
- public Env<AttrContext> attribStatToTree(JCTree stmt, Env<AttrContext> env, JCTree tree) {
- breakTree = tree;
- JavaFileObject prev = log.useSource(env.toplevel.sourcefile);
- EnumSet<AnalyzerMode> analyzerModes = EnumSet.copyOf(analyzer.analyzerModes);
- try {
- analyzer.analyzerModes.clear();
- attribStat(stmt, env);
- } catch (BreakAttr b) {
- return b.env;
- } catch (AssertionError ae) {
- if (ae.getCause() instanceof BreakAttr) {
- return ((BreakAttr)(ae.getCause())).env;
- } else {
- throw ae;
- }
- } finally {
- breakTree = null;
- log.useSource(prev);
- analyzer.analyzerModes.addAll(analyzerModes);
}
return env;
}
@@ -1273,7 +1254,7 @@
}
public void visitBlock(JCBlock tree) {
- if (env.info.scope.owner.kind == TYP) {
+ if (env.info.scope.owner.kind == TYP || env.info.scope.owner.kind == ERR) {
// Block is a static or instance initializer;
// let the owner of the environment be a freshly
// created BLOCK-method.
@@ -1533,8 +1514,8 @@
attribCase.accept(c, caseEnv);
} finally {
caseEnv.info.scope.leave();
- addVars(c.stats, switchEnv.info.scope);
}
+ addVars(c.stats, switchEnv.info.scope);
}
} finally {
switchEnv.info.scope.leave();
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java Wed Oct 30 14:52:27 2019 +0100
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java Mon Nov 04 10:58:14 2019 +0100
@@ -47,6 +47,7 @@
import com.sun.tools.javac.tree.JCTree.*;
import com.sun.tools.javac.util.JCDiagnostic.DiagnosticType;
import com.sun.tools.javac.util.Log.DeferredDiagnosticHandler;
+import com.sun.tools.javac.util.Log.DiagnosticHandler;
import java.util.ArrayList;
import java.util.Collection;
@@ -57,13 +58,14 @@
import java.util.Map;
import java.util.Set;
import java.util.WeakHashMap;
-import java.util.function.Function;
+import java.util.function.Supplier;
import com.sun.source.tree.MemberReferenceTree;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.tree.JCTree.JCMemberReference.OverloadKind;
import static com.sun.tools.javac.code.TypeTag.*;
+import com.sun.tools.javac.comp.Annotate.Queues;
import static com.sun.tools.javac.tree.JCTree.Tag.*;
/**
@@ -80,6 +82,7 @@
public class DeferredAttr extends JCTree.Visitor {
protected static final Context.Key<DeferredAttr> deferredAttrKey = new Context.Key<>();
+ final Annotate annotate;
final Attr attr;
final ArgumentAttr argumentAttr;
final Check chk;
@@ -107,6 +110,7 @@
protected DeferredAttr(Context context) {
context.put(deferredAttrKey, this);
+ annotate = Annotate.instance(context);
attr = Attr.instance(context);
argumentAttr = ArgumentAttr.instance(context);
chk = Check.instance(context);
@@ -483,31 +487,44 @@
*/
JCTree attribSpeculative(JCTree tree, Env<AttrContext> env, ResultInfo resultInfo) {
return attribSpeculative(tree, env, resultInfo, treeCopier,
- newTree->new DeferredDiagnosticHandler(log), AttributionMode.SPECULATIVE, null);
+ null, AttributionMode.SPECULATIVE, null);
}
JCTree attribSpeculative(JCTree tree, Env<AttrContext> env, ResultInfo resultInfo, LocalCacheContext localCache) {
return attribSpeculative(tree, env, resultInfo, treeCopier,
- newTree->new DeferredDiagnosticHandler(log), AttributionMode.SPECULATIVE, localCache);
+ null, AttributionMode.SPECULATIVE, localCache);
}
<Z> JCTree attribSpeculative(JCTree tree, Env<AttrContext> env, ResultInfo resultInfo, TreeCopier<Z> deferredCopier,
- Function<JCTree, DeferredDiagnosticHandler> diagHandlerCreator, AttributionMode attributionMode,
+ Supplier<DiagnosticHandler> diagHandlerCreator, AttributionMode attributionMode,
LocalCacheContext localCache) {
final JCTree newTree = deferredCopier.copy(tree);
- Env<AttrContext> speculativeEnv = env.dup(newTree, env.info.dup(env.info.scope.dupUnshared(env.info.scope.owner)));
+ return attribSpeculative(newTree, env, resultInfo, diagHandlerCreator, attributionMode, localCache);
+ }
+
+ /**
+ * Attribute the given tree, mostly reverting side-effects applied to shared
+ * compiler state. Exceptions include the ArgumentAttr.argumentTypeCache,
+ * changes to which may be preserved if localCache is null.
+ */
+ <Z> JCTree attribSpeculative(JCTree tree, Env<AttrContext> env, ResultInfo resultInfo,
+ Supplier<DiagnosticHandler> diagHandlerCreator, AttributionMode attributionMode,
+ LocalCacheContext localCache) {
+ Env<AttrContext> speculativeEnv = env.dup(tree, env.info.dup(env.info.scope.dupUnshared(env.info.scope.owner)));
speculativeEnv.info.attributionMode = attributionMode;
- Log.DeferredDiagnosticHandler deferredDiagnosticHandler = diagHandlerCreator.apply(newTree);
+ Log.DiagnosticHandler deferredDiagnosticHandler = diagHandlerCreator != null ? diagHandlerCreator.get() : new DeferredDiagnosticHandler(log);
DeferredCompletionFailureHandler.Handler prevCFHandler = dcfh.setHandler(dcfh.speculativeCodeHandler);
+ Queues prevQueues = annotate.setQueues(new Queues());
int nwarnings = log.nwarnings;
log.nwarnings = 0;
try {
- attr.attribTree(newTree, speculativeEnv, resultInfo);
- return newTree;
+ attr.attribTree(tree, speculativeEnv, resultInfo);
+ return tree;
} finally {
+ annotate.setQueues(prevQueues);
dcfh.setHandler(prevCFHandler);
log.nwarnings += nwarnings;
- enter.unenter(env.toplevel, newTree);
+ enter.unenter(env.toplevel, tree);
log.popDiagnosticHandler(deferredDiagnosticHandler);
if (localCache != null) {
localCache.leave();
--- a/test/langtools/tools/javac/api/TestGetScopeResult.java Wed Oct 30 14:52:27 2019 +0100
+++ b/test/langtools/tools/javac/api/TestGetScopeResult.java Mon Nov 04 10:58:14 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019, 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
@@ -23,7 +23,7 @@
/*
* @test
- * @bug 8205418 8207229 8207230
+ * @bug 8205418 8207229 8207230 8230847
* @summary Test the outcomes from Trees.getScope
* @modules jdk.compiler/com.sun.tools.javac.api
* jdk.compiler/com.sun.tools.javac.comp
@@ -42,11 +42,19 @@
import javax.tools.StandardJavaFileManager;
import javax.tools.ToolProvider;
+import com.sun.source.tree.BlockTree;
import com.sun.source.tree.CompilationUnitTree;
+import com.sun.source.tree.ConditionalExpressionTree;
+import com.sun.source.tree.IdentifierTree;
import com.sun.source.tree.LambdaExpressionTree;
+import com.sun.source.tree.MethodInvocationTree;
+import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Scope;
+import com.sun.source.tree.Tree;
import com.sun.source.tree.VariableTree;
import com.sun.source.util.JavacTask;
+import com.sun.source.util.TaskEvent;
+import com.sun.source.util.TaskListener;
import com.sun.source.util.TreePath;
import com.sun.source.util.TreePathScanner;
import com.sun.source.util.Trees;
@@ -65,6 +73,11 @@
public static void main(String... args) throws IOException {
new TestGetScopeResult().run();
new TestGetScopeResult().testAnalyzerDisabled();
+ new TestGetScopeResult().testVariablesInSwitch();
+ new TestGetScopeResult().testMemberRefs();
+ new TestGetScopeResult().testAnnotations();
+ new TestGetScopeResult().testAnnotationsLazy();
+ new TestGetScopeResult().testCircular();
}
public void run() throws IOException {
@@ -259,5 +272,225 @@
super.analyze(statement, env);
}
}
+
+ void testVariablesInSwitch() throws IOException {
+ JavacTool c = JavacTool.create();
+ try (StandardJavaFileManager fm = c.getStandardFileManager(null, null, null)) {
+ class MyFileObject extends SimpleJavaFileObject {
+ MyFileObject() {
+ super(URI.create("myfo:///Test.java"), SOURCE);
+ }
+ @Override
+ public String getCharContent(boolean ignoreEncodingErrors) {
+ return "class Test {" +
+ " void test() {\n" +
+ " E e = E.A;\n" +
+ " Object o = E.A;\n" +
+ " switch (e) {\n" +
+ " case A:\n" +
+ " return;\n" +
+ " case B:\n" +
+ " test();\n" +
+ " E ee = null;\n" +
+ " break;\n" +
+ " }\n" +
+ " }\n" +
+ " enum E {A, B}\n" +
+ "}";
+ }
+ }
+ Context ctx = new Context();
+ TestAnalyzer.preRegister(ctx);
+ JavacTask t = (JavacTask) c.getTask(null, fm, null, null, null,
+ List.of(new MyFileObject()), ctx);
+ CompilationUnitTree cut = t.parse().iterator().next();
+ t.analyze();
+
+ new TreePathScanner<Void, Void>() {
+ @Override
+ public Void visitMethodInvocation(MethodInvocationTree node, Void p) {
+ Trees.instance(t).getScope(getCurrentPath());
+ return super.visitMethodInvocation(node, p);
+ }
+ }.scan(cut, null);
+ }
+ }
+
+ void testMemberRefs() throws IOException {
+ JavacTool c = JavacTool.create();
+ try (StandardJavaFileManager fm = c.getStandardFileManager(null, null, null)) {
+ class MyFileObject extends SimpleJavaFileObject {
+ MyFileObject() {
+ super(URI.create("myfo:///Test.java"), SOURCE);
+ }
+ @Override
+ public String getCharContent(boolean ignoreEncodingErrors) {
+ return "class Test {" +
+ " void test() {\n" +
+ " Test t = this;\n" +
+ " Runnable r1 = t::test;\n" +
+ " Runnable r2 = true ? t::test : t::test;\n" +
+ " c(t::test);\n" +
+ " c(true ? t::test : t::test);\n" +
+ " }\n" +
+ " void c(Runnable r) {}\n" +
+ "}";
+ }
+ }
+ Context ctx = new Context();
+ TestAnalyzer.preRegister(ctx);
+ JavacTask t = (JavacTask) c.getTask(null, fm, null, null, null,
+ List.of(new MyFileObject()), ctx);
+ CompilationUnitTree cut = t.parse().iterator().next();
+ t.analyze();
+
+ new TreePathScanner<Void, Void>() {
+ @Override
+ public Void visitConditionalExpression(ConditionalExpressionTree node, Void p) {
+ Trees.instance(t).getScope(new TreePath(getCurrentPath(), node.getCondition()));
+ return super.visitConditionalExpression(node, p);
+ }
+
+ @Override
+ public Void visitBlock(BlockTree node, Void p) {
+ Trees.instance(t).getScope(getCurrentPath());
+ return super.visitBlock(node, p);
+ }
+ }.scan(cut, null);
+ }
+ }
+
+ void testAnnotations() throws IOException {
+ JavacTool c = JavacTool.create();
+ try (StandardJavaFileManager fm = c.getStandardFileManager(null, null, null)) {
+ class MyFileObject extends SimpleJavaFileObject {
+ MyFileObject() {
+ super(URI.create("myfo:///Test.java"), SOURCE);
+ }
+ @Override
+ public String getCharContent(boolean ignoreEncodingErrors) {
+ return "class Test {" +
+ " void test() {\n" +
+ " new Object() {\n" +
+ " @A\n" +
+ " public String t() { return null; }\n" +
+ " };\n" +
+ " }\n" +
+ " @interface A {}\n" +
+ "}";
+ }
+ }
+ Context ctx = new Context();
+ TestAnalyzer.preRegister(ctx);
+ JavacTask t = (JavacTask) c.getTask(null, fm, null, null, null,
+ List.of(new MyFileObject()), ctx);
+ CompilationUnitTree cut = t.parse().iterator().next();
+ t.analyze();
+
+ new TreePathScanner<Void, Void>() {
+ @Override
+ public Void visitIdentifier(IdentifierTree node, Void p) {
+ if (node.getName().contentEquals("A")) {
+ Trees.instance(t).getScope(getCurrentPath());
+ }
+ return super.visitIdentifier(node, p);
+ }
+
+ @Override
+ public Void visitMethod(MethodTree node, Void p) {
+ super.visitMethod(node, p);
+ if (node.getReturnType() != null) {
+ Trees.instance(t).getScope(new TreePath(getCurrentPath(), node.getReturnType()));
+ }
+ return null;
+ }
+ }.scan(cut, null);
+ }
+ }
+
+ void testAnnotationsLazy() throws IOException {
+ JavacTool c = JavacTool.create();
+ try (StandardJavaFileManager fm = c.getStandardFileManager(null, null, null)) {
+ class MyFileObject extends SimpleJavaFileObject {
+ MyFileObject() {
+ super(URI.create("myfo:///Test.java"), SOURCE);
+ }
+ @Override
+ public String getCharContent(boolean ignoreEncodingErrors) {
+ return "import java.lang.annotation.*;\n" +
+ "\n" +
+ "class ClassA {\n" +
+ " Object o = ClassB.lcv;\n" +
+ "}\n" +
+ "\n" +
+ "class ClassB {\n" +
+ " static final String[] lcv = new @TA String[0];\n" +
+ "}\n" +
+ "\n" +
+ "class ClassC {\n" +
+ " static final Object o = (@TA Object) null;\n" +
+ "}\n" +
+ "\n" +
+ "@Target({ElementType.TYPE_USE, ElementType.TYPE_PARAMETER})\n" +
+ "@interface TA {}\n";
+ }
+ }
+ Context ctx = new Context();
+ TestAnalyzer.preRegister(ctx);
+ JavacTask t = (JavacTask) c.getTask(null, fm, null, null, null,
+ List.of(new MyFileObject()), ctx);
+ t.addTaskListener(new TaskListener() {
+ @Override
+ public void finished(TaskEvent e) {
+ if (e.getKind() == TaskEvent.Kind.ANALYZE) {
+ new TreePathScanner<Void, Void>() {
+ @Override
+ public Void scan(Tree tree, Void p) {
+ if (tree != null) {
+ Trees.instance(t).getScope(new TreePath(getCurrentPath(), tree));
+ }
+ return super.scan(tree, p);
+ }
+ }.scan(Trees.instance(t).getPath(e.getTypeElement()), null);
+ }
+ }
+ });
+
+ t.call();
+ }
+ }
+
+ void testCircular() throws IOException {
+ JavacTool c = JavacTool.create();
+ try (StandardJavaFileManager fm = c.getStandardFileManager(null, null, null)) {
+ class MyFileObject extends SimpleJavaFileObject {
+ MyFileObject() {
+ super(URI.create("myfo:///Test.java"), SOURCE);
+ }
+ @Override
+ public String getCharContent(boolean ignoreEncodingErrors) {
+ return "class Test extends Test {" +
+ " {\n" +
+ " int i;\n" +
+ " }\n" +
+ "}";
+ }
+ }
+ Context ctx = new Context();
+ TestAnalyzer.preRegister(ctx);
+ JavacTask t = (JavacTask) c.getTask(null, fm, null, null, null,
+ List.of(new MyFileObject()), ctx);
+ CompilationUnitTree cut = t.parse().iterator().next();
+ t.analyze();
+
+ new TreePathScanner<Void, Void>() {
+ @Override
+ public Void visitBlock(BlockTree node, Void p) {
+ Trees.instance(t).getScope(getCurrentPath());
+ return super.visitBlock(node, p);
+ }
+ }.scan(cut, null);
+ }
+ }
+
}
-