From da8cafed7f08d918f77b3c0afb9b0ded66cd6df7 Mon Sep 17 00:00:00 2001 From: sahilagichani Date: Wed, 14 Aug 2024 00:53:11 +0200 Subject: [PATCH 1/4] modified CopyPropagator to function as expected --- .../core/interceptors/CopyPropagator.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/sootup.java.core/src/main/java/sootup/java/core/interceptors/CopyPropagator.java b/sootup.java.core/src/main/java/sootup/java/core/interceptors/CopyPropagator.java index 6f397ef157c..e2b36438a35 100644 --- a/sootup.java.core/src/main/java/sootup/java/core/interceptors/CopyPropagator.java +++ b/sootup.java.core/src/main/java/sootup/java/core/interceptors/CopyPropagator.java @@ -34,7 +34,6 @@ import sootup.core.jimple.common.constant.NullConstant; import sootup.core.jimple.common.expr.JCastExpr; import sootup.core.jimple.common.stmt.AbstractDefinitionStmt; -import sootup.core.jimple.common.stmt.InvokableStmt; import sootup.core.jimple.common.stmt.JAssignStmt; import sootup.core.jimple.common.stmt.Stmt; import sootup.core.model.Body; @@ -61,13 +60,14 @@ public class CopyPropagator implements BodyInterceptor { public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view) { MutableStmtGraph stmtGraph = builder.getStmtGraph(); for (Stmt stmt : Lists.newArrayList(stmtGraph)) { - for (Iterator iterator = stmt.getUses().iterator(); iterator.hasNext(); ) { + Stmt newStmt = stmt; + for (Iterator iterator = newStmt.getUses().iterator(); iterator.hasNext(); ) { Value use = iterator.next(); if (!(use instanceof Local)) { continue; } - List defsOfUse = ((Local) use).getDefsForLocalUse(stmtGraph, stmt); + List defsOfUse = ((Local) use).getDefsForLocalUse(stmtGraph, newStmt); if (!isPropatabable(defsOfUse)) { continue; } @@ -75,9 +75,8 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view) AbstractDefinitionStmt defStmt = (AbstractDefinitionStmt) defsOfUse.get(0); Value rhs = defStmt.getRightOp(); // if rhs is a constant, then replace use, if it is possible - if (rhs instanceof Constant - && !((stmt instanceof InvokableStmt) && ((InvokableStmt) stmt).containsInvokeExpr())) { - replaceUse(stmtGraph, stmt, use, rhs); + if (rhs instanceof Constant) { + newStmt = replaceUse(stmtGraph, newStmt, use, rhs); } // if rhs is a cast expr with a ref type and its op is 0 (IntConstant or LongConstant) @@ -86,25 +85,27 @@ else if (rhs instanceof JCastExpr && rhs.getType() instanceof ReferenceType) { Value op = ((JCastExpr) rhs).getOp(); if (zeroIntConstInstance.equals(op) || zeroLongConstInstance.equals(op)) { - replaceUse(stmtGraph, stmt, use, NullConstant.getInstance()); + newStmt = replaceUse(stmtGraph, newStmt, use, NullConstant.getInstance()); } } // if rhs is a local, then replace use, if it is possible else if (rhs instanceof Local && !rhs.equivTo(use)) { - replaceUse(stmtGraph, stmt, use, rhs); + newStmt = replaceUse(stmtGraph, newStmt, use, rhs); } } } } - private void replaceUse( + private Stmt replaceUse( @Nonnull MutableStmtGraph graph, @Nonnull Stmt stmt, @Nonnull Value use, @Nonnull Value rhs) { if (!use.equivTo(rhs)) { // TODO: ms: check if rhs!=use would be enough Stmt newStmt = stmt.withNewUse(use, rhs); if (newStmt != stmt) { graph.replaceNode(stmt, newStmt); } + return newStmt; } + return stmt; } private boolean isPropatabable(@Nonnull List defsOfUse) { From 7a77c813970ef397adc54414d982521911bf5ffd Mon Sep 17 00:00:00 2001 From: sahilagichani Date: Wed, 14 Aug 2024 21:43:32 +0200 Subject: [PATCH 2/4] using set as stmt use as can't modify duplicate list elements while iterating --- .../java/sootup/java/core/interceptors/CopyPropagator.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sootup.java.core/src/main/java/sootup/java/core/interceptors/CopyPropagator.java b/sootup.java.core/src/main/java/sootup/java/core/interceptors/CopyPropagator.java index e2b36438a35..14097c826c5 100644 --- a/sootup.java.core/src/main/java/sootup/java/core/interceptors/CopyPropagator.java +++ b/sootup.java.core/src/main/java/sootup/java/core/interceptors/CopyPropagator.java @@ -22,8 +22,9 @@ */ import com.google.common.collect.Lists; -import java.util.Iterator; import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; import javax.annotation.Nonnull; import sootup.core.graph.MutableStmtGraph; import sootup.core.jimple.basic.Local; @@ -61,8 +62,8 @@ public void interceptBody(@Nonnull Body.BodyBuilder builder, @Nonnull View view) MutableStmtGraph stmtGraph = builder.getStmtGraph(); for (Stmt stmt : Lists.newArrayList(stmtGraph)) { Stmt newStmt = stmt; - for (Iterator iterator = newStmt.getUses().iterator(); iterator.hasNext(); ) { - Value use = iterator.next(); + Set valueList = newStmt.getUses().collect(Collectors.toSet()); + for (Value use : valueList) { if (!(use instanceof Local)) { continue; } From 972ffed39c0924fd1c23fd7f408b7ba402dd8c2f Mon Sep 17 00:00:00 2001 From: sahilagichani Date: Thu, 15 Aug 2024 17:56:53 +0200 Subject: [PATCH 3/4] added a check & removed sout in test --- .../sootup/java/bytecode/interceptors/CopyPropagatorTest.java | 3 ++- .../java/sootup/java/core/interceptors/CopyPropagator.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/CopyPropagatorTest.java b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/CopyPropagatorTest.java index 31333e15565..1415c00a12d 100644 --- a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/CopyPropagatorTest.java +++ b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/CopyPropagatorTest.java @@ -1,5 +1,6 @@ package sootup.java.bytecode.interceptors; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import categories.TestCategories; @@ -375,6 +376,6 @@ void testBigInput() { .get(); Body body = sootMethod.getBody(); - System.out.println(body); + assertFalse(body.toString().isEmpty()); } } diff --git a/sootup.java.core/src/main/java/sootup/java/core/interceptors/CopyPropagator.java b/sootup.java.core/src/main/java/sootup/java/core/interceptors/CopyPropagator.java index 14097c826c5..2e4d15867ff 100644 --- a/sootup.java.core/src/main/java/sootup/java/core/interceptors/CopyPropagator.java +++ b/sootup.java.core/src/main/java/sootup/java/core/interceptors/CopyPropagator.java @@ -99,7 +99,7 @@ else if (rhs instanceof Local && !rhs.equivTo(use)) { private Stmt replaceUse( @Nonnull MutableStmtGraph graph, @Nonnull Stmt stmt, @Nonnull Value use, @Nonnull Value rhs) { - if (!use.equivTo(rhs)) { // TODO: ms: check if rhs!=use would be enough + if (!use.equivTo(rhs) || rhs != use) { Stmt newStmt = stmt.withNewUse(use, rhs); if (newStmt != stmt) { graph.replaceNode(stmt, newStmt); From 02004ec65563fe8745b6f19f805159186e2e0fc4 Mon Sep 17 00:00:00 2001 From: sahilagichani Date: Fri, 16 Aug 2024 00:14:45 +0200 Subject: [PATCH 4/4] changed CopyPropagator acc to definition, added tc --- docs/bodyinterceptors.md | 4 +- .../interceptors/CopyPropagatorTest.class | Bin 0 -> 297 bytes .../interceptors/CopyPropagatorTest.java | 11 ++++ .../interceptors/CopyPropagatorTest.java | 47 +++++++++++++++++- .../core/interceptors/CopyPropagator.java | 44 +++++++++++++++- 5 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 shared-test-resources/interceptors/CopyPropagatorTest.class create mode 100644 shared-test-resources/interceptors/CopyPropagatorTest.java diff --git a/docs/bodyinterceptors.md b/docs/bodyinterceptors.md index 4039ee4508e..bd1de886a9a 100644 --- a/docs/bodyinterceptors.md +++ b/docs/bodyinterceptors.md @@ -101,7 +101,9 @@ Obviously, the code segmentl2 = 2; l3 = 3;is unreachable. It will b ### CopyPropagator -CopyPropagator is aBodyInterceptorthat supports the global copy propagation and constant propagation. +CopyPropagator is aBodyInterceptorthat supports the global copy propagation and constant propagation. + +Refer [Sreekala, S. K. and Vineeth Kumar Paleri. “Copy Propagation subsumes Constant Propagation.” ArXiv abs/2207.03894 (2022): n. pag.](https://arxiv.org/pdf/2207.03894) Example for global copy propagation: diff --git a/shared-test-resources/interceptors/CopyPropagatorTest.class b/shared-test-resources/interceptors/CopyPropagatorTest.class new file mode 100644 index 0000000000000000000000000000000000000000..5f0ebb5f6b7b8025e8ad85d7b42e0e32acf6e2b3 GIT binary patch literal 297 zcmZWj!AiqG6r7iAni{JKMJT8}cxydaym*R%QV@hv1xwFv;!;v-w#2mHXL%Al_yK;D zIP1lO`{2#&&dl5Q_5JY)V2n6|2S0@BAV5oytn^Ng*1B4b?(&r>8bK>9tFp-i{$RKW z(H3+kcJq2)+l^l8#@2K5LYtb{CnJcbWo2gD=iJnDov-=gH^n98!R>G%h#u^=F3e5o zl-~b27w&(YA>>-_co4T3`yOw7q7Z}gcgPzF2eLcaabO=EqO+!q2E@eAPOg(EO^&kD c{^OPUOC2)@s9&EV4_Oh~h?y50Q1fv31MQ+Py8r+H literal 0 HcmV?d00001 diff --git a/shared-test-resources/interceptors/CopyPropagatorTest.java b/shared-test-resources/interceptors/CopyPropagatorTest.java new file mode 100644 index 00000000000..b0a9ebbe41c --- /dev/null +++ b/shared-test-resources/interceptors/CopyPropagatorTest.java @@ -0,0 +1,11 @@ +public class CopyPropagatorTest { + + void tc1(int w) { + int y, z = 0; + y = w; + w = 10; + z = 20; + int x = y + z; + } + +} \ No newline at end of file diff --git a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/CopyPropagatorTest.java b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/CopyPropagatorTest.java index 1415c00a12d..6503c11d978 100644 --- a/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/CopyPropagatorTest.java +++ b/sootup.java.bytecode/src/test/java/sootup/java/bytecode/interceptors/CopyPropagatorTest.java @@ -1,12 +1,14 @@ package sootup.java.bytecode.interceptors; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; import categories.TestCategories; import java.nio.file.Paths; +import java.util.Arrays; import java.util.Collections; import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import sootup.core.graph.MutableStmtGraph; @@ -28,6 +30,9 @@ import sootup.core.signatures.MethodSignature; import sootup.core.types.VoidType; import sootup.core.util.ImmutableUtils; +import sootup.core.util.Utils; +import sootup.core.views.View; +import sootup.java.bytecode.inputlocation.JavaClassPathAnalysisInputLocation; import sootup.java.bytecode.inputlocation.PathBasedAnalysisInputLocation; import sootup.java.core.JavaIdentifierFactory; import sootup.java.core.interceptors.CopyPropagator; @@ -128,6 +133,44 @@ public class CopyPropagatorTest { JAssignStmt estmt13 = JavaJimple.newAssignStmt(r5, NullConstant.getInstance(), noStmtPositionInfo); + public View setUp() { + String baseDir = "../shared-test-resources/interceptors/"; + JavaClassPathAnalysisInputLocation inputLocation = + new JavaClassPathAnalysisInputLocation( + baseDir, SourceType.Library, Collections.emptyList()); + final JavaView view = new JavaView(Arrays.asList(inputLocation)); + return view; + } + + @Test + public void testCopyPropagationWithRedefinition() { + View view = setUp(); + final MethodSignature methodSignature = + view.getIdentifierFactory() + .getMethodSignature( + "CopyPropagatorTest", "tc1", "void", Collections.singletonList("int")); + Body bodyBefore = view.getMethod(methodSignature).get().getBody(); + final Body.BodyBuilder builder = Body.builder(bodyBefore, Collections.emptySet()); + new CopyPropagator().interceptBody(builder, view); + Body bodyAfter = builder.build(); + assertEquals( + Stream.of( + "CopyPropagatorTest this", + "int l1", + "unknown l2, l3, l4", + "this := @this: CopyPropagatorTest", + "l1 := @parameter0: int", + "l3 = 0", + "l2 = l1", + "l1 = 10", + "l3 = 20", + // l2 should not be replaced with l1 as l1 gets redefined + "l4 = l2 + 20", + "return") + .collect(Collectors.toList()), + Utils.filterJimple(bodyAfter.toString())); + } + @Test public void testEqualStmt() { assertTrue(eestmt4.equivTo(eestmt4.withRValue(NullConstant.getInstance()))); diff --git a/sootup.java.core/src/main/java/sootup/java/core/interceptors/CopyPropagator.java b/sootup.java.core/src/main/java/sootup/java/core/interceptors/CopyPropagator.java index 2e4d15867ff..4509c3fe262 100644 --- a/sootup.java.core/src/main/java/sootup/java/core/interceptors/CopyPropagator.java +++ b/sootup.java.core/src/main/java/sootup/java/core/interceptors/CopyPropagator.java @@ -22,6 +22,7 @@ */ import com.google.common.collect.Lists; +import java.util.Iterator; import java.util.List; import java.util.Set; import java.util.stream.Collectors; @@ -91,7 +92,48 @@ else if (rhs instanceof JCastExpr && rhs.getType() instanceof ReferenceType) { } // if rhs is a local, then replace use, if it is possible else if (rhs instanceof Local && !rhs.equivTo(use)) { - newStmt = replaceUse(stmtGraph, newStmt, use, rhs); + Local m = (Local) rhs; + if (use != m) { + Integer defCount = m.getDefs(stmtGraph.getStmts()).size(); + if (defCount == null || defCount == 0) { + throw new RuntimeException("Variable " + m + " used without definition!"); + } else if (defCount == 1) { + newStmt = replaceUse(stmtGraph, newStmt, use, rhs); + continue; + } + + List path = stmtGraph.getExtendedBasicBlockPathBetween(defStmt, newStmt); + if (path == null) { + // no path in the extended basic block + continue; + } + { + boolean isRedefined = false; + Iterator pathIt = path.iterator(); + // Skip first node + pathIt.next(); + // Make sure that m is not redefined along path + while (pathIt.hasNext()) { + Stmt s = (Stmt) pathIt.next(); + if (newStmt == s) { + // Don't look at the last statement + // since it is evaluated after the uses. + break; + } + if (s instanceof AbstractDefinitionStmt) { + if (((AbstractDefinitionStmt) s).getLeftOp() == m) { + isRedefined = true; + break; + } + } + } + + if (isRedefined) { + continue; + } + } + newStmt = replaceUse(stmtGraph, newStmt, use, rhs); + } } } }