[GR-60042] Include fields that are not backed by a reflection field

Zeavee · Zeavee · commit 3fcef8b98dfe · 2024-12-19T20:40:34.000Z
PullRequest: graal/19411
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ClassInclusionPolicy.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ClassInclusionPolicy.java
@@ -32,6 +32,7 @@
 import org.graalvm.nativeimage.AnnotationAccess;
 import org.graalvm.nativeimage.hosted.Feature;
 
+import com.oracle.graal.pointsto.meta.AnalysisField;
 import com.oracle.graal.pointsto.meta.AnalysisMethod;
 import com.oracle.svm.core.annotate.TargetClass;
 
@@ -88,6 +89,16 @@ public boolean isFieldIncluded(Field field) {
         return bb.getHostVM().isFieldIncluded(bb, field);
     }
 
+    /**
+     * Determine if the given field needs to be included in the image according to the policy.
+     */
+    public boolean isFieldIncluded(AnalysisField field) {
+        if (!bb.getHostVM().platformSupported(field)) {
+            return false;
+        }
+        return bb.getHostVM().isFieldIncluded(bb, field);
+    }
+
     /**
      * Includes the given class in the image.
      */
@@ -121,6 +132,13 @@ public void includeField(Field field) {
         bb.postTask(debug -> bb.addRootField(field));
     }
 
+    /**
+     * Includes the given field in the image.
+     */
+    public void includeField(AnalysisField field) {
+        bb.postTask(debug -> bb.addRootField(field));
+    }
+
     /**
      * The analysis for the base layer of a layered image assumes that any method that is reachable
      * using the base java access rules can be an entry point. An upper layer does not have access
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/PointsToAnalysis.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/PointsToAnalysis.java
@@ -501,12 +501,15 @@ public AnalysisType addRootField(Class<?> clazz, String fieldName) {
 
     @Override
     public AnalysisType addRootField(Field field) {
-        AnalysisField analysisField = getMetaAccess().lookupJavaField(field);
-        if (analysisField.isStatic()) {
-            return addRootStaticField(analysisField);
+        return addRootField(getMetaAccess().lookupJavaField(field));
+    }
+
+    @Override
+    public AnalysisType addRootField(AnalysisField field) {
+        if (field.isStatic()) {
+            return addRootStaticField(field);
         } else {
-            AnalysisType analysisType = getMetaAccess().lookupJavaType(field.getDeclaringClass());
-            return addRootField(analysisType, analysisField);
+            return addRootField(field.getDeclaringClass(), field);
         }
     }
 
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ReachabilityAnalysis.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ReachabilityAnalysis.java
@@ -60,6 +60,8 @@ public interface ReachabilityAnalysis {
 
     AnalysisType addRootField(Field field);
 
+    AnalysisType addRootField(AnalysisField field);
+
     /**
      * Registers the method as root. Must be an {@link MultiMethod#ORIGINAL_METHOD}.
      *
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/HostVM.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/HostVM.java
@@ -42,6 +42,7 @@
 import com.oracle.graal.pointsto.BigBang;
 import com.oracle.graal.pointsto.PointsToAnalysis;
 import com.oracle.graal.pointsto.flow.InvokeTypeFlow;
+import com.oracle.graal.pointsto.meta.AnalysisField;
 import com.oracle.graal.pointsto.meta.AnalysisMethod;
 import com.oracle.graal.pointsto.meta.AnalysisType;
 import com.oracle.graal.pointsto.meta.AnalysisUniverse;
@@ -330,11 +331,23 @@ public HostedProviders getProviders(MultiMethod.MultiMethodKey key) {
         return providers;
     }
 
+    /**
+     * This method should only be used by the {@code ClassInclusionPolicy} to determine which fields
+     * should be included in the shared layer.
+     */
     @SuppressWarnings("unused")
     public boolean isFieldIncluded(BigBang bb, Field field) {
         return true;
     }
 
+    /**
+     * See {@link HostVM#isFieldIncluded(BigBang, Field)}.
+     */
+    @SuppressWarnings("unused")
+    public boolean isFieldIncluded(BigBang bb, AnalysisField field) {
+        return true;
+    }
+
     public boolean isClosedTypeWorld() {
         return true;
     }
diff --git a/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/ReachabilityAnalysisEngine.java b/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/ReachabilityAnalysisEngine.java
@@ -143,8 +143,7 @@ public AnalysisType addRootField(Class<?> clazz, String fieldName) {
         for (ResolvedJavaField javaField : type.getInstanceFields(true)) {
             AnalysisField field = (AnalysisField) javaField;
             if (field.getName().equals(fieldName)) {
-                field.registerAsAccessed("root field");
-                return field.getType();
+                return addRootField(field);
             }
         }
         throw AnalysisError.userError("Field not found: " + fieldName);
@@ -153,6 +152,11 @@ public AnalysisType addRootField(Class<?> clazz, String fieldName) {
     @Override
     public AnalysisType addRootField(Field field) {
         AnalysisField analysisField = getMetaAccess().lookupJavaField(field);
+        return addRootField(analysisField);
+    }
+
+    @Override
+    public AnalysisType addRootField(AnalysisField analysisField) {
         analysisField.registerAsAccessed("root field");
         return analysisField.getType();
     }
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SVMHost.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SVMHost.java
@@ -58,6 +58,7 @@
 import com.oracle.graal.pointsto.constraints.UnsupportedFeatureException;
 import com.oracle.graal.pointsto.heap.ImageLayerLoader;
 import com.oracle.graal.pointsto.infrastructure.OriginalClassProvider;
+import com.oracle.graal.pointsto.infrastructure.OriginalFieldProvider;
 import com.oracle.graal.pointsto.meta.AnalysisField;
 import com.oracle.graal.pointsto.meta.AnalysisMetaAccess;
 import com.oracle.graal.pointsto.meta.AnalysisMethod;
@@ -76,6 +77,7 @@
 import com.oracle.svm.core.RuntimeAssertionsSupport;
 import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.SubstrateOptions.OptimizationLevel;
+import com.oracle.svm.core.annotate.Delete;
 import com.oracle.svm.core.annotate.InjectAccessors;
 import com.oracle.svm.core.c.CGlobalData;
 import com.oracle.svm.core.graal.meta.SubstrateForeignCallLinkage;
@@ -195,6 +197,8 @@ public enum UsageKind {
     private final int layerId;
     private final boolean useBaseLayer;
     private Set<Field> excludedFields;
+    private AnalysisType cGlobalData;
+    private AnalysisType optionKey;
 
     private final Boolean optionAllowUnsafeAllocationOfAllInstantiatedTypes = SubstrateOptions.AllowUnsafeAllocationOfAllInstantiatedTypes.getValue();
     private final boolean isClosedTypeWorld = SubstrateOptions.useClosedTypeWorld();
@@ -906,6 +910,10 @@ private void initializeExcludedFields() {
         excludedFields.add(ReflectionUtil.lookupField(NativeLibraries.class, "nativeLibraryLockMap"));
     }
 
+    /**
+     * This method cannot use an {@link AnalysisField} because it is used before the analysis is set
+     * up.
+     */
     @Override
     public boolean isFieldIncluded(BigBang bb, Field field) {
         /*
@@ -935,7 +943,53 @@ public boolean isFieldIncluded(BigBang bb, Field field) {
         /* Fields that are deleted or substituted should not be in the image */
         if (bb instanceof NativeImagePointsToAnalysis nativeImagePointsToAnalysis) {
             AnnotationSubstitutionProcessor annotationSubstitutionProcessor = nativeImagePointsToAnalysis.getAnnotationSubstitutionProcessor();
-            return !annotationSubstitutionProcessor.isDeleted(field) && !annotationSubstitutionProcessor.isAnnotationPresent(field, InjectAccessors.class);
+            boolean included = !annotationSubstitutionProcessor.isDeleted(field) && !annotationSubstitutionProcessor.isAnnotationPresent(field, InjectAccessors.class);
+            if (!included) {
+                return false;
+            }
+        }
+        return super.isFieldIncluded(bb, field);
+    }
+
+    /**
+     * This method needs to be kept in sync with {@link SVMHost#isFieldIncluded(BigBang, Field)}.
+     */
+    @Override
+    public boolean isFieldIncluded(BigBang bb, AnalysisField field) {
+        /*
+         * Fields of type CGlobalData can use a CGlobalDataFactory which must not be reachable at
+         * run time
+         */
+        if (cGlobalData == null) {
+            cGlobalData = bb.getMetaAccess().lookupJavaType(CGlobalData.class);
+        }
+        if (field.getType().equals(cGlobalData)) {
+            return false;
+        }
+
+        if (excludedFields.contains(OriginalFieldProvider.getJavaField(field))) {
+            return false;
+        }
+
+        /* Fields with those names are not allowed in the image */
+        if (NativeImageGenerator.checkName(field.getType().toJavaName() + "." + field.getName()) != null) {
+            return false;
+        }
+        /* Options should not be in the image */
+        if (optionKey == null) {
+            optionKey = bb.getMetaAccess().lookupJavaType(OptionKey.class);
+        }
+        if (optionKey.isAssignableFrom(field.getType())) {
+            return false;
+        }
+        /* Fields from this package should not be in the image */
+        if (field.getDeclaringClass().toJavaName().startsWith("jdk.graal.compiler")) {
+            return false;
+        }
+        /* Fields that are deleted or substituted should not be in the image */
+        boolean included = field.getAnnotation(Delete.class) == null && field.getAnnotation(InjectAccessors.class) == null;
+        if (!included) {
+            return false;
         }
         return super.isFieldIncluded(bb, field);
     }
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/analysis/NativeImagePointsToAnalysis.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/analysis/NativeImagePointsToAnalysis.java
@@ -37,7 +37,6 @@
 import com.oracle.graal.pointsto.constraints.UnsupportedFeatures;
 import com.oracle.graal.pointsto.flow.MethodFlowsGraph;
 import com.oracle.graal.pointsto.flow.MethodTypeFlowBuilder;
-import com.oracle.graal.pointsto.infrastructure.OriginalFieldProvider;
 import com.oracle.graal.pointsto.meta.AnalysisField;
 import com.oracle.graal.pointsto.meta.AnalysisMetaAccess;
 import com.oracle.graal.pointsto.meta.AnalysisMethod;
@@ -154,9 +153,8 @@ public void onTypeReachable(AnalysisType type) {
                  */
                 Stream.concat(Arrays.stream(getOrDefault(type, t -> t.getInstanceFields(true), new AnalysisField[0])),
                                 Arrays.stream(getOrDefault(type, AnalysisType::getStaticFields, new AnalysisField[0])))
-                                .map(OriginalFieldProvider::getJavaField)
-                                .filter(field -> field != null && classInclusionPolicy.isFieldIncluded(field))
-                                .forEach(classInclusionPolicy::includeField);
+                                .filter(field -> field != null && classInclusionPolicy.isFieldIncluded((AnalysisField) field))
+                                .forEach(field -> classInclusionPolicy.includeField((AnalysisField) field));
 
                 /*
                  * Only the class initializers that are executed at run time should be included in

Original file line number	Diff line number	Diff line change
`@@ -60,6 +60,8 @@ public interface ReachabilityAnalysis {`
`60`	`60`
`61`	`61`	`AnalysisType addRootField(Field field);`
`62`	`62`
	`63`	`+ AnalysisType addRootField(AnalysisField field);`
	`64`	`+`
`63`	`65`	`/**`
`64`	`66`	`* Registers the method as root. Must be an {@link MultiMethod#ORIGINAL_METHOD}.`
`65`	`67`	`*`