fix: process try/catch without move-exception instruction (#395)

jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockExceptionHandler.java

 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);
 		}
 	}
 }

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<Integer, BlockNode> 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<Integer, BlockNode> 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;
 			});

jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchNoMoveExc2.java

+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();"
+		));
+	}
+}

jadx-core/src/test/smali/trycatch/TestTryCatchNoMoveExc2.smali

+.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