re-enable modifyStmtToBlockAtTail test and fix removing edges of loops

soot-oss · Oct 10, 2023 · 8021671 · 8021671
1 parent a428c02
commit 8021671
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 31 deletions.
diff --git a/sootup.core/src/main/java/sootup/core/graph/MutableBlockStmtGraph.java b/sootup.core/src/main/java/sootup/core/graph/MutableBlockStmtGraph.java
@@ -967,8 +967,30 @@ public boolean removeEdge(@Nonnull Stmt from, @Nonnull Stmt to) {
       return false;
     }
 
-    // divide block if from and to are from the same block
-    if (blockOfFrom == blockOfTo) {
+    if (blockOfFrom.getTail() == from && blockOfTo.getHead() == to) {
+      // `from` and `to` are the tail and head of their respective blocks,
+      // meaning they either connect different blocks,
+      // or are a loop of the same block
+
+      // remove the connection between the blocks
+      boolean predecessorRemoved = blockOfTo.removePredecessorBlock(blockOfFrom);
+      boolean successorRemoved = blockOfFrom.removeSuccessorBlock(blockOfTo);
+      assert predecessorRemoved == successorRemoved;
+
+      if (!predecessorRemoved) {
+        // the blocks weren't connected
+        return false;
+      }
+
+      // the removal of the edge between `from` and `to` might have created blocks that can be merged
+      tryMergeWithPredecessorBlock(blockOfTo);
+      tryMergeWithSuccessorBlock(blockOfFrom);
+
+      return true;
+    } else if (blockOfFrom == blockOfTo) {
+      // `from` and `to` are part of the same block but aren't the tail and head,
+      // which means they are "inner" statements in the block and the block needs to be divided
+
       // divide block and don't link them
       final List<Stmt> stmtsOfBlock = blockOfFrom.getStmts();
       int toIdx = stmtsOfBlock.indexOf(from) + 1;
@@ -986,29 +1008,10 @@ public boolean removeEdge(@Nonnull Stmt from, @Nonnull Stmt to) {
         return false;
       }
     } else {
-      // `from` and `to` are part of different blocks
-
-      if (blockOfFrom.getTail() != from || blockOfTo.getHead() != to) {
-        // `from` and `to` aren't the tail and head of their respective blocks,
-        // which means they aren't connected
-        return false;
-      }
-
-      // remove the connection between the two blocks
-      boolean predecessorRemoved = blockOfTo.removePredecessorBlock(blockOfFrom);
-      boolean successorRemoved = blockOfFrom.removeSuccessorBlock(blockOfTo);
-      assert predecessorRemoved == successorRemoved;
-
-      if (!predecessorRemoved) {
-        // the blocks weren't connected
-        return false;
-      }
-
-      // the removal of the edge between `from` and `to` might have created blocks that can be merged
-      tryMergeWithPredecessorBlock(blockOfTo);
-      tryMergeWithSuccessorBlock(blockOfFrom);
-
-      return true;
+      // `from` and `to` are part of different blocks,
+      // and aren't tail and head of their respective block,
+      // which means they aren't connected
+      return false;
     }
   }
 

diff --git a/sootup.core/src/test/java/sootup/core/graph/MutableBlockStmtGraphTest.java b/sootup.core/src/test/java/sootup/core/graph/MutableBlockStmtGraphTest.java
@@ -211,6 +211,7 @@ public void removeStmtConditionalTailBetweenBlocks() {
         Collections.singletonList(thirdNop).toString(), blocksSorted.get(2).getStmts().toString());
   }
 
+  @Test
   public void modifyStmtToBlockAtTail() {
     MutableBlockStmtGraph graph = new MutableBlockStmtGraph();
     assertEquals(0, graph.getBlocksSorted().size());
@@ -247,13 +248,13 @@ public void modifyStmtToBlockAtTail() {
     graph.putEdge(conditionalStmt, secondNop);
     assertEquals(2, graph.getBlocksSorted().size());
 
-    assertEquals(3, graph.getBlocksSorted().get(0).getStmts().size());
-    assertEquals(2, graph.getBlocksSorted().get(0).getPredecessors().size());
-    assertEquals(2, graph.getBlocksSorted().get(0).getSuccessors().size());
+    assertEquals(3, graph.getBlockOf(conditionalStmt).getStmts().size());
+    assertEquals(2, graph.getBlockOf(conditionalStmt).getPredecessors().size());
+    assertEquals(2, graph.getBlockOf(conditionalStmt).getSuccessors().size());
 
-    assertEquals(1, graph.getBlocksSorted().get(1).getStmts().size());
-    assertEquals(1, graph.getBlocksSorted().get(1).getPredecessors().size());
-    assertEquals(1, graph.getBlocksSorted().get(1).getSuccessors().size());
+    assertEquals(1, graph.getBlockOf(firstNop).getStmts().size());
+    assertEquals(1, graph.getBlockOf(firstNop).getPredecessors().size());
+    assertEquals(1, graph.getBlockOf(firstNop).getSuccessors().size());
 
     // remove non-existing edge
     assertFalse(graph.removeEdge(firstNop, conditionalStmt));