From 440357d2e832b823616dfc809d44881eabcb4ddd Mon Sep 17 00:00:00 2001 From: Skylot Date: Mon, 1 Jun 2020 22:24:42 +0100 Subject: [PATCH] fix: allow cross-block move inline (#946) Signed-off-by: Skylot --- jadx-core/src/main/java/jadx/core/Jadx.java | 3 -- .../core/dex/visitors/MoveInlineVisitor.java | 38 +++++++------------ .../tests/integration/TestReturnWrapping.java | 22 ++++------- 3 files changed, 22 insertions(+), 41 deletions(-) diff --git a/jadx-core/src/main/java/jadx/core/Jadx.java b/jadx-core/src/main/java/jadx/core/Jadx.java index 83cbfa589b6..09e97771ed6 100644 --- a/jadx-core/src/main/java/jadx/core/Jadx.java +++ b/jadx-core/src/main/java/jadx/core/Jadx.java @@ -113,9 +113,6 @@ public static List getPassesList(JadxArgs args) { passes.add(new MarkFinallyVisitor()); passes.add(new ConstInlineVisitor()); passes.add(new TypeInferenceVisitor()); - if (args.isRawCFGOutput()) { - passes.add(DotGraphVisitor.dumpRaw()); - } if (args.isDebugInfo()) { passes.add(new DebugInfoApplyVisitor()); } 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 index 766faddf898..d01705456d5 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/MoveInlineVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/MoveInlineVisitor.java @@ -1,7 +1,6 @@ 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; @@ -35,12 +34,9 @@ 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); + for (InsnNode insn : block.getInstructions()) { if (insn.getType() == InsnType.MOVE - && processMove(mth, block, insn, i)) { + && processMove(mth, insn)) { remover.addAndUnbind(insn); } } @@ -48,7 +44,7 @@ && processMove(mth, block, insn, i)) { } } - private static boolean processMove(MethodNode mth, BlockNode block, InsnNode move, int i) { + private static boolean processMove(MethodNode mth, InsnNode move) { RegisterArg resultArg = move.getResult(); InsnArg moveArg = move.getArg(0); if (resultArg.sameRegAndSVar(moveArg)) { @@ -58,21 +54,26 @@ private static boolean processMove(MethodNode mth, BlockNode block, InsnNode mov if (ssaVar.isUsedInPhi()) { return false; } - RegDebugInfoAttr debugInfo = resultArg.get(AType.REG_DEBUG_INFO); + RegDebugInfoAttr debugInfo = moveArg.get(AType.REG_DEBUG_INFO); for (RegisterArg useArg : ssaVar.getUseList()) { InsnNode useInsn = useArg.getParentInsn(); - if (useInsn == null || !fromThisBlock(block, useInsn, i)) { + if (useInsn == null) { return false; } - RegDebugInfoAttr debugInfoAttr = useArg.get(AType.REG_DEBUG_INFO); - if (debugInfoAttr != null) { - debugInfo = debugInfoAttr; + if (debugInfo == null) { + 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(); + if (useInsn == null) { + continue; + } InsnArg replaceArg; if (moveArg.isRegister()) { replaceArg = ((RegisterArg) moveArg).duplicate(useArg.getInitType()); @@ -83,21 +84,10 @@ private static boolean processMove(MethodNode mth, BlockNode block, InsnNode mov if (debugInfo != null) { replaceArg.addAttr(debugInfo); } - if (useInsn == null || !useInsn.replaceArg(useArg, replaceArg)) { + if (!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/test/java/jadx/tests/integration/TestReturnWrapping.java b/jadx-core/src/test/java/jadx/tests/integration/TestReturnWrapping.java index a74aaaf527a..763e3fd77e1 100644 --- a/jadx-core/src/test/java/jadx/tests/integration/TestReturnWrapping.java +++ b/jadx-core/src/test/java/jadx/tests/integration/TestReturnWrapping.java @@ -24,15 +24,14 @@ public static Object f2(Object arg0, int arg1) { int i = arg1; if (arg0 == null) { return ret + Integer.toHexString(i); - } else { - i++; - try { - ret = new Object().getClass(); - } catch (Exception e) { - ret = "Qwerty"; - } - return i > 128 ? arg0.toString() + ret.toString() : i; } + i++; + try { + ret = new Object().getClass(); + } catch (Exception e) { + ret = "Qwerty"; + } + return i > 128 ? arg0.toString() + ret.toString() : i; } public static int f3(int arg0) { @@ -54,12 +53,7 @@ public void test() { assertThat(code, containsString("return 255;")); assertThat(code, containsString("return arg0 + 1;")); - - // TODO: reduce code vars by name - // assertThat(code, containsString("return i > 128 ? arg0.toString() + ret.toString() : - // Integer.valueOf(i);")); - assertThat(code, containsString("return i2 > 128 ? arg0.toString() + ret.toString() : Integer.valueOf(i2);")); - + assertThat(code, containsString("return i > 128 ? arg0.toString() + ret.toString() : Integer.valueOf(i);")); assertThat(code, containsString("return arg0 + 2;")); assertThat(code, containsString("arg0 -= 951;")); }