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

fix a FlowAnalysis bug were non-returning loops were skipped #1688

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 8 additions & 9 deletions src/main/java/soot/Body.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import soot.jimple.IdentityStmt;
import soot.jimple.ParameterRef;
import soot.jimple.ThisRef;
import soot.options.Options;
Expand Down Expand Up @@ -340,7 +339,7 @@ public Chain<Trap> getTraps() {
*/
public Unit getThisUnit() {
for (Unit u : getUnits()) {
if (u instanceof IdentityStmt && ((IdentityStmt) u).getRightOp() instanceof ThisRef) {
if (u instanceof IdentityUnit && ((IdentityUnit) u).getRightOp() instanceof ThisRef) {
return u;
}
}
Expand All @@ -354,7 +353,7 @@ public Unit getThisUnit() {
* @return
*/
public Local getThisLocal() {
return (Local) (((IdentityStmt) getThisUnit()).getLeftOp());
return (Local) (((IdentityUnit) getThisUnit()).getLeftOp());
}

/**
Expand All @@ -366,8 +365,8 @@ public Local getThisLocal() {
*/
public Local getParameterLocal(int i) {
for (Unit s : getUnits()) {
if (s instanceof IdentityStmt) {
IdentityStmt is = (IdentityStmt) s;
if (s instanceof IdentityUnit) {
IdentityUnit is = (IdentityUnit) s;
Value rightOp = is.getRightOp();
if (rightOp instanceof ParameterRef) {
ParameterRef pr = (ParameterRef) rightOp;
Expand All @@ -394,8 +393,8 @@ public List<Local> getParameterLocals() {
Local[] res = new Local[numParams];
int numFound = 0;
for (Unit u : getUnits()) {
if (u instanceof IdentityStmt) {
IdentityStmt is = (IdentityStmt) u;
if (u instanceof IdentityUnit) {
IdentityUnit is = (IdentityUnit) u;
Value rightOp = is.getRightOp();
if (rightOp instanceof ParameterRef) {
int idx = ((ParameterRef) rightOp).getIndex();
Expand Down Expand Up @@ -433,8 +432,8 @@ public List<Value> getParameterRefs() {
Value[] res = new Value[numParams];
int numFound = 0;
for (Unit u : getUnits()) {
if (u instanceof IdentityStmt) {
Value rightOp = ((IdentityStmt) u).getRightOp();
if (u instanceof IdentityUnit) {
Value rightOp = ((IdentityUnit) u).getRightOp();
if (rightOp instanceof ParameterRef) {
ParameterRef pr = (ParameterRef) rightOp;
int idx = pr.getIndex();
Expand Down
82 changes: 38 additions & 44 deletions src/main/java/soot/toolkits/scalar/FlowAnalysis.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Queue;
import java.util.RandomAccess;
import java.util.Set;

import soot.baf.GotoInst;
import soot.jimple.GotoStmt;
Expand Down Expand Up @@ -129,59 +129,53 @@ <D, F> List<Entry<D, F>> newUniverse(DirectedGraph<D> g, GraphView gv, F entryFl
// out of universe node
Entry<D, F> superEntry = new Entry<D, F>(null, null);

List<D> entries = null;
List<D> actualEntries = gv.getEntries(g);

if (!actualEntries.isEmpty()) {
// normal cases: there is at least
// one return statement for a backward analysis
// or one entry statement for a forward analysis
entries = actualEntries;
} else {
// cases without any entry statement

List<D> entries;
{
List<D> actualEntries = gv.getEntries(g);
if (isForward) {
// case of a forward flow analysis on
// a method without any entry point
throw new RuntimeException("error: no entry point for method in forward analysis");
if (!actualEntries.isEmpty()) {
// normal case: there is at least one entry statement
entries = actualEntries;
} else {
// error case: forward flow analysis on a method without any entry point
throw new RuntimeException("error: forward analysis on an empty entry set.");
}
} else {
// case of backward analysis on
// a method which potentially has
// an infinite loop and no return statement
entries = new ArrayList<D>();

// a single head is expected
assert g.getHeads().size() == 1;
D head = g.getHeads().get(0);

// collect all 'goto' statements to catch the 'goto' from the infinite loop
Set<D> visitedNodes = new HashSet<D>();
List<D> workList = new ArrayList<D>();
workList.add(head);
for (D current; !workList.isEmpty();) {
current = workList.remove(0);
visitedNodes.add(current);

// only add 'goto' statements
if (current instanceof GotoInst || current instanceof GotoStmt) {
entries.add(current);
}
// In a backward analysis, entry points must include any return
// statements that exist (i.e. the 'actualEntries') but there
// could also be infinite loops with no return statement that
// must be included in the analysis as well.
ArrayList<D> extraEntries = new ArrayList<D>();

// Collect all 'goto' statements to capture the 'goto' from infinite loop(s)
HashSet<D> visitedNodes = new HashSet<D>();
LinkedList<D> workList = new LinkedList<D>(g.getHeads());
while (!workList.isEmpty()) {
D current = workList.remove(0);
if (visitedNodes.add(current)) {
// only add 'goto' statements
if (current instanceof GotoStmt || current instanceof GotoInst) {
extraEntries.add(current);
}

for (D next : g.getSuccsOf(current)) {
if (visitedNodes.contains(next)) {
continue;
for (D next : g.getSuccsOf(current)) {
if (!visitedNodes.contains(next)) {
workList.add(next);
}
}
workList.add(next);
}
}

//
if (entries.isEmpty()) {
if (actualEntries.isEmpty() && extraEntries.isEmpty()) {
throw new RuntimeException("error: backward analysis on an empty entry set.");
}
entries = new ArrayList<D>(actualEntries.size() + extraEntries.size());
entries.addAll(actualEntries);
entries.addAll(extraEntries);
}
assert (!entries.isEmpty());
assert (entries.stream().distinct().count() == entries.size());
}

visitEntry(visited, superEntry, entries);
superEntry.inFlow = entryFlow;
superEntry.outFlow = entryFlow;
Expand Down
Loading