Skip to content

Commit

Permalink
fix: force one branch ternary in constructors (#685)
Browse files Browse the repository at this point in the history
  • Loading branch information
skylot committed Jul 5, 2019
1 parent 533b686 commit 8410e62
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
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
Expand All @@ -41,7 +42,14 @@ private static boolean makeTernaryInsn(MethodNode mth, IfRegion ifRegion) {
}
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);
Expand Down Expand Up @@ -219,4 +227,62 @@ private static boolean checkLineStats(InsnNode t, InsnNode e) {
}
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);
}
}
Original file line number Diff line number Diff line change
@@ -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("//")));
}
}

0 comments on commit 8410e62

Please sign in to comment.