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 5aff40cc2..9d37b769b 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 @@ -1,8 +1,5 @@ package jadx.core.dex.visitors.blocksmaker; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import jadx.core.dex.attributes.AFlag; import jadx.core.dex.attributes.AType; import jadx.core.dex.instructions.InsnType; @@ -23,8 +20,6 @@ import jadx.core.utils.InstructionRemover; public class BlockExceptionHandler extends AbstractVisitor { - private static final Logger LOG = LoggerFactory.getLogger(BlockExceptionHandler.class); - @Override public void visit(MethodNode mth) { if (mth.isNoCode()) { @@ -40,7 +35,7 @@ public class BlockExceptionHandler extends AbstractVisitor { processExceptionHandlers(mth, block); } for (BlockNode block : mth.getBasicBlocks()) { - processTryCatchBlocks(mth, block); + processTryCatchBlocks(block); } } @@ -48,27 +43,26 @@ public class BlockExceptionHandler extends AbstractVisitor { * Set exception handler attribute for whole block */ private static void markExceptionHandlers(BlockNode block) { - if (block.getInstructions().isEmpty()) { - return; - } - InsnNode me = block.getInstructions().get(0); - ExcHandlerAttr handlerAttr = me.get(AType.EXC_HANDLER); + ExcHandlerAttr handlerAttr = block.get(AType.EXC_HANDLER); if (handlerAttr == null) { return; } ExceptionHandler excHandler = handlerAttr.getHandler(); - block.addAttr(handlerAttr); ArgType argType = excHandler.isCatchAll() ? ArgType.THROWABLE : excHandler.getCatchType().getType(); - if (me.getType() == InsnType.MOVE_EXCEPTION) { - // set correct type for 'move-exception' operation - RegisterArg resArg = InsnArg.reg(me.getResult().getRegNum(), argType); - me.setResult(resArg); - me.add(AFlag.DONT_INLINE); - excHandler.setArg(resArg); - } else { - // handler arguments not used - excHandler.setArg(new NamedArg("unused", argType)); + if (!block.getInstructions().isEmpty()) { + InsnNode me = block.getInstructions().get(0); + if (me.getType() == InsnType.MOVE_EXCEPTION) { + // set correct type for 'move-exception' operation + RegisterArg resArg = InsnArg.reg(me.getResult().getRegNum(), argType); + resArg.copyAttributesFrom(me); + me.setResult(resArg); + me.add(AFlag.DONT_INLINE); + excHandler.setArg(resArg); + return; + } } + // handler arguments not used + excHandler.setArg(new NamedArg("unused", argType)); } private static void processExceptionHandlers(MethodNode mth, BlockNode block) { @@ -113,9 +107,7 @@ public class BlockExceptionHandler extends AbstractVisitor { private static boolean onlyAllHandler(TryCatchBlock tryBlock) { if (tryBlock.getHandlersCount() == 1) { ExceptionHandler eh = tryBlock.getHandlers().iterator().next(); - if (eh.isCatchAll() || eh.isFinally()) { - return true; - } + return eh.isCatchAll() || eh.isFinally(); } return false; } @@ -123,7 +115,7 @@ public class BlockExceptionHandler extends AbstractVisitor { /** * If all instructions in block have same 'catch' attribute mark it as 'TryCatch' block. */ - private static void processTryCatchBlocks(MethodNode mth, BlockNode block) { + private static void processTryCatchBlocks(BlockNode block) { CatchAttr commonCatchAttr = null; for (InsnNode insn : block.getInstructions()) { CatchAttr catchAttr = insn.get(AType.CATCH_BLOCK); @@ -139,24 +131,6 @@ public class BlockExceptionHandler extends AbstractVisitor { } if (commonCatchAttr != null) { block.addAttr(commonCatchAttr); - // connect handler to block - for (ExceptionHandler handler : commonCatchAttr.getTryBlock().getHandlers()) { - connectHandler(mth, handler); - } - } - } - - private static void connectHandler(MethodNode mth, ExceptionHandler handler) { - int addr = handler.getHandleOffset(); - for (BlockNode block : mth.getBasicBlocks()) { - ExcHandlerAttr bh = block.get(AType.EXC_HANDLER); - if (bh != null && bh.getHandler().getHandleOffset() == addr) { - handler.setHandlerBlock(block); - break; - } - } - if (handler.getHandlerBlock() == null) { - LOG.warn("Exception handler block not set for {}, mth: {}", handler, mth); } } } 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 1007f8c7e..d800a0f47 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 @@ -16,6 +16,7 @@ import jadx.core.dex.nodes.BlockNode; 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.SplitterBlockAttr; import jadx.core.dex.visitors.AbstractVisitor; @@ -81,11 +82,11 @@ public class BlockSplitter extends AbstractVisitor { if (type == InsnType.RETURN || type == InsnType.THROW) { mth.addExitBlock(curBlock); } - BlockNode block = startNewBlock(mth, insn.getOffset()); + BlockNode newBlock = startNewBlock(mth, insn.getOffset()); if (type == InsnType.MONITOR_ENTER || type == InsnType.MONITOR_EXIT) { - connect(curBlock, block); + connect(curBlock, newBlock); } - curBlock = block; + curBlock = newBlock; startNew = true; } else { startNew = isSplitByJump(prevInsn, insn) @@ -95,40 +96,79 @@ public class BlockSplitter extends AbstractVisitor { || prevInsn.contains(AFlag.TRY_LEAVE) || prevInsn.getType() == InsnType.MOVE_EXCEPTION; if (startNew) { - BlockNode block = startNewBlock(mth, insn.getOffset()); - connect(curBlock, block); - curBlock = block; + curBlock = connectNewBlock(mth, curBlock, insn.getOffset()); } } } - // for try/catch make empty block for connect handlers if (insn.contains(AFlag.TRY_ENTER)) { - BlockNode block; - if (insn.getOffset() != 0 && !startNew) { - block = startNewBlock(mth, insn.getOffset()); - connect(curBlock, block); - curBlock = block; - } + curBlock = insertSplitterBlock(mth, blocksMap, curBlock, insn, startNew); + } else if (insn.contains(AType.EXC_HANDLER)) { + processExceptionHandler(mth, curBlock, insn); blocksMap.put(insn.getOffset(), curBlock); - - // add this insn in new block - block = startNewBlock(mth, -1); - curBlock.add(AFlag.SYNTHETIC); - SplitterBlockAttr splitter = new SplitterBlockAttr(curBlock); - block.addAttr(splitter); - curBlock.addAttr(splitter); - connect(curBlock, block); - curBlock = block; + curBlock.getInstructions().add(insn); } else { blocksMap.put(insn.getOffset(), curBlock); + curBlock.getInstructions().add(insn); } - curBlock.getInstructions().add(insn); prevInsn = insn; } // setup missing connections setupConnections(mth, blocksMap); } + /** + * Make separate block for exception handler. New block already added if MOVE_EXCEPTION insn exists. + * Also link ExceptionHandler with current block. + */ + private static void processExceptionHandler(MethodNode mth, BlockNode curBlock, InsnNode insn) { + ExcHandlerAttr excHandlerAttr = insn.get(AType.EXC_HANDLER); + insn.remove(AType.EXC_HANDLER); + + BlockNode excHandlerBlock; + if (insn.getType() == InsnType.MOVE_EXCEPTION) { + excHandlerBlock = curBlock; + } else { + BlockNode newBlock = startNewBlock(mth, -1); + newBlock.add(AFlag.SYNTHETIC); + connect(newBlock, curBlock); + + excHandlerBlock = newBlock; + } + excHandlerBlock.addAttr(excHandlerAttr); + excHandlerAttr.getHandler().setHandlerBlock(excHandlerBlock); + } + + /** + * For try/catch make empty (splitter) block for connect handlers + */ + private static BlockNode insertSplitterBlock(MethodNode mth, Map blocksMap, + BlockNode curBlock, InsnNode insn, boolean startNew) { + BlockNode splitterBlock; + if (insn.getOffset() == 0 || startNew) { + splitterBlock = curBlock; + } else { + splitterBlock = connectNewBlock(mth, curBlock, insn.getOffset()); + } + blocksMap.put(insn.getOffset(), splitterBlock); + + SplitterBlockAttr splitterAttr = new SplitterBlockAttr(splitterBlock); + splitterBlock.add(AFlag.SYNTHETIC); + splitterBlock.addAttr(splitterAttr); + + // add this insn in new block + BlockNode newBlock = startNewBlock(mth, -1); + newBlock.getInstructions().add(insn); + newBlock.addAttr(splitterAttr); + connect(splitterBlock, newBlock); + return newBlock; + } + + private static BlockNode connectNewBlock(MethodNode mth, BlockNode curBlock, int offset) { + BlockNode block = startNewBlock(mth, offset); + connect(curBlock, block); + return block; + } + static BlockNode startNewBlock(MethodNode mth, int offset) { BlockNode block = new BlockNode(mth.getBasicBlocks().size(), offset); mth.getBasicBlocks().add(block); @@ -183,12 +223,12 @@ public class BlockSplitter extends AbstractVisitor { BlockNode thisBlock = getBlock(jump.getDest(), blocksMap); connect(srcBlock, thisBlock); } - connectExceptionHandlers(blocksMap, block, insn); + connectExceptionHandlers(block, insn); } } } - private static void connectExceptionHandlers(Map blocksMap, BlockNode block, InsnNode insn) { + private static void connectExceptionHandlers(BlockNode block, InsnNode insn) { CatchAttr catches = insn.get(AType.CATCH_BLOCK); SplitterBlockAttr spl = block.get(AType.SPLITTER_BLOCK); if (catches == null || spl == null) { @@ -197,7 +237,7 @@ public class BlockSplitter extends AbstractVisitor { BlockNode splitterBlock = spl.getBlock(); boolean tryEnd = insn.contains(AFlag.TRY_LEAVE); for (ExceptionHandler h : catches.getTryBlock().getHandlers()) { - BlockNode handlerBlock = getBlock(h.getHandleOffset(), blocksMap); + BlockNode handlerBlock = h.getHandlerBlock(); // skip self loop in handler if (splitterBlock != handlerBlock) { if (!handlerBlock.contains(AType.SPLITTER_BLOCK)) { @@ -248,6 +288,9 @@ public class BlockSplitter extends AbstractVisitor { private static void removeInsns(MethodNode mth) { for (BlockNode block : mth.getBasicBlocks()) { block.getInstructions().removeIf(insn -> { + if (!insn.isAttrStorageEmpty()) { + return false; + } InsnType insnType = insn.getType(); return insnType == InsnType.GOTO || insnType == InsnType.NOP; }); diff --git a/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchNoMoveExc2.java b/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchNoMoveExc2.java new file mode 100644 index 000000000..fad6bb631 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchNoMoveExc2.java @@ -0,0 +1,39 @@ +package jadx.tests.integration.trycatch; + +import org.junit.Test; + +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.SmaliTest; + +import static jadx.tests.api.utils.JadxMatchers.containsLines; +import static jadx.tests.api.utils.JadxMatchers.containsOne; +import static org.junit.Assert.assertThat; + +/** + * Issue: https://github.com/skylot/jadx/issues/395 + */ +public class TestTryCatchNoMoveExc2 extends SmaliTest { + +// private static void test(AutoCloseable closeable) { +// if (closeable != null) { +// try { +// closeable.close(); +// } catch (Exception unused) { +// } +// System.nanoTime(); +// } +// } + + @Test + public void test() { + ClassNode cls = getClassNodeFromSmaliWithPath("trycatch", "TestTryCatchNoMoveExc2"); + String code = cls.getCode().toString(); + + assertThat(code, containsOne("try {")); + assertThat(code, containsLines(2, + "} catch (Exception unused) {", + "}", + "System.nanoTime();" + )); + } +} diff --git a/jadx-core/src/test/smali/trycatch/TestTryCatchNoMoveExc2.smali b/jadx-core/src/test/smali/trycatch/TestTryCatchNoMoveExc2.smali new file mode 100644 index 000000000..4d2b9f836 --- /dev/null +++ b/jadx-core/src/test/smali/trycatch/TestTryCatchNoMoveExc2.smali @@ -0,0 +1,16 @@ +.class public LTestTryCatchNoMoveExc2; +.super Ljava/lang/Object; + +.method private static test(Ljava/lang/AutoCloseable;)V + .locals 0 + + :try_start_0 + invoke-interface {p0}, Ljava/lang/AutoCloseable;->close()V + :try_end_0 + .catch Ljava/lang/Exception; {:try_start_0 .. :try_end_0} :catch_0 + + :catch_0 + invoke-static {}, Ljava/lang/System;->nanoTime()J + + return-void +.end method