Skip to content

Commit 6ff3cc3

Browse files
committed
[GR-62081] Fix TypeFlowGraph linearization
PullRequest: graal/20022
2 parents ba4f045 + 1a3da8f commit 6ff3cc3

File tree

7 files changed

+42
-19
lines changed

7 files changed

+42
-19
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
* A sink type flow for the context insensitive invoke used to link in parameters in each caller
3333
* context.
3434
*/
35-
public class ActualParameterTypeFlow extends TypeFlow<ValueNode> {
35+
public class ActualParameterTypeFlow extends TypeFlow<ValueNode> implements GlobalFlow {
3636
public ActualParameterTypeFlow(AnalysisType declaredType) {
3737
super(null, filterUncheckedInterface(declaredType));
3838
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
/**
2828
* Keeps track of all synchronized types.
2929
*/
30-
public class AllSynchronizedTypeFlow extends TypeFlow<Object> {
30+
public class AllSynchronizedTypeFlow extends TypeFlow<Object> implements GlobalFlow {
3131

3232
@Override
3333
public String toString() {

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,21 +27,23 @@
2727
import com.oracle.graal.pointsto.PointsToAnalysis;
2828
import com.oracle.graal.pointsto.meta.AnalysisType;
2929

30+
import jdk.vm.ci.code.BytecodePosition;
31+
3032
/**
3133
* A local use of AllInstantiatedFlow that can have a predicate.
3234
*/
33-
public class LocalAllInstantiatedFlow extends TypeFlow<AnalysisType> {
35+
public class LocalAllInstantiatedFlow extends TypeFlow<BytecodePosition> {
3436

35-
public LocalAllInstantiatedFlow(AnalysisType declaredType) {
36-
super(declaredType, declaredType);
37+
public LocalAllInstantiatedFlow(BytecodePosition position, AnalysisType declaredType) {
38+
super(position, declaredType);
3739
}
3840

3941
private LocalAllInstantiatedFlow(MethodFlowsGraph methodFlows, LocalAllInstantiatedFlow original) {
4042
super(original, methodFlows);
4143
}
4244

4345
@Override
44-
public TypeFlow<AnalysisType> copy(PointsToAnalysis bb, MethodFlowsGraph methodFlows) {
46+
public TypeFlow<BytecodePosition> copy(PointsToAnalysis bb, MethodFlowsGraph methodFlows) {
4547
return new LocalAllInstantiatedFlow(methodFlows, this);
4648
}
4749
}

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343

4444
import jdk.graal.compiler.graph.Node;
4545
import jdk.graal.compiler.nodes.EncodedGraph.EncodedNodeReference;
46+
import jdk.vm.ci.code.BytecodePosition;
4647

4748
public class MethodFlowsGraph implements MethodFlowsGraphInfo {
4849
/**
@@ -124,7 +125,7 @@ public static boolean nonMethodFlow(TypeFlow<?> flow) {
124125
*
125126
* AnyPrimitiveFlow can be either global (source == null) or local (source != null)
126127
*/
127-
return flow instanceof AllInstantiatedTypeFlow || flow instanceof AllSynchronizedTypeFlow || (flow instanceof AnyPrimitiveSourceTypeFlow && flow.getSource() == null);
128+
return flow instanceof GlobalFlow || flow.isContextInsensitive() || (flow instanceof AnyPrimitiveSourceTypeFlow && flow.getSource() == null);
128129
}
129130

130131
/**
@@ -224,10 +225,27 @@ public boolean hasNext() {
224225
@Override
225226
public TypeFlow<?> next() {
226227
TypeFlow<?> current = next;
228+
assert withinMethod(current) : "Flow " + current + " has source " + current.source + " that is not a part of " + method;
227229
next = findNext();
228230
return current;
229231
}
230232

233+
/**
234+
* Check that the flow is local to the typeflow graph of this method. The iterator
235+
* should not leave the scope of this method, but it may include flows originated from
236+
* inlined methods.
237+
*/
238+
private boolean withinMethod(TypeFlow<?> flow) {
239+
var source = flow.getSource();
240+
while (source instanceof BytecodePosition position) {
241+
if (method.equals(position.getMethod())) {
242+
return true;
243+
}
244+
source = position.getCaller();
245+
}
246+
return false;
247+
}
248+
231249
/** Get the next flow and expand the work list. */
232250
private TypeFlow<?> findNext() {
233251
/* pollFirst returns null if the deque is empty. */

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

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -750,9 +750,9 @@ protected void apply(boolean forceReparse, Object reason) {
750750
* It only makes sense to create a local version of all instantiated if it will be guarded by a
751751
* predicate more precise than alwaysEnabled.
752752
*/
753-
protected TypeFlow<?> maybePatchAllInstantiated(TypeFlow<?> flow, AnalysisType declaredType, Object predicate) {
753+
protected TypeFlow<?> maybePatchAllInstantiated(TypeFlow<?> flow, BytecodePosition position, AnalysisType declaredType, Object predicate) {
754754
if (bb.usePredicates() && flow instanceof AllInstantiatedTypeFlow && predicate != alwaysEnabled) {
755-
var localFlow = new LocalAllInstantiatedFlow(declaredType);
755+
var localFlow = new LocalAllInstantiatedFlow(position, declaredType);
756756
flowsGraph.addMiscEntryFlow(localFlow);
757757
flow.addUse(bb, localFlow);
758758
return localFlow;
@@ -818,13 +818,14 @@ protected TypeFlowBuilder<?> handleObjectStamp(ObjectStamp stamp, ValueNode node
818818
throw AnalysisError.shouldNotReachHere("Stamp for node " + node + " is empty.");
819819
}
820820
AnalysisType stampType = (AnalysisType) StampTool.typeOrNull(stamp, bb.getMetaAccess());
821+
BytecodePosition position = AbstractAnalysisEngine.sourcePosition(node);
821822
if (stamp.isExactType()) {
822823
/*
823824
* We are lucky: the stamp tells us which type the node has. Happens e.g. for a
824825
* predicated boxed node.
825826
*/
826827
return TypeFlowBuilder.create(bb, method, getPredicate(), node, SourceTypeFlow.class, () -> {
827-
SourceTypeFlow src = new SourceTypeFlow(AbstractAnalysisEngine.sourcePosition(node), stampType, !stamp.nonNull());
828+
SourceTypeFlow src = new SourceTypeFlow(position, stampType, !stamp.nonNull());
828829
flowsGraph.addMiscEntryFlow(src);
829830
return src;
830831
});
@@ -835,9 +836,9 @@ protected TypeFlowBuilder<?> handleObjectStamp(ObjectStamp stamp, ValueNode node
835836
*/
836837
TypeFlowBuilder<?> predicate = getPredicate();
837838
return TypeFlowBuilder.create(bb, method, predicate, node, TypeFlow.class, () -> {
838-
TypeFlow<?> proxy = bb.analysisPolicy().proxy(AbstractAnalysisEngine.sourcePosition(node), stampType.getTypeFlow(bb, true));
839+
TypeFlow<?> proxy = bb.analysisPolicy().proxy(position, stampType.getTypeFlow(bb, true));
839840
flowsGraph.addMiscEntryFlow(proxy);
840-
return maybePatchAllInstantiated(proxy, stampType, predicate);
841+
return maybePatchAllInstantiated(proxy, position, stampType, predicate);
841842
});
842843
}
843844
}
@@ -1470,9 +1471,10 @@ protected void node(FixedNode n) {
14701471
TypeFlowBuilder<?> exceptionObjectBuilder = TypeFlowBuilder.create(bb, method, predicate, node, TypeFlow.class, () -> {
14711472
AnalysisType analysisType = (AnalysisType) StampTool.typeOrNull(node, bb.getMetaAccess());
14721473
TypeFlow<?> input = analysisType.getTypeFlow(bb, false);
1473-
TypeFlow<?> exceptionObjectFlow = bb.analysisPolicy().proxy(AbstractAnalysisEngine.sourcePosition(node), input);
1474+
BytecodePosition position = AbstractAnalysisEngine.sourcePosition(node);
1475+
TypeFlow<?> exceptionObjectFlow = bb.analysisPolicy().proxy(position, input);
14741476
flowsGraph.addMiscEntryFlow(exceptionObjectFlow);
1475-
return maybePatchAllInstantiated(exceptionObjectFlow, analysisType, predicate);
1477+
return maybePatchAllInstantiated(exceptionObjectFlow, position, analysisType, predicate);
14761478
});
14771479
state.add(node, exceptionObjectBuilder);
14781480

@@ -1533,7 +1535,7 @@ protected void node(FixedNode n) {
15331535
instanceType = bb.getObjectType();
15341536
TypeFlowBuilder<?> predicate = state.getPredicate();
15351537
instanceTypeBuilder = TypeFlowBuilder.create(bb, method, predicate, instanceType, TypeFlow.class,
1536-
() -> maybePatchAllInstantiated(instanceType.getTypeFlow(bb, false), instanceType, predicate));
1538+
() -> maybePatchAllInstantiated(instanceType.getTypeFlow(bb, false), AbstractAnalysisEngine.sourcePosition(node), instanceType, predicate));
15371539
}
15381540
TypeFlowBuilder<DynamicNewInstanceTypeFlow> dynamicNewInstanceBuilder = TypeFlowBuilder.create(bb, method, state.getPredicate(), node, DynamicNewInstanceTypeFlow.class, () -> {
15391541
DynamicNewInstanceTypeFlow newInstanceTypeFlow = new DynamicNewInstanceTypeFlow(AbstractAnalysisEngine.sourcePosition(node), instanceTypeBuilder.get(), instanceType);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,10 +103,10 @@ public String toString() {
103103
/**
104104
* The state of the StoreFieldTypeFlow reflects the state of the stored value. The
105105
* StoreFieldTypeFlow is an observer of the receiver flow, i.e. flow modeling the receiver
106-
* object of the store operation..
106+
* object of the store operation.
107107
*
108108
* Every time the state of the receiver flow changes the corresponding field flows are added as
109-
* uses to the store field flow. Thus the stored value is propagated to the store field flow
109+
* uses to the store field flow. Thus, the stored value is propagated to the store field flow
110110
* into the field flows.
111111
*/
112112
public static class StoreInstanceFieldTypeFlow extends StoreFieldTypeFlow {

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/analysis/flow/SVMMethodTypeFlowBuilder.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,9 +170,10 @@ private void storeVMThreadLocal(TypeFlowsOfNodes state, ValueNode storeNode, Val
170170

171171
TypeFlowBuilder<?> predicate = state.getPredicate();
172172
TypeFlowBuilder<?> storeBuilder = TypeFlowBuilder.create(bb, method, predicate, storeNode, TypeFlow.class, () -> {
173-
TypeFlow<?> proxy = bb.analysisPolicy().proxy(AbstractAnalysisEngine.sourcePosition(storeNode), valueType.getTypeFlow(bb, false));
173+
BytecodePosition position = AbstractAnalysisEngine.sourcePosition(storeNode);
174+
TypeFlow<?> proxy = bb.analysisPolicy().proxy(position, valueType.getTypeFlow(bb, false));
174175
flowsGraph.addMiscEntryFlow(proxy);
175-
return maybePatchAllInstantiated(proxy, valueType, predicate);
176+
return maybePatchAllInstantiated(proxy, position, valueType, predicate);
176177
});
177178
storeBuilder.addUseDependency(valueBuilder);
178179
typeFlowGraphBuilder.registerSinkBuilder(storeBuilder);

0 commit comments

Comments
 (0)