Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce bottom store to improve dataflow analysis #167

Open
wants to merge 27 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
7e91f46
initial bottom-store implementation
d367wang Apr 1, 2021
2237641
Merge branch 'master' of https://github.com/opprop/checker-framework …
d367wang May 20, 2021
6ec3d77
Merge branch 'master' of https://github.com/opprop/checker-framework …
d367wang Jul 6, 2021
0122edf
undo import *
d367wang Jul 6, 2021
c276efa
fix compile error
d367wang Aug 4, 2021
cceadf7
fix bug
d367wang Aug 4, 2021
81c8059
add lock bottom store; remove FPs involving if(true/false)
d367wang Aug 5, 2021
887a888
NullnessBottom.getValue returns two qualifiers
d367wang Aug 5, 2021
9af8f2e
xxxBottomStore.getValue returns abstract value with bottom annotations
d367wang Aug 5, 2021
0a4adad
remove @Nullable on xxxBottomStore.getValue
d367wang Aug 5, 2021
6775dfa
fix bug
d367wang Aug 5, 2021
68b238b
make bottom-store logic coherent to CFAbstract classes
d367wang Aug 9, 2021
826d25a
Merge branch 'master' of https://github.com/opprop/checker-framework …
d367wang Aug 27, 2021
d6ddb82
adapt expected files
d367wang Aug 27, 2021
ee35b60
Merge branch 'master' of https://github.com/opprop/checker-framework …
d367wang Sep 16, 2021
080ff48
remove BOTH_TO_THEN and BOTH_TO_ELSE
d367wang Sep 16, 2021
d70186b
fix bug and refactor
d367wang Sep 17, 2021
5b616d3
hack CFAbastractValue.combineOneAnnotation
d367wang Sep 22, 2021
e83dea7
only use bottom store when the boolean literal is a condition on its own
d367wang Sep 22, 2021
ea69c0a
suppress intern error
d367wang Sep 22, 2021
2e03227
adapt expected error
d367wang Sep 22, 2021
7408317
change bottom value for lock type system
d367wang Sep 23, 2021
44c97a7
fix bug in bottom store, to resolve the must-call case
d367wang Sep 23, 2021
a6509e5
adapt bottom value for lock type system
d367wang Sep 23, 2021
9041117
no insertion to bottom store
d367wang Sep 28, 2021
40a27ca
add javadoc
d367wang Sep 28, 2021
a131f41
apply default to void methodreturns
d367wang Sep 28, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,9 @@ public CFValue createAbstractValue(
Set<AnnotationMirror> annotations, TypeMirror underlyingType) {
return defaultCreateAbstractValue(this, annotations, underlyingType);
}

@Override
public LockStore getBottomStore(boolean sequentialSemantics) {
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,12 @@ public KeyForValue createAbstractValue(
}
return new KeyForValue(this, annotations, underlyingType);
}

@Override
public KeyForStore getBottomStore(boolean sequentialSemantics) {
if (bottomStore == null) {
bottomStore = new KeyForBottomStore(this, sequentialSemantics);
}
return bottomStore;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
package org.checkerframework.checker.nullness;

import org.checkerframework.checker.nullness.qual.Nullable;
import org.checkerframework.dataflow.analysis.FlowExpressions;
import org.checkerframework.dataflow.cfg.node.ArrayAccessNode;
import org.checkerframework.dataflow.cfg.node.FieldAccessNode;
import org.checkerframework.dataflow.cfg.node.LocalVariableNode;
import org.checkerframework.dataflow.cfg.node.MethodInvocationNode;
import org.checkerframework.dataflow.cfg.node.Node;
import org.checkerframework.dataflow.cfg.node.ThisLiteralNode;
import org.checkerframework.framework.flow.CFAbstractAnalysis;
import org.checkerframework.framework.type.AnnotatedTypeFactory;

import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.type.TypeMirror;

public class KeyForBottomStore extends KeyForStore {
public KeyForBottomStore(
CFAbstractAnalysis<KeyForValue, KeyForStore, ?> analysis, boolean sequentialSemantics) {
super(analysis, sequentialSemantics);
}

@Override
public boolean isBottom() {
return true;
}

@Override
public KeyForStore copy() {
// throw new BugInCF("Copying of bottom store is not allowed.");
return this;
}

/** The LUB of a bottom store and a second store is the second store */
@Override
public KeyForStore leastUpperBound(KeyForStore other) {
return other;
}

@Override
public KeyForStore widenedUpperBound(KeyForStore previous) {
return previous;
}

@Override
public boolean equals(@Nullable Object o) {
return this == o;
}

@Nullable @Override
public KeyForValue getValue(FlowExpressions.Receiver expr) {
return null;
}

@Nullable @Override
public KeyForValue getValue(FieldAccessNode n) {
return null;
}

@Nullable @Override
public KeyForValue getValue(MethodInvocationNode n) {
return null;
}

@Nullable @Override
public KeyForValue getValue(ArrayAccessNode n) {
return null;
}

@Nullable @Override
public KeyForValue getValue(LocalVariableNode n) {
return null;
}

@Nullable @Override
public KeyForValue getValue(ThisLiteralNode n) {
return null;
}

@Override
public void updateForMethodCall(
MethodInvocationNode n, AnnotatedTypeFactory atypeFactory, KeyForValue val) {}

@Override
public void insertValue(FlowExpressions.Receiver r, AnnotationMirror a) {}

@Override
public void insertOrRefine(FlowExpressions.Receiver r, AnnotationMirror newAnno) {}

@Override
public void insertValue(FlowExpressions.Receiver r, @Nullable KeyForValue value) {}

@Override
public void insertThisValue(AnnotationMirror a, TypeMirror underlyingType) {}

@Override
public void replaceValue(FlowExpressions.Receiver r, @Nullable KeyForValue value) {}

@Override
public void clearValue(FlowExpressions.Receiver r) {}

@Override
public void updateForAssignment(Node n, @Nullable KeyForValue val) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,12 @@ public NullnessValue createAbstractValue(
}
return new NullnessValue(this, annotations, underlyingType);
}

@Override
public NullnessStore getBottomStore(boolean sequentialSemantics) {
if (bottomStore == null) {
bottomStore = new NullnessBottomStore(this, sequentialSemantics);
}
return bottomStore;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
package org.checkerframework.checker.nullness;

import org.checkerframework.checker.nullness.qual.Nullable;
import org.checkerframework.dataflow.analysis.FlowExpressions;
import org.checkerframework.dataflow.cfg.node.ArrayAccessNode;
import org.checkerframework.dataflow.cfg.node.FieldAccessNode;
import org.checkerframework.dataflow.cfg.node.LocalVariableNode;
import org.checkerframework.dataflow.cfg.node.MethodInvocationNode;
import org.checkerframework.dataflow.cfg.node.Node;
import org.checkerframework.dataflow.cfg.node.ThisLiteralNode;
import org.checkerframework.framework.flow.CFAbstractAnalysis;
import org.checkerframework.framework.type.AnnotatedTypeFactory;

import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.type.TypeMirror;

public class NullnessBottomStore extends NullnessStore {
private final AnnotationMirror NONNULL;

public NullnessBottomStore(
CFAbstractAnalysis<NullnessValue, NullnessStore, ?> analysis,
boolean sequentialSemantics) {
super(analysis, sequentialSemantics);
NONNULL = ((NullnessAnnotatedTypeFactory) analysis.getTypeFactory()).NONNULL;
}

@Override
public boolean isBottom() {
return true;
}

@Override
public NullnessStore copy() {
// throw new BugInCF("Copying of bottom store is not allowed.");
return this;
}

/** The LUB of a bottom store and a second store is the second store */
@Override
public NullnessStore leastUpperBound(NullnessStore other) {
return other;
}

@Override
public NullnessStore widenedUpperBound(NullnessStore previous) {
return previous;
}

@Override
public boolean equals(@Nullable Object o) {
return this == o;
}

@Nullable @Override
public NullnessValue getValue(FlowExpressions.Receiver expr) {
return analysis.createSingleAnnotationValue(NONNULL, expr.getType());
}

private NullnessValue getBottomValue(Node n) {
return analysis.createSingleAnnotationValue(NONNULL, n.getType());
}

@Nullable @Override
public NullnessValue getValue(FieldAccessNode n) {
return getBottomValue(n);
}

@Nullable @Override
public NullnessValue getValue(MethodInvocationNode n) {
return getBottomValue(n);
}

@Nullable @Override
public NullnessValue getValue(ArrayAccessNode n) {
return getBottomValue(n);
}

@Nullable @Override
public NullnessValue getValue(LocalVariableNode n) {
return getBottomValue(n);
}

@Nullable @Override
public NullnessValue getValue(ThisLiteralNode n) {
return getBottomValue(n);
}

@Override
public void updateForMethodCall(
MethodInvocationNode n, AnnotatedTypeFactory atypeFactory, NullnessValue val) {}

@Override
public void insertValue(FlowExpressions.Receiver r, AnnotationMirror a) {}

@Override
public void insertOrRefine(FlowExpressions.Receiver r, AnnotationMirror newAnno) {}

@Override
public void insertValue(FlowExpressions.Receiver r, @Nullable NullnessValue value) {}

@Override
public void insertThisValue(AnnotationMirror a, TypeMirror underlyingType) {}

@Override
public void replaceValue(FlowExpressions.Receiver r, @Nullable NullnessValue value) {}

@Override
public void clearValue(FlowExpressions.Receiver r) {}

@Override
public void updateForAssignment(Node n, @Nullable NullnessValue val) {}
}
11 changes: 11 additions & 0 deletions checker/tests/nullness/flow/DeadBranch.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import org.checkerframework.checker.nullness.qual.Nullable;

public class IfTrue {

Object foo(@Nullable Object myObj, boolean x) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need boolean x?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing out! I need to add another case using x as the condition.

if (x) {
myObj = new Object();
}
return myObj;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,11 @@ public static enum FlowRule {
* @return the String representation of this store
*/
String visualize(CFGVisualizer<?, S, ?> viz);

/**
* Is this store a bottom store?
*
* @return true if this is a bottom store
*/
boolean isBottom();
}
Original file line number Diff line number Diff line change
Expand Up @@ -167,4 +167,9 @@ public boolean canAlias(FlowExpressions.Receiver a, FlowExpressions.Receiver b)
public String visualize(CFGVisualizer<?, ConstantPropagationStore, ?> viz) {
return viz.visualizeStoreKeyVal("constant propagation", null);
}

@Override
public boolean isBottom() {
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -142,4 +142,9 @@ public String visualize(CFGVisualizer<?, LiveVarStore, ?> viz) {
public String toString() {
return liveVarValueSet.toString();
}

@Override
public boolean isBottom() {
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ public abstract class CFAbstractAnalysis<
/** Instance of the types utility. */
protected final Types types;

protected S bottomStore;

/**
* Create a CFAbstractAnalysis.
*
Expand Down Expand Up @@ -131,6 +133,13 @@ public T createTransferFunction() {
*/
public abstract S createEmptyStore(boolean sequentialSemantics);

/**
* Create the unique shared instance of bottom store for the underlying type system
*
* @return the bottom store instance of the appropriate type
*/
public abstract S getBottomStore(boolean sequentialSemantics);

/**
* Returns an identical copy of the store {@code s}.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -853,13 +853,19 @@ public S copy() {
return analysis.createCopiedStore((S) this);
}

@SuppressWarnings("unchecked")
@Override
public S leastUpperBound(S other) {
if (this.isBottom()) return other;
if (other != null && other.isBottom()) return (S) this;
return upperBound(other, false);
}

@SuppressWarnings("unchecked")
@Override
public S widenedUpperBound(S previous) {
if (this.isBottom()) return previous;
if (previous != null && previous.isBottom()) return (S) this;
return upperBound(previous, true);
}

Expand Down Expand Up @@ -1060,4 +1066,9 @@ protected String internalVisualize(CFGVisualizer<V, S, ?> viz) {
}
return res.toString();
}

@Override
public boolean isBottom() {
return true;
}
}
Loading