From 3598a1279ca68160c81e4525f55f81b8873b4800 Mon Sep 17 00:00:00 2001 From: Skylot Date: Sun, 30 Jun 2013 22:48:16 +0400 Subject: [PATCH] Process complex 'if' conditions, refactoring --- src/main/java/jadx/codegen/RegionGen.java | 86 ++++++------- .../jadx/dex/attributes/AttributeFlag.java | 1 + src/main/java/jadx/dex/instructions/IfOp.java | 4 +- .../dex/nodes/parser/DebugInfoParser.java | 27 ++--- .../parser/LocalVar.java} | 8 +- .../java/jadx/dex/regions/IfCondition.java | 114 ++++++++++++++++++ src/main/java/jadx/dex/regions/IfRegion.java | 20 +-- .../java/jadx/dex/regions/LoopRegion.java | 14 ++- .../dex/visitors/regions/CheckRegions.java | 12 +- .../dex/visitors/regions/RegionMaker.java | 59 +++++++-- .../dex/visitors/regions/RegionStack.java | 25 ++-- .../java/jadx/samples/AbstractTest.java | 6 + src/samples/java/jadx/samples/RunTests.java | 5 +- .../java/jadx/samples/TestConditions.java | 57 +++++++++ .../jadx/samples/TestStringProcessing.java | 15 ++- 15 files changed, 344 insertions(+), 109 deletions(-) rename src/main/java/jadx/dex/{info/LocalVarInfo.java => nodes/parser/LocalVar.java} (87%) create mode 100644 src/main/java/jadx/dex/regions/IfCondition.java create mode 100644 src/samples/java/jadx/samples/TestConditions.java diff --git a/src/main/java/jadx/codegen/RegionGen.java b/src/main/java/jadx/codegen/RegionGen.java index 3bf0afdd1..7fc582638 100644 --- a/src/main/java/jadx/codegen/RegionGen.java +++ b/src/main/java/jadx/codegen/RegionGen.java @@ -5,7 +5,6 @@ import jadx.dex.attributes.AttributeType; import jadx.dex.attributes.DeclareVariableAttr; import jadx.dex.attributes.ForceReturnAttr; import jadx.dex.attributes.IAttribute; -import jadx.dex.instructions.IfNode; import jadx.dex.instructions.IfOp; import jadx.dex.instructions.SwitchNode; import jadx.dex.instructions.args.ArgType; @@ -17,6 +16,7 @@ import jadx.dex.nodes.IContainer; import jadx.dex.nodes.IRegion; import jadx.dex.nodes.InsnNode; import jadx.dex.nodes.MethodNode; +import jadx.dex.regions.IfCondition; import jadx.dex.regions.IfRegion; import jadx.dex.regions.LoopRegion; import jadx.dex.regions.Region; @@ -104,8 +104,7 @@ public class RegionGen extends InsnGen { } private void makeIf(IfRegion region, CodeWriter code) throws CodegenException { - IfNode insn = region.getIfInsn(); - code.add("if ").add(makeCondition(insn)).add(" {"); + code.add("if (").add(makeCondition(region.getCondition())).add(") {"); makeRegionIndent(code, region.getThenRegion()); code.startLine('}'); @@ -116,9 +115,9 @@ public class RegionGen extends InsnGen { // connect if-else-if block if (els instanceof Region) { Region re = (Region) els; - if (re.getSubBlocks().size() == 1 - && re.getSubBlocks().get(0) instanceof IfRegion) { - makeIf((IfRegion) re.getSubBlocks().get(0), code); + List subBlocks = re.getSubBlocks(); + if (subBlocks.size() == 1 && subBlocks.get(0) instanceof IfRegion) { + makeIf((IfRegion) subBlocks.get(0), code); return; } } @@ -130,7 +129,8 @@ public class RegionGen extends InsnGen { } private CodeWriter makeLoop(LoopRegion region, CodeWriter code) throws CodegenException { - if (region.getConditionBlock() == null) { + IfCondition condition = region.getCondition(); + if (condition == null) { // infinite loop code.startLine("while (true) {"); makeRegionIndent(code, region.getBody()); @@ -138,15 +138,14 @@ public class RegionGen extends InsnGen { return code; } - IfNode insn = region.getIfInsn(); - if (!region.isConditionAtEnd()) { - code.startLine("while ").add(makeCondition(insn)).add(" {"); - makeRegionIndent(code, region.getBody()); - code.startLine('}'); - } else { + if (region.isConditionAtEnd()) { code.startLine("do {"); makeRegionIndent(code, region.getBody()); - code.startLine("} while ").add(makeCondition(insn)).add(';'); + code.startLine("} while (").add(makeCondition(condition)).add(");"); + } else { + code.startLine("while (").add(makeCondition(condition)).add(") {"); + makeRegionIndent(code, region.getBody()); + code.startLine('}'); } return code; } @@ -157,44 +156,47 @@ public class RegionGen extends InsnGen { code.startLine('}'); } - private String makeCondition(IfNode insn) throws CodegenException { - String simple = simplifyCondition(insn); - if (simple != null) - return simple; - - String second; - if (insn.isZeroCmp()) { - second = arg(InsnArg.lit(0, insn.getArg(0).getType())); - } else { - second = arg(insn.getArg(1)); + private String makeCondition(IfCondition condition) throws CodegenException { + switch (condition.getMode()) { + case COMPARE: + return makeCompare(condition.getCompare()); + case NOT: + return "!" + makeCondition(condition.getArgs().get(0)); + case AND: + case OR: + String mode = condition.getMode() == IfCondition.MODE.AND ? " && " : " || "; + CodeWriter cw = new CodeWriter(); + for (IfCondition arg : condition.getArgs()) { + if (cw.notEmpty()) { + cw.add(mode); + } + cw.add('(').add(makeCondition(arg)).add(')'); + } + return cw.toString(); + default: + return "??" + condition.toString(); } - return "(" + arg(insn.getArg(0)) + " " + insn.getOp().getSymbol() + " " + second + ")"; } - private String simplifyCondition(IfNode insn) throws CodegenException { - InsnArg firstArg = insn.getArg(0); - if (firstArg.getType().equals(ArgType.BOOLEAN)) { - IfOp op = insn.getOp(); - if (insn.isZeroCmp()) { + private String makeCompare(IfCondition.Compare compare) throws CodegenException { + IfOp op = compare.getOp(); + InsnArg firstArg = compare.getA(); + InsnArg secondArg = compare.getB(); + if (firstArg.getType().equals(ArgType.BOOLEAN) + && secondArg.isLiteral() + && secondArg.getType().equals(ArgType.BOOLEAN)) { + LiteralArg lit = (LiteralArg) secondArg; + if (lit.getLiteral() == 0) op = op.invert(); - } else { - InsnArg secondArg = insn.getArg(1); - if (!secondArg.isLiteral() || !secondArg.getType().equals(ArgType.BOOLEAN)) - return null; - - LiteralArg lit = (LiteralArg) secondArg; - if (lit.getLiteral() == 0) - op = op.invert(); - } if (op == IfOp.EQ) { - return "(" + arg(firstArg) + ")"; // == true + return arg(firstArg); // == true } else if (op == IfOp.NE) { - return "(!" + arg(firstArg) + ")"; // != true + return "!" + arg(firstArg); // != true } LOG.warn(ErrorsCounter.formatErrorMsg(mth, "Unsupported boolean condition " + op.getSymbol())); } - return null; + return arg(firstArg) + " " + op.getSymbol() + " " + arg(secondArg); } private CodeWriter makeSwitch(SwitchRegion sw, CodeWriter code) throws CodegenException { diff --git a/src/main/java/jadx/dex/attributes/AttributeFlag.java b/src/main/java/jadx/dex/attributes/AttributeFlag.java index a41a6f67c..ffadfe840 100644 --- a/src/main/java/jadx/dex/attributes/AttributeFlag.java +++ b/src/main/java/jadx/dex/attributes/AttributeFlag.java @@ -14,6 +14,7 @@ public enum AttributeFlag { DONT_SHRINK, DONT_GENERATE, + SKIP, INCONSISTENT_CODE, // warning about incorrect decompilation } diff --git a/src/main/java/jadx/dex/instructions/IfOp.java b/src/main/java/jadx/dex/instructions/IfOp.java index 454766c26..55372c3b2 100644 --- a/src/main/java/jadx/dex/instructions/IfOp.java +++ b/src/main/java/jadx/dex/instructions/IfOp.java @@ -1,5 +1,7 @@ package jadx.dex.instructions; +import jadx.utils.exceptions.JadxRuntimeException; + public enum IfOp { EQ("=="), NE("!="), @@ -36,7 +38,7 @@ public enum IfOp { return IfOp.LT; default: - return null; + throw new JadxRuntimeException("Unknown if operations type: " + this); } } } diff --git a/src/main/java/jadx/dex/nodes/parser/DebugInfoParser.java b/src/main/java/jadx/dex/nodes/parser/DebugInfoParser.java index 152f8cf04..63481c6ca 100644 --- a/src/main/java/jadx/dex/nodes/parser/DebugInfoParser.java +++ b/src/main/java/jadx/dex/nodes/parser/DebugInfoParser.java @@ -1,7 +1,6 @@ package jadx.dex.nodes.parser; import jadx.dex.attributes.SourceFileAttr; -import jadx.dex.info.LocalVarInfo; import jadx.dex.instructions.args.InsnArg; import jadx.dex.instructions.args.RegisterArg; import jadx.dex.nodes.DexNode; @@ -34,7 +33,7 @@ public class DebugInfoParser { private final Section section; private final DexNode dex; - private final LocalVarInfo[] locals; + private final LocalVar[] locals; private final InsnArg[] activeRegisters; private final InsnNode[] insnByOffset; @@ -43,7 +42,7 @@ public class DebugInfoParser { this.dex = mth.dex(); this.section = dex.openSection(debugOffset); - this.locals = new LocalVarInfo[mth.getRegsCount()]; + this.locals = new LocalVar[mth.getRegsCount()]; this.activeRegisters = new InsnArg[mth.getRegsCount()]; this.insnByOffset = insnByOffset; } @@ -67,7 +66,7 @@ public class DebugInfoParser { for (RegisterArg arg : mthArgs) { int rn = arg.getRegNum(); - locals[rn] = new LocalVarInfo(arg); + locals[rn] = new LocalVar(arg); activeRegisters[rn] = arg; } @@ -90,7 +89,7 @@ public class DebugInfoParser { int regNum = section.readUleb128(); int nameId = section.readUleb128() - 1; int type = section.readUleb128() - 1; - LocalVarInfo var = new LocalVarInfo(dex, regNum, nameId, type, DexNode.NO_INDEX); + LocalVar var = new LocalVar(dex, regNum, nameId, type, DexNode.NO_INDEX); startVar(var, addr, line); break; } @@ -99,13 +98,13 @@ public class DebugInfoParser { int nameId = section.readUleb128() - 1; int type = section.readUleb128() - 1; int sign = section.readUleb128() - 1; - LocalVarInfo var = new LocalVarInfo(dex, regNum, nameId, type, sign); + LocalVar var = new LocalVar(dex, regNum, nameId, type, sign); startVar(var, addr, line); break; } case DBG_RESTART_LOCAL: { int regNum = section.readUleb128(); - LocalVarInfo var = locals[regNum]; + LocalVar var = locals[regNum]; if (var != null) { var.end(addr, line); setVar(var); @@ -115,7 +114,7 @@ public class DebugInfoParser { } case DBG_END_LOCAL: { int regNum = section.readUleb128(); - LocalVarInfo var = locals[regNum]; + LocalVar var = locals[regNum]; if (var != null) { var.end(addr, line); setVar(var); @@ -153,7 +152,7 @@ public class DebugInfoParser { c = section.readByte() & 0xFF; } - for (LocalVarInfo var : locals) { + for (LocalVar var : locals) { if (var != null && !var.isEnd()) { var.end(addr, line); setVar(var); @@ -180,9 +179,9 @@ public class DebugInfoParser { return newAddr; } - private void startVar(LocalVarInfo var, int addr, int line) { + private void startVar(LocalVar var, int addr, int line) { int regNum = var.getRegNum(); - LocalVarInfo prev = locals[regNum]; + LocalVar prev = locals[regNum]; if (prev != null && !prev.isEnd()) { prev.end(addr, line); setVar(prev); @@ -191,7 +190,7 @@ public class DebugInfoParser { locals[regNum] = var; } - private void setVar(LocalVarInfo var) { + private void setVar(LocalVar var) { int start = var.getStartAddr(); int end = var.getEndAddr(); @@ -203,7 +202,7 @@ public class DebugInfoParser { merge(activeRegisters[var.getRegNum()], var); } - private static void fillLocals(InsnNode insn, LocalVarInfo var) { + private static void fillLocals(InsnNode insn, LocalVar var) { if (insn.getResult() != null) merge(insn.getResult(), var); @@ -211,7 +210,7 @@ public class DebugInfoParser { merge(arg, var); } - private static void merge(InsnArg arg, LocalVarInfo var) { + private static void merge(InsnArg arg, LocalVar var) { if (arg != null && arg.isRegister()) { if (var.getRegNum() == arg.getRegNum()) arg.setTypedVar(var.getTypedVar()); diff --git a/src/main/java/jadx/dex/info/LocalVarInfo.java b/src/main/java/jadx/dex/nodes/parser/LocalVar.java similarity index 87% rename from src/main/java/jadx/dex/info/LocalVarInfo.java rename to src/main/java/jadx/dex/nodes/parser/LocalVar.java index 4286818fd..fb073bff3 100644 --- a/src/main/java/jadx/dex/info/LocalVarInfo.java +++ b/src/main/java/jadx/dex/nodes/parser/LocalVar.java @@ -1,4 +1,4 @@ -package jadx.dex.info; +package jadx.dex.nodes.parser; import jadx.dex.instructions.args.ArgType; import jadx.dex.instructions.args.RegisterArg; @@ -6,14 +6,14 @@ import jadx.dex.instructions.args.TypedVar; import jadx.dex.nodes.DexNode; import jadx.utils.InsnUtils; -public class LocalVarInfo extends RegisterArg { +final class LocalVar extends RegisterArg { private boolean isEnd; private int startAddr; private int endAddr; - public LocalVarInfo(DexNode dex, int rn, int nameId, int typeId, int signId) { + public LocalVar(DexNode dex, int rn, int nameId, int typeId, int signId) { super(rn); String name = (nameId == DexNode.NO_INDEX ? null : dex.getString(nameId)); ArgType type = (typeId == DexNode.NO_INDEX ? null : dex.getType(typeId)); @@ -22,7 +22,7 @@ public class LocalVarInfo extends RegisterArg { init(name, type, sign); } - public LocalVarInfo(RegisterArg arg) { + public LocalVar(RegisterArg arg) { super(arg.getRegNum()); init(arg.getTypedVar().getName(), arg.getType(), null); } diff --git a/src/main/java/jadx/dex/regions/IfCondition.java b/src/main/java/jadx/dex/regions/IfCondition.java new file mode 100644 index 000000000..2894985c4 --- /dev/null +++ b/src/main/java/jadx/dex/regions/IfCondition.java @@ -0,0 +1,114 @@ +package jadx.dex.regions; + +import jadx.dex.instructions.IfNode; +import jadx.dex.instructions.IfOp; +import jadx.dex.instructions.args.InsnArg; +import jadx.dex.nodes.BlockNode; + +import java.util.Arrays; +import java.util.List; + +public final class IfCondition { + + public static IfCondition fromIfBlock(BlockNode header) { + if (header == null) + return null; + + IfNode ifNode = (IfNode) header.getInstructions().get(0); + return new IfCondition(new Compare(ifNode)); + } + + public static IfCondition not(IfCondition a) { + return new IfCondition(MODE.NOT, Arrays.asList(a)); + } + + public static IfCondition and(IfCondition a, IfCondition b) { + return new IfCondition(MODE.AND, Arrays.asList(a, b)); + } + + public static IfCondition or(IfCondition a, IfCondition b) { + return new IfCondition(MODE.OR, Arrays.asList(a, b)); + } + + public static final class Compare { + private final IfNode insn; + + public Compare(IfNode ifNode) { + this.insn = ifNode; + } + + public IfOp getOp() { + return insn.getOp(); + } + + public InsnArg getA() { + return insn.getArg(0); + } + + public InsnArg getB() { + if (insn.isZeroCmp()) + return InsnArg.lit(0, getA().getType()); + else + return insn.getArg(1); + } + + @Override + public String toString() { + return getA() + " " + getOp().getSymbol() + " " + getB(); + } + } + + public static enum MODE { + COMPARE, + NOT, + AND, + OR + } + + private final MODE mode; + private final List args; + private final Compare compare; + + private IfCondition(Compare compare) { + this.mode = MODE.COMPARE; + this.compare = compare; + this.args = null; + } + + private IfCondition(MODE mode, List args) { + this.mode = mode; + this.args = args; + this.compare = null; + } + + public MODE getMode() { + return mode; + } + + public List getArgs() { + return args; + } + + public boolean isCompare() { + return mode == MODE.COMPARE; + } + + public Compare getCompare() { + return compare; + } + + @Override + public String toString() { + switch (mode) { + case COMPARE: + return compare.toString(); + case NOT: + return "!" + args; + case AND: + return "&& " + args; + case OR: + return "||" + args; + } + return "??"; + } +} diff --git a/src/main/java/jadx/dex/regions/IfRegion.java b/src/main/java/jadx/dex/regions/IfRegion.java index 0958c40a0..bf2de11fd 100644 --- a/src/main/java/jadx/dex/regions/IfRegion.java +++ b/src/main/java/jadx/dex/regions/IfRegion.java @@ -1,6 +1,5 @@ package jadx.dex.regions; -import jadx.dex.instructions.IfNode; import jadx.dex.nodes.BlockNode; import jadx.dex.nodes.IContainer; import jadx.dex.nodes.IRegion; @@ -11,22 +10,25 @@ import java.util.List; public final class IfRegion extends AbstractRegion { - protected BlockNode header; - protected IContainer thenRegion; - protected IContainer elseRegion; + private final BlockNode header; + + private IfCondition condition; + private IContainer thenRegion; + private IContainer elseRegion; public IfRegion(IRegion parent, BlockNode header) { super(parent); assert header.getInstructions().size() == 1; this.header = header; + this.condition = IfCondition.fromIfBlock(header); } - public IfNode getIfInsn() { - return (IfNode) header.getInstructions().get(0); + public IfCondition getCondition() { + return condition; } - public BlockNode getHeader() { - return header; + public void setCondition(IfCondition condition) { + this.condition = condition; } public IContainer getThenRegion() { @@ -58,6 +60,6 @@ public final class IfRegion extends AbstractRegion { @Override public String toString() { - return "IF(" + header + ") then " + thenRegion + " else " + elseRegion; + return "IF(" + condition + ") then " + thenRegion + " else " + elseRegion; } } diff --git a/src/main/java/jadx/dex/regions/LoopRegion.java b/src/main/java/jadx/dex/regions/LoopRegion.java index 9d78e7120..595a2da44 100644 --- a/src/main/java/jadx/dex/regions/LoopRegion.java +++ b/src/main/java/jadx/dex/regions/LoopRegion.java @@ -14,6 +14,7 @@ import java.util.List; public final class LoopRegion extends AbstractRegion { // loop header contains one 'if' insn, equals null for infinite loop + private final IfCondition condition; private final BlockNode conditionBlock; // instruction which must be executed before condition in every loop private BlockNode preCondition = null; @@ -23,21 +24,18 @@ public final class LoopRegion extends AbstractRegion { public LoopRegion(IRegion parent, BlockNode header, boolean reversed) { super(parent); this.conditionBlock = header; + this.condition = IfCondition.fromIfBlock(header); this.conditionAtEnd = reversed; } - public IfNode getIfInsn() { - return (IfNode) conditionBlock.getInstructions().get(conditionBlock.getInstructions().size() - 1); + public IfCondition getCondition() { + return condition; } public BlockNode getHeader() { return conditionBlock; } - public BlockNode getConditionBlock() { - return conditionBlock; - } - public IContainer getBody() { return body; } @@ -57,6 +55,10 @@ public final class LoopRegion extends AbstractRegion { this.preCondition = preCondition; } + private IfNode getIfInsn() { + return (IfNode) conditionBlock.getInstructions().get(conditionBlock.getInstructions().size() - 1); + } + /** * Check if pre-conditions can be inlined into loop condition */ diff --git a/src/main/java/jadx/dex/visitors/regions/CheckRegions.java b/src/main/java/jadx/dex/visitors/regions/CheckRegions.java index 9e54f3769..d96fb6e2b 100644 --- a/src/main/java/jadx/dex/visitors/regions/CheckRegions.java +++ b/src/main/java/jadx/dex/visitors/regions/CheckRegions.java @@ -41,10 +41,11 @@ public class CheckRegions extends AbstractVisitor { if (mth.getBasicBlocks().size() != blocksInRegions.size()) { for (BlockNode block : mth.getBasicBlocks()) { if (!blocksInRegions.contains(block)) { - if (!block.getInstructions().isEmpty()) { + if (!block.getInstructions().isEmpty() + && !block.getAttributes().contains(AttributeFlag.SKIP)) { mth.getAttributes().add(AttributeFlag.INCONSISTENT_CODE); if (Consts.DEBUG) - LOG.debug("Missing block: {} in {}", block, mth); + LOG.debug(" Missing block: {} in {}", block, mth); else break; } @@ -58,15 +59,14 @@ public class CheckRegions extends AbstractVisitor { public void enterRegion(MethodNode mth, IRegion region) { if (region instanceof LoopRegion) { LoopRegion loop = (LoopRegion) region; - if (loop.getConditionBlock() != null - && loop.getConditionBlock().getInstructions().size() != 1) { - ErrorsCounter.methodError(mth, "Incorrect condition in loop: " + loop.getConditionBlock()); + BlockNode loopHeader = loop.getHeader(); + if (loopHeader != null && loopHeader.getInstructions().size() != 1) { + ErrorsCounter.methodError(mth, "Incorrect condition in loop: " + loopHeader); mth.getAttributes().add(AttributeFlag.INCONSISTENT_CODE); } } } }; DepthRegionTraverser.traverseAll(mth, checkLoops); - } } diff --git a/src/main/java/jadx/dex/visitors/regions/RegionMaker.java b/src/main/java/jadx/dex/visitors/regions/RegionMaker.java index 03fe62186..3bbcd40fd 100644 --- a/src/main/java/jadx/dex/visitors/regions/RegionMaker.java +++ b/src/main/java/jadx/dex/visitors/regions/RegionMaker.java @@ -14,6 +14,7 @@ import jadx.dex.nodes.BlockNode; import jadx.dex.nodes.IRegion; import jadx.dex.nodes.InsnNode; import jadx.dex.nodes.MethodNode; +import jadx.dex.regions.IfCondition; import jadx.dex.regions.IfRegion; import jadx.dex.regions.LoopRegion; import jadx.dex.regions.Region; @@ -41,12 +42,20 @@ public class RegionMaker { private static final Logger LOG = LoggerFactory.getLogger(RegionMaker.class); private final MethodNode mth; + private final BitSet processedBlocks; public RegionMaker(MethodNode mth) { this.mth = mth; + this.processedBlocks = new BitSet(mth.getBasicBlocks().size()); } public Region makeRegion(BlockNode startBlock, RegionStack stack) { + int id = startBlock.getId(); + if (processedBlocks.get(id)) + LOG.debug(" Block already processed: " + startBlock + ", mth: " + mth); + else + processedBlocks.set(id); + Region r = new Region(stack.peekRegion()); BlockNode next = startBlock; while (next != null) { @@ -351,6 +360,11 @@ public class RegionMaker { ifnode.invertOp(bThen.getStartOffset()); + if (block.getAttributes().contains(AttributeFlag.SKIP)) { + // block already included in other if region + return bThen; + } + BlockNode out = null; BlockNode thenBlock; BlockNode elseBlock = null; @@ -358,13 +372,9 @@ public class RegionMaker { thenBlock = bThen; // select else and exit blocks if (block.getDominatesOn().size() == 2) { - if (bElse.getPredecessors().size() == 1) - elseBlock = bElse; - else - out = bElse; + elseBlock = bElse; } else { if (bElse.getPredecessors().size() != 1) { - elseBlock = null; out = bElse; } else { elseBlock = bElse; @@ -381,14 +391,47 @@ public class RegionMaker { out = null; } + IfRegion ifRegion = new IfRegion(currentRegion, block); + currentRegion.getSubBlocks().add(ifRegion); + + if (elseBlock != null) { + if (elseBlock.getPredecessors().size() > 1) { + // if else block shares between several 'if' instructions => merge conditions + for (BlockNode pred : elseBlock.getPredecessors()) { + if (!pred.equals(block)) { + List insns = pred.getInstructions(); + if (insns.size() == 1 && insns.get(0).getType() == InsnType.IF) { + IfNode otherNode = (IfNode) insns.get(0); + int elseOffset = elseBlock.getStartOffset(); + if (elseOffset != otherNode.getTarget()) { + otherNode.invertOp(elseOffset); + } + IfCondition newArg = IfCondition.fromIfBlock(pred); + IfCondition condition; + if (otherNode.getTarget() != pred.getStartOffset()) { + condition = IfCondition.and(ifRegion.getCondition(), newArg); + } else { + condition = IfCondition.or(ifRegion.getCondition(), newArg); + } + ifRegion.setCondition(condition); + pred.getAttributes().add(AttributeFlag.SKIP); + } + } + } + for (BlockNode d : block.getDominatesOn()) { + if (d != bThen && d != bElse) { + out = d; + break; + } + } + } + } + if (elseBlock != null) { if (stack.containsExit(elseBlock)) elseBlock = null; } - IfRegion ifRegion = new IfRegion(currentRegion, block); - currentRegion.getSubBlocks().add(ifRegion); - stack.push(ifRegion); stack.addExit(out); diff --git a/src/main/java/jadx/dex/visitors/regions/RegionStack.java b/src/main/java/jadx/dex/visitors/regions/RegionStack.java index aaedf9508..0c5fc58e7 100644 --- a/src/main/java/jadx/dex/visitors/regions/RegionStack.java +++ b/src/main/java/jadx/dex/visitors/regions/RegionStack.java @@ -4,28 +4,29 @@ import jadx.dex.nodes.BlockNode; import jadx.dex.nodes.IRegion; import jadx.dex.nodes.MethodNode; +import java.util.ArrayDeque; +import java.util.Deque; import java.util.HashSet; import java.util.Set; -import java.util.Stack; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class RegionStack { +final class RegionStack { private static final Logger LOG = LoggerFactory.getLogger(RegionStack.class); private static final boolean DEBUG = false; static { if (DEBUG) - LOG.debug("Debug enabled for " + RegionStack.class); + LOG.debug("Debug enabled for {}", RegionStack.class); } - private static class State { + private static final class State { final Set exits; IRegion region; public State() { - exits = new HashSet(); + exits = new HashSet(4); } private State(State c) { @@ -38,17 +39,17 @@ public class RegionStack { @Override public String toString() { - return "Exits: " + exits; + return "Region: " + region + ", exits: " + exits; } } - private final Stack stack; + private final Deque stack; private State curState; public RegionStack(MethodNode mth) { if (DEBUG) LOG.debug("New RegionStack: {}", mth); - this.stack = new Stack(); + this.stack = new ArrayDeque(); this.curState = new State(); } @@ -59,16 +60,14 @@ public class RegionStack { curState = curState.copy(); curState.region = region; - if (DEBUG) { - LOG.debug("Stack push: {} = {}", region, curState); - LOG.debug("Stack size: {}", size()); - } + if (DEBUG) + LOG.debug("Stack push: {}: {}", size(), curState); } public void pop() { curState = stack.pop(); if (DEBUG) - LOG.debug("Stack pop : {}", curState); + LOG.debug("Stack pop: {}: {}", size(), curState); } /** diff --git a/src/samples/java/jadx/samples/AbstractTest.java b/src/samples/java/jadx/samples/AbstractTest.java index d2067a861..f0613d52c 100644 --- a/src/samples/java/jadx/samples/AbstractTest.java +++ b/src/samples/java/jadx/samples/AbstractTest.java @@ -10,6 +10,12 @@ public abstract class AbstractTest { } } + public static void assertFalse(boolean condition) { + if (condition) { + throw new AssertionError(); + } + } + public static void assertTrue(boolean condition, String msg) { if (!condition) { throw new AssertionError(msg); diff --git a/src/samples/java/jadx/samples/RunTests.java b/src/samples/java/jadx/samples/RunTests.java index fc68deafa..b6a1da43d 100644 --- a/src/samples/java/jadx/samples/RunTests.java +++ b/src/samples/java/jadx/samples/RunTests.java @@ -22,7 +22,7 @@ public class RunTests { System.exit(1); } - int timeout = 3 * clsList.size(); + int timeout = 2 * clsList.size(); System.err.println("Set timeout to " + timeout + " seconds"); new Timer().schedule(new TerminateTask(), timeout * 1000); @@ -50,9 +50,8 @@ public class RunTests { Class cls = Class.forName(clsName); if (cls.getSuperclass() == AbstractTest.class) { Method mth = cls.getMethod("testRun"); - - AbstractTest test = (AbstractTest) cls.getConstructor().newInstance(); try { + AbstractTest test = (AbstractTest) cls.getConstructor().newInstance(); pass = (Boolean) mth.invoke(test); } catch (InvocationTargetException e) { pass = false; diff --git a/src/samples/java/jadx/samples/TestConditions.java b/src/samples/java/jadx/samples/TestConditions.java new file mode 100644 index 000000000..1afe44125 --- /dev/null +++ b/src/samples/java/jadx/samples/TestConditions.java @@ -0,0 +1,57 @@ +package jadx.samples; + +/** + * Failed tests for current jadx version + */ +public class TestConditions extends AbstractTest { + + public int test1(int num) { + boolean inRange = (num >= 59 && num <= 66); + if (inRange) { + num++; + } + return num; + } + + public int test1a(int num) { + boolean notInRange = (num < 59 || num > 66); + if (notInRange) { + num--; + } + return num; + } + + public int test1b(int num) { + boolean inc = (num >= 59 && num <= 66 && num != 62); + if (inc) { + num++; + } + return num; + } + + public boolean test1c(int num) { + return num == 4 || num == 6; + } + + @Override + public boolean testRun() throws Exception { + TestConditions c = new TestConditions(); + + assertEquals(c.test1(50), 50); + assertEquals(c.test1(60), 61); + + assertEquals(c.test1a(50), 49); + assertEquals(c.test1a(60), 60); + + assertEquals(c.test1b(60), 61); + assertEquals(c.test1b(62), 62); + + assertTrue(c.test1c(4)); + assertFalse(c.test1c(5)); + return true; + } + + public static void main(String[] args) throws Exception { + new TestConditions().testRun(); + } +} diff --git a/src/samples/java/jadx/samples/TestStringProcessing.java b/src/samples/java/jadx/samples/TestStringProcessing.java index 797a4040d..c32928492 100644 --- a/src/samples/java/jadx/samples/TestStringProcessing.java +++ b/src/samples/java/jadx/samples/TestStringProcessing.java @@ -2,14 +2,23 @@ package jadx.samples; public class TestStringProcessing extends AbstractTest { - @Override - public boolean testRun() { + public void testStringEscape() { String str = "test\tstr\n"; assertTrue(str.length() == 9); str = "test\bunicode\u1234"; assertTrue(str.charAt(4) == '\b'); - return true; } + public void testStringConcat() { + String s = "1"; + assertEquals("a" + s, "a1"); + } + + @Override + public boolean testRun() throws Exception { + testStringEscape(); + testStringConcat(); + return true; + } }