Merge pull request #700 from Timbals/fix/aggregator

fix aggregator combining variables even when conflicting uses exist
soot-oss · Oct 7, 2023 · e8c1cb0 · e8c1cb0
2 parents 6f25a81 + 637fbd9
commit e8c1cb0
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 3 deletions.
diff --git a/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/Aggregator.java b/sootup.java.bytecode/src/main/java/sootup/java/bytecode/interceptors/Aggregator.java
@@ -21,14 +21,16 @@
  * #L%
  */
 
-import com.google.common.collect.Lists;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import javax.annotation.Nonnull;
 import sootup.core.graph.StmtGraph;
 import sootup.core.jimple.basic.Immediate;
+import sootup.core.jimple.basic.LValue;
 import sootup.core.jimple.basic.Local;
 import sootup.core.jimple.basic.Value;
 import sootup.core.jimple.common.expr.AbstractBinopExpr;
@@ -69,11 +71,11 @@ public Aggregator(boolean dontAggregateFieldLocals) {
    */
   @Override
   public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View<?> view) {
-
     StmtGraph<?> graph = builder.getStmtGraph();
     List<Stmt> stmts = builder.getStmts();
+    Map<LValue, Collection<Stmt>> usesMap = Body.collectUses(stmts);
 
-    for (Stmt stmt : Lists.newArrayList(stmts)) {
+    for (Stmt stmt : stmts) {
       if (!(stmt instanceof JAssignStmt)) {
         continue;
       }
@@ -90,6 +92,10 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View<?> vi
         if (!(val instanceof Local)) {
           continue;
         }
+        if (usesMap.get(val).size() > 1) {
+          // there are other uses, so it can't be aggregated
+          continue;
+        }
         List<AbstractDefinitionStmt> defs = ((Local) val).getDefs(stmts);
         if (defs.size() != 1) {
           continue;

diff --git a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/AggregatorTest.java b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/AggregatorTest.java
@@ -10,6 +10,7 @@
 import org.junit.Test;
 import sootup.core.inputlocation.AnalysisInputLocation;
 import sootup.core.inputlocation.ClassLoadingOptions;
+import sootup.core.jimple.Jimple;
 import sootup.core.jimple.basic.Local;
 import sootup.core.jimple.basic.NoPositionInformation;
 import sootup.core.jimple.basic.StmtPositionInfo;
@@ -27,6 +28,7 @@
 import sootup.java.core.JavaSootClass;
 import sootup.java.core.language.JavaJimple;
 import sootup.java.core.language.JavaLanguage;
+import sootup.java.core.types.JavaClassType;
 import sootup.java.core.views.JavaView;
 
 public class AggregatorTest {
@@ -77,6 +79,38 @@ public void testNoAggregation() {
     }
   }
 
+  @Test
+  public void noAggregationWithUse() {
+    Body.BodyBuilder builder = Body.builder();
+
+    StmtPositionInfo noPositionInfo = StmtPositionInfo.createNoStmtPositionInfo();
+
+    JavaClassType fileType = JavaIdentifierFactory.getInstance().getClassType("File");
+
+    Local a = JavaJimple.newLocal("a", fileType);
+    Local b = JavaJimple.newLocal("b", fileType);
+
+    Stmt assignA = JavaJimple.newAssignStmt(a, JavaJimple.newNewExpr(fileType), noPositionInfo);
+    // this use of `a` should prevent the aggregator from changing anything
+    Stmt useA = JavaJimple.newInvokeStmt(Jimple.newSpecialInvokeExpr(a, JavaIdentifierFactory.getInstance().parseMethodSignature("<File: void <init>()>")), noPositionInfo);
+    Stmt assignB = JavaJimple.newAssignStmt(b, a, noPositionInfo);
+    Stmt ret = JavaJimple.newReturnVoidStmt(noPositionInfo);
+
+    builder.setStartingStmt(assignA);
+    builder.addFlow(assignA, useA);
+    builder.addFlow(useA, assignB);
+    builder.addFlow(assignB, ret);
+
+    builder.setMethodSignature(
+            JavaIdentifierFactory.getInstance()
+                    .getMethodSignature("test", "ab.c", "void", Collections.emptyList()));
+
+    new Aggregator().interceptBody(builder, null);
+
+    // ensure that the assigner doesn't remove any statements
+    assertEquals(4, builder.getStmts().size());
+  }
+
   private static Body.BodyBuilder createBodyBuilder(boolean withAggregation) {
     StmtPositionInfo noPositionInfo = StmtPositionInfo.createNoStmtPositionInfo();