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 and implement ConditionalBranchFolder, Unreachable Code Eliminator #1041

Draft
wants to merge 20 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
9f9400a
fix ConditionalBranchFolder, forloop jimple order changed, fix try ca…
sahilagichani14 Aug 25, 2024
f3141e3
better approach
sahilagichani14 Aug 28, 2024
6a8c54f
fix and added test cases
sahilagichani14 Aug 29, 2024
7a40810
Merge branch 'refs/heads/develop' into 930-bug-conditionalbranchfolde…
sahilagichani14 Sep 2, 2024
a848013
added test case for ConditionalBranchFolder
sahilagichani14 Sep 3, 2024
4c6f99e
fix trap is not iterated exception in valid cases (i.e. there exists …
swissiety Sep 3, 2024
51f56cc
fix style
sahilagichani14 Sep 10, 2024
b801cd9
Merge branch 'refs/heads/develop' into 930-bug-conditionalbranchfolde…
sahilagichani14 Sep 10, 2024
626e249
fix exceptionial flow caused nontermination + order improvement
swissiety Sep 13, 2024
0452813
reduced more ordering issues
swissiety Sep 13, 2024
9a7555b
fix order for exceptions as well;
swissiety Sep 13, 2024
2424032
added tc
sahilagichani14 Sep 22, 2024
6577ab8
just 7 testcases of minimaltestsuite failing
swissiety Sep 17, 2024
92a3033
8 testcases of minimaltestsuite failing - exceptional ones
swissiety Sep 19, 2024
e39acf7
Merge branch 'refs/heads/develop' into 930-bug-conditionalbranchfolde…
sahilagichani14 Sep 30, 2024
5dad178
merged with develop
sahilagichani14 Sep 30, 2024
133a79f
Merge branch 'refs/heads/develop' into 930-bug-conditionalbranchfolde…
sahilagichani14 Oct 1, 2024
e0afcca
use UCE at the end, added a tc
sahilagichani14 Oct 2, 2024
dd5b793
Merge branch 'develop' into 930-bug-conditionalbranchfolder-removes-c…
stschott Oct 16, 2024
e517d7a
Merge branch 'refs/heads/develop' into 930-bug-conditionalbranchfolde…
sahilagichani14 Oct 25, 2024
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 @@ -562,7 +562,7 @@ private MutableBasicBlock addBlockInternal(
}

@Override
public void removeBlock(BasicBlock<?> block) {
public List<Stmt> removeBlock(BasicBlock<?> block) {
Pair<Integer, MutableBasicBlock> blockOfPair = stmtToBlock.get(block.getHead());
if (blockOfPair.getRight() != block) {
throw new IllegalArgumentException(
Expand All @@ -573,12 +573,40 @@ public void removeBlock(BasicBlock<?> block) {
List<Stmt> stmts = block.getStmts();
stmts.forEach(stmtToBlock::remove);

// remove current block from Predecessor & Successor
blockOf
.getPredecessors()
.forEach(
pred -> {
pred.removeFromSuccessorBlocks(blockOf);
});
blockOf
.getSuccessors()
.forEach(
succ -> {
succ.removePredecessorBlock(blockOf);
});

// unlink block from graph
blockOf.clearPredecessorBlocks();
blockOf.clearSuccessorBlocks();
blockOf.clearExceptionalSuccessorBlocks();

blocks.remove(blockOf);
return stmts;
}

@Override
public void replaceStmt(@Nonnull Stmt oldStmt, @Nonnull Stmt newStmt) {
Pair<Integer, MutableBasicBlock> blockPair = stmtToBlock.get(oldStmt);
if (blockPair == null) {
// Stmt does not exist in the graph
throw new IllegalArgumentException("splitStmt does not exist in this block!");
}
MutableBasicBlock block = blockPair.getRight();
block.replaceStmt(oldStmt, newStmt);
stmtToBlock.remove(oldStmt);
stmtToBlock.put(newStmt, blockPair);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,16 @@ public void addNode(@Nonnull Stmt stmt) {
addNode(stmt, Collections.emptyMap());
}

/** replace a "oldStmt" with "newStmt" in the StmtGraph */
public abstract void replaceStmt(@Nonnull Stmt oldStmt, @Nonnull Stmt newStmt);

/** inserts a "stmt" with exceptional flows "traps" into the StmtGraph */
public abstract void addNode(@Nonnull Stmt stmt, @Nonnull Map<ClassType, Stmt> traps);

/** creates a whole BasicBlock with the details from the parameters */
public abstract void addBlock(@Nonnull List<Stmt> stmts, @Nonnull Map<ClassType, Stmt> traps);

public abstract void removeBlock(BasicBlock<?> block);
public abstract List<Stmt> removeBlock(BasicBlock<?> block);

/**
* creates a whole BasicBlock which contains the sequence of (n-1)*fallsthrough()-stmt + optional
Expand Down
168 changes: 168 additions & 0 deletions sootup.core/src/main/java/sootup/core/graph/StmtGraph.java
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,170 @@ public List<Stmt> getExtendedBasicBlockPathBetween(@Nonnull Stmt from, @Nonnull
return null;
}

/**
* Finds and returns all possible paths between two blocks in a control flow graph. Look for a
* path of blocks in graph. ** Note: This path handle cycle ** (and this property implies
* uniqueness.). The path returned includes from and to.
*
* @param from start point for the path.
* @param to end point for the path.
* @return a list of paths, where each path is represented as a list of {@link MutableBasicBlock}.
* Each path in the list starts with the 'from' block and ends with the 'to' block. If no
* paths exist between the two blocks, an empty list is returned.
* @implNote This method performs a depth-first search (DFS) to explore all possible paths between
* the two blocks.
*/
public List<List<MutableBasicBlock>> getAllPaths(
@Nonnull MutableBasicBlock from, @Nonnull MutableBasicBlock to) {
List<List<MutableBasicBlock>> allPaths = new ArrayList<>();
List<MutableBasicBlock> currentPath = new ArrayList<>();
Set<MutableBasicBlock> visited = new HashSet<>();
dfs(from, to, visited, currentPath, allPaths);
return allPaths;
}

// Helper function to perform DFS and find all paths between current and target
private void dfs(
MutableBasicBlock current,
MutableBasicBlock target,
Set<MutableBasicBlock> visited,
List<MutableBasicBlock> currentPath,
List<List<MutableBasicBlock>> allPaths) {
// Add the current block to the path and mark it as visited
currentPath.add(current);
visited.add(current);

// If we reach the target block, add the current path to allPaths
if (current.equals(target)) {
allPaths.add(new ArrayList<>(currentPath));
} else {
// Explore all successors of the current block
for (MutableBasicBlock successor : current.getSuccessors()) {
if (!visited.contains(successor)) {
dfs(successor, target, visited, currentPath, allPaths);
}
}
}

// Backtrack: remove the current block from the path and mark it as unvisited
currentPath.remove(currentPath.size() - 1);
visited.remove(current);
}

/**
* Finds and returns all possible paths from start block in a control flow graph. Note: This path
* handle cycle ** (and this property implies uniqueness.). The path returned includes start
* block.
*
* @param start start point for the path.
* @return a list of paths, where each path is represented as a list of {@link MutableBasicBlock}.
* Each path in the list starts with the 'start' block. If no paths exist, an empty list is
* returned.
* @implNote This method performs a depth-first search (DFS) to explore all possible paths from
* start block.
*/
public List<List<MutableBasicBlock>> getAllPaths(@Nonnull MutableBasicBlock start) {
List<List<MutableBasicBlock>> allPaths = new ArrayList<>();
List<MutableBasicBlock> currentPath = new ArrayList<>();
Set<MutableBasicBlock> visited = new HashSet<>();
dfs(start, currentPath, visited, allPaths);
return allPaths;
}

// Helper function to perform DFS to explore all paths and find all paths from current block,
// considering loops
private void dfs(
MutableBasicBlock current,
List<MutableBasicBlock> currentPath,
Set<MutableBasicBlock> visited,
List<List<MutableBasicBlock>> allPaths) {
// Add current block to the path and mark it as visited
currentPath.add(current);
visited.add(current);

// If the current block has no successors, this is an end path
if (current.getSuccessors().isEmpty()) {
allPaths.add(new ArrayList<>(currentPath)); // Add a copy of the current path to allPaths
} else {
// Explore all successors
for (MutableBasicBlock successor : current.getSuccessors()) {
// Check if the successor is already in the current path (indicating a loop)
if (!visited.contains(successor)) {
dfs(successor, currentPath, visited, allPaths);
} else {
// Optional: Handle loops by recording the path up to the loop, if needed
allPaths.add(new ArrayList<>(currentPath));
}
}
}

// Backtrack: remove the current block from the path and visited set before going back
currentPath.remove(currentPath.size() - 1);
visited.remove(current);
}

/**
* Finds the first merge point between two starting blocks in a control flow graph (CFG). This
* method performs a breadth-first search (BFS) starting from two distinct blocks (`start1` and
* `start2`) and attempts to find the first common block (merge point) that can be reached from
* both starting points. The method returns the first merge point encountered during the
* traversal, or an empty set if no merge point is found.
*
* @param start1 The starting block for the first path in the CFG.
* @param start2 The starting block for the second path in the CFG.
* @return A set containing the first merge point block, or an empty set if no merge point is
* found. The set will contain a single `MutableBasicBlock` if a merge point is found.
*/
public List<MutableBasicBlock> findFirstMergePoint(
@Nonnull MutableBasicBlock start1, @Nonnull MutableBasicBlock start2) {
// Step 1: Initialize two sets to store the visited blocks from both start points
Set<MutableBasicBlock> visitedFromStart1 = new HashSet<>();
Set<MutableBasicBlock> visitedFromStart2 = new HashSet<>();

// Step 2: Initialize two queues for BFS traversal from both nodes
Queue<MutableBasicBlock> queue1 = new LinkedList<>();
Queue<MutableBasicBlock> queue2 = new LinkedList<>();

queue1.add(start1);
queue2.add(start2);
visitedFromStart1.add(start1);
visitedFromStart2.add(start2);

// Step 3: Traverse from both nodes simultaneously
while (!queue1.isEmpty() || !queue2.isEmpty()) {
if (!queue1.isEmpty()) {
MutableBasicBlock current1 = queue1.poll();
if (visitedFromStart2.contains(current1)) {
return Collections.singletonList(current1); // Found the first merge point
}
for (MutableBasicBlock successor : current1.getSuccessors()) {
// handle cycles in CFG
if (!visitedFromStart1.contains(successor)) {
visitedFromStart1.add(successor);
queue1.add(successor);
}
}
}

if (!queue2.isEmpty()) {
MutableBasicBlock current2 = queue2.poll();
if (visitedFromStart1.contains(current2)) {
return Collections.singletonList(current2); // Found the first merge point
}
for (MutableBasicBlock successor : current2.getSuccessors()) {
// handle cycles in CFG
if (!visitedFromStart2.contains(successor)) {
visitedFromStart2.add(successor);
queue2.add(successor);
}
}
}
}

// Step 4: If no merge point is found, return null
return Collections.emptyList();
}

@Override
public boolean equals(Object o) {
if (o == this) {
Expand Down Expand Up @@ -356,6 +520,10 @@ public Iterator<Stmt> iterator() {
public List<Stmt> getBranchTargetsOf(BranchingStmt fromStmt) {
final List<Stmt> successors = successors(fromStmt);
if (fromStmt instanceof JIfStmt) {
// Conditional Branch Folder removes neverReachedSucessor Target
if (successors.size() == 1) {
return Collections.singletonList(successors.get(0));
}
swissiety marked this conversation as resolved.
Show resolved Hide resolved
// remove the first successor as if its a fallsthrough stmt and not a branch target
return Collections.singletonList(successors.get(1));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,12 @@
import com.google.common.collect.Lists;
import java.util.*;
import javax.annotation.Nonnull;
import sootup.core.graph.MutableBasicBlock;
import sootup.core.graph.MutableStmtGraph;
import sootup.core.graph.StmtGraph;
import sootup.core.jimple.Jimple;
import sootup.core.jimple.common.constant.*;
import sootup.core.jimple.common.stmt.BranchingStmt;
import sootup.core.jimple.common.stmt.FallsThroughStmt;
import sootup.core.jimple.common.stmt.JGotoStmt;
import sootup.core.jimple.common.stmt.JIfStmt;
import sootup.core.jimple.common.stmt.Stmt;
import sootup.core.model.Body;
Expand All @@ -50,7 +51,14 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view)

final MutableStmtGraph stmtGraph = builder.getStmtGraph();

for (Stmt stmt : Lists.newArrayList(stmtGraph.getNodes())) {
ArrayList<Stmt> stmtsList = Lists.newArrayList(stmtGraph.getNodes());
List<Stmt> removedStmts = new ArrayList<>();
for (Stmt stmt : stmtsList) {
// Statements which were removed while removing nodes
if (removedStmts.contains(stmt)) {
swissiety marked this conversation as resolved.
Show resolved Hide resolved
continue;
}

if (!(stmt instanceof JIfStmt)) {
continue;
}
Expand Down Expand Up @@ -85,7 +93,11 @@ else if (evaluatedCondition instanceof DoubleConstant) {
continue;
}

final List<Stmt> ifSuccessors = stmtGraph.successors(ifStmt);
List<Stmt> ifSuccessors = stmtGraph.successors(ifStmt);
// The successors of IfStmt have true branch at index 0 & false branch at index 1.
// However, in other parts of code, TRUE_BRANCH_IDX is defined as 1 & FALSE_BRANCH_IDX as 0.
// To maintain consistency, we need to reverse the order of the successors.
ifSuccessors = Lists.reverse(ifSuccessors);
final Stmt tautologicSuccessor;
final Stmt neverReachedSucessor;

Expand All @@ -102,28 +114,64 @@ else if (evaluatedCondition instanceof DoubleConstant) {
neverReachedSucessor = ifSuccessors.get(JIfStmt.FALSE_BRANCH_IDX);
}

// link previous stmt with always-reached successor of the if-Stmt
for (Stmt predecessor : stmtGraph.predecessors(ifStmt)) {
List<Integer> successorIdxList = stmtGraph.removeEdge(predecessor, ifStmt);

if (predecessor instanceof FallsThroughStmt) {
FallsThroughStmt fallsThroughPred = (FallsThroughStmt) predecessor;
for (Integer successorIdx : successorIdxList) {
stmtGraph.putEdge(fallsThroughPred, tautologicSuccessor);
}
MutableBasicBlock ifStmtBlock = (MutableBasicBlock) stmtGraph.getBlockOf(ifStmt);
MutableBasicBlock tautologicSuccessorBlock =
(MutableBasicBlock) stmtGraph.getBlockOf(tautologicSuccessor);
MutableBasicBlock neverReachedSucessorBlock =
(MutableBasicBlock) stmtGraph.getBlockOf(neverReachedSucessor);
List<MutableBasicBlock> firstMergePoint =
stmtGraph.findFirstMergePoint(tautologicSuccessorBlock, neverReachedSucessorBlock);
// No merge point found
assert firstMergePoint.size() <= 1;
if (firstMergePoint.isEmpty()) {
// get all paths from ifStmt and remove the paths which contains neverReachedSucessorBlock
List<List<MutableBasicBlock>> allPaths = stmtGraph.getAllPaths(ifStmtBlock);
System.out.println("No merge point: " + allPaths);
Set<MutableBasicBlock> blocksToRemove = new HashSet<>();
allPaths.stream()
.filter(path -> path.contains(neverReachedSucessorBlock))
.forEach(path -> blocksToRemove.addAll(path));
System.out.println("No merge point after filtering paths that contain NR: " + allPaths);
// we will remove ifStmtBlock at the end
blocksToRemove.remove(ifStmtBlock);
for (MutableBasicBlock block : blocksToRemove) {
List<Stmt> stmts = stmtGraph.removeBlock(block);
removedStmts.addAll(stmts);
}
} else {
MutableBasicBlock mergePoint = firstMergePoint.get(0);
if (mergePoint == neverReachedSucessorBlock) {
List<Integer> successorIdxList = stmtGraph.removeEdge(ifStmt, neverReachedSucessor);
} else {
// should not be anything else than BranchingStmt.. just Stmt can have no successor
BranchingStmt branchingPred = (BranchingStmt) predecessor;
for (Integer successorIdx : successorIdxList) {
stmtGraph.putEdge(branchingPred, successorIdx, tautologicSuccessor);
List<List<MutableBasicBlock>> allPaths = stmtGraph.getAllPaths(ifStmtBlock, mergePoint);
System.out.println("If to Merge point: " + allPaths);
Set<MutableBasicBlock> blocksToRemove = new HashSet<>();
allPaths.stream()
.filter(path -> path.contains(neverReachedSucessorBlock))
.forEach(path -> blocksToRemove.addAll(path));
System.out.println("Merge point, After filtering paths that contain NR: " + allPaths);
// we will remove ifStmtBlock at the end
blocksToRemove.remove(ifStmtBlock);
blocksToRemove.remove(mergePoint);
for (MutableBasicBlock block : blocksToRemove) {
List<Stmt> stmts = stmtGraph.removeBlock(block);
removedStmts.addAll(stmts);
}
}
}

stmtGraph.removeNode(ifStmt, false);
// replace ifStmt block
JGotoStmt gotoStmt = Jimple.newGotoStmt(ifStmt.getPositionInfo());
// ifStmtBlock.replaceStmt(ifStmt, gotoStmt);

// ifStmtBlock.replaceSuccessorBlock(neverReachedSucessorBlock, null);
stmtGraph.replaceStmt(ifStmt, gotoStmt);

pruneExclusivelyReachableStmts(builder, neverReachedSucessor);
// stmtGraph.removeStmt(ifStmt);
removedStmts.add(ifStmt);
removedStmts.add(gotoStmt);
}
System.out.println("New StatementGraph" + stmtGraph);
}

private void pruneExclusivelyReachableStmts(
Expand Down
Loading