core: process complex condition in loop header

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);

jadx-core/src/main/java/jadx/core/codegen/MethodGen.java

 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(" {");

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,11 +69,30 @@ 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
@@ -72,6 +101,6 @@ public class IfNode extends GotoNode {
 				+ InsnUtils.insnTypeToString(insnType)
 				+ getArg(0) + " " + op.getSymbol()
 				+ " " + (zeroCmp ? "0" : getArg(1))
-				+ "  -> " + InsnUtils.formatOffset(target);
+				+ "  -> " + (thenBlock != null ? thenBlock : InsnUtils.formatOffset(target));
 	}
 }

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<IfCondition> newArgs = new ArrayList<IfCondition>(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) {

jadx-core/src/main/java/jadx/core/dex/regions/IfInfo.java

+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;
+	}
+}

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;
 	}

jadx-core/src/main/java/jadx/core/dex/visitors/BlockProcessingHelper.java

 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<InsnNode> instructions = block.getInstructions();
+		if (instructions.size() == 1) {
+			InsnNode insn = instructions.get(0);
+			if (insn.getType() == InsnType.IF) {
+				((IfNode) insn).initBlocks(block);
+			}
+		}
+	}
 }

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<BlockNode> exitBlocks = new ArrayList<BlockNode>(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<BlockNode> merged = new ArrayList<BlockNode>(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<BlockNode> cacheSet = new HashSet<BlockNode>();
+	private final Set<BlockNode> cacheSet = new HashSet<BlockNode>();
 
 	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<BlockNode> 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<InsnNode> 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));

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) {

jadx-core/src/test/java/jadx/api/InternalJadxTest.java

@@ -125,4 +125,8 @@ public abstract class InternalJadxTest {
 			}
 		}
 	}
+
+	protected void setOutputCFG() {
+		this.outputCFG = true;
+	}
 }

jadx-core/src/test/java/jadx/tests/internal/TestLoopCondition.java

+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) {"));
+	}
+}

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<String> 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;
 	}