diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/maker/ExcHandlersRegionMaker.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/maker/ExcHandlersRegionMaker.java index 8d74dc0a5..c289d6f81 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/maker/ExcHandlersRegionMaker.java +++ b/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 handlerExits = BlockUtils.bitSetToBlocks(mth, domFrontier); + List 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)) diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/maker/IfRegionMaker.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/maker/IfRegionMaker.java index 63fe63986..b682084df 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/maker/IfRegionMaker.java +++ b/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 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(); diff --git a/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java b/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java index 6e491bc45..789f6a69f 100644 --- a/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java +++ b/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. + *
+ * 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 allHandlers = handler.getTryBlock().getHandlers(); + List 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 blocks, AFlag flag) { for (BlockNode block : blocks) { diff --git a/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestIfInTryCatch.java b/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestIfInTryCatch.java new file mode 100644 index 000000000..db67be800 --- /dev/null +++ b/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(); + } +} diff --git a/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchInIf2.java b/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchInIf2.java new file mode 100644 index 000000000..de8bad72c --- /dev/null +++ b/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(); + } +}