core: improve processing of 'try/catch' and 'break' in loops

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

@@ -44,7 +44,7 @@ public class InsnDecoder {
 			while (in.hasMore()) {
 				decoded[in.cursor()] = DecodedInstruction.decode(in);
 			}
-		} catch (EOFException e) {
+		} catch (Exception e) {
 			throw new DecodeException(method, "", e);
 		}
 		insnArr = decoded;

jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java

@@ -230,6 +230,9 @@ public class MethodNode extends LineAttrNode implements ILoadable {
 		InsnNode[] insnByOffset = instructions;
 		CatchHandler[] catchBlocks = mthCode.getCatchHandlers();
 		Try[] tries = mthCode.getTries();
+		if (catchBlocks.length == 0 && tries.length == 0) {
+			return;
+		}
 
 		int hc = 0;
 		Set<Integer> addrs = new HashSet<Integer>();
@@ -287,13 +290,17 @@ public class MethodNode extends LineAttrNode implements ILoadable {
 			int offset = aTry.getStartAddress();
 			int end = offset + aTry.getInstructionCount() - 1;
 
-			insnByOffset[offset].add(AFlag.TRY_ENTER);
+			InsnNode insn = insnByOffset[offset];
+			insn.add(AFlag.TRY_ENTER);
 			while (offset <= end && offset >= 0) {
-				catchBlock.addInsn(insnByOffset[offset]);
+				insn = insnByOffset[offset];
+				catchBlock.addInsn(insn);
 				offset = InsnDecoder.getNextInsnOffset(insnByOffset, offset);
 			}
 			if (insnByOffset[end] != null) {
 				insnByOffset[end].add(AFlag.TRY_LEAVE);
+			} else {
+				insn.add(AFlag.TRY_LEAVE);
 			}
 		}
 	}
@@ -498,13 +505,12 @@ public class MethodNode extends LineAttrNode implements ILoadable {
 
 			/** workaround for non-static inner class constructor, that has
 			 * synthetic argument */
-			if ((parentClass != null) && parentClass.getClassInfo().isInner()) {
-				if (!parentClass.getAccessFlags().isStatic()) {
-					ClassNode outerCls = parentClass.getParentClass();
-					if ((argsList != null) && (argsList.size() >= 1)) {
-						if (argsList.get(0).getType().equals(outerCls.getClassInfo().getType())) {
-							defaultArgCount = 1;
-						}
+			if (parentClass.getClassInfo().isInner()
+					&& !parentClass.getAccessFlags().isStatic()) {
+				ClassNode outerCls = parentClass.getParentClass();
+				if ((argsList != null) && (argsList.size() >= 1)) {
+					if (argsList.get(0).getType().equals(outerCls.getClassInfo().getType())) {
+						defaultArgCount = 1;
 					}
 				}
 			}

jadx-core/src/main/java/jadx/core/dex/regions/loops/LoopRegion.java

@@ -88,25 +88,24 @@ public final class LoopRegion extends AbstractRegion {
 			InsnNode insn = insns.get(i);
 			if (insn.getResult() == null) {
 				return false;
-			} else {
-				RegisterArg res = insn.getResult();
-				if (res.getSVar().getUseCount() > 1) {
-					return false;
-				}
-				boolean found = false;
-				// search result arg in other insns
-				for (int j = i + 1; j < size; j++) {
-					if (insns.get(i).containsArg(res)) {
-						found = true;
-					}
-				}
-				// or in if insn
-				if (!found && ifInsn.containsArg(res)) {
+			}
+			RegisterArg res = insn.getResult();
+			if (res.getSVar().getUseCount() > 1) {
+				return false;
+			}
+			boolean found = false;
+			// search result arg in other insns
+			for (int j = i + 1; j < size; j++) {
+				if (insns.get(i).containsArg(res)) {
 					found = true;
 				}
-				if (!found) {
-					return false;
-				}
+			}
+			// or in if insn
+			if (!found && ifInsn.containsArg(res)) {
+				found = true;
+			}
+			if (!found) {
+				return false;
 			}
 		}
 		return true;
@@ -163,6 +162,6 @@ public final class LoopRegion extends AbstractRegion {
 
 	@Override
 	public String toString() {
-		return "LOOP: " + baseString();
+		return "LOOP:" + info.getId() + ": " + baseString();
 	}
 }

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

@@ -103,7 +103,9 @@ public class BlockMakerVisitor extends AbstractVisitor {
 				// add this insn in new block
 				block = startNewBlock(mth, -1);
 				curBlock.add(AFlag.SYNTHETIC);
-				block.addAttr(new SplitterBlockAttr(curBlock));
+				SplitterBlockAttr splitter = new SplitterBlockAttr(curBlock);
+				block.addAttr(splitter);
+				curBlock.addAttr(splitter);
 				connect(curBlock, block);
 				curBlock = block;
 			} else {
@@ -131,12 +133,16 @@ public class BlockMakerVisitor extends AbstractVisitor {
 				// get synthetic block for handlers
 				SplitterBlockAttr spl = block.get(AType.SPLITTER_BLOCK);
 				if (catches != null && spl != null) {
-					BlockNode connBlock = spl.getBlock();
+					BlockNode splitterBlock = spl.getBlock();
+					boolean tryEnd = insn.contains(AFlag.TRY_LEAVE);
 					for (ExceptionHandler h : catches.getTryBlock().getHandlers()) {
-						BlockNode destBlock = getBlock(h.getHandleOffset(), blocksMap);
+						BlockNode handlerBlock = getBlock(h.getHandleOffset(), blocksMap);
 						// skip self loop in handler
-						if (connBlock != destBlock) {
-							connect(connBlock, destBlock);
+						if (splitterBlock != handlerBlock) {
+							connect(splitterBlock, handlerBlock);
+						}
+						if (tryEnd) {
+							connect(block, handlerBlock);
 						}
 					}
 				}

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

@@ -182,7 +182,7 @@ public class RegionMaker {
 		loopRegion.setCondition(condInfo.getCondition());
 		exitBlocks.removeAll(condInfo.getMergedBlocks());
 
-		if (exitBlocks.size() > 0) {
+		if (!exitBlocks.isEmpty()) {
 			BlockNode loopExit = condInfo.getElseBlock();
 			if (loopExit != null) {
 				// add 'break' instruction before path cross between main loop exit and sub-exit
@@ -264,6 +264,25 @@ public class RegionMaker {
 				found = false;
 			}
 			if (found) {
+				List<LoopInfo> list = mth.getAllLoopsForBlock(block);
+				if (list.size() >= 2) {
+					// bad condition if successors going out of all loops
+					boolean allOuter = true;
+					for (BlockNode outerBlock : block.getCleanSuccessors()) {
+						List<LoopInfo> outLoopList = mth.getAllLoopsForBlock(outerBlock);
+						outLoopList.remove(loop);
+						if (!outLoopList.isEmpty()) {
+							// goes to outer loop
+							allOuter = false;
+							break;
+						}
+					}
+					if (allOuter) {
+						found = false;
+					}
+				}
+			}
+			if (found) {
 				return loopRegion;
 			}
 		}
@@ -281,16 +300,13 @@ public class RegionMaker {
 		BlockNode loopExit = null;
 		// insert 'break' for exits
 		List<Edge> exitEdges = loop.getExitEdges();
-		if (exitEdges.size() == 1) {
-			for (Edge exitEdge : exitEdges) {
-				BlockNode exit = exitEdge.getTarget();
-				if (canInsertBreak(exit)) {
-					exit.getInstructions().add(new InsnNode(InsnType.BREAK, 0));
-					BlockNode nextBlock = getNextBlock(exit);
-					if (nextBlock != null) {
-						stack.addExit(nextBlock);
-						loopExit = nextBlock;
-					}
+		for (Edge exitEdge : exitEdges) {
+			BlockNode exit = exitEdge.getTarget();
+			if (tryInsertBreak(stack, exit, exitEdge)) {
+				BlockNode nextBlock = getNextBlock(exit);
+				if (nextBlock != null) {
+					stack.addExit(nextBlock);
+					loopExit = nextBlock;
 				}
 			}
 		}
@@ -313,7 +329,8 @@ public class RegionMaker {
 	}
 
 	private boolean canInsertBreak(BlockNode exit) {
-		if (exit.contains(AFlag.RETURN)) {
+		if (exit.contains(AFlag.RETURN)
+				|| BlockUtils.checkLastInsnType(exit, InsnType.BREAK)) {
 			return false;
 		}
 		List<BlockNode> simplePath = BlockUtils.buildSimplePath(exit);
@@ -331,35 +348,60 @@ public class RegionMaker {
 		return true;
 	}
 
-	private void tryInsertBreak(RegionStack stack, BlockNode loopExit, Edge exitEdge) {
-		BlockNode prev = null;
+	private boolean tryInsertBreak(RegionStack stack, BlockNode loopExit, Edge exitEdge) {
 		BlockNode exit = exitEdge.getTarget();
-		while (exit != null) {
-			if (prev != null && isPathExists(loopExit, exit)) {
-				// found cross
-				if (canInsertBreak(exit)) {
-					InsnNode breakInsn = new InsnNode(InsnType.BREAK, 0);
-					prev.getInstructions().add(breakInsn);
-					stack.addExit(exit);
-					// add label to 'break' if needed
-					List<LoopInfo> loops = mth.getAllLoopsForBlock(exitEdge.getSource());
-					if (loops.size() >= 2) {
-						// find parent loop
-						for (LoopInfo loop : loops) {
-							if (loop.getParentLoop() == null) {
-								LoopLabelAttr labelAttr = new LoopLabelAttr(loop);
-								breakInsn.addAttr(labelAttr);
-								loop.getStart().addAttr(labelAttr);
-								break;
-							}
-						}
+		BlockNode insertBlock = null;
+		boolean confirm = false;
+		// process special cases
+		if (loopExit == exit) {
+			// try/catch at loop end
+			BlockNode source = exitEdge.getSource();
+			if (source.contains(AType.CATCH_BLOCK)) {
+				BlockNode other = BlockUtils.selectOther(loopExit, source.getSuccessors());
+				if (other != null && other.contains(AType.EXC_HANDLER)) {
+					insertBlock = source;
+					confirm = true;
+				}
+			}
+		}
+		if (!confirm) {
+			while (exit != null) {
+				if (insertBlock != null && isPathExists(loopExit, exit)) {
+					// found cross
+					if (canInsertBreak(insertBlock)) {
+						confirm = true;
+						break;
 					}
+					return false;
+				}
+				insertBlock = exit;
+				List<BlockNode> cs = exit.getCleanSuccessors();
+				exit = cs.size() == 1 ? cs.get(0) : null;
+			}
+		}
+		if (!confirm) {
+			return false;
+		}
+		InsnNode breakInsn = new InsnNode(InsnType.BREAK, 0);
+		insertBlock.getInstructions().add(breakInsn);
+		stack.addExit(exit);
+		// add label to 'break' if needed
+		List<LoopInfo> loops = mth.getAllLoopsForBlock(exitEdge.getSource());
+		if (loops.size() >= 2) {
+			// find parent loop
+			for (LoopInfo loop : loops) {
+				LoopInfo parentLoop = loop.getParentLoop();
+				if (parentLoop == null
+						&& loop.getEnd() != exit
+						&& !loop.getExitNodes().contains(exit)) {
+					LoopLabelAttr labelAttr = new LoopLabelAttr(loop);
+					breakInsn.addAttr(labelAttr);
+					loop.getStart().addAttr(labelAttr);
+					break;
 				}
-				return;
 			}
-			prev = exit;
-			exit = getNextBlock(exit);
 		}
+		return true;
 	}
 
 	private final Set<BlockNode> cacheSet = new HashSet<BlockNode>();

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

@@ -246,16 +246,16 @@ public class BlockUtils {
 		Set<BlockNode> set = new HashSet<BlockNode>();
 		set.add(start);
 		if (start != end) {
-			addPredcessors(set, end, start);
+			addPredecessors(set, end, start);
 		}
 		return set;
 	}
 
-	private static void addPredcessors(Set<BlockNode> set, BlockNode from, BlockNode until) {
+	private static void addPredecessors(Set<BlockNode> set, BlockNode from, BlockNode until) {
 		set.add(from);
 		for (BlockNode pred : from.getPredecessors()) {
 			if (pred != until && !set.contains(pred)) {
-				addPredcessors(set, pred, until);
+				addPredecessors(set, pred, until);
 			}
 		}
 	}

jadx-core/src/test/java/jadx/tests/integration/loops/TestNestedLoops3.java

+package jadx.tests.integration.loops;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.IntegrationTest;
+
+import org.junit.Test;
+
+import static jadx.tests.api.utils.JadxMatchers.containsOne;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+public class TestNestedLoops3 extends IntegrationTest {
+
+	public static class TestCls {
+		int c = 0;
+
+		public int test(int b) {
+			int i;
+			loop0:
+			while (true) {
+				f1();
+				i = 0;
+				while (true) {
+					f2();
+					if (i != 0) {
+						break loop0;
+					}
+					i += 3;
+					if (b >= 16) {
+						break loop0;
+					}
+					try {
+						exc();
+						break;
+					} catch (Exception e) {
+						//
+					}
+				}
+			}
+			return i;
+		}
+
+		private void exc() throws Exception {
+			if (c > 200) {
+				throw new Exception();
+			}
+		}
+
+		private void f1() {
+			c += 1;
+		}
+
+		private void f2() {
+			c += 100;
+		}
+
+		public void check() {
+			test(1);
+			assertEquals(302, c);
+		}
+	}
+
+	@Test
+	public void test() {
+		setOutputCFG();
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+
+		assertThat(code, containsOne("} catch (Exception e) {"));
+	}
+}

jadx-core/src/test/java/jadx/tests/integration/loops/TestTryCatchInLoop.java

+package jadx.tests.integration.loops;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.IntegrationTest;
+
+import org.junit.Test;
+
+import static jadx.tests.api.utils.JadxMatchers.containsOne;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+public class TestTryCatchInLoop extends IntegrationTest {
+
+	public static class TestCls {
+		int c = 0;
+
+		public int test() {
+			while (true) {
+				try {
+					exc();
+					break;
+				} catch (Exception e) {
+					//
+				}
+			}
+			if (c == 5) {
+				System.out.println(c);
+			}
+			return 0;
+		}
+
+		private void exc() throws Exception {
+			c++;
+			if (c < 3) {
+				throw new Exception();
+			}
+		}
+
+		public void check() {
+			test();
+			assertEquals(3, c);
+		}
+	}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+
+		assertThat(code, containsOne("} catch (Exception e) {"));
+		assertThat(code, containsOne("break;"));
+	}
+}