From 8410e625317e73569abec394cc6fc95e9bee84ea Mon Sep 17 00:00:00 2001 From: Skylot Date: Fri, 5 Jul 2019 17:08:15 +0300 Subject: [PATCH] fix: force one branch ternary in constructors (#685) --- .../core/dex/visitors/regions/TernaryMod.java | 68 ++++++++++++++++++- .../TestTernaryOneBranchInConstructor.java | 51 ++++++++++++++ 2 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/conditions/TestTernaryOneBranchInConstructor.java diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/TernaryMod.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/TernaryMod.java index ecdc0d265..8a830f83f 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/TernaryMod.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/TernaryMod.java @@ -21,6 +21,7 @@ import jadx.core.dex.regions.Region; import jadx.core.dex.regions.conditions.IfRegion; import jadx.core.dex.visitors.shrink.CodeShrinkVisitor; import jadx.core.utils.InsnList; +import jadx.core.utils.InsnRemover; /** * Convert 'if' to ternary operation @@ -41,7 +42,14 @@ public class TernaryMod implements IRegionIterativeVisitor { } IContainer thenRegion = ifRegion.getThenRegion(); IContainer elseRegion = ifRegion.getElseRegion(); - if (thenRegion == null || elseRegion == null) { + if (thenRegion == null) { + return false; + } + if (elseRegion == null) { + if (mth.isConstructor()) { + // force ternary conversion to inline all code in 'super' or 'this' calls + return processOneBranchTernary(mth, ifRegion); + } return false; } BlockNode tb = getTernaryInsnBlock(thenRegion); @@ -219,4 +227,62 @@ public class TernaryMod implements IRegionIterativeVisitor { } return false; } + + /** + * Convert one variable change with only 'then' branch: + * 'if (c) {r = a;}' to 'r = c ? a : r' + * Also convert only if 'r' used only once + */ + private static boolean processOneBranchTernary(MethodNode mth, IfRegion ifRegion) { + IContainer thenRegion = ifRegion.getThenRegion(); + BlockNode block = getTernaryInsnBlock(thenRegion); + if (block != null) { + InsnNode insn = block.getInstructions().get(0); + RegisterArg result = insn.getResult(); + if (result != null) { + replaceWithTernary(mth, ifRegion, block, insn); + } + } + return false; + } + + private static void replaceWithTernary(MethodNode mth, IfRegion ifRegion, BlockNode block, InsnNode insn) { + BlockNode header = ifRegion.getConditionBlocks().get(0); + if (!ifRegion.getParent().replaceSubBlock(ifRegion, header)) { + return; + } + RegisterArg resArg = insn.getResult(); + if (resArg.getSVar().getUseList().size() != 1) { + return; + } + PhiInsn phiInsn = resArg.getSVar().getOnlyOneUseInPhi(); + if (phiInsn == null || phiInsn.getArgsCount() != 2) { + return; + } + RegisterArg otherArg = null; + for (InsnArg arg : phiInsn.getArguments()) { + if (arg != resArg && arg instanceof RegisterArg) { + otherArg = (RegisterArg) arg; + break; + } + } + if (otherArg == null) { + return; + } + + // all checks passed + InsnList.remove(block, insn); + TernaryInsn ternInsn = new TernaryInsn(ifRegion.getCondition(), + phiInsn.getResult(), InsnArg.wrapArg(insn), otherArg); + ternInsn.setSourceLine(insn.getSourceLine()); + + InsnRemover.unbindInsn(mth, phiInsn); + header.getInstructions().clear(); + header.getInstructions().add(ternInsn); + + clearConditionBlocks(ifRegion.getConditionBlocks(), header); + + // shrink method again + CodeShrinkVisitor.shrinkMethod(mth); + } } diff --git a/jadx-core/src/test/java/jadx/tests/integration/conditions/TestTernaryOneBranchInConstructor.java b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestTernaryOneBranchInConstructor.java new file mode 100644 index 000000000..200cc3c1f --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestTernaryOneBranchInConstructor.java @@ -0,0 +1,51 @@ +package jadx.tests.integration.conditions; + +import org.junit.jupiter.api.Test; + +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.IntegrationTest; + +import static jadx.tests.api.utils.JadxMatchers.containsOne; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.MatcherAssert.assertThat; + +public class TestTernaryOneBranchInConstructor extends IntegrationTest { + + public static class TestCls { + public TestCls(String str, int i) { + this(str == null ? 0 : i); + } + + public TestCls(int i) { + } + } + + @Test + public void test() { + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + + assertThat(code, containsOne("this(str == null ? 0 : i);")); + assertThat(code, not(containsString("//"))); + } + + public static class TestCls2 { + public TestCls2(String str, int i) { + this(i == 1 ? str : "", i == 0 ? "" : str); + } + + public TestCls2(String a, String b) { + } + } + + @Test + public void test2() { + noDebugInfo(); + ClassNode cls = getClassNode(TestCls2.class); + String code = cls.getCode().toString(); + + assertThat(code, containsOne("this(i == 1 ? str : \"\", i == 0 ? \"\" : str);")); + assertThat(code, not(containsString("//"))); + } +}