From e22474e0a70609d311a0abde4da7048979ef515b Mon Sep 17 00:00:00 2001 From: Skylot Date: Sat, 9 May 2020 15:38:18 +0100 Subject: [PATCH] fix: inline move instructions to help process constructors (#927) --- jadx-core/src/main/java/jadx/core/Jadx.java | 1 + .../dex/instructions/args/RegisterArg.java | 10 +- .../java/jadx/core/dex/nodes/InsnNode.java | 11 ++ .../core/dex/visitors/ConstructorVisitor.java | 2 +- .../core/dex/visitors/MoveInlineVisitor.java | 103 ++++++++++++++++++ .../regions/variables/ProcessVariables.java | 20 ++++ .../invoke/TestConstructorWithMoves.java | 33 ++++++ .../invoke/TestConstructorWithMoves.smali | 19 ++++ 8 files changed, 197 insertions(+), 2 deletions(-) create mode 100644 jadx-core/src/main/java/jadx/core/dex/visitors/MoveInlineVisitor.java create mode 100644 jadx-core/src/test/java/jadx/tests/integration/invoke/TestConstructorWithMoves.java create mode 100644 jadx-core/src/test/smali/invoke/TestConstructorWithMoves.smali diff --git a/jadx-core/src/main/java/jadx/core/Jadx.java b/jadx-core/src/main/java/jadx/core/Jadx.java index 068436ea6..cf09e7dcd 100644 --- a/jadx-core/src/main/java/jadx/core/Jadx.java +++ b/jadx-core/src/main/java/jadx/core/Jadx.java @@ -58,6 +58,7 @@ public class Jadx { passes.add(new BlockFinish()); passes.add(new SSATransform()); + passes.add(new MoveInlineVisitor()); passes.add(new ConstructorVisitor()); passes.add(new InitCodeVariables()); passes.add(new MarkFinallyVisitor()); diff --git a/jadx-core/src/main/java/jadx/core/dex/instructions/args/RegisterArg.java b/jadx-core/src/main/java/jadx/core/dex/instructions/args/RegisterArg.java index ca9fe1ce9..f035224f0 100644 --- a/jadx-core/src/main/java/jadx/core/dex/instructions/args/RegisterArg.java +++ b/jadx-core/src/main/java/jadx/core/dex/instructions/args/RegisterArg.java @@ -130,12 +130,20 @@ public class RegisterArg extends InsnArg implements Named { return duplicate(getRegNum(), sVar); } + public RegisterArg duplicate(ArgType initType) { + return duplicate(getRegNum(), initType, sVar); + } + public RegisterArg duplicate(@Nullable SSAVar ssaVar) { return duplicate(getRegNum(), ssaVar); } public RegisterArg duplicate(int regNum, @Nullable SSAVar sVar) { - RegisterArg dup = new RegisterArg(regNum, getInitType()); + return duplicate(regNum, getInitType(), sVar); + } + + public RegisterArg duplicate(int regNum, ArgType initType, @Nullable SSAVar sVar) { + RegisterArg dup = new RegisterArg(regNum, initType); if (sVar != null) { // only 'set' here, 'assign' or 'use' will binds later dup.setSVar(sVar); diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/InsnNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/InsnNode.java index 98b3b7070..bd579e737 100644 --- a/jadx-core/src/main/java/jadx/core/dex/nodes/InsnNode.java +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/InsnNode.java @@ -217,6 +217,17 @@ public class InsnNode extends LineAttrNode { } } + public boolean canRemoveResult() { + switch (getType()) { + case INVOKE: + case CONSTRUCTOR: + return true; + + default: + return false; + } + } + public boolean canReorder() { if (contains(AFlag.DONT_GENERATE)) { if (getType() == InsnType.MONITOR_EXIT) { diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ConstructorVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ConstructorVisitor.java index fe866cc0c..90593ca6f 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ConstructorVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ConstructorVisitor.java @@ -24,7 +24,7 @@ import jadx.core.utils.InsnRemover; @JadxVisitor( name = "ConstructorVisitor", desc = "Replace invoke with constructor call", - runAfter = SSATransform.class, + runAfter = { SSATransform.class, MoveInlineVisitor.class }, runBefore = TypeInferenceVisitor.class ) public class ConstructorVisitor extends AbstractVisitor { diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/MoveInlineVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/MoveInlineVisitor.java new file mode 100644 index 000000000..766faddf8 --- /dev/null +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/MoveInlineVisitor.java @@ -0,0 +1,103 @@ +package jadx.core.dex.visitors; + +import java.util.ArrayList; +import java.util.List; + +import jadx.core.dex.attributes.AType; +import jadx.core.dex.attributes.nodes.RegDebugInfoAttr; +import jadx.core.dex.instructions.InsnType; +import jadx.core.dex.instructions.args.InsnArg; +import jadx.core.dex.instructions.args.RegisterArg; +import jadx.core.dex.instructions.args.SSAVar; +import jadx.core.dex.nodes.BlockNode; +import jadx.core.dex.nodes.InsnNode; +import jadx.core.dex.nodes.MethodNode; +import jadx.core.dex.visitors.shrink.CodeShrinkVisitor; +import jadx.core.dex.visitors.ssa.SSATransform; +import jadx.core.utils.InsnRemover; + +@JadxVisitor( + name = "MoveInlineVisitor", + desc = "Inline redundant move instructions", + runAfter = SSATransform.class, + runBefore = CodeShrinkVisitor.class +) +public class MoveInlineVisitor extends AbstractVisitor { + @Override + public void visit(MethodNode mth) { + if (mth.isNoCode()) { + return; + } + moveInline(mth); + } + + private static void moveInline(MethodNode mth) { + InsnRemover remover = new InsnRemover(mth); + for (BlockNode block : mth.getBasicBlocks()) { + remover.setBlock(block); + List insns = block.getInstructions(); + int size = insns.size(); + for (int i = 0; i < size; i++) { + InsnNode insn = insns.get(i); + if (insn.getType() == InsnType.MOVE + && processMove(mth, block, insn, i)) { + remover.addAndUnbind(insn); + } + } + remover.perform(); + } + } + + private static boolean processMove(MethodNode mth, BlockNode block, InsnNode move, int i) { + RegisterArg resultArg = move.getResult(); + InsnArg moveArg = move.getArg(0); + if (resultArg.sameRegAndSVar(moveArg)) { + return true; + } + SSAVar ssaVar = resultArg.getSVar(); + if (ssaVar.isUsedInPhi()) { + return false; + } + RegDebugInfoAttr debugInfo = resultArg.get(AType.REG_DEBUG_INFO); + for (RegisterArg useArg : ssaVar.getUseList()) { + InsnNode useInsn = useArg.getParentInsn(); + if (useInsn == null || !fromThisBlock(block, useInsn, i)) { + return false; + } + RegDebugInfoAttr debugInfoAttr = useArg.get(AType.REG_DEBUG_INFO); + if (debugInfoAttr != null) { + debugInfo = debugInfoAttr; + } + } + + // all checks passed, execute inline + for (RegisterArg useArg : new ArrayList<>(ssaVar.getUseList())) { + InsnNode useInsn = useArg.getParentInsn(); + InsnArg replaceArg; + if (moveArg.isRegister()) { + replaceArg = ((RegisterArg) moveArg).duplicate(useArg.getInitType()); + } else { + replaceArg = moveArg.duplicate(); + } + replaceArg.copyAttributesFrom(useArg); + if (debugInfo != null) { + replaceArg.addAttr(debugInfo); + } + if (useInsn == null || !useInsn.replaceArg(useArg, replaceArg)) { + mth.addWarnComment("Failed to replace arg in insn: " + useInsn); + } + } + return true; + } + + private static boolean fromThisBlock(BlockNode block, InsnNode insn, int curPos) { + List list = block.getInstructions(); + int size = list.size(); + for (int j = curPos; j < size; j++) { + if (list.get(j) == insn) { + return true; + } + } + return false; + } +} diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/variables/ProcessVariables.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/variables/ProcessVariables.java index a709995e7..d54efc374 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/variables/ProcessVariables.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/variables/ProcessVariables.java @@ -26,6 +26,7 @@ import jadx.core.dex.nodes.InsnNode; import jadx.core.dex.nodes.MethodNode; import jadx.core.dex.regions.loops.LoopRegion; import jadx.core.dex.visitors.AbstractVisitor; +import jadx.core.dex.visitors.regions.AbstractRegionVisitor; import jadx.core.dex.visitors.regions.DepthRegionTraversal; import jadx.core.dex.visitors.typeinference.TypeCompare; import jadx.core.dex.visitors.typeinference.TypeCompareEnum; @@ -41,6 +42,7 @@ public class ProcessVariables extends AbstractVisitor { if (mth.isNoCode() || mth.getSVars().isEmpty()) { return; } + removeUnusedResults(mth); List codeVars = collectCodeVars(mth); if (codeVars.isEmpty()) { @@ -64,6 +66,24 @@ public class ProcessVariables extends AbstractVisitor { } } + private static void removeUnusedResults(MethodNode mth) { + DepthRegionTraversal.traverse(mth, new AbstractRegionVisitor() { + @Override + public void processBlock(MethodNode mth, IBlock container) { + for (InsnNode insn : container.getInstructions()) { + RegisterArg resultArg = insn.getResult(); + if (resultArg != null) { + SSAVar ssaVar = resultArg.getSVar(); + if (ssaVar.getUseList().isEmpty() && insn.canRemoveResult()) { + insn.setResult(null); + mth.removeSVar(ssaVar); + } + } + } + } + }); + } + private void checkCodeVars(MethodNode mth, List codeVars) { int unknownTypesCount = 0; for (CodeVar codeVar : codeVars) { diff --git a/jadx-core/src/test/java/jadx/tests/integration/invoke/TestConstructorWithMoves.java b/jadx-core/src/test/java/jadx/tests/integration/invoke/TestConstructorWithMoves.java new file mode 100644 index 000000000..84ffe7138 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/invoke/TestConstructorWithMoves.java @@ -0,0 +1,33 @@ +package jadx.tests.integration.invoke; + +import org.junit.jupiter.api.Test; + +import jadx.tests.api.SmaliTest; + +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; + +public class TestConstructorWithMoves extends SmaliTest { + // @formatter:off + /* + public boolean test() { + java.lang.Boolean r5 = new java.lang.Boolean + r8 = r5 + r5 = r8 + r6 = r8 + java.lang.String r7 = "test" + r6.(r7) + java.lang.Boolean r5 = (java.lang.Boolean) r5 + boolean r5 = r5.booleanValue() + r3 = r5 + return r3 + } + */ + // @formatter:on + + @Test + public void test() { + assertThat(getClassNodeFromSmali()) + .code() + .containsOne("return new Boolean(\"test\").booleanValue();"); + } +} diff --git a/jadx-core/src/test/smali/invoke/TestConstructorWithMoves.smali b/jadx-core/src/test/smali/invoke/TestConstructorWithMoves.smali new file mode 100644 index 000000000..b001fbba2 --- /dev/null +++ b/jadx-core/src/test/smali/invoke/TestConstructorWithMoves.smali @@ -0,0 +1,19 @@ +.class public Linvoke/TestConstructorWithMoves; +.super Ljava/lang/Object; + +.method static public test()Z + .registers 11 + + new-instance v5, Ljava/lang/Boolean; + move-object v8, v5 + move-object v5, v8 + move-object v6, v8 + const-string v7, "test" + invoke-direct {v6, v7}, Ljava/lang/Boolean;->(Ljava/lang/String;)V + check-cast v5, Ljava/lang/Boolean; + invoke-virtual {v5}, Ljava/lang/Boolean;->booleanValue()Z + move-result v5 + move v3, v5 + return v3 +.end method +