fix: handle empty endless loop (#1611)

skylot · Aug 10, 2022 · 8ba0c17 · 8ba0c17
1 parent cd32151
commit 8ba0c17
Show file tree

Hide file tree

Showing 8 changed files with 145 additions and 5 deletions.
diff --git a/jadx-core/src/main/java/jadx/core/dex/attributes/nodes/LoopInfo.java b/jadx-core/src/main/java/jadx/core/dex/attributes/nodes/LoopInfo.java
@@ -91,6 +91,19 @@ public void setParentLoop(LoopInfo parentLoop) {
 		this.parentLoop = parentLoop;
 	}
 
+	public boolean hasParent(LoopInfo searchLoop) {
+		LoopInfo parent = parentLoop;
+		while (true) {
+			if (parent == null) {
+				return false;
+			}
+			if (parent == searchLoop) {
+				return true;
+			}
+			parent = parent.getParentLoop();
+		}
+	}
+
 	@Override
 	public String toString() {
 		return "LOOP:" + id + ": " + start + "->" + end;

diff --git a/jadx-core/src/main/java/jadx/core/dex/regions/loops/LoopRegion.java b/jadx-core/src/main/java/jadx/core/dex/regions/loops/LoopRegion.java
@@ -49,6 +49,10 @@ public BlockNode getHeader() {
 		return header;
 	}
 
+	public boolean isEndless() {
+		return header == null;
+	}
+
 	public IRegion getBody() {
 		return body;
 	}

diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/blocks/BlockSplitter.java b/jadx-core/src/main/java/jadx/core/dex/visitors/blocks/BlockSplitter.java
@@ -391,7 +391,8 @@ private static boolean canRemoveBlock(BlockNode block) {
 				&& block.getSuccessors().size() <= 1
 				&& !block.getPredecessors().isEmpty()
 				&& !block.contains(AFlag.MTH_ENTER_BLOCK)
-				&& !block.contains(AFlag.MTH_EXIT_BLOCK);
+				&& !block.contains(AFlag.MTH_EXIT_BLOCK)
+				&& !block.getSuccessors().contains(block); // no self loop
 	}
 
 	static void collectSuccessors(BlockNode startBlock, BlockNode methodEnterBlock, Set<BlockNode> toRemove) {

diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/CleanRegions.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/CleanRegions.java
@@ -8,6 +8,7 @@
 import jadx.core.dex.nodes.IRegion;
 import jadx.core.dex.nodes.MethodNode;
 import jadx.core.dex.regions.Region;
+import jadx.core.dex.regions.loops.LoopRegion;
 import jadx.core.dex.visitors.AbstractVisitor;
 
 public class CleanRegions extends AbstractVisitor {
@@ -42,6 +43,13 @@ private static boolean canRemoveRegion(IContainer container) {
 				BlockNode block = (BlockNode) container;
 				return block.getInstructions().isEmpty();
 			}
+			if (container instanceof LoopRegion) {
+				LoopRegion loopRegion = (LoopRegion) container;
+				if (loopRegion.isEndless()) {
+					// keep empty endless loops
+					return false;
+				}
+			}
 			if (container instanceof IRegion) {
 				List<IContainer> subBlocks = ((IRegion) container).getSubBlocks();
 				for (IContainer subBlock : subBlocks) {

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
@@ -464,7 +464,7 @@ private boolean insertLoopBreak(RegionStack stack, LoopInfo loop, BlockNode loop
 		BlockNode exitEnd = BlockUtils.followEmptyPath(exit);
 		List<LoopInfo> loops = exitEnd.getAll(AType.LOOP);
 		for (LoopInfo loopAtEnd : loops) {
-			if (loopAtEnd != loop) {
+			if (loopAtEnd != loop && loop.hasParent(loopAtEnd)) {
 				insertEdge = exitEdge;
 				confirm = true;
 				break;

diff --git a/jadx-core/src/main/java/jadx/core/utils/RegionUtils.java b/jadx-core/src/main/java/jadx/core/utils/RegionUtils.java
@@ -22,6 +22,7 @@
 import jadx.core.dex.nodes.InsnNode;
 import jadx.core.dex.nodes.MethodNode;
 import jadx.core.dex.regions.Region;
+import jadx.core.dex.regions.loops.LoopRegion;
 import jadx.core.dex.trycatch.CatchAttr;
 import jadx.core.dex.trycatch.ExceptionHandler;
 import jadx.core.dex.trycatch.TryCatchBlockAttr;
@@ -289,17 +290,20 @@ public static boolean notEmpty(@Nullable IContainer container) {
 				}
 			}
 			return false;
-		} else if (container instanceof IRegion) {
+		}
+		if (container instanceof LoopRegion) {
+			return true;
+		}
+		if (container instanceof IRegion) {
 			IRegion region = (IRegion) container;
 			for (IContainer block : region.getSubBlocks()) {
 				if (notEmpty(block)) {
 					return true;
 				}
 			}
 			return false;
-		} else {
-			throw new JadxRuntimeException(unknownContainerType(container));
 		}
+		throw new JadxRuntimeException(unknownContainerType(container));
 	}
 
 	public static void getAllRegionBlocks(IContainer container, Set<IBlock> blocks) {

diff --git a/jadx-core/src/test/java/jadx/tests/integration/loops/TestEndlessLoop2.java b/jadx-core/src/test/java/jadx/tests/integration/loops/TestEndlessLoop2.java
@@ -0,0 +1,20 @@
+package jadx.tests.integration.loops;
+
+import org.junit.jupiter.api.Test;
+
+import jadx.tests.api.SmaliTest;
+
+import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
+
+/**
+ * Empty endless loop, issue #1611
+ */
+public class TestEndlessLoop2 extends SmaliTest {
+	@Test
+	public void test() {
+		disableCompilation();
+		assertThat(getClassNodeFromSmali())
+				.code()
+				.countString(2, "while (true) {");
+	}
+}
diff --git a/jadx-core/src/test/smali/loops/TestEndlessLoop2.smali b/jadx-core/src/test/smali/loops/TestEndlessLoop2.smali
@@ -0,0 +1,90 @@
+.class Lloops/TestEndlessLoop2;
+.super Ljava/lang/Object;
+
+.field instanceCount:J
+
+.method test([Ljava/lang/String;)V
+    .registers 10
+
+    const/16 p1, 0xb
+    invoke-virtual {p0, p1}, Lloops/TestEndlessLoop2;->vMeth(I)V
+    const/16 v0, 0xf1
+    const-wide/high16 v1, 0x4032000000000000L    # 18.0
+
+    :goto_a
+    const-wide/high16 v3, 0x4076000000000000L    # 352.0
+    const/4 v5, 0x1
+    cmpg-double v6, v1, v3
+    if-gez v6, :cond_1c
+    const/4 v0, 0x1
+
+    :goto_12
+    add-int/2addr v0, v5
+    const/16 v3, 0x4b
+    if-ge v0, v3, :cond_18
+    goto :goto_12
+
+    :cond_18
+    const-wide/high16 v3, 0x3ff0000000000000L    # 1.0
+    add-double/2addr v1, v3
+    goto :goto_a
+
+    :cond_1c
+    iget-wide v3, p0, Lloops/TestEndlessLoop2;->instanceCount:J
+    long-to-int v4, v3
+    const/16 v3, 0xb
+
+    :goto_21
+    const/16 v6, 0xf3
+
+    if-ge v5, v6, :cond_41
+    rem-int/lit8 v6, v5, 0x9
+    add-int/lit8 v6, v6, 0x12
+    if-eq v6, p1, :cond_3e
+    const/16 v7, 0x15
+    if-eq v6, v7, :cond_36
+    const/16 v7, 0x16
+    if-eq v6, v7, :cond_34
+    goto :goto_3b
+
+
+    :cond_34
+    add-int/2addr v4, v0
+    goto :goto_3b
+
+    :cond_36
+    const v6, 0xeed9
+    div-int/2addr v3, v6
+    nop
+
+    :goto_3b
+    add-int/lit8 v5, v5, 0x1
+    goto :goto_21
+
+    :cond_3e
+    nop
+
+    :goto_3f
+    nop
+	# endless loop with empty body
+    goto :goto_3f
+
+    :cond_41
+    sget-object p1, Ljava/lang/System;->out:Ljava/io/PrintStream;
+    invoke-static {v1, v2}, Ljava/lang/Double;->doubleToLongBits(D)J
+    move-result-wide v0
+    new-instance v2, Ljava/lang/StringBuilder;
+    invoke-direct {v2}, Ljava/lang/StringBuilder;-><init>()V
+    const-string v5, "i21 d2 i22 = "
+    invoke-virtual {v2, v5}, Ljava/lang/StringBuilder;->append(Ljava/lang/String;)Ljava/lang/StringBuilder;
+    invoke-virtual {v2, v3}, Ljava/lang/StringBuilder;->append(I)Ljava/lang/StringBuilder;
+    const-string v3, ","
+    invoke-virtual {v2, v3}, Ljava/lang/StringBuilder;->append(Ljava/lang/String;)Ljava/lang/StringBuilder;
+    invoke-virtual {v2, v0, v1}, Ljava/lang/StringBuilder;->append(J)Ljava/lang/StringBuilder;
+    invoke-virtual {v2, v3}, Ljava/lang/StringBuilder;->append(Ljava/lang/String;)Ljava/lang/StringBuilder;
+    invoke-virtual {v2, v4}, Ljava/lang/StringBuilder;->append(I)Ljava/lang/StringBuilder;
+    invoke-virtual {v2}, Ljava/lang/StringBuilder;->toString()Ljava/lang/String;
+    move-result-object v0
+    invoke-virtual {p1, v0}, Ljava/io/PrintStream;->println(Ljava/lang/String;)V
+    return-void
+.end method