Skip to content

Commit 6a550f9

Browse files
cstancucstancu
committed
[GR-62828] Fix the points-to analysis model for reflective/unsafe reads/writes in layered builds.
PullRequest: graal/20248
2 parents 330aed9 + 6a4b206 commit 6a550f9

File tree

13 files changed

+167
-116
lines changed

13 files changed

+167
-116
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ public abstract class AnalysisPolicy {
6969
protected final boolean limitObjectArrayLength;
7070
protected final int maxObjectSetSize;
7171
protected final boolean hybridStaticContext;
72+
protected final boolean useConservativeUnsafeAccess;
7273

7374
public AnalysisPolicy(OptionValues options) {
7475
this.options = options;
@@ -82,6 +83,7 @@ public AnalysisPolicy(OptionValues options) {
8283
limitObjectArrayLength = PointstoOptions.LimitObjectArrayLength.getValue(options);
8384
maxObjectSetSize = PointstoOptions.MaxObjectSetSize.getValue(options);
8485
hybridStaticContext = PointstoOptions.HybridStaticContext.getValue(options);
86+
useConservativeUnsafeAccess = PointstoOptions.UseConservativeUnsafeAccess.getValue(options);
8587
}
8688

8789
public abstract boolean isContextSensitiveAnalysis();
@@ -118,6 +120,10 @@ public boolean useHybridStaticContext() {
118120
return hybridStaticContext;
119121
}
120122

123+
public boolean useConservativeUnsafeAccess() {
124+
return useConservativeUnsafeAccess;
125+
}
126+
121127
public abstract MethodTypeFlow createMethodTypeFlow(PointsToAnalysisMethod method);
122128

123129
/**

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/PointsToAnalysis.java

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@
5353
import com.oracle.graal.pointsto.flow.MethodFlowsGraphInfo;
5454
import com.oracle.graal.pointsto.flow.MethodTypeFlow;
5555
import com.oracle.graal.pointsto.flow.MethodTypeFlowBuilder;
56-
import com.oracle.graal.pointsto.flow.OffsetLoadTypeFlow.AbstractUnsafeLoadTypeFlow;
57-
import com.oracle.graal.pointsto.flow.OffsetStoreTypeFlow.AbstractUnsafeStoreTypeFlow;
56+
import com.oracle.graal.pointsto.flow.OffsetLoadTypeFlow.UnsafeLoadTypeFlow;
57+
import com.oracle.graal.pointsto.flow.OffsetStoreTypeFlow.UnsafeStoreTypeFlow;
5858
import com.oracle.graal.pointsto.flow.TypeFlow;
5959
import com.oracle.graal.pointsto.meta.AnalysisField;
6060
import com.oracle.graal.pointsto.meta.AnalysisMetaAccess;
@@ -110,8 +110,8 @@ public abstract class PointsToAnalysis extends AbstractAnalysisEngine {
110110
protected final boolean trackTypeFlowInputs;
111111
protected final boolean reportAnalysisStatistics;
112112

113-
private ConcurrentMap<AbstractUnsafeLoadTypeFlow, Boolean> unsafeLoads;
114-
private ConcurrentMap<AbstractUnsafeStoreTypeFlow, Boolean> unsafeStores;
113+
private ConcurrentMap<UnsafeLoadTypeFlow, Boolean> unsafeLoads;
114+
private ConcurrentMap<UnsafeStoreTypeFlow, Boolean> unsafeStores;
115115

116116
public final AtomicLong numParsedGraphs = new AtomicLong();
117117
private final CompletionExecutor.Timing timing;
@@ -145,8 +145,8 @@ public PointsToAnalysis(OptionValues options, AnalysisUniverse universe, HostVM
145145
PointsToStats.init(this);
146146
}
147147

148-
unsafeLoads = new ConcurrentHashMap<>();
149-
unsafeStores = new ConcurrentHashMap<>();
148+
unsafeLoads = analysisPolicy.useConservativeUnsafeAccess() ? null : new ConcurrentHashMap<>();
149+
unsafeStores = analysisPolicy.useConservativeUnsafeAccess() ? null : new ConcurrentHashMap<>();
150150

151151
timing = PointstoOptions.ProfileAnalysisOperations.getValue(options) ? new AnalysisTiming() : null;
152152
executor.init(timing);
@@ -200,11 +200,11 @@ public MethodTypeFlowBuilder createMethodTypeFlowBuilder(PointsToAnalysis bb, Po
200200
return new MethodTypeFlowBuilder(bb, method, flowsGraph, graphKind);
201201
}
202202

203-
public void registerUnsafeLoad(AbstractUnsafeLoadTypeFlow unsafeLoad) {
203+
public void registerUnsafeLoad(UnsafeLoadTypeFlow unsafeLoad) {
204204
unsafeLoads.putIfAbsent(unsafeLoad, true);
205205
}
206206

207-
public void registerUnsafeStore(AbstractUnsafeStoreTypeFlow unsafeStore) {
207+
public void registerUnsafeStore(UnsafeStoreTypeFlow unsafeStore) {
208208
unsafeStores.putIfAbsent(unsafeStore, true);
209209
}
210210

@@ -216,13 +216,16 @@ public void registerUnsafeStore(AbstractUnsafeStoreTypeFlow unsafeStore) {
216216
* unsafe access flows that need to be updated.
217217
*/
218218
public void forceUnsafeUpdate(AnalysisField field) {
219+
if (analysisPolicy.useConservativeUnsafeAccess()) {
220+
return;
221+
}
219222
/*
220223
* It is cheaper to post the flows of all loads and stores even if they are not related to
221224
* the provided field.
222225
*/
223226

224227
// force update of the unsafe loads
225-
for (AbstractUnsafeLoadTypeFlow unsafeLoad : unsafeLoads.keySet()) {
228+
for (UnsafeLoadTypeFlow unsafeLoad : unsafeLoads.keySet()) {
226229
/* Force update for unsafe accessed static fields. */
227230
unsafeLoad.forceUpdate(this);
228231

@@ -237,7 +240,7 @@ public void forceUnsafeUpdate(AnalysisField field) {
237240
}
238241

239242
// force update of the unsafe stores
240-
for (AbstractUnsafeStoreTypeFlow unsafeStore : unsafeStores.keySet()) {
243+
for (UnsafeStoreTypeFlow unsafeStore : unsafeStores.keySet()) {
241244
/* Force update for unsafe accessed static fields. */
242245
unsafeStore.forceUpdate(this);
243246

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,15 @@ protected void onValueUpdate(EconomicMap<OptionKey<?>, Object> values, Boolean o
134134
@Option(help = "Allow a type flow state to contain types not compatible with its declared type.")//
135135
public static final OptionKey<Boolean> RelaxTypeFlowStateConstraints = new OptionKey<>(true);
136136

137+
/**
138+
* This flag enables conservative modeling of unsafe access during the analysis. First, the type
139+
* state of unsafe accessed fields now contains all instantiated subtypes of their declared
140+
* type. Second, all unsafe loads are saturated by default; we do this because we assume we
141+
* cannot accurately track which fields can be unsafe accessed.
142+
*/
143+
@Option(help = "Conservative unsafe access injects all unsafe accessed fields with the instantiated subtypes of their declared type and saturates all unsafe loads.")//
144+
public static final OptionKey<Boolean> UseConservativeUnsafeAccess = new OptionKey<>(false);
145+
137146
@Option(help = "Deprecated, option no longer has any effect.", deprecated = true)//
138147
static final OptionKey<Boolean> UnresolvedIsError = new OptionKey<>(true);
139148

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

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2111,22 +2111,44 @@ protected void processUnsafeLoad(ValueNode node, ValueNode object, TypeFlowsOfNo
21112111
if (node.getStackKind() == JavaKind.Object) {
21122112
TypeFlowBuilder<?> objectBuilder = state.lookup(object);
21132113

2114-
/*
2115-
* Use the Object type as a conservative approximation for both the receiver object type
2116-
* and the loaded values type.
2117-
*/
2118-
var loadBuilder = TypeFlowBuilder.create(bb, method, state.getPredicate(), node, UnsafeLoadTypeFlow.class, () -> {
2119-
UnsafeLoadTypeFlow loadTypeFlow = new UnsafeLoadTypeFlow(AbstractAnalysisEngine.sourcePosition(node), bb.getObjectType(), bb.getObjectType(), objectBuilder.get());
2120-
flowsGraph.addMiscEntryFlow(loadTypeFlow);
2121-
return loadTypeFlow;
2122-
});
2114+
TypeFlowBuilder<?> loadBuilder;
2115+
if (bb.analysisPolicy().useConservativeUnsafeAccess()) {
2116+
/*
2117+
* When unsafe loads are modeled conservatively they start as saturated since the
2118+
* exact fields that are marked as unsafe accessed are not tracked and cannot be
2119+
* used as an input to the UnsafeLoadTypeFlow. Using a pre-saturated flow will
2120+
* signal the saturation to any future uses.
2121+
*/
2122+
loadBuilder = TypeFlowBuilder.create(bb, method, state.getPredicate(), node, PreSaturatedTypeFlow.class, () -> {
2123+
PreSaturatedTypeFlow preSaturated = new PreSaturatedTypeFlow(AbstractAnalysisEngine.sourcePosition(node));
2124+
flowsGraph.addMiscEntryFlow(preSaturated);
2125+
return preSaturated;
2126+
});
2127+
} else {
2128+
/*
2129+
* Use the Object type as a conservative approximation for both the receiver object
2130+
* type and the loaded values type.
2131+
*/
2132+
loadBuilder = TypeFlowBuilder.create(bb, method, state.getPredicate(), node, UnsafeLoadTypeFlow.class, () -> {
2133+
UnsafeLoadTypeFlow loadTypeFlow = new UnsafeLoadTypeFlow(AbstractAnalysisEngine.sourcePosition(node), bb.getObjectType(), bb.getObjectType(), objectBuilder.get());
2134+
flowsGraph.addMiscEntryFlow(loadTypeFlow);
2135+
return loadTypeFlow;
2136+
});
2137+
}
21232138

21242139
loadBuilder.addObserverDependency(objectBuilder);
21252140
state.add(node, loadBuilder);
21262141
}
21272142
}
21282143

21292144
protected void processUnsafeStore(ValueNode node, ValueNode object, ValueNode newValue, JavaKind newValueKind, TypeFlowsOfNodes state) {
2145+
if (bb.analysisPolicy().useConservativeUnsafeAccess()) {
2146+
/*
2147+
* When unsafe writes are modeled conservatively all unsafe accessed fields contain all
2148+
* instantiated subtypes of their declared type, so no need to model the unsafe store.
2149+
*/
2150+
return;
2151+
}
21302152
/* All unsafe accessed primitive fields are always saturated. */
21312153
if (newValueKind == JavaKind.Object) {
21322154
TypeFlowBuilder<?> objectBuilder = state.lookup(object);

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/OffsetLoadTypeFlow.java

Lines changed: 18 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -156,16 +156,26 @@ public String toString() {
156156

157157
}
158158

159-
public abstract static class AbstractUnsafeLoadTypeFlow extends OffsetLoadTypeFlow {
159+
public static class UnsafeLoadTypeFlow extends OffsetLoadTypeFlow {
160160

161-
AbstractUnsafeLoadTypeFlow(BytecodePosition loadLocation, AnalysisType objectType, AnalysisType componentType, TypeFlow<?> objectFlow) {
161+
UnsafeLoadTypeFlow(BytecodePosition loadLocation, AnalysisType objectType, AnalysisType componentType, TypeFlow<?> objectFlow) {
162162
super(loadLocation, objectType, filterUncheckedInterface(componentType), objectFlow);
163163
}
164164

165-
AbstractUnsafeLoadTypeFlow(PointsToAnalysis bb, MethodFlowsGraph methodFlows, AbstractUnsafeLoadTypeFlow original) {
165+
UnsafeLoadTypeFlow(PointsToAnalysis bb, MethodFlowsGraph methodFlows, UnsafeLoadTypeFlow original) {
166166
super(bb, methodFlows, original);
167167
}
168168

169+
@Override
170+
public UnsafeLoadTypeFlow copy(PointsToAnalysis bb, MethodFlowsGraph methodFlows) {
171+
return new UnsafeLoadTypeFlow(bb, methodFlows, this);
172+
}
173+
174+
@Override
175+
public boolean needsInitialization() {
176+
return true;
177+
}
178+
169179
@Override
170180
public void initFlow(PointsToAnalysis bb) {
171181
assert !bb.analysisPolicy().isContextSensitiveAnalysis() || this.isClone() : this;
@@ -177,15 +187,10 @@ public void initFlow(PointsToAnalysis bb) {
177187
forceUpdate(bb);
178188
}
179189

180-
@Override
181-
public boolean needsInitialization() {
182-
return true;
183-
}
184-
185190
public void forceUpdate(PointsToAnalysis bb) {
186191
/*
187192
* Unsafe load type flow models unsafe reads from both instance and static fields. From
188-
* an analysis stand point for static fields the base doesn't matter. An unsafe load can
193+
* an analysis standpoint for static fields the base doesn't matter. An unsafe load can
189194
* read from any of the static fields marked for unsafe access.
190195
*/
191196
for (AnalysisField field : bb.getUniverse().getUnsafeAccessedStaticFields()) {
@@ -200,29 +205,6 @@ public void forceUpdate(PointsToAnalysis bb) {
200205
}
201206
}
202207

203-
protected void processField(PointsToAnalysis bb, AnalysisObject object, AnalysisField field) {
204-
if (field.getStorageKind().isObject()) {
205-
TypeFlow<?> fieldFlow = object.getInstanceFieldFlow(bb, objectFlow, source, field, false);
206-
fieldFlow.addUse(bb, this);
207-
}
208-
}
209-
}
210-
211-
public static class UnsafeLoadTypeFlow extends AbstractUnsafeLoadTypeFlow {
212-
213-
public UnsafeLoadTypeFlow(BytecodePosition loadLocation, AnalysisType objectType, AnalysisType componentType, TypeFlow<?> arrayFlow) {
214-
super(loadLocation, objectType, componentType, arrayFlow);
215-
}
216-
217-
private UnsafeLoadTypeFlow(PointsToAnalysis bb, MethodFlowsGraph methodFlows, UnsafeLoadTypeFlow original) {
218-
super(bb, methodFlows, original);
219-
}
220-
221-
@Override
222-
public AbstractUnsafeLoadTypeFlow copy(PointsToAnalysis bb, MethodFlowsGraph methodFlows) {
223-
return new UnsafeLoadTypeFlow(bb, methodFlows, this);
224-
}
225-
226208
@Override
227209
public void onObservedUpdate(PointsToAnalysis bb) {
228210
TypeState objectState = getObjectState();
@@ -244,8 +226,10 @@ public void onObservedUpdate(PointsToAnalysis bb) {
244226
elementsFlow.addUse(bb, this);
245227
} else {
246228
for (AnalysisField field : objectType.unsafeAccessedFields()) {
247-
assert field != null;
248-
processField(bb, object, field);
229+
if (field.getStorageKind().isObject()) {
230+
TypeFlow<?> fieldFlow = object.getInstanceFieldFlow(bb, objectFlow, source, field, false);
231+
fieldFlow.addUse(bb, this);
232+
}
249233
}
250234
}
251235
}

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/OffsetStoreTypeFlow.java

Lines changed: 15 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@
2424
*/
2525
package com.oracle.graal.pointsto.flow;
2626

27-
import java.util.Collection;
28-
2927
import com.oracle.graal.pointsto.PointsToAnalysis;
3028
import com.oracle.graal.pointsto.flow.context.object.AnalysisObject;
3129
import com.oracle.graal.pointsto.meta.AnalysisField;
@@ -49,7 +47,7 @@ public abstract class OffsetStoreTypeFlow extends TypeFlow<BytecodePosition> {
4947

5048
/*
5149
* The type of the receiver object of the offset store operation. Can be approximated by Object
52-
* or Object[] when it cannot be infered from stamps.
50+
* or Object[] when it cannot be inferred from stamps.
5351
*/
5452
protected final AnalysisType objectType;
5553

@@ -177,22 +175,25 @@ public String toString() {
177175
}
178176
}
179177

180-
public abstract static class AbstractUnsafeStoreTypeFlow extends OffsetStoreTypeFlow {
178+
public static class UnsafeStoreTypeFlow extends OffsetStoreTypeFlow {
181179

182-
AbstractUnsafeStoreTypeFlow(BytecodePosition storeLocation, AnalysisType objectType, AnalysisType componentType, TypeFlow<?> objectFlow, TypeFlow<?> valueFlow) {
180+
public UnsafeStoreTypeFlow(BytecodePosition storeLocation, AnalysisType objectType, AnalysisType componentType, TypeFlow<?> objectFlow, TypeFlow<?> valueFlow) {
183181
super(storeLocation, objectType, filterUncheckedInterface(componentType), objectFlow, valueFlow);
184182
}
185183

186-
AbstractUnsafeStoreTypeFlow(PointsToAnalysis bb, MethodFlowsGraph methodFlows, OffsetStoreTypeFlow original) {
184+
public UnsafeStoreTypeFlow(PointsToAnalysis bb, MethodFlowsGraph methodFlows, UnsafeStoreTypeFlow original) {
187185
super(bb, methodFlows, original);
188186
}
189187

190188
@Override
191-
public final AbstractUnsafeStoreTypeFlow copy(PointsToAnalysis bb, MethodFlowsGraph methodFlows) {
192-
return makeCopy(bb, methodFlows);
189+
public final UnsafeStoreTypeFlow copy(PointsToAnalysis bb, MethodFlowsGraph methodFlows) {
190+
return new UnsafeStoreTypeFlow(bb, methodFlows, this);
193191
}
194192

195-
protected abstract AbstractUnsafeStoreTypeFlow makeCopy(PointsToAnalysis bb, MethodFlowsGraph methodFlows);
193+
@Override
194+
public boolean needsInitialization() {
195+
return true;
196+
}
196197

197198
@Override
198199
public void initFlow(PointsToAnalysis bb) {
@@ -205,11 +206,6 @@ public void initFlow(PointsToAnalysis bb) {
205206
forceUpdate(bb);
206207
}
207208

208-
@Override
209-
public boolean needsInitialization() {
210-
return true;
211-
}
212-
213209
public void forceUpdate(PointsToAnalysis bb) {
214210
/*
215211
* Unsafe store type flow models unsafe writes to both instance and static fields. From
@@ -221,32 +217,6 @@ public void forceUpdate(PointsToAnalysis bb) {
221217
}
222218
}
223219

224-
void handleUnsafeAccessedFields(PointsToAnalysis bb, Collection<AnalysisField> unsafeAccessedFields, AnalysisObject object) {
225-
for (AnalysisField field : unsafeAccessedFields) {
226-
/* Write through the field filter flow. */
227-
addUse(bb, object.getInstanceFieldFilterFlow(bb, objectFlow, source, field));
228-
}
229-
}
230-
}
231-
232-
/**
233-
* Implements an unsafe store operation type flow.
234-
*/
235-
public static class UnsafeStoreTypeFlow extends AbstractUnsafeStoreTypeFlow {
236-
237-
public UnsafeStoreTypeFlow(BytecodePosition storeLocation, AnalysisType objectType, AnalysisType componentType, TypeFlow<?> objectFlow, TypeFlow<?> valueFlow) {
238-
super(storeLocation, objectType, componentType, objectFlow, valueFlow);
239-
}
240-
241-
public UnsafeStoreTypeFlow(PointsToAnalysis bb, MethodFlowsGraph methodFlows, UnsafeStoreTypeFlow original) {
242-
super(bb, methodFlows, original);
243-
}
244-
245-
@Override
246-
public UnsafeStoreTypeFlow makeCopy(PointsToAnalysis bb, MethodFlowsGraph methodFlows) {
247-
return new UnsafeStoreTypeFlow(bb, methodFlows, this);
248-
}
249-
250220
@Override
251221
public void onObservedUpdate(PointsToAnalysis bb) {
252222
TypeState objectState = objectFlow.getState();
@@ -267,7 +237,10 @@ public void onObservedUpdate(PointsToAnalysis bb) {
267237
TypeFlow<?> elementsFlow = object.getArrayElementsFlow(bb, true);
268238
this.addUse(bb, elementsFlow);
269239
} else {
270-
handleUnsafeAccessedFields(bb, type.unsafeAccessedFields(), object);
240+
for (AnalysisField field : type.unsafeAccessedFields()) {
241+
/* Write through the field filter flow. */
242+
this.addUse(bb, object.getInstanceFieldFilterFlow(bb, objectFlow, source, field));
243+
}
271244
}
272245
}
273246
}
@@ -292,7 +265,7 @@ public void onObservedSaturated(PointsToAnalysis bb, TypeFlow<?> observed) {
292265
valueFlow.removeUse(this);
293266

294267
/* Link the saturated store. */
295-
AbstractUnsafeStoreTypeFlow contextInsensitiveStore = ((PointsToAnalysisType) objectType).initAndGetContextInsensitiveUnsafeStore(bb, source);
268+
UnsafeStoreTypeFlow contextInsensitiveStore = ((PointsToAnalysisType) objectType).initAndGetContextInsensitiveUnsafeStore(bb, source);
296269
/*
297270
* Link the value flow to the saturated store. The receiver is already set in the
298271
* saturated store.

0 commit comments

Comments
 (0)