diff --git a/jadx-core/src/main/java/jadx/core/dex/regions/TryCatchRegion.java b/jadx-core/src/main/java/jadx/core/dex/regions/TryCatchRegion.java index 84a38eaa9e742773337803827281c112a80a925d..5fa7486d6cdc8463588d2d2b40c064b841cd3348 100644 --- a/jadx-core/src/main/java/jadx/core/dex/regions/TryCatchRegion.java +++ b/jadx-core/src/main/java/jadx/core/dex/regions/TryCatchRegion.java @@ -84,8 +84,14 @@ public final class TryCatchRegion extends AbstractRegion implements IBranchRegio @Override public String toString() { - return "Try: " + tryRegion - + " catches: " + Utils.listToString(catchRegions.values()) - + (finallyRegion == null ? "" : " finally: " + finallyRegion); + StringBuilder sb = new StringBuilder(); + sb.append("Try: ").append(tryRegion); + if (!catchRegions.isEmpty()) { + sb.append(" catches: ").append(Utils.listToString(catchRegions.values())); + } + if (finallyRegion != null) { + sb.append(" finally: ").append(finallyRegion); + } + return sb.toString(); } } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockExceptionHandler.java b/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockExceptionHandler.java index e92be623d27050255d107850a439c51e9cd9cf79..2a60b3768828101d6b33539195968b726674ddb0 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockExceptionHandler.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockExceptionHandler.java @@ -65,37 +65,38 @@ public class BlockExceptionHandler extends AbstractVisitor { private static void processExceptionHandlers(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()) { - // remove 'monitor-exit' from exception handler blocks - 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); - } + if (handlerAttr == null) { + return; + } + ExceptionHandler excHandler = handlerAttr.getHandler(); + excHandler.addBlock(block); + for (BlockNode node : BlockUtils.collectBlocksDominatedBy(block, block)) { + excHandler.addBlock(node); + } + for (BlockNode excBlock : excHandler.getBlocks()) { + // remove 'monitor-exit' from exception handler blocks + 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(); + } + remover.perform(); - // if 'throw' in exception handler block have 'catch' - merge these catch blocks - for (InsnNode insn : excBlock.getInstructions()) { - CatchAttr catchAttr = insn.get(AType.CATCH_BLOCK); - if (catchAttr == null) { - continue; - } - if (insn.getType() == InsnType.THROW - || onlyAllHandler(catchAttr.getTryBlock())) { - TryCatchBlock handlerBlock = handlerAttr.getTryBlock(); - TryCatchBlock catchBlock = catchAttr.getTryBlock(); - handlerBlock.merge(mth, catchBlock); - } + // if 'throw' in exception handler block have 'catch' - merge these catch blocks + for (InsnNode insn : excBlock.getInstructions()) { + CatchAttr catchAttr = insn.get(AType.CATCH_BLOCK); + if (catchAttr == null) { + continue; + } + if (insn.getType() == InsnType.THROW + || onlyAllHandler(catchAttr.getTryBlock())) { + TryCatchBlock handlerBlock = handlerAttr.getTryBlock(); + TryCatchBlock catchBlock = catchAttr.getTryBlock(); + handlerBlock.merge(mth, catchBlock); } } } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockFinish.java b/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockFinish.java index 6db4ff758b921b2e792ae371f95ecccda9c6fcba..bbbe26db2ec302889a87f2bed7449e592a3d2304 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockFinish.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockFinish.java @@ -1,16 +1,27 @@ package jadx.core.dex.visitors.blocksmaker; +import jadx.core.dex.attributes.AType; import jadx.core.dex.instructions.IfNode; import jadx.core.dex.instructions.InsnType; import jadx.core.dex.nodes.BlockNode; import jadx.core.dex.nodes.InsnNode; import jadx.core.dex.nodes.MethodNode; +import jadx.core.dex.trycatch.ExcHandlerAttr; +import jadx.core.dex.trycatch.SplitterBlockAttr; import jadx.core.dex.visitors.AbstractVisitor; +import jadx.core.utils.BlockUtils; +import java.util.HashMap; import java.util.List; +import java.util.Map; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class BlockFinish extends AbstractVisitor { + private static final Logger LOG = LoggerFactory.getLogger(BlockFinish.class); + @Override public void visit(MethodNode mth) { if (mth.isNoCode()) { @@ -20,6 +31,7 @@ public class BlockFinish extends AbstractVisitor { for (BlockNode block : mth.getBasicBlocks()) { block.updateCleanSuccessors(); initBlocksInIfNodes(block); + fixSplitterBlock(block); } mth.finishBasicBlocks(); @@ -37,4 +49,47 @@ public class BlockFinish extends AbstractVisitor { } } } + + /** + * For evey exception handler must be only one splitter block, + * select correct one and remove others if necessary. + */ + private static void fixSplitterBlock(BlockNode block) { + ExcHandlerAttr excHandlerAttr = block.get(AType.EXC_HANDLER); + if (excHandlerAttr == null) { + return; + } + BlockNode handlerBlock = excHandlerAttr.getHandler().getHandlerBlock(); + if (handlerBlock.getPredecessors().size() < 2) { + return; + } + Map splitters = new HashMap(); + for (BlockNode pred : handlerBlock.getPredecessors()) { + pred = BlockUtils.skipSyntheticPredecessor(pred); + SplitterBlockAttr splitterAttr = pred.get(AType.SPLITTER_BLOCK); + if (splitterAttr != null && pred == splitterAttr.getBlock()) { + splitters.put(pred, splitterAttr); + } + } + if (splitters.size() < 2) { + return; + } + BlockNode topSplitter = BlockUtils.getTopBlock(splitters.keySet()); + if (topSplitter == null) { + LOG.warn("Unknown top splitter block from list: {}", splitters); + return; + } + for (Map.Entry entry : splitters.entrySet()) { + BlockNode pred = entry.getKey(); + SplitterBlockAttr splitterAttr = entry.getValue(); + if (pred == topSplitter) { + block.addAttr(splitterAttr); + } else { + pred.remove(AType.SPLITTER_BLOCK); + for (BlockNode s : pred.getCleanSuccessors()) { + s.remove(AType.SPLITTER_BLOCK); + } + } + } + } } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockSplitter.java b/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockSplitter.java index b65ae22ddf1a453464f6df969694140e730aece9..fcd6da704d79147c1c5efbb3e00d71ecf11f1493 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockSplitter.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockSplitter.java @@ -151,26 +151,30 @@ public class BlockSplitter extends AbstractVisitor { BlockNode thisBlock = getBlock(jump.getDest(), blocksMap); connect(srcBlock, thisBlock); } + connectExceptionHandlers(blocksMap, block, insn); + } + } + } - // connect exception handlers - CatchAttr catches = insn.get(AType.CATCH_BLOCK); - // get synthetic block for handlers - SplitterBlockAttr spl = block.get(AType.SPLITTER_BLOCK); - if (catches != null && spl != null) { - BlockNode splitterBlock = spl.getBlock(); - boolean tryEnd = insn.contains(AFlag.TRY_LEAVE); - for (ExceptionHandler h : catches.getTryBlock().getHandlers()) { - BlockNode handlerBlock = getBlock(h.getHandleOffset(), blocksMap); - // skip self loop in handler - if (splitterBlock != handlerBlock) { - handlerBlock.addAttr(spl); - connect(splitterBlock, handlerBlock); - } - if (tryEnd) { - connect(block, handlerBlock); - } - } + private static void connectExceptionHandlers(Map blocksMap, BlockNode block, InsnNode insn) { + CatchAttr catches = insn.get(AType.CATCH_BLOCK); + SplitterBlockAttr spl = block.get(AType.SPLITTER_BLOCK); + if (catches == null || spl == null) { + return; + } + BlockNode splitterBlock = spl.getBlock(); + boolean tryEnd = insn.contains(AFlag.TRY_LEAVE); + for (ExceptionHandler h : catches.getTryBlock().getHandlers()) { + BlockNode handlerBlock = getBlock(h.getHandleOffset(), blocksMap); + // skip self loop in handler + if (splitterBlock != handlerBlock) { + if (!handlerBlock.contains(AType.SPLITTER_BLOCK)) { + handlerBlock.addAttr(spl); } + connect(splitterBlock, handlerBlock); + } + if (tryEnd) { + connect(block, handlerBlock); } } } 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 634e89da406d606109ac1a8b0cea586290b1219c..da018c3e6044f26b6457ff2158d77028e7a1c97a 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 @@ -12,6 +12,7 @@ import jadx.core.dex.regions.TryCatchRegion; import jadx.core.dex.regions.loops.LoopRegion; import jadx.core.dex.trycatch.CatchAttr; import jadx.core.dex.trycatch.ExceptionHandler; +import jadx.core.dex.trycatch.SplitterBlockAttr; import jadx.core.dex.trycatch.TryCatchBlock; import jadx.core.utils.BlockUtils; import jadx.core.utils.ErrorsCounter; @@ -65,39 +66,28 @@ public class ProcessTryCatchRegions extends AbstractRegionVisitor { // for each try block search nearest dominator block for (TryCatchBlock tb : tryBlocks) { - BitSet bs = null; - // build bitset with dominators of blocks covered with this try/catch block - for (BlockNode block : mth.getBasicBlocks()) { - CatchAttr c = block.get(AType.CATCH_BLOCK); - if (c != null && c.getTryBlock() == tb) { - if (bs == null) { - bs = (BitSet) block.getDoms().clone(); - } else { - bs.and(block.getDoms()); - } + BitSet bs = new BitSet(mth.getBasicBlocks().size()); + for (ExceptionHandler excHandler : tb.getHandlers()) { + SplitterBlockAttr splitter = excHandler.getHandlerBlock().get(AType.SPLITTER_BLOCK); + if (splitter != null) { + BlockNode block = splitter.getBlock(); + bs.set(block.getId()); } } - if (bs == null) { - LOG.debug(" Can't build try/catch dominators bitset, tb: {}, mth: {} ", tb, mth); - continue; - } - - // intersect to get dominator of dominators List domBlocks = BlockUtils.bitSetToBlocks(mth, bs); - for (BlockNode block : domBlocks) { - bs.andNot(block.getDoms()); - } - domBlocks = BlockUtils.bitSetToBlocks(mth, bs); + BlockNode domBlock; if (domBlocks.size() != 1) { - throw new JadxRuntimeException( - "Exception block dominator not found, method:" + mth + ". bs: " + bs); + domBlock = BlockUtils.getTopBlock(domBlocks); + if (domBlock == null) { + throw new JadxRuntimeException( + "Exception block dominator not found, method:" + mth + ". bs: " + domBlocks); + } + } else { + domBlock = domBlocks.get(0); } - - BlockNode domBlock = domBlocks.get(0); - TryCatchBlock prevTB = tryBlocksMap.put(domBlock, tb); if (prevTB != null) { - LOG.info("!!! TODO: merge try blocks in {}", mth); + ErrorsCounter.methodError(mth, "Failed to process nested try/catch"); } } } @@ -136,7 +126,7 @@ public class ProcessTryCatchRegions extends AbstractRegionVisitor { Region tryRegion = new Region(replaceRegion); List subBlocks = replaceRegion.getSubBlocks(); for (IContainer cont : subBlocks) { - if (RegionUtils.isDominatedBy(dominator, cont)) { + if (RegionUtils.hasPathThroughBlock(dominator, cont)) { if (isHandlerPath(tb, cont)) { break; } @@ -170,7 +160,7 @@ public class ProcessTryCatchRegions extends AbstractRegionVisitor { private static boolean isHandlerPath(TryCatchBlock tb, IContainer cont) { for (ExceptionHandler h : tb.getHandlers()) { - if (RegionUtils.hasPathThruBlock(h.getHandlerBlock(), cont)) { + if (RegionUtils.hasPathThroughBlock(h.getHandlerBlock(), cont)) { return true; } } 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 011b254eed0a498d01fd03536db6af4fa0045e61..062d5ad8322a149285dc2ae102cfa34b700b1e86 100644 --- a/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java @@ -19,6 +19,7 @@ import jadx.core.utils.exceptions.JadxRuntimeException; import java.util.ArrayList; import java.util.BitSet; +import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.LinkedList; @@ -342,6 +343,25 @@ public class BlockUtils { return traverseSuccessorsUntil(start, end, new BitSet()); } + public static BlockNode getTopBlock(Collection blocks) { + if (blocks.size() == 1) { + return blocks.iterator().next(); + } + for (BlockNode from : blocks) { + boolean top = true; + for (BlockNode to : blocks) { + if (from != to && !isPathExists(from, to)) { + top = false; + break; + } + } + if (top) { + return from; + } + } + return null; + } + public static boolean isOnlyOnePathExists(BlockNode start, BlockNode end) { if (start == end) { return true; diff --git a/jadx-core/src/main/java/jadx/core/utils/DebugUtils.java b/jadx-core/src/main/java/jadx/core/utils/DebugUtils.java index 2df200bda2903aaaed8e7b0259f098d5138f0f2f..c273b6da72cfef106a7b9c5606f327ea3bda45c8 100644 --- a/jadx-core/src/main/java/jadx/core/utils/DebugUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/DebugUtils.java @@ -59,6 +59,10 @@ public class DebugUtils { printRegions(mth, false); } + public static void printRegion(MethodNode mth, IRegion region, boolean printInsn) { + printRegion(mth, region, "", printInsn); + } + public static void printRegions(MethodNode mth, boolean printInsns) { LOG.debug("|{}", mth.toString()); printRegion(mth, mth.getRegion(), "| ", printInsns); diff --git a/jadx-core/src/main/java/jadx/core/utils/RegionUtils.java b/jadx-core/src/main/java/jadx/core/utils/RegionUtils.java index 2ebdb32e34e7bcbfc3d0cb8b8899d9314ec6a9fa..64d337c28a102f8d54afffd884cef6384cbe1b1a 100644 --- a/jadx-core/src/main/java/jadx/core/utils/RegionUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/RegionUtils.java @@ -271,7 +271,7 @@ public class RegionUtils { } } - public static boolean hasPathThruBlock(BlockNode block, IContainer cont) { + public static boolean hasPathThroughBlock(BlockNode block, IContainer cont) { if (block == cont) { return true; } @@ -282,7 +282,7 @@ public class RegionUtils { } else if (cont instanceof IRegion) { IRegion region = (IRegion) cont; for (IContainer c : region.getSubBlocks()) { - if (!hasPathThruBlock(block, c)) { + if (!hasPathThroughBlock(block, c)) { return false; } } diff --git a/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchFinally4.java b/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchFinally4.java new file mode 100644 index 0000000000000000000000000000000000000000..c344430dc80b5605c6fe1856d1ae419c7e9eba62 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchFinally4.java @@ -0,0 +1,46 @@ +package jadx.tests.integration.trycatch; + +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.IntegrationTest; + +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.OutputStream; + +import org.junit.Test; + +import static jadx.tests.api.utils.JadxMatchers.containsOne; +import static org.junit.Assert.assertThat; + +public class TestTryCatchFinally4 extends IntegrationTest { + + public static class TestCls { + public void test() throws IOException { + File file = File.createTempFile("test", "txt"); + OutputStream outputStream = new FileOutputStream(file); + try { + outputStream.write(1); + } finally { + try { + outputStream.close(); + file.delete(); + } catch (IOException e) { + } + } + } + } + + @Test + public void test() { + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + + assertThat(code, containsOne("File file = File.createTempFile(\"test\", \"txt\");")); + assertThat(code, containsOne("OutputStream outputStream = new FileOutputStream(file);")); + assertThat(code, containsOne("outputStream.write(1);")); + assertThat(code, containsOne("} finally {")); + assertThat(code, containsOne("outputStream.close();")); + assertThat(code, containsOne("} catch (IOException e) {")); + } +}