core: fix variable declaration used in several loops

jadx-core/src/main/java/jadx/core/Jadx.java

@@ -82,7 +82,6 @@ public class Jadx {
 
 			passes.add(new CodeShrinker());
 			passes.add(new SimplifyVisitor());
-			passes.add(new ProcessVariables());
 			passes.add(new CheckRegions());
 
 			if (args.isCFGOutput()) {
@@ -93,6 +92,7 @@ public class Jadx {
 			passes.add(new ClassModifier());
 			passes.add(new PrepareForCodeGen());
 			passes.add(new LoopRegionVisitor());
+			passes.add(new ProcessVariables());
 		}
 		passes.add(new CodeGen(args));
 		return passes;

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

@@ -15,6 +15,9 @@ import jadx.core.dex.nodes.InsnNode;
 import jadx.core.dex.nodes.MethodNode;
 import jadx.core.dex.regions.SwitchRegion;
 import jadx.core.dex.regions.conditions.IfRegion;
+import jadx.core.dex.regions.loops.ForLoop;
+import jadx.core.dex.regions.loops.LoopRegion;
+import jadx.core.dex.regions.loops.LoopType;
 import jadx.core.dex.visitors.AbstractVisitor;
 import jadx.core.utils.RegionUtils;
 import jadx.core.utils.exceptions.JadxException;
@@ -112,45 +115,77 @@ public class ProcessVariables extends AbstractVisitor {
 		}
 	}
 
+	private static class CollectUsageRegionVisitor extends TracedRegionVisitor {
+		private final List<RegisterArg> args;
+		private final Map<Variable, Usage> usageMap;
+
+		public CollectUsageRegionVisitor(Map<Variable, Usage> usageMap) {
+			this.usageMap = usageMap;
+			args = new ArrayList<RegisterArg>();
+		}
+
+		@Override
+		public void processBlockTraced(MethodNode mth, IBlock container, IRegion curRegion) {
+			regionProcess(mth, curRegion);
+			int len = container.getInstructions().size();
+			for (int i = 0; i < len; i++) {
+				InsnNode insn = container.getInstructions().get(i);
+				if (insn.contains(AFlag.SKIP)) {
+					continue;
+				}
+				args.clear();
+				processInsn(insn, curRegion);
+			}
+		}
+
+		private void regionProcess(MethodNode mth, IRegion region) {
+			if (region instanceof LoopRegion) {
+				LoopRegion loopRegion = (LoopRegion) region;
+				LoopType loopType = loopRegion.getType();
+				if (loopType instanceof ForLoop) {
+					ForLoop forLoop = (ForLoop) loopType;
+					processInsn(forLoop.getInitInsn(), region);
+					processInsn(forLoop.getIncrInsn(), region);
+				}
+			}
+		}
+
+		void processInsn(InsnNode insn, IRegion curRegion) {
+			if (insn == null) {
+				return;
+			}
+			// result
+			RegisterArg result = insn.getResult();
+			if (result != null && result.isRegister()) {
+				Usage u = addToUsageMap(result, usageMap);
+				if (u.getArg() == null) {
+					u.setArg(result);
+					u.setArgRegion(curRegion);
+				}
+				u.getAssigns().add(curRegion);
+			}
+			// args
+			args.clear();
+			insn.getRegisterArgs(args);
+			for (RegisterArg arg : args) {
+				Usage u = addToUsageMap(arg, usageMap);
+				u.getUseRegions().add(curRegion);
+			}
+		}
+	}
+
 	@Override
 	public void visit(MethodNode mth) throws JadxException {
 		if (mth.isNoCode()) {
 			return;
 		}
-
 		final Map<Variable, Usage> usageMap = new LinkedHashMap<Variable, Usage>();
 		for (RegisterArg arg : mth.getArguments(true)) {
 			addToUsageMap(arg, usageMap);
 		}
 
 		// collect all variables usage
-		IRegionVisitor collect = new TracedRegionVisitor() {
-			@Override
-			public void processBlockTraced(MethodNode mth, IBlock container, IRegion curRegion) {
-				int len = container.getInstructions().size();
-				List<RegisterArg> args = new ArrayList<RegisterArg>();
-				for (int i = 0; i < len; i++) {
-					InsnNode insn = container.getInstructions().get(i);
-					// result
-					RegisterArg result = insn.getResult();
-					if (result != null && result.isRegister()) {
-						Usage u = addToUsageMap(result, usageMap);
-						if (u.getArg() == null) {
-							u.setArg(result);
-							u.setArgRegion(curRegion);
-						}
-						u.getAssigns().add(curRegion);
-					}
-					// args
-					args.clear();
-					insn.getRegisterArgs(args);
-					for (RegisterArg arg : args) {
-						Usage u = addToUsageMap(arg, usageMap);
-						u.getUseRegions().add(curRegion);
-					}
-				}
-			}
-		};
+		IRegionVisitor collect = new CollectUsageRegionVisitor(usageMap);
 		DepthRegionTraversal.traverseAll(mth, collect);
 
 		// reduce assigns map
@@ -189,10 +224,10 @@ public class ProcessVariables extends AbstractVisitor {
 			for (IRegion assignRegion : u.getAssigns()) {
 				if (u.getArgRegion() == assignRegion
 						&& canDeclareInRegion(u, assignRegion, regionsOrder)) {
-					u.getArg().getParentInsn().add(AFlag.DECLARE_VAR);
-					processVar(u.getArg());
-					it.remove();
-					break;
+					if (declareAtAssign(u)) {
+						it.remove();
+						break;
+					}
 				}
 			}
 		}
@@ -239,7 +274,7 @@ public class ProcessVariables extends AbstractVisitor {
 		}
 	}
 
-	Usage addToUsageMap(RegisterArg arg, Map<Variable, Usage> usageMap) {
+	private static Usage addToUsageMap(RegisterArg arg, Map<Variable, Usage> usageMap) {
 		Variable varId = new Variable(arg);
 		Usage usage = usageMap.get(varId);
 		if (usage == null) {
@@ -260,6 +295,17 @@ public class ProcessVariables extends AbstractVisitor {
 		return usage;
 	}
 
+	private static boolean declareAtAssign(Usage u) {
+		RegisterArg arg = u.getArg();
+		InsnNode parentInsn = arg.getParentInsn();
+		if (!arg.equals(parentInsn.getResult())) {
+			return false;
+		}
+		parentInsn.add(AFlag.DECLARE_VAR);
+		processVar(arg);
+		return true;
+	}
+
 	private static void declareVar(IContainer region, RegisterArg arg) {
 		DeclareVariablesAttr dv = region.get(AType.DECLARE_VARIABLES);
 		if (dv == null) {
@@ -308,6 +354,14 @@ public class ProcessVariables extends AbstractVisitor {
 			LOG.debug("TODO: Not found order for region {} for {}", region, u);
 			return false;
 		}
+		// workaround for declare variables used in several loops
+		if (region instanceof LoopRegion) {
+			for (IRegion r : u.getAssigns()) {
+				if (!RegionUtils.isRegionContainsRegion(region, r)) {
+					return false;
+				}
+			}
+		}
 		return isAllRegionsAfter(region, pos, u.getAssigns(), regionsOrder)
 				&& isAllRegionsAfter(region, pos, u.getUseRegions(), regionsOrder);
 	}

jadx-core/src/test/java/jadx/tests/integration/names/TestNameAssign2.java

+package jadx.tests.integration.names;
+
+import jadx.core.dex.nodes.BlockNode;
+import jadx.core.dex.nodes.ClassNode;
+import jadx.core.dex.nodes.MethodNode;
+import jadx.core.dex.visitors.ssa.LiveVarAnalysis;
+import jadx.tests.api.IntegrationTest;
+
+import java.util.BitSet;
+import java.util.Deque;
+import java.util.LinkedList;
+import java.util.List;
+
+import org.junit.Test;
+
+import static jadx.tests.api.utils.JadxMatchers.containsOne;
+import static org.junit.Assert.assertThat;
+
+public class TestNameAssign2 extends IntegrationTest {
+
+	public static class TestCls {
+
+		private static void test(MethodNode mth, int regNum, LiveVarAnalysis la) {
+			List<BlockNode> blocks = mth.getBasicBlocks();
+			int blocksCount = blocks.size();
+			BitSet hasPhi = new BitSet(blocksCount);
+			BitSet processed = new BitSet(blocksCount);
+			Deque<BlockNode> workList = new LinkedList<BlockNode>();
+
+			BitSet assignBlocks = la.getAssignBlocks(regNum);
+			for (int id = assignBlocks.nextSetBit(0); id >= 0; id = assignBlocks.nextSetBit(id + 1)) {
+				processed.set(id);
+				workList.add(blocks.get(id));
+			}
+			while (!workList.isEmpty()) {
+				BlockNode block = workList.pop();
+				BitSet domFrontier = block.getDomFrontier();
+				for (int id = domFrontier.nextSetBit(0); id >= 0; id = domFrontier.nextSetBit(id + 1)) {
+					if (!hasPhi.get(id) && la.isLive(id, regNum)) {
+						BlockNode df = blocks.get(id);
+						addPhi(df, regNum);
+						hasPhi.set(id);
+						if (!processed.get(id)) {
+							processed.set(id);
+							workList.add(df);
+						}
+					}
+				}
+			}
+		}
+
+		private static void addPhi(BlockNode df, int regNum) {
+		}
+	}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+
+		// TODO:
+		assertThat(code, containsOne("int id;"));
+	}
+}