fix: allow cross-block move inline (#946)

Signed-off-by: Skylot <skylot@gmail.com>
skylot · Jun 1, 2020 · 440357d · 440357d
1 parent 5e62b90
commit 440357d
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 41 deletions.
diff --git 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<IDexTreeVisitor> 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
@@ -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,20 +34,17 @@ private static void moveInline(MethodNode mth) {
 		InsnRemover remover = new InsnRemover(mth);
 		for (BlockNode block : mth.getBasicBlocks()) {
 			remover.setBlock(block);
-			List<InsnNode> 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);
 				}
 			}
 			remover.perform();
 		}
 	}
 
-	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<InsnNode> 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
@@ -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;"));
 	}