core: fix processing try/catch in other catch

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

@@ -11,7 +11,9 @@ import jadx.core.dex.visitors.AbstractVisitor;
 import jadx.core.utils.ErrorsCounter;
 import jadx.core.utils.exceptions.JadxException;
 
+import java.util.ArrayList;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 
 import org.slf4j.Logger;
@@ -47,6 +49,7 @@ public class CheckRegions extends AbstractVisitor {
 					// TODO
 					// mth.add(AFlag.INCONSISTENT_CODE);
 					LOG.debug(" Duplicated block: {} in {}", block, mth);
+					// printRegionsWithBlock(mth, block);
 				}
 			}
 		});
@@ -75,4 +78,17 @@ public class CheckRegions extends AbstractVisitor {
 			}
 		});
 	}
+
+	private static void printRegionsWithBlock(MethodNode mth, final BlockNode block) {
+		final List<IRegion> regions = new ArrayList<IRegion>();
+		DepthRegionTraversal.traverseAll(mth, new TracedRegionVisitor() {
+			@Override
+			public void processBlockTraced(MethodNode mth, IBlock container, IRegion currentRegion) {
+				if (block.equals(container)) {
+					regions.add(currentRegion);
+				}
+			}
+		});
+		LOG.debug(" Found block: {} in regions: {}", block, regions);
+	}
 }

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

@@ -28,23 +28,33 @@ import org.slf4j.LoggerFactory;
  * Extract blocks to separate try/catch region
  */
 public class ProcessTryCatchRegions extends AbstractRegionVisitor {
+
 	private static final Logger LOG = LoggerFactory.getLogger(ProcessTryCatchRegions.class);
-	private static final boolean DEBUG = false;
 
-	static {
-		if (DEBUG) {
-			LOG.debug("Debug enabled for " + ProcessTryCatchRegions.class);
+	public static void process(MethodNode mth) {
+		if (mth.isNoCode() || mth.isNoExceptionHandlers()) {
+			return;
 		}
-	}
 
-	private final Map<BlockNode, TryCatchBlock> tryBlocksMap = new HashMap<BlockNode, TryCatchBlock>(2);
+		final Map<BlockNode, TryCatchBlock> tryBlocksMap = new HashMap<BlockNode, TryCatchBlock>(2);
+		searchTryCatchDominators(mth, tryBlocksMap);
 
-	public ProcessTryCatchRegions(MethodNode mth) {
-		if (mth.isNoCode() || mth.isNoExceptionHandlers()) {
-			return;
+		int k = 0;
+		while (!tryBlocksMap.isEmpty()) {
+			DepthRegionTraversal.traverseAll(mth, new AbstractRegionVisitor() {
+				@Override
+				public void leaveRegion(MethodNode mth, IRegion region) {
+					checkAndWrap(tryBlocksMap, region);
+				}
+			});
+			if (k++ > 100) {
+				throw new JadxRuntimeException("Try/catch wrap count limit reached in " + mth);
+			}
 		}
+	}
 
-		Set<TryCatchBlock> tryBlocks = new HashSet<TryCatchBlock>();
+	private static void searchTryCatchDominators(MethodNode mth, Map<BlockNode, TryCatchBlock> tryBlocksMap) {
+		final Set<TryCatchBlock> tryBlocks = new HashSet<TryCatchBlock>();
 		// collect all try/catch blocks
 		for (BlockNode block : mth.getBasicBlocks()) {
 			CatchAttr c = block.get(AType.CATCH_BLOCK);
@@ -67,7 +77,6 @@ public class ProcessTryCatchRegions extends AbstractRegionVisitor {
 					}
 				}
 			}
-			assert bs != null;
 
 			// intersect to get dominator of dominators
 			List<BlockNode> domBlocks = BlockUtils.bitSetToBlocks(mth, bs);
@@ -87,28 +96,16 @@ public class ProcessTryCatchRegions extends AbstractRegionVisitor {
 				LOG.info("!!! TODO: merge try blocks in " + mth);
 			}
 		}
-
-		if (DEBUG && !tryBlocksMap.isEmpty()) {
-			LOG.debug("ProcessTryCatchRegions: \n {} \n {}", mth, tryBlocksMap);
-		}
 	}
 
-	@Override
-	public void leaveRegion(MethodNode mth, IRegion region) {
-		if (tryBlocksMap.isEmpty() || !(region instanceof Region)) {
-			return;
-		}
+	private static void checkAndWrap(Map<BlockNode, TryCatchBlock> tryBlocksMap, IRegion region) {
 		// search dominator blocks in this region (don't need to go deeper)
-		for (BlockNode dominator : tryBlocksMap.keySet()) {
+		for (Map.Entry<BlockNode, TryCatchBlock> entry : tryBlocksMap.entrySet()) {
+			BlockNode dominator = entry.getKey();
 			if (region.getSubBlocks().contains(dominator)) {
-				Region newRegion = wrapBlocks(region, dominator);
+				TryCatchBlock tb = tryBlocksMap.get(dominator);
+				wrapBlocks(region, tb, dominator);
 				tryBlocksMap.remove(dominator);
-				if (newRegion != null) {
-					// dominator may be moved into new region
-					leaveRegion(mth, newRegion);
-					// if region is modified rerun this method
-					leaveRegion(mth, region);
-				}
 				return;
 			}
 		}
@@ -117,11 +114,8 @@ public class ProcessTryCatchRegions extends AbstractRegionVisitor {
 	/**
 	 * Extract all block dominated by 'dominator' to separate region and mark as try/catch block
 	 */
-	private Region wrapBlocks(IRegion region, BlockNode dominator) {
+	private static void wrapBlocks(IRegion region, TryCatchBlock tb, BlockNode dominator) {
 		Region newRegion = new Region(region);
-		TryCatchBlock tb = tryBlocksMap.get(dominator);
-		assert tb != null;
-
 		for (IContainer cont : region.getSubBlocks()) {
 			if (RegionUtils.isDominatedBy(dominator, cont)) {
 				if (isHandlerPath(tb, cont)) {
@@ -131,10 +125,7 @@ public class ProcessTryCatchRegions extends AbstractRegionVisitor {
 			}
 		}
 		if (newRegion.getSubBlocks().isEmpty()) {
-			return null;
-		}
-		if (DEBUG) {
-			LOG.debug("ProcessTryCatchRegions mark: {}", newRegion);
+			return;
 		}
 		// replace first node by region
 		IContainer firstNode = newRegion.getSubBlocks().get(0);
@@ -151,11 +142,9 @@ public class ProcessTryCatchRegions extends AbstractRegionVisitor {
 				aReg.setParent(newRegion);
 			}
 		}
-
-		return newRegion;
 	}
 
-	private boolean isHandlerPath(TryCatchBlock tb, IContainer cont) {
+	private static boolean isHandlerPath(TryCatchBlock tb, IContainer cont) {
 		for (ExceptionHandler h : tb.getHandlers()) {
 			if (RegionUtils.hasPathThruBlock(h.getHandlerBlock(), cont)) {
 				return true;
@@ -163,5 +152,4 @@ public class ProcessTryCatchRegions extends AbstractRegionVisitor {
 		}
 		return false;
 	}
-
 }

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

@@ -598,6 +598,12 @@ public class RegionMaker {
 
 		RegionStack stack = new RegionStack(mth);
 		stack.addExits(exits);
+
+		BlockNode exit = BlockUtils.traverseWhileDominates(start, start);
+		if (exit != null && RegionUtils.isRegionContainsBlock(mth.getRegion(), exit)) {
+			stack.addExit(exit);
+		}
+
 		handler.setHandlerRegion(makeRegion(start, stack));
 
 		ExcHandlerAttr excHandlerAttr = start.get(AType.EXC_HANDLER);

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

@@ -42,9 +42,8 @@ public class RegionMakerVisitor extends AbstractVisitor {
 
 	private static void postProcessRegions(MethodNode mth) {
 		// make try-catch regions
-		if (!mth.isNoExceptionHandlers()) {
-			DepthRegionTraversal.traverse(mth, new ProcessTryCatchRegions(mth));
-		}
+		ProcessTryCatchRegions.process(mth);
+
 		// merge conditions in loops
 		if (mth.getLoopsCount() != 0) {
 			DepthRegionTraversal.traverseAll(mth, new AbstractRegionVisitor() {

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

@@ -12,12 +12,12 @@ public abstract class TracedRegionVisitor implements IRegionVisitor {
 	protected final Deque<IRegion> regionStack = new ArrayDeque<IRegion>();
 
 	@Override
-	public void enterRegion(MethodNode mth, IRegion region) {
+	public final void enterRegion(MethodNode mth, IRegion region) {
 		regionStack.push(region);
 	}
 
 	@Override
-	public void processBlock(MethodNode mth, IBlock container) {
+	public final void processBlock(MethodNode mth, IBlock container) {
 		IRegion curRegion = regionStack.peek();
 		processBlockTraced(mth, container, curRegion);
 	}
@@ -25,7 +25,7 @@ public abstract class TracedRegionVisitor implements IRegionVisitor {
 	public abstract void processBlockTraced(MethodNode mth, IBlock container, IRegion currentRegion);
 
 	@Override
-	public void leaveRegion(MethodNode mth, IRegion region) {
+	public final void leaveRegion(MethodNode mth, IRegion region) {
 		regionStack.pop();
 	}
 }

jadx-core/src/test/java/jadx/tests/internal/others/TestIfTryInCatch.java

+package jadx.tests.internal.others;
+
+import jadx.api.InternalJadxTest;
+import jadx.core.dex.nodes.ClassNode;
+
+import org.junit.Test;
+
+import static jadx.tests.utils.JadxMatchers.containsOne;
+import static jadx.tests.utils.JadxMatchers.countString;
+import static org.junit.Assert.assertThat;
+
+public class TestIfTryInCatch extends InternalJadxTest {
+
+	public static class TestCls {
+		private static final String TAG = "TAG";
+		private Exception exception;
+		private java.lang.Object data;
+
+		public java.lang.Object test(final Object obj) {
+			exception = null;
+			try {
+				return f();
+			} catch (Exception e) {
+				if (a(e) && b(obj)) {
+					try {
+						return f();
+					} catch (Exception e2) {
+						e = e2;
+					}
+				}
+				System.out.println("Exception" + e);
+				exception = e;
+				return data;
+			}
+		}
+
+		private static boolean b(Object obj) {
+			return false;
+		}
+
+		private static boolean a(Exception e) {
+			return false;
+		}
+
+		private Object f() {
+			return null;
+		}
+	}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+		System.out.println(code);
+
+		assertThat(code, countString(2, "try {"));
+		assertThat(code, containsOne("if ("));
+		assertThat(code, countString(2, "return f();"));
+	}
+}