fix: unbind unused ssa variable after ternary conversion (#708)

jadx-core/src/main/java/jadx/core/dex/visitors/regions/TernaryMod.java

@@ -99,6 +99,7 @@ public class TernaryMod implements IRegionIterativeVisitor {
 			RegisterArg resArg;
 			if (thenPhi.getArgsCount() == 2) {
 				resArg = thenPhi.getResult();
+				InsnRemover.unbindResult(mth, thenInsn);
 			} else {
 				resArg = thenResArg;
 				thenPhi.removeArg(elseResArg);
@@ -107,6 +108,8 @@ public class TernaryMod implements IRegionIterativeVisitor {
 					resArg, InsnArg.wrapArg(thenInsn), InsnArg.wrapArg(elseInsn));
 			ternInsn.setSourceLine(thenInsn.getSourceLine());
 
+			InsnRemover.unbindResult(mth, elseInsn);
+
 			// remove 'if' instruction
 			header.getInstructions().clear();
 			header.getInstructions().add(ternInsn);

jadx-core/src/main/java/jadx/core/dex/visitors/regions/variables/ProcessVariables.java

@@ -14,6 +14,7 @@ import org.slf4j.LoggerFactory;
 import jadx.core.dex.attributes.AFlag;
 import jadx.core.dex.attributes.AType;
 import jadx.core.dex.attributes.nodes.DeclareVariablesAttr;
+import jadx.core.dex.instructions.InsnType;
 import jadx.core.dex.instructions.args.ArgType;
 import jadx.core.dex.instructions.args.CodeVar;
 import jadx.core.dex.instructions.args.RegisterArg;
@@ -228,7 +229,9 @@ public class ProcessVariables extends AbstractVisitor {
 	private static boolean checkDeclareAtAssign(SSAVar var) {
 		RegisterArg arg = var.getAssign();
 		InsnNode parentInsn = arg.getParentInsn();
-		if (parentInsn == null) {
+		if (parentInsn == null
+				|| parentInsn.contains(AFlag.WRAPPED)
+				|| parentInsn.getType() == InsnType.PHI) {
 			return false;
 		}
 		if (!arg.equals(parentInsn.getResult())) {

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

@@ -20,6 +20,7 @@ import jadx.core.dex.visitors.JadxVisitor;
 import jadx.core.dex.visitors.ModVisitor;
 import jadx.core.utils.BlockUtils;
 import jadx.core.utils.InsnList;
+import jadx.core.utils.InsnRemover;
 import jadx.core.utils.exceptions.JadxRuntimeException;
 
 @JadxVisitor(
@@ -67,7 +68,7 @@ public class CodeShrinkVisitor extends AbstractVisitor {
 		}
 		if (!wrapList.isEmpty()) {
 			for (WrapInfo wrapInfo : wrapList) {
-				inline(wrapInfo.getArg(), wrapInfo.getInsn(), block);
+				inline(mth, wrapInfo.getArg(), wrapInfo.getInsn(), block);
 			}
 		}
 	}
@@ -106,12 +107,12 @@ public class CodeShrinkVisitor extends AbstractVisitor {
 			if (assignBlock != null
 					&& assignInsn != arg.getParentInsn()
 					&& canMoveBetweenBlocks(assignInsn, assignBlock, block, argsInfo.getInsn())) {
-				inline(arg, assignInsn, assignBlock);
+				inline(mth, arg, assignInsn, assignBlock);
 			}
 		}
 	}
 
-	private static boolean inline(RegisterArg arg, InsnNode insn, BlockNode block) {
+	private static boolean inline(MethodNode mth, RegisterArg arg, InsnNode insn, BlockNode block) {
 		InsnNode parentInsn = arg.getParentInsn();
 		if (parentInsn != null && parentInsn.getType() == InsnType.RETURN) {
 			parentInsn.setSourceLine(insn.getSourceLine());
@@ -119,6 +120,7 @@ public class CodeShrinkVisitor extends AbstractVisitor {
 		boolean replaced = arg.wrapInstruction(insn) != null;
 		if (replaced) {
 			InsnList.remove(block, insn);
+			InsnRemover.unbindResult(mth, insn);
 		}
 		return replaced;
 	}

jadx-core/src/main/java/jadx/core/utils/InsnRemover.java

@@ -8,6 +8,7 @@ import org.jetbrains.annotations.Nullable;
 
 import jadx.core.dex.attributes.AFlag;
 import jadx.core.dex.instructions.InsnType;
+import jadx.core.dex.instructions.PhiInsn;
 import jadx.core.dex.instructions.args.InsnArg;
 import jadx.core.dex.instructions.args.InsnWrapArg;
 import jadx.core.dex.instructions.args.RegisterArg;
@@ -66,7 +67,7 @@ public class InsnRemover {
 		toRemove.clear();
 	}
 
-	public static void unbindInsn(MethodNode mth, InsnNode insn) {
+	public static void unbindInsn(@Nullable MethodNode mth, InsnNode insn) {
 		for (InsnArg arg : insn.getArguments()) {
 			unbindArgUsage(mth, arg);
 		}
@@ -81,17 +82,41 @@ public class InsnRemover {
 		insn.add(AFlag.REMOVE);
 	}
 
-	public static void unbindResult(MethodNode mth, InsnNode insn) {
+	public static void unbindResult(@Nullable MethodNode mth, InsnNode insn) {
 		RegisterArg r = insn.getResult();
 		if (r != null && r.getSVar() != null && mth != null) {
 			SSAVar ssaVar = r.getSVar();
-			if (ssaVar.getUseCount() == 0) {
-				mth.removeSVar(ssaVar);
+			removeSsaVar(mth, ssaVar);
+		}
+	}
+
+	private static void removeSsaVar(MethodNode mth, SSAVar ssaVar) {
+		int useCount = ssaVar.getUseCount();
+		if (useCount == 0) {
+			mth.removeSVar(ssaVar);
+			return;
+		}
+		// check if all usage only in PHI insns
+		boolean allPhis = true;
+		for (RegisterArg arg : ssaVar.getUseList()) {
+			InsnNode parentInsn = arg.getParentInsn();
+			if (parentInsn == null || parentInsn.getType() != InsnType.PHI) {
+				allPhis = false;
+				break;
+			}
+		}
+		if (allPhis) {
+			for (RegisterArg arg : new ArrayList<>(ssaVar.getUseList())) {
+				InsnNode parentInsn = arg.getParentInsn();
+				if (parentInsn != null) {
+					((PhiInsn) parentInsn).removeArg(arg);
+				}
 			}
+			mth.removeSVar(ssaVar);
 		}
 	}
 
-	public static void unbindArgUsage(MethodNode mth, InsnArg arg) {
+	public static void unbindArgUsage(@Nullable MethodNode mth, InsnArg arg) {
 		if (arg instanceof RegisterArg) {
 			RegisterArg reg = (RegisterArg) arg;
 			SSAVar sVar = reg.getSVar();

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

@@ -17,7 +17,7 @@ public class TestConditions14 extends IntegrationTest {
 			if (r) {
 				return false;
 			}
-			System.out.println("1");
+			System.out.println("r=" + r);
 			return true;
 		}
 	}
@@ -29,6 +29,6 @@ public class TestConditions14 extends IntegrationTest {
 
 		assertThat(code, containsOne("boolean r = a == null ? b != null : !a.equals(b);"));
 		assertThat(code, containsOne("if (r) {"));
-		assertThat(code, containsOne("System.out.println(\"1\");"));
+		assertThat(code, containsOne("System.out.println(\"r=\" + r);"));
 	}
 }

jadx-core/src/test/java/jadx/tests/integration/conditions/TestTernary4.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 org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public class TestTernary4 extends SmaliTest {
+
+	// @formatter:off
+	/*
+		private Set test(HashMap<String, Object> hashMap) {
+			boolean z;
+			HashSet hashSet = new HashSet();
+			synchronized (this.defaultValuesByPath) {
+				for (String next : this.defaultValuesByPath.keySet()) {
+					Object obj = hashMap.get(next);
+					if (obj != null) {
+						z = !getValueObject(next).equals(obj);
+					} else {
+						z = this.valuesByPath.get(next) != null;;
+					}
+					if (z) {
+						hashSet.add(next);
+					}
+				}
+			}
+			return hashSet;
+		}
+	*/
+	// @formatter:on
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNodeFromSmali();
+		String code = cls.getCode().toString();
+
+		assertThat(code, not(containsString("r5")));
+		assertThat(code, not(containsString("try")));
+	}
+}

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

@@ -2,7 +2,6 @@ package jadx.tests.integration.types;
 
 import org.junit.jupiter.api.Test;
 
-import jadx.NotYetImplemented;
 import jadx.core.dex.nodes.ClassNode;
 import jadx.tests.api.IntegrationTest;
 
@@ -27,15 +26,6 @@ public class TestTypeResolver3 extends IntegrationTest {
 		ClassNode cls = getClassNode(TestCls.class);
 		String code = cls.getCode().toString();
 
-		assertThat(code, containsOne("s1.length() == s2.length() ? 0 : s1.length() < s2.length() ? -1 : 1;"));
-	}
-
-	@Test
-	@NotYetImplemented
-	public void test3() {
-		ClassNode cls = getClassNode(TestCls.class);
-		String code = cls.getCode().toString();
-
 		assertThat(code, containsOne("return s1.length() == s2.length() ? 0 : s1.length() < s2.length() ? -1 : 1;"));
 	}
 

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

+.class public final Lconditions/TestTernary4;
+.super Ljava/lang/Object;
+
+.field private defaultValuesByPath:Ljava/util/Map;
+    .annotation system Ldalvik/annotation/Signature;
+        value = {
+            "Ljava/util/Map<",
+            "Ljava/lang/String;",
+            "Ljava/lang/Object;",
+            ">;"
+        }
+    .end annotation
+.end field
+
+.field private valuesByPath:Ljava/util/Map;
+    .annotation system Ldalvik/annotation/Signature;
+        value = {
+            "Ljava/util/Map<",
+            "Ljava/lang/String;",
+            "Ljava/lang/Object;",
+            ">;"
+        }
+    .end annotation
+.end field
+
+.method private test(Ljava/util/HashMap;)Ljava/util/Set;
+    .registers 10
+    .annotation system Ldalvik/annotation/Signature;
+        value = {
+            "(",
+            "Ljava/util/HashMap<",
+            "Ljava/lang/String;",
+            "Ljava/lang/Object;",
+            ">;)",
+            "Ljava/util/Set;"
+        }
+    .end annotation
+
+    .line 278
+    new-instance v0, Ljava/util/HashSet;
+
+    invoke-direct {v0}, Ljava/util/HashSet;-><init>()V
+
+    .line 280
+    iget-object v1, p0, Lconditions/TestTernary4;->defaultValuesByPath:Ljava/util/Map;
+
+    monitor-enter v1
+
+    .line 281
+    :try_start_14
+    iget-object v3, p0, Lconditions/TestTernary4;->defaultValuesByPath:Ljava/util/Map;
+
+    invoke-interface {v3}, Ljava/util/Map;->keySet()Ljava/util/Set;
+
+    move-result-object v3
+
+    invoke-interface {v3}, Ljava/util/Set;->iterator()Ljava/util/Iterator;
+
+    move-result-object v3
+
+    :cond_1e
+    :goto_1e
+    invoke-interface {v3}, Ljava/util/Iterator;->hasNext()Z
+
+    move-result v4
+
+    if-eqz v4, :cond_4c
+
+    invoke-interface {v3}, Ljava/util/Iterator;->next()Ljava/lang/Object;
+
+    move-result-object v4
+
+    check-cast v4, Ljava/lang/String;
+
+    .line 286
+    invoke-virtual {p1, v4}, Ljava/util/HashMap;->get(Ljava/lang/Object;)Ljava/lang/Object;
+
+    move-result-object v5
+
+    const/4 v6, 0x1
+
+    if-eqz v5, :cond_3b
+
+    .line 287
+    invoke-direct {p0, v4}, Lconditions/TestTernary4;->getValueObject(Ljava/lang/String;)Ljava/lang/Object;
+
+    move-result-object v7
+
+    invoke-virtual {v7, v5}, Ljava/lang/Object;->equals(Ljava/lang/Object;)Z
+
+    move-result v5
+
+    xor-int/2addr v5, v6
+
+    goto :goto_46
+
+    .line 289
+    :cond_3b
+    iget-object v5, p0, Lconditions/TestTernary4;->valuesByPath:Ljava/util/Map;
+
+    invoke-interface {v5, v4}, Ljava/util/Map;->get(Ljava/lang/Object;)Ljava/lang/Object;
+
+    move-result-object v5
+
+    if-eqz v5, :cond_45
+
+    const/4 v5, 0x1
+
+    goto :goto_46
+
+    :cond_45
+    const/4 v5, 0x0
+
+    :goto_46
+    if-eqz v5, :cond_1e
+
+    .line 293
+    invoke-interface {v0, v4}, Ljava/util/Set;->add(Ljava/lang/Object;)Z
+
+    goto :goto_1e
+
+    .line 296
+    :cond_4c
+    monitor-exit v1
+
+    return-object v0
+
+    :catchall_4e
+    move-exception p1
+
+    monitor-exit v1
+    :try_end_50
+    .catchall {:try_start_14 .. :try_end_50} :catchall_4e
+
+    throw p1
+
+    return-void
+.end method
+
+.method private getValueObject(Ljava/lang/String;)Ljava/lang/Object;
+    .locals 1
+    const/4 v0, 0x0
+    return-object v0
+.end method