From b25684f7661f12f858367ce6219fde0cc6de9987 Mon Sep 17 00:00:00 2001 From: Doug Simon Date: Thu, 3 Jul 2025 14:21:00 +0200 Subject: [PATCH] fixed negative cases in getAnnotationData --- .../hotspot/HotSpotResolvedJavaFieldImpl.java | 11 +++- .../HotSpotResolvedJavaMethodImpl.java | 11 +++- .../ci/hotspot/HotSpotResolvedJavaType.java | 19 ++++++ .../HotSpotResolvedObjectTypeImpl.java | 8 ++- .../hotspot/HotSpotResolvedPrimitiveType.java | 7 ++- .../jdk/vm/ci/meta/ResolvedJavaType.java | 9 +++ .../ci/runtime/test/TestResolvedJavaType.java | 62 ++++++++++++++++--- 7 files changed, 110 insertions(+), 17 deletions(-) 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 index 6eb4dfb9b0215..b75edb9df08c2 100644 --- 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 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 index 9440a719dd4b0..224e8b1a070e2 100644 --- 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 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 index fbb5a3fc9b5c4..566a660d53117 100644 --- 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 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 index 17eede6f49074..63ab5aea0fe7e 100644 --- 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 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 index b707906af0826..d77ab1f08a1f8 100644 --- 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 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 index 929a4713330ad..cdb24c5f1adbf 100644 --- 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 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 index ee48724b71d1b..19df327c867ad 100644 --- 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 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 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 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 = 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 metaAccess.lookupJavaType(a.annotationType())).// toArray(ResolvedJavaType[]::new); - List annotationData = toAnnotated(annotated).getAnnotationData(type1, type2, types); + List annotationData = annotated.getAnnotationData(type1, type2, types); assertEquals(2 + types.length, annotationData.size()); for (int j = 0; j < annotationData.size(); j++) {