From 37857e88ea6cd709343403367167f27de045ceff Mon Sep 17 00:00:00 2001 From: Skylot Date: Mon, 23 Jun 2014 22:26:30 +0400 Subject: [PATCH] core: fix switch statement processing (issue #9 case 2) --- .../java/jadx/core/codegen/RegionGen.java | 2 +- .../dex/visitors/regions/RegionMaker.java | 32 +++++++----- .../main/java/jadx/core/utils/BlockUtils.java | 5 +- .../internal/{ => switches}/TestSwitch.java | 7 ++- .../{ => switches}/TestSwitchLabels.java | 2 +- .../switches/TestSwitchNoDefault.java | 42 +++++++++++++++ .../internal/switches/TestSwitchSimple.java | 52 +++++++++++++++++++ 7 files changed, 122 insertions(+), 20 deletions(-) rename jadx-core/src/test/java/jadx/tests/internal/{ => switches}/TestSwitch.java (81%) rename jadx-core/src/test/java/jadx/tests/internal/{ => switches}/TestSwitchLabels.java (96%) create mode 100644 jadx-core/src/test/java/jadx/tests/internal/switches/TestSwitchNoDefault.java create mode 100644 jadx-core/src/test/java/jadx/tests/internal/switches/TestSwitchSimple.java diff --git a/jadx-core/src/main/java/jadx/core/codegen/RegionGen.java b/jadx-core/src/main/java/jadx/core/codegen/RegionGen.java index dca77cc7..252e20cc 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/RegionGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/RegionGen.java @@ -199,7 +199,7 @@ public class RegionGen extends InsnGen { SwitchNode insn = (SwitchNode) sw.getHeader().getInstructions().get(0); InsnArg arg = insn.getArg(0); code.startLine("switch ("); - addArg(code, arg); + addArg(code, arg, false); code.add(") {"); code.incIndent(); 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 2134ab5c..067dd94f 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 @@ -627,7 +627,7 @@ public class RegionMaker { BitSet succ = BlockUtils.blocksToBitSet(mth, block.getSuccessors()); BitSet domsOn = BlockUtils.blocksToBitSet(mth, block.getDominatesOn()); - domsOn.and(succ); // filter 'out' block + domsOn.xor(succ); // filter 'out' block BlockNode defCase = getBlockByOffset(insn.getDefaultCaseOffset(), block.getSuccessors()); if (defCase != null) { @@ -642,13 +642,11 @@ public class RegionMaker { } if (outCount > 1) { // filter successors of other blocks + List blocks = mth.getBasicBlocks(); for (int i = domsOn.nextSetBit(0); i >= 0; i = domsOn.nextSetBit(i + 1)) { - BlockNode b = mth.getBasicBlocks().get(i); + BlockNode b = blocks.get(i); for (BlockNode s : b.getCleanSuccessors()) { - int id = s.getId(); - if (domsOn.get(id)) { - domsOn.clear(id); - } + domsOn.clear(s.getId()); } } outCount = domsOn.cardinality(); @@ -658,19 +656,27 @@ public class RegionMaker { if (outCount == 1) { out = mth.getBasicBlocks().get(domsOn.nextSetBit(0)); } else if (outCount == 0) { - // default and out blocks are same - out = defCase; + // one or several case blocks are empty, + // run expensive algorithm for find 'out' block + for (BlockNode maybeOut : block.getSuccessors()) { + boolean allReached = true; + for (BlockNode s : block.getSuccessors()) { + if (!BlockUtils.isPathExists(s, maybeOut)) { + allReached = false; + break; + } + } + if (allReached) { + out = maybeOut; + break; + } + } } stack.push(sw); if (out != null) { stack.addExit(out); } -// else { -// for (BlockNode e : BlockUtils.bitSetToBlocks(mth, domsOn)) { -// stack.addExit(e); -// } -// } if (!stack.containsExit(defCase)) { sw.setDefaultCase(makeRegion(defCase, stack)); diff --git a/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java b/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java index 9219ae41..d8b69623 100644 --- a/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java @@ -203,10 +203,7 @@ public class BlockUtils { } public static boolean isPathExists(BlockNode start, BlockNode end) { - if (start == end) { - return true; - } - if (end.isDominator(start)) { + if (start == end || end.isDominator(start)) { return true; } return traverseSuccessorsUntil(start, end, new BitSet()); diff --git a/jadx-core/src/test/java/jadx/tests/internal/TestSwitch.java b/jadx-core/src/test/java/jadx/tests/internal/switches/TestSwitch.java similarity index 81% rename from jadx-core/src/test/java/jadx/tests/internal/TestSwitch.java rename to jadx-core/src/test/java/jadx/tests/internal/switches/TestSwitch.java index f8281ea3..4ef3c292 100644 --- a/jadx-core/src/test/java/jadx/tests/internal/TestSwitch.java +++ b/jadx-core/src/test/java/jadx/tests/internal/switches/TestSwitch.java @@ -1,4 +1,4 @@ -package jadx.tests.internal; +package jadx.tests.internal.switches; import jadx.api.InternalJadxTest; import jadx.core.dex.nodes.ClassNode; @@ -6,6 +6,7 @@ import jadx.core.dex.nodes.ClassNode; import org.junit.Test; import static org.hamcrest.CoreMatchers.containsString; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; public class TestSwitch extends InternalJadxTest { @@ -45,5 +46,9 @@ public class TestSwitch extends InternalJadxTest { assertThat(code, containsString("case '/':")); assertThat(code, containsString(indent(5) + "break;")); + assertThat(code, containsString(indent(4) + "default:")); + + assertEquals(1, count(code, "i++")); + assertEquals(4, count(code, "break;")); } } diff --git a/jadx-core/src/test/java/jadx/tests/internal/TestSwitchLabels.java b/jadx-core/src/test/java/jadx/tests/internal/switches/TestSwitchLabels.java similarity index 96% rename from jadx-core/src/test/java/jadx/tests/internal/TestSwitchLabels.java rename to jadx-core/src/test/java/jadx/tests/internal/switches/TestSwitchLabels.java index 124a33a4..4a4d6018 100644 --- a/jadx-core/src/test/java/jadx/tests/internal/TestSwitchLabels.java +++ b/jadx-core/src/test/java/jadx/tests/internal/switches/TestSwitchLabels.java @@ -1,4 +1,4 @@ -package jadx.tests.internal; +package jadx.tests.internal.switches; import jadx.api.InternalJadxTest; import jadx.core.dex.nodes.ClassNode; diff --git a/jadx-core/src/test/java/jadx/tests/internal/switches/TestSwitchNoDefault.java b/jadx-core/src/test/java/jadx/tests/internal/switches/TestSwitchNoDefault.java new file mode 100644 index 00000000..89f8cd97 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/internal/switches/TestSwitchNoDefault.java @@ -0,0 +1,42 @@ +package jadx.tests.internal.switches; + +import jadx.api.InternalJadxTest; +import jadx.core.dex.nodes.ClassNode; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +public class TestSwitchNoDefault extends InternalJadxTest { + + public static class TestCls { + public void test(int a) { + String s = null; + switch (a) { + case 1: + s = "1"; + break; + case 2: + s = "2"; + break; + case 3: + s = "3"; + break; + case 4: + s = "4"; + break; + } + System.out.println(s); + } + } + + @Test + public void test() { + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + System.out.println(code); + + assertEquals(4, count(code, "break;")); + assertEquals(1, count(code, "System.out.println(s);")); + } +} diff --git a/jadx-core/src/test/java/jadx/tests/internal/switches/TestSwitchSimple.java b/jadx-core/src/test/java/jadx/tests/internal/switches/TestSwitchSimple.java new file mode 100644 index 00000000..6f588dd4 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/internal/switches/TestSwitchSimple.java @@ -0,0 +1,52 @@ +package jadx.tests.internal.switches; + +import jadx.api.InternalJadxTest; +import jadx.core.dex.nodes.ClassNode; + +import org.junit.Test; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.not; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; + +public class TestSwitchSimple extends InternalJadxTest { + + public static class TestCls { + public void test(int a) { + String s = null; + switch (a % 4) { + case 1: + s = "1"; + break; + case 2: + s = "2"; + break; + case 3: + s = "3"; + break; + case 4: + s = "4"; + break; + default: + System.out.println("Not Reach"); + break; + } + System.out.println(s); + } + } + + @Test + public void test() { + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + System.out.println(code); + + assertEquals(5, count(code, "break;")); + assertEquals(1, count(code, "System.out.println(s);")); + assertEquals(1, count(code, "System.out.println(\"Not Reach\");")); + + assertThat(code, not(containsString("switch ((a % 4)) {"))); + assertThat(code, containsString("switch (a % 4) {")); + } +} -- GitLab