Skip to content

Commit

Permalink
[GR-51974] Track never-null instance fields in the static analysis.
Browse files Browse the repository at this point in the history
PullRequest: graal/16922
  • Loading branch information
christianwimmer committed Jun 15, 2024
2 parents c10d824 + e2e2569 commit 7a033b9
Show file tree
Hide file tree
Showing 28 changed files with 186 additions and 166 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ default void onFieldAccessed(AnalysisField field) {
}

@SuppressWarnings("unused")
default void injectFieldTypes(AnalysisField aField, AnalysisType... customTypes) {
default void injectFieldTypes(AnalysisField aField, List<AnalysisType> customTypes, boolean canBeNull) {
}

@SuppressWarnings("unused")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ protected void onValueUpdate(EconomicMap<OptionKey<?>, Object> values, Boolean o
@Option(help = "Run conditional elimination before static analysis.", type = Expert)//
public static final OptionKey<Boolean> ConditionalEliminationBeforeAnalysis = new OptionKey<>(true);

@Option(help = "Track in the static analysis whether an instance field is never null.")//
public static final OptionKey<Boolean> TrackNeverNullInstanceFields = new OptionKey<>(true);

/**
* Controls the static analysis context sensitivity. Available values:
* <p/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,6 @@ public class FieldTypeFlow extends TypeFlow<AnalysisField> {
private static final AtomicReferenceFieldUpdater<FieldTypeFlow, FieldFilterTypeFlow> FILTER_FLOW_UPDATER = AtomicReferenceFieldUpdater.newUpdater(FieldTypeFlow.class, FieldFilterTypeFlow.class,
"filterFlow");

private static TypeState initialFieldState(AnalysisField field) {
if (field.getStorageKind().isPrimitive()) {
return TypeState.forPrimitiveConstant(0);
} else if (field.canBeNull()) {
/*
* All object type instance fields of a new object can be null. Instance fields are null
* in the time between the new-instance and the first write to a field. This is even
* true for non-null final fields because even final fields are null until they are
* initialized in a constructor.
*/
return TypeState.forNull();
}
return TypeState.forEmpty();
}

/** The holder of the field flow (null for static fields). */
private final AnalysisObject object;

Expand All @@ -65,7 +50,7 @@ public FieldTypeFlow(AnalysisField field, AnalysisType type) {
}

public FieldTypeFlow(AnalysisField field, AnalysisType type, AnalysisObject object) {
super(field, filterUncheckedInterface(type), initialFieldState(field));
super(field, filterUncheckedInterface(type), TypeState.forEmpty());
this.object = object;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,10 @@ public static void registerUsedElements(PointsToAnalysis bb, StructuredGraph gra
NewInstanceNode node = (NewInstanceNode) n;
AnalysisType type = (AnalysisType) node.instanceClass();
type.registerAsInstantiated(AbstractAnalysisEngine.sourcePosition(node));
for (var f : type.getInstanceFields(true)) {
var field = (AnalysisField) f;
field.getInitialFlow().addState(bb, TypeState.defaultValueForKind(field.getStorageKind()));
}

} else if (n instanceof NewInstanceWithExceptionNode) {
NewInstanceWithExceptionNode node = (NewInstanceWithExceptionNode) n;
Expand Down Expand Up @@ -1441,6 +1445,11 @@ protected void processCommitAllocation(CommitAllocationNode commitAllocationNode
AnalysisField field = (AnalysisField) ((VirtualInstanceNode) virtualObject).field(i);
processStoreField(commitAllocationNode, field, object, value, value.getStackKind(), state);
}
} else {
if (!type.isArray()) {
AnalysisField field = (AnalysisField) ((VirtualInstanceNode) virtualObject).field(i);
field.getInitialFlow().addState(bb, TypeState.defaultValueForKind(field.getStorageKind()));
}
}
}
objectStartIndex += virtualObject.entryCount();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,9 @@ private void checkField(PointsToAnalysis bb, TypeFlow<?> objectFlow, BytecodePos
@SuppressWarnings("unused")
protected void linkFieldFlows(PointsToAnalysis bb, AnalysisField field, FieldTypeStore fieldStore) {
// link the initial instance field flow to the field write flow
field.getInitialInstanceFieldFlow().addUse(bb, fieldStore.writeFlow());
// link the field read flow to the context insensitive instance field flow
fieldStore.readFlow().addUse(bb, field.getInstanceFieldFlow());
field.getInitialFlow().addUse(bb, fieldStore.writeFlow());
// link the field read flow to the sink flow that accumulates all field types
fieldStore.readFlow().addUse(bb, field.getSinkFlow());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,9 @@ public FieldTypeFlow getInstanceFieldFlow(PointsToAnalysis bb, TypeFlow<?> objec
@Override
protected void linkFieldFlows(PointsToAnalysis bb, AnalysisField field, FieldTypeStore fieldStore) {
// link the initial instance field flow to the field write flow
field.getInitialInstanceFieldFlow().addUse(bb, fieldStore.writeFlow());
// link the field read flow to the instance field flow
fieldStore.readFlow().addUse(bb, field.getInstanceFieldFlow());
field.getInitialFlow().addUse(bb, fieldStore.writeFlow());
// link the field read flow to the sink flow that accumulates all field types
fieldStore.readFlow().addUse(bb, field.getSinkFlow());
// Also link the field read flow the field flow on the context insensitive object.
// This ensures that the all values flowing into a context-sensitive field flow
// are also visible from the context-insensitive field flow.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

import java.lang.reflect.Field;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
Expand Down Expand Up @@ -143,16 +144,14 @@ public void onFieldRead(AnalysisField field) {
if (fieldType.isArray() || (fieldType.isInstanceClass() && !fieldType.isAbstract())) {
fieldType.registerAsInstantiated(field);
}
bb.injectFieldTypes(field, fieldType);
bb.injectFieldTypes(field, List.of(fieldType), true);
}
return;
}
if (isValueAvailable(field)) {
JavaConstant fieldValue = readStaticFieldValue(field);
markReachable(fieldValue, reason);
notifyAnalysis(field, null, fieldValue, reason);
} else if (field.canBeNull()) {
notifyAnalysis(field, null, JavaConstant.NULL_POINTER, reason);
}
} else {
/* Trigger field scanning for the already processed objects. */
Expand Down Expand Up @@ -616,8 +615,6 @@ private void updateInstanceField(AnalysisField field, ImageHeapInstance imageHea
JavaConstant fieldValue = imageHeapInstance.readFieldValue(field);
markReachable(fieldValue, reason, onAnalysisModified);
notifyAnalysis(field, imageHeapInstance, fieldValue, reason, onAnalysisModified);
} else if (field.canBeNull()) {
notifyAnalysis(field, imageHeapInstance, JavaConstant.NULL_POINTER, reason, onAnalysisModified);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,12 @@
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;

import com.oracle.graal.pointsto.api.HostVM;
import com.oracle.graal.pointsto.api.PointstoOptions;
import com.oracle.graal.pointsto.flow.ContextInsensitiveFieldTypeFlow;
import com.oracle.graal.pointsto.flow.FieldTypeFlow;
import com.oracle.graal.pointsto.infrastructure.OriginalClassProvider;
import com.oracle.graal.pointsto.infrastructure.OriginalFieldProvider;
import com.oracle.graal.pointsto.infrastructure.WrappedJavaField;
import com.oracle.graal.pointsto.typestate.TypeState;
import com.oracle.graal.pointsto.util.AnalysisError;
import com.oracle.graal.pointsto.util.AnalysisFuture;
import com.oracle.graal.pointsto.util.AtomicUtils;
Expand Down Expand Up @@ -73,17 +71,17 @@ public abstract class AnalysisField extends AnalysisElement implements WrappedJa

public final ResolvedJavaField wrapped;

/** Field type flow for the static fields. */
protected FieldTypeFlow staticFieldFlow;

/** Initial field type flow, i.e., as specified by the analysis client. */
protected FieldTypeFlow initialInstanceFieldFlow;

/**
* Initial field type flow, i.e., as specified by the analysis client. It can be used to inject
* specific types into a field that the analysis would not see on its own, and to inject the
* null value into a field.
*/
protected FieldTypeFlow initialFlow;
/**
* Field type flow that reflects all the types flowing in this field on its declaring type and
* all the sub-types. It doesn't track any context-sensitive information.
* all the sub-types. It does not track any context-sensitive information.
*/
protected ContextInsensitiveFieldTypeFlow instanceFieldFlow;
protected FieldTypeFlow sinkFlow;

/** The reason flags contain a {@link BytecodePosition} or a reason object. */
@SuppressWarnings("unused") private volatile Object isRead;
Expand All @@ -92,12 +90,6 @@ public abstract class AnalysisField extends AnalysisElement implements WrappedJa
@SuppressWarnings("unused") private volatile Object isFolded;
@SuppressWarnings("unused") private volatile Object isUnsafeAccessed;

/**
* By default all instance fields are null before are initialized. It can be specified by
* {@link HostVM} that certain fields will not be null.
*/
private boolean canBeNull;

private ConcurrentMap<Object, Boolean> readBy;
private ConcurrentMap<Object, Boolean> writtenBy;

Expand Down Expand Up @@ -129,14 +121,16 @@ public AnalysisField(AnalysisUniverse universe, ResolvedJavaField wrappedField)
declaringClass = universe.lookup(wrappedField.getDeclaringClass());
fieldType = getDeclaredType(universe, wrappedField);

initialFlow = new FieldTypeFlow(this, getType());
if (this.isStatic()) {
this.canBeNull = false;
this.staticFieldFlow = new FieldTypeFlow(this, getType());
this.initialInstanceFieldFlow = null;
/* There is never any context-sensitivity for static fields. */
sinkFlow = initialFlow;
} else {
this.canBeNull = !getStorageKind().isPrimitive();
this.instanceFieldFlow = new ContextInsensitiveFieldTypeFlow(this, getType());
this.initialInstanceFieldFlow = new FieldTypeFlow(this, getType());
/*
* Regardless of the context-sensitivity policy, there is always this single type flow
* that accumulates all types.
*/
sinkFlow = new ContextInsensitiveFieldTypeFlow(this, getType());
}

if (universe.hostVM().useBaseLayer()) {
Expand Down Expand Up @@ -202,45 +196,22 @@ public JavaKind getStorageKind() {

}

/**
* Returns all possible types that this field can have. The result is not context sensitive,
* i.e., it is a union of all types found in all contexts.
*/
public TypeState getTypeState() {
if (getType().getStorageKind() != JavaKind.Object) {
return null;
} else if (isStatic()) {
return staticFieldFlow.getState();
} else {
return getInstanceFieldTypeState();
}
}

public TypeState getInstanceFieldTypeState() {
return instanceFieldFlow.getState();
public FieldTypeFlow getInitialFlow() {
return initialFlow;
}

public FieldTypeFlow getInitialInstanceFieldFlow() {
return initialInstanceFieldFlow;
public FieldTypeFlow getSinkFlow() {
return sinkFlow;
}

public FieldTypeFlow getStaticFieldFlow() {
assert Modifier.isStatic(this.getModifiers()) : this;

return staticFieldFlow;
}

/** Get the field type flow, stripped of any context. */
public ContextInsensitiveFieldTypeFlow getInstanceFieldFlow() {
assert !Modifier.isStatic(this.getModifiers()) : this;

return instanceFieldFlow;
return sinkFlow;
}

public void cleanupAfterAnalysis() {
staticFieldFlow = null;
instanceFieldFlow = null;
initialInstanceFieldFlow = null;
initialFlow = null;
sinkFlow = null;
readBy = null;
writtenBy = null;
}
Expand Down Expand Up @@ -420,14 +391,6 @@ public void setFieldValueInterceptor(Object fieldValueInterceptor) {
this.fieldValueInterceptor = fieldValueInterceptor;
}

public void setCanBeNull(boolean canBeNull) {
this.canBeNull = canBeNull;
}

public boolean canBeNull() {
return canBeNull;
}

@Override
public String getName() {
return wrapped.getName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,7 @@ public boolean registerAsUnsafeAccessed(Object reason) {
public void saturatePrimitiveField() {
assert fieldType.isPrimitive() || fieldType.isWordType() : this;
var bb = ((PointsToAnalysis) getUniverse().getBigbang());
if (isStatic()) {
staticFieldFlow.addState(bb, TypeState.anyPrimitiveState());
} else {
initialInstanceFieldFlow.addState(bb, TypeState.anyPrimitiveState());
instanceFieldFlow.addState(bb, TypeState.anyPrimitiveState());
}
initialFlow.addState(bb, TypeState.anyPrimitiveState());
sinkFlow.addState(bb, TypeState.anyPrimitiveState());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,19 @@ public class PointsToAnalysisType extends AnalysisType {
super(universe, javaType, storageKind, objectType, cloneableType);
}

@Override
public boolean registerAsUnsafeAllocated(Object reason) {
boolean result = super.registerAsUnsafeAllocated(reason);
if (result) {
var bb = (PointsToAnalysis) universe.getBigbang();
for (var f : getInstanceFields(true)) {
var field = (AnalysisField) f;
field.getInitialFlow().addState(bb, TypeState.defaultValueForKind(field.getStorageKind()));
}
}
return result;
}

/**
* @see AnalysisType#registerAsAssignable(BigBang)
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public class InlineBeforeAnalysisMethodScope extends PEMethodScope {
for (int i = 0; i < arguments.length; i++) {
constArgsWithReceiver[i] = arguments[i].isConstant();
}
policyScope = policy.openCalleeScope(cast(caller).policyScope, method);
policyScope = policy.openCalleeScope(cast(caller).policyScope, (AnalysisMethod) caller.method, method);
if (graph.getDebug().isLogEnabled()) {
graph.getDebug().logv(" ".repeat(inliningDepth) + "openCalleeScope for " + method.format("%H.%n(%p)") + ": " + policyScope);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ protected InlineBeforeAnalysisPolicy(NodePlugin[] nodePlugins) {

protected abstract AbstractPolicyScope createRootScope();

protected abstract AbstractPolicyScope openCalleeScope(AbstractPolicyScope outer, AnalysisMethod method);
protected abstract AbstractPolicyScope openCalleeScope(AbstractPolicyScope outer, AnalysisMethod caller, AnalysisMethod method);

/** @see InlineBeforeAnalysisGraphDecoder#shouldOmitIntermediateMethodInStates */
protected abstract boolean shouldOmitIntermediateMethodInState(AnalysisMethod method);
Expand Down Expand Up @@ -142,7 +142,7 @@ protected AbstractPolicyScope createRootScope() {
}

@Override
protected AbstractPolicyScope openCalleeScope(AbstractPolicyScope outer, AnalysisMethod method) {
protected AbstractPolicyScope openCalleeScope(AbstractPolicyScope outer, AnalysisMethod caller, AnalysisMethod method) {
throw AnalysisError.shouldNotReachHere("NO_INLINING policy should not try to inline");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,12 +172,30 @@ public StrengthenGraphs(BigBang bb, Universe converter) {
if (ImageBuildStatistics.Options.CollectImageBuildStatistics.getValue(bb.getOptions())) {
beforeCounters = new StrengthenGraphsCounters(ImageBuildStatistics.CheckCountLocation.BEFORE_STRENGTHEN_GRAPHS);
afterCounters = new StrengthenGraphsCounters(ImageBuildStatistics.CheckCountLocation.AFTER_STRENGTHEN_GRAPHS);
reportNeverNullInstanceFields(bb);
} else {
beforeCounters = null;
afterCounters = null;
}
}

private static void reportNeverNullInstanceFields(BigBang bb) {
int neverNull = 0;
int canBeNull = 0;
for (var field : bb.getUniverse().getFields()) {
if (!field.isStatic() && field.isReachable() && field.getType().getStorageKind() == JavaKind.Object) {
if (field.getSinkFlow().getState().canBeNull()) {
canBeNull++;
} else {
neverNull++;
}
}
}
ImageBuildStatistics imageBuildStats = ImageBuildStatistics.counters();
imageBuildStats.insert("instancefield_neverNull").addAndGet(neverNull);
imageBuildStats.insert("instancefield_canBeNull").addAndGet(canBeNull);
}

@SuppressWarnings("try")
public final void applyResults(AnalysisMethod method) {
var nodeReferences = method instanceof PointsToAnalysisMethod ptaMethod && ptaMethod.getTypeFlow().flowsGraphCreated()
Expand Down Expand Up @@ -369,9 +387,7 @@ public void simplify(Node n, SimplifierTool tool) {
* update the stamp directly to the stamp that is correct for the whole method and
* all inlined methods.
*/
var field = (AnalysisField) node.field();
var fieldTypeFlow = field.isStatic() ? field.getStaticFieldFlow() : field.getInstanceFieldFlow();
Object fieldNewStampOrConstant = strengthenStampFromTypeFlow(node, fieldTypeFlow, node, tool);
Object fieldNewStampOrConstant = strengthenStampFromTypeFlow(node, ((AnalysisField) node.field()).getSinkFlow(), node, tool);
if (fieldNewStampOrConstant instanceof JavaConstant) {
ConstantNode replacement = ConstantNode.forConstant((JavaConstant) fieldNewStampOrConstant, bb.getMetaAccess(), graph);
graph.replaceFixedWithFloating(node, replacement);
Expand Down
Loading

0 comments on commit 7a033b9

Please sign in to comment.