fix: improve exception handlers checks (#2086)

skylot · Jul 27, 2024 · 04a4540 · 04a4540
1 parent 33bcbfe
commit 04a4540
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 11 deletions.
diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/BlockNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/BlockNode.java
@@ -226,6 +226,10 @@ public boolean isReturnBlock() {
 		return contains(AFlag.RETURN);
 	}
 
+	public boolean isMthExitBlock() {
+		return contains(AFlag.MTH_EXIT_BLOCK);
+	}
+
 	public boolean isEmpty() {
 		return instructions.isEmpty();
 	}

diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/blocks/BlockExceptionHandler.java b/jadx-core/src/main/java/jadx/core/dex/visitors/blocks/BlockExceptionHandler.java
@@ -627,7 +627,7 @@ private static void removeUnusedExcHandlers(MethodNode mth, List<TryCatchBlockAt
 				continue;
 			}
 			for (TryCatchBlockAttr tcb : tryBlocks) {
-				if (tcb.getHandlers().contains(handlerBlock)) {
+				if (tcb.getHandlers().contains(eh)) {
 					notProcessed = false;
 					break;
 				}

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
@@ -445,8 +445,9 @@ private boolean canInsertBreak(BlockNode exit) {
 		List<BlockNode> simplePath = BlockUtils.buildSimplePath(exit);
 		if (!simplePath.isEmpty()) {
 			BlockNode lastBlock = simplePath.get(simplePath.size() - 1);
-			if (lastBlock.contains(AFlag.RETURN)
-					|| lastBlock.getSuccessors().isEmpty()) {
+			if (lastBlock.isMthExitBlock()
+					|| lastBlock.isReturnBlock()
+					|| mth.isPreExitBlock(lastBlock)) {
 				return false;
 			}
 		}

diff --git a/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestUnreachableCatch2.java b/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestUnreachableCatch2.java
@@ -10,25 +10,24 @@
 import jadx.tests.api.SmaliTest;
 
 import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
-import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.MatcherAssert.assertThat;
 
-@SuppressWarnings("CommentedOutCode")
 public class TestUnreachableCatch2 extends SmaliTest {
 
+	@SuppressWarnings({ "unused", "DataFlowIssue" })
 	public static class UnusedExceptionHandlers1 implements AutoCloseable {
-		public static void doSomething(final Object unused1, final Object[] array, final Object o1,
+		public static void test(final Object unused1, final Object[] array, final Object o1,
 				final Object o2, final Object unused2) {
 			for (final Object item : array) {
 				ByteBuffer buffer = null;
 				try (final UnusedExceptionHandlers1 u = doSomething2(o1, "", o2)) {
 					try (final FileInputStream fis = new FileInputStream(u.getFilename())) {
 						final FileChannel fileChannel = fis.getChannel();
 						buffer = fileChannel.map(FileChannel.MapMode.READ_ONLY, 0, 42);
-					} catch (final IOException e) {
+					} catch (IOException e) {
 						// ignore
 					}
-				} catch (final IOException e) {
+				} catch (IOException e) {
 					// ignore
 				}
 			}
@@ -50,9 +49,12 @@ public void close() throws IOException {
 
 	@Test
 	public void test() {
-		// TODO: result code not compilable because of 'break' after 'throw'
+		// TODO: result code not compilable because 'try' block split into 2 block and 'fis' var become
+		// uninitialized
 		disableCompilation();
-		String code = getClassNode(UnusedExceptionHandlers1.class).getCode().toString();
-		assertThat(code, containsString("IOException"));
+		assertThat(getClassNode(UnusedExceptionHandlers1.class))
+				.code()
+				.doesNotContain("break;")
+				.countString(2, "} catch (IOException e");
 	}
 }