Remove GuardedAnnotationAccess and DirectAnnotationAccess

Author: Christian Wimmer

sdk/mx.sdk/suite.py

@@ -437,7 +437,7 @@
           "org.graalvm.word",
           "org.graalvm.polyglot.impl to org.graalvm.truffle, com.oracle.graal.graal_enterprise",
           "org.graalvm.word.impl to jdk.internal.vm.compiler",
-          "org.graalvm.nativeimage.impl to org.graalvm.nativeimage.builder,org.graalvm.nativeimage.configure,com.oracle.svm.svm_enterprise",
+          "org.graalvm.nativeimage.impl to org.graalvm.nativeimage.base,org.graalvm.nativeimage.builder,org.graalvm.nativeimage.configure,com.oracle.svm.svm_enterprise",
           "org.graalvm.nativeimage.impl.clinit to org.graalvm.nativeimage.builder",
         ],
         "uses" : [

sdk/src/org.graalvm.nativeimage/src/org/graalvm/nativeimage/AnnotationAccess.java

@@ -44,7 +44,7 @@
 import java.lang.reflect.AnnotatedElement;
 import java.util.Arrays;
 
-import org.graalvm.nativeimage.impl.ImageBuildtimeCodeAnnotationAccessSupport;
+import org.graalvm.nativeimage.impl.AnnotationExtracter;
 
 /**
  * This class provides methods to query annotation information on {@link AnnotatedElement}s while
@@ -79,7 +79,7 @@ public final class AnnotationAccess {
      */
     public static boolean isAnnotationPresent(AnnotatedElement element, Class<? extends Annotation> annotationClass) {
         if (ImageInfo.inImageBuildtimeCode()) {
-            return ImageSingletons.lookup(ImageBuildtimeCodeAnnotationAccessSupport.class).isAnnotationPresent(element, annotationClass);
+            return ImageSingletons.lookup(AnnotationExtracter.class).hasAnnotation(element, annotationClass);
         } else {
             return element.isAnnotationPresent(annotationClass);
         }
@@ -93,7 +93,7 @@ public static boolean isAnnotationPresent(AnnotatedElement element, Class<? exte
     @SuppressWarnings("unchecked")
     public static <T extends Annotation> T getAnnotation(AnnotatedElement element, Class<T> annotationType) {
         if (ImageInfo.inImageBuildtimeCode()) {
-            return (T) ImageSingletons.lookup(ImageBuildtimeCodeAnnotationAccessSupport.class).getAnnotation(element, annotationType);
+            return ImageSingletons.lookup(AnnotationExtracter.class).extractAnnotation(element, annotationType, false);
         } else {
             return element.getAnnotation(annotationType);
         }
@@ -107,7 +107,7 @@ public static <T extends Annotation> T getAnnotation(AnnotatedElement element, C
     @SuppressWarnings("unchecked")
     public static Class<? extends Annotation>[] getAnnotationTypes(AnnotatedElement element) {
         if (ImageInfo.inImageBuildtimeCode()) {
-            return ImageSingletons.lookup(ImageBuildtimeCodeAnnotationAccessSupport.class).getAnnotationTypes(element);
+            return ImageSingletons.lookup(AnnotationExtracter.class).getAnnotationTypes(element);
         } else {
             return Arrays.stream(element.getAnnotations()).map(Annotation::annotationType).toArray(Class[]::new);
         }

sdk/src/org.graalvm.nativeimage/src/org/graalvm/nativeimage/impl/ImageBuildtimeCodeAnnotationAccessSupport.java renamed to sdk/src/org.graalvm.nativeimage/src/org/graalvm/nativeimage/impl/AnnotationExtracter.java

@@ -43,12 +43,14 @@
 import java.lang.annotation.Annotation;
 import java.lang.reflect.AnnotatedElement;
 
-public interface ImageBuildtimeCodeAnnotationAccessSupport {
-    // needed as Annotation Access-specific ImageSingletons key
+import org.graalvm.nativeimage.Platform;
+import org.graalvm.nativeimage.Platforms;
 
-    boolean isAnnotationPresent(AnnotatedElement element, Class<? extends Annotation> annotationClass);
+@Platforms(Platform.HOSTED_ONLY.class)
+public interface AnnotationExtracter {
+    boolean hasAnnotation(AnnotatedElement element, Class<? extends Annotation> annotationType);
 
-    Annotation getAnnotation(AnnotatedElement element, Class<? extends Annotation> annotationType);
+    <T extends Annotation> T extractAnnotation(AnnotatedElement element, Class<T> annotationType, boolean declaredOnly);
 
     Class<? extends Annotation>[] getAnnotationTypes(AnnotatedElement element);
 }

substratevm/src/com.oracle.svm.core/.checkstyle_checks.xml

@@ -6,15 +6,15 @@
   <module name="TreeWalker">
 
     <!-- Disallows direct calls to "AnnotatedElement.get(Declared)Annotation(s)". This check can yield false positives,
-    i.e., it will match any ".get(Declared)Annotation(s)" calls that are not preceded by "GuardedAnnotationAccess"
+    i.e., it will match any ".get(Declared)Annotation(s)" calls that are not preceded by "AnnotationAccess"
     since checkstyle relies only on pattern matching and it cannot determine the type of the call's receiver object.
      -->
     <module name="RegexpSinglelineJava">
       <property name="id" value="annotationAccess"/>
       <metadata name="net.sf.eclipsecs.core.comment" value="Disallow calls to AnnotatedElement.get(Declared)Annotation(s)."/>
       <property name="severity" value="error"/>
       <property name="format" value="(?&lt;!AnnotationAccess)\.(getAnnotation|getAnnotations|getDeclaredAnnotation|getDeclaredAnnotations|isAnnotationPresent)\b"/>
-      <property name="message" value="Direct calls to java.lang.reflect.AnnotatedElement.get(Declared)Annotation(s) are restricted. Use either com.oracle.svm.util.GuardedAnnotationAccess or com.oracle.svm.util.DirectAnnotationAccess methods. (Use &quot;// Checkstyle: allow direct annotation access... // Checkstyle: disallow direct annotation access&quot; to disable this check.)"/>
+      <property name="message" value="Direct calls to java.lang.reflect.AnnotatedElement.get(Declared)Annotation(s) are restricted. Use org.graalvm.nativeimage.AnnotationAccess methods. (Use &quot;// Checkstyle: allow direct annotation access... // Checkstyle: disallow direct annotation access&quot; to disable this check.)"/>
       <property name="ignoreComments" value="true"/>
     </module>
     <module name="SuppressionCommentFilter">

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ImageClassLoader.java

@@ -51,7 +51,6 @@
 
 import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.TypeResult;
-import com.oracle.svm.util.GuardedAnnotationAccess;
 import com.oracle.svm.util.ReflectionUtil;
 
 public final class ImageClassLoader {
@@ -143,7 +142,7 @@ private void findSystemElements(Class<?> systemClass) {
 
     private boolean isInPlatform(AnnotatedElement element) {
         try {
-            Platforms platformAnnotation = GuardedAnnotationAccess.getAnnotation(classLoaderSupport.annotationExtracter, element, Platforms.class);
+            Platforms platformAnnotation = classLoaderSupport.annotationExtracter.extractAnnotation(element, Platforms.class, false);
             return NativeImageGenerator.includedIn(platform, platformAnnotation);
         } catch (Throwable t) {
             handleClassLoadingError(t);
@@ -176,7 +175,7 @@ void handleClass(Class<?> clazz) {
         do {
             Platforms platformsAnnotation;
             try {
-                platformsAnnotation = GuardedAnnotationAccess.getAnnotation(classLoaderSupport.annotationExtracter, cur, Platforms.class);
+                platformsAnnotation = classLoaderSupport.annotationExtracter.extractAnnotation(cur, Platforms.class, false);
             } catch (Throwable t) {
                 handleClassLoadingError(t);
                 return;
@@ -374,7 +373,7 @@ public List<Class<?>> findAnnotatedClasses(Class<? extends Annotation> annotatio
 
     private void addAnnotatedClasses(EconomicSet<Class<?>> classes, Class<? extends Annotation> annotationClass, ArrayList<Class<?>> result) {
         for (Class<?> systemClass : classes) {
-            if (GuardedAnnotationAccess.isAnnotationPresent(classLoaderSupport.annotationExtracter, systemClass, annotationClass)) {
+            if (classLoaderSupport.annotationExtracter.hasAnnotation(systemClass, annotationClass)) {
                 result.add(systemClass);
             }
         }
@@ -383,7 +382,7 @@ private void addAnnotatedClasses(EconomicSet<Class<?>> classes, Class<? extends
     public List<Method> findAnnotatedMethods(Class<? extends Annotation> annotationClass) {
         ArrayList<Method> result = new ArrayList<>();
         for (Method method : systemMethods) {
-            if (GuardedAnnotationAccess.isAnnotationPresent(classLoaderSupport.annotationExtracter, method, annotationClass)) {
+            if (classLoaderSupport.annotationExtracter.hasAnnotation(method, annotationClass)) {
                 result.add(method);
             }
         }
@@ -395,7 +394,7 @@ public List<Method> findAnnotatedMethods(Class<? extends Annotation>[] annotatio
         for (Method method : systemMethods) {
             boolean match = true;
             for (Class<? extends Annotation> annotationClass : annotationClasses) {
-                if (!GuardedAnnotationAccess.isAnnotationPresent(classLoaderSupport.annotationExtracter, method, annotationClass)) {
+                if (!classLoaderSupport.annotationExtracter.hasAnnotation(method, annotationClass)) {
                     match = false;
                     break;
                 }
@@ -410,7 +409,7 @@ public List<Method> findAnnotatedMethods(Class<? extends Annotation>[] annotatio
     public List<Field> findAnnotatedFields(Class<? extends Annotation> annotationClass) {
         ArrayList<Field> result = new ArrayList<>();
         for (Field field : systemFields) {
-            if (GuardedAnnotationAccess.isAnnotationPresent(classLoaderSupport.annotationExtracter, field, annotationClass)) {
+            if (classLoaderSupport.annotationExtracter.hasAnnotation(field, annotationClass)) {
                 result.add(field);
             }
         }
@@ -432,13 +431,13 @@ public List<Class<? extends Annotation>> allAnnotations() {
     public <T extends Annotation> List<T> findAnnotations(Class<T> annotationClass) {
         List<T> result = new ArrayList<>();
         for (Class<?> clazz : findAnnotatedClasses(annotationClass, false)) {
-            result.add(GuardedAnnotationAccess.getAnnotation(classLoaderSupport.annotationExtracter, clazz, annotationClass));
+            result.add(classLoaderSupport.annotationExtracter.extractAnnotation(clazz, annotationClass, false));
         }
         for (Method method : findAnnotatedMethods(annotationClass)) {
-            result.add(GuardedAnnotationAccess.getAnnotation(classLoaderSupport.annotationExtracter, method, annotationClass));
+            result.add(classLoaderSupport.annotationExtracter.extractAnnotation(method, annotationClass, false));
         }
         for (Field field : findAnnotatedFields(annotationClass)) {
-            result.add(GuardedAnnotationAccess.getAnnotation(classLoaderSupport.annotationExtracter, field, annotationClass));
+            result.add(classLoaderSupport.annotationExtracter.extractAnnotation(field, annotationClass, false));
         }
         return result;
     }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageClassLoaderSupport.java

@@ -76,6 +76,7 @@
 import org.graalvm.collections.Pair;
 import org.graalvm.compiler.options.OptionKey;
 import org.graalvm.compiler.options.OptionValues;
+import org.graalvm.nativeimage.impl.AnnotationExtracter;
 
 import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.option.LocatableMultiOptionValue;
@@ -87,7 +88,6 @@
 import com.oracle.svm.core.util.VMError;
 import com.oracle.svm.hosted.annotation.SubstrateAnnotationExtracter;
 import com.oracle.svm.hosted.option.HostedOptionParser;
-import com.oracle.svm.util.AnnotationExtracter;
 import com.oracle.svm.util.ModuleSupport;
 import com.oracle.svm.util.ReflectionUtil;
 

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java

@@ -125,8 +125,8 @@
 import org.graalvm.nativeimage.c.struct.RawStructure;
 import org.graalvm.nativeimage.hosted.Feature;
 import org.graalvm.nativeimage.hosted.Feature.OnAnalysisExitAccess;
+import org.graalvm.nativeimage.impl.AnnotationExtracter;
 import org.graalvm.nativeimage.impl.CConstantValueSupport;
-import org.graalvm.nativeimage.impl.ImageBuildtimeCodeAnnotationAccessSupport;
 import org.graalvm.nativeimage.impl.RuntimeClassInitializationSupport;
 import org.graalvm.nativeimage.impl.SizeOfSupport;
 import org.graalvm.nativeimage.impl.clinit.ClassInitializationTracking;
@@ -251,7 +251,6 @@
 import com.oracle.svm.hosted.analysis.SVMAnalysisMetaAccess;
 import com.oracle.svm.hosted.analysis.SubstrateUnsupportedFeatures;
 import com.oracle.svm.hosted.annotation.AnnotationSupport;
-import com.oracle.svm.hosted.annotation.ImageBuildtimeCodeAnnotationAccessSupportSingleton;
 import com.oracle.svm.hosted.c.CAnnotationProcessorCache;
 import com.oracle.svm.hosted.c.CConstantValueSupportImpl;
 import com.oracle.svm.hosted.c.NativeLibraries;
@@ -300,7 +299,6 @@
 import com.oracle.svm.hosted.substitute.AnnotationSubstitutionProcessor;
 import com.oracle.svm.hosted.substitute.DeletedFieldsPlugin;
 import com.oracle.svm.hosted.substitute.UnsafeAutomaticSubstitutionProcessor;
-import com.oracle.svm.util.AnnotationExtracter;
 import com.oracle.svm.util.ClassUtil;
 import com.oracle.svm.util.ImageBuildStatistics;
 import com.oracle.svm.util.ReflectionUtil;
@@ -527,7 +525,6 @@ public void run(Map<Method, CEntryPointData> entryPoints,
             ImageSingletons.add(TimerCollection.class, timerCollection);
             ImageSingletons.add(ImageBuildStatistics.TimerCollectionPrinter.class, timerCollection);
             ImageSingletons.add(AnnotationExtracter.class, loader.classLoaderSupport.annotationExtracter);
-            ImageSingletons.add(ImageBuildtimeCodeAnnotationAccessSupport.class, new ImageBuildtimeCodeAnnotationAccessSupportSingleton());
             ImageSingletons.add(BuildArtifacts.class, (type, artifact) -> buildArtifacts.computeIfAbsent(type, t -> new ArrayList<>()).add(artifact));
             ImageSingletons.add(HostedOptionValues.class, new HostedOptionValues(optionProvider.getHostedValues()));
             ImageSingletons.add(RuntimeOptionValues.class, new RuntimeOptionValues(optionProvider.getRuntimeValues(), allOptionNames));

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/annotation/ImageBuildtimeCodeAnnotationAccessSupportSingleton.java

@@ -41,11 +41,11 @@
 import java.util.concurrent.ConcurrentHashMap;
 
 import org.graalvm.compiler.debug.GraalError;
+import org.graalvm.nativeimage.AnnotationAccess;
+import org.graalvm.nativeimage.impl.AnnotationExtracter;
 
 import com.oracle.svm.hosted.annotation.AnnotationMetadata.AnnotationExtractionError;
-import com.oracle.svm.util.AnnotationExtracter;
 import com.oracle.svm.util.AnnotationWrapper;
-import com.oracle.svm.util.GuardedAnnotationAccess;
 import com.oracle.svm.util.ReflectionUtil;
 
 import jdk.internal.reflect.ConstantPool;
@@ -71,8 +71,8 @@
  * subclass of {@link AnnotationMemberValue}. The actual annotation can then be created using the
  * {@link AnnotationMemberValue#get(Class)} method.
  *
- * The {@link SubstrateAnnotationExtracter} is tightly coupled with {@link GuardedAnnotationAccess},
- * which provides implementations of {@link AnnotatedElement#isAnnotationPresent(Class)} and
+ * The {@link SubstrateAnnotationExtracter} is tightly coupled with {@link AnnotationAccess}, which
+ * provides implementations of {@link AnnotatedElement#isAnnotationPresent(Class)} and
  * {@link AnnotatedElement#getAnnotation(Class)}. {@link AnnotatedElement#getAnnotations()} should
  * in principle not be used during Native Image generation.
  */
@@ -163,22 +163,43 @@ public class SubstrateAnnotationExtracter implements AnnotationExtracter {
     @SuppressWarnings("unchecked")
     @Override
     public <T extends Annotation> T extractAnnotation(AnnotatedElement element, Class<T> annotationType, boolean declaredOnly) {
-        for (AnnotationValue annotation : getAnnotationData(element, declaredOnly)) {
-            if (annotation.type != null && annotation.type.equals(annotationType)) {
-                return (T) resolvedAnnotationsCache.computeIfAbsent(annotation, value -> (Annotation) value.get(annotationType));
+        try {
+            for (AnnotationValue annotation : getAnnotationData(element, declaredOnly)) {
+                if (annotation.type != null && annotation.type.equals(annotationType)) {
+                    return (T) resolvedAnnotationsCache.computeIfAbsent(annotation, value -> (Annotation) value.get(annotationType));
+                }
             }
+            return null;
+        } catch (LinkageError e) {
+            /*
+             * Returning null essentially means that the element doesn't declare the annotationType,
+             * but we cannot know that since the annotation parsing failed. However, this allows us
+             * to defend against crashing the image builder if the user code references types
+             * missing from the classpath.
+             */
+            return null;
         }
-        return null;
+
     }
 
     @Override
     public boolean hasAnnotation(AnnotatedElement element, Class<? extends Annotation> annotationType) {
-        for (AnnotationValue annotation : getAnnotationData(element, false)) {
-            if (annotation.type != null && annotation.type.equals(annotationType)) {
-                return true;
+        try {
+            for (AnnotationValue annotation : getAnnotationData(element, false)) {
+                if (annotation.type != null && annotation.type.equals(annotationType)) {
+                    return true;
+                }
             }
+            return false;
+        } catch (LinkageError e) {
+            /*
+             * Returning false essentially means that the element doesn't declare the
+             * annotationType, but we cannot know that since the annotation parsing failed. However,
+             * this allows us to defend against crashing the image builder if the user code
+             * references types missing from the classpath.
+             */
+            return false;
         }
-        return false;
     }
 
     @SuppressWarnings("unchecked")