diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/CheckRegions.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/CheckRegions.java index ab7b650c7..6009bf921 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/CheckRegions.java +++ b/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 regions = new ArrayList(); + 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); + } } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/ProcessTryCatchRegions.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/ProcessTryCatchRegions.java index 8ca9baf3d..bbbd2bbd9 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/ProcessTryCatchRegions.java +++ b/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); - } - } - - private final Map tryBlocksMap = new HashMap(2); - - public ProcessTryCatchRegions(MethodNode mth) { + public static void process(MethodNode mth) { if (mth.isNoCode() || mth.isNoExceptionHandlers()) { return; } - Set tryBlocks = new HashSet(); + final Map tryBlocksMap = new HashMap(2); + searchTryCatchDominators(mth, tryBlocksMap); + + 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); + } + } + } + + private static void searchTryCatchDominators(MethodNode mth, Map tryBlocksMap) { + final Set tryBlocks = new HashSet(); // 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 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 tryBlocksMap, IRegion region) { // search dominator blocks in this region (don't need to go deeper) - for (BlockNode dominator : tryBlocksMap.keySet()) { + for (Map.Entry 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; } - } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMaker.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMaker.java index 169c73026..5a993a5d9 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMaker.java +++ b/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); diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMakerVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMakerVisitor.java index f64e1fdbf..689efc198 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMakerVisitor.java +++ b/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() { diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/TracedRegionVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/TracedRegionVisitor.java index 3b6e961fc..404e19253 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/TracedRegionVisitor.java +++ b/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 regionStack = new ArrayDeque(); @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(); } } diff --git a/jadx-core/src/test/java/jadx/tests/internal/others/TestIfTryInCatch.java b/jadx-core/src/test/java/jadx/tests/internal/others/TestIfTryInCatch.java new file mode 100644 index 000000000..a1c00b059 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/internal/others/TestIfTryInCatch.java @@ -0,0 +1,60 @@ +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();")); + } +}