Skip to content

Commit

Permalink
fix: additional checks for 'if' blocks inside loops (#809)
Browse files Browse the repository at this point in the history
  • Loading branch information
skylot committed Dec 27, 2019
1 parent 99eb31b commit 04e309a
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 1 deletion.
4 changes: 3 additions & 1 deletion jadx-core/src/main/java/jadx/core/codegen/InsnGen.java
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,9 @@ private void makeInsnBody(CodeWriter code, InsnNode insn, Set<Flags> state) thro
case MONITOR_EXIT:
if (isFallback()) {
code.add("monitor-exit(");
addArg(code, insn.getArg(0));
if (insn.getArgsCount() == 1) {
addArg(code, insn.getArg(0));
}
code.add(')');
}
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,10 @@ static IfInfo mergeNestedIfNodes(IfInfo currentIf) {
if (curThen == curElse) {
return null;
}
if (BlockUtils.isFollowBackEdge(curThen)
|| BlockUtils.isFollowBackEdge(curElse)) {
return null;
}
boolean followThenBranch;
IfInfo nextIf = getNextIf(currentIf, curThen);
if (nextIf != null) {
Expand Down Expand Up @@ -380,6 +384,9 @@ private static IfInfo getNextIfNodeInfo(IfInfo info, BlockNode block) {
if (next.getPredecessors().size() != 1) {
return null;
}
if (next.contains(AFlag.ADDED_TO_REGION)) {
return null;
}
List<InsnNode> insns = block.getInstructions();
boolean pass = true;
List<InsnNode> forceInlineInsns = new ArrayList<>();
Expand Down
22 changes: 22 additions & 0 deletions jadx-core/src/main/java/jadx/core/utils/BlockUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import jadx.core.dex.attributes.AFlag;
import jadx.core.dex.attributes.AType;
import jadx.core.dex.attributes.nodes.IgnoreEdgeAttr;
import jadx.core.dex.attributes.nodes.LoopInfo;
import jadx.core.dex.attributes.nodes.PhiListAttr;
import jadx.core.dex.instructions.IfNode;
import jadx.core.dex.instructions.InsnType;
Expand Down Expand Up @@ -137,6 +138,27 @@ public static boolean isBackEdge(BlockNode from, BlockNode to) {
return from.getSuccessors().contains(to);
}

public static boolean isFollowBackEdge(BlockNode block) {
if (block == null) {
return false;
}
if (block.contains(AFlag.LOOP_START)) {
List<BlockNode> predecessors = block.getPredecessors();
if (predecessors.size() == 1) {
BlockNode loopEndBlock = predecessors.get(0);
if (loopEndBlock.contains(AFlag.LOOP_END)) {
List<LoopInfo> loops = loopEndBlock.getAll(AType.LOOP);
for (LoopInfo loop : loops) {
if (loop.getStart().equals(block) && loop.getEnd().equals(loopEndBlock)) {
return true;
}
}
}
}
}
return false;
}

/**
* Check if instruction contains in block (use == for comparison, not equals)
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package jadx.tests.integration.loops;

import java.util.Arrays;
import java.util.Iterator;

import org.junit.jupiter.api.Test;

import jadx.tests.api.IntegrationTest;

import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;

public class TestDoWhileBreak2 extends IntegrationTest {

public static class TestCls {
Iterator<String> it;

@SuppressWarnings("ConstantConditions")
public Object test() {
String obj;
do {
obj = this.it.next();
if (obj == null) {
return obj; // 'return null' works
}
} while (this.it.hasNext());
return obj;
}

public void check() {
this.it = Arrays.asList("a", "b").iterator();
assertThat(test()).isEqualTo("b");

this.it = Arrays.asList("a", "b", null).iterator();
assertThat(test()).isEqualTo(null);
}
}

@Test
public void test() {
assertThat(getClassNode(TestCls.class))
.code()
.containsLine(2, "do {")
.containsLine(3, "obj = this.it.next();")
.containsLine(3, "if (obj == null) {")
.containsLine(2, "} while (this.it.hasNext());");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package jadx.tests.integration.loops;

import java.util.Iterator;

import org.junit.jupiter.api.Test;

import jadx.NotYetImplemented;
import jadx.tests.api.IntegrationTest;

import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;

public class TestDoWhileBreak3 extends IntegrationTest {

public static class TestCls {
Iterator<String> it;

public void test() {
do {
if (!it.hasNext()) {
break;
}
} while (it.next() != null);
}
}

@NotYetImplemented
@Test
public void test() {
assertThat(getClassNode(TestCls.class))
.code()
.containsOnlyOnce("while");
}
}

0 comments on commit 04e309a

Please sign in to comment.