[GR-57552] Allow registerOpaqueMethodReturn/ReturnsAllInstantiatedTypes for methods returning primitives

PullRequest: graal/18646

Author: d-kozak

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

@@ -73,11 +73,10 @@ public MethodFlowsGraphInfo addContext(PointsToAnalysis bb, AnalysisContext call
                     newFlows.cloneOriginalFlows(bb);
                     newFlows.linkCloneFlows(bb);
                     /*
-                     * If this method returns all instantiated types, then it will not be linked to
-                     * any internal flows. Instead, it needs to be linked to its declared type's
-                     * flow.
+                     * If this method has opaque return, then it will not be linked to any internal
+                     * flows. Instead, it needs to be linked to its declared type's flow.
                      */
-                    if (flowsGraph.method.getReturnsAllInstantiatedTypes()) {
+                    if (flowsGraph.method.hasOpaqueReturn()) {
                         var newReturnFlow = newFlows.getReturnFlow();
                         newReturnFlow.getDeclaredType().getTypeFlow(bb, true).addUse(bb, newReturnFlow);
                     }

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

@@ -171,7 +171,7 @@ private synchronized void createFlowsGraph(PointsToAnalysis bb, InvokeTypeFlow r
                 }
                 bb.numParsedGraphs.incrementAndGet();
 
-                boolean computeIndex = !method.getReturnsAllInstantiatedTypes() && bb.getHostVM().getMultiMethodAnalysisPolicy().canComputeReturnedParameterIndex(method.getMultiMethodKey());
+                boolean computeIndex = !method.hasOpaqueReturn() && bb.getHostVM().getMultiMethodAnalysisPolicy().canComputeReturnedParameterIndex(method.getMultiMethodKey());
                 returnedParameterIndex = computeIndex ? computeReturnedParameterIndex(builder.graph) : -1;
 
                 /* Set the flows graph after fully built. */

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

@@ -468,11 +468,14 @@ private boolean handleNodeIntrinsic() {
         return false;
     }
 
-    private void insertAllInstantiatedTypesReturn() {
+    /**
+     * If a method has opaque return, we model it conservatively by returning a) all instantiated
+     * subtypes of its return type for an object type or b) any primitive value for a primitive.
+     */
+    private void handleOpaqueReturn() {
         AnalysisError.guarantee(flowsGraph.getReturnFlow() == null, "Expected null return flow");
 
         AnalysisType returnType = TypeFlow.filterUncheckedInterface(method.getSignature().getReturnType());
-        AnalysisError.guarantee(returnType.getJavaKind().isObject(), "Unexpected return type: %s", returnType);
 
         BytecodePosition position = AbstractAnalysisEngine.syntheticSourcePosition(null, method);
         var returnFlow = new FormalReturnTypeFlow(position, returnType);
@@ -626,9 +629,9 @@ protected void apply(boolean forceReparse, Object reason) {
              */
             AnalysisType returnType = method.getSignature().getReturnType();
             if (returnType.getJavaKind().isObject()) {
-                // GR-52421: the return type state should not be all-instantiated, it should be the
+                // GR-52421: the return should not be opaque, it should be the
                 // persisted result of the open-world analysis
-                insertAllInstantiatedTypesReturn();
+                handleOpaqueReturn();
             }
             // GR-52421: verify that tracked parameter state is subset of persisted state
             insertPlaceholderParamAndReturnFlows();
@@ -637,12 +640,12 @@ protected void apply(boolean forceReparse, Object reason) {
 
         // assert method.getAnnotation(Fold.class) == null : method;
         if (handleNodeIntrinsic()) {
-            assert !method.getReturnsAllInstantiatedTypes() : method;
+            assert !method.hasOpaqueReturn() : method;
             return;
         }
 
-        if (method.getReturnsAllInstantiatedTypes()) {
-            insertAllInstantiatedTypesReturn();
+        if (method.hasOpaqueReturn()) {
+            handleOpaqueReturn();
         }
 
         boolean insertPlaceholderFlows = bb.getHostVM().getMultiMethodAnalysisPolicy().insertPlaceholderParamAndReturnFlows(method.getMultiMethodKey());
@@ -954,10 +957,10 @@ protected void node(FixedNode n) {
 
             } else if (n instanceof ReturnNode) {
                 /*
-                 * Return type flows within the graph do not need to be linked when all instantiated
-                 * types are returned.
+                 * Return type flows within the graph do not need to be linked if the method has
+                 * opaque return.
                  */
-                if (!method.getReturnsAllInstantiatedTypes()) {
+                if (!method.hasOpaqueReturn()) {
                     ReturnNode node = (ReturnNode) n;
                     if (node.result() != null && bb.isSupportedJavaKind(node.result().getStackKind())) {
                         TypeFlowBuilder<?> returnFlowBuilder = uniqueReturnFlowBuilder(node);

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java

@@ -182,14 +182,13 @@ public record Signature(String name, AnalysisType[] parameterTypes) {
     @SuppressWarnings("unused") private volatile Object allImplementations;
 
     /**
-     * Indicates that this method returns all instantiated types. This is necessary when there are
-     * control flows present which cannot be tracked by analysis, which happens for continuation
-     * support.
+     * Indicates that this method has opaque return. This is necessary when there are control flows
+     * present which cannot be tracked by analysis, which happens for continuation support.
      *
      * This should only be set via calling
      * {@code FeatureImpl.BeforeAnalysisAccessImpl#registerOpaqueMethodReturn}.
      */
-    private boolean returnsAllInstantiatedTypes;
+    private boolean hasOpaqueReturn;
 
     @SuppressWarnings({"this-escape", "unchecked"})
     protected AnalysisMethod(AnalysisUniverse universe, ResolvedJavaMethod wrapped, MultiMethodKey multiMethodKey, Map<MultiMethodKey, MultiMethod> multiMethodMap) {
@@ -288,7 +287,7 @@ protected AnalysisMethod(AnalysisMethod original, MultiMethodKey multiMethodKey)
         this.multiMethodKey = multiMethodKey;
         assert original.multiMethodMap != null;
         multiMethodMap = original.multiMethodMap;
-        returnsAllInstantiatedTypes = original.returnsAllInstantiatedTypes;
+        hasOpaqueReturn = original.hasOpaqueReturn;
 
         if (PointstoOptions.TrackAccessChain.getValue(declaringClass.universe.hostVM().options())) {
             startTrackInvocations();
@@ -1138,12 +1137,12 @@ public AnalysisMethod getOrCreateMultiMethod(MultiMethodKey key, Consumer<Analys
      * This should only be set via calling
      * {@code FeatureImpl.BeforeAnalysisAccessImpl#registerOpaqueMethodReturn}.
      */
-    public void setReturnsAllInstantiatedTypes() {
-        returnsAllInstantiatedTypes = true;
+    public void setOpaqueReturn() {
+        hasOpaqueReturn = true;
     }
 
-    public boolean getReturnsAllInstantiatedTypes() {
-        return returnsAllInstantiatedTypes;
+    public boolean hasOpaqueReturn() {
+        return hasOpaqueReturn;
     }
 
     protected abstract AnalysisMethod createMultiMethod(AnalysisMethod analysisMethod, MultiMethodKey newMultiMethodKey);

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java

@@ -488,8 +488,8 @@ private void mergeConstantObjects(PointsToAnalysis bb) {
     public AllInstantiatedTypeFlow instantiatedTypesNonNull;
 
     /*
-     * Returns a type flow containing all types that are assignable from this type and are also
-     * instantiated.
+     * Returns a generic type flow containing a) all types that are assignable from this type and
+     * are also instantiated for objects or b) any primitive value for primitives.
      */
     public TypeFlow<?> getTypeFlow(@SuppressWarnings("unused") BigBang bb, boolean includeNull) {
         if (isPrimitive() || isWordType()) {

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/PointsToAnalysisMethod.java

@@ -244,8 +244,8 @@ public void cleanupAfterAnalysis() {
     }
 
     @Override
-    public void setReturnsAllInstantiatedTypes() {
-        super.setReturnsAllInstantiatedTypes();
-        assert !getTypeFlow().flowsGraphCreated() : "must call setReturnsAllInstantiatedTypes before typeflow is created";
+    public void setOpaqueReturn() {
+        super.setOpaqueReturn();
+        assert !getTypeFlow().flowsGraphCreated() : "must call setOpaqueReturn before typeflow is created";
     }
 }

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

@@ -492,16 +492,11 @@ public void registerFieldValueTransformer(Field field, FieldValueTransformer tra
         /**
          * Registers a method as having an analysis-opaque return value. This designation limits the
          * type-flow analysis performed on the method's return value.
-         *
-         * Currently we expect only methods with the Object return type to be registered via this
-         * method; however, the underlying analysis support can handle other object types (including
-         * untrusted interfaces).
          */
         public void registerOpaqueMethodReturn(Method method) {
             AnalysisMethod aMethod = bb.getMetaAccess().lookupJavaMethod(method);
             VMError.guarantee(aMethod.getAllMultiMethods().size() == 1, "Opaque method return called for method with >1 multimethods: %s ", method);
-            VMError.guarantee(method.getReturnType().equals(Object.class), "Called registerOpaqueMethodReturn for a method with a non-Object return type: %s", method);
-            aMethod.setReturnsAllInstantiatedTypes();
+            aMethod.setOpaqueReturn();
         }
     }
 

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/InlineBeforeAnalysisPolicyUtils.java

@@ -235,11 +235,11 @@ public static boolean inliningAllowed(SVMHost hostVM, GraphBuilderContext b, Ana
         if (!Uninterruptible.Utils.inliningAllowed(caller, callee)) {
             return false;
         }
-        if (callee.getReturnsAllInstantiatedTypes()) {
+        if (callee.hasOpaqueReturn()) {
             /*
-             * When a callee returns all instantiated types then it cannot be inlined. Inlining the
-             * method would expose the method's return values instead of treating it as an
-             * AllInstantiatedTypeFlow.
+             * When a callee has opaque return then it cannot be inlined. Inlining the method would
+             * expose the method's return values, which might change e.g. due to later
+             * intrinsification of the invoke.
              */
             return false;
         }