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 index dfc9d5df06e4..bae562b8bbb4 100644 --- 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 @@ -647,6 +647,27 @@ public boolean finish() throws InterruptedException { } } + @Override + public void afterAnalysis() { + /* + * Only verify in the context-insensitive configuration as context-sensitive analysis is not + * compatible with predicates. + */ + assert !PointstoOptions.AnalysisContextSensitivity.getValue(options).equals("insens") || validateFixedPointState(); + } + + /** + * This method checks that the typeflow graph is in a valid state when a fixed point is reached. + * The goal of this check is to detect cases where the analysis did not propagate all updates + * correctly (e.g. due to a concurrency bug) and provide concrete localized counter-examples to + * ease the debugging of such issues. + *

+ * As these checks can be expensive, this method should be executed only if asserts are enabled. + */ + public boolean validateFixedPointState() { + return universe.getMethods().parallelStream().allMatch(m -> m.validateFixedPointState(this)); + } + @SuppressWarnings("try") public boolean doTypeflow() throws InterruptedException { boolean didSomeWork; diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/ArrayElementsTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/ArrayElementsTypeFlow.java index 201f8925663c..4577bdd122cb 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/ArrayElementsTypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/ArrayElementsTypeFlow.java @@ -69,8 +69,11 @@ protected void onInputSaturated(PointsToAnalysis bb, TypeFlow input) { } } + /** + * Filters the incoming type state using the declared type. + */ @Override - public TypeState filter(PointsToAnalysis bb, TypeState update) { + protected TypeState processInputState(PointsToAnalysis bb, TypeState update) { if (declaredType.equals(bb.getObjectType())) { /* No need to filter. */ return update; diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/BooleanInstanceOfCheckTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/BooleanInstanceOfCheckTypeFlow.java index 0d9e851b34f0..6c7319d75188 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/BooleanInstanceOfCheckTypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/BooleanInstanceOfCheckTypeFlow.java @@ -57,8 +57,14 @@ public TypeFlow copy(PointsToAnalysis bb, MethodFlowsGraph met return new BooleanInstanceOfCheckTypeFlow(methodFlows, this); } + /** + * Creates a primitive type state that corresponds to the result of a type check of the incoming + * value. + * + * @return can be either empty, true, false, or any. + */ @Override - public TypeState filter(PointsToAnalysis bb, TypeState update) { + protected TypeState processInputState(PointsToAnalysis bb, TypeState update) { TypeState canBeTrue; TypeState canBeFalse; if (isExact) { diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/BooleanNullCheckTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/BooleanNullCheckTypeFlow.java index e66fbba961bb..8c3e58e6f848 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/BooleanNullCheckTypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/BooleanNullCheckTypeFlow.java @@ -48,8 +48,14 @@ public TypeFlow copy(PointsToAnalysis bb, MethodFlowsGraph met return new BooleanNullCheckTypeFlow(methodFlows, this); } + /** + * Creates a primitive type state that corresponds to the result of a null check of the incoming + * value. + * + * @return can be either empty, true, false, or any. + */ @Override - public TypeState filter(PointsToAnalysis bb, TypeState newState) { + protected TypeState processInputState(PointsToAnalysis bb, TypeState newState) { var hasNull = newState.canBeNull(); var hasTypes = newState.typesCount() > 0; return convertToBoolean(bb, hasNull, hasTypes); diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/BooleanPrimitiveCheckTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/BooleanPrimitiveCheckTypeFlow.java index ce4b1909d2aa..72a7f9f148ca 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/BooleanPrimitiveCheckTypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/BooleanPrimitiveCheckTypeFlow.java @@ -62,26 +62,22 @@ public TypeFlow copy(PointsToAnalysis bb, MethodFlowsGraph met return new BooleanPrimitiveCheckTypeFlow(bb, methodFlows, this); } - @Override - public boolean addState(PointsToAnalysis bb, TypeState add) { - return super.addState(bb, eval(bb)); - } - @Override protected void onInputSaturated(PointsToAnalysis bb, TypeFlow input) { /* * If an input saturated, it does not mean that the condition has to always saturate as * well, e.g. Any == {5} will return {5}. */ - super.addState(bb, eval(bb)); + addState(bb, TypeState.forEmpty()); } /** - * Compares the type states of left and right. + * Computes new type state of this flow by comparing the type states of left and right. * * @return can be either empty, true, false, or any. */ - public TypeState eval(PointsToAnalysis bb) { + @Override + protected TypeState processInputState(PointsToAnalysis bb, TypeState newState) { var leftState = left.getOutputState(bb); var rightState = right.getOutputState(bb); if (leftState.isEmpty() || rightState.isEmpty()) { diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/ConditionalFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/ConditionalFlow.java index 00c5ece7d606..6443d2b25666 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/ConditionalFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/ConditionalFlow.java @@ -74,25 +74,24 @@ protected void onInputSaturated(PointsToAnalysis bb, TypeFlow input) { * GR-58387: This could stop the propagation of saturation, so it can be problematic for * open-world analysis. */ - super.addState(bb, eval(bb)); + addState(bb, TypeState.forEmpty()); } @Override public void onObservedUpdate(PointsToAnalysis bb) { - super.addState(bb, eval(bb)); + addState(bb, TypeState.forEmpty()); } @Override public void onObservedSaturated(PointsToAnalysis bb, TypeFlow observed) { - super.addState(bb, eval(bb)); + addState(bb, TypeState.forEmpty()); } + /** + * Depending on the state of the condition, return none, one, or both of the true/false inputs. + */ @Override - public boolean addState(PointsToAnalysis bb, TypeState add) { - return super.addState(bb, eval(bb)); - } - - private TypeState eval(PointsToAnalysis bb) { + protected TypeState processInputState(PointsToAnalysis bb, TypeState newState) { TypeState trueState = trueValue.getOutputState(bb); TypeState falseState = falseValue.getOutputState(bb); if (condition.isSaturated()) { diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FieldFilterTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FieldFilterTypeFlow.java index 60c34f601a43..de88f68e037f 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FieldFilterTypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FieldFilterTypeFlow.java @@ -44,8 +44,11 @@ public FieldFilterTypeFlow(AnalysisField field) { super(field, field.getType()); } + /** + * Filter the incoming type state based on the declared type. + */ @Override - public TypeState filter(PointsToAnalysis bb, TypeState update) { + protected TypeState processInputState(PointsToAnalysis bb, TypeState update) { if (isPrimitiveFlow) { if (!update.isPrimitive()) { /* diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FilterTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FilterTypeFlow.java index 0517788e0e90..031ce6ad53a8 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FilterTypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FilterTypeFlow.java @@ -72,8 +72,11 @@ public TypeFlow copy(PointsToAnalysis bb, MethodFlowsGraph met return new FilterTypeFlow(methodFlows, this); } + /** + * Filter the incoming type state using the checked type. + */ @Override - public TypeState filter(PointsToAnalysis bb, TypeState update) { + protected TypeState processInputState(PointsToAnalysis bb, TypeState update) { TypeState result; if (isExact) { /* diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FormalParamTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FormalParamTypeFlow.java index 3a972ccf92a1..8b32db7f770e 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FormalParamTypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FormalParamTypeFlow.java @@ -52,8 +52,11 @@ public TypeFlow copy(PointsToAnalysis bb, MethodFlowsGraph met return new FormalParamTypeFlow(this, methodFlows); } + /** + * Filters the incoming type state using the declared type. + */ @Override - public TypeState filter(PointsToAnalysis bb, TypeState newState) { + protected TypeState processInputState(PointsToAnalysis bb, TypeState newState) { /* * If the type flow constraints are relaxed filter the incoming value using the parameter's * declared type. diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FormalReceiverTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FormalReceiverTypeFlow.java index d9b949f282b2..7ed1e4462b6a 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FormalReceiverTypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FormalReceiverTypeFlow.java @@ -48,8 +48,11 @@ public FormalReceiverTypeFlow copy(PointsToAnalysis bb, MethodFlowsGraph methodF return new FormalReceiverTypeFlow(this, methodFlows); } + /** + * Filters the incoming type state using the declared type. + */ @Override - public TypeState filter(PointsToAnalysis bb, TypeState newState) { + protected TypeState processInputState(PointsToAnalysis bb, TypeState newState) { /* * If the type flow constraints are relaxed filter the incoming value using the receiver's * declared type. diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FormalReturnTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FormalReturnTypeFlow.java index 26ce11f7cba7..00462cac83de 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FormalReturnTypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FormalReturnTypeFlow.java @@ -40,8 +40,11 @@ public FormalReturnTypeFlow(FormalReturnTypeFlow original, MethodFlowsGraph meth super(original, methodFlows); } + /** + * Filters the incoming type state using the declared type. + */ @Override - public TypeState filter(PointsToAnalysis bb, TypeState newState) { + protected TypeState processInputState(PointsToAnalysis bb, TypeState newState) { if (declaredType.getJavaKind() == JavaKind.Void) { /* * Void ReturnTypeFlow has a use edge from the latest predicate, which can propagate diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlowBuilder.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlowBuilder.java index 7930c5ad6ed2..8c6e75b6959d 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlowBuilder.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlowBuilder.java @@ -1316,16 +1316,34 @@ private TypeFlowBuilder uniqueReturnFlowBuilder(ReturnNode node) { private void handleCompareNode(ValueNode source, CompareNode condition, PrimitiveComparison comparison, boolean isUnsigned) { var xNode = typeFlowUnproxify(condition.getX()); var yNode = typeFlowUnproxify(condition.getY()); - var xConstant = xNode.isConstant(); - var yConstant = yNode.isConstant(); + /* Ensure that if one input is constant, it is always y. */ + PrimitiveComparison maybeFlipped; + if (xNode.isConstant() && !yNode.isConstant()) { + var tmp = xNode; + xNode = yNode; + yNode = tmp; + maybeFlipped = comparison.flip(); + } else { + maybeFlipped = comparison; + } var xFlow = state.lookup(xNode); - var yFlow = state.lookup(yNode); - if (!xConstant) { + if (yNode.isConstant()) { + TypeState rightState = TypeState.forPrimitiveConstant(bb, yNode.asJavaConstant().asLong()); + var builder = TypeFlowBuilder.create(bb, method, state.getPredicate(), source, PrimitiveFilterTypeFlow.class, () -> { + var flow = new PrimitiveFilterTypeFlow.ConstantFilter(AbstractAnalysisEngine.sourcePosition(source), xFlow.get().declaredType, xFlow.get(), rightState, maybeFlipped, isUnsigned); + flowsGraph.addNodeFlow(source, flow); + return flow; + }); + builder.addUseDependency(xFlow); + typeFlowGraphBuilder.registerSinkBuilder(builder); + state.update(xNode, builder); + state.setPredicate(builder); + } else { + var yFlow = state.lookup(yNode); var leftFlowBuilder = TypeFlowBuilder.create(bb, method, state.getPredicate(), source, PrimitiveFilterTypeFlow.class, () -> { - var flow = new PrimitiveFilterTypeFlow(AbstractAnalysisEngine.sourcePosition(source), xFlow.get().declaredType, xFlow.get(), yFlow.get(), comparison, isUnsigned); - if (yConstant) { - flowsGraph.addNodeFlow(source, flow); - } + var flow = new PrimitiveFilterTypeFlow.VariableFilter(AbstractAnalysisEngine.sourcePosition(source), xFlow.get().declaredType, xFlow.get(), yFlow.get(), maybeFlipped, + isUnsigned); + flowsGraph.addNodeFlow(source, flow); return flow; }); leftFlowBuilder.addUseDependency(xFlow); @@ -1333,13 +1351,11 @@ private void handleCompareNode(ValueNode source, CompareNode condition, Primitiv typeFlowGraphBuilder.registerSinkBuilder(leftFlowBuilder); state.update(xNode, leftFlowBuilder); state.setPredicate(leftFlowBuilder); - } - if (!yConstant) { var rightFlowBuilder = TypeFlowBuilder.create(bb, method, state.getPredicate(), source, PrimitiveFilterTypeFlow.class, () -> { - var flow = new PrimitiveFilterTypeFlow(AbstractAnalysisEngine.sourcePosition(source), yFlow.get().declaredType, yFlow.get(), xFlow.get(), comparison.flip(), isUnsigned); - flowsGraph.addNodeFlow(source, flow); + var flow = new PrimitiveFilterTypeFlow.VariableFilter(AbstractAnalysisEngine.sourcePosition(source), yFlow.get().declaredType, yFlow.get(), xFlow.get(), maybeFlipped.flip(), + isUnsigned); + flowsGraph.addMiscEntryFlow(flow); return flow; - }); rightFlowBuilder.addUseDependency(yFlow); rightFlowBuilder.addUseDependency(xFlow); diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/NullCheckTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/NullCheckTypeFlow.java index 2665c6aaee7b..d0512298cf79 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/NullCheckTypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/NullCheckTypeFlow.java @@ -59,8 +59,11 @@ public boolean isBlockingNull() { return blockNull; } + /** + * Filters the incoming type state by performing a null check. + */ @Override - public TypeState filter(PointsToAnalysis bb, TypeState newState) { + protected TypeState processInputState(PointsToAnalysis bb, TypeState newState) { if (blockNull) { return newState.forNonNull(bb); } else if (newState.canBeNull()) { diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/OffsetLoadTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/OffsetLoadTypeFlow.java index 5574bebc4d06..3b9087374a44 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/OffsetLoadTypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/OffsetLoadTypeFlow.java @@ -137,8 +137,11 @@ public void onObservedUpdate(PointsToAnalysis bb) { } } + /** + * Filters the incoming type state using the declared type. + */ @Override - public TypeState filter(PointsToAnalysis bb, TypeState newState) { + protected TypeState processInputState(PointsToAnalysis bb, TypeState newState) { /* * If the type flow constraints are relaxed filter the loaded value using the array's * declared type. diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/PrimitiveFilterTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/PrimitiveFilterTypeFlow.java index 0ef1d364b271..af98d1313387 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/PrimitiveFilterTypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/PrimitiveFilterTypeFlow.java @@ -38,56 +38,38 @@ * op and y, and the second filters y with respect to * op and x. */ -public class PrimitiveFilterTypeFlow extends TypeFlow { +public abstract class PrimitiveFilterTypeFlow extends TypeFlow { + protected final TypeFlow left; + protected final PrimitiveComparison comparison; + protected final boolean isUnsigned; - private final TypeFlow left; - private final TypeFlow right; - private final PrimitiveComparison comparison; - private final boolean isUnsigned; - - public PrimitiveFilterTypeFlow(BytecodePosition position, AnalysisType declaredType, TypeFlow left, TypeFlow right, PrimitiveComparison comparison, boolean isUnsigned) { + private PrimitiveFilterTypeFlow(BytecodePosition position, AnalysisType declaredType, TypeFlow left, PrimitiveComparison comparison, boolean isUnsigned) { super(position, declaredType); this.left = left; - this.right = right; this.comparison = comparison; this.isUnsigned = isUnsigned; } - private PrimitiveFilterTypeFlow(PointsToAnalysis bb, MethodFlowsGraph methodFlows, PrimitiveFilterTypeFlow original) { - super(original, methodFlows); - this.left = methodFlows.lookupCloneOf(bb, original.left); - this.right = methodFlows.lookupCloneOf(bb, original.right); - this.comparison = original.comparison; - this.isUnsigned = original.isUnsigned; - } - - @Override - public TypeFlow copy(PointsToAnalysis bb, MethodFlowsGraph methodFlows) { - return new PrimitiveFilterTypeFlow(bb, methodFlows, this); - } - - @Override - public boolean addState(PointsToAnalysis bb, TypeState add) { - return super.addState(bb, eval(bb)); - } - @Override protected void onInputSaturated(PointsToAnalysis bb, TypeFlow input) { /* * If an input saturated, it does not mean that the condition has to always saturate as * well, e.g. Any == 5 still returns 5. */ - super.addState(bb, eval(bb)); + addState(bb, TypeState.forEmpty()); } + public abstract TypeState getRightState(PointsToAnalysis bb); + /** * Filters the type state of left using condition and right. */ - private TypeState eval(PointsToAnalysis bb) { + @Override + protected TypeState processInputState(PointsToAnalysis bb, TypeState newState) { var leftState = left.getOutputState(bb); - var rightState = right.getOutputState(bb); + var rightState = getRightState(bb); assert leftState.isPrimitive() || leftState.isEmpty() : left; - assert rightState.isPrimitive() || rightState.isEmpty() : right; + assert rightState.isPrimitive() || rightState.isEmpty() : this; return TypeState.filter(leftState, comparison, rightState, isUnsigned); } @@ -98,4 +80,33 @@ public PrimitiveComparison getComparison() { public TypeFlow getLeft() { return left; } + + public static class ConstantFilter extends PrimitiveFilterTypeFlow { + private final TypeState rightState; + + public ConstantFilter(BytecodePosition position, AnalysisType declaredType, TypeFlow left, TypeState rightState, PrimitiveComparison comparison, boolean isUnsigned) { + super(position, declaredType, left, comparison, isUnsigned); + this.rightState = rightState; + } + + @Override + public TypeState getRightState(PointsToAnalysis bb) { + return rightState; + } + } + + public static class VariableFilter extends PrimitiveFilterTypeFlow { + private final TypeFlow right; + + public VariableFilter(BytecodePosition position, AnalysisType declaredType, TypeFlow left, TypeFlow right, PrimitiveComparison comparison, boolean isUnsigned) { + super(position, declaredType, left, comparison, isUnsigned); + this.right = right; + } + + @Override + public TypeState getRightState(PointsToAnalysis bb) { + return right.getOutputState(bb); + } + + } } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/StoreFieldTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/StoreFieldTypeFlow.java index 1acfd966cb51..89287dee949b 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/StoreFieldTypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/StoreFieldTypeFlow.java @@ -45,8 +45,11 @@ protected StoreFieldTypeFlow(StoreFieldTypeFlow original, MethodFlowsGraph metho super(original, methodFlows); } + /** + * Filters the incoming type state using the declared type. + */ @Override - public TypeState filter(PointsToAnalysis bb, TypeState newState) { + protected TypeState processInputState(PointsToAnalysis bb, TypeState newState) { /* * If the type flow constraints are relaxed filter the stored value using the field's * declared type. diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/TypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/TypeFlow.java index 8e92a1ecdd1b..cfed3c511eea 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/TypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/TypeFlow.java @@ -526,11 +526,9 @@ public boolean addState(PointsToAnalysis bb, TypeState add, boolean postFlow) { PointsToStats.registerTypeFlowUpdate(bb, this, add); TypeState before; TypeState after; - TypeState filteredAdd; do { before = state; - filteredAdd = filter(bb, add); - after = TypeState.forUnion(bb, before, filteredAdd); + after = newState(bb, add, before); if (after.equals(before)) { return false; } @@ -548,6 +546,14 @@ public boolean addState(PointsToAnalysis bb, TypeState add, boolean postFlow) { return true; } + /** + * Side-effect-free method for retrieving the next type state of this flow. + */ + private TypeState newState(PointsToAnalysis bb, TypeState add, TypeState before) { + TypeState filteredAdd = processInputState(bb, add); + return TypeState.forUnion(bb, before, filteredAdd); + } + protected void propagateState(PointsToAnalysis bb, boolean postFlow, TypeState newState) { assert isFlowEnabled() : "A flow cannot propagate state before it is enabled: " + this; assert newState.isNotEmpty() : "Empty state should not trigger propagation: " + this; @@ -787,7 +793,13 @@ public void clearInputs() { ConcurrentLightHashSet.clear(this, INPUTS_UPDATER); } - public TypeState filter(@SuppressWarnings("unused") PointsToAnalysis bb, TypeState newState) { + /** + * Hook for subclasses to transform the incoming type state based on the semantics of the given + * flow. Most flows perform only filtering, but in some cases, e.g. + * {@link BooleanInstanceOfCheckTypeFlow}, an incoming object type state is transformed into a + * primitive state. + */ + protected TypeState processInputState(@SuppressWarnings("unused") PointsToAnalysis bb, TypeState newState) { return newState; } @@ -952,6 +964,9 @@ protected void swapOut(PointsToAnalysis bb, TypeFlow newFlow) { for (TypeFlow predicatedFlow : getPredicatedFlows()) { swapAtPredicated(bb, newFlow, predicatedFlow); } + if (isSaturated()) { + AtomicUtils.atomicMark(this, PREDICATE_TRIGGERED_UPDATER); + } } protected void swapAtUse(PointsToAnalysis bb, TypeFlow newFlow, TypeFlow use) { @@ -1066,6 +1081,53 @@ void updateSource(T newSource) { validateSource(); } + /** + * @see PointsToAnalysis#validateFixedPointState + */ + public boolean validateFixedPointState(BigBang bb) { + if (!isFlowEnabled()) { + assert !isSaturated() : "Flows cannot be saturated before they are enabled " + this; + assert !predicateAlreadyTriggered() : "This flow is disabled, predicate edge should not have been triggered " + this; + return true; + } + if (!isSaturated() && state.isEmpty()) { + assert !predicateAlreadyTriggered() : "Predicate edge should only be triggered after the state becomes non-empty or the flow saturates " + this; + return true; + } + /* This flow is either saturated or has non-empty state */ + if (this instanceof InvokeTypeFlow) { + assert getPredicatedFlows().isEmpty() : "Invoke flows should have no predicated flows " + this; + assert !predicateAlreadyTriggered() : "Invoke flows should not use their predicate edge at all" + this; + } else { + assert predicateAlreadyTriggered() : "This flow is either saturated or has non-empty state, therefore the predicate edge should have been already triggered " + this; + for (TypeFlow predicated : getPredicatedFlows()) { + assert predicated.isFlowEnabled() : "Predicate edge was triggered, therefore " + predicated + " should have been enabled from " + this; + } + } + PointsToAnalysis pta = (PointsToAnalysis) bb; + if (isSaturated()) { + assert getUses().isEmpty() : "Uses of saturated flows should have been removed by now " + this + " " + getUses(); + assert getObservers().isEmpty() : "Observers of saturated flows should have been removed by now " + this + " " + getObservers(); + } else { + for (TypeFlow use : getUses()) { + /* + * The type state of saturated flows is not updated anymore. FormalReceiverTypeFlow + * has a special update method. + */ + if (use.isSaturated() || use instanceof FormalReceiverTypeFlow) { + continue; + } + /* + * Ensure that the type state was propagated correctly along this use edge. + */ + var nextState = use.newState(pta, state, use.state); + assert nextState.equals(use.state) || (!use.isFlowEnabled() && INPUT_SATURATED_UPDATER.get(use) == SIGNAL_RECEIVED) : "State from " + this + " to " + use + + " was not propagated correctly: " + use.state + " " + state + " => " + nextState; + } + } + return true; + } + public String formatSource() { if (source instanceof BytecodePosition) { BytecodePosition position = (BytecodePosition) source; diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java index 00ace3d66c06..61f4567aebe6 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java @@ -50,6 +50,7 @@ import org.graalvm.nativeimage.hosted.Feature.DuringAnalysisAccess; import com.oracle.graal.pointsto.BigBang; +import com.oracle.graal.pointsto.PointsToAnalysis; import com.oracle.graal.pointsto.api.ImageLayerLoader; import com.oracle.graal.pointsto.api.ImageLayerWriter; import com.oracle.graal.pointsto.api.PointstoOptions; @@ -407,6 +408,13 @@ public AnalysisMethod getIndirectCallTarget() { return setIndirectCallTarget(this, false); } + /** + * @see PointsToAnalysis#validateFixedPointState + */ + public boolean validateFixedPointState(@SuppressWarnings("unused") BigBang bb) { + return true; + } + public void cleanupAfterAnalysis() { GraphCacheEntry graphCacheEntry = parsedGraphCacheState.get(); if (graphCacheEntry != GraphCacheEntry.CLEARED) { diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/PointsToAnalysisMethod.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/PointsToAnalysisMethod.java index c6bd793ac246..360ac40b3e83 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/PointsToAnalysisMethod.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/PointsToAnalysisMethod.java @@ -30,11 +30,13 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; +import com.oracle.graal.pointsto.BigBang; import com.oracle.graal.pointsto.PointsToAnalysis; import com.oracle.graal.pointsto.flow.AbstractVirtualInvokeTypeFlow; import com.oracle.graal.pointsto.flow.ActualParameterTypeFlow; import com.oracle.graal.pointsto.flow.ActualReturnTypeFlow; import com.oracle.graal.pointsto.flow.InvokeTypeFlow; +import com.oracle.graal.pointsto.flow.MethodFlowsGraph; import com.oracle.graal.pointsto.flow.MethodTypeFlow; import com.oracle.graal.pointsto.flow.TypeFlow; import com.oracle.graal.pointsto.util.AnalysisError; @@ -237,6 +239,18 @@ public InvokeTypeFlow getContextInsensitiveVirtualInvoke(MultiMethodKey callerMu return invoke; } + @Override + public boolean validateFixedPointState(BigBang bb) { + if (typeFlow != null) { + for (MethodFlowsGraph flowsGraph : typeFlow.getFlows()) { + for (TypeFlow flow : flowsGraph.flows()) { + assert flow.validateFixedPointState(bb); + } + } + } + return true; + } + @Override public void cleanupAfterAnalysis() { super.cleanupAfterAnalysis(); diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/results/StrengthenGraphs.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/results/StrengthenGraphs.java index c1deccc191b5..7c7fecf0df04 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/results/StrengthenGraphs.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/results/StrengthenGraphs.java @@ -606,7 +606,8 @@ private void handleInvoke(Invoke invoke, SimplifierTool tool) { * trigger. But when only running the reachability analysis, there is no detailed * list of callees. */ - unreachableInvoke(invoke, tool); + unreachableInvoke(invoke, tool, () -> "method " + getQualifiedName(graph) + ", node " + invoke + + ": target method is not marked as simply implementation invoked"); /* Invoke is unreachable, there is no point in improving any types further. */ return; } @@ -616,12 +617,19 @@ private void handleInvoke(Invoke invoke, SimplifierTool tool) { /* No points-to analysis results. */ return; } + if (!invokeFlow.isFlowEnabled()) { + unreachableInvoke(invoke, tool, () -> "method " + getQualifiedName(graph) + ", node " + invoke + + ": flow is not enabled by its predicate " + invokeFlow.getPredicate()); + /* Invoke is unreachable, there is no point in improving any types further. */ + return; + } Collection callees = invokeFlow.getOriginalCallees(); if (callees.isEmpty()) { if (isClosedTypeWorld) { /* Invoke is unreachable, there is no point in improving any types further. */ - unreachableInvoke(invoke, tool); + unreachableInvoke(invoke, tool, () -> "method " + getQualifiedName(graph) + ", node " + invoke + + ": empty list of callees for call to " + ((AnalysisMethod) invoke.callTarget().targetMethod()).getQualifiedName()); } /* In open world we cannot make any assumptions about an invoke with 0 callees. */ return; @@ -833,7 +841,7 @@ protected void invokeWithNullReceiver(Invoke invoke) { /** * The invoke has no callee, i.e., it is unreachable. */ - private void unreachableInvoke(Invoke invoke, SimplifierTool tool) { + private void unreachableInvoke(Invoke invoke, SimplifierTool tool, Supplier messageSupplier) { if (invoke.getInvokeKind() != CallTargetNode.InvokeKind.Static) { /* * Ensure that a null check for the receiver remains in the graph. There should be @@ -842,8 +850,7 @@ private void unreachableInvoke(Invoke invoke, SimplifierTool tool) { InliningUtil.nonNullReceiver(invoke); } - makeUnreachable(invoke.asFixedNode(), tool, () -> "method " + getQualifiedName(graph) + ", node " + invoke + - ": empty list of callees for call to " + ((AnalysisMethod) invoke.callTarget().targetMethod()).getQualifiedName()); + makeUnreachable(invoke.asFixedNode(), tool, messageSupplier); } /** @@ -962,7 +969,13 @@ private Object strengthenStampFromTypeFlow(ValueNode node, TypeFlow nodeFlow, */ boolean hasUsages = node.usages().filter(n -> !(n instanceof FrameState)).isNotEmpty(); - TypeState nodeTypeState = nodeFlow.isFlowEnabled() ? methodFlow.foldTypeFlow((PointsToAnalysis) bb, nodeFlow) : TypeState.forEmpty(); + if (!nodeFlow.isFlowEnabled()) { + makeUnreachable(anchorPoint.next(), tool, + () -> "method " + getQualifiedName(graph) + ", node " + node + ": flow is not enabled by its predicate " + nodeFlow.getPredicate()); + unreachableValues.add(node); + return null; + } + TypeState nodeTypeState = methodFlow.foldTypeFlow((PointsToAnalysis) bb, nodeFlow); if (hasUsages && allowConstantFolding && !nodeTypeState.canBeNull()) { JavaConstant constantValue = nodeTypeState.asConstant(); diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestate/DefaultVirtualInvokeTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestate/DefaultVirtualInvokeTypeFlow.java index d3de0688bacd..6c49025f7304 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestate/DefaultVirtualInvokeTypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/typestate/DefaultVirtualInvokeTypeFlow.java @@ -139,6 +139,7 @@ public void onObservedUpdate(PointsToAnalysis bb) { @Override public void onObservedSaturated(PointsToAnalysis bb, TypeFlow observed) { + assert isFlowEnabled() : "Should only be executed if this flow is enabled " + this; if (!setSaturated()) { return; }