From 276ee537e15ffb23becc55d423fef9b5ea831cae Mon Sep 17 00:00:00 2001 From: Away <60818070+Away-pp@users.noreply.github.com> Date: Sun, 4 Feb 2024 19:02:34 +0100 Subject: [PATCH] fix: remove unused ExcHandlers blocks (PR #2086) * Removing unused ExcHandlers blocks * Improving removing unused ExcHandlers blocks * Removing gradlew of the commit * Adding test 2 for UnreachableCatch * Update jadx-core/src/test/java/jadx/tests/integration/trycatch/TestUnreachableCatch2.java --------- Co-authored-by: Away-pp Co-authored-by: skylot <118523+skylot@users.noreply.github.com> --- .../blocks/BlockExceptionHandler.java | 28 ++ .../dex/visitors/blocks/BlockProcessor.java | 20 +- .../trycatch/TestUnreachableCatch.java | 70 ++++ .../trycatch/TestUnreachableCatch2.java | 58 ++++ .../smali/trycatch/TestUnreachableCatch.smali | 308 ++++++++++++++++++ 5 files changed, 481 insertions(+), 3 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/trycatch/TestUnreachableCatch.java create mode 100644 jadx-core/src/test/java/jadx/tests/integration/trycatch/TestUnreachableCatch2.java create mode 100644 jadx-core/src/test/smali/trycatch/TestUnreachableCatch.smali 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 index 571a27f2624..da000e53d54 100644 --- 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 @@ -41,6 +41,7 @@ import jadx.core.utils.BlockUtils; import jadx.core.utils.InsnRemover; import jadx.core.utils.ListUtils; +import jadx.core.utils.blocks.BlockSet; import jadx.core.utils.exceptions.JadxRuntimeException; public class BlockExceptionHandler { @@ -65,6 +66,10 @@ public static boolean process(MethodNode mth) { removeMonitorExitFromExcHandler(mth, eh); } BlockProcessor.removeMarkedBlocks(mth); + + BlockSet sorted = new BlockSet(mth); + BlockUtils.dfsVisit(mth, sorted::set); + removeUnusedExcHandlers(mth, tryBlocks, sorted); return true; } @@ -586,4 +591,27 @@ private static int compareByTypeAndName(Comparator comparator, ClassInf } return r; } + + /** + * Remove excHandlers that were not used when connecting. + * Check first if the blocks are unreachable. + */ + private static void removeUnusedExcHandlers(MethodNode mth, List tryBlocks, BlockSet blocks) { + for (ExceptionHandler eh : mth.getExceptionHandlers()) { + boolean notProcessed = true; + BlockNode handlerBlock = eh.getHandlerBlock(); + if (blocks.get(handlerBlock)) { + continue; + } + for (TryCatchBlockAttr tcb : tryBlocks) { + if (tcb.getHandlers().contains(handlerBlock)) { + notProcessed = false; + break; + } + } + if (notProcessed) { + BlockProcessor.removeUnreachableBlock(handlerBlock, mth); + } + } + } } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/blocks/BlockProcessor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/blocks/BlockProcessor.java index f00b32e2cdb..66fcb104674 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/blocks/BlockProcessor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/blocks/BlockProcessor.java @@ -646,10 +646,24 @@ public static void removeMarkedBlocks(MethodNode mth) { private static void removeUnreachableBlocks(MethodNode mth) { Set toRemove = new LinkedHashSet<>(); for (BlockNode block : mth.getBasicBlocks()) { - if (block.getPredecessors().isEmpty() && block != mth.getEnterBlock()) { - BlockSplitter.collectSuccessors(block, mth.getEnterBlock(), toRemove); - } + computeUnreachableFromBlock(toRemove, block, mth); + } + removeFromMethod(toRemove, mth); + } + + public static void removeUnreachableBlock(BlockNode blockToRemove, MethodNode mth) { + Set toRemove = new LinkedHashSet<>(); + computeUnreachableFromBlock(toRemove, blockToRemove, mth); + removeFromMethod(toRemove, mth); + } + + private static void computeUnreachableFromBlock(Set toRemove, BlockNode block, MethodNode mth) { + if (block.getPredecessors().isEmpty() && block != mth.getEnterBlock()) { + BlockSplitter.collectSuccessors(block, mth.getEnterBlock(), toRemove); } + } + + private static void removeFromMethod(Set toRemove, MethodNode mth) { if (toRemove.isEmpty()) { return; } diff --git a/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestUnreachableCatch.java b/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestUnreachableCatch.java new file mode 100644 index 00000000000..41c905982c6 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestUnreachableCatch.java @@ -0,0 +1,70 @@ +package jadx.tests.integration.trycatch; + +import org.junit.jupiter.api.Test; + +import jadx.core.dex.nodes.ClassNode; +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 TestUnreachableCatch extends SmaliTest { + + // @formatter:off + /* + private static Map prepareFontData(Context context, FontInfo[] fonts, + CancellationSignal cancellationSignal) { + final HashMap out = new HashMap<>(); + final ContentResolver resolver = context.getContentResolver(); + + for (FontInfo font : fonts) { + if (font.getResultCode() != Columns.RESULT_CODE_OK) { + continue; + } + + final Uri uri = font.getUri(); + if (out.containsKey(uri)) { + continue; + } + + ByteBuffer buffer = null; + try (final ParcelFileDescriptor pfd = + resolver.openFileDescriptor(uri, "r", cancellationSignal)) { + if (pfd != null) { + try (final FileInputStream fis = + new FileInputStream(pfd.getFileDescriptor())) { + final FileChannel fileChannel = fis.getChannel(); + final long size = fileChannel.size(); + buffer = fileChannel.map(FileChannel.MapMode.READ_ONLY, 0, size); + } catch (IOException e) { + // ignore + } + } + } catch (IOException e) { + // ignore + } + + // TODO: try other approach?, e.g. read all contents instead of mmap. + + out.put(uri, buffer); + } + return Collections.unmodifiableMap(out); + } + + */ + // @formatter:on + + @Test + public void test() { + disableCompilation(); + allowWarnInCode(); + + ClassNode cls = getClassNodeFromSmali(); + String code = cls.getCode().toString(); + + assertThat(code, containsString("IOException")); + assertThat(code, containsString("Collections.unmodifiableMap")); + } +} 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 new file mode 100644 index 00000000000..c1f275bdb47 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestUnreachableCatch2.java @@ -0,0 +1,58 @@ +package jadx.tests.integration.trycatch; + +import java.io.FileInputStream; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.channels.FileChannel; + +import org.junit.jupiter.api.Test; + +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 { + + public static class UnusedExceptionHandlers1 implements AutoCloseable { + public static void doSomething(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) { + // ignore + } + } catch (final IOException e) { + // ignore + } + } + } + + private String getFilename() { + return null; + } + + private static UnusedExceptionHandlers1 doSomething2(final Object o1, final String s, + final Object o2) { + return null; + } + + @Override + public void close() throws IOException { + } + } + + @Test + public void test() { + // TODO: result code not compilable because of 'break' after 'throw' + disableCompilation(); + String code = getClassNode(UnusedExceptionHandlers1.class).getCode().toString(); + assertThat(code, containsString("IOException")); + } +} diff --git a/jadx-core/src/test/smali/trycatch/TestUnreachableCatch.smali b/jadx-core/src/test/smali/trycatch/TestUnreachableCatch.smali new file mode 100644 index 00000000000..7f512e9f54a --- /dev/null +++ b/jadx-core/src/test/smali/trycatch/TestUnreachableCatch.smali @@ -0,0 +1,308 @@ +.class public Ltrycatch/TestUnreachableCatch; +.super Ljava/lang/Object; + +.method private static prepareFontData(Landroid/content/Context;[Landroid/provider/FontsContract$FontInfo;Landroid/os/CancellationSignal;)Ljava/util/Map; + .locals 18 + .param p0, "context" # Landroid/content/Context; + .param p1, "fonts" # [Landroid/provider/FontsContract$FontInfo; + .param p2, "cancellationSignal" # Landroid/os/CancellationSignal; + .annotation system Ldalvik/annotation/Signature; + value = { + "(", + "Landroid/content/Context;", + "[", + "Landroid/provider/FontsContract$FontInfo;", + "Landroid/os/CancellationSignal;", + ")", + "Ljava/util/Map<", + "Landroid/net/Uri;", + "Ljava/nio/ByteBuffer;", + ">;" + } + .end annotation + + .line 728 + move-object/from16 v1, p1 + + new-instance v0, Ljava/util/HashMap; + + invoke-direct {v0}, Ljava/util/HashMap;->()V + + move-object v2, v0 + + .line 729 + .local v2, "out":Ljava/util/HashMap;, "Ljava/util/HashMap;" + invoke-virtual/range {p0 .. p0}, Landroid/content/Context;->getContentResolver()Landroid/content/ContentResolver; + + move-result-object v3 + + .line 731 + .local v3, "resolver":Landroid/content/ContentResolver; + array-length v4, v1 + + const/4 v0, 0x0 + + move v5, v0 + + :goto_0 + if-ge v5, v4, :cond_5 + + aget-object v6, v1, v5 + + .line 732 + .local v6, "font":Landroid/provider/FontsContract$FontInfo; + invoke-virtual {v6}, Landroid/provider/FontsContract$FontInfo;->getResultCode()I + + move-result v0 + + if-eqz v0, :cond_0 + + .line 733 + move-object/from16 v9, p2 + + goto/16 :goto_5 + + .line 736 + :cond_0 + invoke-virtual {v6}, Landroid/provider/FontsContract$FontInfo;->getUri()Landroid/net/Uri; + + move-result-object v7 + + .line 737 + .local v7, "uri":Landroid/net/Uri; + invoke-virtual {v2, v7}, Ljava/util/HashMap;->containsKey(Ljava/lang/Object;)Z + + move-result v0 + + if-eqz v0, :cond_1 + + .line 738 + move-object/from16 v9, p2 + + goto :goto_5 + + .line 741 + :cond_1 + const/4 v8, 0x0 + + .line 742 + .local v8, "buffer":Ljava/nio/ByteBuffer; + :try_start_0 + const-string/jumbo v0, "r" + :try_end_0 + .catch Ljava/io/IOException; {:try_start_0 .. :try_end_0} :catch_2 + + .line 743 + move-object/from16 v9, p2 + + :try_start_1 + invoke-virtual {v3, v7, v0, v9}, Landroid/content/ContentResolver;->openFileDescriptor(Landroid/net/Uri;Ljava/lang/String;Landroid/os/CancellationSignal;)Landroid/os/ParcelFileDescriptor; + + move-result-object v0 + :try_end_1 + .catch Ljava/io/IOException; {:try_start_1 .. :try_end_1} :catch_1 + + move-object v10, v0 + + .line 744 + .local v10, "pfd":Landroid/os/ParcelFileDescriptor; + if-eqz v10, :cond_3 + + .line 745 + :try_start_2 + new-instance v0, Ljava/io/FileInputStream; + + .line 746 + invoke-virtual {v10}, Landroid/os/ParcelFileDescriptor;->getFileDescriptor()Ljava/io/FileDescriptor; + + move-result-object v11 + + invoke-direct {v0, v11}, Ljava/io/FileInputStream;->(Ljava/io/FileDescriptor;)V + :try_end_2 + .catch Ljava/io/IOException; {:try_start_2 .. :try_end_2} :catch_0 + .catchall {:try_start_2 .. :try_end_2} :catchall_2 + + move-object v11, v0 + + .line 747 + .local v11, "fis":Ljava/io/FileInputStream; + :try_start_3 + invoke-virtual {v11}, Ljava/io/FileInputStream;->getChannel()Ljava/nio/channels/FileChannel; + + move-result-object v12 + + .line 748 + .local v12, "fileChannel":Ljava/nio/channels/FileChannel; + invoke-virtual {v12}, Ljava/nio/channels/FileChannel;->size()J + + move-result-wide v16 + + .line 749 + .local v16, "size":J + sget-object v13, Ljava/nio/channels/FileChannel$MapMode;->READ_ONLY:Ljava/nio/channels/FileChannel$MapMode; + + const-wide/16 v14, 0x0 + + invoke-virtual/range {v12 .. v17}, Ljava/nio/channels/FileChannel;->map(Ljava/nio/channels/FileChannel$MapMode;JJ)Ljava/nio/MappedByteBuffer; + + move-result-object v0 + :try_end_3 + .catchall {:try_start_3 .. :try_end_3} :catchall_0 + + move-object v8, v0 + + .line 750 + .end local v12 # "fileChannel":Ljava/nio/channels/FileChannel; + .end local v16 # "size":J + :try_start_4 + invoke-virtual {v11}, Ljava/io/FileInputStream;->close()V + :try_end_4 + .catch Ljava/io/IOException; {:try_start_4 .. :try_end_4} :catch_0 + .catchall {:try_start_4 .. :try_end_4} :catchall_2 + + .line 752 + .end local v11 # "fis":Ljava/io/FileInputStream; + goto :goto_3 + + .line 745 + .restart local v11 # "fis":Ljava/io/FileInputStream; + :catchall_0 + move-exception v0 + + move-object v12, v0 + + :try_start_5 + invoke-virtual {v11}, Ljava/io/FileInputStream;->close()V + :try_end_5 + .catchall {:try_start_5 .. :try_end_5} :catchall_1 + + goto :goto_1 + + :catchall_1 + move-exception v0 + + move-object v13, v0 + + :try_start_6 + invoke-virtual {v12, v13}, Ljava/lang/Throwable;->addSuppressed(Ljava/lang/Throwable;)V + + .end local v2 # "out":Ljava/util/HashMap;, "Ljava/util/HashMap;" + .end local v3 # "resolver":Landroid/content/ContentResolver; + .end local v6 # "font":Landroid/provider/FontsContract$FontInfo; + .end local v7 # "uri":Landroid/net/Uri; + .end local v8 # "buffer":Ljava/nio/ByteBuffer; + .end local v10 # "pfd":Landroid/os/ParcelFileDescriptor; + .end local p0 # "context":Landroid/content/Context; + .end local p1 # "fonts":[Landroid/provider/FontsContract$FontInfo; + .end local p2 # "cancellationSignal":Landroid/os/CancellationSignal; + :goto_1 + throw v12 + :try_end_6 + .catch Ljava/io/IOException; {:try_start_6 .. :try_end_6} :catch_0 + .catchall {:try_start_6 .. :try_end_6} :catchall_2 + + .line 742 + .end local v11 # "fis":Ljava/io/FileInputStream; + .restart local v2 # "out":Ljava/util/HashMap;, "Ljava/util/HashMap;" + .restart local v3 # "resolver":Landroid/content/ContentResolver; + .restart local v6 # "font":Landroid/provider/FontsContract$FontInfo; + .restart local v7 # "uri":Landroid/net/Uri; + .restart local v8 # "buffer":Ljava/nio/ByteBuffer; + .restart local v10 # "pfd":Landroid/os/ParcelFileDescriptor; + .restart local p0 # "context":Landroid/content/Context; + .restart local p1 # "fonts":[Landroid/provider/FontsContract$FontInfo; + .restart local p2 # "cancellationSignal":Landroid/os/CancellationSignal; + :catchall_2 + move-exception v0 + + move-object v11, v0 + + if-eqz v10, :cond_2 + + :try_start_7 + invoke-virtual {v10}, Landroid/os/ParcelFileDescriptor;->close()V + :try_end_7 + .catchall {:try_start_7 .. :try_end_7} :catchall_3 + + goto :goto_2 + + :catchall_3 + move-exception v0 + + move-object v12, v0 + + :try_start_8 + invoke-virtual {v11, v12}, Ljava/lang/Throwable;->addSuppressed(Ljava/lang/Throwable;)V + + .end local v2 # "out":Ljava/util/HashMap;, "Ljava/util/HashMap;" + .end local v3 # "resolver":Landroid/content/ContentResolver; + .end local v6 # "font":Landroid/provider/FontsContract$FontInfo; + .end local v7 # "uri":Landroid/net/Uri; + .end local v8 # "buffer":Ljava/nio/ByteBuffer; + .end local p0 # "context":Landroid/content/Context; + .end local p1 # "fonts":[Landroid/provider/FontsContract$FontInfo; + .end local p2 # "cancellationSignal":Landroid/os/CancellationSignal; + :cond_2 + :goto_2 + throw v11 + + .line 750 + .restart local v2 # "out":Ljava/util/HashMap;, "Ljava/util/HashMap;" + .restart local v3 # "resolver":Landroid/content/ContentResolver; + .restart local v6 # "font":Landroid/provider/FontsContract$FontInfo; + .restart local v7 # "uri":Landroid/net/Uri; + .restart local v8 # "buffer":Ljava/nio/ByteBuffer; + .restart local p0 # "context":Landroid/content/Context; + .restart local p1 # "fonts":[Landroid/provider/FontsContract$FontInfo; + .restart local p2 # "cancellationSignal":Landroid/os/CancellationSignal; + :catch_0 + move-exception v0 + + .line 754 + :cond_3 + :goto_3 + if-eqz v10, :cond_4 + + invoke-virtual {v10}, Landroid/os/ParcelFileDescriptor;->close()V + :try_end_8 + .catch Ljava/io/IOException; {:try_start_8 .. :try_end_8} :catch_1 + + .line 756 + .end local v10 # "pfd":Landroid/os/ParcelFileDescriptor; + :cond_4 + goto :goto_4 + + .line 754 + :catch_1 + move-exception v0 + + goto :goto_4 + + :catch_2 + move-exception v0 + + move-object/from16 v9, p2 + + .line 760 + :goto_4 + invoke-virtual {v2, v7, v8}, Ljava/util/HashMap;->put(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object; + + .line 731 + .end local v6 # "font":Landroid/provider/FontsContract$FontInfo; + .end local v7 # "uri":Landroid/net/Uri; + .end local v8 # "buffer":Ljava/nio/ByteBuffer; + :goto_5 + add-int/lit8 v5, v5, 0x1 + + goto :goto_0 + + .line 762 + :cond_5 + move-object/from16 v9, p2 + + invoke-static {v2}, Ljava/util/Collections;->unmodifiableMap(Ljava/util/Map;)Ljava/util/Map; + + move-result-object v0 + + return-object v0 +.end method