fix: add more checks to better find handler's end (PR #2749)

jadx-core/src/main/java/jadx/core/dex/visitors/regions/maker/ExcHandlersRegionMaker.java

@@ -132,8 +132,18 @@ public class ExcHandlersRegionMaker {
 		if (dom.contains(AFlag.REMOVE)) {
 			return;
 		}
-		BitSet domFrontier = dom.getDomFrontier();
-		List<BlockNode> handlerExits = BlockUtils.bitSetToBlocks(mth, domFrontier);
+		List<BlockNode> handlerExits = new ArrayList<>();
+
+		BlockNode handlerOutBlock = BlockUtils.getTryAndHandlerCrossBlock(mth, handler);
+		if (handlerOutBlock != null) {
+			// ensure frontier's other predecessors comes from try end
+			handlerExits.add(handlerOutBlock);
+		} else {
+			// fallback to simple frontier
+			BitSet domFrontier = dom.getDomFrontier();
+			handlerExits.addAll(BlockUtils.bitSetToBlocks(mth, domFrontier));
+		}
+
 		boolean inLoop = mth.getLoopForBlock(start) != null;
 		for (BlockNode exit : handlerExits) {
 			if ((!inLoop || BlockUtils.isPathExists(start, exit))

jadx-core/src/main/java/jadx/core/dex/visitors/regions/maker/IfRegionMaker.java

@@ -28,6 +28,7 @@ import jadx.core.dex.regions.conditions.IfCondition;
 import jadx.core.dex.regions.conditions.IfInfo;
 import jadx.core.dex.regions.conditions.IfRegion;
 import jadx.core.dex.regions.loops.LoopRegion;
+import jadx.core.dex.trycatch.ExcHandlerAttr;
 import jadx.core.utils.BlockUtils;
 import jadx.core.utils.blocks.BlockSet;
 import jadx.core.utils.exceptions.JadxRuntimeException;
@@ -197,6 +198,21 @@ final class IfRegionMaker {
 			info = new IfInfo(info, elseBlock, null);
 			info.setOutBlock(thenBlock);
 		}
+
+		// getPathCross may not find outBlock (e.g. one branch has return, outBlock definitely is
+		// null), so should check further
+		if (info.getOutBlock() == null) {
+			BlockNode scopeOutBlockThen = findScopeOutBlock(mth, info.getThenBlock());
+			BlockNode scopeOutBlockElse = findScopeOutBlock(mth, info.getElseBlock());
+			if (scopeOutBlockThen == null && scopeOutBlockElse != null) {
+				info.setOutBlock(scopeOutBlockElse);
+			} else if (scopeOutBlockThen != null && scopeOutBlockElse == null) {
+				info.setOutBlock(scopeOutBlockThen);
+			} else if (scopeOutBlockThen != null && scopeOutBlockThen == scopeOutBlockElse) {
+				info.setOutBlock(scopeOutBlockThen);
+			}
+		}
+
 		if (BlockUtils.isBackEdge(block, info.getOutBlock())) {
 			info.setOutBlock(null);
 		}
@@ -243,6 +259,33 @@ final class IfRegionMaker {
 		return true;
 	}
 
+	/**
+	 * if startBlock is in a (try) scope, find the scope end as outBlock
+	 */
+	private @Nullable static BlockNode findScopeOutBlock(MethodNode mth, BlockNode startBlock) {
+		if (startBlock == null) {
+			return null;
+		}
+		List<BlockNode> domFrontiers = BlockUtils.bitSetToBlocks(mth, startBlock.getDomFrontier());
+		BlockNode scopeOutBlock = null;
+
+		// find handler from domFrontier(could be scope end), if domFrontier is handler
+		// and its topSplitter dominates branch block, then branch should end
+		for (BlockNode domFrontier : domFrontiers) {
+			ExcHandlerAttr handler = domFrontier.get(AType.EXC_HANDLER);
+			if (handler == null) {
+				continue;
+			}
+			BlockNode topSplitter = handler.getTryBlock().getTopSplitter();
+			if (startBlock.isDominator(topSplitter)) {
+				scopeOutBlock = BlockUtils.getTryAndHandlerCrossBlock(mth, handler.getHandler());
+				break;
+			}
+		}
+
+		return scopeOutBlock;
+	}
+
 	static IfInfo mergeNestedIfNodes(IfInfo currentIf) {
 		BlockNode curThen = currentIf.getThenBlock();
 		BlockNode curElse = currentIf.getElseBlock();

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

@@ -1184,6 +1184,47 @@ public class BlockUtils {
 		return block;
 	}
 
+	/**
+	 * Return out block of try catch, by finding where try branch meets catch branch.
+	 * It traverse domFrontier start from handler block, find the first frontier
+	 * whose predecessor is try end.
+	 * <br>
+	 * It could return null if they never meets, but this doesn't mean that catch
+	 * ends at the method exit.
+	 * (see TestSwitchWithTryCatch and ExcHandlersRegionMaker#processExcHandler).
+	 */
+	@Nullable
+	public static BlockNode getTryAndHandlerCrossBlock(MethodNode mth, ExceptionHandler handler) {
+		BlockNode start = handler.getHandlerBlock();
+		BlockNode topSplitter = BlockUtils.getTopSplitterForHandler(start);
+		List<ExceptionHandler> allHandlers = handler.getTryBlock().getHandlers();
+		List<BlockNode> handlerExitsCandidate = new ArrayList<>(BlockUtils.bitSetToBlocks(mth, start.getDomFrontier()));
+		BitSet visited = newBlocksBitSet(mth);
+		while (!handlerExitsCandidate.isEmpty()) {
+			BlockNode frontier = handlerExitsCandidate.remove(0);
+			if (visited.get(frontier.getPos())) {
+				continue;
+			}
+			visited.set(frontier.getPos());
+			// In some cases, handler's domFrontier is in the half of catch block
+			// instead of the end, so we need to make sure frontier's predecessor
+			// comes from try branch end:
+			// 1. not from handler branch, doesn't exist path from handler to pred
+			// 2. from try branch, exists path from topSplitter to pred
+			// 3. skip method exit
+			for (BlockNode pred : frontier.getPredecessors()) {
+				boolean predFromHandler = allHandlers.stream().anyMatch(h -> isPathExists(h.getHandlerBlock(), pred));
+				if (!predFromHandler && BlockUtils.isPathExists(topSplitter, pred)
+						&& frontier != mth.getExitBlock()) {
+					return frontier;
+				}
+			}
+			// if not found, add this frontier's frontier to candidate list
+			handlerExitsCandidate.addAll(BlockUtils.bitSetToBlocks(mth, frontier.getDomFrontier()));
+		}
+		return null;
+	}
+
 	@Nullable
 	public static BlockNode getBlockWithFlag(List<BlockNode> blocks, AFlag flag) {
 		for (BlockNode block : blocks) {

jadx-core/src/test/java/jadx/tests/integration/trycatch/TestIfInTryCatch.java

@@ -0,0 +1,41 @@
+package jadx.tests.integration.trycatch;
+
+import org.junit.jupiter.api.Test;
+
+import jadx.tests.api.IntegrationTest;
+import jadx.tests.api.utils.assertj.JadxAssertions;
+
+public class TestIfInTryCatch extends IntegrationTest {
+	public static class TestCls {
+		private void test() {
+			/*
+			 * 1. if in try
+			 * 2. then branch is return
+			 * 3. after if, there's more blocks inside try
+			 * 4. after try, there's more blocks
+			 * this will result in if block and below moved out of try
+			 */
+			try {
+				if (getDouble() > 0.5) {
+					return;
+				}
+				System.out.println("after if");
+			} catch (Exception e) {
+				System.out.println("exception");
+			}
+			System.out.println("after try");
+		}
+
+		private static double getDouble() throws InterruptedException {
+			Thread.sleep(50L);
+			return Math.random();
+		}
+	}
+
+	@Test
+	public void test() {
+		// if ifBlock is moved out of try, there will be uncaught exception
+		JadxAssertions.assertThat(getClassNode(TestCls.class))
+				.code();
+	}
+}

jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchInIf2.java

@@ -0,0 +1,32 @@
+package jadx.tests.integration.trycatch;
+
+import org.junit.jupiter.api.Test;
+
+import jadx.tests.api.IntegrationTest;
+import jadx.tests.api.utils.assertj.JadxAssertions;
+
+// #2384
+public class TestTryCatchInIf2 extends IntegrationTest {
+	public static class TestCls {
+		public void test(Class<?> cls) {
+			Object obj = null;
+			if (cls != null) {
+				try {
+					obj = cls.getDeclaredConstructor().newInstance();
+				} catch (Exception e) {
+					System.out.println("error");
+				}
+			}
+			System.out.println("obj = " + obj);
+		}
+	}
+
+	@Test
+	public void test() {
+		// happens only without debug info and java version >= 10
+		noDebugInfo();
+		useTargetJavaVersion(10);
+		JadxAssertions.assertThat(getClassNode(TestCls.class))
+				.code();
+	}
+}