diff --git a/src/main/java/jadx/dex/visitors/regions/RegionMaker.java b/src/main/java/jadx/dex/visitors/regions/RegionMaker.java index 7e14a676..7dc9146e 100644 --- a/src/main/java/jadx/dex/visitors/regions/RegionMaker.java +++ b/src/main/java/jadx/dex/visitors/regions/RegionMaker.java @@ -124,17 +124,17 @@ public class RegionMaker { IfNode ifnode = null; LoopRegion loopRegion = null; Set exitBlocksSet = loop.getExitNodes(); + // set exit blocks scan order by priority - // this can help if we have several exit from loop (after using 'break' or 'return' in loop) + // this can help if loop have several exits (after using 'break' or 'return' in loop) List exitBlocks = new ArrayList(exitBlocksSet.size()); - if (exitBlocksSet.contains(loop.getEnd())) { + BlockNode nextStart = BlockUtils.getNextBlock(loopStart); + if (nextStart != null && exitBlocksSet.remove(nextStart)) + exitBlocks.add(nextStart); + if (exitBlocksSet.remove(loop.getEnd())) exitBlocks.add(loop.getEnd()); - exitBlocksSet.remove(loop.getEnd()); - } - if (exitBlocksSet.contains(loopStart)) { + if (exitBlocksSet.remove(loopStart)) exitBlocks.add(loopStart); - exitBlocksSet.remove(loopStart); - } exitBlocks.addAll(exitBlocksSet); exitBlocksSet = null; @@ -189,7 +189,11 @@ public class RegionMaker { stack.pop(); loopStart.getAttributes().add(loop); - return BlockUtils.getNextBlock(loop.getEnd()); + BlockNode next = BlockUtils.getNextBlock(loop.getEnd()); + if (!RegionUtils.isRegionContainsBlock(body, next)) + return next; + else + return null; } stack.push(loopRegion); @@ -372,6 +376,10 @@ public class RegionMaker { } } + if (BlockUtils.isBackEdge(block, out)) { + out = null; + } + if (stack.containsExit(elseBlock)) elseBlock = null; diff --git a/src/main/java/jadx/utils/BlockUtils.java b/src/main/java/jadx/utils/BlockUtils.java index 3aba03d6..acad20f2 100644 --- a/src/main/java/jadx/utils/BlockUtils.java +++ b/src/main/java/jadx/utils/BlockUtils.java @@ -48,6 +48,16 @@ public class BlockUtils { return ret; } + public static boolean isBackEdge(BlockNode from, BlockNode to) { + if (from.getCleanSuccessors().contains(to)) + return false; // already checked + + if (!from.getSuccessors().contains(to)) + return false; // not even successor + + return true; + } + /** * Remove exception handlers from block nodes bitset */ diff --git a/src/samples/java/jadx/samples/TestCF3.java b/src/samples/java/jadx/samples/TestCF3.java index 1be4fbfc..6a8943b1 100644 --- a/src/samples/java/jadx/samples/TestCF3.java +++ b/src/samples/java/jadx/samples/TestCF3.java @@ -18,6 +18,10 @@ public class TestCF3 extends AbstractTest { return 1; } + private int exc() throws Exception { + return 1; + } + public void testSwitchInLoop() throws Exception { while (true) { int n = next(); @@ -103,6 +107,31 @@ public class TestCF3 extends AbstractTest { return foundIt; } + public String testReturnInLoop(List list) { + Iterator it = list.iterator(); + while (it.hasNext()) { + String ver = it.next(); + if (ver != null) + return ver; + } + return "error"; + } + + public String testReturnInLoop2(List list) { + try { + Iterator it = list.iterator(); + while (it.hasNext()) { + String ver = it.next(); + exc(); + if (ver != null) + return ver; + } + } catch (Exception e) { + setEnabled(false); + } + return "error"; + } + @Override public boolean testRun() throws Exception { setEnabled(false); @@ -117,6 +146,17 @@ public class TestCF3 extends AbstractTest { new ArrayList(Arrays.asList("a1", "a2")), new ArrayList(Arrays.asList("a1", "b2")))); + List list1 = Arrays.asList(null, "a", "b"); + + // TODO this line required to omit generic information because it create List + // List list2 = Arrays.asList(null, null, null); + + assertEquals(testReturnInLoop(list1), "a"); + assertEquals(testReturnInLoop2(list1), "a"); + + // assertEquals(testReturnInLoop(list2), "error"); + // assertEquals(testReturnInLoop2(list2), "error"); + // assertTrue(testLabeledBreakContinue()); return true; }