[GR-33184] Disable optimization for LocatableAnnotation.

PullRequest: graal/9603

Author: cstancu

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

@@ -59,9 +59,6 @@
 import java.util.stream.Collectors;
 import java.util.stream.StreamSupport;
 
-import com.oracle.graal.pointsto.BigBang;
-import com.oracle.svm.hosted.analysis.Inflation;
-import com.oracle.svm.hosted.analysis.NativeImagePointsToAnalysis;
 import org.graalvm.collections.EconomicSet;
 import org.graalvm.collections.Pair;
 import org.graalvm.compiler.api.replacements.Fold;
@@ -136,6 +133,7 @@
 import org.graalvm.word.PointerBase;
 
 import com.oracle.graal.pointsto.AnalysisPolicy;
+import com.oracle.graal.pointsto.BigBang;
 import com.oracle.graal.pointsto.BytecodeSensitiveAnalysisPolicy;
 import com.oracle.graal.pointsto.DefaultAnalysisPolicy;
 import com.oracle.graal.pointsto.api.PointstoOptions;
@@ -228,6 +226,8 @@
 import com.oracle.svm.hosted.FeatureImpl.OnAnalysisExitAccessImpl;
 import com.oracle.svm.hosted.ameta.AnalysisConstantFieldProvider;
 import com.oracle.svm.hosted.ameta.AnalysisConstantReflectionProvider;
+import com.oracle.svm.hosted.analysis.Inflation;
+import com.oracle.svm.hosted.analysis.NativeImagePointsToAnalysis;
 import com.oracle.svm.hosted.analysis.SVMAnalysisMetaAccess;
 import com.oracle.svm.hosted.annotation.AnnotationSupport;
 import com.oracle.svm.hosted.c.CAnnotationProcessorCache;
@@ -1437,6 +1437,10 @@ private static boolean addAssertionLIRPhases(LIRSuites lirSuites, boolean hosted
         return true;
     }
 
+    private static String format(TypeState state) {
+        return state.typesStream().map(t -> t.toJavaName(true)).collect(Collectors.joining(","));
+    }
+
     private void checkUniverse() {
         if (bb instanceof NativeImagePointsToAnalysis) {
             NativeImagePointsToAnalysis bigbang = (NativeImagePointsToAnalysis) this.bb;
@@ -1460,7 +1464,8 @@ private void checkUniverse() {
                                 String methodKey = method.format("%H.%n(%p)");
                                 bigbang.getUnsupportedFeatures().addMessage(methodKey, method,
                                                 "Parameter " + i + " of " + methodKey + " has declared type " + declaredType.toJavaName(true) +
-                                                                " with state " + declaredTypeState + " which is incompatible with types in parameter state: " + parameterState);
+                                                                ", with assignable types: " + format(declaredTypeState) +
+                                                                ", which is incompatible with analysis inferred types: " + format(parameterState) + ".");
                             }
                         }
                     }
@@ -1471,11 +1476,14 @@ private void checkUniverse() {
                 if (state != null) {
                     AnalysisType declaredType = field.getType();
                     if (declaredType.isInterface()) {
-                        state = TypeState.forSubtraction(bigbang, state, declaredType.getAssignableTypes(true));
+                        TypeState declaredTypeState = declaredType.getAssignableTypes(true);
+                        state = TypeState.forSubtraction(bigbang, state, declaredTypeState);
                         if (!state.isEmpty()) {
                             String fieldKey = field.format("%H.%n");
                             bigbang.getUnsupportedFeatures().addMessage(fieldKey, null,
-                                            "Field " + fieldKey + " has declared type " + declaredType.toJavaName(true) + " which is incompatible with types in state: " + state);
+                                            "Field " + fieldKey + " has declared type " + declaredType.toJavaName(true) +
+                                                            ", with assignable types: " + format(declaredTypeState) +
+                                                            ", which is incompatible with analysis inferred types: " + format(state) + ".");
                         }
                     }
                 }

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

@@ -24,7 +24,6 @@
  */
 package com.oracle.svm.hosted.annotation;
 
-import java.lang.annotation.Annotation;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Modifier;
 import java.lang.reflect.Proxy;
@@ -125,7 +124,6 @@ public class AnnotationSupport extends CustomSubstitution<AnnotationSubstitution
 
     private final SnippetReflectionProvider snippetReflection;
 
-    private final ResolvedJavaType javaLangAnnotationAnnotation;
     private final ResolvedJavaType javaLangReflectProxy;
     private final ResolvedJavaType constantAnnotationMarkerOriginalType;
 
@@ -143,7 +141,6 @@ public AnnotationSupport(MetaAccessProvider metaAccess, SnippetReflectionProvide
         super(metaAccess);
         this.snippetReflection = snippetReflection;
 
-        javaLangAnnotationAnnotation = metaAccess.lookupJavaType(java.lang.annotation.Annotation.class);
         javaLangReflectProxy = metaAccess.lookupJavaType(java.lang.reflect.Proxy.class);
         constantAnnotationMarkerOriginalType = metaAccess.lookupJavaType(constantAnnotationMarkerInterface);
         constantAnnotationMarkerSubstitutionType = new ConstantAnnotationMarkerSubstitutionType(constantAnnotationMarkerOriginalType, this);
@@ -153,16 +150,35 @@ public AnnotationSupport(MetaAccessProvider metaAccess, SnippetReflectionProvide
 
     private boolean isConstantAnnotationType(ResolvedJavaType type) {
         /*
-         * Check if the type implements all of Annotation, Proxy and the constant-annotation-marker
-         * interface. If so, then it is the type of a annotation proxy object encountered during
+         * Check if the type implements all of: Annotation, Proxy and the constant-annotation-marker
+         * interface. If so, then it is the type of an annotation proxy object encountered during
          * heap scanning. Only those types are substituted with a more efficient annotation proxy
-         * type implementation. If a type implements only Annotation and Proxy but not the
-         * constant-annotation-marker interface then it is a proxy type registered via the dynamic
-         * proxy API. Such type is used to allocate annotation instances at run time and must not be
-         * replaced.
+         * type implementation.
+         * 
+         * If a type implements only Annotation and Proxy but not the constant-annotation-marker
+         * interface then it is a proxy type registered via the dynamic proxy API. Such type is used
+         * to allocate annotation instances at run time and must not be replaced.
+         * 
+         * If the type implements more than two interfaces then it could be a non-standard
+         * annotation implementations like
+         * com.sun.xml.internal.bind.v2.model.annotation.LocatableAnnotation, i.e., an annotation
+         * wrapper that also implements com.sun.xml.internal.bind.v2.model.annotation.Locatable. We
+         * don't optimize these types; the implementation would be too complicated for a marginal
+         * benefit. Therefore, the type must implement two and only two interfaces: the annotation
+         * interface and the marker interface.
          */
-        return javaLangAnnotationAnnotation.isAssignableFrom(type) && javaLangReflectProxy.isAssignableFrom(type) &&
-                        constantAnnotationMarkerOriginalType.isAssignableFrom(type);
+        return type.getInterfaces().length == 2 &&
+                        isAnnotation(type.getInterfaces()[0]) &&
+                        type.getInterfaces()[1].equals(constantAnnotationMarkerOriginalType) &&
+                        javaLangReflectProxy.isAssignableFrom(type);
+    }
+
+    /* Value copied from java.lang.Class. */
+    private static final int ANNOTATION = 0x00002000;
+
+    /* Method copied from java.lang.Class. */
+    private static boolean isAnnotation(ResolvedJavaType type) {
+        return (type.getModifiers() & ANNOTATION) != 0;
     }
 
     @Override
@@ -665,7 +681,14 @@ class AnnotationObjectReplacer implements Function<Object, Object> {
     @Override
     public Object apply(Object original) {
         Class<?> clazz = original.getClass();
-        if (Annotation.class.isAssignableFrom(clazz) && Proxy.class.isAssignableFrom(clazz)) {
+        /*
+         * We only optimize standard implementation annotation types, i.e., the type must implement
+         * one and only one interface: the annotation interface, which in turn must implement
+         * java.lang.Annotation. Non-standard annotation implementations use their default
+         * implementation at run time.
+         */
+        if (clazz.getInterfaces().length == 1 && clazz.getInterfaces()[0].isAnnotation() &&
+                        Proxy.class.isAssignableFrom(clazz)) {
             return objectCache.computeIfAbsent(original, AnnotationObjectReplacer::replacementComputer);
         } else if (original instanceof SubstrateAnnotationInvocationHandler) {
             return SINGLETON_HANDLER;