[GR-29145] Points-to analysis constant propagation in default policy.

PullRequest: graal/11594

Author: cstancu

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

@@ -75,6 +75,8 @@ public abstract class AbstractAnalysisEngine implements BigBang {
     private final HeapScanningPolicy heapScanningPolicy;
 
     protected final Boolean extendedAsserts;
+    protected final int maxConstantObjectsPerType;
+    protected final boolean profileConstantObjects;
 
     protected final OptionValues options;
     protected final DebugContext debug;
@@ -115,6 +117,8 @@ public AbstractAnalysisEngine(OptionValues options, AnalysisUniverse universe, H
         this.analysisTimer = timerCollection.get(TimerCollection.Registry.ANALYSIS);
 
         this.extendedAsserts = PointstoOptions.ExtendedAsserts.getValue(options);
+        maxConstantObjectsPerType = PointstoOptions.MaxConstantObjectsPerType.getValue(options);
+        profileConstantObjects = PointstoOptions.ProfileConstantObjects.getValue(options);
 
         this.heapScanningPolicy = PointstoOptions.ExhaustiveHeapScan.getValue(options)
                         ? HeapScanningPolicy.scanAll()
@@ -230,6 +234,17 @@ public boolean extendedAsserts() {
         return extendedAsserts;
     }
 
+    public int maxConstantObjectsPerType() {
+        return maxConstantObjectsPerType;
+    }
+
+    public void profileConstantObject(AnalysisType type) {
+        if (profileConstantObjects) {
+            PointsToAnalysis.ConstantObjectsProfiler.registerConstant(type);
+            PointsToAnalysis.ConstantObjectsProfiler.maybeDumpConstantHistogram();
+        }
+    }
+
     @Override
     public OptionValues getOptions() {
         return options;

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

@@ -67,10 +67,8 @@ public boolean forNonNullFieldValue(JavaConstant receiver, AnalysisField field,
 
         /* Add the constant value object to the field's type flow. */
         FieldTypeFlow fieldTypeFlow = getFieldTypeFlow(field, receiver);
-        AnalysisObject constantObject = bb.analysisPolicy().createConstantObject(analysis, fieldValue, fieldType);
         /* Add the new constant to the field's flow state. */
-        TypeState constantTypeState = TypeState.forNonNullObject(analysis, constantObject);
-        return fieldTypeFlow.addState(analysis, constantTypeState);
+        return fieldTypeFlow.addState(analysis, bb.analysisPolicy().constantTypeState(analysis, fieldValue, fieldType));
     }
 
     /**
@@ -107,10 +105,8 @@ public boolean forNullArrayElement(JavaConstant array, AnalysisType arrayType, i
     public boolean forNonNullArrayElement(JavaConstant array, AnalysisType arrayType, JavaConstant elementConstant, AnalysisType elementType, int elementIndex, ScanReason reason) {
         ArrayElementsTypeFlow arrayObjElementsFlow = getArrayElementsFlow(array, arrayType);
         PointsToAnalysis analysis = getAnalysis();
-        AnalysisObject constantObject = bb.analysisPolicy().createConstantObject(analysis, elementConstant, elementType);
         /* Add the constant element to the constant's array type flow. */
-        TypeState elementTypeState = TypeState.forNonNullObject(analysis, constantObject);
-        return arrayObjElementsFlow.addState(analysis, elementTypeState);
+        return arrayObjElementsFlow.addState(analysis, bb.analysisPolicy().constantTypeState(analysis, elementConstant, elementType));
     }
 
     /**

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

@@ -123,6 +123,9 @@ public int typeFlowSaturationCutoff() {
     /** Create a constant object abstraction. */
     public abstract AnalysisObject createConstantObject(PointsToAnalysis bb, JavaConstant constant, AnalysisType exactType);
 
+    /** Wrap a constant into a type state abstraction. */
+    public abstract TypeState constantTypeState(PointsToAnalysis bb, JavaConstant constant, AnalysisType exactType);
+
     /** Create type state for dynamic new instance. */
     public abstract TypeState dynamicNewInstanceState(PointsToAnalysis bb, TypeState currentState, TypeState newState, BytecodePosition allocationSite, AnalysisContext allocationContext);
 
@@ -135,7 +138,7 @@ public int typeFlowSaturationCutoff() {
      */
     public abstract void linkClonedObjects(PointsToAnalysis bb, TypeFlow<?> inputFlow, CloneTypeFlow cloneFlow, BytecodePosition source);
 
-    public abstract FieldTypeStore createFieldTypeStore(AnalysisObject object, AnalysisField field, AnalysisUniverse universe);
+    public abstract FieldTypeStore createFieldTypeStore(PointsToAnalysis bb, AnalysisObject object, AnalysisField field, AnalysisUniverse universe);
 
     public abstract ArrayElementsTypeStore createArrayElementsTypeStore(AnalysisObject object, AnalysisUniverse universe);
 

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

@@ -68,8 +68,9 @@ public class PointstoOptions {
     @Option(help = "The maximum number of objects recorded for each type of a type state before disabling heap sensitivity for that type. The analysis must be heap sensitive. It has a minimum value of 1.")//
     public static final OptionKey<Integer> MaxObjectSetSize = new OptionKey<>(100);
 
-    @Option(help = "The maximum number of constant objects recorded for each type before merging the constants into one unique constant object per type. The analysis must be heap sensitive. It has a minimum value of 1.")//
-    public static final OptionKey<Integer> MaxConstantObjectsPerType = new OptionKey<>(100);
+    @Option(help = "The maximum number of constant objects recorded for each type before merging the constants into one unique constant object per type. " +
+                    "If the value is 0 there is no limit.")//
+    public static final OptionKey<Integer> MaxConstantObjectsPerType = new OptionKey<>(0);
 
     @Option(help = "Track the progress of the static analysis.")//
     public static final OptionKey<Boolean> ProfileAnalysisOperations = new OptionKey<>(false);

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

@@ -143,6 +143,10 @@ public EconomicMap<Object, InvokeTypeFlow> getInvokes() {
         return flowsGraph == null ? EconomicMap.emptyMap() : flowsGraph.getInvokes();
     }
 
+    public TypeFlow<?> getParameter(int idx) {
+        return flowsGraph == null ? null : flowsGraph.getParameter(idx);
+    }
+
     public Iterable<TypeFlow<?>> getParameters() {
         return flowsGraph == null ? Collections.emptyList() : Arrays.asList(flowsGraph.getParameters());
     }

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/context/bytecode/BytecodeSensitiveAnalysisPolicy.java

@@ -54,6 +54,7 @@
 import com.oracle.graal.pointsto.flow.context.AnalysisContext;
 import com.oracle.graal.pointsto.flow.context.object.AllocationContextSensitiveObject;
 import com.oracle.graal.pointsto.flow.context.object.AnalysisObject;
+import com.oracle.graal.pointsto.flow.context.object.ConstantContextSensitiveObject;
 import com.oracle.graal.pointsto.meta.AnalysisField;
 import com.oracle.graal.pointsto.meta.AnalysisType;
 import com.oracle.graal.pointsto.meta.AnalysisUniverse;
@@ -152,12 +153,18 @@ public AnalysisObject createHeapObject(PointsToAnalysis bb, AnalysisType type, B
     public AnalysisObject createConstantObject(PointsToAnalysis bb, JavaConstant constant, AnalysisType exactType) {
         /* Get the analysis object wrapping the JavaConstant. */
         if (bb.trackConcreteAnalysisObjects(exactType)) {
-            return exactType.getCachedConstantObject(bb, constant);
+            return exactType.getCachedConstantObject(bb, constant, (c) -> new ConstantContextSensitiveObject(bb, exactType, c));
         } else {
             return exactType.getContextInsensitiveAnalysisObject();
         }
     }
 
+    @Override
+    public TypeState constantTypeState(PointsToAnalysis bb, JavaConstant constant, AnalysisType exactType) {
+        AnalysisObject constantObject = bb.analysisPolicy().createConstantObject(bb, constant, exactType);
+        return TypeState.forNonNullObject(bb, constantObject);
+    }
+
     @Override
     public TypeState dynamicNewInstanceState(PointsToAnalysis bb, TypeState currentState, TypeState newState, BytecodePosition allocationSite, AnalysisContext allocationContext) {
         /* Generate a heap object for every new incoming type. */
@@ -258,7 +265,7 @@ public void linkClonedObjects(PointsToAnalysis bb, TypeFlow<?> inputFlow, CloneT
     }
 
     @Override
-    public FieldTypeStore createFieldTypeStore(AnalysisObject object, AnalysisField field, AnalysisUniverse universe) {
+    public FieldTypeStore createFieldTypeStore(PointsToAnalysis bb, AnalysisObject object, AnalysisField field, AnalysisUniverse universe) {
         assert PointstoOptions.AllocationSiteSensitiveHeap.getValue(options);
         if (object.isContextInsensitiveObject()) {
             /*

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/context/object/AnalysisObject.java

@@ -60,6 +60,7 @@ public class AnalysisObject implements Comparable<AnalysisObject> {
     protected enum AnalysisObjectKind {
         /** The types of runtime objects that the analysis models. */
         ContextInsensitive("!S"),
+        ConstantObject("C"),
         AllocationContextSensitive("AS"),
         ConstantContextSensitive("CS");
 
@@ -82,8 +83,19 @@ protected enum AnalysisObjectKind {
     protected final AnalysisObjectKind kind;
 
     /**
-     * Is this a context sensitive object that was merged with a context insensitive object, or a
-     * context insensitive object that has merged some context sensitive objects?
+     * The merging of analysis objects is used to erase the identity of more concrete analysis
+     * objects (i.e., objects that wrap a constant, or retain information about their allocation
+     * location) and to replace them with the per-type context-insensitive analysis object.
+     * 
+     * This distinction between merged and un-merged objects is essential to correctly track field
+     * and array flows reads and writes. For example when a concrete analysis object is the receiver
+     * of a write operation then only the field flow associated with that receiver will get the
+     * state of the stored value, such that any read from the same receiver and field will return
+     * the written values, but no others. However, if there is a union between the concrete receiver
+     * and a context-insensitive object of the same type then the receiver needs to be marked as
+     * `merged` to signal that all reads from a field of this object must include all the state
+     * written to the corresponding field flow of the context-insensitive object and all writes must
+     * also flow in the corresponding field flow of the context-insensitive object.
      */
     protected volatile boolean merged;
 
@@ -171,8 +183,8 @@ public final boolean isConstantContextSensitiveObject() {
         return this.kind == AnalysisObjectKind.ConstantContextSensitive;
     }
 
-    public final boolean isContextSensitiveObject() {
-        return this.isAllocationContextSensitiveObject() || this.isConstantContextSensitiveObject();
+    public final boolean isConstantObject() {
+        return this.kind == AnalysisObjectKind.ConstantObject;
     }
 
     public ArrayElementsTypeStore getArrayElementsTypeStore() {
@@ -203,6 +215,10 @@ public UnsafeWriteSinkTypeFlow getUnsafeWriteSinkFrozenFilterFlow(PointsToAnalys
         return fieldTypeStore.unsafeWriteSinkFlow(bb);
     }
 
+    public FieldTypeStore getInstanceFieldTypeStore(PointsToAnalysis bb, AnalysisField field) {
+        return getInstanceFieldTypeStore(bb, null, null, field);
+    }
+
     /** Returns the instance field flow corresponding to a filed of the object's type. */
     public FieldTypeFlow getInstanceFieldFlow(PointsToAnalysis bb, AnalysisField field, boolean isStore) {
         return getInstanceFieldFlow(bb, null, null, field, isStore);
@@ -231,7 +247,7 @@ final FieldTypeStore getInstanceFieldTypeStore(PointsToAnalysis bb, TypeFlow<?>
 
         FieldTypeStore fieldStore = instanceFieldsTypeStore.get(field.getPosition());
         if (fieldStore == null) {
-            fieldStore = bb.analysisPolicy().createFieldTypeStore(this, field, bb.getUniverse());
+            fieldStore = bb.analysisPolicy().createFieldTypeStore(bb, this, field, bb.getUniverse());
             boolean result = instanceFieldsTypeStore.compareAndSet(field.getPosition(), null, fieldStore);
             if (result) {
                 fieldStore.init(bb);

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/context/object/ConstantContextSensitiveObject.java

@@ -33,8 +33,8 @@
 import jdk.vm.ci.meta.JavaConstant;
 
 /**
- * A context sensitive analysis object that represents a constant. The context for this analysis
- * object is the constant it wraps.
+ * A context-sensitive analysis object that represents a constant. The implicit context for this
+ * analysis object is the constant that it wraps.
  */
 public class ConstantContextSensitiveObject extends ContextSensitiveAnalysisObject {
 
@@ -76,6 +76,7 @@ public ConstantContextSensitiveObject(PointsToAnalysis bb, AnalysisType type, Ja
         super(bb.getUniverse(), type, AnalysisObjectKind.ConstantContextSensitive);
         assert bb.trackConcreteAnalysisObjects(type);
         this.constant = constant;
+        bb.profileConstantObject(type);
     }
 
     public JavaConstant getConstant() {
@@ -137,7 +138,7 @@ public String toString() {
         if (constant == null) {
             result.append("MERGED CONSTANT");
         } else {
-            // result.append(constant);
+            result.append(constant);
         }
         return result.toString();
     }

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/context/object/ContextSensitiveAnalysisObject.java

@@ -41,6 +41,13 @@
 
 import jdk.vm.ci.code.BytecodePosition;
 
+/**
+ * This class models analysis objects that retain some information about their allocation. So here
+ * context doesn't refer to calling context, but rather to the allocation context of this object
+ * (which could contain information about the calling context of its allocator). More importantly,
+ * this object abstraction is responsible four correct routing of field and array read and write
+ * flows.
+ */
 public class ContextSensitiveAnalysisObject extends AnalysisObject {
 
     public ContextSensitiveAnalysisObject(AnalysisUniverse universe, AnalysisType type, AnalysisObjectKind kind) {

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

@@ -39,6 +39,7 @@
 import java.util.concurrent.atomic.AtomicIntegerFieldUpdater;
 import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
 import java.util.function.Consumer;
+import java.util.function.Function;
 
 import org.graalvm.compiler.debug.GraalError;
 import org.graalvm.compiler.graph.Node;
@@ -47,9 +48,7 @@
 
 import com.oracle.graal.pointsto.BigBang;
 import com.oracle.graal.pointsto.PointsToAnalysis;
-import com.oracle.graal.pointsto.PointsToAnalysis.ConstantObjectsProfiler;
 import com.oracle.graal.pointsto.api.DefaultUnsafePartition;
-import com.oracle.graal.pointsto.api.PointstoOptions;
 import com.oracle.graal.pointsto.constraints.UnsupportedFeatureException;
 import com.oracle.graal.pointsto.flow.AllInstantiatedTypeFlow;
 import com.oracle.graal.pointsto.flow.context.object.AnalysisObject;
@@ -66,7 +65,6 @@
 import com.oracle.svm.util.UnsafePartitionKind;
 
 import jdk.vm.ci.meta.Assumptions.AssumptionResult;
-import jdk.vm.ci.meta.Constant;
 import jdk.vm.ci.meta.JavaConstant;
 import jdk.vm.ci.meta.JavaKind;
 import jdk.vm.ci.meta.PrimitiveConstant;
@@ -81,8 +79,8 @@ public abstract class AnalysisType extends AnalysisElement implements WrappedJav
     private static final AtomicReferenceFieldUpdater<AnalysisType, ConcurrentHashMap> UNSAFE_ACCESS_FIELDS_UPDATER = //
                     AtomicReferenceFieldUpdater.newUpdater(AnalysisType.class, ConcurrentHashMap.class, "unsafeAccessedFields");
 
-    private static final AtomicReferenceFieldUpdater<AnalysisType, ConstantContextSensitiveObject> UNIQUE_CONSTANT_UPDATER = //
-                    AtomicReferenceFieldUpdater.newUpdater(AnalysisType.class, ConstantContextSensitiveObject.class, "uniqueConstant");
+    private static final AtomicReferenceFieldUpdater<AnalysisType, AnalysisObject> UNIQUE_CONSTANT_UPDATER = //
+                    AtomicReferenceFieldUpdater.newUpdater(AnalysisType.class, AnalysisObject.class, "uniqueConstant");
 
     @SuppressWarnings("rawtypes")//
     private static final AtomicReferenceFieldUpdater<AnalysisType, Object> INTERCEPTORS_UPDATER = //
@@ -140,13 +138,13 @@ public abstract class AnalysisType extends AnalysisElement implements WrappedJav
     /** The unique context insensitive analysis object for this type. */
     private AnalysisObject contextInsensitiveAnalysisObject;
     /** Mapping from JavaConstant to the analysis ConstantObject. */
-    private ConcurrentMap<Constant, ConstantContextSensitiveObject> constantObjectsCache;
+    private ConcurrentMap<JavaConstant, AnalysisObject> constantObjectsCache;
     /**
      * A unique ConstantObject per analysis type. When the size of {@link #constantObjectsCache} is
      * above a threshold all the ConstantObject recorded until that moment are merged in the
      * {@link #uniqueConstant}.
      */
-    private volatile ConstantContextSensitiveObject uniqueConstant;
+    private volatile AnalysisObject uniqueConstant;
 
     /**
      * Cache for the resolved methods.
@@ -341,10 +339,10 @@ public AnalysisObject getUniqueConstantObject() {
         return uniqueConstant;
     }
 
-    public AnalysisObject getCachedConstantObject(PointsToAnalysis bb, JavaConstant constant) {
+    public AnalysisObject getCachedConstantObject(PointsToAnalysis bb, JavaConstant constant, Function<JavaConstant, AnalysisObject> constantTransformer) {
 
         /*
-         * Constant caching is only used we certain analysis policies. Ideally we would store the
+         * Constant caching is only used with certain analysis policies. Ideally we would store the
          * cache in the policy, but it is simpler to store the cache for each type.
          */
         assert bb.analysisPolicy().needsConstantCache() : "The analysis policy doesn't specify the need for a constants cache.";
@@ -356,41 +354,32 @@ public AnalysisObject getCachedConstantObject(PointsToAnalysis bb, JavaConstant
             return uniqueConstant;
         }
 
-        if (constantObjectsCache.size() >= PointstoOptions.MaxConstantObjectsPerType.getValue(bb.getOptions())) {
+        /* If maxConstantObjectsPerType is 0 there is no limit, i.e., we track all constants. */
+        if (bb.maxConstantObjectsPerType() > 0 && constantObjectsCache.size() >= bb.maxConstantObjectsPerType()) {
             // The number of constant objects has increased above the limit,
             // merge the constants in the uniqueConstant and return it
             mergeConstantObjects(bb);
             return uniqueConstant;
         }
 
-        // Get the analysis ConstantObject modeling the JavaConstant
-        AnalysisObject result = constantObjectsCache.get(constant);
-        if (result == null) {
-            // Create a ConstantObject to model each JavaConstant
-            ConstantContextSensitiveObject newValue = new ConstantContextSensitiveObject(bb, this, constant);
-            ConstantContextSensitiveObject oldValue = constantObjectsCache.putIfAbsent(constant, newValue);
-            result = oldValue != null ? oldValue : newValue;
-
-            if (PointstoOptions.ProfileConstantObjects.getValue(bb.getOptions())) {
-                ConstantObjectsProfiler.registerConstant(this);
-                ConstantObjectsProfiler.maybeDumpConstantHistogram();
-            }
-        }
-
-        return result;
+        /* Get the analysis ConstantObject modeling the JavaConstant. */
+        return constantObjectsCache.computeIfAbsent(constant, constantTransformer);
     }
 
     private void mergeConstantObjects(PointsToAnalysis bb) {
-        ConstantContextSensitiveObject uConstant = new ConstantContextSensitiveObject(bb, this, null);
+        ConstantContextSensitiveObject uConstant = new ConstantContextSensitiveObject(bb, this);
         if (UNIQUE_CONSTANT_UPDATER.compareAndSet(this, null, uConstant)) {
-            constantObjectsCache.values().stream().forEach(constantObject -> {
+            constantObjectsCache.values().forEach(constantObject -> {
                 /*
                  * The order of the two lines below matters: setting the merged flag first, before
                  * doing the actual merging, ensures that concurrent updates to the flow are still
                  * merged correctly.
                  */
-                constantObject.setMergedWithUniqueConstantObject();
-                constantObject.mergeInstanceFieldsFlows(bb, uniqueConstant);
+                if (constantObject instanceof ConstantContextSensitiveObject) {
+                    ConstantContextSensitiveObject ct = (ConstantContextSensitiveObject) constantObject;
+                    ct.setMergedWithUniqueConstantObject();
+                    ct.mergeInstanceFieldsFlows(bb, uniqueConstant);
+                }
             });
         }
     }