8230847: Trees.getScope may crash when invoked for statement inside switch

More thoroughly avoiding side-effects when attributing (to) for Trees.getScope.

Reviewed-by: mcimadamore

src/jdk.compiler/share/classes/com/sun/tools/javac/api/JavacTrees.java

@@ -951,34 +951,20 @@ public class JavacTrees extends DocTrees {
 
     private Env<AttrContext> attribStatToTree(JCTree stat, Env<AttrContext>env,
                                               JCTree tree, Map<JCClassDecl, JCClassDecl> copiedClasses) {
-        JavaFileObject prev = log.useSource(env.toplevel.sourcefile);
+        Env<AttrContext> result = attr.attribStatToTree(stat, env, tree);
-        Log.DiagnosticHandler diagHandler = new Log.DiscardDiagnosticHandler(log);
-        try {
-            Env<AttrContext> result = attr.attribStatToTree(stat, env, tree);
 
-            enter.unenter(env.toplevel, stat);
+        fixLocalClassNames(copiedClasses, env);
-            fixLocalClassNames(copiedClasses, env);
+
-            return result;
+        return result;
-        } finally {
-            log.popDiagnosticHandler(diagHandler);
-            log.useSource(prev);
-        }
     }
 
     private Env<AttrContext> attribExprToTree(JCExpression expr, Env<AttrContext>env,
                                               JCTree tree, Map<JCClassDecl, JCClassDecl> copiedClasses) {
-        JavaFileObject prev = log.useSource(env.toplevel.sourcefile);
+        Env<AttrContext> result = attr.attribExprToTree(expr, env, tree);
-        Log.DiagnosticHandler diagHandler = new Log.DiscardDiagnosticHandler(log);
-        try {
-            Env<AttrContext> result = attr.attribExprToTree(expr, env, tree);
 
-            enter.unenter(env.toplevel, expr);
+        fixLocalClassNames(copiedClasses, env);
-            fixLocalClassNames(copiedClasses, env);
+
-            return result;
+        return result;
-        } finally {
-            log.popDiagnosticHandler(diagHandler);
-            log.useSource(prev);
-        }
     }
 
     /* Change the flatnames of the local and anonymous classes in the Scope to

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Analyzer.java

@@ -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 @@ public class Analyzer {
 
             //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: " +

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Annotate.java

@@ -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 class Annotate {
     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;
+        }
+    }
 }

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java

@@ -74,11 +74,8 @@ import static com.sun.tools.javac.code.Kinds.*;
 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,35 +391,20 @@ public class Attr extends JCTree.Visitor {
     }
 
     public Env<AttrContext> attribExprToTree(JCTree expr, Env<AttrContext> env, JCTree tree) {
-        breakTree = tree;
+        return attribToTree(expr, env, tree, unknownExprInfo);
-        JavaFileObject prev = log.useSource(env.toplevel.sourcefile);
-        EnumSet<AnalyzerMode> analyzerModes = EnumSet.copyOf(analyzer.analyzerModes);
-        try {
-            analyzer.analyzerModes.clear();
-            attribExpr(expr, 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;
     }
 
     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();
+            deferredAttr.attribSpeculative(root, env, resultInfo,
-            attribStat(stmt, env);
+                    null, DeferredAttr.AttributionMode.ANALYZER,
+                    argumentAttr.withLocalCacheContext());
         } catch (BreakAttr b) {
             return b.env;
         } catch (AssertionError ae) {
@@ -434,7 +416,6 @@ public class Attr extends JCTree.Visitor {
         } finally {
             breakTree = null;
             log.useSource(prev);
-            analyzer.analyzerModes.addAll(analyzerModes);
         }
         return env;
     }
@@ -1273,7 +1254,7 @@ public class Attr extends JCTree.Visitor {
     }
 
     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 @@ public class Attr extends JCTree.Visitor {
                     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();

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java

@@ -47,6 +47,7 @@ import com.sun.tools.javac.resources.CompilerProperties.Errors;
 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.LinkedHashSet;
 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 @@ import static com.sun.tools.javac.tree.JCTree.Tag.*;
 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 @@ public class DeferredAttr extends JCTree.Visitor {
 
     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 @@ public class DeferredAttr extends JCTree.Visitor {
      */
     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);
+            attr.attribTree(tree, speculativeEnv, resultInfo);
-            return newTree;
+            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();

test/langtools/tools/javac/api/TestGetScopeResult.java

@@ -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.SimpleJavaFileObject;
 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 class TestGetScopeResult {
     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 @@ public class TestGetScopeResult {
             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);
+        }
+    }
+
+}