[GR-59340] Saturate all reachable methods in open world.

PullRequest: graal/19126

Author: cstancu

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/PointsToAnalysis.java

@@ -349,11 +349,6 @@ public AnalysisMethod forcedAddRootMethod(AnalysisMethod method, boolean invokeS
              * optimized away by the analysis.
              */
             typeFlow.ensureFlowsGraphCreated(this, null);
-            /*
-             * Saturating all the parameters of the method allows to enforce that no optimization is
-             * performed using the types of the parameters of the methods.
-             */
-            typeFlow.getMethodFlowsGraph().saturateAllParameters(this);
         });
         return addRootMethod(analysisMethod, invokeSpecial, reason, otherRoots);
     }

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ReachabilityAnalysis.java

@@ -91,8 +91,8 @@ public interface ReachabilityAnalysis {
 
     /**
      * In addition to register the method as a root, saturate all the parameters. Meant to be used
-     * under the {@code LayeredBaseImageAnalysis} option to ensure the invocation is replaced by the
-     * context-insensitive invoke.
+     * under the {@code UseBaseLayerInclusionPolicy} option to ensure the invocation is replaced by
+     * the context-insensitive invoke.
      *
      * @see ReachabilityAnalysis#addRootMethod(AnalysisMethod, boolean, Object,
      *      MultiMethod.MultiMethodKey...)

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/HostVM.java

@@ -168,6 +168,10 @@ public void checkType(ResolvedJavaType type, AnalysisUniverse universe) {
     public void onTypeInstantiated(BigBang bb, AnalysisType type) {
     }
 
+    public boolean isCoreType(@SuppressWarnings("unused") AnalysisType type) {
+        return false;
+    }
+
     public boolean useBaseLayer() {
         return false;
     }

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodFlowsGraph.java

@@ -37,6 +37,7 @@
 
 import com.oracle.graal.pointsto.PointsToAnalysis;
 import com.oracle.graal.pointsto.meta.AnalysisMethod;
+import com.oracle.graal.pointsto.meta.AnalysisType;
 import com.oracle.graal.pointsto.meta.PointsToAnalysisMethod;
 import com.oracle.graal.pointsto.util.AnalysisError;
 
@@ -443,11 +444,20 @@ void updateInternalState(GraphKind newGraphKind) {
     }
 
     /**
-     * Saturates all the formal parameters of the graph. This is expected to be used by layered base
-     * image analysis.
+     * For open world analysis saturate all the formal parameters and the returns from virtual
+     * invokes for all methods that can be directly accessible from the open world. This doesn't
+     * include methods in core VM classes.
      */
-    public void saturateAllParameters(PointsToAnalysis bb) {
-        AnalysisError.guarantee(bb.isBaseLayerAnalysisEnabled());
+    void saturateForOpenTypeWorld(PointsToAnalysis bb) {
+        /*
+         * Skip methods declared in core VM types. The core is not extensible from the open world,
+         * and it should not be invoked directly.
+         */
+        AnalysisType declaringClass = method.getDeclaringClass();
+        if (bb.getHostVM().isCoreType(declaringClass)) {
+            return;
+        }
+
         for (TypeFlow<?> parameter : getParameters()) {
             if (parameter != null && parameter.canSaturate(bb)) {
                 parameter.enableFlow(bb);
@@ -456,19 +466,15 @@ public void saturateAllParameters(PointsToAnalysis bb) {
         }
 
         /*
-         * Even if saturating only parameters already ensures that all code from the base layer is
-         * reached, it is still necessary to saturate flows from miscEntryFlows, as the extension
-         * image can introduce new types and methods. For example, an ActualReturnTypeFlow returning
-         * from an AbstractVirtualInvokeTypeFlow would not be saturated only by saturating
-         * parameters, as there is no direct use/observer link between the two flows. While all the
-         * types returned by the implementations from the base image will be in the TypeState, a
-         * different type might be returned by an implementation introduced by the extension image.
+         * Saturate the return of virtual invokes that could return new types from the open world.
+         * Returns from methods that cannot be overwritten, i.e., the receiver type is closed, are
+         * not saturated.
          */
-        if (miscEntryFlows != null) {
-            for (TypeFlow<?> miscEntryFlow : miscEntryFlows) {
-                if (miscEntryFlow != null && miscEntryFlow.canSaturate(bb)) {
-                    miscEntryFlow.enableFlow(bb);
-                    miscEntryFlow.onSaturated(bb);
+        for (InvokeTypeFlow invokeTypeFlow : getInvokes()) {
+            if (!invokeTypeFlow.isDirectInvoke() && !bb.isClosed(invokeTypeFlow.getReceiverType())) {
+                if (invokeTypeFlow.actualReturn != null) {
+                    invokeTypeFlow.actualReturn.enableFlow(bb);
+                    invokeTypeFlow.actualReturn.onSaturated(bb);
                 }
             }
         }

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlow.java

@@ -179,6 +179,11 @@ private synchronized void createFlowsGraph(PointsToAnalysis bb, InvokeTypeFlow r
                 assert flowsGraph != null;
 
                 initFlowsGraph(bb, builder.postInitFlows);
+
+                if (!bb.getHostVM().isClosedTypeWorld()) {
+                    flowsGraph.saturateForOpenTypeWorld(bb);
+                }
+
             } catch (Throwable t) {
                 /* Wrap all errors as parsing errors. */
                 throw AnalysisError.parsingError(method, t);

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java

@@ -1293,8 +1293,8 @@ public static boolean includeAll() {
         return IncludeAllFromModule.hasBeenSet() || IncludeAllFromPath.hasBeenSet() || IncludeAllFromClassPath.hasBeenSet();
     }
 
-    @Option(help = "Run layered image base layer open-world analysis. Includes all public types and methods that can be reached using normal Java access rules.")//
-    public static final HostedOptionKey<Boolean> LayeredBaseImageAnalysis = new HostedOptionKey<>(false);
+    @Option(help = "Force include include all public types and methods that can be reached using normal Java access rules.")//
+    public static final HostedOptionKey<Boolean> UseBaseLayerInclusionPolicy = new HostedOptionKey<>(false);
 
     @Option(help = "Support for calls via the Java Foreign Function and Memory API", type = Expert) //
     public static final HostedOptionKey<Boolean> ForeignAPISupport = new HostedOptionKey<>(false);

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

@@ -1299,7 +1299,7 @@ private static Inflation createBigBang(DebugContext debug, OptionValues options,
         ConstantReflectionProvider constantReflectionProvider = aProviders.getConstantReflection();
         WordTypes wordTypes = aProviders.getWordTypes();
         String reason = "Included in the base image";
-        ClassInclusionPolicy classInclusionPolicy = SubstrateOptions.LayeredBaseImageAnalysis.getValue(options) ? new ClassInclusionPolicy.LayeredBaseImageInclusionPolicy(reason)
+        ClassInclusionPolicy classInclusionPolicy = SubstrateOptions.UseBaseLayerInclusionPolicy.getValue(options) ? new ClassInclusionPolicy.LayeredBaseImageInclusionPolicy(reason)
                         : new ClassInclusionPolicy.DefaultAllInclusionPolicy(reason);
         if (PointstoOptions.UseExperimentalReachabilityAnalysis.getValue(options)) {
             ReachabilityMethodProcessingHandler reachabilityMethodProcessingHandler;

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

@@ -202,6 +202,9 @@ public enum UsageKind {
     private final boolean isClosedTypeWorld = SubstrateOptions.useClosedTypeWorld();
     private final boolean enableTrackAcrossLayers;
 
+    /** Modules containing all {@code svm.core} and {@code svm.hosted} classes. */
+    private final Set<Module> builderModules;
+
     @SuppressWarnings("this-escape")
     public SVMHost(OptionValues options, ImageClassLoader loader, ClassInitializationSupport classInitializationSupport, AnnotationSubstitutionProcessor annotationSubstitutions,
                     MissingRegistrationSupport missingRegistrationSupport) {
@@ -235,6 +238,23 @@ public SVMHost(OptionValues options, ImageClassLoader loader, ClassInitializatio
         }
 
         enableTrackAcrossLayers = ImageLayerBuildingSupport.buildingSharedLayer();
+        builderModules = getBuilderModules();
+    }
+
+    private static Set<Module> getBuilderModules() {
+        Module m0 = ImageSingletons.lookup(VMFeature.class).getClass().getModule();
+        Module m1 = SVMHost.class.getModule();
+        return m0.equals(m1) ? Set.of(m0) : Set.of(m0, m1);
+    }
+
+    /**
+     * Returns true if the type is part of the {@code svm.core} module. Note that builderModules
+     * also encloses the {@code svm.hosted} classes, but since those classes are not allowed at run
+     * time then they cannot be an {@link AnalysisType}.
+     */
+    @Override
+    public boolean isCoreType(AnalysisType type) {
+        return builderModules.contains(type.getJavaClass().getModule());
     }
 
     @Override

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/HostedImageLayerBuildingSupport.java

@@ -131,7 +131,7 @@ public static void processLayerOptions(EconomicMap<OptionKey<?>, Object> values)
                     }
                 });
 
-                SubstrateOptions.LayeredBaseImageAnalysis.update(values, true);
+                SubstrateOptions.UseBaseLayerInclusionPolicy.update(values, true);
                 SubstrateOptions.ClosedTypeWorld.update(values, false);
                 if (imageLayerEnabledHandler != null) {
                     imageLayerEnabledHandler.onOptionEnabled(values);

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jdk/JDKInitializationFeature.java

@@ -58,6 +58,7 @@ public void afterRegistration(AfterRegistrationAccess access) {
         rci.initializeAtBuildTime("java.nio", JDK_CLASS_REASON);
         rci.initializeAtBuildTime("java.text", JDK_CLASS_REASON);
         rci.initializeAtBuildTime("java.time", JDK_CLASS_REASON);
+        rci.initializeAtRunTime("java.time.chrono.HijrahChronology", "Reads java.home in class initializer.");
         rci.initializeAtBuildTime("java.util", JDK_CLASS_REASON);
         rci.initializeAtRunTime("java.util.concurrent.SubmissionPublisher", "Executor service must be recomputed");