fix: inline constant strings (#685)

jadx-core/src/main/java/jadx/core/dex/instructions/args/InsnArg.java

@@ -159,6 +159,12 @@ public abstract class InsnArg extends Typed {
 		return contains(AFlag.THIS);
 	}
 
+	protected InsnArg copyCommonParams(InsnArg copy) {
+		copy.copyAttributesFrom(this);
+		copy.setParentInsn(parentInsn);
+		return copy;
+	}
+
 	public InsnArg duplicate() {
 		return this;
 	}

jadx-core/src/main/java/jadx/core/dex/instructions/args/InsnWrapArg.java

@@ -27,6 +27,11 @@ public final class InsnWrapArg extends InsnArg {
 		this.parentInsn = parentInsn;
 	}
 
+	@Override
+	public InsnArg duplicate() {
+		return copyCommonParams(new InsnWrapArg(wrappedInsn.copy()));
+	}
+
 	@Override
 	public boolean isInsnWrap() {
 		return true;

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

@@ -4,6 +4,7 @@ import java.util.ArrayList;
 import java.util.List;
 
 import jadx.core.dex.attributes.AFlag;
+import jadx.core.dex.instructions.ConstStringNode;
 import jadx.core.dex.instructions.IndexInsnNode;
 import jadx.core.dex.instructions.InsnType;
 import jadx.core.dex.instructions.InvokeNode;
@@ -51,36 +52,53 @@ public class ConstInlineVisitor extends AbstractVisitor {
 	}
 
 	private static void checkInsn(MethodNode mth, InsnNode insn, List<InsnNode> toRemove) {
-		if (insn.contains(AFlag.DONT_INLINE) || insn.contains(AFlag.DONT_GENERATE)) {
+		if (insn.contains(AFlag.DONT_INLINE)
+				|| insn.contains(AFlag.DONT_GENERATE)
+				|| insn.getResult() == null) {
 			return;
 		}
-		InsnType insnType = insn.getType();
-		if (insnType != InsnType.CONST && insnType != InsnType.MOVE) {
-			return;
-		}
-		InsnArg arg = insn.getArg(0);
-		if (!arg.isLiteral()) {
-			return;
-		}
-		long lit = ((LiteralArg) arg).getLiteral();
 
 		SSAVar sVar = insn.getResult().getSVar();
-		if (lit == 0 && checkObjectInline(sVar)) {
-			if (sVar.getUseCount() == 1) {
-				InsnNode assignInsn = insn.getResult().getAssignInsn();
-				if (assignInsn != null) {
-					assignInsn.add(AFlag.DONT_INLINE);
+		InsnArg constArg;
+
+		InsnType insnType = insn.getType();
+		if (insnType == InsnType.CONST || insnType == InsnType.MOVE) {
+			constArg = insn.getArg(0);
+			if (!constArg.isLiteral()) {
+				return;
+			}
+			long lit = ((LiteralArg) constArg).getLiteral();
+			if (lit == 0 && checkObjectInline(sVar)) {
+				if (sVar.getUseCount() == 1) {
+					InsnNode assignInsn = insn.getResult().getAssignInsn();
+					if (assignInsn != null) {
+						assignInsn.add(AFlag.DONT_INLINE);
+					}
 				}
+				return;
 			}
+		} else if (insnType == InsnType.CONST_STR) {
+			if (sVar.isUsedInPhi()) {
+				return;
+			}
+			String s = ((ConstStringNode) insn).getString();
+			FieldNode f = mth.getParentClass().getConstField(s);
+			if (f == null) {
+				constArg = InsnArg.wrapArg(insn.copy());
+			} else {
+				InsnNode constGet = new IndexInsnNode(InsnType.SGET, f.getFieldInfo(), 0);
+				constGet.setResult(insn.getResult().duplicate());
+				constArg = InsnArg.wrapArg(constGet);
+			}
+		} else {
 			return;
 		}
-
 		if (checkForFinallyBlock(sVar)) {
 			return;
 		}
 
 		// all check passed, run replace
-		replaceConst(mth, insn, lit, toRemove);
+		replaceConst(mth, insn, constArg, toRemove);
 	}
 
 	private static boolean checkForFinallyBlock(SSAVar sVar) {
@@ -131,12 +149,12 @@ public class ConstInlineVisitor extends AbstractVisitor {
 		return false;
 	}
 
-	private static int replaceConst(MethodNode mth, InsnNode constInsn, long literal, List<InsnNode> toRemove) {
+	private static int replaceConst(MethodNode mth, InsnNode constInsn, InsnArg constArg, List<InsnNode> toRemove) {
 		SSAVar ssaVar = constInsn.getResult().getSVar();
 		List<RegisterArg> useList = new ArrayList<>(ssaVar.getUseList());
 		int replaceCount = 0;
 		for (RegisterArg arg : useList) {
-			if (replaceArg(mth, arg, literal, constInsn, toRemove)) {
+			if (replaceArg(mth, arg, constArg, constInsn, toRemove)) {
 				replaceCount++;
 			}
 		}
@@ -146,7 +164,7 @@ public class ConstInlineVisitor extends AbstractVisitor {
 		return replaceCount;
 	}
 
-	private static boolean replaceArg(MethodNode mth, RegisterArg arg, long literal, InsnNode constInsn, List<InsnNode> toRemove) {
+	private static boolean replaceArg(MethodNode mth, RegisterArg arg, InsnArg constArg, InsnNode constInsn, List<InsnNode> toRemove) {
 		InsnNode useInsn = arg.getParentInsn();
 		if (useInsn == null) {
 			return false;
@@ -155,33 +173,40 @@ public class ConstInlineVisitor extends AbstractVisitor {
 		if (insnType == InsnType.PHI) {
 			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 (constArg.isLiteral()) {
+			long literal = ((LiteralArg) constArg).getLiteral();
+			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));
+			}
+		} else {
+			if (!useInsn.replaceArg(arg, constArg.duplicate())) {
+				return false;
+			}
+		}
 		if (insnType == InsnType.RETURN) {
 			useInsn.setSourceLine(constInsn.getSourceLine());
 		} else if (insnType == InsnType.MOVE) {
 			try {
-				replaceConst(mth, useInsn, literal, toRemove);
+				replaceConst(mth, useInsn, constArg, toRemove);
 			} catch (StackOverflowError e) {
 				throw new JadxOverflowException("Stack overflow at const inline visitor");
 			}

jadx-core/src/main/java/jadx/core/dex/visitors/shrink/CodeShrinkVisitor.java

@@ -57,48 +57,11 @@ public class CodeShrinkVisitor extends AbstractVisitor {
 		List<WrapInfo> wrapList = new ArrayList<>();
 		for (ArgsInfo argsInfo : argsList) {
 			List<RegisterArg> args = argsInfo.getArgs();
-			if (args.isEmpty()) {
-				continue;
-			}
-			ListIterator<RegisterArg> it = args.listIterator(args.size());
-			while (it.hasPrevious()) {
-				RegisterArg arg = it.previous();
-				// if (arg.getName() != null) {
-				// continue;
-				// }
-				SSAVar sVar = arg.getSVar();
-				// allow inline only one use arg
-				if (sVar == null
-						|| sVar.getVariableUseCount() != 1
-						|| sVar.contains(AFlag.DONT_INLINE)) {
-					continue;
-				}
-				InsnNode assignInsn = sVar.getAssign().getParentInsn();
-				if (assignInsn == null || assignInsn.contains(AFlag.DONT_INLINE)) {
-					continue;
-				}
-				List<RegisterArg> useList = sVar.getUseList();
-				if (!useList.isEmpty()) {
-					InsnNode parentInsn = useList.get(0).getParentInsn();
-					if (parentInsn != null && parentInsn.contains(AFlag.DONT_GENERATE)) {
-						continue;
-					}
-				}
-
-				int assignPos = insnList.getIndex(assignInsn);
-				if (assignPos != -1) {
-					WrapInfo wrapInfo = argsInfo.checkInline(assignPos, arg);
-					if (wrapInfo != null) {
-						wrapList.add(wrapInfo);
-					}
-				} else {
-					// another block
-					BlockNode assignBlock = BlockUtils.getBlockByInsn(mth, assignInsn);
-					if (assignBlock != null
-							&& assignInsn != arg.getParentInsn()
-							&& canMoveBetweenBlocks(assignInsn, assignBlock, block, argsInfo.getInsn())) {
-						inline(arg, assignInsn, assignBlock);
-					}
+			if (!args.isEmpty()) {
+				ListIterator<RegisterArg> it = args.listIterator(args.size());
+				while (it.hasPrevious()) {
+					RegisterArg arg = it.previous();
+					checkInline(mth, block, insnList, wrapList, argsInfo, arg);
 				}
 			}
 		}
@@ -109,6 +72,45 @@ public class CodeShrinkVisitor extends AbstractVisitor {
 		}
 	}
 
+	private static void checkInline(MethodNode mth, BlockNode block, InsnList insnList,
+			List<WrapInfo> wrapList, ArgsInfo argsInfo, RegisterArg arg) {
+		SSAVar sVar = arg.getSVar();
+		if (sVar == null || sVar.contains(AFlag.DONT_INLINE)) {
+			return;
+		}
+		// allow inline only one use arg
+		if (sVar.getVariableUseCount() != 1) {
+			return;
+		}
+		InsnNode assignInsn = sVar.getAssign().getParentInsn();
+		if (assignInsn == null || assignInsn.contains(AFlag.DONT_INLINE)) {
+			return;
+		}
+		List<RegisterArg> useList = sVar.getUseList();
+		if (!useList.isEmpty()) {
+			InsnNode parentInsn = useList.get(0).getParentInsn();
+			if (parentInsn != null && parentInsn.contains(AFlag.DONT_GENERATE)) {
+				return;
+			}
+		}
+
+		int assignPos = insnList.getIndex(assignInsn);
+		if (assignPos != -1) {
+			WrapInfo wrapInfo = argsInfo.checkInline(assignPos, arg);
+			if (wrapInfo != null) {
+				wrapList.add(wrapInfo);
+			}
+		} else {
+			// another block
+			BlockNode assignBlock = BlockUtils.getBlockByInsn(mth, assignInsn);
+			if (assignBlock != null
+					&& assignInsn != arg.getParentInsn()
+					&& canMoveBetweenBlocks(assignInsn, assignBlock, block, argsInfo.getInsn())) {
+				inline(arg, assignInsn, assignBlock);
+			}
+		}
+	}
+
 	private static boolean inline(RegisterArg arg, InsnNode insn, BlockNode block) {
 		InsnNode parentInsn = arg.getParentInsn();
 		if (parentInsn != null && parentInsn.getType() == InsnType.RETURN) {

jadx-core/src/test/java/jadx/tests/integration/conditions/TestTernaryOneBranchInConstructor2.java

+package jadx.tests.integration.conditions;
+
+import org.junit.jupiter.api.Test;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.SmaliTest;
+
+import static jadx.tests.api.utils.JadxMatchers.containsOne;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public class TestTernaryOneBranchInConstructor2 extends SmaliTest {
+
+	// @formatter:off
+	/*
+		public class A {
+			public A(String str, String str2, String str3, boolean z) {}
+
+			public A(String str, String str2, String str3, boolean z, int i, int i2) {
+				this(str, (i & 2) != 0 ? "" : str2, (i & 4) != 0 ? "" : str3, (i & 8) != 0 ? false : z);
+			}
+		}
+	*/
+	// @formatter:on
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNodeFromSmali();
+		String code = cls.getCode().toString();
+
+		assertThat(code, containsOne("this(str, (i & 2) != 0 ? \"\" : str2,"
+				+ " (i & 4) != 0 ? \"\" : str3,"
+				+ " (i & 8) != 0 ? false : z);"));
+		assertThat(code, not(containsString("//")));
+	}
+}

jadx-core/src/test/smali/conditions/TestTernaryOneBranchInConstructor2.smali

+.class public final Lconditions/TestTernaryOneBranchInConstructor2;
+.super Ljava/lang/Object;
+
+
+.method public constructor <init>(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V
+    .locals 1
+
+    invoke-direct {p0}, Ljava/lang/Object;-><init>()V
+
+    return-void
+.end method
+
+.method public synthetic constructor <init>(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ZII)V
+    .locals 1
+
+    and-int/lit8 p6, p5, 0x2
+
+    const-string v0, ""
+
+    if-eqz p6, :cond_0
+
+    move-object p2, v0
+
+    :cond_0
+    and-int/lit8 p6, p5, 0x4
+
+    if-eqz p6, :cond_1
+
+    move-object p3, v0
+
+    :cond_1
+    and-int/lit8 p5, p5, 0x8
+
+    if-eqz p5, :cond_2
+
+    const/4 p4, 0x0
+
+    :cond_2
+    invoke-direct {p0, p1, p2, p3, p4}, Lconditions/TestTernaryOneBranchInConstructor2;-><init>(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V
+
+    return-void
+.end method