From a587ce88ea8919bf73f79406b9605c2046cc8820 Mon Sep 17 00:00:00 2001 From: Skylot Date: Fri, 12 Jul 2019 20:55:19 +0300 Subject: [PATCH] fix: ignore finally extraction with only one 'if' instruction (#709) --- .../core/dex/visitors/MarkFinallyVisitor.java | 22 +++- .../trycatch/TestTryCatchFinally10.java | 22 ++++ .../trycatch/TestTryCatchFinally9.java | 46 ++++++++ .../trycatch/TestTryCatchFinally10.smali | 107 ++++++++++++++++++ 4 files changed, 192 insertions(+), 5 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchFinally10.java create mode 100644 jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchFinally9.java create mode 100644 jadx-core/src/test/smali/trycatch/TestTryCatchFinally10.smali diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/MarkFinallyVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/MarkFinallyVisitor.java index 9675736b94d..9df4970f5f7 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/MarkFinallyVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/MarkFinallyVisitor.java @@ -5,6 +5,7 @@ import java.util.List; import java.util.Set; +import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -271,13 +272,24 @@ private static InsnsSlice searchFromFirstBlock(BlockNode dupBlock, BlockNode sta if (dupSlice == null) { return null; } - if (dupSlice.isComplete()) { - return dupSlice; - } - if (!checkBlocksTree(dupBlock, startBlock, dupSlice, extractInfo)) { + if (!dupSlice.isComplete() + && !checkBlocksTree(dupBlock, startBlock, dupSlice, extractInfo)) { return null; } - return dupSlice; + return checkSlice(dupSlice); + } + + @Nullable + private static InsnsSlice checkSlice(InsnsSlice slice) { + List insnsList = slice.getInsnsList(); + // ignore slice with only one 'if' insn + if (insnsList.size() == 1) { + InsnNode insnNode = insnsList.get(0); + if (insnNode.getType() == InsnType.IF) { + return null; + } + } + return slice; } /** diff --git a/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchFinally10.java b/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchFinally10.java new file mode 100644 index 00000000000..0d0a96ee117 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchFinally10.java @@ -0,0 +1,22 @@ +package jadx.tests.integration.trycatch; + +import org.junit.jupiter.api.Test; + +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.SmaliTest; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.not; + +public class TestTryCatchFinally10 extends SmaliTest { + + @Test + public void test() { + disableCompilation(); + ClassNode cls = getClassNodeFromSmali(); + String code = cls.getCode().toString(); + + assertThat(code, not(containsString("boolean z = null;"))); + } +} diff --git a/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchFinally9.java b/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchFinally9.java new file mode 100644 index 00000000000..ae815cc54e9 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchFinally9.java @@ -0,0 +1,46 @@ +package jadx.tests.integration.trycatch; + +import java.io.IOException; +import java.io.InputStream; +import java.util.Scanner; + +import org.junit.jupiter.api.Test; + +import jadx.NotYetImplemented; +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.IntegrationTest; + +import static jadx.tests.api.utils.JadxMatchers.containsOne; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.not; + +public class TestTryCatchFinally9 extends IntegrationTest { + + public static class TestCls { + public String test() throws IOException { + InputStream input = null; + try { + input = this.getClass().getResourceAsStream("resource"); + Scanner scanner = new Scanner(input).useDelimiter("\\A"); + return scanner.hasNext() ? scanner.next() : ""; + } finally { + if (input != null) { + input.close(); + } + } + } + } + + @Test + @NotYetImplemented("finally extraction") + public void test() { + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + + assertThat(code, not(containsString("JADX INFO: finally extract failed"))); + assertThat(code, not(containsString("throw"))); + assertThat(code, containsOne("} finally {")); + assertThat(code, containsOne("if (input != null) {")); + } +} diff --git a/jadx-core/src/test/smali/trycatch/TestTryCatchFinally10.smali b/jadx-core/src/test/smali/trycatch/TestTryCatchFinally10.smali new file mode 100644 index 00000000000..d96c91e5b10 --- /dev/null +++ b/jadx-core/src/test/smali/trycatch/TestTryCatchFinally10.smali @@ -0,0 +1,107 @@ +.class public Ltrycatch/TestTryCatchFinally10; +.super Ljava/lang/Object; + + +# static fields +.field private static final l:Llog/DebugLogger; + +.method public static test(Landroid/content/Context;I)Ljava/lang/String; + .locals 2 + + .line 46 + invoke-static {p0}, LCommonContracts;->requireNonNull(Ljava/lang/Object;)V + + const/4 v0, 0x0 + + .line 50 + :try_start_0 + invoke-virtual {p0}, Landroid/content/Context;->getResources()Landroid/content/res/Resources; + + move-result-object p0 + + invoke-virtual {p0, p1}, Landroid/content/res/Resources;->openRawResource(I)Ljava/io/InputStream; + + move-result-object v0 + + .line 51 + new-instance p0, Ljava/util/Scanner; + + invoke-direct {p0, v0}, Ljava/util/Scanner;->(Ljava/io/InputStream;)V + + const-string p1, "\\A" + + invoke-virtual {p0, p1}, Ljava/util/Scanner;->useDelimiter(Ljava/lang/String;)Ljava/util/Scanner; + + move-result-object p0 + + .line 52 + invoke-virtual {p0}, Ljava/util/Scanner;->hasNext()Z + + move-result p1 + + if-eqz p1, :cond_0 + + invoke-virtual {p0}, Ljava/util/Scanner;->next()Ljava/lang/String; + + move-result-object p0 + + goto :goto_0 + + :cond_0 + const-string p0, "" + :try_end_0 + .catchall {:try_start_0 .. :try_end_0} :catchall_0 + + :goto_0 + if-eqz v0, :cond_1 + + .line 56 + :try_start_1 + invoke-virtual {v0}, Ljava/io/InputStream;->close()V + :try_end_1 + .catch Ljava/io/IOException; {:try_start_1 .. :try_end_1} :catch_0 + + goto :goto_1 + + :catch_0 + move-exception p1 + + .line 58 + sget-object v0, Ltrycatch/TestTryCatchFinally10;->l:Llog/DebugLogger; + + sget-object v1, Llog/DebugLogger$LogLevel;->ERROR:Llog/DebugLogger$LogLevel; + + invoke-virtual {v0, v1, p1}, Llog/DebugLogger;->logException(Llog/DebugLogger$LogLevel;Ljava/lang/Exception;)V + + :cond_1 + :goto_1 + return-object p0 + + :catchall_0 + move-exception p0 + + if-eqz v0, :cond_2 + + .line 56 + :try_start_2 + invoke-virtual {v0}, Ljava/io/InputStream;->close()V + :try_end_2 + .catch Ljava/io/IOException; {:try_start_2 .. :try_end_2} :catch_1 + + goto :goto_2 + + :catch_1 + move-exception p1 + + .line 58 + sget-object v0, Ltrycatch/TestTryCatchFinally10;->l:Llog/DebugLogger; + + sget-object v1, Llog/DebugLogger$LogLevel;->ERROR:Llog/DebugLogger$LogLevel; + + invoke-virtual {v0, v1, p1}, Llog/DebugLogger;->logException(Llog/DebugLogger$LogLevel;Ljava/lang/Exception;)V + + .line 61 + :cond_2 + :goto_2 + throw p0 +.end method