From 26800fb79083dc084a869aac24efe997bbafa006 Mon Sep 17 00:00:00 2001 From: Skylot Date: Thu, 6 Jun 2013 21:48:57 +0400 Subject: [PATCH] Refactor increment and decrement operations codegen --- src/main/java/jadx/codegen/InsnGen.java | 41 +++++++++++-------- src/main/java/jadx/codegen/RegionGen.java | 4 +- .../java/jadx/dex/instructions/ArithNode.java | 8 ++-- .../java/jadx/dex/instructions/ArithOp.java | 3 -- .../dex/instructions/args/LiteralArg.java | 9 ++++ .../java/jadx/dex/visitors/CodeShrinker.java | 37 ++++++----------- .../dex/visitors/regions/RegionMaker.java | 6 ++- 7 files changed, 56 insertions(+), 52 deletions(-) diff --git a/src/main/java/jadx/codegen/InsnGen.java b/src/main/java/jadx/codegen/InsnGen.java index 03535fd53..077eb0669 100644 --- a/src/main/java/jadx/codegen/InsnGen.java +++ b/src/main/java/jadx/codegen/InsnGen.java @@ -73,7 +73,7 @@ public class InsnGen { public String arg(InsnArg arg) throws CodegenException { if (arg.isRegister()) { - return mgen.makeArgName((RegisterArg) arg); + return arg((RegisterArg) arg); } else if (arg.isLiteral()) { return lit((LiteralArg) arg); } else { @@ -83,12 +83,16 @@ public class InsnGen { } } + public String arg(RegisterArg arg) { + return mgen.makeArgName(arg); + } + public String assignVar(InsnNode insn) { RegisterArg arg = insn.getResult(); if (insn.getAttributes().contains(AttributeType.DECLARE_VARIABLE)) { return declareVar(arg); } else { - return mgen.makeArgName(arg); + return arg(arg); } } @@ -324,11 +328,11 @@ public class InsnGen { code.add(arg(insn.getArg(0))); break; - /* fallback mode instructions */ case NOP: state.add(InsnGenState.SKIP); break; + /* fallback mode instructions */ case IF: assert isFallback(); IfNode ifInsn = (IfNode) insn; @@ -548,25 +552,28 @@ public class InsnGen { private void makeArith(ArithNode insn, CodeWriter code, EnumSet state) throws CodegenException { ArithOp op = insn.getOp(); String v1 = arg(insn.getArg(0)); - - if (op == ArithOp.INC || op == ArithOp.DEC) { - code.add(v1 + op.getSymbol()); - state.add(InsnGenState.NO_RESULT); + String v2 = arg(insn.getArg(1)); + if (state.contains(InsnGenState.BODY_ONLY)) { + // wrap insn in brackets for save correct operation order + // TODO don't wrap first insn in wrapped stack + code.add('(').add(v1).add(' ').add(op.getSymbol()).add(' ').add(v2).add(')'); } else { String res = arg(insn.getResult()); - String v2 = arg(insn.getArg(1)); - if (res.equals(v1) && !state.contains(InsnGenState.BODY_ONLY)) { - code.add(assignVar(insn) + " " + op.getSymbol() + "= " + v2); + if (res.equals(v1)) { state.add(InsnGenState.NO_RESULT); + // "++" or "--" + if (insn.getArg(1).isLiteral() && (op == ArithOp.ADD || op == ArithOp.SUB)) { + LiteralArg lit = (LiteralArg) insn.getArg(1); + if (Math.abs(lit.getLiteral()) == 1 && lit.isInteger()) { + code.add(assignVar(insn)).add(op.getSymbol()).add(op.getSymbol()); + return; + } + } + // +=, -= ... + code.add(assignVar(insn)).add(' ').add(op.getSymbol()).add("= ").add(v2); } else { - if (state.contains(InsnGenState.BODY_ONLY)) - // wrap insn in brackets for save correct operation order - // TODO don't wrap first insn in wrapped stack - code.add("(" + v1 + " " + op.getSymbol() + " " + v2 + ")"); - else - code.add(v1 + " " + op.getSymbol() + " " + v2); + code.add(v1).add(' ').add(op.getSymbol()).add(' ').add(v2); } } } - } diff --git a/src/main/java/jadx/codegen/RegionGen.java b/src/main/java/jadx/codegen/RegionGen.java index f24d06f61..7104fb528 100644 --- a/src/main/java/jadx/codegen/RegionGen.java +++ b/src/main/java/jadx/codegen/RegionGen.java @@ -208,16 +208,16 @@ public class RegionGen extends InsnGen { } private void makeCaseBlock(IContainer c, CodeWriter code) throws CodegenException { + code.add(" {"); if (RegionUtils.notEmpty(c)) { - code.add(" {"); makeRegionIndent(code, c); if (RegionUtils.hasExitEdge(c)) { code.startLine(1, "break;"); } - code.startLine('}'); } else { code.startLine(1, "break;"); } + code.startLine('}'); } private void makeTryCatch(IContainer region, TryCatchBlock tryCatchBlock, CodeWriter code) diff --git a/src/main/java/jadx/dex/instructions/ArithNode.java b/src/main/java/jadx/dex/instructions/ArithNode.java index a4c1025ff..faa8d00a3 100644 --- a/src/main/java/jadx/dex/instructions/ArithNode.java +++ b/src/main/java/jadx/dex/instructions/ArithNode.java @@ -44,17 +44,17 @@ public class ArithNode extends InsnNode { public ArithNode(ArithOp op, RegisterArg res, InsnArg a, InsnArg b) { super(InsnType.ARITH, 2); + this.op = op; setResult(res); addArg(a); addArg(b); - this.op = op; } public ArithNode(ArithOp op, RegisterArg res, InsnArg a) { super(InsnType.ARITH, 1); + this.op = op; setResult(res); addArg(a); - this.op = op; } public ArithOp getOp() { @@ -66,7 +66,9 @@ public class ArithNode extends InsnNode { return InsnUtils.formatOffset(offset) + ": " + InsnUtils.insnTypeToString(insnType) + getResult() + " = " - + getArg(0) + " " + op.getSymbol() + " " + getArg(1); + + getArg(0) + " " + + op.getSymbol() + " " + + (getArgsCount() == 2 ? getArg(1) : ""); } } diff --git a/src/main/java/jadx/dex/instructions/ArithOp.java b/src/main/java/jadx/dex/instructions/ArithOp.java index 55294a48f..efd40c27d 100644 --- a/src/main/java/jadx/dex/instructions/ArithOp.java +++ b/src/main/java/jadx/dex/instructions/ArithOp.java @@ -7,9 +7,6 @@ public enum ArithOp { DIV("/"), REM("%"), - INC("++"), - DEC("--"), - AND("&"), OR("|"), XOR("^"), diff --git a/src/main/java/jadx/dex/instructions/args/LiteralArg.java b/src/main/java/jadx/dex/instructions/args/LiteralArg.java index f8cef9ac7..1ced310dd 100644 --- a/src/main/java/jadx/dex/instructions/args/LiteralArg.java +++ b/src/main/java/jadx/dex/instructions/args/LiteralArg.java @@ -23,6 +23,15 @@ public class LiteralArg extends InsnArg { return true; } + public boolean isInteger() { + PrimitiveType type = typedVar.getType().getPrimitiveType(); + return (type == PrimitiveType.INT + || type == PrimitiveType.BYTE + || type == PrimitiveType.CHAR + || type == PrimitiveType.SHORT + || type == PrimitiveType.LONG); + } + @Override public String toString() { try { diff --git a/src/main/java/jadx/dex/visitors/CodeShrinker.java b/src/main/java/jadx/dex/visitors/CodeShrinker.java index 76e8f112b..fd3500933 100644 --- a/src/main/java/jadx/dex/visitors/CodeShrinker.java +++ b/src/main/java/jadx/dex/visitors/CodeShrinker.java @@ -30,10 +30,6 @@ public class CodeShrinker extends AbstractVisitor { if (mth.isNoCode() || mth.getAttributes().contains(AttributeFlag.DONT_SHRINK)) return; - // TODO pretify required inlined consts (made by shrink) - // 'dec' and 'inc' don't need to be inlined => must be replaced before shrink - pretify(mth); - shrink(mth); pretify(mth); } @@ -57,11 +53,11 @@ public class CodeShrinker extends AbstractVisitor { LOG.debug("parent insn null in " + useInsnArg + " from " + insn + " mth: " + mth); } else if (useInsn != insn) { boolean wrap = false; - // TODO don't reorder methods invocations - // if (insn.getResult().getTypedVar().getName() != null) { - // wrap = false; - // } else - if (BlockUtils.blockContains(block, useInsn)) { + if (false && insn.getResult().getTypedVar().getName() != null) { + // don't wrap if result variable has name from debug info + wrap = false; + } else if (BlockUtils.blockContains(block, useInsn)) { + // TODO don't reorder methods invocations // wrap insn from current block wrap = true; } else { @@ -72,8 +68,13 @@ public class CodeShrinker extends AbstractVisitor { } } if (wrap) { - useInsnArg.wrapInstruction(insn); - remover.add(insn); + if (useInsn.getType() == InsnType.MOVE) { + // TODO + // remover.add(useInsn); + } else { + useInsnArg.wrapInstruction(insn); + remover.add(insn); + } } } } @@ -124,20 +125,6 @@ public class CodeShrinker extends AbstractVisitor { if (arith.getOp() == ArithOp.ADD && lit < 0) invert = true; - if (false && (lit == 1 || lit == -1) && arith.getArg(0).isRegister()) { - ArithOp op = arith.getOp(); - if (op == ArithOp.ADD || op == ArithOp.SUB) { - RegisterArg res = arith.getResult(); - RegisterArg v0 = (RegisterArg) arith.getArg(0); - if (v0.equals(res)) { - // use 'a++' instead 'a = a + 1' (similar for minus) - boolean inc = ((op == ArithOp.ADD && lit == 1) - || (op == ArithOp.SUB && lit == -1)); - return new ArithNode(inc ? ArithOp.INC : ArithOp.DEC, null, v0); - } - } - } - // fix 'c + (-1)' => 'c - (1)' if (invert) { return new ArithNode(ArithOp.SUB, diff --git a/src/main/java/jadx/dex/visitors/regions/RegionMaker.java b/src/main/java/jadx/dex/visitors/regions/RegionMaker.java index 488ddfc90..b4a667c95 100644 --- a/src/main/java/jadx/dex/visitors/regions/RegionMaker.java +++ b/src/main/java/jadx/dex/visitors/regions/RegionMaker.java @@ -382,8 +382,10 @@ public class RegionMaker { out = null; } - if (stack.containsExit(elseBlock)) - elseBlock = null; + if (elseBlock != null) { + if (stack.containsExit(elseBlock)) + elseBlock = null; + } IfRegion ifRegion = new IfRegion(currentRegion, block); currentRegion.getSubBlocks().add(ifRegion);