From d748e004d2c0010e4c4f045e7cc1b5bc8e8a05ba Mon Sep 17 00:00:00 2001 From: Skylot Date: Sat, 8 Nov 2014 15:35:49 +0300 Subject: [PATCH] core: fix missing parenthesis in conditions --- .../java/jadx/core/codegen/ConditionGen.java | 92 ++++++++++++++----- .../dex/visitors/regions/RegionMaker.java | 3 +- .../integration/conditions/TestTernary3.java | 50 ++++++++++ 3 files changed, 118 insertions(+), 27 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/conditions/TestTernary3.java diff --git a/jadx-core/src/main/java/jadx/core/codegen/ConditionGen.java b/jadx-core/src/main/java/jadx/core/codegen/ConditionGen.java index 650b48bbd..8e77f7a7d 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/ConditionGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/ConditionGen.java @@ -16,6 +16,8 @@ import jadx.core.utils.exceptions.CodegenException; import jadx.core.utils.exceptions.JadxRuntimeException; import java.util.Iterator; +import java.util.LinkedList; +import java.util.Queue; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -23,46 +25,83 @@ import org.slf4j.LoggerFactory; public class ConditionGen extends InsnGen { private static final Logger LOG = LoggerFactory.getLogger(ConditionGen.class); + private static class CondStack { + private final Queue stack = new LinkedList(); + + public Queue getStack() { + return stack; + } + + public void push(IfCondition cond) { + stack.add(cond); + } + + public IfCondition pop() { + return stack.poll(); + } + } + public ConditionGen(InsnGen insnGen) { super(insnGen.mgen, insnGen.fallback); } void add(CodeWriter code, IfCondition condition) throws CodegenException { + add(code, new CondStack(), condition); + } + + void wrap(CodeWriter code, IfCondition condition) throws CodegenException { + wrap(code, new CondStack(), condition); + } + + private void add(CodeWriter code, CondStack stack, IfCondition condition) throws CodegenException { + stack.push(condition); switch (condition.getMode()) { case COMPARE: - addCompare(code, condition.getCompare()); + addCompare(code, stack, condition.getCompare()); break; case TERNARY: - addTernary(code, condition); + addTernary(code, stack, condition); break; case NOT: - addNot(code, condition); + addNot(code, stack, condition); break; case AND: case OR: - addAndOr(code, condition); + addAndOr(code, stack, condition); break; default: throw new JadxRuntimeException("Unknown condition mode: " + condition.getMode()); } + stack.pop(); } - void wrap(CodeWriter code, IfCondition cond) throws CodegenException { + private void wrap(CodeWriter code, CondStack stack, IfCondition cond) throws CodegenException { boolean wrap = isWrapNeeded(cond); if (wrap) { code.add('('); } - add(code, cond); + add(code, stack, cond); if (wrap) { code.add(')'); } } - private void addCompare(CodeWriter code, Compare compare) throws CodegenException { + private void wrap(CodeWriter code, InsnArg firstArg) throws CodegenException { + boolean wrap = isArgWrapNeeded(firstArg); + if (wrap) { + code.add('('); + } + addArg(code, firstArg, false); + if (wrap) { + code.add(')'); + } + } + + private void addCompare(CodeWriter code, CondStack stack, Compare compare) throws CodegenException { IfOp op = compare.getOp(); InsnArg firstArg = compare.getA(); InsnArg secondArg = compare.getB(); @@ -75,19 +114,16 @@ public class ConditionGen extends InsnGen { } if (op == IfOp.EQ) { // == true - addArg(code, firstArg, false); + if (stack.getStack().size() == 1) { + addArg(code, firstArg, false); + } else { + wrap(code, firstArg); + } return; } else if (op == IfOp.NE) { // != true code.add('!'); - boolean wrap = isArgWrapNeeded(firstArg); - if (wrap) { - code.add('('); - } - addArg(code, firstArg, false); - if (wrap) { - code.add(')'); - } + wrap(code, firstArg); return; } LOG.warn(ErrorsCounter.formatErrorMsg(mth, "Unsupported boolean condition " + op.getSymbol())); @@ -98,24 +134,24 @@ public class ConditionGen extends InsnGen { addArg(code, secondArg, isArgWrapNeeded(secondArg)); } - private void addTernary(CodeWriter code, IfCondition condition) throws CodegenException { - add(code, condition.first()); + private void addTernary(CodeWriter code, CondStack stack, IfCondition condition) throws CodegenException { + add(code, stack, condition.first()); code.add(" ? "); - add(code, condition.second()); + add(code, stack, condition.second()); code.add(" : "); - add(code, condition.third()); + add(code, stack, condition.third()); } - private void addNot(CodeWriter code, IfCondition condition) throws CodegenException { + private void addNot(CodeWriter code, CondStack stack, IfCondition condition) throws CodegenException { code.add('!'); - wrap(code, condition.getArgs().get(0)); + wrap(code, stack, condition.getArgs().get(0)); } - private void addAndOr(CodeWriter code, IfCondition condition) throws CodegenException { + private void addAndOr(CodeWriter code, CondStack stack, IfCondition condition) throws CodegenException { String mode = condition.getMode() == Mode.AND ? " && " : " || "; Iterator it = condition.getArgs().iterator(); while (it.hasNext()) { - wrap(code, it.next()); + wrap(code, stack, it.next()); if (it.hasNext()) { code.add(mode); } @@ -123,7 +159,13 @@ public class ConditionGen extends InsnGen { } private boolean isWrapNeeded(IfCondition condition) { - return !condition.isCompare() && condition.getMode() != Mode.NOT; + if (condition.isCompare()) { + return false; + } + if (condition.getMode() != Mode.NOT) { + return true; + } + return false; } private static boolean isArgWrapNeeded(InsnArg arg) { 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 d32440c65..bf01cf74b 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 @@ -140,9 +140,8 @@ public class RegionMaker { } if (next != null && !stack.containsExit(block) && !stack.containsExit(next)) { return next; - } else { - return null; } + return null; } private BlockNode processLoop(IRegion curRegion, LoopInfo loop, RegionStack stack) { diff --git a/jadx-core/src/test/java/jadx/tests/integration/conditions/TestTernary3.java b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestTernary3.java new file mode 100644 index 000000000..b1ecc2962 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestTernary3.java @@ -0,0 +1,50 @@ +package jadx.tests.integration.conditions; + +import jadx.core.dex.instructions.args.InsnArg; +import jadx.core.dex.instructions.args.Named; +import jadx.core.dex.instructions.args.RegisterArg; +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.IntegrationTest; + +import org.junit.Test; + +import static jadx.tests.api.utils.JadxMatchers.containsOne; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.not; +import static org.junit.Assert.assertThat; + +public class TestTernary3 extends IntegrationTest { + + public static class TestCls { + + public boolean isNameEquals(InsnArg arg) { + String n = getName(arg); + if (n == null || !(arg instanceof Named)) { + return false; + } + return n.equals(((Named) arg).getName()); + } + + private String getName(InsnArg arg) { + if (arg instanceof RegisterArg) { + return "r"; + } + if (arg instanceof Named) { + return "n"; + } + return arg.toString(); + } + } + + @Test + public void test() { + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + + // TODO: + assertThat(code, containsOne("return (n == null || !(arg instanceof Named)) " + + "? false : n.equals(((Named) arg).getName());")); + + assertThat(code, not(containsString("if ((arg instanceof RegisterArg)) {"))); + } +}