From d6ad21f6f98d1e91c6ef24a24c45e9ced606809c Mon Sep 17 00:00:00 2001 From: Skylot Date: Sun, 9 Aug 2020 15:00:50 +0100 Subject: [PATCH] fix: correct detection of exits in synchronized block (#942) --- .../core/dex/regions/SynchronizedRegion.java | 4 +- .../main/java/jadx/core/utils/BlockUtils.java | 16 ++- .../synchronize/TestSynchronized4.java | 31 ++++++ .../smali/synchronize/TestSynchronized4.smali | 105 ++++++++++++++++++ 4 files changed, 148 insertions(+), 8 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/synchronize/TestSynchronized4.java create mode 100644 jadx-core/src/test/smali/synchronize/TestSynchronized4.smali diff --git a/jadx-core/src/main/java/jadx/core/dex/regions/SynchronizedRegion.java b/jadx-core/src/main/java/jadx/core/dex/regions/SynchronizedRegion.java index 345704280..8a88fe106 100644 --- a/jadx-core/src/main/java/jadx/core/dex/regions/SynchronizedRegion.java +++ b/jadx-core/src/main/java/jadx/core/dex/regions/SynchronizedRegion.java @@ -1,6 +1,6 @@ package jadx.core.dex.regions; -import java.util.LinkedList; +import java.util.ArrayList; import java.util.List; import jadx.core.dex.nodes.IContainer; @@ -10,7 +10,7 @@ import jadx.core.dex.nodes.InsnNode; public final class SynchronizedRegion extends AbstractRegion { private final InsnNode enterInsn; - private final List exitInsns = new LinkedList<>(); + private final List exitInsns = new ArrayList<>(); private final Region region; public SynchronizedRegion(IRegion parent, InsnNode insn) { diff --git a/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java b/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java index a6cc9fd65..154741c72 100644 --- a/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java @@ -6,7 +6,6 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; -import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; @@ -538,17 +537,22 @@ public class BlockUtils { } public static List buildSimplePath(BlockNode block) { - List list = new LinkedList<>(); - BlockNode currentBlock = block; + if (block == null) { + return Collections.emptyList(); + } + List list = new ArrayList<>(); + if (block.getCleanSuccessors().size() >= 2) { + return Collections.emptyList(); + } + list.add(block); + + BlockNode currentBlock = getNextBlock(block); while (currentBlock != null && currentBlock.getCleanSuccessors().size() < 2 && currentBlock.getPredecessors().size() == 1) { list.add(currentBlock); currentBlock = getNextBlock(currentBlock); } - if (list.isEmpty()) { - return Collections.emptyList(); - } return list; } diff --git a/jadx-core/src/test/java/jadx/tests/integration/synchronize/TestSynchronized4.java b/jadx-core/src/test/java/jadx/tests/integration/synchronize/TestSynchronized4.java new file mode 100644 index 000000000..4329030a1 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/synchronize/TestSynchronized4.java @@ -0,0 +1,31 @@ +package jadx.tests.integration.synchronize; + +import org.junit.jupiter.api.Test; + +import jadx.tests.api.SmaliTest; + +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; + +public class TestSynchronized4 extends SmaliTest { + + // @formatter:off + /* + public boolean test(int i) { + synchronized (this.obj) { + if (isZero(i)) { + return call(obj, i); + } + System.out.println(); + return getField() == null; + } + } + */ + // @formatter:on + + @Test + public void test() { + assertThat(getClassNodeFromSmali()) + .code() + .containsOne("synchronized (this.obj) {"); + } +} diff --git a/jadx-core/src/test/smali/synchronize/TestSynchronized4.smali b/jadx-core/src/test/smali/synchronize/TestSynchronized4.smali new file mode 100644 index 000000000..bf908e9a1 --- /dev/null +++ b/jadx-core/src/test/smali/synchronize/TestSynchronized4.smali @@ -0,0 +1,105 @@ +.class public Lsynchronize/TestSynchronized4; +.super Ljava/lang/Object; + +.field private final obj:Ljava/lang/Object; + +.method public constructor ()V + .registers 2 + + invoke-direct {p0}, Ljava/lang/Object;->()V + + new-instance v0, Ljava/lang/Object; + + invoke-direct {v0}, Ljava/lang/Object;->()V + + iput-object v0, p0, Lsynchronize/TestSynchronized4;->obj:Ljava/lang/Object; + + return-void +.end method + +.method public test(I)Z + .registers 4 + + iget-object v1, p0, Lsynchronize/TestSynchronized4;->obj:Ljava/lang/Object; + + monitor-enter v1 + + :try_start_3 + invoke-direct {p0, p1}, Lsynchronize/TestSynchronized4;->isZero(I)Z + move-result v0 + + if-eqz v0, :cond_11 + + iget-object v0, p0, Lsynchronize/TestSynchronized4;->obj:Ljava/lang/Object; + + invoke-direct {p0, v0, p1}, Lsynchronize/TestSynchronized4;->call(Ljava/lang/Object;I)Z + move-result v0 + + monitor-exit v1 + + :goto_10 + return v0 + + :cond_11 + sget-object v0, Ljava/lang/System;->out:Ljava/io/PrintStream; + + invoke-virtual {v0}, Ljava/io/PrintStream;->println()V + + invoke-direct {p0}, Lsynchronize/TestSynchronized4;->getField()Ljava/lang/Object; + + move-result-object v0 + + if-nez v0, :cond_22 + + const/4 v0, 0x1 + + :goto_1d + monitor-exit v1 + return v0 + + :catchall_1f + move-exception v0 + + monitor-exit v1 + :try_end_21 + .catchall {:try_start_3 .. :try_end_21} :catchall_1f + + throw v0 + + :cond_22 + const/4 v0, 0x0 + + goto :goto_1d +.end method + +.method private call(Ljava/lang/Object;I)Z + .registers 4 + + const/4 v0, 0x0 + + return v0 +.end method + +.method private getField()Ljava/lang/Object; + .registers 2 + + const/4 v0, 0x0 + + return-object v0 +.end method + +.method private isZero(I)Z + .registers 3 + + if-nez p1, :cond_4 + + const/4 v0, 0x1 + + :goto_3 + return v0 + + :cond_4 + const/4 v0, 0x0 + + goto :goto_3 +.end method