[GR-43619] Refactor points-to analysis.

PullRequest: graal/13711

Author: cstancu

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

@@ -27,12 +27,14 @@
 import java.util.BitSet;
 
 import org.graalvm.compiler.options.OptionValues;
+import org.graalvm.compiler.replacements.nodes.BasicArrayCopyNode;
 
 import com.oracle.graal.pointsto.api.PointstoOptions;
 import com.oracle.graal.pointsto.flow.AbstractSpecialInvokeTypeFlow;
 import com.oracle.graal.pointsto.flow.AbstractStaticInvokeTypeFlow;
 import com.oracle.graal.pointsto.flow.AbstractVirtualInvokeTypeFlow;
 import com.oracle.graal.pointsto.flow.ActualReturnTypeFlow;
+import com.oracle.graal.pointsto.flow.ArrayElementsTypeFlow;
 import com.oracle.graal.pointsto.flow.CloneTypeFlow;
 import com.oracle.graal.pointsto.flow.InvokeTypeFlow;
 import com.oracle.graal.pointsto.flow.MethodFlowsGraph;
@@ -68,6 +70,7 @@ public abstract class AnalysisPolicy {
     protected final int maxHeapContextDepth;
     protected final boolean limitObjectArrayLength;
     protected final int maxObjectSetSize;
+    protected final boolean hybridStaticContext;
 
     public AnalysisPolicy(OptionValues options) {
         this.options = options;
@@ -80,7 +83,7 @@ public AnalysisPolicy(OptionValues options) {
         maxHeapContextDepth = PointstoOptions.MaxHeapContextDepth.getValue(options);
         limitObjectArrayLength = PointstoOptions.LimitObjectArrayLength.getValue(options);
         maxObjectSetSize = PointstoOptions.MaxObjectSetSize.getValue(options);
-
+        hybridStaticContext = PointstoOptions.HybridStaticContext.getValue(options);
     }
 
     public abstract boolean isContextSensitiveAnalysis();
@@ -113,6 +116,10 @@ public int maxObjectSetSize() {
         return maxObjectSetSize;
     }
 
+    public boolean useHybridStaticContext() {
+        return hybridStaticContext;
+    }
+
     public abstract MethodTypeFlow createMethodTypeFlow(PointsToAnalysisMethod method);
 
     /**
@@ -272,4 +279,32 @@ public final TypeState doSubtraction(PointsToAnalysis bb, SingleTypeState s1, Mu
     public abstract TypeState doSubtraction(PointsToAnalysis bb, MultiTypeState s1, SingleTypeState s2);
 
     public abstract TypeState doSubtraction(PointsToAnalysis bb, MultiTypeState s1, MultiTypeState s2);
+
+    public abstract void processArrayCopyStates(PointsToAnalysis bb, TypeState srcArrayState, TypeState dstArrayState);
+
+    /**
+     * System.arraycopy() type compatibility is defined as: can elements of the source array be
+     * converted to the component type of the destination array by assignment conversion.
+     * <p>
+     * System.arraycopy() semantics doesn't check the compatibility of the source and destination
+     * arrays statically, it instead relies on runtime checks to verify the compatibility between
+     * the copied objects and the destination array. For example System.arraycopy() can copy from an
+     * Object[] to SomeOtherObject[]. That's why {@link ArrayElementsTypeFlow} tests each individual
+     * copied object for compatibility with the defined type of the destination array and filters
+     * out those not assignable. From System.arraycopy() javadoc: "...if any actual component of the
+     * source array [...] cannot be converted to the component type of the destination array by
+     * assignment conversion, an ArrayStoreException is thrown."
+     * <p>
+     * Here we detect incompatible types eagerly, i.e., array types whose elements would be filtered
+     * out by ArrayElementsTypeFlow anyway, by checking
+     * {@code dstType.isAssignableFrom(srcType) || srcType.isAssignableFrom(dstType)}.
+     * <p>
+     * By skipping incompatible types when modeling an {@link BasicArrayCopyNode} we avoid adding
+     * any use links between ArrayElementsTypeFlow that would filter out all elements anyway. (Note
+     * that the filter in ArrayElementsTypeFlow is still necessary for partially compatible copying,
+     * e.g., when copying from an array to another array of one of its subtypes.)
+     */
+    protected static boolean areTypesCompatibleForSystemArraycopy(AnalysisType srcType, AnalysisType dstType) {
+        return dstType.isAssignableFrom(srcType) || srcType.isAssignableFrom(dstType);
+    }
 }

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

@@ -25,7 +25,6 @@
 package com.oracle.graal.pointsto.flow;
 
 import com.oracle.graal.pointsto.PointsToAnalysis;
-import com.oracle.graal.pointsto.flow.context.object.AnalysisObject;
 import com.oracle.graal.pointsto.meta.AnalysisType;
 import com.oracle.graal.pointsto.typestate.TypeState;
 import com.oracle.graal.pointsto.util.AnalysisError;
@@ -123,61 +122,7 @@ public void onObservedUpdate(PointsToAnalysis bb) {
     }
 
     private static void processStates(PointsToAnalysis bb, TypeState srcArrayState, TypeState dstArrayState) {
-        /*
-         * The source and destination array can have reference types which, although must be
-         * compatible, can be different.
-         */
-        for (AnalysisObject srcArrayObject : srcArrayState.objects(bb)) {
-            if (!srcArrayObject.type().isArray()) {
-                /*
-                 * Ignore non-array type. Sometimes the analysis cannot filter out non-array types
-                 * flowing into array copy, however this will fail at runtime.
-                 */
-                continue;
-            }
-            assert srcArrayObject.type().isArray();
-
-            if (srcArrayObject.isPrimitiveArray() || srcArrayObject.isEmptyObjectArrayConstant(bb)) {
-                /* Nothing to read from a primitive array or an empty array constant. */
-                continue;
-            }
-
-            ArrayElementsTypeFlow srcArrayElementsFlow = srcArrayObject.getArrayElementsFlow(bb, false);
-
-            for (AnalysisObject dstArrayObject : dstArrayState.objects(bb)) {
-                if (!dstArrayObject.type().isArray()) {
-                    /* Ignore non-array type. */
-                    continue;
-                }
-
-                assert dstArrayObject.type().isArray();
-
-                if (dstArrayObject.isPrimitiveArray() || dstArrayObject.isEmptyObjectArrayConstant(bb)) {
-                    /* Cannot write to a primitive array or an empty array constant. */
-                    continue;
-                }
-
-                /*
-                 * As far as the ArrayCopyTypeFlow is concerned the source and destination types can
-                 * be compatible or not, where compatibility is defined as: the component of the
-                 * source array can be converted to the component type of the destination array by
-                 * assignment conversion. System.arraycopy() semantics doesn't check the
-                 * compatibility of the source and destination arrays, it instead relies on runtime
-                 * checks of the compatibility of the copied objects and the destination array. For
-                 * example System.arraycopy() can copy from an Object[] to SomeOtherObject[]. In
-                 * this case a check dstArrayObject.type().isAssignableFrom(srcArrayObject.type()
-                 * would fail but it is actually a valid use. That's why ArrayElementsTypeFlow will
-                 * test each individual copied object for compatibility with the defined type of the
-                 * destination array and filter out those not assignable. From System.arraycopy()
-                 * javadoc: "...if any actual component of the source array from position srcPos
-                 * through srcPos+length-1 cannot be converted to the component type of the
-                 * destination array by assignment conversion, an ArrayStoreException is thrown."
-                 */
-
-                ArrayElementsTypeFlow dstArrayElementsFlow = dstArrayObject.getArrayElementsFlow(bb, true);
-                srcArrayElementsFlow.addUse(bb, dstArrayElementsFlow);
-            }
-        }
+        bb.analysisPolicy().processArrayCopyStates(bb, srcArrayState, dstArrayState);
     }
 
     @Override
@@ -195,8 +140,6 @@ public TypeFlow<?> destination() {
 
     @Override
     public String toString() {
-        StringBuilder str = new StringBuilder();
-        str.append("ArrayCopyTypeFlow<").append(getState()).append(">");
-        return str.toString();
+        return "ArrayCopyTypeFlow<" + getState() + ">";
     }
 }

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

@@ -117,7 +117,7 @@ public BytecodeAnalysisContext staticCalleeContext(PointsToAnalysis bb, Bytecode
          * the invoke location, otherwise just reuse the caller context.
          */
 
-        if (!PointstoOptions.HybridStaticContext.getValue(bb.getOptions())) {
+        if (!bb.analysisPolicy().useHybridStaticContext()) {
             return callerContext;
         }
 

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

@@ -77,6 +77,7 @@
 
 import jdk.vm.ci.code.BytecodePosition;
 import jdk.vm.ci.meta.JavaConstant;
+import jdk.vm.ci.meta.JavaKind;
 
 public final class BytecodeSensitiveAnalysisPolicy extends AnalysisPolicy {
 
@@ -539,9 +540,9 @@ public TypeState doUnion(PointsToAnalysis bb, MultiTypeState state1, SingleTypeS
             return result;
         } else {
             AnalysisObject[] resultObjects;
-            if (s2.exactType().getId() < s1.firstType().getId()) {
+            if (s2.exactType().getId() < s1.firstTypeId()) {
                 resultObjects = TypeStateUtils.concat(so2, so1);
-            } else if (s2.exactType().getId() > s1.lastType().getId()) {
+            } else if (s2.exactType().getId() > s1.lastTypeId()) {
                 resultObjects = TypeStateUtils.concat(so1, so2);
             } else {
 
@@ -598,7 +599,7 @@ private TypeState doUnion0(PointsToAnalysis bb, ContextSensitiveMultiTypeState s
 
         /* Speculate that s1 and s2 are distinct sets. */
 
-        if (s1.lastType().getId() < s2.firstType().getId()) {
+        if (s1.lastTypeId() < s2.firstTypeId()) {
             /* Speculate that objects in s2 follow after objects in s1. */
 
             /* Concatenate the objects. */
@@ -612,7 +613,7 @@ private TypeState doUnion0(PointsToAnalysis bb, ContextSensitiveMultiTypeState s
             PointsToStats.registerUnionOperation(bb, s1, s2, result);
             return result;
 
-        } else if (s2.lastType().getId() < s1.firstType().getId()) {
+        } else if (s2.lastTypeId() < s1.firstTypeId()) {
             /* Speculate that objects in s1 follow after objects in s2. */
 
             /* Concatenate the objects. */
@@ -1178,4 +1179,69 @@ private static TypeState doSubtraction2(PointsToAnalysis bb, ContextSensitiveMul
             }
         }
     }
+
+    @Override
+    public void processArrayCopyStates(PointsToAnalysis bb, TypeState srcArrayState, TypeState dstArrayState) {
+        /*
+         * The types-objects iterator uses a sliding window to iterate over the objects of a type,
+         * and can quickly skip over the objects of specific types. This iterator is ideal since we
+         * want to quickly skip over the objects in the source whose type is not compatible with the
+         * destination (see AnalysisPolicy.areTypesCompatibleForSystemArraycopy() for details). This
+         * iterator also has the ability to reset back to the beginning of a type's partition, so
+         * after processing all objects in a pair of source-destination compatible types you can
+         * reset the source type partition back to the beginning and look for the next compatible
+         * destination type.
+         * 
+         * Although the resulting code is still quadratic from an algorithmic complexity standpoint,
+         * in practice you can never reach the asymptotic complexity given that in a usual type
+         * hierarchy most types are not compatible with most other types for System.arraycopy().
+         */
+        TypesObjectsIterator srcIterator = new TypesObjectsIterator(srcArrayState);
+        while (srcIterator.hasNextType()) {
+            AnalysisType srcType = srcIterator.nextType();
+            if (!isObjectArrayType(srcType)) {
+                srcIterator.skipObjects(srcType);
+                continue;
+            }
+            srcIterator.memoizePosition();
+            TypesObjectsIterator dstIterator = new TypesObjectsIterator(dstArrayState);
+            while (dstIterator.hasNextType()) {
+                AnalysisType dstType = dstIterator.nextType();
+                if (!isObjectArrayType(dstType)) {
+                    dstIterator.skipObjects(dstType);
+                    continue;
+                }
+                if (areTypesCompatibleForSystemArraycopy(srcType, dstType)) {
+                    srcIterator.reset();
+                    dstIterator.memoizePosition();
+                    while (srcIterator.hasNextObject(srcType)) {
+                        AnalysisObject srcObject = srcIterator.nextObject(srcType);
+                        if (srcObject.isEmptyObjectArrayConstant(bb)) {
+                            continue;
+                        }
+                        var srcArrayElements = srcObject.getArrayElementsFlow(bb, true);
+                        dstIterator.reset();
+                        while (dstIterator.hasNextObject(dstType)) {
+                            AnalysisObject dstObject = dstIterator.nextObject(dstType);
+                            if (dstObject.isEmptyObjectArrayConstant(bb)) {
+                                continue;
+                            }
+                            var dstArrayElements = dstObject.getArrayElementsFlow(bb, true);
+                            srcArrayElements.addUse(bb, dstArrayElements);
+                        }
+                    }
+                } else {
+                    dstIterator.skipObjects(dstType);
+                }
+            }
+        }
+    }
+
+    /*
+     * We ignore non-array types. Sometimes the analysis cannot filter out non-array types flowing
+     * into array copy, however this will fail at runtime.
+     */
+    private static boolean isObjectArrayType(AnalysisType type) {
+        return type.isArray() && type.getComponentType().getJavaKind() == JavaKind.Object;
+    }
 }

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

@@ -106,10 +106,7 @@ public void onObservedUpdate(PointsToAnalysis bb) {
                  * Type states can be conservative, i.e., we can have receiver types that do not
                  * implement the method. Just ignore such types.
                  */
-                while (toi.hasNextObject(type)) {
-                    // skip the rest of the objects of the same type
-                    toi.nextObject(type);
-                }
+                toi.skipObjects(type);
                 continue;
             }
 

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

@@ -33,9 +33,7 @@
 import com.oracle.graal.pointsto.flow.context.object.AnalysisObject;
 import com.oracle.graal.pointsto.meta.AnalysisType;
 import com.oracle.graal.pointsto.typestate.MultiTypeState;
-import com.oracle.graal.pointsto.typestate.PointsToStats;
 import com.oracle.graal.pointsto.typestate.TypeState;
-import com.oracle.graal.pointsto.typestate.TypeStateUtils;
 
 public class ContextSensitiveMultiTypeState extends MultiTypeState {
 
@@ -48,33 +46,16 @@ public class ContextSensitiveMultiTypeState extends MultiTypeState {
     public ContextSensitiveMultiTypeState(PointsToAnalysis bb, boolean canBeNull, int properties, BitSet typesBitSet, AnalysisObject... objects) {
         super(bb, canBeNull, properties, typesBitSet);
         this.objects = objects;
-        /*
-         * Trim the typesBitSet to size eagerly. The typesBitSet is effectively immutable, i.e., no
-         * calls to mutating methods are made on it after it is set in the MultiTypeState, thus we
-         * don't need to use any external synchronization. However, to keep it immutable we use
-         * BitSet.clone() when deriving a new BitSet since the set operations (and, or, etc.) mutate
-         * the original object. The problem is that BitSet.clone() breaks the informal contract that
-         * the clone method should not modify the original object; it calls trimToSize() before
-         * creating a copy. Thus, trimming the bit set here ensures that cloning does not modify the
-         * typesBitSet. Since BitSet is not thread safe mutating it during cloning is problematic in
-         * a multithreaded environment. If for example you iterate over the bits at the same time as
-         * another thread calls clone() the words[] array can be in an inconsistent state.
-         */
-        TypeStateUtils.trimBitSetToSize(typesBitSet);
-        long cardinality = typesBitSet.cardinality();
-        assert cardinality < Integer.MAX_VALUE : "We don't expect so much types.";
         assert typesCount > 1 : "Multi type state with single type.";
         assert objects.length > 1 : "Multi type state with single object.";
         assert !bb.extendedAsserts() || checkObjects(bb);
-        PointsToStats.registerTypeState(bb, this);
     }
 
     /** Create a type state with the same content and a reversed canBeNull value. */
     protected ContextSensitiveMultiTypeState(PointsToAnalysis bb, boolean canBeNull, ContextSensitiveMultiTypeState other) {
         super(bb, canBeNull, other);
         this.objects = other.objects;
         this.merged = other.merged;
-        PointsToStats.registerTypeState(bb, this);
     }
 
     protected BitSet bitSet() {
@@ -130,14 +111,18 @@ protected Iterator<AnalysisObject> objectsIterator(BigBang bb) {
         return Arrays.asList(objects).iterator();
     }
 
-    /** Get the type of the first object group. */
-    public AnalysisType firstType() {
-        return objects[0].type();
+    /**
+     * Get the type of the first object group.
+     */
+    public int firstTypeId() {
+        return objects[0].getTypeId();
     }
 
-    /** Get the type of the last object group. */
-    public AnalysisType lastType() {
-        return objects[objects.length - 1].type();
+    /**
+     * Get the type of the last object group.
+     */
+    public int lastTypeId() {
+        return objects[objects.length - 1].getTypeId();
     }
 
     @Override

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

@@ -24,15 +24,13 @@
  */
 package com.oracle.graal.pointsto.flow.context.bytecode;
 
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Iterator;
 
 import com.oracle.graal.pointsto.BigBang;
 import com.oracle.graal.pointsto.PointsToAnalysis;
 import com.oracle.graal.pointsto.flow.context.object.AnalysisObject;
 import com.oracle.graal.pointsto.meta.AnalysisType;
-import com.oracle.graal.pointsto.typestate.PointsToStats;
 import com.oracle.graal.pointsto.typestate.SingleTypeState;
 import com.oracle.graal.pointsto.typestate.TypeState;
 
@@ -42,26 +40,17 @@ public class ContextSensitiveSingleTypeState extends SingleTypeState {
     /** The objects of this type state. */
     protected final AnalysisObject[] objects;
 
-    public ContextSensitiveSingleTypeState(PointsToAnalysis bb, boolean canBeNull, int properties, AnalysisType type, ArrayList<AnalysisObject> objects) {
-        this(bb, canBeNull, properties, type, objects.toArray(AnalysisObject.EMPTY_ARRAY));
-        assert objects.size() > 0 : "Single type state with no objects.";
-    }
-
     /** Creates a new type state from incoming objects. */
     public ContextSensitiveSingleTypeState(PointsToAnalysis bb, boolean canBeNull, int properties, AnalysisType type, AnalysisObject... objects) {
         super(bb, canBeNull, properties, type);
         this.objects = objects;
         assert !bb.extendedAsserts() || checkObjects(bb);
-
-        PointsToStats.registerTypeState(bb, this);
     }
 
     /** Create a type state with the same content and a reversed canBeNull value. */
     protected ContextSensitiveSingleTypeState(PointsToAnalysis bb, boolean canBeNull, ContextSensitiveSingleTypeState other) {
         super(bb, canBeNull, other);
         this.objects = other.objects;
-
-        PointsToStats.registerTypeState(bb, this);
     }
 
     protected boolean checkObjects(BigBang bb) {