diff --git a/jadx-core/src/main/java/jadx/core/codegen/RegionGen.java b/jadx-core/src/main/java/jadx/core/codegen/RegionGen.java index 1b9490634..73dfc0be7 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/RegionGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/RegionGen.java @@ -93,14 +93,10 @@ public class RegionGen extends InsnGen { for (InsnNode insn : block.getInstructions()) { makeInsn(insn, code); } - if (block.getAttributes().contains(AttributeFlag.BREAK)) { - code.startLine("break;"); - } else { - IAttribute attr; - if ((attr = block.getAttributes().get(AttributeType.FORCE_RETURN)) != null) { - ForceReturnAttr retAttr = (ForceReturnAttr) attr; - makeInsn(retAttr.getReturnInsn(), code); - } + IAttribute attr; + if ((attr = block.getAttributes().get(AttributeType.FORCE_RETURN)) != null) { + ForceReturnAttr retAttr = (ForceReturnAttr) attr; + makeInsn(retAttr.getReturnInsn(), code); } } diff --git a/jadx-core/src/main/java/jadx/core/dex/attributes/AttributeFlag.java b/jadx-core/src/main/java/jadx/core/dex/attributes/AttributeFlag.java index 23f17bd17..7d7d63f07 100644 --- a/jadx-core/src/main/java/jadx/core/dex/attributes/AttributeFlag.java +++ b/jadx-core/src/main/java/jadx/core/dex/attributes/AttributeFlag.java @@ -9,7 +9,6 @@ public enum AttributeFlag { SYNTHETIC, - BREAK, RETURN, // block contains only return instruction DECLARE_VAR, diff --git a/jadx-core/src/main/java/jadx/core/dex/attributes/LoopAttr.java b/jadx-core/src/main/java/jadx/core/dex/attributes/LoopAttr.java index ed17523da..f87c15c9d 100644 --- a/jadx-core/src/main/java/jadx/core/dex/attributes/LoopAttr.java +++ b/jadx-core/src/main/java/jadx/core/dex/attributes/LoopAttr.java @@ -1,10 +1,13 @@ package jadx.core.dex.attributes; import jadx.core.dex.nodes.BlockNode; +import jadx.core.dex.nodes.Edge; import jadx.core.utils.BlockUtils; import java.util.Collections; import java.util.HashSet; +import java.util.LinkedList; +import java.util.List; import java.util.Set; public class LoopAttr implements IAttribute { @@ -37,16 +40,16 @@ public class LoopAttr implements IAttribute { } /** - * Return block nodes with exit edges from loop
+ * Return source blocks of exit edges.
* Exit nodes belongs to loop (contains in {@code loopBlocks}) */ public Set getExitNodes() { Set nodes = new HashSet(); - Set inloop = getLoopBlocks(); - for (BlockNode block : inloop) { + Set blocks = getLoopBlocks(); + for (BlockNode block : blocks) { // exit: successor node not from this loop, (don't change to getCleanSuccessors) for (BlockNode s : block.getSuccessors()) { - if (!inloop.contains(s) && !s.getAttributes().contains(AttributeType.EXC_HANDLER)) { + if (!blocks.contains(s) && !s.getAttributes().contains(AttributeType.EXC_HANDLER)) { nodes.add(block); } } @@ -54,6 +57,22 @@ public class LoopAttr implements IAttribute { return nodes; } + /** + * Return loop exit edges. + */ + public List getExitEdges() { + List edges = new LinkedList(); + Set blocks = getLoopBlocks(); + for (BlockNode block : blocks) { + for (BlockNode s : block.getSuccessors()) { + if (!blocks.contains(s) && !s.getAttributes().contains(AttributeType.EXC_HANDLER)) { + edges.add(new Edge(block, s)); + } + } + } + return edges; + } + @Override public String toString() { return "LOOP: " + start + "->" + end; diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/BlockNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/BlockNode.java index 3817f3172..0dc25a49e 100644 --- a/jadx-core/src/main/java/jadx/core/dex/nodes/BlockNode.java +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/BlockNode.java @@ -1,6 +1,7 @@ package jadx.core.dex.nodes; import jadx.core.dex.attributes.AttrNode; +import jadx.core.dex.attributes.AttributeFlag; import jadx.core.dex.attributes.AttributeType; import jadx.core.dex.attributes.BlockRegState; import jadx.core.dex.attributes.LoopAttr; @@ -148,6 +149,14 @@ public class BlockNode extends AttrNode implements IBlock { this.endState = endState; } + public boolean isSynthetic() { + return getAttributes().contains(AttributeFlag.SYNTHETIC); + } + + public boolean isReturnBlock() { + return getAttributes().contains(AttributeFlag.RETURN); + } + @Override public int hashCode() { return id; // TODO id can change during reindex diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/Edge.java b/jadx-core/src/main/java/jadx/core/dex/nodes/Edge.java new file mode 100644 index 000000000..7e24f4d69 --- /dev/null +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/Edge.java @@ -0,0 +1,42 @@ +package jadx.core.dex.nodes; + +public class Edge { + private final BlockNode source; + private final BlockNode target; + + public Edge(BlockNode source, BlockNode target) { + this.source = source; + this.target = target; + } + + public BlockNode getSource() { + return source; + } + + public BlockNode getTarget() { + return target; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + Edge edge = (Edge) o; + return source.equals(edge.source) && target.equals(edge.target); + + } + + @Override + public int hashCode() { + return source.hashCode() + 31 * target.hashCode(); + } + + @Override + public String toString() { + return "Edge: " + source + " -> " + target; + } +} diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/BlockMakerVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/BlockMakerVisitor.java index d73c35fae..2a5a6c27b 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/BlockMakerVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/BlockMakerVisitor.java @@ -12,6 +12,7 @@ import jadx.core.dex.instructions.args.ArgType; import jadx.core.dex.instructions.args.InsnArg; import jadx.core.dex.instructions.args.RegisterArg; import jadx.core.dex.nodes.BlockNode; +import jadx.core.dex.nodes.Edge; import jadx.core.dex.nodes.InsnNode; import jadx.core.dex.nodes.MethodNode; import jadx.core.dex.trycatch.CatchAttr; @@ -299,6 +300,16 @@ public class BlockMakerVisitor extends AbstractVisitor { } } + private static void markReturnBlocks(MethodNode mth) { + mth.getExitBlocks().clear(); + for (BlockNode block : mth.getBasicBlocks()) { + if (BlockUtils.lastInsnType(block, InsnType.RETURN)) { + block.getAttributes().add(AttributeFlag.RETURN); + mth.getExitBlocks().add(block); + } + } + } + private static void markLoops(MethodNode mth) { for (BlockNode block : mth.getBasicBlocks()) { for (BlockNode succ : block.getSuccessors()) { @@ -316,16 +327,6 @@ public class BlockMakerVisitor extends AbstractVisitor { } } - private static void markReturnBlocks(MethodNode mth) { - mth.getExitBlocks().clear(); - for (BlockNode block : mth.getBasicBlocks()) { - if (BlockUtils.lastInsnType(block, InsnType.RETURN)) { - block.getAttributes().add(AttributeFlag.RETURN); - mth.getExitBlocks().add(block); - } - } - } - private static void registerLoops(MethodNode mth) { for (BlockNode block : mth.getBasicBlocks()) { AttributesList attributes = block.getAttributes(); @@ -356,6 +357,7 @@ public class BlockMakerVisitor extends AbstractVisitor { if (oneHeader) { // several back edges connected to one loop header => make additional block BlockNode newLoopHeader = startNewBlock(mth, block.getStartOffset()); + newLoopHeader.getAttributes().add(AttributeFlag.SYNTHETIC); connect(newLoopHeader, block); for (IAttribute a : loops) { LoopAttr la = (LoopAttr) a; @@ -366,6 +368,24 @@ public class BlockMakerVisitor extends AbstractVisitor { return true; } } + // insert additional blocks if loop has several exits + if (loops.size() == 1) { + LoopAttr loop = (LoopAttr) loops.get(0); + List edges = loop.getExitEdges(); + if (edges.size() > 1) { + boolean change = false; + for (Edge edge : edges) { + BlockNode target = edge.getTarget(); + if (!target.getAttributes().contains(AttributeFlag.SYNTHETIC)) { + insertBlockBetween(mth, edge.getSource(), target); + change = true; + } + } + if (change) { + return true; + } + } + } } if (splitReturn(mth)) { return true; @@ -376,6 +396,15 @@ public class BlockMakerVisitor extends AbstractVisitor { return false; } + private static BlockNode insertBlockBetween(MethodNode mth, BlockNode source, BlockNode target) { + BlockNode newBlock = startNewBlock(mth, target.getStartOffset()); + newBlock.getAttributes().add(AttributeFlag.SYNTHETIC); + removeConnection(source, target); + connect(source, newBlock); + connect(newBlock, target); + return newBlock; + } + /** * Merge return blocks for void methods */ @@ -414,7 +443,6 @@ public class BlockMakerVisitor extends AbstractVisitor { if (mth.getExitBlocks().size() != 1) { return false; } - boolean split = false; BlockNode exitBlock = mth.getExitBlocks().get(0); if (exitBlock.getPredecessors().size() > 1 && exitBlock.getInstructions().size() == 1 @@ -422,10 +450,9 @@ public class BlockMakerVisitor extends AbstractVisitor { && !exitBlock.getAttributes().contains(AttributeFlag.SYNTHETIC)) { InsnNode returnInsn = exitBlock.getInstructions().get(0); List preds = new ArrayList(exitBlock.getPredecessors()); - if (returnInsn.getArgsCount() != 0 && !isReturnArgAssignInPred(mth, preds, returnInsn)) { + if (returnInsn.getArgsCount() != 0 && !isReturnArgAssignInPred(preds, returnInsn)) { return false; } - split = true; for (BlockNode pred : preds) { BlockNode newRetBlock = startNewBlock(mth, exitBlock.getStartOffset()); newRetBlock.getAttributes().add(AttributeFlag.SYNTHETIC); @@ -433,14 +460,13 @@ public class BlockMakerVisitor extends AbstractVisitor { removeConnection(pred, exitBlock); connect(pred, newRetBlock); } - } - if (split) { cleanExitNodes(mth); + return true; } - return split; + return false; } - private static boolean isReturnArgAssignInPred(MethodNode mth, List preds, InsnNode returnInsn) { + private static boolean isReturnArgAssignInPred(List preds, InsnNode returnInsn) { RegisterArg arg = (RegisterArg) returnInsn.getArg(0); int regNum = arg.getRegNum(); for (BlockNode pred : preds) { diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlinerVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlinerVisitor.java index 621d0cbe6..46c18f4ab 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlinerVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlinerVisitor.java @@ -46,39 +46,37 @@ public class ConstInlinerVisitor extends AbstractVisitor { long lit = ((LiteralArg) arg).getLiteral(); return replaceConst(mth, block, insn, lit); } - // TODO process string and class const } return false; } private static boolean replaceConst(MethodNode mth, BlockNode block, InsnNode insn, long literal) { List use = insn.getResult().getTypedVar().getUseList(); - int replace = 0; + int replaceCount = 0; for (InsnArg arg : use) { InsnNode useInsn = arg.getParentInsn(); - if (useInsn == null) { + if (arg == insn.getResult() || useInsn == null) { continue; } BlockNode useBlock = BlockUtils.getBlockByInsn(mth, useInsn); - if (useBlock == block || useBlock.isDominator(block)) { - if (arg != insn.getResult() && !registerReassignOnPath(block, useBlock, insn)) { - LiteralArg litArg; - if (use.size() == 2) { - // arg used only in one place - litArg = InsnArg.lit(literal, arg.getType()); - } else { - // in most cases type not equal arg.getType() - // just set unknown type and run type fixer - litArg = InsnArg.lit(literal, ArgType.UNKNOWN); - } - if (useInsn.replaceArg(arg, litArg)) { - fixTypes(mth, useInsn, litArg); - replace++; - } + boolean isDominator = useBlock == block || useBlock.isDominator(block); + if (isDominator && !registerReassignOnPath(block, useBlock, insn)) { + LiteralArg litArg; + if (use.size() == 2) { + // arg used only in one place + litArg = InsnArg.lit(literal, arg.getType()); + } else { + // in most cases type not equal arg.getType() + // just set unknown type and run type fixer + litArg = InsnArg.lit(literal, ArgType.UNKNOWN); + } + if (useInsn.replaceArg(arg, litArg)) { + fixTypes(mth, useInsn, litArg); + replaceCount++; } } } - return (replace + 1) == use.size(); + return replaceCount == use.size() - 1; } private static boolean registerReassignOnPath(BlockNode block, BlockNode useBlock, InsnNode assignInsn) { @@ -86,7 +84,6 @@ public class ConstInlinerVisitor extends AbstractVisitor { return false; } Set blocks = BlockUtils.getAllPathsBlocks(block, useBlock); - // TODO store list of assign insn for each register int regNum = assignInsn.getResult().getRegNum(); for (BlockNode b : blocks) { for (InsnNode insn : b.getInstructions()) { 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 a1b099b68..b4e9a84bc 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 @@ -12,6 +12,7 @@ import jadx.core.dex.instructions.SwitchNode; import jadx.core.dex.instructions.args.InsnArg; import jadx.core.dex.instructions.args.RegisterArg; import jadx.core.dex.nodes.BlockNode; +import jadx.core.dex.nodes.Edge; import jadx.core.dex.nodes.IRegion; import jadx.core.dex.nodes.InsnNode; import jadx.core.dex.nodes.MethodNode; @@ -143,8 +144,6 @@ public class RegionMaker { private BlockNode processLoop(IRegion curRegion, LoopAttr loop, RegionStack stack) { BlockNode loopStart = loop.getStart(); - IfNode ifnode = null; - LoopRegion loopRegion = null; Set exitBlocksSet = loop.getExitNodes(); // set exit blocks scan order by priority @@ -163,6 +162,9 @@ public class RegionMaker { exitBlocks.addAll(exitBlocksSet); exitBlocksSet = null; + IfNode ifnode = null; + LoopRegion loopRegion = null; + // exit block with loop condition BlockNode condBlock = null; @@ -174,114 +176,80 @@ public class RegionMaker { || exit.getInstructions().size() != 1) { continue; } - InsnNode insn = exit.getInstructions().get(0); - if (insn.getType() == InsnType.IF) { - boolean found = true; - ifnode = (IfNode) insn; - condBlock = exit; - loopRegion = new LoopRegion(curRegion, condBlock, condBlock == loop.getEnd()); + if (insn.getType() != InsnType.IF) { + continue; + } + ifnode = (IfNode) insn; + condBlock = exit; - if (loopRegion.isConditionAtEnd()) { - // TODO: add some checks + loopRegion = new LoopRegion(curRegion, condBlock, condBlock == loop.getEnd()); + boolean found = true; + if (condBlock != loopStart) { + if (condBlock.getPredecessors().contains(loopStart)) { + loopRegion.setPreCondition(loopStart); + // if we can't merge pre-condition this is not correct header + found = loopRegion.checkPreCondition(); } else { - if (condBlock != loopStart) { - if (condBlock.getPredecessors().contains(loopStart)) { - loopRegion.setPreCondition(loopStart); - // if we can't merge pre-condition this is not correct header - found = loopRegion.checkPreCondition(); - } else { - found = false; - } - } - } - if (!found) { - ifnode = null; - loopRegion = null; - condBlock = null; - // try another exit - } 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(IfCondition.invert(mergedIf.getCondition())); - bThen = mergedIf.getElseBlock(); - } else { - loopRegion.setCondition(mergedIf.getCondition()); - bThen = mergedIf.getThenBlock(); - } - exitBlocks.removeAll(merged); - } - break; + found = false; } } + if (!found) { + ifnode = null; + loopRegion = null; + condBlock = null; + // try another exit + continue; + } + + 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(IfCondition.invert(mergedIf.getCondition())); + bThen = mergedIf.getElseBlock(); + } else { + loopRegion.setCondition(mergedIf.getCondition()); + bThen = mergedIf.getThenBlock(); + } + exitBlocks.removeAll(merged); + } + break; } // endless loop if (loopRegion == null) { - loopRegion = new LoopRegion(curRegion, null, false); - curRegion.getSubBlocks().add(loopRegion); - - loopStart.getAttributes().remove(AttributeType.LOOP); - stack.push(loopRegion); - Region body = makeRegion(loopStart, stack); - 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)) { - return next; - } else { - return null; - } - } - - curRegion.getSubBlocks().add(loopRegion); - - exitBlocks.remove(condBlock); - if (exitBlocks.size() > 0) { - // set BREAK or FORCE_RETURN attributes - // before path cross between main loop exit and subexit - BlockNode loopExit = BlockUtils.getNextBlock(condBlock); - for (BlockNode exit : exitBlocks) { - BlockNode next = BlockUtils.getNextBlock(exit); - while (next != null) { - if (isPathExists(loopExit, next)) { - // found cross - /* - if (next.getCleanSuccessors().size() == 1) { - BlockNode r = BlockUtils.getNextBlock(next); - if (r != null - && r.getAttributes().contains(AttributeFlag.RETURN) - && r.getInstructions().size() > 0 - && r.getInstructions().get(0).getType() == InsnType.RETURN) { - next.getAttributes().add(new ForceReturnAttr(r.getInstructions().get(0))); - } else { - next.getAttributes().add(AttributeFlag.BREAK); - stack.addExit(r); - } - } else { - stack.addExit(next); - } - */ - break; - } - next = BlockUtils.getNextBlock(next); - } - } + return makeEndlessLoop(curRegion, stack, loop, loopStart); } if (bThen == null) { bThen = ifnode.getThenBlock(); } + BlockNode loopBody = null; + for (BlockNode s : condBlock.getSuccessors()) { + if (loop.getLoopBlocks().contains(s)) { + loopBody = s; + break; + } + } + + curRegion.getSubBlocks().add(loopRegion); + stack.push(loopRegion); + + exitBlocks.remove(condBlock); + if (exitBlocks.size() > 0) { + // add 'break' instruction before path cross between main loop exit and subexit + BlockNode loopExit = BlockUtils.selectOther(loopBody, condBlock.getCleanSuccessors()); + for (Edge exitEdge : loop.getExitEdges()) { + if (!exitBlocks.contains(exitEdge.getSource())) { + continue; + } + insertBreak(stack, loopExit, exitEdge); + } + } BlockNode out; if (loopRegion.isConditionAtEnd()) { @@ -290,20 +258,10 @@ public class RegionMaker { loopStart.getAttributes().remove(AttributeType.LOOP); - stack.push(loopRegion); stack.addExit(loop.getEnd()); loopRegion.setBody(makeRegion(loopStart, stack)); loopStart.getAttributes().add(loop); - stack.pop(); } else { - Set loopBlocks = loop.getLoopBlocks(); - BlockNode loopBody = null; - for (BlockNode s : condBlock.getSuccessors()) { - if (loopBlocks.contains(s)) { - loopBody = s; - break; - } - } if (bThen != loopBody) { loopRegion.setCondition(IfCondition.invert(loopRegion.getCondition())); } @@ -313,21 +271,55 @@ public class RegionMaker { && outAttrs.get(AttributeType.LOOP) != loop && stack.peekRegion() instanceof LoopRegion) { LoopRegion outerLoop = (LoopRegion) stack.peekRegion(); - if (outerLoop.getBody() == null /* processing not yet finished */ - || RegionUtils.isRegionContainsBlock(outerLoop, out)) { + boolean notYetProcessed = outerLoop.getBody() == null; + if (notYetProcessed || RegionUtils.isRegionContainsBlock(outerLoop, out)) { // exit to outer loop which already processed out = null; } } - - stack.push(loopRegion); stack.addExit(out); loopRegion.setBody(makeRegion(loopBody, stack)); - stack.pop(); } + stack.pop(); return out; } + private BlockNode makeEndlessLoop(IRegion curRegion, RegionStack stack, LoopAttr loop, BlockNode loopStart) { + LoopRegion loopRegion; + loopRegion = new LoopRegion(curRegion, null, false); + curRegion.getSubBlocks().add(loopRegion); + + loopStart.getAttributes().remove(AttributeType.LOOP); + stack.push(loopRegion); + Region body = makeRegion(loopStart, stack); + 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()); + return RegionUtils.isRegionContainsBlock(body, next) ? null : next; + } + + private void insertBreak(RegionStack stack, BlockNode loopExit, Edge exitEdge) { + BlockNode prev = null; + BlockNode exit = exitEdge.getTarget(); + while (exit != null) { + if (prev != null && isPathExists(loopExit, exit)) { + // found cross + if (!exit.getAttributes().contains(AttributeFlag.RETURN)) { + prev.getInstructions().add(new InsnNode(InsnType.BREAK, 0)); + stack.addExit(exit); + } + return; + } + prev = exit; + exit = BlockUtils.getNextBlock(exit); + } + } + private final Set cacheSet = new HashSet(); private BlockNode processMonitorEnter(IRegion curRegion, BlockNode block, InsnNode insn, RegionStack stack) { @@ -509,8 +501,8 @@ public class RegionMaker { IfCondition nestedCondition = IfCondition.fromIfNode(nestedIfInsn); if (isPathExists(bElse, nestedIfBlock)) { // else branch - if (!isEqualReturns(bThen, nbThen)) { - if (!isEqualReturns(bThen, nbElse)) { + if (!isEqualPaths(bThen, nbThen)) { + if (!isEqualPaths(bThen, nbElse)) { // not connected conditions break; } @@ -520,8 +512,8 @@ public class RegionMaker { condition = IfCondition.merge(Mode.OR, condition, nestedCondition); } else { // then branch - if (!isEqualReturns(bElse, nbElse)) { - if (!isEqualReturns(bElse, nbThen)) { + if (!isEqualPaths(bElse, nbElse)) { + if (!isEqualPaths(bElse, nbThen)) { // not connected conditions break; } @@ -533,7 +525,7 @@ public class RegionMaker { result = new IfInfo(); result.setCondition(condition); nestedIfBlock.getAttributes().add(AttributeFlag.SKIP); - bThen.getAttributes().add(AttributeFlag.SKIP); + skipSimplePath(bThen); if (merged != null) { merged.add(nestedIfBlock); @@ -702,27 +694,54 @@ public class RegionMaker { handler.getHandlerRegion().getAttributes().add(excHandlerAttr); } - private static boolean isEqualReturns(BlockNode b1, BlockNode b2) { + private void skipSimplePath(BlockNode block) { + while (block != null + && block.getCleanSuccessors().size() < 2 + && block.getPredecessors().size() == 1) { + block.getAttributes().add(AttributeFlag.SKIP); + block = BlockUtils.getNextBlock(block); + } + } + + private static boolean isEqualPaths(BlockNode b1, BlockNode b2) { if (b1 == b2) { return true; } if (b1 == null || b2 == null) { return false; } + if (isReturnBlocks(b1, b2)) { + return true; + } + if (isSyntheticPath(b1, b2)) { + return true; + } + return false; + } + + private static boolean isSyntheticPath(BlockNode b1, BlockNode b2) { + if (!b1.isSynthetic() || !b2.isSynthetic()) { + return false; + } + BlockNode n1 = BlockUtils.getNextBlock(b1); + BlockNode n2 = BlockUtils.getNextBlock(b2); + return isEqualPaths(n1, n2); + } + + private static boolean isReturnBlocks(BlockNode b1, BlockNode b2) { + if (!b1.isReturnBlock() || !b2.isReturnBlock()) { + return false; + } List b1Insns = b1.getInstructions(); List b2Insns = b2.getInstructions(); if (b1Insns.size() != 1 || b2Insns.size() != 1) { return false; } - if (!b1.getAttributes().contains(AttributeFlag.RETURN) - || !b2.getAttributes().contains(AttributeFlag.RETURN)) { + InsnNode i1 = b1Insns.get(0); + InsnNode i2 = b2Insns.get(0); + if (i1.getArgsCount() == 0 || i2.getArgsCount() == 0) { return false; } - if (b1Insns.get(0).getArgsCount() > 0) { - if (b1Insns.get(0).getArg(0) != b2Insns.get(0).getArg(0)) { - return false; - } - } - return true; + return i1.getArg(0).equals(i2.getArg(0)); } } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/typeresolver/TypeResolver.java b/jadx-core/src/main/java/jadx/core/dex/visitors/typeresolver/TypeResolver.java index 8916b3c04..773fa8c49 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/typeresolver/TypeResolver.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/typeresolver/TypeResolver.java @@ -10,7 +10,11 @@ import jadx.core.dex.visitors.AbstractVisitor; import java.util.List; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + public class TypeResolver extends AbstractVisitor { + private static final Logger LOG = LoggerFactory.getLogger(TypeResolver.class); @Override public void visit(MethodNode mth) { @@ -67,11 +71,22 @@ public class TypeResolver extends AbstractVisitor { } } while (changed); - for (BlockNode block : mth.getBasicBlocks()) { - for (BlockNode dest : block.getSuccessors()) { - connectEdges(mth, block, dest, false); + int i = 0; + do { + changed = false; + for (BlockNode block : mth.getBasicBlocks()) { + for (BlockNode dest : block.getSuccessors()) { + if (connectEdges(mth, block, dest, false)) { + changed = true; + } + } } - } + i++; + if (i > 10) { + LOG.warn("Can't resolve types (forward connectEdges pass) in method: {}", mth); + break; + } + } while (changed && i < 10); } private static boolean connectEdges(MethodNode mth, BlockNode from, BlockNode to, boolean back) { diff --git a/jadx-core/src/test/java/jadx/tests/internal/TestReturnWrapping.java b/jadx-core/src/test/java/jadx/tests/internal/TestReturnWrapping.java index a6f128e55..bdb404368 100644 --- a/jadx-core/src/test/java/jadx/tests/internal/TestReturnWrapping.java +++ b/jadx-core/src/test/java/jadx/tests/internal/TestReturnWrapping.java @@ -10,16 +10,15 @@ import static org.junit.Assert.assertThat; public class TestReturnWrapping extends InternalJadxTest { public static class TestCls { - /**/ + public static int f1(int arg0) { switch (arg0) { case 1: return 255; } return arg0 + 1; - }/**/ + } - /**/ public static Object f2(Object arg0, int arg1) { Object ret = null; int i = arg1; @@ -34,9 +33,8 @@ public class TestReturnWrapping extends InternalJadxTest { } return i > 128 ? arg0.toString() + ret.toString() : i; } - }/**/ + } - /**/ public static int f3(int arg0) { while (arg0 > 10) { int abc = 951; @@ -46,7 +44,7 @@ public class TestReturnWrapping extends InternalJadxTest { arg0 -= abc; } return arg0; - }/**/ + } } @Test @@ -56,7 +54,6 @@ public class TestReturnWrapping extends InternalJadxTest { assertThat(code, containsString("return 255;")); assertThat(code, containsString("return arg0 + 1;")); - //assertThat(code, containsString("return Integer.toHexString(i);")); assertThat(code, containsString("return i > 128 ? arg0.toString() + ret.toString() : Integer.valueOf(i);")); assertThat(code, containsString("return arg0 + 2;")); assertThat(code, containsString("arg0 -= 951;")); diff --git a/jadx-core/src/test/java/jadx/tests/internal/loops/TestBreakInLoop.java b/jadx-core/src/test/java/jadx/tests/internal/loops/TestBreakInLoop.java new file mode 100644 index 000000000..8e32f50cd --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/internal/loops/TestBreakInLoop.java @@ -0,0 +1,40 @@ +package jadx.tests.internal.loops; + +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.assertEquals; +import static org.junit.Assert.assertThat; + +public class TestBreakInLoop extends InternalJadxTest { + + public static class TestCls { + private int f; + + private void test(int[] a, int b) { + int i = 0; + while (i < a.length) { + a[i]++; + if (i < b) { + break; + } + i++; + } + this.f++; + } + } + + @Test + public void test() { + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + System.out.println(code); + + assertEquals(count(code, "this.f++;"), 1); + assertThat(code, containsString("if (i < b) {")); + assertThat(code, containsString("break;")); + } +} diff --git a/jadx-core/src/test/java/jadx/tests/internal/loops/TestLoopCondition.java b/jadx-core/src/test/java/jadx/tests/internal/loops/TestLoopCondition.java index a0eae622e..911ddd35b 100644 --- a/jadx-core/src/test/java/jadx/tests/internal/loops/TestLoopCondition.java +++ b/jadx-core/src/test/java/jadx/tests/internal/loops/TestLoopCondition.java @@ -10,8 +10,7 @@ import static org.junit.Assert.assertThat; public class TestLoopCondition extends InternalJadxTest { - @SuppressWarnings("serial") - public static class TestCls extends Exception { + public static class TestCls { public String f; private void setEnabled(boolean r1z) { @@ -32,14 +31,6 @@ public class TestLoopCondition extends InternalJadxTest { setEnabled(false); } - public int testComplexIfInLoop(boolean a) { - int i = 0; - while (a && i < 10) { - i++; - } - return i; - } - private void testMoreComplexIfInLoop(java.util.ArrayList list) throws Exception { for (int i = 0; i != 16 && i < 255; i++) { list.set(i, "ABC"); @@ -55,9 +46,9 @@ public class TestLoopCondition extends InternalJadxTest { public void test() { ClassNode cls = getClassNode(TestCls.class); String code = cls.getCode().toString(); + System.out.println(code); assertThat(code, containsString("i < this.f.length()")); - assertThat(code, containsString("while (a && i < 10) {")); assertThat(code, containsString("list.set(i, \"ABC\")")); assertThat(code, containsString("list.set(i, \"DEF\")")); } diff --git a/jadx-core/src/test/java/jadx/tests/internal/loops/TestLoopCondition2.java b/jadx-core/src/test/java/jadx/tests/internal/loops/TestLoopCondition2.java new file mode 100644 index 000000000..aef25cb5e --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/internal/loops/TestLoopCondition2.java @@ -0,0 +1,32 @@ +package jadx.tests.internal.loops; + +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 TestLoopCondition2 extends InternalJadxTest { + + public static class TestCls { + + public int test(boolean a) { + int i = 0; + while (a && i < 10) { + i++; + } + return i; + } + } + + @Test + public void test() { + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + System.out.println(code); + + 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 5e722ebc6..0e446ecd2 100644 --- a/jadx-samples/src/main/java/jadx/samples/TestCF3.java +++ b/jadx-samples/src/main/java/jadx/samples/TestCF3.java @@ -222,15 +222,13 @@ public class TestCF3 extends AbstractTest { new ArrayList(Arrays.asList("a1", "b2")))); List list1 = Arrays.asList(null, "a", "b"); - - // TODO this line required to omit generic information because it create List - // List list2 = Arrays.asList(null, null, null); - assertEquals(testReturnInLoop(list1), "a"); assertEquals(testReturnInLoop2(list1), "a"); - // assertEquals(testReturnInLoop(list2), "error"); - // assertEquals(testReturnInLoop2(list2), "error"); + // TODO this line required to omit generic information because it create List +// List list2 = Arrays.asList(null, null, null); +// assertEquals(testReturnInLoop(list2), "error"); +// assertEquals(testReturnInLoop2(list2), "error"); // assertTrue(testLabeledBreakContinue());