Magic-Mirror/jadx
1
0
Fork 0
mirror of https://github.com/skylot/jadx.git synced 2024-11-23 12:50:02 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

fix: split loop exit at loop end (#1866)

Browse Source 
Loop exit (like condition) in loop end block can confuse loop restructure.
To resolve this try to split back edge and make loop end a simple path.
This commit is contained in:
Skylot 2023-05-13 21:29:20 +01:00
parent 0fd9a9df29
commit 3474f0da04
No known key found for this signature in database
GPG Key ID: 1E23F5B52567AA39
8 changed files with 84 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
jadx-core/src/main/java/jadx/core/codegen/MethodGen.java
View File

@ -553,7 +553,7 @@ public class MethodGen {
}
public static String getLabelName(BlockNode block) {
return String.format("L%d", block.getId());
return String.format("L%d", block.getCId());
}
public static String getLabelName(IfNode insn) {

18
jadx-core/src/main/java/jadx/core/dex/visitors/blocks/BlockProcessor.java
View File

@ -329,6 +329,21 @@ public class BlockProcessor extends AbstractVisitor {
return changed;
}
private static boolean simplifyLoopEnd(MethodNode mth, LoopInfo loop) {
BlockNode loopEnd = loop.getEnd();
if (loopEnd.getSuccessors().size() > 1) {
// make loop end a simple path block
BlockNode newLoopEnd = BlockSplitter.startNewBlock(mth, -1);
newLoopEnd.add(AFlag.SYNTHETIC);
newLoopEnd.add(AFlag.LOOP_END);
BlockNode loopStart = loop.getStart();
BlockSplitter.replaceConnection(loopEnd, loopStart, newLoopEnd);
BlockSplitter.connect(newLoopEnd, loopStart);
return true;
}
return false;
}
private static boolean checkLoops(MethodNode mth, BlockNode block) {
if (!block.contains(AFlag.LOOP_START)) {
return false;
@ -350,7 +365,8 @@ public class BlockProcessor extends AbstractVisitor {
LoopInfo loop = loops.get(0);
return insertBlocksForContinue(mth, loop)
|| insertBlockForPredecessors(mth, loop)
|| insertPreHeader(mth, loop);
|| insertPreHeader(mth, loop)
|| simplifyLoopEnd(mth, loop);
}
return false;
}

15
jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMaker.java
View File

@ -44,6 +44,7 @@ import jadx.core.dex.trycatch.ExcHandlerAttr;
import jadx.core.dex.trycatch.ExceptionHandler;
import jadx.core.dex.trycatch.TryCatchBlockAttr;
import jadx.core.utils.BlockUtils;
import jadx.core.utils.ListUtils;
import jadx.core.utils.RegionUtils;
import jadx.core.utils.Utils;
import jadx.core.utils.exceptions.JadxOverflowException;
@ -217,6 +218,8 @@ public class RegionMaker {
condInfo = IfInfo.invert(condInfo);
}
loopRegion.updateCondition(condInfo);
// prevent if's merge with loop condition
condInfo.getMergedBlocks().forEach(b -> b.add(AFlag.ADDED_TO_REGION));
exitBlocks.removeAll(condInfo.getMergedBlocks());
if (!exitBlocks.isEmpty()) {
@ -234,7 +237,8 @@ public class RegionMaker {
BlockNode out;
if (loopRegion.isConditionAtEnd()) {
BlockNode thenBlock = condInfo.getThenBlock();
out = thenBlock == loopStart ? condInfo.getElseBlock() : thenBlock;
out = (thenBlock == loop.getEnd() || thenBlock == loopStart) ? condInfo.getElseBlock() : thenBlock;
out = BlockUtils.followEmptyPath(out);
loopStart.remove(AType.LOOP);
loop.getEnd().add(AFlag.ADDED_TO_REGION);
stack.addExit(loop.getEnd());
@ -246,6 +250,7 @@ public class RegionMaker {
} else {
out = condInfo.getElseBlock();
if (outerRegion != null
&& out != null
&& out.contains(AFlag.LOOP_START)
&& !out.getAll(AType.LOOP).contains(loop)
&& RegionUtils.isRegionContainsBlock(outerRegion, out)) {
@ -298,9 +303,13 @@ public class RegionMaker {
// skip nested loop condition
continue;
}
LoopRegion loopRegion = new LoopRegion(curRegion, loop, block, block == loop.getEnd());
BlockNode loopEnd = loop.getEnd();
boolean exitAtLoopEnd = block == loopEnd
|| (loopEnd.getInstructions().isEmpty() && ListUtils.isSingleElement(loopEnd.getPredecessors(), block));
LoopRegion loopRegion = new LoopRegion(curRegion, loop, block, exitAtLoopEnd);
boolean found;
if (block == loop.getStart() || block == loop.getEnd()
if (block == loop.getStart() || exitAtLoopEnd
|| BlockUtils.isEmptySimplePath(loop.getStart(), block)) {
found = true;
} else if (block.getPredecessors().contains(loop.getStart())) {

2
jadx-core/src/test/java/jadx/tests/integration/loops/TestDoWhileBreak2.java
View File

@ -20,7 +20,7 @@ public class TestDoWhileBreak2 extends IntegrationTest {
do {
obj = this.it.next();
if (obj == null) {
return obj; // 'return null' works
return obj; // 'return null' or 'break' also fine
}
} while (this.it.hasNext());
return obj;

29
jadx-core/src/test/java/jadx/tests/integration/loops/TestIterableForEach4.java Normal file
View File

@ -0,0 +1,29 @@
package jadx.tests.integration.loops;
import java.util.List;
import org.junit.jupiter.api.Test;
import jadx.tests.api.IntegrationTest;
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
public class TestIterableForEach4 extends IntegrationTest {
public static class TestCls {
public void test(List<Object> objects) {
for (Object o : objects) {
if (o.hashCode() != 42 || o.hashCode() != 1) {
break;
}
}
}
}
@Test
public void test() {
assertThat(getClassNode(TestCls.class))
.code()
.doesNotContain("while (");
}
}

5
jadx-core/src/test/java/jadx/tests/integration/loops/TestLoopCondition5.java
View File

@ -38,7 +38,10 @@ public class TestLoopCondition5 extends SmaliTest {
ClassNode cls = getClassNodeFromSmaliWithPath("loops", "TestLoopCondition5");
String code = cls.getCode().toString();
assertThat(code, anyOf(containsOne("for ("), containsOne("while (true) {")));
assertThat(code, anyOf(
containsOne("for ("),
containsOne("while (true) {"),
containsOne("} while (iArr[i3] != i);")));
assertThat(code, containsOne("return -1;"));
assertThat(code, countString(2, "return "));
}

14
jadx-core/src/test/java/jadx/tests/integration/loops/TestNestedLoops5.java
View File

@ -2,13 +2,9 @@ package jadx.tests.integration.loops;
import org.junit.jupiter.api.Test;
import jadx.NotYetImplemented;
import jadx.core.dex.nodes.ClassNode;
import jadx.tests.api.IntegrationTest;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.not;
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
public class TestNestedLoops5 extends IntegrationTest {
@ -36,11 +32,9 @@ public class TestNestedLoops5 extends IntegrationTest {
}
@Test
@NotYetImplemented
public void test() {
ClassNode cls = getClassNode(TestCls.class);
String code = cls.getCode().toString();
assertThat(code, not(containsString("continue;")));
assertThat(getClassNode(TestCls.class))
.code()
.doesNotContain("continue;");
}
}

16
jadx-core/src/test/java/jadx/tests/integration/trycatch/TestLoopInTryCatch.java
View File

@ -40,6 +40,22 @@ public class TestLoopInTryCatch extends SmaliTest {
"}",
"if (i != 1) {",
" getI();",
"}"),
// TODO: weird result but correct, better to not use do-while if not really needed
c -> c.containsLines(2,
"int i;",
"do {",
" try {",
" i = getI();",
" if (i == 1) {",
" break;",
" }",
" } catch (RuntimeException unused) {",
" return;",
" }",
"} while (i != 2);",
"if (i != 1) {",
" getI();",
"}"));
}
}