From 7a006b3608743dfdc6c738d8417e42541178430e Mon Sep 17 00:00:00 2001 From: Jan Lahoda Date: Mon, 4 Nov 2019 10:58:14 +0100 Subject: [PATCH] 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 --- .../com/sun/tools/javac/api/JavacTrees.java | 30 +-- .../com/sun/tools/javac/comp/Analyzer.java | 4 +- .../com/sun/tools/javac/comp/Annotate.java | 29 ++- .../com/sun/tools/javac/comp/Attr.java | 39 +-- .../sun/tools/javac/comp/DeferredAttr.java | 35 ++- .../tools/javac/api/TestGetScopeResult.java | 239 +++++++++++++++++- 6 files changed, 310 insertions(+), 66 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/api/JavacTrees.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/api/JavacTrees.java index d1d0b4d1209..52b289a434d 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/api/JavacTrees.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/api/JavacTrees.java @@ -951,34 +951,20 @@ public class JavacTrees extends DocTrees { private Env attribStatToTree(JCTree stat, Envenv, JCTree tree, Map copiedClasses) { - JavaFileObject prev = log.useSource(env.toplevel.sourcefile); - Log.DiagnosticHandler diagHandler = new Log.DiscardDiagnosticHandler(log); - try { - Env result = attr.attribStatToTree(stat, env, tree); + Env 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 attribExprToTree(JCExpression expr, Envenv, JCTree tree, Map copiedClasses) { - JavaFileObject prev = log.useSource(env.toplevel.sourcefile); - Log.DiagnosticHandler diagHandler = new Log.DiscardDiagnosticHandler(log); - try { - Env result = attr.attribExprToTree(expr, env, tree); + Env 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 diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Analyzer.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Analyzer.java index 4305007c0e2..a2d4e4727d1 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Analyzer.java +++ b/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: " + diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Annotate.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Annotate.java index 091a9e8026d..2d3f8999b28 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Annotate.java +++ b/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 q; + private final ListBuffer validateQ; + private final ListBuffer typesQ; + private final ListBuffer afterTypesQ; + + public Queues() { + this(new ListBuffer(), new ListBuffer(), new ListBuffer(), new ListBuffer()); + } + + public Queues(ListBuffer q, ListBuffer validateQ, ListBuffer typesQ, ListBuffer afterTypesQ) { + this.q = q; + this.validateQ = validateQ; + this.typesQ = typesQ; + this.afterTypesQ = afterTypesQ; + } + } } diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java index c1ca4078310..724debae5f9 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java +++ b/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 attribExprToTree(JCTree expr, Env env, JCTree tree) { - breakTree = tree; - JavaFileObject prev = log.useSource(env.toplevel.sourcefile); - EnumSet 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; + return attribToTree(expr, env, tree, unknownExprInfo); } public Env attribStatToTree(JCTree stmt, Env env, JCTree tree) { + return attribToTree(stmt, env, tree, statInfo); + } + + private Env attribToTree(JCTree root, Env env, JCTree tree, ResultInfo resultInfo) { breakTree = tree; JavaFileObject prev = log.useSource(env.toplevel.sourcefile); - EnumSet analyzerModes = EnumSet.copyOf(analyzer.analyzerModes); try { - analyzer.analyzerModes.clear(); - attribStat(stmt, env); + deferredAttr.attribSpeculative(root, env, resultInfo, + 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(); diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java index e2c72fc559f..ce31de68847 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java +++ b/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 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 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 env, ResultInfo resultInfo, LocalCacheContext localCache) { return attribSpeculative(tree, env, resultInfo, treeCopier, - newTree->new DeferredDiagnosticHandler(log), AttributionMode.SPECULATIVE, localCache); + null, AttributionMode.SPECULATIVE, localCache); } JCTree attribSpeculative(JCTree tree, Env env, ResultInfo resultInfo, TreeCopier deferredCopier, - Function diagHandlerCreator, AttributionMode attributionMode, + Supplier diagHandlerCreator, AttributionMode attributionMode, LocalCacheContext localCache) { final JCTree newTree = deferredCopier.copy(tree); - Env 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. + */ + JCTree attribSpeculative(JCTree tree, Env env, ResultInfo resultInfo, + Supplier diagHandlerCreator, AttributionMode attributionMode, + LocalCacheContext localCache) { + Env 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(); diff --git a/test/langtools/tools/javac/api/TestGetScopeResult.java b/test/langtools/tools/javac/api/TestGetScopeResult.java index 88201a3bf65..b698271d4bf 100644 --- a/test/langtools/tools/javac/api/TestGetScopeResult.java +++ b/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() { + @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() { + @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() { + @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() { + @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() { + @Override + public Void visitBlock(BlockTree node, Void p) { + Trees.instance(t).getScope(getCurrentPath()); + return super.visitBlock(node, p); + } + }.scan(cut, null); + } + } + +}