From 8ba0c1725924df0844b41d348740fa9044d3d934 Mon Sep 17 00:00:00 2001 From: Skylot Date: Wed, 10 Aug 2022 21:40:31 +0100 Subject: [PATCH] fix: handle empty endless loop (#1611) --- .../core/dex/attributes/nodes/LoopInfo.java | 13 +++ .../core/dex/regions/loops/LoopRegion.java | 4 + .../dex/visitors/blocks/BlockSplitter.java | 3 +- .../dex/visitors/regions/CleanRegions.java | 8 ++ .../dex/visitors/regions/RegionMaker.java | 2 +- .../java/jadx/core/utils/RegionUtils.java | 10 ++- .../integration/loops/TestEndlessLoop2.java | 20 +++++ .../test/smali/loops/TestEndlessLoop2.smali | 90 +++++++++++++++++++ 8 files changed, 145 insertions(+), 5 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/loops/TestEndlessLoop2.java create mode 100644 jadx-core/src/test/smali/loops/TestEndlessLoop2.smali 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 index 5695ff27a..c42525e5c 100644 --- 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 class LoopInfo { 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 index fe5a94b16..c86350656 100644 --- 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 final class LoopRegion extends ConditionRegion { 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 index a0916ce66..532517d66 100644 --- 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 @@ public class BlockSplitter extends AbstractVisitor { && 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 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 index b47a3881a..9ecb5638a 100644 --- 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.IContainer; 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 @@ public class CleanRegions extends AbstractVisitor { 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 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 index 9cfe5a803..819b733b8 100644 --- 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 @@ public class RegionMaker { BlockNode exitEnd = BlockUtils.followEmptyPath(exit); List 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 index ef6f90075..aa0ffc903 100644 --- 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.IRegion; 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,7 +290,11 @@ public class RegionUtils { } } 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)) { @@ -297,9 +302,8 @@ public class RegionUtils { } } return false; - } else { - throw new JadxRuntimeException(unknownContainerType(container)); } + throw new JadxRuntimeException(unknownContainerType(container)); } public static void getAllRegionBlocks(IContainer container, Set 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 new file mode 100644 index 000000000..a08d83a9f --- /dev/null +++ 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 new file mode 100644 index 000000000..c01d71d43 --- /dev/null +++ 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;->()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