From ef85e29a9b5ff336f1ef20bebf486b73bb3e1e34 Mon Sep 17 00:00:00 2001 From: Skylot Date: Fri, 7 Nov 2014 23:03:32 +0300 Subject: [PATCH] core: improve 'break' and 'continue' insertion --- .../dex/visitors/regions/RegionMaker.java | 115 +++++++++++------- .../main/java/jadx/core/utils/BlockUtils.java | 23 ---- .../loops/TestContinueInLoop2.java | 73 +++++++++++ 3 files changed, 147 insertions(+), 64 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/loops/TestContinueInLoop2.java 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 2b3dbcfb9..d32440c65 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 @@ -167,7 +167,7 @@ public class RegionMaker { LoopRegion loopRegion = makeLoopRegion(curRegion, loop, exitBlocks); if (loopRegion == null) { BlockNode exit = makeEndlessLoop(curRegion, stack, loop, loopStart); - insertContinueInsns(loop); + insertContinue(loop); return exit; } curRegion.getSubBlocks().add(loopRegion); @@ -192,7 +192,7 @@ public class RegionMaker { if (!exitBlocks.contains(exitEdge.getSource())) { continue; } - tryInsertBreak(stack, loopExit, exitEdge); + insertBreak(stack, loopExit, exitEdge); } } } @@ -235,7 +235,7 @@ public class RegionMaker { loopRegion.setBody(body); } stack.pop(); - insertContinueInsns(loop); + insertContinue(loop); return out; } @@ -305,7 +305,7 @@ public class RegionMaker { List exitEdges = loop.getExitEdges(); for (Edge exitEdge : exitEdges) { BlockNode exit = exitEdge.getTarget(); - if (tryInsertBreak(stack, exit, exitEdge)) { + if (insertBreak(stack, exit, exitEdge)) { BlockNode nextBlock = getNextBlock(exit); if (nextBlock != null) { stack.addExit(nextBlock); @@ -351,7 +351,7 @@ public class RegionMaker { return true; } - private boolean tryInsertBreak(RegionStack stack, BlockNode loopExit, Edge exitEdge) { + private boolean insertBreak(RegionStack stack, BlockNode loopExit, Edge exitEdge) { BlockNode exit = exitEdge.getTarget(); BlockNode insertBlock = null; boolean confirm = false; @@ -393,54 +393,87 @@ public class RegionMaker { insertBlock.getInstructions().add(breakInsn); stack.addExit(exit); // add label to 'break' if needed - List loops = mth.getAllLoopsForBlock(exitEdge.getSource()); - if (loops.size() >= 2) { - // find parent loop - for (LoopInfo loop : loops) { - LoopInfo parentLoop = loop.getParentLoop(); - if (parentLoop == null - && loop.getEnd() != exit - && !loop.getExitNodes().contains(exit)) { - LoopLabelAttr labelAttr = new LoopLabelAttr(loop); - breakInsn.addAttr(labelAttr); - loop.getStart().addAttr(labelAttr); - break; - } - } - } + addBreakLabel(exitEdge, exit, breakInsn); return true; } - private static void insertContinueInsns(LoopInfo loop) { + private void addBreakLabel(Edge exitEdge, BlockNode exit, InsnNode breakInsn) { + BlockNode outBlock = BlockUtils.getNextBlock(exitEdge.getTarget()); + if (outBlock == null) { + return; + } + List exitLoop = mth.getAllLoopsForBlock(outBlock); + if (!exitLoop.isEmpty()) { + return; + } + List inLoops = mth.getAllLoopsForBlock(exitEdge.getSource()); + if (inLoops.size() < 2) { + return; + } + // search for parent loop + LoopInfo parentLoop = null; + for (LoopInfo loop : inLoops) { + if (loop.getParentLoop() == null) { + parentLoop = loop; + break; + } + } + if (parentLoop == null) { + return; + } + if (parentLoop.getEnd() != exit && !parentLoop.getExitNodes().contains(exit)) { + LoopLabelAttr labelAttr = new LoopLabelAttr(parentLoop); + breakInsn.addAttr(labelAttr); + parentLoop.getStart().addAttr(labelAttr); + } + } + + private static void insertContinue(LoopInfo loop) { BlockNode loopEnd = loop.getEnd(); List predecessors = loopEnd.getPredecessors(); if (predecessors.size() <= 1) { return; } + Set loopExitNodes = loop.getExitNodes(); for (BlockNode pred : predecessors) { - if (!pred.contains(AFlag.SYNTHETIC) - || BlockUtils.checkLastInsnType(pred, InsnType.CONTINUE)) { - continue; - } - List nodes = pred.getPredecessors(); - if (nodes.isEmpty()) { - continue; - } - BlockNode codePred = nodes.get(0); - if (codePred.contains(AFlag.SKIP)) { - continue; - } - if (!isDominatedOnBlocks(codePred, predecessors)) { - for (BlockNode blockNode : predecessors) { - if (blockNode != pred && BlockUtils.isPathExists(codePred, blockNode)) { - InsnNode cont = new InsnNode(InsnType.CONTINUE, 0); - pred.getInstructions().add(cont); - } - } + if (canInsertContinue(pred, predecessors, loopEnd, loopExitNodes)) { + InsnNode cont = new InsnNode(InsnType.CONTINUE, 0); + pred.getInstructions().add(cont); } } } + private static boolean canInsertContinue(BlockNode pred, List predecessors, BlockNode loopEnd, + Set loopExitNodes) { + if (!pred.contains(AFlag.SYNTHETIC) + || BlockUtils.checkLastInsnType(pred, InsnType.CONTINUE)) { + return false; + } + List preds = pred.getPredecessors(); + if (preds.isEmpty()) { + return false; + } + BlockNode codePred = preds.get(0); + if (codePred.contains(AFlag.SKIP)) { + return false; + } + if (loopEnd.isDominator(codePred) + || loopExitNodes.contains(codePred)) { + return false; + } + if (isDominatedOnBlocks(codePred, predecessors)) { + return false; + } + boolean gotoExit = false; + for (BlockNode exit : loopExitNodes) { + if (BlockUtils.isPathExists(codePred, exit)) { + gotoExit = true; + break; + } + } + return gotoExit; + } + private static boolean isDominatedOnBlocks(BlockNode dom, List blocks) { for (BlockNode node : blocks) { if (!node.isDominator(dom)) { @@ -601,7 +634,7 @@ public class RegionMaker { } Map> blocksMap = new LinkedHashMap>(len); - for (Entry> entry : casesMap.entrySet()) { + for (Map.Entry> entry : casesMap.entrySet()) { BlockNode c = getBlockByOffset(entry.getKey(), block.getSuccessors()); assert c != null; blocksMap.put(c, entry.getValue()); 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 c05632d04..1dc1dd862 100644 --- a/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java @@ -310,29 +310,6 @@ public class BlockUtils { return traverseSuccessorsUntil(start, end, new BitSet()); } - public static boolean isCleanPathExists(BlockNode start, BlockNode end) { - if (start == end || start.getCleanSuccessors().contains(end)) { - return true; - } - return traverseCleanSuccessorsUntil(start, end, new BitSet()); - } - - private static boolean traverseCleanSuccessorsUntil(BlockNode from, BlockNode until, BitSet visited) { - for (BlockNode s : from.getCleanSuccessors()) { - if (s == until) { - return true; - } - int id = s.getId(); - if (!visited.get(id) && !s.contains(AType.EXC_HANDLER)) { - visited.set(id); - if (traverseSuccessorsUntil(s, until, visited)) { - return true; - } - } - } - return false; - } - public static boolean isOnlyOnePathExists(BlockNode start, BlockNode end) { if (start == end) { return true; diff --git a/jadx-core/src/test/java/jadx/tests/integration/loops/TestContinueInLoop2.java b/jadx-core/src/test/java/jadx/tests/integration/loops/TestContinueInLoop2.java new file mode 100644 index 000000000..c73e3ee65 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/loops/TestContinueInLoop2.java @@ -0,0 +1,73 @@ +package jadx.tests.integration.loops; + +import jadx.core.dex.attributes.AType; +import jadx.core.dex.instructions.InsnType; +import jadx.core.dex.nodes.BlockNode; +import jadx.core.dex.nodes.ClassNode; +import jadx.core.dex.nodes.InsnNode; +import jadx.core.dex.nodes.MethodNode; +import jadx.core.dex.trycatch.CatchAttr; +import jadx.core.dex.trycatch.ExcHandlerAttr; +import jadx.core.dex.trycatch.ExceptionHandler; +import jadx.core.dex.trycatch.TryCatchBlock; +import jadx.core.utils.BlockUtils; +import jadx.core.utils.InstructionRemover; +import jadx.tests.api.IntegrationTest; + +import org.junit.Test; + +import static jadx.tests.api.utils.JadxMatchers.containsOne; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.not; +import static org.junit.Assert.assertThat; + +public class TestContinueInLoop2 extends IntegrationTest { + + public static class TestCls { + private static void test(MethodNode mth, BlockNode block) { + ExcHandlerAttr handlerAttr = block.get(AType.EXC_HANDLER); + if (handlerAttr != null) { + ExceptionHandler excHandler = handlerAttr.getHandler(); + excHandler.addBlock(block); + for (BlockNode node : BlockUtils.collectBlocksDominatedBy(block, block)) { + excHandler.addBlock(node); + } + for (BlockNode excBlock : excHandler.getBlocks()) { + InstructionRemover remover = new InstructionRemover(mth, excBlock); + for (InsnNode insn : excBlock.getInstructions()) { + if (insn.getType() == InsnType.MONITOR_ENTER) { + break; + } + if (insn.getType() == InsnType.MONITOR_EXIT) { + remover.add(insn); + } + } + remover.perform(); + + for (InsnNode insn : excBlock.getInstructions()) { + if (insn.getType() == InsnType.THROW) { + CatchAttr catchAttr = insn.get(AType.CATCH_BLOCK); + if (catchAttr != null) { + TryCatchBlock handlerBlock = handlerAttr.getTryBlock(); + TryCatchBlock catchBlock = catchAttr.getTryBlock(); + if (handlerBlock != catchBlock) { + handlerBlock.merge(mth, catchBlock); + catchBlock.removeInsn(insn); + } + } + } + } + } + } + } + } + + @Test + public void test() { + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + + assertThat(code, containsOne("break;")); + assertThat(code, not(containsString("continue;"))); + } +}