fix: handle cases with SSA variable used in several PHI's (#667)

jadx-core/src/main/java/jadx/core/dex/instructions/PhiInsn.java

@@ -12,7 +12,6 @@ import jadx.core.dex.instructions.args.InsnArg;
 import jadx.core.dex.instructions.args.RegisterArg;
 import jadx.core.dex.nodes.BlockNode;
 import jadx.core.dex.nodes.InsnNode;
-import jadx.core.utils.InsnRemover;
 import jadx.core.utils.Utils;
 import jadx.core.utils.exceptions.JadxRuntimeException;
 
@@ -76,7 +75,7 @@ public final class PhiInsn extends InsnNode {
 	protected RegisterArg removeArg(int index) {
 		RegisterArg reg = (RegisterArg) super.removeArg(index);
 		blockBinds.remove(index);
-		InsnRemover.fixUsedInPhiFlag(reg);
+		reg.getSVar().updateUsedInPhiList();
 		return reg;
 	}
 
@@ -98,7 +97,7 @@ public final class PhiInsn extends InsnNode {
 
 		RegisterArg reg = (RegisterArg) to;
 		bindArg(reg, pred);
-		reg.getSVar().setUsedInPhi(this);
+		reg.getSVar().addUsedInPhi(this);
 		return true;
 	}
 

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

@@ -12,7 +12,9 @@ import org.jetbrains.annotations.Nullable;
 import jadx.core.dex.attributes.AType;
 import jadx.core.dex.attributes.AttrNode;
 import jadx.core.dex.attributes.nodes.RegDebugInfoAttr;
+import jadx.core.dex.instructions.InsnType;
 import jadx.core.dex.instructions.PhiInsn;
+import jadx.core.dex.nodes.InsnNode;
 import jadx.core.dex.nodes.MethodNode;
 import jadx.core.dex.visitors.typeinference.TypeInfo;
 import jadx.core.utils.StringUtils;
@@ -24,8 +26,7 @@ public class SSAVar extends AttrNode {
 
 	private RegisterArg assign;
 	private final List<RegisterArg> useList = new ArrayList<>(2);
-	@Nullable
-	private PhiInsn usedInPhi;
+	private List<PhiInsn> usedInPhi = null;
 
 	private TypeInfo typeInfo = new TypeInfo();
 
@@ -85,24 +86,60 @@ public class SSAVar extends AttrNode {
 		useList.removeIf(registerArg -> registerArg == arg);
 	}
 
-	public void setUsedInPhi(@Nullable PhiInsn usedInPhi) {
-		this.usedInPhi = usedInPhi;
+	public void addUsedInPhi(PhiInsn phiInsn) {
+		if (usedInPhi == null) {
+			usedInPhi = new ArrayList<>(1);
+		}
+		usedInPhi.add(phiInsn);
+	}
+
+	public void removeUsedInPhi(PhiInsn phiInsn) {
+		if (usedInPhi != null) {
+			usedInPhi.removeIf(insn -> insn == phiInsn);
+			if (usedInPhi.isEmpty()) {
+				usedInPhi = null;
+			}
+		}
+	}
+
+	public void updateUsedInPhiList() {
+		this.usedInPhi = null;
+		for (RegisterArg reg : useList) {
+			InsnNode parentInsn = reg.getParentInsn();
+			if (parentInsn != null && parentInsn.getType() == InsnType.PHI) {
+				addUsedInPhi((PhiInsn) parentInsn);
+			}
+		}
 	}
 
 	@Nullable
-	public PhiInsn getUsedInPhi() {
+	public PhiInsn getOnlyOneUseInPhi() {
+		if (usedInPhi != null && usedInPhi.size() == 1) {
+			return usedInPhi.get(0);
+		}
+		return null;
+	}
+
+	public List<PhiInsn> getUsedInPhi() {
+		if (usedInPhi == null) {
+			return Collections.emptyList();
+		}
 		return usedInPhi;
 	}
 
 	public boolean isUsedInPhi() {
-		return usedInPhi != null;
+		return usedInPhi != null && !usedInPhi.isEmpty();
 	}
 
 	public int getVariableUseCount() {
+		int count = useList.size();
 		if (usedInPhi == null) {
-			return useList.size();
+			return count;
+		}
+		for (PhiInsn phiInsn : usedInPhi) {
+			count += phiInsn.getResult().getSVar().getUseCount();
 		}
-		return useList.size() + usedInPhi.getResult().getSVar().getUseCount();
+		return count;
 	}
 
 	public void setName(String name) {

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

@@ -58,11 +58,11 @@ public class InitCodeVariables extends AbstractVisitor {
 	}
 
 	private static void setCodeVar(SSAVar ssaVar, CodeVar codeVar) {
-		PhiInsn usedInPhi = ssaVar.getUsedInPhi();
-		if (usedInPhi != null) {
+		List<PhiInsn> usedInPhiList = ssaVar.getUsedInPhi();
+		if (!usedInPhiList.isEmpty()) {
 			Set<SSAVar> vars = new LinkedHashSet<>();
 			vars.add(ssaVar);
-			collectConnectedVars(usedInPhi, vars);
+			collectConnectedVars(usedInPhiList, vars);
 			setCodeVarType(codeVar, vars);
 			vars.forEach(var -> {
 				if (var.isCodeVarSet()) {
@@ -92,19 +92,18 @@ public class InitCodeVariables extends AbstractVisitor {
 		}
 	}
 
-	private static void collectConnectedVars(PhiInsn phiInsn, Set<SSAVar> vars) {
-		if (phiInsn == null) {
-			return;
-		}
-		SSAVar resultVar = phiInsn.getResult().getSVar();
-		if (vars.add(resultVar)) {
-			collectConnectedVars(resultVar.getUsedInPhi(), vars);
-		}
-		phiInsn.getArguments().forEach(arg -> {
-			SSAVar sVar = ((RegisterArg) arg).getSVar();
-			if (vars.add(sVar)) {
-				collectConnectedVars(sVar.getUsedInPhi(), vars);
+	private static void collectConnectedVars(List<PhiInsn> phiInsnList, Set<SSAVar> vars) {
+		for (PhiInsn phiInsn : phiInsnList) {
+			SSAVar resultVar = phiInsn.getResult().getSVar();
+			if (vars.add(resultVar)) {
+				collectConnectedVars(resultVar.getUsedInPhi(), vars);
 			}
-		});
+			phiInsn.getArguments().forEach(arg -> {
+				SSAVar sVar = ((RegisterArg) arg).getSVar();
+				if (vars.add(sVar)) {
+					collectConnectedVars(sVar.getUsedInPhi(), vars);
+				}
+			});
+		}
 	}
 }

jadx-core/src/main/java/jadx/core/dex/visitors/debuginfo/DebugInfoApplyVisitor.java

@@ -203,8 +203,7 @@ public class DebugInfoApplyVisitor extends AbstractVisitor {
 
 	private static void fixNamesForPhiInsns(MethodNode mth) {
 		mth.getSVars().forEach(ssaVar -> {
-			PhiInsn phiInsn = ssaVar.getUsedInPhi();
-			if (phiInsn != null) {
+			for (PhiInsn phiInsn : ssaVar.getUsedInPhi()) {
 				Set<String> names = new HashSet<>(1 + phiInsn.getArgsCount());
 				addArgName(phiInsn.getResult(), names);
 				phiInsn.getArguments().forEach(arg -> addArgName(arg, names));

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

@@ -89,9 +89,12 @@ public class LoopRegionVisitor extends AbstractVisitor implements IRegionVisitor
 				|| !incrArg.getSVar().isUsedInPhi()) {
 			return false;
 		}
-		PhiInsn phiInsn = incrArg.getSVar().getUsedInPhi();
-		if (phiInsn == null
-				|| phiInsn.getArgsCount() != 2
+		List<PhiInsn> phiInsnList = incrArg.getSVar().getUsedInPhi();
+		if (phiInsnList.size() != 1) {
+			return false;
+		}
+		PhiInsn phiInsn = phiInsnList.get(0);
+		if (phiInsn.getArgsCount() != 2
 				|| !phiInsn.containsArg(incrArg)
 				|| incrArg.getSVar().getUseCount() != 1) {
 			return false;

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

@@ -62,8 +62,8 @@ public class TernaryMod {
 		RegisterArg thenResArg = thenInsn.getResult();
 		RegisterArg elseResArg = elseInsn.getResult();
 		if (thenResArg != null && elseResArg != null) {
-			PhiInsn thenPhi = thenResArg.getSVar().getUsedInPhi();
-			PhiInsn elsePhi = elseResArg.getSVar().getUsedInPhi();
+			PhiInsn thenPhi = thenResArg.getSVar().getOnlyOneUseInPhi();
+			PhiInsn elsePhi = elseResArg.getSVar().getOnlyOneUseInPhi();
 			if (thenPhi == null || thenPhi != elsePhi) {
 				return false;
 			}
@@ -165,8 +165,8 @@ public class TernaryMod {
 		if (t.getResult() == null || e.getResult() == null) {
 			return false;
 		}
-		PhiInsn tPhi = t.getResult().getSVar().getUsedInPhi();
-		PhiInsn ePhi = e.getResult().getSVar().getUsedInPhi();
+		PhiInsn tPhi = t.getResult().getSVar().getOnlyOneUseInPhi();
+		PhiInsn ePhi = e.getResult().getSVar().getOnlyOneUseInPhi();
 		if (ePhi == null || tPhi != ePhi) {
 			return false;
 		}

jadx-core/src/main/java/jadx/core/dex/visitors/ssa/SSATransform.java

@@ -187,7 +187,7 @@ public class SSATransform extends AbstractVisitor {
 		}
 		RegisterArg arg = phiInsn.bindArg(state.getBlock());
 		var.use(arg);
-		var.setUsedInPhi(phiInsn);
+		var.addUsedInPhi(phiInsn);
 	}
 
 	/**
@@ -323,7 +323,7 @@ public class SSATransform extends AbstractVisitor {
 						}
 						SSAVar sVar = ((RegisterArg) arg).getSVar();
 						if (sVar != null) {
-							sVar.setUsedInPhi(null);
+							sVar.removeUsedInPhi(phiInsn);
 						}
 					}
 					InsnRemover.remove(mth, block, phiInsn);
@@ -347,13 +347,13 @@ public class SSATransform extends AbstractVisitor {
 		SSAVar argVar = arg.getSVar();
 		if (argVar != null) {
 			argVar.removeUse(arg);
-			argVar.setUsedInPhi(null);
+			argVar.removeUsedInPhi(phi);
 		}
 		// try inline
 		if (inlinePhiInsn(mth, block, phi)) {
 			insns.remove(phiIndex);
 		} else {
-			assign.setUsedInPhi(null);
+			assign.removeUsedInPhi(phi);
 
 			InsnNode m = new InsnNode(InsnType.MOVE, 1);
 			m.add(AFlag.SYNTHETIC);

jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeInferenceVisitor.java

@@ -196,8 +196,7 @@ public final class TypeInferenceVisitor extends AbstractVisitor {
 	}
 
 	private void mergePhiBounds(SSAVar ssaVar) {
-		PhiInsn usedInPhi = ssaVar.getUsedInPhi();
-		if (usedInPhi != null) {
+		for (PhiInsn usedInPhi : ssaVar.getUsedInPhi()) {
 			Set<ITypeBound> bounds = ssaVar.getTypeInfo().getBounds();
 			bounds.addAll(usedInPhi.getResult().getSVar().getTypeInfo().getBounds());
 			for (InsnArg arg : usedInPhi.getArguments()) {
@@ -307,8 +306,8 @@ public final class TypeInferenceVisitor extends AbstractVisitor {
 		if (var.getTypeInfo().getType().isTypeKnown()) {
 			return false;
 		}
-		PhiInsn phiInsn = var.getUsedInPhi();
-		if (phiInsn == null) {
+		List<PhiInsn> usedInPhiList = var.getUsedInPhi();
+		if (usedInPhiList.isEmpty()) {
 			return false;
 		}
 		if (var.getUseCount() == 1) {
@@ -317,7 +316,15 @@ public final class TypeInferenceVisitor extends AbstractVisitor {
 				return false;
 			}
 		}
+		for (PhiInsn phiInsn : usedInPhiList) {
+			if (!insertMoveForPhi(mth, phiInsn, var)) {
+				return false;
+			}
+		}
+		return true;
+	}
 
+	private boolean insertMoveForPhi(MethodNode mth, PhiInsn phiInsn, SSAVar var) {
 		int argsCount = phiInsn.getArgsCount();
 		for (int argIndex = 0; argIndex < argsCount; argIndex++) {
 			RegisterArg reg = phiInsn.getArg(argIndex);

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

@@ -193,8 +193,7 @@ public class DebugUtils {
 			}
 		}
 		for (SSAVar ssaVar : mth.getSVars()) {
-			PhiInsn usedInPhi = ssaVar.getUsedInPhi();
-			if (usedInPhi != null) {
+			for (PhiInsn usedInPhi : ssaVar.getUsedInPhi()) {
 				boolean found = false;
 				for (RegisterArg useArg : ssaVar.getUseList()) {
 					InsnNode parentInsn = useArg.getParentInsn();

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

@@ -6,7 +6,6 @@ import java.util.List;
 
 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;
@@ -65,7 +64,7 @@ public class InsnRemover {
 		if (insn.getType() == InsnType.PHI) {
 			for (InsnArg arg : insn.getArguments()) {
 				if (arg instanceof RegisterArg) {
-					fixUsedInPhiFlag((RegisterArg) arg);
+					((RegisterArg) arg).getSVar().updateUsedInPhiList();
 				}
 			}
 		}
@@ -73,19 +72,6 @@ public class InsnRemover {
 		insn.add(AFlag.REMOVE);
 	}
 
-	public static void fixUsedInPhiFlag(RegisterArg useReg) {
-		PhiInsn usedIn = null;
-		for (RegisterArg reg : useReg.getSVar().getUseList()) {
-			InsnNode parentInsn = reg.getParentInsn();
-			if (parentInsn != null
-					&& parentInsn.getType() == InsnType.PHI
-					&& parentInsn.containsArg(useReg)) {
-				usedIn = (PhiInsn) parentInsn;
-			}
-		}
-		useReg.getSVar().setUsedInPhi(usedIn);
-	}
-
 	public static void unbindResult(MethodNode mth, InsnNode insn) {
 		RegisterArg r = insn.getResult();
 		if (r != null && r.getSVar() != null && mth != null) {

jadx-core/src/test/java/jadx/tests/integration/variables/TestVariablesDefinitions2.java

+package jadx.tests.integration.variables;
+
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.IntegrationTest;
+
+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 TestVariablesDefinitions2 extends IntegrationTest {
+
+	public static class TestCls {
+
+		public static int test(List<String> list) {
+			int i = 0;
+			if (list != null) {
+				for (String str : list) {
+					if (str.isEmpty()) {
+						i++;
+					}
+				}
+			}
+			return i;
+		}
+	}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+
+		assertThat(code, containsOne("int i = 0;"));
+		assertThat(code, containsOne("i++;"));
+		assertThat(code, containsOne("return i;"));
+		assertThat(code, not(containsString("i2;")));
+	}
+}