Fix reflection metadata-related test failures

loicottet · loicottet · commit 8f636ab32b67 · 2022-03-22T18:18:58.000+01:00
diff --git a/sdk/src/org.graalvm.nativeimage/src/org/graalvm/nativeimage/impl/RuntimeReflectionSupport.java b/sdk/src/org.graalvm.nativeimage/src/org/graalvm/nativeimage/impl/RuntimeReflectionSupport.java
@@ -56,9 +56,11 @@ public interface RuntimeReflectionSupport extends ReflectionRegistry {
     Object getAccessor(Executable method);
 
     /*
-     * Returns the methods that shadow a superclass method registered for reflection, to be excluded
-     * from reflection queries.
+     * Returns the methods and fields that shadow a superclass element registered for reflection, to
+     * be excluded from reflection queries.
      */
+    Set<?> getHidingReflectionFields();
+
     Set<?> getHidingReflectionMethods();
 
     Object[] getRecordComponents(Class<?> type);
diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/hub/DynamicHub.java
@@ -39,6 +39,7 @@
 import java.lang.reflect.Field;
 import java.lang.reflect.GenericDeclaration;
 import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Member;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.lang.reflect.Type;
@@ -49,6 +50,7 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.StringJoiner;
 
@@ -899,8 +901,20 @@ public <T extends Annotation> T[] getAnnotationsByType(Class<T> annotationClass)
     @KeepOriginal
     private native Constructor<?>[] getConstructors();
 
-    @KeepOriginal
-    private native Field getField(@SuppressWarnings("hiding") String name) throws NoSuchMethodException;
+    @Substitute
+    public Field getField(String fieldName) throws NoSuchFieldException, SecurityException {
+        Objects.requireNonNull(fieldName);
+        @SuppressWarnings("removal")
+        SecurityManager sm = System.getSecurityManager();
+        if (sm != null) {
+            checkMemberAccess(sm, Member.PUBLIC, Reflection.getCallerClass(), true);
+        }
+        Field field = getField0(fieldName);
+        if (field == null || ImageSingletons.lookup(ReflectionMetadataDecoder.class).isHiding(field.getModifiers())) {
+            throw new NoSuchFieldException(fieldName);
+        }
+        return getReflectionFactory().copyField(field);
+    }
 
     @KeepOriginal
     private native Method getMethod(@SuppressWarnings("hiding") String name, Class<?>... parameterTypes) throws NoSuchMethodException;
@@ -923,8 +937,23 @@ public <T extends Annotation> T[] getAnnotationsByType(Class<T> annotationClass)
     @KeepOriginal
     private native Constructor<?>[] getDeclaredConstructors();
 
-    @KeepOriginal
-    private native Field getDeclaredField(@SuppressWarnings("hiding") String name);
+    /**
+     * @see #filterHidingFields(Field...)
+     */
+    @Substitute
+    public Field getDeclaredField(String fieldName) throws NoSuchFieldException, SecurityException {
+        Objects.requireNonNull(fieldName);
+        @SuppressWarnings("removal")
+        SecurityManager sm = System.getSecurityManager();
+        if (sm != null) {
+            checkMemberAccess(sm, Member.DECLARED, Reflection.getCallerClass(), true);
+        }
+        Field field = searchFields(privateGetDeclaredFields(false), fieldName);
+        if (field == null || ImageSingletons.lookup(ReflectionMetadataDecoder.class).isHiding(field.getModifiers())) {
+            throw new NoSuchFieldException(fieldName);
+        }
+        return getReflectionFactory().copyField(field);
+    }
 
     @KeepOriginal
     private native Method getDeclaredMethod(@SuppressWarnings("hiding") String name, Class<?>... parameterTypes);
@@ -1009,8 +1038,19 @@ private static Method searchMethods(Method[] allMethods, String name, Class<?>[]
     @KeepOriginal
     private static native boolean arrayContentsEq(Object[] a1, Object[] a2);
 
-    @KeepOriginal
-    private static native Field[] copyFields(Field[] arg);
+    /**
+     * @see #filterHidingFields(Field...)
+     */
+    @Substitute
+    private static Field[] copyFields(Field[] original) {
+        Field[] arg = filterHidingFields(original);
+        Field[] out = new Field[arg.length];
+        ReflectionFactory fact = getReflectionFactory();
+        for (int i = 0; i < arg.length; i++) {
+            out[i] = fact.copyField(arg[i]);
+        }
+        return out;
+    }
 
     /**
      * @see #filterHidingMethods(Method...)
@@ -1463,14 +1503,24 @@ private Class<?>[] getPermittedSubclasses0() {
     private native boolean isDirectSubType(Class<?> c);
 
     /*
-     * We need to filter out hiding methods at the last moment. This ensures that the JDK internals
-     * see them as regular methods and ensure the visibility of methods is correct, but they should
-     * not be returned to application code.
+     * We need to filter out hiding elements at the last moment. This ensures that the JDK internals
+     * see them as regular methods and fields and ensure their visibility is correct, but they
+     * should not be returned to application code.
      */
+    private static Field[] filterHidingFields(Field... fields) {
+        List<Field> filtered = new ArrayList<>();
+        for (Field field : fields) {
+            if (!ImageSingletons.lookup(ReflectionMetadataDecoder.class).isHiding(field.getModifiers())) {
+                filtered.add(field);
+            }
+        }
+        return filtered.toArray(new Field[0]);
+    }
+
     private static Method[] filterHidingMethods(Method... methods) {
         List<Method> filtered = new ArrayList<>();
         for (Method method : methods) {
-            if (!ImageSingletons.lookup(ReflectionMetadataDecoder.class).isHidingMethod(method.getModifiers())) {
+            if (!ImageSingletons.lookup(ReflectionMetadataDecoder.class).isHiding(method.getModifiers())) {
                 filtered.add(method);
             }
         }
@@ -1687,7 +1737,7 @@ Method getMostSpecific() {
             }
         }
         /* Filter out hiding methods after the retursive lookup is done */
-        return ImageSingletons.lookup(ReflectionMetadataDecoder.class).isHidingMethod(m.getModifiers()) ? null : m;
+        return ImageSingletons.lookup(ReflectionMetadataDecoder.class).isHiding(m.getModifiers()) ? null : m;
     }
 }
 
diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/ReflectionMetadataDecoder.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/reflect/ReflectionMetadataDecoder.java
@@ -51,7 +51,7 @@ public interface ReflectionMetadataDecoder {
 
     byte[] parseByteArray(int index);
 
-    boolean isHidingMethod(int modifiers);
+    boolean isHiding(int modifiers);
 
     long getMetadataByteLength();
 }
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageCodeCache.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageCodeCache.java
@@ -60,6 +60,7 @@
 import org.graalvm.word.WordFactory;
 
 import com.oracle.graal.pointsto.BigBang;
+import com.oracle.graal.pointsto.meta.AnalysisField;
 import com.oracle.graal.pointsto.meta.AnalysisMethod;
 import com.oracle.objectfile.ObjectFile;
 import com.oracle.svm.core.SubstrateOptions;
@@ -282,6 +283,21 @@ public void buildRuntimeMetadata(CFunctionPointer firstMethod, UnsignedWord code
             }
         }
 
+        for (Object field : reflectionSupport.getHidingReflectionFields()) {
+            AnalysisField hidingField = (AnalysisField) field;
+            HostedField hostedField = hUniverse.optionalLookup(hidingField);
+            if (hostedField == null || !includedFields.contains(hostedField)) {
+                HostedType declaringType = hUniverse.lookup(hidingField.getDeclaringClass());
+                String name = hidingField.getName();
+                HostedType type = hUniverse.lookup(hidingField.getType());
+                int modifiers = hidingField.getModifiers();
+                reflectionMetadataEncoder.addHidingFieldMetadata(hidingField, declaringType, name, type, modifiers);
+                if (hostedField != null) {
+                    includedFields.add(hostedField);
+                }
+            }
+        }
+
         for (Object method : reflectionSupport.getHidingReflectionMethods()) {
             AnalysisMethod hidingMethod = (AnalysisMethod) method;
             HostedMethod hostedMethod = hUniverse.optionalLookup(hidingMethod);
@@ -624,6 +640,8 @@ public interface ReflectionMetadataEncoder {
 
         void addHeapAccessibleObjectMetadata(MetaAccessProvider metaAccess, AccessibleObject object);
 
+        void addHidingFieldMetadata(AnalysisField analysisField, HostedType declType, String name, HostedType type, int modifiers);
+
         void addHidingMethodMetadata(AnalysisMethod analysisMethod, HostedType declType, String name, HostedType[] paramTypes, int modifiers, HostedType returnType);
 
         void addReachableFieldMetadata(HostedField field);
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/lambda/LambdaSubstitutionType.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/lambda/LambdaSubstitutionType.java
@@ -399,7 +399,10 @@ public <T extends Annotation> T[] getAnnotationsByType(Class<T> annotationClass)
 
     @Override
     public <T extends Annotation> T getDeclaredAnnotation(Class<T> annotationClass) {
-        return original.getDeclaredAnnotation(annotationClass);
+        if (annotationClass == LambdaFormHiddenMethod.class) {
+            return annotationClass.cast(LambdaFormHiddenMethod.Holder.INSTANCE);
+        }
+        return null;
     }
 
     @Override
@@ -409,7 +412,7 @@ public <T extends Annotation> T[] getDeclaredAnnotationsByType(Class<T> annotati
 
     @Override
     public Annotation[] getDeclaredAnnotations() {
-        return original.getDeclaredAnnotations();
+        return LambdaFormHiddenMethod.Holder.ARRAY;
     }
 
     public ResolvedJavaType getOriginal() {
diff --git a/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/hosted/ReflectionDataBuilder.java b/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/hosted/ReflectionDataBuilder.java
@@ -79,6 +79,7 @@
 import com.oracle.svm.util.ModuleSupport;
 
 import jdk.vm.ci.meta.JavaType;
+import jdk.vm.ci.meta.ResolvedJavaField;
 import jdk.vm.ci.meta.ResolvedJavaMethod;
 import jdk.vm.ci.meta.ResolvedJavaType;
 import sun.reflect.annotation.AnnotationType;
@@ -96,6 +97,7 @@ public class ReflectionDataBuilder extends ConditionalConfigurationRegistry impl
     private final Map<Executable, ExecutableAccessibility> reflectionMethods = new ConcurrentHashMap<>();
     private final Map<Executable, Object> methodAccessors = new ConcurrentHashMap<>();
     private final Set<Field> reflectionFields = Collections.newSetFromMap(new ConcurrentHashMap<>());
+    private final Set<AnalysisField> hidingFields = ConcurrentHashMap.newKeySet();
     private final Set<AnalysisMethod> hidingMethods = ConcurrentHashMap.newKeySet();
     private final Set<Executable> registeredMethods = ConcurrentHashMap.newKeySet();
     private final Set<Field> registeredFields = ConcurrentHashMap.newKeySet();
@@ -107,6 +109,7 @@ public class ReflectionDataBuilder extends ConditionalConfigurationRegistry impl
     private final Set<Class<?>> processedClasses = new HashSet<>();
     private final Set<Type> processedTypes = new HashSet<>();
     private final Set<DynamicHub> processedDynamicHubs = new HashSet<>();
+    private final Map<AnalysisField, Set<AnalysisType>> processedHidingFields = new HashMap<>();
     private final Map<AnalysisMethod, Set<AnalysisType>> processedHidingMethods = new HashMap<>();
     private final Set<AccessibleObject> processedHeapReflectionObjects = new HashSet<>();
 
@@ -266,6 +269,7 @@ protected void processMethodMetadata(DuringAnalysisAccessImpl access) {
             if (!registeredFields.contains(reflectField) && !SubstitutionReflectivityFilter.shouldExclude(reflectField, access.getMetaAccess(), access.getUniverse())) {
                 AnalysisField analysisField = access.getMetaAccess().lookupJavaField(reflectField);
                 registerTypesForField(access, analysisField, reflectField);
+                registerHidingSubTypeFields(access, analysisField, analysisField.getDeclaringClass());
                 registeredFields.add(reflectField);
             }
         }
@@ -291,13 +295,18 @@ protected void processMethodMetadata(DuringAnalysisAccessImpl access) {
             if (!processedHeapReflectionObjects.contains(object)) {
                 if (object instanceof Field) {
                     Field field = (Field) object;
-                    AnalysisField analysisField = access.getMetaAccess().lookupJavaField(field);
-                    registerTypesForField(access, analysisField, field);
+                    if (!SubstitutionReflectivityFilter.shouldExclude(field, access.getMetaAccess(), access.getUniverse())) {
+                        AnalysisField analysisField = access.getMetaAccess().lookupJavaField(field);
+                        registerTypesForField(access, analysisField, field);
+                        registerHidingSubTypeFields(access, analysisField, analysisField.getDeclaringClass());
+                    }
                 } else if (object instanceof Executable) {
                     Executable executable = (Executable) object;
-                    AnalysisMethod analysisMethod = access.getMetaAccess().lookupJavaMethod(executable);
-                    registerTypesForMethod(access, analysisMethod, executable);
-                    registerHidingSubTypeMethods(access, analysisMethod, analysisMethod.getDeclaringClass());
+                    if (!SubstitutionReflectivityFilter.shouldExclude(executable, access.getMetaAccess(), access.getUniverse())) {
+                        AnalysisMethod analysisMethod = access.getMetaAccess().lookupJavaMethod(executable);
+                        registerTypesForMethod(access, analysisMethod, executable);
+                        registerHidingSubTypeMethods(access, analysisMethod, analysisMethod.getDeclaringClass());
+                    }
                 }
                 processedHeapReflectionObjects.add(object);
             }
@@ -316,6 +325,38 @@ protected void processMethodMetadata(DuringAnalysisAccessImpl access) {
         }
     }
 
+    private void registerHidingSubTypeFields(DuringAnalysisAccess access, AnalysisField field, AnalysisType type) {
+        if (!type.equals(field.getDeclaringClass()) && type.isReachable()) {
+            if (!processedHidingFields.containsKey(field) || !processedHidingFields.get(field).contains(type)) {
+                processedHidingFields.computeIfAbsent(field, m -> ConcurrentHashMap.newKeySet()).add(type);
+                try {
+                    AnalysisField[] subClassFields = field.isStatic() ? type.getStaticFields() : type.getInstanceFields(false);
+                    for (AnalysisField subclassField : subClassFields) {
+                        if (subclassField.getName().equals(field.getName())) {
+                            hidingFields.add(subclassField);
+                        }
+                    }
+                    /*
+                     * Lookup can lead to the creation of new AnalysisField objects, so we need to
+                     * run another analysis iteration.
+                     */
+                    access.requireAnalysisIteration();
+
+                } catch (UnsupportedFeatureException | LinkageError e) {
+                    /*
+                     * A field that is not supposed to end up in the image is considered as being
+                     * absent for reflection purposes.
+                     */
+                }
+            }
+        }
+        for (AnalysisType subType : type.getSubTypes()) {
+            if (!subType.equals(type)) {
+                registerHidingSubTypeFields(access, field, subType);
+            }
+        }
+    }
+
     private void registerHidingSubTypeMethods(DuringAnalysisAccess access, AnalysisMethod method, AnalysisType type) {
         if (!type.equals(method.getDeclaringClass()) && type.isReachable()) {
             if (!processedHidingMethods.containsKey(method) || !processedHidingMethods.get(method).contains(type)) {
@@ -470,7 +511,6 @@ private static void registerTypesForReachableMethod(DuringAnalysisAccessImpl acc
         for (JavaType paramType : analysisMethod.toParameterTypes()) {
             makeAnalysisTypeReachable(access, (AnalysisType) paramType);
         }
-        makeAnalysisTypeReachable(access, (AnalysisType) analysisMethod.getSignature().getReturnType(null));
     }
 
     private void makeTypeReachable(DuringAnalysisAccessImpl access, Type type) {
@@ -779,6 +819,12 @@ public Object getAccessor(Executable method) {
         return methodAccessors.get(method);
     }
 
+    @Override
+    public Set<ResolvedJavaField> getHidingReflectionFields() {
+        assert sealed;
+        return Collections.unmodifiableSet(hidingFields);
+    }
+
     @Override
     public Set<ResolvedJavaMethod> getHidingReflectionMethods() {
         assert sealed;
diff --git a/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/hosted/ReflectionMetadata.java b/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/hosted/ReflectionMetadata.java
diff --git a/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/hosted/ReflectionMetadataEncoderImpl.java b/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/hosted/ReflectionMetadataEncoderImpl.java
diff --git a/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/target/ReflectionMetadataDecoderImpl.java b/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/target/ReflectionMetadataDecoderImpl.java

Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,7 @@ public interface ReflectionMetadataDecoder {`
`51`	`51`
`52`	`52`	`byte[] parseByteArray(int index);`
`53`	`53`
`54`		`- boolean isHidingMethod(int modifiers);`
	`54`	`+ boolean isHiding(int modifiers);`
`55`	`55`
`56`	`56`	`long getMetadataByteLength();`
`57`	`57`	`}`