fix: inline constants in chained move instructions (#399)

jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlineVisitor.java

@@ -22,6 +22,7 @@
 import jadx.core.dex.visitors.typeinference.TypeInferenceVisitor;
 import jadx.core.utils.InstructionRemover;
 import jadx.core.utils.exceptions.JadxException;
+import jadx.core.utils.exceptions.JadxOverflowException;
 
 @JadxVisitor(
 		name = "Constants Inline",
@@ -40,21 +41,23 @@ public void visit(MethodNode mth) throws JadxException {
 		for (BlockNode block : mth.getBasicBlocks()) {
 			toRemove.clear();
 			for (InsnNode insn : block.getInstructions()) {
-				if (checkInsn(mth, insn)) {
-					toRemove.add(insn);
-				}
+				checkInsn(mth, insn, toRemove);
 			}
 			InstructionRemover.removeAll(mth, block, toRemove);
 		}
 	}
 
-	private static boolean checkInsn(MethodNode mth, InsnNode insn) {
-		if (insn.getType() != InsnType.CONST || insn.contains(AFlag.DONT_INLINE)) {
-			return false;
+	private static void checkInsn(MethodNode mth, InsnNode insn, List<InsnNode> toRemove) {
+		if (insn.contains(AFlag.DONT_INLINE)) {
+			return;
+		}
+		InsnType insnType = insn.getType();
+		if (insnType != InsnType.CONST && insnType != InsnType.MOVE) {
+			return;
 		}
 		InsnArg arg = insn.getArg(0);
 		if (!arg.isLiteral()) {
-			return false;
+			return;
 		}
 		long lit = ((LiteralArg) arg).getLiteral();
 
@@ -66,9 +69,9 @@ private static boolean checkInsn(MethodNode mth, InsnNode insn) {
 					assignInsn.add(AFlag.DONT_INLINE);
 				}
 			}
-			return false;
+			return;
 		}
-		return replaceConst(mth, insn, lit);
+		replaceConst(mth, insn, lit, toRemove);
 	}
 
 	/**
@@ -98,54 +101,60 @@ private static boolean checkObjectInline(SSAVar sVar) {
 		return false;
 	}
 
-	private static boolean replaceConst(MethodNode mth, InsnNode constInsn, long literal) {
-		SSAVar sVar = constInsn.getResult().getSVar();
-		List<RegisterArg> use = new ArrayList<>(sVar.getUseList());
+	private static void replaceConst(MethodNode mth, InsnNode constInsn, long literal, List<InsnNode> toRemove) {
+		SSAVar ssaVar = constInsn.getResult().getSVar();
+		List<RegisterArg> useList = new ArrayList<>(ssaVar.getUseList());
 		int replaceCount = 0;
-		for (RegisterArg arg : use) {
-			InsnNode useInsn = arg.getParentInsn();
-			if (useInsn == null
-					|| useInsn.getType() == InsnType.PHI
-					|| useInsn.getType() == InsnType.MERGE
-			) {
-				continue;
-			}
-			LiteralArg litArg;
-			ArgType argType = arg.getType();
-			if (argType.isObject() && literal != 0) {
-				argType = ArgType.NARROW_NUMBERS;
-			}
-			if (use.size() == 1 || arg.isTypeImmutable()) {
-				// arg used only in one place
-				litArg = InsnArg.lit(literal, argType);
-			} else if (useInsn.getType() == InsnType.MOVE
-					&& !useInsn.getResult().getType().isTypeKnown()) {
-				// save type for 'move' instructions (hard to find type in chains of 'move')
-				litArg = InsnArg.lit(literal, argType);
-			} else {
-				// in most cases type not equal arg.getType()
-				// just set unknown type and run type fixer
-				litArg = InsnArg.lit(literal, ArgType.UNKNOWN);
-			}
-			if (useInsn.replaceArg(arg, litArg)) {
-				litArg.setType(arg.getInitType());
+		for (RegisterArg arg : useList) {
+			if (replaceArg(mth, arg, literal, constInsn, toRemove)) {
 				replaceCount++;
-				if (useInsn.getType() == InsnType.RETURN) {
-					useInsn.setSourceLine(constInsn.getSourceLine());
-				}
+			}
+		}
+		if (replaceCount == useList.size()) {
+			toRemove.add(constInsn);
+		}
+	}
 
-				FieldNode f = null;
-				ArgType litArgType = litArg.getType();
-				if (litArgType.isTypeKnown()) {
-					f = mth.getParentClass().getConstFieldByLiteralArg(litArg);
-				} else if (litArgType.contains(PrimitiveType.INT)) {
-					f = mth.getParentClass().getConstField((int) literal, false);
-				}
-				if (f != null) {
-					litArg.wrapInstruction(new IndexInsnNode(InsnType.SGET, f.getFieldInfo(), 0));
-				}
+	private static boolean replaceArg(MethodNode mth, RegisterArg arg, long literal, InsnNode constInsn, List<InsnNode> toRemove) {
+		InsnNode useInsn = arg.getParentInsn();
+		if (useInsn == null) {
+			return false;
+		}
+		InsnType insnType = useInsn.getType();
+		if (insnType == InsnType.PHI || insnType == InsnType.MERGE) {
+			return false;
+		}
+		ArgType argType = arg.getInitType();
+		if (argType.isObject() && literal != 0) {
+			argType = ArgType.NARROW_NUMBERS;
+		}
+		LiteralArg litArg = InsnArg.lit(literal, argType);
+		if (!useInsn.replaceArg(arg, litArg)) {
+			return false;
+		}
+		// arg replaced, made some optimizations
+		litArg.setType(arg.getInitType());
+
+		FieldNode fieldNode = null;
+		ArgType litArgType = litArg.getType();
+		if (litArgType.isTypeKnown()) {
+			fieldNode = mth.getParentClass().getConstFieldByLiteralArg(litArg);
+		} else if (litArgType.contains(PrimitiveType.INT)) {
+			fieldNode = mth.getParentClass().getConstField((int) literal, false);
+		}
+		if (fieldNode != null) {
+			litArg.wrapInstruction(new IndexInsnNode(InsnType.SGET, fieldNode.getFieldInfo(), 0));
+		}
+
+		if (insnType == InsnType.RETURN) {
+			useInsn.setSourceLine(constInsn.getSourceLine());
+		} else if (insnType == InsnType.MOVE) {
+			try {
+				replaceConst(mth, useInsn, literal, toRemove);
+			} catch (StackOverflowError e) {
+				throw new JadxOverflowException("Stack overflow at const inline visitor");
 			}
 		}
-		return replaceCount == use.size();
+		return true;
 	}
 }

jadx-core/src/test/java/jadx/tests/api/SmaliTest.java

@@ -24,6 +24,10 @@ protected ClassNode getClassNodeFromSmaliWithPath(String path, String clsName) {
 		return getClassNodeFromSmali(path + File.separatorChar + clsName, clsName);
 	}
 
+	protected ClassNode getClassNodeFromSmaliWithPkg(String pkg, String clsName) {
+		return getClassNodeFromSmali(pkg + File.separatorChar + clsName, pkg + '.' + clsName);
+	}
+
 	protected ClassNode getClassNodeFromSmali(String clsName) {
 		return getClassNodeFromSmali(clsName, clsName);
 	}

jadx-core/src/test/java/jadx/tests/integration/types/TestConstInline.java

@@ -0,0 +1,38 @@
+package jadx.tests.integration.types;
+
+import org.junit.Test;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.SmaliTest;
+
+import static jadx.tests.api.utils.JadxMatchers.containsOne;
+import static org.junit.Assert.assertThat;
+
+public class TestConstInline extends SmaliTest {
+
+//		private static String test(boolean b) {
+//			List<String> list;
+//			String str;
+//			if (b) {
+//				list = Collections.emptyList();
+//				str = "1";
+//			} else {
+//				list = null;
+//				str = list; // not correct assign in java but bytecode allow it
+//			}
+//			return use(list, str);
+//		}
+//
+//		private static String use(List<String> list, String str) {
+//			return list + str;
+//		}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNodeFromSmaliWithPkg("types", "TestConstInline");
+		String code = cls.getCode().toString();
+
+		assertThat(code, containsOne("list = null;"));
+		assertThat(code, containsOne("str = null;"));
+	}
+}

jadx-core/src/test/smali/types/TestConstInline.smali

@@ -0,0 +1,72 @@
+.class public Ltypes/TestConstInline;
+.super Ljava/lang/Object;
+
+.method private static test(Z)Ljava/lang/String;
+    .registers 4
+    .param p0, "b"    # Z
+
+    if-eqz p0, :cond_d
+    invoke-static {}, Ltypes/TestConstInline;->list()Ljava/util/List;
+    move-result-object v0
+    const-string v1, "1"
+    goto :goto_return
+
+    :cond_d
+    const/4 v2, 0x0
+    # chained move instead zero const loading
+    move v0, v2
+    move v1, v0
+    goto :goto_return
+
+    :goto_return
+    invoke-static {v0, v1}, Ltypes/TestConstInline;->use(Ljava/util/List;Ljava/lang/String;)Ljava/lang/String;
+    move-result-object v2
+    return-object v2
+.end method
+
+.method private static use(Ljava/util/List;Ljava/lang/String;)Ljava/lang/String;
+    .registers 3
+    .annotation system Ldalvik/annotation/Signature;
+        value = {
+            "(",
+            "Ljava/util/List",
+            "<",
+            "Ljava/lang/String;",
+            ">;",
+            "Ljava/lang/String;",
+            ")",
+            "Ljava/lang/String;"
+        }
+    .end annotation
+
+    new-instance v0, Ljava/lang/StringBuilder;
+    invoke-direct {v0}, Ljava/lang/StringBuilder;-><init>()V
+
+    invoke-virtual {v0, p0}, Ljava/lang/StringBuilder;->append(Ljava/lang/Object;)Ljava/lang/StringBuilder;
+    move-result-object v0
+
+    invoke-virtual {v0, p1}, Ljava/lang/StringBuilder;->append(Ljava/lang/String;)Ljava/lang/StringBuilder;
+    move-result-object v0
+
+    invoke-virtual {v0}, Ljava/lang/StringBuilder;->toString()Ljava/lang/String;
+    move-result-object v0
+
+    return-object v0
+.end method
+
+.method private static list()Ljava/util/List;
+    .registers 1
+    .annotation system Ldalvik/annotation/Signature;
+        value = {
+            "()",
+            "Ljava/util/List",
+            "<",
+            "Ljava/lang/String;",
+            ">;"
+        }
+    .end annotation
+
+    invoke-static {}, Ljava/util/Collections;->emptyList()Ljava/util/List;
+    move-result-object v0
+    return-object v0
+.end method