fix removeDefLocalsOf at many places

soot-oss · Oct 19, 2024 · e57b2b7 · e57b2b7
1 parent c89d53f
commit e57b2b7
Show file tree

Hide file tree

Showing 11 changed files with 122 additions and 21 deletions.
diff --git a/shared-test-resources/bugfixes/DeadAssignmentEliminatorTest.class b/shared-test-resources/bugfixes/DeadAssignmentEliminatorTest.class
diff --git a/shared-test-resources/bugfixes/DeadAssignmentEliminatorTest.java b/shared-test-resources/bugfixes/DeadAssignmentEliminatorTest.java
@@ -23,4 +23,14 @@ void tc2() {
         }
         System.out.println(x);
     }
+
+    void tc3(int x) {
+        boolean trackReusableBuffers = false;
+        try {
+            trackReusableBuffers = "true".equals(System.getProperty("com.fasterxml.jackson.core.util.BufferRecyclers.trackReusableBuffers"));
+        } catch (SecurityException var2) {
+        }
+        boolean bufferRecyclerTracker = trackReusableBuffers ? true : null;
+    }
+
 }
diff --git a/sootup.core/src/main/java/sootup/core/jimple/basic/Local.java b/sootup.core/src/main/java/sootup/core/jimple/basic/Local.java
@@ -28,6 +28,7 @@
 import sootup.core.graph.StmtGraph;
 import sootup.core.jimple.Jimple;
 import sootup.core.jimple.common.stmt.AbstractDefinitionStmt;
+import sootup.core.jimple.common.stmt.JIfStmt;
 import sootup.core.jimple.common.stmt.Stmt;
 import sootup.core.jimple.visitor.Acceptor;
 import sootup.core.jimple.visitor.ImmediateVisitor;
@@ -153,6 +154,36 @@ public List<Stmt> getDefsForLocalUse(StmtGraph<?> graph, Stmt stmt) {
     return defStmts;
   }
 
+  public List<Stmt> getLocalOccurrences(Collection<Stmt> stmts) {
+    List<Stmt> localOccurrences = new ArrayList<>();
+    for (Stmt stmt : stmts) {
+      if (stmt instanceof AbstractDefinitionStmt) {
+        if (((AbstractDefinitionStmt) stmt).getLeftOp().equivTo(this)) {
+          localOccurrences.add(stmt);
+        }
+        ((AbstractDefinitionStmt) stmt)
+            .getUses()
+            .forEach(
+                use -> {
+                  if (use instanceof Local && ((Local) use).equivTo(this)) {
+                    localOccurrences.add(stmt);
+                  }
+                });
+      } else if (stmt instanceof JIfStmt) {
+        ((JIfStmt) stmt)
+            .getCondition()
+            .getUses()
+            .forEach(
+                value -> {
+                  if (value instanceof Local && value.equivTo(this)) {
+                    localOccurrences.add(stmt);
+                  }
+                });
+      }
+    }
+    return localOccurrences;
+  }
+
   @Override
   public <V extends ImmediateVisitor> V accept(@Nonnull V v) {
     v.caseLocal(this);

diff --git a/sootup.core/src/main/java/sootup/core/model/Body.java b/sootup.core/src/main/java/sootup/core/model/Body.java
@@ -459,12 +459,18 @@ public String toString() {
       }
     }
 
-    public void removeDefLocalsOf(@Nonnull Stmt stmt) {
+    public void removeDefLocalsOf(@Nonnull Stmt stmt, BodyBuilder builder) {
       stmt.getDef()
           .ifPresent(
               def -> {
                 if (def instanceof Local) {
-                  locals.remove(def);
+                  List<Stmt> localOccurrences =
+                      ((Local) def).getLocalOccurrences(builder.getStmts());
+                  // after removing stmt, if the local variable doesn't occur anywhere else then
+                  // safely remove
+                  if (localOccurrences.isEmpty()) {
+                    locals.remove(def);
+                  }
                 }
               });
     }

diff --git a/sootup.interceptors/src/main/java/sootup/interceptors/Aggregator.java b/sootup.interceptors/src/main/java/sootup/interceptors/Aggregator.java
@@ -235,7 +235,7 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view)
             graph.setStartingStmt(newStartingStmt);
           }
           graph.removeNode(relevantDef);
-          builder.removeDefLocalsOf(relevantDef);
+          builder.removeDefLocalsOf(relevantDef, builder);
         }
       }
     }

diff --git a/sootup.interceptors/src/main/java/sootup/interceptors/CastAndReturnInliner.java b/sootup.interceptors/src/main/java/sootup/interceptors/CastAndReturnInliner.java
@@ -120,7 +120,7 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view)
         graph.removeNode(assign, false);
         if (graph.predecessors(retStmt).isEmpty()) {
           graph.removeNode(retStmt, false);
-          builder.removeDefLocalsOf(assign);
+          builder.removeDefLocalsOf(assign, builder);
         }
       }
     }

diff --git a/sootup.interceptors/src/main/java/sootup/interceptors/ConditionalBranchFolder.java b/sootup.interceptors/src/main/java/sootup/interceptors/ConditionalBranchFolder.java
@@ -159,7 +159,7 @@ private void pruneExclusivelyReachableStmts(
         if (isExclusivelyReachable(stmtGraph, itStmt, reachedBranchingStmts)) {
           q.addAll(stmtGraph.successors(itStmt));
           stmtGraph.removeNode(itStmt, false);
-          builder.removeDefLocalsOf(itStmt);
+          builder.removeDefLocalsOf(itStmt, builder);
         }
       }
     }

diff --git a/sootup.interceptors/src/main/java/sootup/interceptors/DeadAssignmentEliminator.java b/sootup.interceptors/src/main/java/sootup/interceptors/DeadAssignmentEliminator.java
@@ -202,7 +202,7 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view)
       if (!essentialStmts.contains(stmt)) {
         if (stmtGraph.containsNode(stmt)) {
           stmtGraph.removeNode(stmt);
-          builder.removeDefLocalsOf(stmt);
+          builder.removeDefLocalsOf(stmt, builder);
         }
       }
     }
@@ -246,7 +246,7 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view)
       Stmt newInvoke =
           Jimple.newInvokeStmt(assignStmt.getInvokeExpr().get(), assignStmt.getPositionInfo());
       stmtGraph.replaceNode(assignStmt, newInvoke);
-      builder.removeDefLocalsOf(assignStmt);
+      builder.removeDefLocalsOf(assignStmt, builder);
     }
   }
 }
diff --git a/sootup.interceptors/src/main/java/sootup/interceptors/LocalPacker.java b/sootup.interceptors/src/main/java/sootup/interceptors/LocalPacker.java
@@ -92,6 +92,9 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view)
         Value use = iterator.next();
         if (use instanceof Local) {
           Local newLocal = localToNewLocal.get(use);
+          if (newLocal == null) {
+            continue;
+          }
           // assign a reasonable name
           if (!newLocals.contains(newLocal)) {
             int starPos = newLocal.getName().indexOf('*');
@@ -110,18 +113,20 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view)
       if (defOpt.isPresent() && defOpt.get() instanceof Local) {
         Local def = (Local) defOpt.get();
         Local newLocal = localToNewLocal.get(def);
-        // assign a reasonable name
-        if (!newLocals.contains(newLocal)) {
-          int starPos = newLocal.getName().indexOf('*');
-          String reasonableName = newLocal.getName().substring(0, starPos) + newLocals.size();
-          List<Local> oriLocals = newLocalToLocals.get(newLocal);
-          newLocal = newLocal.withName(reasonableName);
-          newLocals.add(newLocal);
-          for (Local ori : oriLocals) {
-            localToNewLocal.put(ori, newLocal);
+        if (newLocal != null) {
+          // assign a reasonable name
+          if (!newLocals.contains(newLocal)) {
+            int starPos = newLocal.getName().indexOf('*');
+            String reasonableName = newLocal.getName().substring(0, starPos) + newLocals.size();
+            List<Local> oriLocals = newLocalToLocals.get(newLocal);
+            newLocal = newLocal.withName(reasonableName);
+            newLocals.add(newLocal);
+            for (Local ori : oriLocals) {
+              localToNewLocal.put(ori, newLocal);
+            }
           }
+          newStmt = ((AbstractDefinitionStmt) newStmt).withNewDef(newLocal);
         }
-        newStmt = ((AbstractDefinitionStmt) newStmt).withNewDef(newLocal);
       }
       if (!stmt.equals(newStmt)) {
         stmtGraph.replaceNode(stmt, newStmt);

diff --git a/sootup.interceptors/src/main/java/sootup/interceptors/UnreachableCodeEliminator.java b/sootup.interceptors/src/main/java/sootup/interceptors/UnreachableCodeEliminator.java
@@ -72,7 +72,7 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view)
 
     for (Stmt stmt : removeQ) {
       graph.removeNode(stmt, false);
-      builder.removeDefLocalsOf(stmt);
+      builder.removeDefLocalsOf(stmt, builder);
     }
   }
 }
diff --git a/...rc/test/java/sootup/java/bytecode/frontend/interceptors/DeadAssignmentEliminatorTest.java b/...rc/test/java/sootup/java/bytecode/frontend/interceptors/DeadAssignmentEliminatorTest.java
@@ -1,8 +1,5 @@
 package sootup.java.bytecode.frontend.interceptors;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
-
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.*;
@@ -27,12 +24,15 @@
 import sootup.core.types.PrimitiveType;
 import sootup.core.util.Utils;
 import sootup.interceptors.DeadAssignmentEliminator;
+import sootup.interceptors.LocalPacker;
 import sootup.java.bytecode.frontend.inputlocation.PathBasedAnalysisInputLocation;
 import sootup.java.core.JavaIdentifierFactory;
 import sootup.java.core.language.JavaJimple;
 import sootup.java.core.types.JavaClassType;
 import sootup.java.core.views.JavaView;
 
+import static org.junit.jupiter.api.Assertions.*;
+
 @Tag("Java8")
 public class DeadAssignmentEliminatorTest {
 
@@ -229,4 +229,53 @@ public void testDeadAssignmentEliminator() {
             .collect(Collectors.toList()),
         Utils.filterJimple(body1.toString()));
   }
+
+  @Test
+  public void testLocalCountAfterDAE() {
+    AnalysisInputLocation inputLocation =
+        new PathBasedAnalysisInputLocation.ClassFileBasedAnalysisInputLocation(
+            classFilePath,
+            "",
+            SourceType.Application,
+            Arrays.asList(new DeadAssignmentEliminator(), new LocalPacker()));
+    JavaView view = new JavaView(Collections.singletonList(inputLocation));
+    final MethodSignature methodSignature =
+        view.getIdentifierFactory()
+            .getMethodSignature(
+                "DeadAssignmentEliminatorTest",
+                "tc3",
+                "void",
+                Collections.singletonList(PrimitiveType.getInt().getName()));
+
+    Body body = view.getMethod(methodSignature).get().getBody();
+    assertTrue(body.getLocals().size() == 5);
+    assertFalse(body.getStmts().isEmpty());
+    assertEquals(
+        Stream.of(
+                "DeadAssignmentEliminatorTest this0",
+                "int l1",
+                "unknown $stack2, l3, l4",
+                "this0 := @this: DeadAssignmentEliminatorTest",
+                "l1 := @parameter0: int",
+                "label1:",
+                "$stack2 = \"true\"",
+                "l3 = staticinvoke <java.lang.System: java.lang.String getProperty(java.lang.String)>(\"com.fasterxml.jackson.core.util.BufferRecyclers.trackReusableBuffers\")",
+                "l4 = virtualinvoke $stack2.<java.lang.String: boolean equals(java.lang.Object)>(l3)",
+                "label2:",
+                "goto label4",
+                "label3:",
+                "l3 := @caughtexception",
+                "label4:",
+                "if l4 == 0 goto label5",
+                "l3 = staticinvoke <java.lang.Boolean: java.lang.Boolean valueOf(boolean)>(1)",
+                "goto label6",
+                "label5:",
+                "l3 = null",
+                "label6:",
+                "l3 = virtualinvoke l3.<java.lang.Boolean: boolean booleanValue()>()",
+                "return",
+                "catch java.lang.SecurityException from label1 to label2 with label3")
+            .collect(Collectors.toList()),
+        Utils.filterJimple(body.toString()));
+  }
 }