From b2f189b5729198d623a5058c99c2d190ef911844 Mon Sep 17 00:00:00 2001 From: Skylot Date: Tue, 12 Nov 2013 21:00:05 +0400 Subject: [PATCH] core: process complex condition in loop header --- .../main/java/jadx/core/codegen/ClassGen.java | 10 + .../java/jadx/core/codegen/MethodGen.java | 15 +- .../jadx/core/dex/instructions/IfNode.java | 39 ++- .../jadx/core/dex/regions/IfCondition.java | 29 ++- .../java/jadx/core/dex/regions/IfInfo.java | 42 ++++ .../jadx/core/dex/regions/LoopRegion.java | 12 +- .../dex/visitors/BlockProcessingHelper.java | 29 ++- .../dex/visitors/regions/RegionMaker.java | 238 +++++++++++------- .../main/java/jadx/core/utils/InsnUtils.java | 6 +- .../test/java/jadx/api/InternalJadxTest.java | 4 + .../tests/internal/TestLoopCondition.java | 54 ++++ .../src/main/java/jadx/samples/TestCF3.java | 52 +++- 12 files changed, 404 insertions(+), 126 deletions(-) create mode 100644 jadx-core/src/main/java/jadx/core/dex/regions/IfInfo.java create mode 100644 jadx-core/src/test/java/jadx/tests/internal/TestLoopCondition.java diff --git a/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java b/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java index 93627870b..fc1202ae9 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java @@ -30,9 +30,14 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.android.dx.rop.code.AccessFlags; public class ClassGen { + private static final Logger LOG = LoggerFactory.getLogger(ClassGen.class); + private final ClassNode cls; private final ClassGen parentGen; private final AnnotationGen annotationGen; @@ -232,6 +237,11 @@ public class ClassGen { continue; MethodGen mthGen = new MethodGen(this, mth); + if (mth.getAttributes().contains(AttributeFlag.INCONSISTENT_CODE)) { + code.startLine("/* JADX WARNING: inconsistent code */"); + LOG.error(ErrorsCounter.formatErrorMsg(mth, " Inconsistent code")); + mthGen.makeMethodDump(code); + } mthGen.addDefinition(code); code.add(" {"); insertSourceFileInfo(code, mth); diff --git a/jadx-core/src/main/java/jadx/core/codegen/MethodGen.java b/jadx-core/src/main/java/jadx/core/codegen/MethodGen.java index 87b492cd9..1f6990641 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/MethodGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/MethodGen.java @@ -1,7 +1,6 @@ package jadx.core.codegen; import jadx.core.Consts; -import jadx.core.dex.attributes.AttributeFlag; import jadx.core.dex.attributes.AttributeType; import jadx.core.dex.attributes.AttributesList; import jadx.core.dex.attributes.JadxErrorAttr; @@ -61,11 +60,6 @@ public class MethodGen { if (mth.getMethodInfo().isClassInit()) { code.startLine("static"); } else { - if (mth.getAttributes().contains(AttributeFlag.INCONSISTENT_CODE) - && !mth.getAttributes().contains(AttributeType.JADX_ERROR)) { - code.startLine("// jadx: inconsistent code"); - } - annotationGen.addForMethod(code, mth); AccessInfo clsAccFlags = mth.getParentClass().getAccessFlags(); @@ -235,16 +229,11 @@ public class MethodGen { code.startLine("Error: ").add(Utils.getStackTrace(cause)); code.add("*/"); } - makeMethodDump(code, mth); + makeMethodDump(code); } else { if (mth.getRegion() != null) { CodeWriter insns = new CodeWriter(mthIndent + 1); (new RegionGen(this, mth)).makeRegion(insns, mth.getRegion()); - - if (mth.getAttributes().contains(AttributeFlag.INCONSISTENT_CODE)) { - LOG.debug(ErrorsCounter.formatErrorMsg(mth, " Inconsistent code")); - // makeMethodDump(code, mth); - } code.add(insns); } else { makeFallbackMethod(code, mth); @@ -253,7 +242,7 @@ public class MethodGen { return code; } - public void makeMethodDump(CodeWriter code, MethodNode mth) { + public void makeMethodDump(CodeWriter code) { code.startLine("/*"); getFallbackMethodGen(mth).addDefinition(code); code.add(" {"); diff --git a/jadx-core/src/main/java/jadx/core/dex/instructions/IfNode.java b/jadx-core/src/main/java/jadx/core/dex/instructions/IfNode.java index b8f812b2c..d0a9d1831 100644 --- a/jadx-core/src/main/java/jadx/core/dex/instructions/IfNode.java +++ b/jadx-core/src/main/java/jadx/core/dex/instructions/IfNode.java @@ -4,15 +4,22 @@ import jadx.core.dex.instructions.args.ArgType; import jadx.core.dex.instructions.args.InsnArg; import jadx.core.dex.instructions.args.LiteralArg; import jadx.core.dex.instructions.args.PrimitiveType; +import jadx.core.dex.nodes.BlockNode; import jadx.core.utils.InsnUtils; import com.android.dx.io.instructions.DecodedInstruction; +import static jadx.core.utils.BlockUtils.getBlockByOffset; +import static jadx.core.utils.BlockUtils.selectOther; + public class IfNode extends GotoNode { protected boolean zeroCmp; protected IfOp op; + private BlockNode thenBlock; + private BlockNode elseBlock; + public IfNode(int targ, InsnArg then, InsnArg els) { super(InsnType.IF, targ); addArg(then); @@ -49,9 +56,12 @@ public class IfNode extends GotoNode { return zeroCmp; } - public void invertOp(int targ) { + public void invertCondition() { op = op.invert(); - target = targ; + BlockNode tmp = thenBlock; + thenBlock = elseBlock; + elseBlock = tmp; + target = thenBlock.getStartOffset(); } public void changeCondition(InsnArg arg1, InsnArg arg2, IfOp op) { @@ -59,19 +69,38 @@ public class IfNode extends GotoNode { this.zeroCmp = arg2.isLiteral() && ((LiteralArg) arg2).getLiteral() == 0; setArg(0, arg1); if (!zeroCmp) { - if (getArgsCount() == 2) + if (getArgsCount() == 2) { setArg(1, arg2); - else + } else { addArg(arg2); + } } } + public void initBlocks(BlockNode curBlock) { + thenBlock = getBlockByOffset(target, curBlock.getSuccessors()); + if (curBlock.getSuccessors().size() == 1) { + elseBlock = thenBlock; + } else { + elseBlock = selectOther(thenBlock, curBlock.getSuccessors()); + } + target = thenBlock.getStartOffset(); + } + + public BlockNode getThenBlock() { + return thenBlock; + } + + public BlockNode getElseBlock() { + return elseBlock; + } + @Override public String toString() { return InsnUtils.formatOffset(offset) + ": " + InsnUtils.insnTypeToString(insnType) + getArg(0) + " " + op.getSymbol() + " " + (zeroCmp ? "0" : getArg(1)) - + " -> " + InsnUtils.formatOffset(target); + + " -> " + (thenBlock != null ? thenBlock : InsnUtils.formatOffset(target)); } } diff --git a/jadx-core/src/main/java/jadx/core/dex/regions/IfCondition.java b/jadx-core/src/main/java/jadx/core/dex/regions/IfCondition.java index d22a05c7b..2b21196c3 100644 --- a/jadx-core/src/main/java/jadx/core/dex/regions/IfCondition.java +++ b/jadx-core/src/main/java/jadx/core/dex/regions/IfCondition.java @@ -12,9 +12,9 @@ import java.util.List; public final class IfCondition { public static IfCondition fromIfBlock(BlockNode header) { - if (header == null) + if (header == null) { return null; - + } return fromIfNode((IfNode) header.getInstructions().get(0)); } @@ -39,8 +39,8 @@ public final class IfCondition { public static final class Compare { private final IfNode insn; - public Compare(IfNode ifNode) { - this.insn = ifNode; + public Compare(IfNode insn) { + this.insn = insn; } public IfOp getOp() { @@ -57,6 +57,10 @@ public final class IfCondition { else return insn.getArg(1); } + public Compare invert() { + insn.invertCondition(); + return this; + } @Override public String toString() { @@ -113,6 +117,23 @@ public final class IfCondition { return compare; } + public IfCondition invert() { + switch (mode) { + case COMPARE: + return new IfCondition(compare.invert()); + case NOT: + return new IfCondition(args.get(0)); + case AND: + case OR: + List newArgs = new ArrayList(args.size()); + for (IfCondition arg : args) { + newArgs.add(arg.invert()); + } + return new IfCondition(mode == Mode.AND ? Mode.OR : Mode.AND, newArgs); + } + throw new RuntimeException("Unknown mode for invert: " + mode); + } + @Override public String toString() { switch (mode) { diff --git a/jadx-core/src/main/java/jadx/core/dex/regions/IfInfo.java b/jadx-core/src/main/java/jadx/core/dex/regions/IfInfo.java new file mode 100644 index 000000000..eb7081719 --- /dev/null +++ b/jadx-core/src/main/java/jadx/core/dex/regions/IfInfo.java @@ -0,0 +1,42 @@ +package jadx.core.dex.regions; + +import jadx.core.dex.nodes.BlockNode; + +public final class IfInfo { + IfCondition condition; + BlockNode ifnode; + BlockNode thenBlock; + BlockNode elseBlock; + + public IfCondition getCondition() { + return condition; + } + + public void setCondition(IfCondition condition) { + this.condition = condition; + } + + public BlockNode getIfnode() { + return ifnode; + } + + public void setIfnode(BlockNode ifnode) { + this.ifnode = ifnode; + } + + public BlockNode getThenBlock() { + return thenBlock; + } + + public void setThenBlock(BlockNode thenBlock) { + this.thenBlock = thenBlock; + } + + public BlockNode getElseBlock() { + return elseBlock; + } + + public void setElseBlock(BlockNode elseBlock) { + this.elseBlock = elseBlock; + } +} diff --git a/jadx-core/src/main/java/jadx/core/dex/regions/LoopRegion.java b/jadx-core/src/main/java/jadx/core/dex/regions/LoopRegion.java index e5f4e890a..9d7332f58 100644 --- a/jadx-core/src/main/java/jadx/core/dex/regions/LoopRegion.java +++ b/jadx-core/src/main/java/jadx/core/dex/regions/LoopRegion.java @@ -14,8 +14,8 @@ 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; + private IfCondition condition; + private BlockNode conditionBlock; // instruction which must be executed before condition in every loop private BlockNode preCondition = null; private IContainer body; @@ -32,10 +32,18 @@ public final class LoopRegion extends AbstractRegion { return condition; } + public void setCondition(IfCondition condition) { + this.condition = condition; + } + public BlockNode getHeader() { return conditionBlock; } + public void setHeader(BlockNode conditionBlock) { + this.conditionBlock = conditionBlock; + } + public IContainer getBody() { return body; } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/BlockProcessingHelper.java b/jadx-core/src/main/java/jadx/core/dex/visitors/BlockProcessingHelper.java index 95762732e..bafdeab72 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/BlockProcessingHelper.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/BlockProcessingHelper.java @@ -1,6 +1,7 @@ package jadx.core.dex.visitors; import jadx.core.dex.attributes.AttributeType; +import jadx.core.dex.instructions.IfNode; import jadx.core.dex.instructions.InsnType; import jadx.core.dex.instructions.args.ArgType; import jadx.core.dex.instructions.args.NamedArg; @@ -14,17 +15,21 @@ import jadx.core.dex.trycatch.ExceptionHandler; import jadx.core.dex.trycatch.TryCatchBlock; import jadx.core.utils.BlockUtils; +import java.util.List; + public class BlockProcessingHelper { public static void visit(MethodNode mth) { - if (mth.isNoCode()) + if (mth.isNoCode()) { return; + } for (BlockNode block : mth.getBasicBlocks()) { markExceptionHandlers(block); } for (BlockNode block : mth.getBasicBlocks()) { block.updateCleanSuccessors(); + initBlocksInIfNodes(block); } for (BlockNode block : mth.getBasicBlocks()) { processExceptionHandlers(mth, block); @@ -77,11 +82,13 @@ public class BlockProcessingHelper { // remove 'monitor-exit' from exception handler blocks InstructionRemover remover = new InstructionRemover(excBlock.getInstructions()); for (InsnNode insn : excBlock.getInstructions()) { - if (insn.getType() == InsnType.MONITOR_ENTER) + if (insn.getType() == InsnType.MONITOR_ENTER) { break; + } - if (insn.getType() == InsnType.MONITOR_EXIT) + if (insn.getType() == InsnType.MONITOR_EXIT) { remover.add(insn); + } } remover.perform(); @@ -108,8 +115,9 @@ public class BlockProcessingHelper { CatchAttr commonCatchAttr = null; for (InsnNode insn : block.getInstructions()) { CatchAttr catchAttr = (CatchAttr) insn.getAttributes().get(AttributeType.CATCH_BLOCK); - if (catchAttr == null) + if (catchAttr == null) { continue; + } if (commonCatchAttr == null) { commonCatchAttr = catchAttr; @@ -137,4 +145,17 @@ public class BlockProcessingHelper { } } } + + /** + * Init 'then' and 'else' blocks for 'if' instruction. + */ + private static void initBlocksInIfNodes(BlockNode block) { + List instructions = block.getInstructions(); + if (instructions.size() == 1) { + InsnNode insn = instructions.get(0); + if (insn.getType() == InsnType.IF) { + ((IfNode) insn).initBlocks(block); + } + } + } } 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 448e159aa..fb7f77018 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 @@ -17,6 +17,7 @@ import jadx.core.dex.nodes.IRegion; import jadx.core.dex.nodes.InsnNode; import jadx.core.dex.nodes.MethodNode; import jadx.core.dex.regions.IfCondition; +import jadx.core.dex.regions.IfInfo; import jadx.core.dex.regions.IfRegion; import jadx.core.dex.regions.LoopRegion; import jadx.core.dex.regions.Region; @@ -61,10 +62,11 @@ public class RegionMaker { public Region makeRegion(BlockNode startBlock, RegionStack stack) { if (Consts.DEBUG) { int id = startBlock.getId(); - if (processedBlocks.get(id)) + if (processedBlocks.get(id)) { LOG.debug(" Block already processed: " + startBlock + ", mth: " + mth); - else + } else { processedBlocks.set(id); + } } Region r = new Region(stack.peekRegion()); @@ -133,10 +135,11 @@ public class RegionMaker { next = BlockUtils.getNextBlock(block); } - if (next != null && !stack.containsExit(block) && !stack.containsExit(next)) + if (next != null && !stack.containsExit(block) && !stack.containsExit(next)) { return next; - else + } else { return null; + } } private BlockNode processLoop(IRegion curRegion, LoopAttr loop, RegionStack stack) { @@ -149,21 +152,29 @@ public class RegionMaker { // this can help if loop have several exits (after using 'break' or 'return' in loop) List exitBlocks = new ArrayList(exitBlocksSet.size()); BlockNode nextStart = BlockUtils.getNextBlock(loopStart); - if (nextStart != null && exitBlocksSet.remove(nextStart)) + if (nextStart != null && exitBlocksSet.remove(nextStart)) { exitBlocks.add(nextStart); - if (exitBlocksSet.remove(loop.getEnd())) + } + if (exitBlocksSet.remove(loop.getEnd())) { exitBlocks.add(loop.getEnd()); - if (exitBlocksSet.remove(loopStart)) + } + if (exitBlocksSet.remove(loopStart)) { exitBlocks.add(loopStart); + } exitBlocks.addAll(exitBlocksSet); exitBlocksSet = null; - BlockNode condBlock = null; // exit block with loop condition + // exit block with loop condition + BlockNode condBlock = null; + + // first block in loop + BlockNode bThen = null; for (BlockNode exit : exitBlocks) { if (exit.getAttributes().contains(AttributeType.EXC_HANDLER) - || exit.getInstructions().size() != 1) + || exit.getInstructions().size() != 1) { continue; + } InsnNode insn = exit.getInstructions().get(0); if (insn.getType() == InsnType.IF) { @@ -190,8 +201,24 @@ public class RegionMaker { loopRegion = null; condBlock = null; // try another exit - } else + } else { + List merged = new ArrayList(2); + IfInfo mergedIf = mergeNestedIfNodes(condBlock, + ifnode.getThenBlock(), ifnode.getElseBlock(), merged); + if (mergedIf != null) { + condBlock = mergedIf.getIfnode(); + if (!loop.getLoopBlocks().contains(mergedIf.getThenBlock())) { + // invert loop condition if it points to exit + loopRegion.setCondition(mergedIf.getCondition().invert()); + bThen = mergedIf.getElseBlock(); + } else { + loopRegion.setCondition(mergedIf.getCondition()); + bThen = mergedIf.getThenBlock(); + } + exitBlocks.removeAll(merged); + } break; + } } } @@ -203,17 +230,19 @@ public class RegionMaker { loopStart.getAttributes().remove(AttributeType.LOOP); stack.push(loopRegion); Region body = makeRegion(loopStart, stack); - if (!RegionUtils.isRegionContainsBlock(body, loop.getEnd())) + if (!RegionUtils.isRegionContainsBlock(body, loop.getEnd())) { body.getSubBlocks().add(loop.getEnd()); + } loopRegion.setBody(body); stack.pop(); loopStart.getAttributes().add(loop); BlockNode next = BlockUtils.getNextBlock(loop.getEnd()); - if (!RegionUtils.isRegionContainsBlock(body, next)) + if (!RegionUtils.isRegionContainsBlock(body, next)) { return next; - else + } else { return null; + } } stack.push(loopRegion); @@ -250,10 +279,13 @@ public class RegionMaker { } } - BlockNode bThen = getBlockByOffset(ifnode.getTarget(), condBlock.getSuccessors()); + if (bThen == null) { + bThen = ifnode.getThenBlock(); + } + BlockNode out; if (loopRegion.isConditionAtEnd()) { - BlockNode bElse = selectOther(bThen, condBlock.getSuccessors()); + BlockNode bElse = ifnode.getElseBlock(); out = (bThen == loopStart ? bElse : bThen); loopStart.getAttributes().remove(AttributeType.LOOP); @@ -269,9 +301,9 @@ public class RegionMaker { break; } } - if (bThen != loopBody) - ifnode.invertOp(bThen.getStartOffset()); - + if (bThen != loopBody) { + loopRegion.setCondition(loopRegion.getCondition().invert()); + } out = selectOther(loopBody, condBlock.getSuccessors()); AttributesList outAttrs = out.getAttributes(); if (outAttrs.contains(AttributeFlag.LOOP_START) @@ -286,7 +318,7 @@ public class RegionMaker { return out; } - private static final Set cacheSet = new HashSet(); + private final Set cacheSet = new HashSet(); private BlockNode processMonitorEnter(IRegion curRegion, BlockNode block, InsnNode insn, RegionStack stack) { SynchronizedRegion synchRegion = new SynchronizedRegion(curRegion, insn); @@ -328,8 +360,9 @@ public class RegionMaker { } } for (BlockNode node : block.getCleanSuccessors()) { - if (!visited.contains(node)) + if (!visited.contains(node)) { traverseMonitorExits(arg, node, exits, visited); + } } } @@ -347,33 +380,28 @@ public class RegionMaker { break; } } - if (cross) + if (cross) { return node; - + } if (!visited.contains(node)) { BlockNode res = traverseMonitorExitsCross(node, exits, visited); - if (res != null) + if (res != null) { return res; + } } } return null; } private BlockNode processIf(IRegion currentRegion, BlockNode block, IfNode ifnode, RegionStack stack) { - BlockNode bThen = getBlockByOffset(ifnode.getTarget(), block.getSuccessors()); + BlockNode bThen = ifnode.getThenBlock(); + BlockNode bElse = ifnode.getElseBlock(); + if (block.getAttributes().contains(AttributeFlag.SKIP)) { // block already included in other if region return bThen; } - BlockNode bElse; - if (block.getSuccessors().size() == 1) { - // TODO eliminate useless 'if' instruction - bElse = bThen; - } else { - bElse = selectOther(bThen, block.getSuccessors()); - } - BlockNode out = null; BlockNode thenBlock = null; BlockNode elseBlock = null; @@ -388,63 +416,19 @@ public class RegionMaker { IfRegion ifRegion = new IfRegion(currentRegion, block); currentRegion.getSubBlocks().add(ifRegion); - // merge nested if nodes - boolean found; - do { - found = false; - for (BlockNode succ : block.getSuccessors()) { - BlockNode nestedIfBlock = getIfNode(succ); - if (nestedIfBlock != null && nestedIfBlock != block) { - IfNode nestedIfInsn = (IfNode) nestedIfBlock.getInstructions().get(0); - BlockNode nbThen = getBlockByOffset(nestedIfInsn.getTarget(), nestedIfBlock.getSuccessors()); - BlockNode nbElse = selectOther(nbThen, nestedIfBlock.getSuccessors()); - - IfCondition condition; - boolean inverted = false; - IfCondition nestedCondition = IfCondition.fromIfNode(nestedIfInsn); - if (isPathExists(bElse, nestedIfBlock)) { - // else branch - if (bThen != nbThen) { - if (bThen != nbElse) { - break; // not connected conditions - } - nestedIfInsn.invertOp(nbElse.getStartOffset()); - inverted = true; - } - condition = IfCondition.merge(Mode.OR, ifRegion.getCondition(), nestedCondition); - } else { - // then branch - if (bElse != nbElse) { - if (bElse != nbThen) { - break; // not connected conditions - } - nestedIfInsn.invertOp(nbElse.getStartOffset()); - inverted = true; - } - condition = IfCondition.merge(Mode.AND, ifRegion.getCondition(), nestedCondition); - } - ifRegion.setCondition(condition); - nestedIfBlock.getAttributes().add(AttributeFlag.SKIP); - // set new blocks - if (inverted) { - thenBlock = nbElse; - elseBlock = nbThen; - } else { - thenBlock = nbThen; - elseBlock = nbElse; - } - found = true; - block = nestedIfBlock; - bThen = thenBlock; - bElse = elseBlock; - break; - } - } - } while (found); + IfInfo mergedIf = mergeNestedIfNodes(block, bThen, bElse, null); + if (mergedIf != null) { + block = mergedIf.getIfnode(); + ifRegion.setCondition(mergedIf.getCondition()); + thenBlock = mergedIf.getThenBlock(); + elseBlock = mergedIf.getElseBlock(); + bThen = thenBlock; + bElse = elseBlock; + } if (thenBlock == null) { // invert condition (compiler often do it) - ifnode.invertOp(bElse.getStartOffset()); + ifnode.invertCondition(); BlockNode tmp = bThen; bThen = bElse; bElse = tmp; @@ -466,15 +450,15 @@ public class RegionMaker { } } } - if (BlockUtils.isBackEdge(block, out)) { out = null; } } if (elseBlock != null) { - if (stack.containsExit(elseBlock)) + if (stack.containsExit(elseBlock)) { elseBlock = null; + } } stack.push(ifRegion); @@ -487,6 +471,72 @@ public class RegionMaker { return out; } + private IfInfo mergeNestedIfNodes(BlockNode block, BlockNode bThen, BlockNode bElse, List merged) { + if (bThen == bElse) { + return null; + } + + boolean found; + IfCondition condition = IfCondition.fromIfBlock(block); + IfInfo result = null; + do { + found = false; + for (BlockNode succ : block.getSuccessors()) { + BlockNode nestedIfBlock = getIfNode(succ); + if (nestedIfBlock != null && nestedIfBlock != block) { + IfNode nestedIfInsn = (IfNode) nestedIfBlock.getInstructions().get(0); + BlockNode nbThen = nestedIfInsn.getThenBlock(); + BlockNode nbElse = nestedIfInsn.getElseBlock(); + + boolean inverted = false; + IfCondition nestedCondition = IfCondition.fromIfNode(nestedIfInsn); + if (isPathExists(bElse, nestedIfBlock)) { + // else branch + if (bThen != nbThen) { + if (bThen != nbElse) { + // not connected conditions + break; + } + nestedIfInsn.invertCondition(); + inverted = true; + } + condition = IfCondition.merge(Mode.OR, condition, nestedCondition); + } else { + // then branch + if (bElse != nbElse) { + if (bElse != nbThen) { + // not connected conditions + break; + } + nestedIfInsn.invertCondition(); + inverted = true; + } + condition = IfCondition.merge(Mode.AND, condition, nestedCondition); + } + result = new IfInfo(); + result.setCondition(condition); + nestedIfBlock.getAttributes().add(AttributeFlag.SKIP); + if (merged != null) { + merged.add(nestedIfBlock); + } + + // set new blocks + result.setIfnode(nestedIfBlock); + result.setThenBlock(inverted ? nbElse : nbThen); + result.setElseBlock(inverted ? nbThen : nbElse); + + found = true; + block = nestedIfBlock; + bThen = result.getThenBlock(); + bElse = result.getElseBlock(); + break; + } + } + } while (found); + + return result; + } + private BlockNode getIfNode(BlockNode block) { if (block != null && !block.getAttributes().contains(AttributeType.LOOP)) { List insns = block.getInstructions(); @@ -571,9 +621,11 @@ public class RegionMaker { // filter successors of other blocks for (int i = domsOn.nextSetBit(0); i >= 0; i = domsOn.nextSetBit(i + 1)) { BlockNode b = mth.getBasicBlocks().get(i); - for (BlockNode s : b.getCleanSuccessors()) - if (domsOn.get(s.getId())) + for (BlockNode s : b.getCleanSuccessors()) { + if (domsOn.get(s.getId())) { domsOn.clear(s.getId()); + } + } } outCount = domsOn.cardinality(); } @@ -590,8 +642,9 @@ public class RegionMaker { if (out != null) { stack.addExit(out); } else { - for (BlockNode e : BlockUtils.bitsetToBlocks(mth, domsOn)) + for (BlockNode e : BlockUtils.bitsetToBlocks(mth, domsOn)) { stack.addExit(e); + } } if (!stack.containsExit(defCase)) { @@ -619,8 +672,9 @@ public class RegionMaker { } BlockNode out = BlockUtils.traverseWhileDominates(start, start); - if (out != null) + if (out != null) { stack.addExit(out); + } // TODO extract finally part which exists in all handlers from same try block // TODO add blocks common for several handlers to some region handler.setHandlerRegion(makeRegion(start, stack)); diff --git a/jadx-core/src/main/java/jadx/core/utils/InsnUtils.java b/jadx-core/src/main/java/jadx/core/utils/InsnUtils.java index 2e9b06fef..a263dede1 100644 --- a/jadx-core/src/main/java/jadx/core/utils/InsnUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/InsnUtils.java @@ -23,7 +23,11 @@ public class InsnUtils { } public static String formatOffset(int offset) { - return String.format("0x%04x", offset); + if (offset < 0) { + return "?"; + } else { + return String.format("0x%04x", offset); + } } public static String insnTypeToString(InsnType type) { diff --git a/jadx-core/src/test/java/jadx/api/InternalJadxTest.java b/jadx-core/src/test/java/jadx/api/InternalJadxTest.java index b4449c130..b332f7c3d 100644 --- a/jadx-core/src/test/java/jadx/api/InternalJadxTest.java +++ b/jadx-core/src/test/java/jadx/api/InternalJadxTest.java @@ -125,4 +125,8 @@ public abstract class InternalJadxTest { } } } + + protected void setOutputCFG() { + this.outputCFG = true; + } } diff --git a/jadx-core/src/test/java/jadx/tests/internal/TestLoopCondition.java b/jadx-core/src/test/java/jadx/tests/internal/TestLoopCondition.java new file mode 100644 index 000000000..c53b141b0 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/internal/TestLoopCondition.java @@ -0,0 +1,54 @@ +package jadx.tests.internal; + +import jadx.api.InternalJadxTest; +import jadx.core.dex.nodes.ClassNode; + +import org.junit.Test; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.junit.Assert.assertThat; + +public class TestLoopCondition extends InternalJadxTest { + + @SuppressWarnings("serial") + public static class TestCls extends Exception { + public String f; + + private void setEnabled(boolean r1z) { + } + + private void testIfInLoop() { + int j = 0; + for (int i = 0; i < f.length(); i++) { + char ch = f.charAt(i); + if (ch == '/') { + j++; + if (j == 2) { + setEnabled(true); + return; + } + } + } + setEnabled(false); + } + + public int testComplexIfInLoop(boolean a) { + int i = 0; + while (a && i < 10) { + i++; + } + return i; + } + } + + @Test + public void test() { + setOutputCFG(); + + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + + assertThat(code, containsString("i < f.length()")); + assertThat(code, containsString("while (a && i < 10) {")); + } +} diff --git a/jadx-samples/src/main/java/jadx/samples/TestCF3.java b/jadx-samples/src/main/java/jadx/samples/TestCF3.java index 6a8943b17..1f51ed2df 100644 --- a/jadx-samples/src/main/java/jadx/samples/TestCF3.java +++ b/jadx-samples/src/main/java/jadx/samples/TestCF3.java @@ -60,15 +60,17 @@ public class TestCF3 extends AbstractTest { while (it2.hasNext()) { String s2 = it2.next(); if (s1.equals(s2)) { - if (s1.length() == 5) + if (s1.length() == 5) { l2.add(s1); - else + } else { l1.remove(s2); + } } } } - if (l2.size() > 0) + if (l2.size() > 0) { l1.clear(); + } return l1.size() == 0; } @@ -111,8 +113,9 @@ public class TestCF3 extends AbstractTest { Iterator it = list.iterator(); while (it.hasNext()) { String ver = it.next(); - if (ver != null) + if (ver != null) { return ver; + } } return "error"; } @@ -123,8 +126,9 @@ public class TestCF3 extends AbstractTest { while (it.hasNext()) { String ver = it.next(); exc(); - if (ver != null) + if (ver != null) { return ver; + } } } catch (Exception e) { setEnabled(false); @@ -132,6 +136,34 @@ public class TestCF3 extends AbstractTest { return "error"; } + public int testComplexIfInLoop(boolean a) { + int i = 0; + while (a && i < 10) { + i++; + } + return i; + } + + public int testComplexIfInLoop2(int k) { + int i = k; + while (i > 5 && i < 10) { + i++; + } + return i; + } + + public int testComplexIfInLoop3(int k) { + int i = k; + while (i > 5 && i < k * 3) { + if (k == 8) { + i++; + } else { + break; + } + } + return i; + } + @Override public boolean testRun() throws Exception { setEnabled(false); @@ -158,6 +190,16 @@ public class TestCF3 extends AbstractTest { // assertEquals(testReturnInLoop2(list2), "error"); // assertTrue(testLabeledBreakContinue()); + + assertEquals(testComplexIfInLoop(false), 0); + assertEquals(testComplexIfInLoop(true), 10); + + assertEquals(testComplexIfInLoop2(2), 2); + assertEquals(testComplexIfInLoop2(6), 10); + + assertEquals(testComplexIfInLoop3(2), 2); + assertEquals(testComplexIfInLoop3(6), 6); + assertEquals(testComplexIfInLoop3(8), 24); return true; }