From 24d22aaafbc626c6aa663d24997caab7462821dd Mon Sep 17 00:00:00 2001 From: Skylot Date: Thu, 1 May 2014 22:40:49 +0400 Subject: [PATCH] core: fix 'if' detection --- .../java/jadx/core/dex/nodes/BlockNode.java | 12 +-- .../core/dex/visitors/BlockMakerVisitor.java | 12 +-- .../dex/visitors/regions/RegionMaker.java | 81 ++++++++++--------- .../internal/conditions/TestTernary2.java | 12 ++- 4 files changed, 62 insertions(+), 55 deletions(-) diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/BlockNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/BlockNode.java index 6a986f2ac..085ed8175 100644 --- a/jadx-core/src/main/java/jadx/core/dex/nodes/BlockNode.java +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/BlockNode.java @@ -72,20 +72,20 @@ public class BlockNode extends AttrNode implements IBlock { private static List cleanSuccessors(BlockNode block) { List sucList = block.getSuccessors(); List nodes = new ArrayList(sucList.size()); - LoopAttr loop = (LoopAttr) block.getAttributes().get(AttributeType.LOOP); - if (loop == null) { + if (block.getAttributes().contains(AttributeFlag.LOOP_END)) { + LoopAttr loop = (LoopAttr) block.getAttributes().get(AttributeType.LOOP); for (BlockNode b : sucList) { if (!b.getAttributes().contains(AttributeType.EXC_HANDLER)) { + // don't follow back edge + if (loop.getStart() == b) { + continue; + } nodes.add(b); } } } else { for (BlockNode b : sucList) { if (!b.getAttributes().contains(AttributeType.EXC_HANDLER)) { - // don't follow back edge - if (loop.getStart() == b && loop.getEnd() == block) { - continue; - } nodes.add(b); } } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/BlockMakerVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/BlockMakerVisitor.java index d85083bd8..5885a6a19 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/BlockMakerVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/BlockMakerVisitor.java @@ -458,12 +458,14 @@ public class BlockMakerVisitor extends AbstractVisitor { if (exitBlock != otherExitBlock && otherExitBlock.isDominator(pred) && otherExitBlock.getPredecessors().size() == 1) { - // merge BlockNode otherPred = otherExitBlock.getPredecessors().get(0); - removeConnection(otherPred, otherExitBlock); - connect(otherPred, exitBlock); - cleanExitNodes(mth); - return true; + if (pred != otherPred) { + // merge + removeConnection(otherPred, otherExitBlock); + connect(otherPred, exitBlock); + cleanExitNodes(mth); + return true; + } } } } 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 9b8541c54..f211d2f87 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 @@ -201,8 +201,7 @@ public class RegionMaker { } List merged = new ArrayList(2); - IfInfo mergedIf = mergeNestedIfNodes(condBlock, - ifnode.getThenBlock(), ifnode.getElseBlock(), merged); + IfInfo mergedIf = mergeNestedIfNodes(condBlock, ifnode, merged); if (mergedIf != null) { condBlock = mergedIf.getIfnode(); if (!loop.getLoopBlocks().contains(mergedIf.getThenBlock())) { @@ -398,54 +397,56 @@ public class RegionMaker { } private BlockNode processIf(IRegion currentRegion, BlockNode block, IfNode ifnode, RegionStack stack) { - BlockNode bThen = ifnode.getThenBlock(); - BlockNode bElse = ifnode.getElseBlock(); - if (block.getAttributes().contains(AttributeFlag.SKIP)) { - // block already included in other if region - return bThen; + // block already included in other 'if' region + return ifnode.getThenBlock(); } - + final BlockNode thenBlock; + final BlockNode elseBlock; BlockNode out = null; - BlockNode thenBlock = null; - BlockNode elseBlock = null; IfRegion ifRegion = new IfRegion(currentRegion, block); currentRegion.getSubBlocks().add(ifRegion); - IfInfo mergedIf = mergeNestedIfNodes(block, bThen, bElse, null); + IfInfo mergedIf = mergeNestedIfNodes(block, ifnode, null); if (mergedIf != null) { ifRegion.setCondition(mergedIf.getCondition()); thenBlock = mergedIf.getThenBlock(); elseBlock = mergedIf.getElseBlock(); out = BlockUtils.getPathCrossBlockFor(mth, thenBlock, elseBlock); } else { - for (BlockNode d : block.getDominatesOn()) { - if (d != bThen && d != bElse) { - out = d; - break; - } - } // invert condition (compiler often do it) ifnode.invertCondition(); - bThen = ifnode.getThenBlock(); - bElse = ifnode.getElseBlock(); + final BlockNode bThen = ifnode.getThenBlock(); + final BlockNode bElse = ifnode.getElseBlock(); - thenBlock = bThen; - // select else and exit blocks - if (block.getDominatesOn().size() == 2 - && !BlockUtils.isPathExists(bThen, bElse)) { + // select 'then', 'else' and 'exit' blocks + if (bElse.getPredecessors().size() != 1 + && BlockUtils.isPathExists(bThen, bElse)) { + thenBlock = bThen; + elseBlock = null; + out = bElse; + } else if (bThen.getPredecessors().size() != 1 + && BlockUtils.isPathExists(bElse, bThen)) { + ifnode.invertCondition(); + thenBlock = ifnode.getThenBlock(); + elseBlock = null; + out = ifnode.getElseBlock(); + } else if (block.getDominatesOn().size() == 2) { + thenBlock = bThen; elseBlock = bElse; + out = BlockUtils.getPathCrossBlockFor(mth, bThen, bElse); + } else if (bElse.getPredecessors().size() != 1) { + thenBlock = bThen; + elseBlock = null; + out = bElse; } else { - if (bElse.getPredecessors().size() != 1) { - out = bElse; - } else { - elseBlock = bElse; - for (BlockNode d : block.getDominatesOn()) { - if (d != bThen && d != bElse) { - out = d; - break; - } + thenBlock = bThen; + elseBlock = bElse; + for (BlockNode d : block.getDominatesOn()) { + if (d != bThen && d != bElse) { + out = d; + break; } } } @@ -454,26 +455,26 @@ public class RegionMaker { } } - if (elseBlock != null && stack.containsExit(elseBlock)) { - elseBlock = null; - } - stack.push(ifRegion); stack.addExit(out); ifRegion.setThenRegion(makeRegion(thenBlock, stack)); - ifRegion.setElseRegion(elseBlock == null ? null : makeRegion(elseBlock, stack)); + if (elseBlock == null || stack.containsExit(elseBlock)) { + ifRegion.setElseRegion(null); + } else { + ifRegion.setElseRegion(makeRegion(elseBlock, stack)); + } stack.pop(); return out; } - private IfInfo mergeNestedIfNodes(BlockNode block, BlockNode bThen, BlockNode bElse, List merged) { + private IfInfo mergeNestedIfNodes(BlockNode block, IfNode ifnode, List merged) { IfInfo info = new IfInfo(); info.setIfnode(block); info.setCondition(IfCondition.fromIfBlock(block)); - info.setThenBlock(bThen); - info.setElseBlock(bElse); + info.setThenBlock(ifnode.getThenBlock()); + info.setElseBlock(ifnode.getElseBlock()); return mergeNestedIfNodes(info, merged); } diff --git a/jadx-core/src/test/java/jadx/tests/internal/conditions/TestTernary2.java b/jadx-core/src/test/java/jadx/tests/internal/conditions/TestTernary2.java index ab90b3968..f8960e73f 100644 --- a/jadx-core/src/test/java/jadx/tests/internal/conditions/TestTernary2.java +++ b/jadx-core/src/test/java/jadx/tests/internal/conditions/TestTernary2.java @@ -3,8 +3,9 @@ package jadx.tests.internal.conditions; import jadx.api.InternalJadxTest; import jadx.core.dex.nodes.ClassNode; -import static org.hamcrest.CoreMatchers.containsString; -import static org.junit.Assert.assertThat; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; public class TestTernary2 extends InternalJadxTest { @@ -20,12 +21,15 @@ public class TestTernary2 extends InternalJadxTest { } } - //@Test + @Test public void test() { ClassNode cls = getClassNode(TestCls.class); String code = cls.getCode().toString(); System.out.println(code); - assertThat(code, containsString("assertTrue(f(1, 0) == 0);")); + assertEquals(1, count(code, "assertTrue")); + assertEquals(1, count(code, "f(1, 0)")); + // TODO: +// assertThat(code, containsString("assertTrue(f(1, 0) == 0);")); } }