Merge pull request #1118 from soot-oss/1095-bug-nullpointerexception-…

…cannot-invoke-sootupcorejimplebasiclocalgetname-because-newlocal-is-null fix bug in removeDefLocalsOf, also lp issue
soot-oss · Oct 30, 2024 · 1390a6e · 1390a6e
2 parents 4298c6e + 8a4b9ef
commit 1390a6e
Show file tree

Hide file tree

Showing 6 changed files with 100 additions and 15 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
@@ -23,6 +23,7 @@
  */
 
 import java.util.*;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import javax.annotation.Nonnull;
 import sootup.core.graph.StmtGraph;
@@ -153,6 +154,20 @@ public List<Stmt> getDefsForLocalUse(StmtGraph<?> graph, Stmt stmt) {
     return defStmts;
   }
 
+  public List<Stmt> getStmtsUsingOrDefiningthisLocal(Collection<Stmt> stmts, Stmt removedStmt) {
+    List<Stmt> localOccurrences = new ArrayList<>();
+    for (Stmt stmt : stmts) {
+      if (stmt.equivTo(removedStmt)) continue;
+      List<Value> stmtUsesAndDefs = stmt.getUsesAndDefs().collect(Collectors.toList());
+      for (Value stmtUse : stmtUsesAndDefs) {
+        if (stmtUse instanceof Local && stmtUse.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
@@ -464,7 +464,13 @@ public void removeDefLocalsOf(@Nonnull Stmt stmt) {
           .ifPresent(
               def -> {
                 if (def instanceof Local) {
-                  locals.remove(def);
+                  List<Stmt> localOccurrences =
+                      ((Local) def).getStmtsUsingOrDefiningthisLocal(this.graph.getStmts(), stmt);
+                  // 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/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/...rc/test/java/sootup/java/bytecode/frontend/interceptors/DeadAssignmentEliminatorTest.java b/...rc/test/java/sootup/java/bytecode/frontend/interceptors/DeadAssignmentEliminatorTest.java
@@ -1,7 +1,6 @@
 package sootup.java.bytecode.frontend.interceptors;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.*;
 
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -27,6 +26,7 @@
 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;
@@ -189,7 +189,7 @@ public void testDeadAssignmentEliminator() {
     assertEquals(
         Stream.of(
                 "DeadAssignmentEliminatorTest this",
-                "unknown $stack3, $stack4, $stack5",
+                "unknown $stack3, $stack4, $stack5, l1, l2",
                 "this := @this: DeadAssignmentEliminatorTest",
                 "l1 = 30",
                 "l2 = l1",
@@ -216,7 +216,7 @@ public void testDeadAssignmentEliminator() {
     assertEquals(
         Stream.of(
                 "DeadAssignmentEliminatorTest this",
-                "unknown $stack2, $stack3",
+                "unknown $stack2, $stack3, l1",
                 "this := @this: DeadAssignmentEliminatorTest",
                 "l1 = \"cde\"",
                 "$stack2 = virtualinvoke l1.<java.lang.String: int length()>()",
@@ -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()));
+  }
 }