8361355: Negative cases of Annotated.getAnnotationData implementations are broken

Doug Simon · Doug Simon · commit 5cf349c3b083 · 2025-07-04T07:37:20.000Z
Reviewed-by: never
diff --git a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotResolvedJavaFieldImpl.java b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotResolvedJavaFieldImpl.java
@@ -25,6 +25,9 @@
 import static jdk.internal.misc.Unsafe.ADDRESS_SIZE;
 import static jdk.vm.ci.hotspot.CompilerToVM.compilerToVM;
 import static jdk.vm.ci.hotspot.HotSpotJVMCIRuntime.runtime;
+import static jdk.vm.ci.hotspot.HotSpotResolvedJavaType.checkAreAnnotations;
+import static jdk.vm.ci.hotspot.HotSpotResolvedJavaType.checkIsAnnotation;
+import static jdk.vm.ci.hotspot.HotSpotResolvedJavaType.getFirstAnnotationOrNull;
 import static jdk.vm.ci.hotspot.HotSpotVMConfig.config;
 import static jdk.vm.ci.hotspot.UnsafeAccess.UNSAFE;
 
@@ -234,15 +237,19 @@ public JavaConstant getConstantValue() {
     @Override
     public AnnotationData getAnnotationData(ResolvedJavaType annotationType) {
         if (!hasAnnotations()) {
+            checkIsAnnotation(annotationType);
             return null;
         }
-        return getAnnotationData0(annotationType).get(0);
+        return getFirstAnnotationOrNull(getAnnotationData0(annotationType));
     }
 
     @Override
     public List<AnnotationData> getAnnotationData(ResolvedJavaType type1, ResolvedJavaType type2, ResolvedJavaType... types) {
+        checkIsAnnotation(type1);
+        checkIsAnnotation(type2);
+        checkAreAnnotations(types);
         if (!hasAnnotations()) {
-            return Collections.emptyList();
+            return List.of();
         }
         return getAnnotationData0(AnnotationDataDecoder.asArray(type1, type2, types));
     }
diff --git a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotResolvedJavaMethodImpl.java b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotResolvedJavaMethodImpl.java
@@ -28,6 +28,9 @@
 import static jdk.vm.ci.hotspot.HotSpotModifiers.SYNTHETIC;
 import static jdk.vm.ci.hotspot.HotSpotModifiers.VARARGS;
 import static jdk.vm.ci.hotspot.HotSpotModifiers.jvmMethodModifiers;
+import static jdk.vm.ci.hotspot.HotSpotResolvedJavaType.checkAreAnnotations;
+import static jdk.vm.ci.hotspot.HotSpotResolvedJavaType.checkIsAnnotation;
+import static jdk.vm.ci.hotspot.HotSpotResolvedJavaType.getFirstAnnotationOrNull;
 import static jdk.vm.ci.hotspot.HotSpotVMConfig.config;
 import static jdk.vm.ci.hotspot.UnsafeAccess.UNSAFE;
 
@@ -775,15 +778,19 @@ public int methodIdnum() {
     @Override
     public AnnotationData getAnnotationData(ResolvedJavaType type) {
         if (!hasAnnotations()) {
+            checkIsAnnotation(type);
             return null;
         }
-        return getAnnotationData0(type).get(0);
+        return getFirstAnnotationOrNull(getAnnotationData0(type));
     }
 
     @Override
     public List<AnnotationData> getAnnotationData(ResolvedJavaType type1, ResolvedJavaType type2, ResolvedJavaType... types) {
+        checkIsAnnotation(type1);
+        checkIsAnnotation(type2);
+        checkAreAnnotations(types);
         if (!hasAnnotations()) {
-            return Collections.emptyList();
+            return List.of();
         }
         return getAnnotationData0(AnnotationDataDecoder.asArray(type1, type2, types));
     }
diff --git a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotResolvedJavaType.java b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotResolvedJavaType.java
@@ -22,6 +22,9 @@
  */
 package jdk.vm.ci.hotspot;
 
+import java.util.List;
+
+import jdk.vm.ci.meta.AnnotationData;
 import jdk.vm.ci.meta.JavaConstant;
 import jdk.vm.ci.meta.ResolvedJavaType;
 
@@ -61,4 +64,20 @@ public final HotSpotResolvedObjectType getArrayClass() {
      * @return {@code true} if this type is being initialized
      */
     abstract boolean isBeingInitialized();
+
+    static void checkIsAnnotation(ResolvedJavaType type) {
+        if (!type.isAnnotation()) {
+            throw new IllegalArgumentException(type.toJavaName() + " is not an annotation interface");
+        }
+    }
+
+    static void checkAreAnnotations(ResolvedJavaType... types) {
+        for (ResolvedJavaType type : types) {
+            checkIsAnnotation(type);
+        }
+    }
+
+    static AnnotationData getFirstAnnotationOrNull(List<AnnotationData> list) {
+        return list.isEmpty() ? null : list.get(0);
+    }
 }
diff --git a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotResolvedObjectTypeImpl.java b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotResolvedObjectTypeImpl.java
@@ -1120,15 +1120,19 @@ public boolean isCloneableWithAllocation() {
     @Override
     public AnnotationData getAnnotationData(ResolvedJavaType annotationType) {
         if (!mayHaveAnnotations(true)) {
+            checkIsAnnotation(annotationType);
             return null;
         }
-        return getAnnotationData0(annotationType).get(0);
+        return getFirstAnnotationOrNull(getAnnotationData0(annotationType));
     }
 
     @Override
     public List<AnnotationData> getAnnotationData(ResolvedJavaType type1, ResolvedJavaType type2, ResolvedJavaType... types) {
         if (!mayHaveAnnotations(true)) {
-            return Collections.emptyList();
+            checkIsAnnotation(type1);
+            checkIsAnnotation(type2);
+            checkAreAnnotations(types);
+            return List.of();
         }
         return getAnnotationData0(AnnotationDataDecoder.asArray(type1, type2, types));
     }
diff --git a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotResolvedPrimitiveType.java b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotResolvedPrimitiveType.java
@@ -327,12 +327,15 @@ JavaConstant getJavaMirror() {
 
     @Override
     public AnnotationData getAnnotationData(ResolvedJavaType type) {
+        checkIsAnnotation(type);
         return null;
     }
 
     @Override
     public List<AnnotationData> getAnnotationData(ResolvedJavaType type1, ResolvedJavaType type2, ResolvedJavaType... types) {
-        return Collections.emptyList();
+        checkIsAnnotation(type1);
+        checkIsAnnotation(type2);
+        checkAreAnnotations(types);
+        return List.of();
     }
-
 }
diff --git a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ResolvedJavaType.java b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/meta/ResolvedJavaType.java
@@ -50,6 +50,15 @@ public interface ResolvedJavaType extends JavaType, ModifiersProvider, Annotated
      */
     AssumptionResult<Boolean> hasFinalizableSubclass();
 
+    /**
+     * Returns true if this type represents an annotation interface.
+     *
+     * @return {@code true} if this type represents an annotation interface
+     */
+    default boolean isAnnotation() {
+        return (getModifiers() & java.lang.reflect.AccessFlag.ANNOTATION.mask()) != 0;
+    }
+
     /**
      * Checks whether this type is an interface.
      *
diff --git a/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java b/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java
@@ -158,6 +158,16 @@ public void findInstanceFieldWithOffsetTest() {
         }
     }
 
+    @Test
+    public void isAnnotationTest() {
+        for (Class<?> c : classes) {
+            ResolvedJavaType type = metaAccess.lookupJavaType(c);
+            boolean expected = c.isAnnotation();
+            boolean actual = type.isAnnotation();
+            assertEquals(expected, actual);
+        }
+    }
+
     @Test
     public void isInterfaceTest() {
         for (Class<?> c : classes) {
@@ -1267,25 +1277,59 @@ private static boolean isSignaturePolymorphic(ResolvedJavaMethod method) {
         return method.getAnnotation(SIGNATURE_POLYMORPHIC_CLASS) != null;
     }
 
+    private static void getAnnotationDataExpectedToFail(Annotated annotated, ResolvedJavaType... annotationTypes) {
+        try {
+            if (annotationTypes.length == 1) {
+                annotated.getAnnotationData(annotationTypes[0]);
+            } else {
+                var tail = Arrays.copyOfRange(annotationTypes, 2, annotationTypes.length);
+                annotated.getAnnotationData(annotationTypes[0], annotationTypes[1], tail);
+            }
+            String s = Stream.of(annotationTypes).map(ResolvedJavaType::toJavaName).collect(Collectors.joining(", "));
+            throw new AssertionError("Expected IllegalArgumentException for retrieving (" + s + " from " + annotated);
+        } catch (IllegalArgumentException iae) {
+            assertTrue(iae.getMessage(), iae.getMessage().contains("not an annotation interface"));
+        }
+    }
+
     /**
      * Tests that {@link AnnotationData} obtained from a {@link Class}, {@link Method} or
      * {@link Field} matches {@link AnnotatedElement#getAnnotations()} for the corresponding JVMCI
      * object.
      *
-     * @param annotated a {@link Class}, {@link Method} or {@link Field} object
+     * @param annotatedElement a {@link Class}, {@link Method} or {@link Field} object
      */
-    public static void getAnnotationDataTest(AnnotatedElement annotated) throws Exception {
-        testGetAnnotationData(annotated, List.of(annotated.getAnnotations()));
-    }
-
-    private static void testGetAnnotationData(AnnotatedElement annotated, List<Annotation> annotations) throws AssertionError {
+    public static void getAnnotationDataTest(AnnotatedElement annotatedElement) throws Exception {
+        Annotated annotated = toAnnotated(annotatedElement);
+        ResolvedJavaType objectType = metaAccess.lookupJavaType(Object.class);
+        ResolvedJavaType suppressWarningsType = metaAccess.lookupJavaType(SuppressWarnings.class);
+        getAnnotationDataExpectedToFail(annotated, objectType);
+        getAnnotationDataExpectedToFail(annotated, suppressWarningsType, objectType);
+        getAnnotationDataExpectedToFail(annotated, suppressWarningsType, suppressWarningsType, objectType);
+
+        // Check that querying a missing annotation returns null or an empty list
+        assertNull(annotated.getAnnotationData(suppressWarningsType));
+        List<AnnotationData> data = annotated.getAnnotationData(suppressWarningsType, suppressWarningsType);
+        assertTrue(data.toString(), data.isEmpty());
+        data = annotated.getAnnotationData(suppressWarningsType, suppressWarningsType, suppressWarningsType, suppressWarningsType);
+        assertTrue(data.toString(), data.isEmpty());
+
+        testGetAnnotationData(annotatedElement, annotated, List.of(annotatedElement.getAnnotations()));
+    }
+
+    private static void testGetAnnotationData(AnnotatedElement annotatedElement, Annotated annotated, List<Annotation> annotations) throws AssertionError {
+        ResolvedJavaType suppressWarningsType = metaAccess.lookupJavaType(SuppressWarnings.class);
         for (Annotation a : annotations) {
-            AnnotationData ad = toAnnotated(annotated).getAnnotationData(metaAccess.lookupJavaType(a.annotationType()));
+            var annotationType = metaAccess.lookupJavaType(a.annotationType());
+            AnnotationData ad = annotated.getAnnotationData(annotationType);
             assertAnnotationsEquals(a, ad);
 
             // Check that encoding/decoding produces a stable result
-            AnnotationData ad2 = toAnnotated(annotated).getAnnotationData(metaAccess.lookupJavaType(a.annotationType()));
+            AnnotationData ad2 = annotated.getAnnotationData(annotationType);
             assertEquals(ad, ad2);
+
+            List<AnnotationData> annotationData = annotated.getAnnotationData(annotationType, suppressWarningsType, suppressWarningsType);
+            assertEquals(1, annotationData.size());
         }
         if (annotations.size() < 2) {
             return;
@@ -1298,7 +1342,7 @@ private static void testGetAnnotationData(AnnotatedElement annotated, List<Annot
                             subList(2, i + 1).//
                             stream().map(a -> metaAccess.lookupJavaType(a.annotationType())).//
                             toArray(ResolvedJavaType[]::new);
-            List<AnnotationData> annotationData = toAnnotated(annotated).getAnnotationData(type1, type2, types);
+            List<AnnotationData> annotationData = annotated.getAnnotationData(type1, type2, types);
             assertEquals(2 + types.length, annotationData.size());
 
             for (int j = 0; j < annotationData.size(); j++) {

Original file line number	Diff line number	Diff line change
`@@ -1120,15 +1120,19 @@ public boolean isCloneableWithAllocation() {`
`1120`	`1120`	`@Override`
`1121`	`1121`	`public AnnotationData getAnnotationData(ResolvedJavaType annotationType) {`
`1122`	`1122`	`if (!mayHaveAnnotations(true)) {`
	`1123`	`+ checkIsAnnotation(annotationType);`
`1123`	`1124`	`return null;`
`1124`	`1125`	`}`
`1125`		`- return getAnnotationData0(annotationType).get(0);`
	`1126`	`+ return getFirstAnnotationOrNull(getAnnotationData0(annotationType));`
`1126`	`1127`	`}`
`1127`	`1128`
`1128`	`1129`	`@Override`
`1129`	`1130`	`public List<AnnotationData> getAnnotationData(ResolvedJavaType type1, ResolvedJavaType type2, ResolvedJavaType... types) {`
`1130`	`1131`	`if (!mayHaveAnnotations(true)) {`
`1131`		`- return Collections.emptyList();`
	`1132`	`+ checkIsAnnotation(type1);`
	`1133`	`+ checkIsAnnotation(type2);`
	`1134`	`+ checkAreAnnotations(types);`
	`1135`	`+ return List.of();`
`1132`	`1136`	`}`
`1133`	`1137`	`return getAnnotationData0(AnnotationDataDecoder.asArray(type1, type2, types));`
`1134`	`1138`	`}`
Original file line number	Diff line number	Diff line change
`@@ -327,12 +327,15 @@ JavaConstant getJavaMirror() {`
`327`	`327`
`328`	`328`	`@Override`
`329`	`329`	`public AnnotationData getAnnotationData(ResolvedJavaType type) {`
	`330`	`+ checkIsAnnotation(type);`
`330`	`331`	`return null;`
`331`	`332`	`}`
`332`	`333`
`333`	`334`	`@Override`
`334`	`335`	`public List<AnnotationData> getAnnotationData(ResolvedJavaType type1, ResolvedJavaType type2, ResolvedJavaType... types) {`
`335`		`- return Collections.emptyList();`
	`336`	`+ checkIsAnnotation(type1);`
	`337`	`+ checkIsAnnotation(type2);`
	`338`	`+ checkAreAnnotations(types);`
	`339`	`+ return List.of();`
`336`	`340`	`}`
`337`		`-`
`338`	`341`	`}`