changeset 2488:b7d8b71e1658

8022765: Compiler crashes with exception on wrong usage of an annotation. Summary: Error recovery for incorrect annotation attribute values - ensure the values are always attributed appropriately Reviewed-by: jfranck, jjg
author jlahoda
date Fri, 27 Sep 2013 17:28:31 +0200
parents 16194509e483
children 2c24a04ebfb4
files src/share/classes/com/sun/tools/javac/comp/Annotate.java src/share/classes/com/sun/tools/javac/comp/Attr.java test/tools/javac/annotations/neg/8022765/T8022765.java test/tools/javac/annotations/neg/8022765/T8022765.out test/tools/javac/annotations/neg/8022765/VerifyAnnotationsAttributed.java
diffstat 5 files changed, 327 insertions(+), 30 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/classes/com/sun/tools/javac/comp/Annotate.java	Fri Sep 27 10:24:56 2013 +0100
+++ b/src/share/classes/com/sun/tools/javac/comp/Annotate.java	Fri Sep 27 17:28:31 2013 +0200
@@ -310,6 +310,7 @@
     Attribute enterAttributeValue(Type expected,
                                   JCExpression tree,
                                   Env<AttrContext> env) {
+        Type original = expected;
         //first, try completing the attribution value sym - if a completion
         //error is thrown, we should recover gracefully, and display an
         //ordinary resolution diagnostic.
@@ -317,7 +318,54 @@
             expected.tsym.complete();
         } catch(CompletionFailure e) {
             log.error(tree.pos(), "cant.resolve", Kinds.kindName(e.sym), e.sym);
-            return new Attribute.Error(expected);
+            expected = syms.errType;
+        }
+        if (expected.hasTag(ARRAY)) {
+            if (!tree.hasTag(NEWARRAY)) {
+                tree = make.at(tree.pos).
+                    NewArray(null, List.<JCExpression>nil(), List.of(tree));
+            }
+            JCNewArray na = (JCNewArray)tree;
+            if (na.elemtype != null) {
+                log.error(na.elemtype.pos(), "new.not.allowed.in.annotation");
+            }
+            ListBuffer<Attribute> buf = new ListBuffer<Attribute>();
+            for (List<JCExpression> l = na.elems; l.nonEmpty(); l=l.tail) {
+                buf.append(enterAttributeValue(types.elemtype(expected),
+                                               l.head,
+                                               env));
+            }
+            na.type = expected;
+            return new Attribute.
+                Array(expected, buf.toArray(new Attribute[buf.length()]));
+        }
+        if (tree.hasTag(NEWARRAY)) { //error recovery
+            if (!expected.isErroneous())
+                log.error(tree.pos(), "annotation.value.not.allowable.type");
+            JCNewArray na = (JCNewArray)tree;
+            if (na.elemtype != null) {
+                log.error(na.elemtype.pos(), "new.not.allowed.in.annotation");
+            }
+            for (List<JCExpression> l = na.elems; l.nonEmpty(); l=l.tail) {
+                enterAttributeValue(syms.errType,
+                                    l.head,
+                                    env);
+            }
+            return new Attribute.Error(original);
+        }
+        if ((expected.tsym.flags() & Flags.ANNOTATION) != 0) {
+            if (tree.hasTag(ANNOTATION)) {
+                return enterAnnotation((JCAnnotation)tree, expected, env);
+            } else {
+                log.error(tree.pos(), "annotation.value.must.be.annotation");
+                expected = syms.errType;
+            }
+        }
+        if (tree.hasTag(ANNOTATION)) { //error recovery
+            if (!expected.isErroneous())
+                log.error(tree.pos(), "annotation.not.valid.for.type", expected);
+            enterAnnotation((JCAnnotation)tree, syms.errType, env);
+            return new Attribute.Error(original);
         }
         if (expected.isPrimitive() || types.isSameType(expected, syms.stringType)) {
             Type result = attr.attribExpr(tree, env, expected);
@@ -353,33 +401,6 @@
             return new Attribute.Class(types,
                                        (((JCFieldAccess) tree).selected).type);
         }
-        if ((expected.tsym.flags() & Flags.ANNOTATION) != 0) {
-            if (!tree.hasTag(ANNOTATION)) {
-                log.error(tree.pos(), "annotation.value.must.be.annotation");
-                expected = syms.errorType;
-            }
-            return enterAnnotation((JCAnnotation)tree, expected, env);
-        }
-        if (expected.hasTag(ARRAY)) { // should really be isArray()
-            if (!tree.hasTag(NEWARRAY)) {
-                tree = make.at(tree.pos).
-                    NewArray(null, List.<JCExpression>nil(), List.of(tree));
-            }
-            JCNewArray na = (JCNewArray)tree;
-            if (na.elemtype != null) {
-                log.error(na.elemtype.pos(), "new.not.allowed.in.annotation");
-                return new Attribute.Error(expected);
-            }
-            ListBuffer<Attribute> buf = new ListBuffer<Attribute>();
-            for (List<JCExpression> l = na.elems; l.nonEmpty(); l=l.tail) {
-                buf.append(enterAttributeValue(types.elemtype(expected),
-                                               l.head,
-                                               env));
-            }
-            na.type = expected;
-            return new Attribute.
-                Array(expected, buf.toArray(new Attribute[buf.length()]));
-        }
         if (expected.hasTag(CLASS) &&
             (expected.tsym.flags() & Flags.ENUM) != 0) {
             attr.attribExpr(tree, env, expected);
@@ -394,6 +415,7 @@
             VarSymbol enumerator = (VarSymbol) sym;
             return new Attribute.Enum(expected, enumerator);
         }
+        //error recovery:
         if (!expected.isErroneous())
             log.error(tree.pos(), "annotation.value.not.allowable.type");
         return new Attribute.Error(attr.attribExpr(tree, env, expected));
--- a/src/share/classes/com/sun/tools/javac/comp/Attr.java	Fri Sep 27 10:24:56 2013 +0100
+++ b/src/share/classes/com/sun/tools/javac/comp/Attr.java	Fri Sep 27 17:28:31 2013 +0200
@@ -4058,8 +4058,7 @@
     }
 
     public void visitAnnotation(JCAnnotation tree) {
-        log.error(tree.pos(), "annotation.not.valid.for.type", pt());
-        result = tree.type = syms.errType;
+        Assert.error("should be handled in Annotate");
     }
 
     public void visitAnnotatedType(JCAnnotatedType tree) {
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/tools/javac/annotations/neg/8022765/T8022765.java	Fri Sep 27 17:28:31 2013 +0200
@@ -0,0 +1,141 @@
+/**
+ * @test /nodynamiccopyright/
+ * @bug 8022765
+ * @summary javac should not crash for incorrect attribute values
+ * @build VerifyAnnotationsAttributed
+ * @run main VerifyAnnotationsAttributed T8022765.java
+ * @compile/fail/ref=T8022765.out -XDrawDiagnostics T8022765.java
+ */
+@Ann(@Override)
+@Primitive(@Override)
+@Str(@Override)
+@En(@Override)
+@ArrAnn(@Override)
+@ArrPrimitive(@Override)
+@ArrStr(@Override)
+@ArrEn(@Override)
+class AnnC { }
+
+class PrimitiveC {
+    private static final int C = 1;
+    @Ann(C)
+    @Primitive(C)
+    @Str(C)
+    @En(C)
+    @ArrAnn(C)
+    @ArrPrimitive(C)
+    @ArrStr(C)
+    @ArrEn(C)
+    class I {
+    }
+}
+
+class StringC {
+
+    private static final String C = "";
+
+    @Ann(C)
+    @Primitive(C)
+    @Str(C)
+    @En(C)
+    @ArrAnn(C)
+    @ArrPrimitive(C)
+    @ArrStr(C)
+    @ArrEn(C)
+    class I {
+    }
+}
+
+@Ann(E.A)
+@Primitive(E.A)
+@Str(E.A)
+@En(E.A)
+@ArrAnn(E.A)
+@ArrPrimitive(E.A)
+@ArrStr(E.A)
+@ArrEn(E.A)
+class EnC { }
+
+@Ann({@Override})
+@Primitive({@Override})
+@Str({@Override})
+@En({@Override})
+@ArrAnn({@Override})
+@ArrPrimitive({@Override})
+@ArrStr({@Override})
+@ArrEn({@Override})
+class ArrAnnC { }
+
+class ArrPrimitiveC {
+    private static final int C = 1;
+    @Ann({C})
+    @Primitive({C})
+    @Str({C})
+    @En({C})
+    @ArrAnn({C})
+    @ArrPrimitive({C})
+    @ArrStr({C})
+    @ArrEn({C})
+    class I {
+    }
+}
+
+class ArrStringC {
+    private static final String C = "";
+    @Ann({C})
+    @Primitive({C})
+    @Str({C})
+    @En({C})
+    @ArrAnn({C})
+    @ArrPrimitive({C})
+    @ArrStr({C})
+    @ArrEn({C})
+    class I {
+    }
+}
+
+@Ann({E.A})
+@Primitive({E.A})
+@Str({E.A})
+@En({E.A})
+@ArrAnn({E.A})
+@ArrPrimitive({E.A})
+@ArrStr({E.A})
+@ArrEn({E.A})
+class ArrEnC { }
+
+@interface Ann {
+    Override value();
+}
+
+@interface Primitive {
+    int value();
+}
+
+@interface Str {
+    String value();
+}
+
+@interface En {
+    E value();
+}
+
+@interface ArrAnn {
+    Override[] value();
+}
+
+@interface ArrPrimitive {
+    int[] value();
+}
+
+@interface ArrStr {
+    String[] value();
+}
+
+@interface ArrEn {
+    E[] value();
+}
+
+enum E {
+    A;
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/tools/javac/annotations/neg/8022765/T8022765.out	Fri Sep 27 17:28:31 2013 +0200
@@ -0,0 +1,57 @@
+T8022765.java:10:12: compiler.err.annotation.not.valid.for.type: int
+T8022765.java:11:6: compiler.err.annotation.not.valid.for.type: java.lang.String
+T8022765.java:12:5: compiler.err.annotation.not.valid.for.type: E
+T8022765.java:14:15: compiler.err.annotation.not.valid.for.type: int
+T8022765.java:15:9: compiler.err.annotation.not.valid.for.type: java.lang.String
+T8022765.java:16:8: compiler.err.annotation.not.valid.for.type: E
+T8022765.java:21:10: compiler.err.annotation.value.must.be.annotation
+T8022765.java:23:10: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: int, java.lang.String)
+T8022765.java:24:9: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: int, E)
+T8022765.java:25:13: compiler.err.annotation.value.must.be.annotation
+T8022765.java:27:13: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: int, java.lang.String)
+T8022765.java:28:12: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: int, E)
+T8022765.java:37:10: compiler.err.annotation.value.must.be.annotation
+T8022765.java:38:16: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.lang.String, int)
+T8022765.java:40:9: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.lang.String, E)
+T8022765.java:41:13: compiler.err.annotation.value.must.be.annotation
+T8022765.java:42:19: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.lang.String, int)
+T8022765.java:44:12: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.lang.String, E)
+T8022765.java:49:7: compiler.err.annotation.value.must.be.annotation
+T8022765.java:50:13: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: E, int)
+T8022765.java:51:7: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: E, java.lang.String)
+T8022765.java:53:10: compiler.err.annotation.value.must.be.annotation
+T8022765.java:54:16: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: E, int)
+T8022765.java:55:10: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: E, java.lang.String)
+T8022765.java:59:6: compiler.err.annotation.value.not.allowable.type
+T8022765.java:60:12: compiler.err.annotation.value.not.allowable.type
+T8022765.java:61:6: compiler.err.annotation.value.not.allowable.type
+T8022765.java:62:5: compiler.err.annotation.value.not.allowable.type
+T8022765.java:64:16: compiler.err.annotation.not.valid.for.type: int
+T8022765.java:65:10: compiler.err.annotation.not.valid.for.type: java.lang.String
+T8022765.java:66:9: compiler.err.annotation.not.valid.for.type: E
+T8022765.java:71:10: compiler.err.annotation.value.not.allowable.type
+T8022765.java:72:16: compiler.err.annotation.value.not.allowable.type
+T8022765.java:73:10: compiler.err.annotation.value.not.allowable.type
+T8022765.java:74:9: compiler.err.annotation.value.not.allowable.type
+T8022765.java:75:14: compiler.err.annotation.value.must.be.annotation
+T8022765.java:77:14: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: int, java.lang.String)
+T8022765.java:78:13: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: int, E)
+T8022765.java:85:10: compiler.err.annotation.value.not.allowable.type
+T8022765.java:86:16: compiler.err.annotation.value.not.allowable.type
+T8022765.java:87:10: compiler.err.annotation.value.not.allowable.type
+T8022765.java:88:9: compiler.err.annotation.value.not.allowable.type
+T8022765.java:89:14: compiler.err.annotation.value.must.be.annotation
+T8022765.java:90:20: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.lang.String, int)
+T8022765.java:92:13: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.lang.String, E)
+T8022765.java:97:6: compiler.err.annotation.value.not.allowable.type
+T8022765.java:97:8: compiler.err.attribute.value.must.be.constant
+T8022765.java:98:12: compiler.err.annotation.value.not.allowable.type
+T8022765.java:98:14: compiler.err.attribute.value.must.be.constant
+T8022765.java:99:6: compiler.err.annotation.value.not.allowable.type
+T8022765.java:99:8: compiler.err.attribute.value.must.be.constant
+T8022765.java:100:5: compiler.err.annotation.value.not.allowable.type
+T8022765.java:100:7: compiler.err.attribute.value.must.be.constant
+T8022765.java:101:11: compiler.err.annotation.value.must.be.annotation
+T8022765.java:102:17: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: E, int)
+T8022765.java:103:11: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: E, java.lang.String)
+56 errors
\ No newline at end of file
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/tools/javac/annotations/neg/8022765/VerifyAnnotationsAttributed.java	Fri Sep 27 17:28:31 2013 +0200
@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2013, 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.
+ */
+import com.sun.source.tree.CompilationUnitTree;
+import com.sun.source.tree.IdentifierTree;
+import com.sun.source.tree.MemberSelectTree;
+import com.sun.source.util.JavacTask;
+import com.sun.source.util.TreePathScanner;
+import com.sun.source.util.Trees;
+import com.sun.tools.javac.api.JavacTool;
+import com.sun.tools.javac.file.JavacFileManager;
+import java.io.File;
+import java.io.IOException;
+import java.net.URISyntaxException;
+import java.util.Collections;
+import javax.lang.model.element.Element;
+import javax.lang.model.element.ElementKind;
+
+public class VerifyAnnotationsAttributed {
+    public static void main(String... args) throws IOException, URISyntaxException {
+        if (args.length != 1) throw new IllegalStateException("Must provide class name!");
+        File testSrc = new File(System.getProperty("test.src"));
+        File testFile = new File(testSrc, args[0]);
+        if (!testFile.canRead()) throw new IllegalStateException("Cannot read the test source");
+        JavacFileManager fm = JavacTool.create().getStandardFileManager(null, null, null);
+        JavacTask task = JavacTool.create().getTask(null,
+                                                    fm,
+                                                    null,
+                                                    Collections.<String>emptyList(),
+                                                    null,
+                                                    fm.getJavaFileObjects(testFile));
+        final Trees trees = Trees.instance(task);
+        final CompilationUnitTree cut = task.parse().iterator().next();
+        task.analyze();
+
+        //ensure all the annotation attributes are annotated meaningfully
+        //all the attributes in the test file should contain either an identifier
+        //or a select, so only checking those for a reasonable Element/Symbol.
+        new TreePathScanner<Void, Void>() {
+            @Override
+            public Void visitIdentifier(IdentifierTree node, Void p) {
+                verifyAttributedMeaningfully();
+                return super.visitIdentifier(node, p);
+            }
+            @Override
+            public Void visitMemberSelect(MemberSelectTree node, Void p) {
+                verifyAttributedMeaningfully();
+                return super.visitMemberSelect(node, p);
+            }
+            private void verifyAttributedMeaningfully() {
+                Element el = trees.getElement(getCurrentPath());
+
+                if (el == null || el.getKind() == ElementKind.OTHER) {
+                    throw new IllegalStateException("Not attributed properly: " + getCurrentPath().getParentPath().getLeaf());
+                }
+            }
+        }.scan(cut, null);
+    }
+}