core: fix complex 'if' processing (issues #9 and #12)

jadx-core/src/main/java/jadx/core/Jadx.java

@@ -60,17 +60,19 @@ public class Jadx {
 
 			passes.add(new ConstInlinerVisitor());
 			passes.add(new FinishTypeInference());
-
+			passes.add(new EliminatePhiNodes());
 			if (args.isRawCFGOutput()) {
 				passes.add(new DotGraphVisitor(outDir, false, true));
 			}
 
-			passes.add(new EliminatePhiNodes());
-
 			passes.add(new ModVisitor());
 			passes.add(new EnumVisitor());
 
 			passes.add(new CodeShrinker());
+			if (args.isCFGOutput()) {
+				passes.add(new DotGraphVisitor(outDir, false));
+			}
+
 			passes.add(new RegionMakerVisitor());
 			passes.add(new IfRegionVisitor());
 			passes.add(new ReturnVisitor());
@@ -87,10 +89,6 @@ public class Jadx {
 			passes.add(new MethodInlineVisitor());
 			passes.add(new ClassModifier());
 			passes.add(new PrepareForCodeGen());
-
-			if (args.isCFGOutput()) {
-				passes.add(new DotGraphVisitor(outDir, false));
-			}
 		}
 		passes.add(new CodeGen(args));
 		return passes;

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

@@ -75,7 +75,7 @@ public class ConditionGen extends InsnGen {
 			} else if (op == IfOp.NE) {
 				// != true
 				code.add('!');
-				boolean wrap = isWrapNeeded(firstArg);
+				boolean wrap = isArgWrapNeeded(firstArg);
 				if (wrap) {
 					code.add('(');
 				}
@@ -88,9 +88,9 @@ public class ConditionGen extends InsnGen {
 			LOG.warn(ErrorsCounter.formatErrorMsg(mth, "Unsupported boolean condition " + op.getSymbol()));
 		}
 
-		addArg(code, firstArg, isWrapNeeded(firstArg));
+		addArg(code, firstArg, isArgWrapNeeded(firstArg));
 		code.add(' ').add(op.getSymbol()).add(' ');
-		addArg(code, secondArg, isWrapNeeded(secondArg));
+		addArg(code, secondArg, isArgWrapNeeded(secondArg));
 	}
 
 	private void addNot(CodeWriter code, IfCondition condition) throws CodegenException {
@@ -110,15 +110,16 @@ public class ConditionGen extends InsnGen {
 	}
 
 	private boolean isWrapNeeded(IfCondition condition) {
-		return !condition.isCompare();
+		return !condition.isCompare() && condition.getMode() != IfCondition.Mode.NOT;
 	}
 
-	private static boolean isWrapNeeded(InsnArg arg) {
+	private static boolean isArgWrapNeeded(InsnArg arg) {
 		if (!arg.isInsnWrap()) {
 			return false;
 		}
 		InsnNode insn = ((InsnWrapArg) arg).getWrapInsn();
-		if (insn.getType() == InsnType.ARITH) {
+		InsnType insnType = insn.getType();
+		if (insnType == InsnType.ARITH) {
 			switch (((ArithNode) insn).getOp()) {
 				case ADD:
 				case SUB:
@@ -127,8 +128,18 @@ public class ConditionGen extends InsnGen {
 				case REM:
 					return false;
 			}
-		} else if (insn.getType() == InsnType.INVOKE) {
-			return false;
+		} else {
+			switch (insnType) {
+				case INVOKE:
+				case SGET:
+				case IGET:
+				case AGET:
+				case CONST:
+				case ARRAY_LENGTH:
+					return false;
+				default:
+					return true;
+			}
 		}
 		return true;
 	}

jadx-core/src/main/java/jadx/core/dex/attributes/AType.java

@@ -25,6 +25,9 @@ import jadx.core.dex.trycatch.SplitterBlockAttr;
  */
 public class AType<T extends IAttribute> {
 
+	private AType() {
+	}
+
 	public static final AType<AttrList<JumpInfo>> JUMP = new AType<AttrList<JumpInfo>>();
 	public static final AType<AttrList<LoopInfo>> LOOP = new AType<AttrList<LoopInfo>>();
 

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

@@ -69,13 +69,8 @@ public final class IfCondition {
 			IfCondition n = new IfCondition(a);
 			n.addArg(b);
 			return n;
-		} else if (b.getMode() == mode) {
-			IfCondition n = new IfCondition(b);
-			n.addArg(a);
-			return n;
-		} else {
-			return new IfCondition(mode, Arrays.asList(a, b));
 		}
+		return new IfCondition(mode, Arrays.asList(a, b));
 	}
 
 	public Mode getMode() {

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

@@ -2,41 +2,64 @@ package jadx.core.dex.regions;
 
 import jadx.core.dex.nodes.BlockNode;
 
+import java.util.HashSet;
+import java.util.Set;
+
 public final class IfInfo {
-	IfCondition condition;
-	BlockNode ifnode;
-	BlockNode thenBlock;
-	BlockNode elseBlock;
+	private final IfCondition condition;
+	private final Set<BlockNode> mergedBlocks = new HashSet<BlockNode>();
+	private final BlockNode thenBlock;
+	private final BlockNode elseBlock;
+	@Deprecated
+	private BlockNode ifBlock;
+
+	public IfInfo(IfCondition condition, BlockNode thenBlock, BlockNode elseBlock) {
+		this.condition = condition;
+		this.thenBlock = thenBlock;
+		this.elseBlock = elseBlock;
+	}
+
+	public IfInfo(IfCondition condition, IfInfo info) {
+		this.condition = condition;
+		this.thenBlock = info.getThenBlock();
+		this.elseBlock = info.getElseBlock();
+		this.mergedBlocks.addAll(info.getMergedBlocks());
+	}
+
+	public static IfInfo invert(IfInfo info) {
+		IfInfo tmpIf = new IfInfo(IfCondition.invert(info.getCondition()),
+				info.getElseBlock(), info.getThenBlock());
+		tmpIf.setIfBlock(info.getIfBlock());
+		tmpIf.getMergedBlocks().addAll(info.getMergedBlocks());
+		return tmpIf;
+	}
 
 	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 Set<BlockNode> getMergedBlocks() {
+		return mergedBlocks;
 	}
 
 	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;
+	public BlockNode getIfBlock() {
+		return ifBlock;
+	}
+
+	public void setIfBlock(BlockNode ifBlock) {
+		this.ifBlock = ifBlock;
+	}
+
+	@Override
+	public String toString() {
+		return "IfInfo: " + condition + ", then: " + thenBlock + ", else: " + elseBlock;
 	}
 }

jadx-core/src/main/java/jadx/core/dex/visitors/regions/CheckRegions.java

@@ -30,16 +30,14 @@ public class CheckRegions extends AbstractVisitor {
 
 		// check if all blocks included in regions
 		final Set<BlockNode> blocksInRegions = new HashSet<BlockNode>();
-		IRegionVisitor collectBlocks = new AbstractRegionVisitor() {
+		DepthRegionTraversal.traverseAll(mth, new AbstractRegionVisitor() {
 			@Override
 			public void processBlock(MethodNode mth, IBlock container) {
 				if (container instanceof BlockNode) {
 					blocksInRegions.add((BlockNode) container);
 				}
 			}
-		};
-		DepthRegionTraversal.traverseAll(mth, collectBlocks);
-
+		});
 		if (mth.getBasicBlocks().size() != blocksInRegions.size()) {
 			for (BlockNode block : mth.getBasicBlocks()) {
 				if (!blocksInRegions.contains(block)
@@ -52,19 +50,17 @@ public class CheckRegions extends AbstractVisitor {
 		}
 
 		// check loop conditions
-		IRegionVisitor checkLoops = new AbstractRegionVisitor() {
+		DepthRegionTraversal.traverseAll(mth, new AbstractRegionVisitor() {
 			@Override
 			public void enterRegion(MethodNode mth, IRegion region) {
 				if (region instanceof LoopRegion) {
-					LoopRegion loop = (LoopRegion) region;
-					BlockNode loopHeader = loop.getHeader();
+					BlockNode loopHeader = ((LoopRegion) region).getHeader();
 					if (loopHeader != null && loopHeader.getInstructions().size() != 1) {
 						ErrorsCounter.methodError(mth, "Incorrect condition in loop: " + loopHeader);
 						mth.add(AFlag.INCONSISTENT_CODE);
 					}
 				}
 			}
-		};
-		DepthRegionTraversal.traverseAll(mth, checkLoops);
+		});
 	}
 }

jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfMakerHelper.java

@@ -0,0 +1,190 @@
+package jadx.core.dex.visitors.regions;
+
+import jadx.core.dex.attributes.AFlag;
+import jadx.core.dex.attributes.AType;
+import jadx.core.dex.instructions.IfNode;
+import jadx.core.dex.instructions.InsnType;
+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.InsnNode;
+import jadx.core.dex.regions.IfCondition;
+import jadx.core.dex.regions.IfCondition.Mode;
+import jadx.core.dex.regions.IfInfo;
+import jadx.core.utils.BlockUtils;
+
+import java.util.List;
+
+import static jadx.core.utils.BlockUtils.isPathExists;
+
+public class IfMakerHelper {
+	private IfMakerHelper() {
+	}
+
+	static IfInfo makeIfInfo(BlockNode ifBlock) {
+		return makeIfInfo(ifBlock, IfCondition.fromIfBlock(ifBlock));
+	}
+
+	static IfInfo mergeNestedIfNodes(BlockNode block) {
+		IfInfo info = makeIfInfo(block);
+		return mergeNestedIfNodes(info);
+	}
+
+	private static IfInfo mergeNestedIfNodes(IfInfo currentIf) {
+		BlockNode curThen = currentIf.getThenBlock();
+		BlockNode curElse = currentIf.getElseBlock();
+		if (curThen == curElse) {
+			return null;
+		}
+		boolean followThenBranch;
+		IfInfo nextIf = getNextIf(currentIf, curThen);
+		if (nextIf != null) {
+			followThenBranch = true;
+		} else {
+			nextIf = getNextIf(currentIf, curElse);
+			if (nextIf != null) {
+				followThenBranch = false;
+			} else {
+				return null;
+			}
+		}
+		if (isInversionNeeded(currentIf, nextIf)) {
+			// invert current node for match pattern
+			nextIf = IfInfo.invert(nextIf);
+		}
+		if (!RegionMaker.isEqualPaths(curElse, nextIf.getElseBlock())
+				&& !RegionMaker.isEqualPaths(curThen, nextIf.getThenBlock())) {
+			// complex condition, run additional checks
+			if (checkConditionBranches(curThen, curElse) || checkConditionBranches(curElse, curThen)) {
+				return null;
+			}
+			BlockNode otherBranchBlock = followThenBranch ? curElse : curThen;
+			if (!isPathExists(nextIf.getIfBlock(), otherBranchBlock)) {
+				return null;
+			}
+			if (isPathExists(nextIf.getThenBlock(), otherBranchBlock)
+					&& isPathExists(nextIf.getElseBlock(), otherBranchBlock)) {
+				// both branches paths points to one block
+				return null;
+			}
+
+			// this is nested conditions with different mode (i.e (a && b) || c),
+			// search next condition for merge, get null if failed
+			IfInfo tmpIf = mergeNestedIfNodes(nextIf);
+			if (tmpIf != null) {
+				nextIf = tmpIf;
+				if (isInversionNeeded(currentIf, nextIf)) {
+					nextIf = IfInfo.invert(nextIf);
+				}
+			}
+		}
+
+		IfInfo result = mergeIfInfo(currentIf, nextIf, followThenBranch);
+		// search next nested if block
+		IfInfo next = mergeNestedIfNodes(result);
+		if (next != null) {
+			return next;
+		}
+		return result;
+	}
+
+	private static boolean isInversionNeeded(IfInfo currentIf, IfInfo nextIf) {
+		return RegionMaker.isEqualPaths(currentIf.getElseBlock(), nextIf.getThenBlock())
+				|| RegionMaker.isEqualPaths(currentIf.getThenBlock(), nextIf.getElseBlock());
+	}
+
+	private static IfInfo makeIfInfo(BlockNode ifBlock, IfCondition condition) {
+		IfNode ifnode = (IfNode) ifBlock.getInstructions().get(0);
+		IfInfo info = new IfInfo(condition, ifnode.getThenBlock(), ifnode.getElseBlock());
+		info.setIfBlock(ifBlock);
+		info.getMergedBlocks().add(ifBlock);
+		return info;
+	}
+
+	private static boolean checkConditionBranches(BlockNode from, BlockNode to) {
+		return from.getCleanSuccessors().size() == 1 && from.getCleanSuccessors().contains(to);
+	}
+
+	private static IfInfo mergeIfInfo(IfInfo first, IfInfo second, boolean followThenBranch) {
+		Mode mergeOperation = followThenBranch ? Mode.AND : Mode.OR;
+		BlockNode otherPathBlock = followThenBranch ? first.getElseBlock() : first.getThenBlock();
+		RegionMaker.skipSimplePath(otherPathBlock);
+		first.getIfBlock().add(AFlag.SKIP);
+		second.getIfBlock().add(AFlag.SKIP);
+
+		IfCondition condition = IfCondition.merge(mergeOperation, first.getCondition(), second.getCondition());
+		IfInfo result = new IfInfo(condition, second);
+		result.setIfBlock(first.getIfBlock());
+		result.getMergedBlocks().addAll(first.getMergedBlocks());
+		result.getMergedBlocks().addAll(second.getMergedBlocks());
+		return result;
+	}
+
+	private static IfInfo getNextIf(IfInfo info, BlockNode block) {
+		if (!canSelectNext(info, block)) {
+			return null;
+		}
+		BlockNode nestedIfBlock = getNextIfNode(block);
+		if (nestedIfBlock != null) {
+			return makeIfInfo(nestedIfBlock);
+		}
+		return null;
+	}
+
+	private static boolean canSelectNext(IfInfo info, BlockNode block) {
+		if (block.getPredecessors().size() == 1) {
+			return true;
+		}
+		if (info.getMergedBlocks().containsAll(block.getPredecessors())) {
+			return true;
+		}
+		return false;
+	}
+
+	private static BlockNode getNextIfNode(BlockNode block) {
+		if (block == null || block.contains(AType.LOOP) || block.contains(AFlag.SKIP)) {
+			return null;
+		}
+		List<InsnNode> insns = block.getInstructions();
+		if (insns.size() == 1 && insns.get(0).getType() == InsnType.IF) {
+			return block;
+		}
+		// skip this block and search in successors chain
+		List<BlockNode> successors = block.getSuccessors();
+		if (successors.size() != 1) {
+			return null;
+		}
+
+		BlockNode next = successors.get(0);
+		if (next.getPredecessors().size() != 1) {
+			return null;
+		}
+		boolean pass = true;
+		if (!insns.isEmpty()) {
+			// check that all instructions can be inlined
+			for (InsnNode insn : insns) {
+				RegisterArg res = insn.getResult();
+				if (res == null) {
+					pass = false;
+					break;
+				}
+				List<RegisterArg> useList = res.getSVar().getUseList();
+				if (useList.size() != 1) {
+					pass = false;
+					break;
+				}
+				InsnArg arg = useList.get(0);
+				InsnNode usePlace = arg.getParentInsn();
+				if (!BlockUtils.blockContains(block, usePlace)
+						&& !BlockUtils.blockContains(next, usePlace)) {
+					pass = false;
+					break;
+				}
+			}
+		}
+		if (pass) {
+			return getNextIfNode(next);
+		}
+		return null;
+	}
+}

jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfRegionVisitor.java

@@ -14,6 +14,8 @@ import jadx.core.utils.RegionUtils;
 
 import java.util.List;
 
+import static jadx.core.utils.RegionUtils.insnsCount;
+
 public class IfRegionVisitor extends AbstractVisitor implements IRegionVisitor, IRegionIterativeVisitor {
 
 	@Override
@@ -60,10 +62,36 @@ public class IfRegionVisitor extends AbstractVisitor implements IRegionVisitor,
 				invertIfRegion(ifRegion);
 			}
 		}
-		if (RegionUtils.isEmpty(ifRegion.getThenRegion())
-				|| hasSimpleReturnBlock(ifRegion.getThenRegion())) {
+		IContainer elseRegion = ifRegion.getElseRegion();
+		if (elseRegion == null || RegionUtils.isEmpty(elseRegion)) {
+			return;
+		}
+		boolean thenIsEmpty = RegionUtils.isEmpty(ifRegion.getThenRegion());
+		if (thenIsEmpty || hasSimpleReturnBlock(ifRegion.getThenRegion())) {
 			invertIfRegion(ifRegion);
 		}
+
+		if (!thenIsEmpty) {
+			// move 'if' from then to make 'else if' chain
+			if (isIfRegion(ifRegion.getThenRegion())
+					&& !isIfRegion(elseRegion)) {
+				invertIfRegion(ifRegion);
+			}
+
+		}
+	}
+
+	private static boolean isIfRegion(IContainer container) {
+		if (container instanceof IfRegion) {
+			return true;
+		}
+		if (container instanceof IRegion) {
+			List<IContainer> subBlocks = ((IRegion) container).getSubBlocks();
+			if (subBlocks.size() == 1 && subBlocks.get(0) instanceof IfRegion) {
+				return true;
+			}
+		}
+		return false;
 	}
 
 	private static void moveReturnToThenBlock(MethodNode mth, IfRegion ifRegion) {
@@ -95,7 +123,8 @@ public class IfRegionVisitor extends AbstractVisitor implements IRegionVisitor,
 		if (ifRegion.getElseRegion() != null
 				&& !ifRegion.contains(AFlag.ELSE_IF_CHAIN)
 				&& !ifRegion.getElseRegion().contains(AFlag.ELSE_IF_CHAIN)
-				&& RegionUtils.hasExitBlock(ifRegion.getThenRegion())) {
+				&& RegionUtils.hasExitBlock(ifRegion.getThenRegion())
+				&& insnsCount(ifRegion.getThenRegion()) < 2) {
 			IRegion parent = ifRegion.getParent();
 			Region newRegion = new Region(parent);
 			if (parent.replaceSubBlock(ifRegion, newRegion)) {

jadx-core/src/main/java/jadx/core/dex/visitors/regions/ProcessTryCatchRegions.java

@@ -151,7 +151,7 @@ public class ProcessTryCatchRegions extends AbstractRegionVisitor {
 				aReg.setParent(newRegion);
 			}
 		}
-		
+
 		return newRegion;
 	}
 

jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMaker.java

@@ -8,13 +8,11 @@ import jadx.core.dex.instructions.IfNode;
 import jadx.core.dex.instructions.InsnType;
 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;
-import jadx.core.dex.regions.IfCondition;
 import jadx.core.dex.regions.IfInfo;
 import jadx.core.dex.regions.IfRegion;
 import jadx.core.dex.regions.LoopRegion;
@@ -42,10 +40,9 @@ import java.util.Set;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static jadx.core.dex.regions.IfCondition.Mode;
+import static jadx.core.dex.visitors.regions.IfMakerHelper.mergeNestedIfNodes;
 import static jadx.core.utils.BlockUtils.getBlockByOffset;
 import static jadx.core.utils.BlockUtils.isPathExists;
-import static jadx.core.utils.BlockUtils.selectOther;
 
 public class RegionMaker {
 	private static final Logger LOG = LoggerFactory.getLogger(RegionMaker.class);
@@ -110,29 +107,26 @@ public class RegionMaker {
 			}
 		}
 
-		if (!processed) {
-			int size = block.getInstructions().size();
-			if (size == 1) {
-				InsnNode insn = block.getInstructions().get(0);
-				switch (insn.getType()) {
-					case IF:
-						next = processIf(r, block, (IfNode) insn, stack);
-						processed = true;
-						break;
+		if (!processed && block.getInstructions().size() == 1) {
+			InsnNode insn = block.getInstructions().get(0);
+			switch (insn.getType()) {
+				case IF:
+					next = processIf(r, block, (IfNode) insn, stack);
+					processed = true;
+					break;
 
-					case SWITCH:
-						next = processSwitch(r, block, (SwitchNode) insn, stack);
-						processed = true;
-						break;
+				case SWITCH:
+					next = processSwitch(r, block, (SwitchNode) insn, stack);
+					processed = true;
+					break;
 
-					case MONITOR_ENTER:
-						next = processMonitorEnter(r, block, insn, stack);
-						processed = true;
-						break;
+				case MONITOR_ENTER:
+					next = processMonitorEnter(r, block, insn, stack);
+					processed = true;
+					break;
 
-					default:
-						break;
-				}
+				default:
+					break;
 			}
 		}
 		if (!processed) {
@@ -170,10 +164,7 @@ public class RegionMaker {
 		LoopRegion loopRegion = null;
 
 		// exit block with loop condition
-		BlockNode condBlock = null;
-
-		// first block in loop
-		BlockNode bThen = null;
+		IfInfo condInfo = null;
 
 		for (BlockNode exit : exitBlocks) {
 			if (exit.contains(AType.EXC_HANDLER)
@@ -185,7 +176,7 @@ public class RegionMaker {
 				continue;
 			}
 			ifnode = (IfNode) insn;
-			condBlock = exit;
+			BlockNode condBlock = exit;
 
 			loopRegion = new LoopRegion(curRegion, condBlock, condBlock == loop.getEnd());
 			boolean found = true;
@@ -205,46 +196,31 @@ public class RegionMaker {
 				// try another exit
 				continue;
 			}
-
-			List<BlockNode> merged = new ArrayList<BlockNode>(2);
-			IfInfo mergedIf = mergeNestedIfNodes(condBlock, ifnode, 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);
+			condInfo = mergeNestedIfNodes(condBlock);
+			if (condInfo == null) {
+				condInfo = IfMakerHelper.makeIfInfo(condBlock);
 			}
 			break;
 		}
-
 		// endless loop
 		if (loopRegion == null) {
 			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;
-			}
+		if (!loop.getLoopBlocks().contains(condInfo.getThenBlock())) {
+			// invert loop condition if 'then' points to exit
+			condInfo = IfInfo.invert(condInfo);
 		}
+		loopRegion.setCondition(condInfo.getCondition());
+		exitBlocks.removeAll(condInfo.getMergedBlocks());
+
+		BlockNode bThen = condInfo.getThenBlock();
 
 		curRegion.getSubBlocks().add(loopRegion);
 		stack.push(loopRegion);
 
-		exitBlocks.remove(condBlock);
 		if (exitBlocks.size() > 0) {
-			BlockNode loopExit = BlockUtils.selectOtherSafe(loopBody, condBlock.getCleanSuccessors());
+			BlockNode loopExit = condInfo.getElseBlock();
 			if (loopExit != null) {
 				// add 'break' instruction before path cross between main loop exit and subexit
 				for (Edge exitEdge : loop.getExitEdges()) {
@@ -267,10 +243,7 @@ public class RegionMaker {
 			loopRegion.setBody(makeRegion(loopStart, stack));
 			loopStart.addAttr(AType.LOOP, loop);
 		} else {
-			if (bThen != loopBody) {
-				loopRegion.setCondition(IfCondition.invert(loopRegion.getCondition()));
-			}
-			out = selectOther(loopBody, condBlock.getSuccessors());
+			out = condInfo.getElseBlock();
 			if (out.contains(AFlag.LOOP_START)
 					&& !out.getAll(AType.LOOP).contains(loop)
 					&& stack.peekRegion() instanceof LoopRegion) {
@@ -282,6 +255,7 @@ public class RegionMaker {
 				}
 			}
 			stack.addExit(out);
+			BlockNode loopBody = condInfo.getThenBlock();
 			loopRegion.setBody(makeRegion(loopBody, stack));
 		}
 		stack.pop();
@@ -415,7 +389,7 @@ public class RegionMaker {
 		IfRegion ifRegion = new IfRegion(currentRegion, block);
 		currentRegion.getSubBlocks().add(ifRegion);
 
-		IfInfo mergedIf = mergeNestedIfNodes(block, ifnode, null);
+		IfInfo mergedIf = mergeNestedIfNodes(block);
 		if (mergedIf != null) {
 			ifRegion.setCondition(mergedIf.getCondition());
 			thenBlock = mergedIf.getThenBlock();
@@ -476,130 +450,6 @@ public class RegionMaker {
 		return out;
 	}
 
-	private IfInfo mergeNestedIfNodes(BlockNode block, IfNode ifnode, List<BlockNode> merged) {
-		IfInfo info = new IfInfo();
-		info.setIfnode(block);
-		info.setCondition(IfCondition.fromIfBlock(block));
-		info.setThenBlock(ifnode.getThenBlock());
-		info.setElseBlock(ifnode.getElseBlock());
-		return mergeNestedIfNodes(info, merged);
-	}
-
-	private IfInfo mergeNestedIfNodes(IfInfo info, List<BlockNode> merged) {
-		BlockNode bThen = info.getThenBlock();
-		BlockNode bElse = info.getElseBlock();
-		if (bThen == bElse) {
-			return null;
-		}
-
-		BlockNode ifBlock = info.getIfnode();
-		BlockNode nestedIfBlock = getNextIfBlock(ifBlock);
-		if (nestedIfBlock == null) {
-			return null;
-		}
-
-		IfNode nestedIfInsn = (IfNode) nestedIfBlock.getInstructions().get(0);
-		IfCondition nestedCondition = IfCondition.fromIfNode(nestedIfInsn);
-		BlockNode nbThen = nestedIfInsn.getThenBlock();
-		BlockNode nbElse = nestedIfInsn.getElseBlock();
-
-		IfCondition condition = info.getCondition();
-		boolean inverted = false;
-		if (isPathExists(bElse, nestedIfBlock)) {
-			// else branch
-			if (!isEqualPaths(bThen, nbThen)) {
-				if (!isEqualPaths(bThen, nbElse)) {
-					// not connected conditions
-					return null;
-				}
-				nestedIfInsn.invertCondition();
-				inverted = true;
-			}
-			condition = IfCondition.merge(Mode.OR, condition, nestedCondition);
-		} else {
-			// then branch
-			if (!isEqualPaths(bElse, nbElse)) {
-				if (!isEqualPaths(bElse, nbThen)) {
-					// not connected conditions
-					return null;
-				}
-				nestedIfInsn.invertCondition();
-				inverted = true;
-			}
-			condition = IfCondition.merge(Mode.AND, condition, nestedCondition);
-		}
-		if (merged != null) {
-			merged.add(nestedIfBlock);
-		}
-		nestedIfBlock.add(AFlag.SKIP);
-		BlockNode blockToNestedIfBlock = BlockUtils.getNextBlockToPath(ifBlock, nestedIfBlock);
-		skipSimplePath(BlockUtils.selectOther(blockToNestedIfBlock, ifBlock.getCleanSuccessors()));
-
-		IfInfo result = new IfInfo();
-		result.setIfnode(nestedIfBlock);
-		result.setCondition(condition);
-		result.setThenBlock(inverted ? nbElse : nbThen);
-		result.setElseBlock(inverted ? nbThen : nbElse);
-
-		// search next nested if block
-		IfInfo next = mergeNestedIfNodes(result, merged);
-		if (next != null) {
-			return next;
-		}
-		return result;
-	}
-
-	private BlockNode getNextIfBlock(BlockNode block) {
-		for (BlockNode succ : block.getSuccessors()) {
-			BlockNode nestedIfBlock = getIfNode(succ);
-			if (nestedIfBlock != null && nestedIfBlock != block) {
-				return nestedIfBlock;
-			}
-		}
-		return null;
-	}
-
-	private static BlockNode getIfNode(BlockNode block) {
-		if (block != null && !block.contains(AType.LOOP)) {
-			List<InsnNode> insns = block.getInstructions();
-			if (insns.size() == 1 && insns.get(0).getType() == InsnType.IF) {
-				return block;
-			}
-			// skip block
-			List<BlockNode> successors = block.getSuccessors();
-			if (successors.size() == 1) {
-				BlockNode next = successors.get(0);
-				boolean pass = true;
-				if (block.getInstructions().size() != 0) {
-					for (InsnNode insn : block.getInstructions()) {
-						RegisterArg res = insn.getResult();
-						if (res == null) {
-							pass = false;
-							break;
-						}
-						List<RegisterArg> useList = res.getSVar().getUseList();
-						if (useList.size() != 1) {
-							pass = false;
-							break;
-						} else {
-							InsnArg arg = useList.get(0);
-							InsnNode usePlace = arg.getParentInsn();
-							if (!BlockUtils.blockContains(block, usePlace)
-									&& !BlockUtils.blockContains(next, usePlace)) {
-								pass = false;
-								break;
-							}
-						}
-					}
-				}
-				if (pass) {
-					return getIfNode(next);
-				}
-			}
-		}
-		return null;
-	}
-
 	private BlockNode processSwitch(IRegion currentRegion, BlockNode block, SwitchNode insn, RegionStack stack) {
 		SwitchRegion sw = new SwitchRegion(currentRegion, block);
 		currentRegion.getSubBlocks().add(sw);
@@ -749,7 +599,7 @@ public class RegionMaker {
 		handler.getHandlerRegion().addAttr(excHandlerAttr);
 	}
 
-	private void skipSimplePath(BlockNode block) {
+	static void skipSimplePath(BlockNode block) {
 		while (block != null
 				&& block.getCleanSuccessors().size() < 2
 				&& block.getPredecessors().size() == 1) {
@@ -758,7 +608,7 @@ public class RegionMaker {
 		}
 	}
 
-	private static boolean isEqualPaths(BlockNode b1, BlockNode b2) {
+	static boolean isEqualPaths(BlockNode b1, BlockNode b2) {
 		if (b1 == b2) {
 			return true;
 		}

jadx-core/src/main/java/jadx/core/dex/visitors/regions/ReturnVisitor.java

@@ -43,7 +43,7 @@ public class ReturnVisitor extends AbstractVisitor {
 				if (insns.size() == 1
 						&& blockNotInLoop(mth, block)
 						&& noTrailInstructions(block)) {
-					insns.remove(insns.size() - 1);
+					insns.remove(0);
 					block.remove(AFlag.RETURN);
 				}
 			}
@@ -96,7 +96,7 @@ public class ReturnVisitor extends AbstractVisitor {
 		/**
 		 * Check if container not contains instructions,
 		 * don't count one 'return' instruction (it will be removed later).
- 		 */
+		 */
 		private static boolean isEmpty(IContainer container) {
 			if (container instanceof BlockNode) {
 				BlockNode block = (BlockNode) container;
@@ -104,7 +104,7 @@ public class ReturnVisitor extends AbstractVisitor {
 			} else if (container instanceof IRegion) {
 				IRegion region = (IRegion) container;
 				for (IContainer block : region.getSubBlocks()) {
-					if(!isEmpty(block)) {
+					if (!isEmpty(block)) {
 						return false;
 					}
 				}

jadx-core/src/main/java/jadx/core/utils/BlockUtils.java

@@ -203,9 +203,14 @@ public class BlockUtils {
 	}
 
 	public static boolean isPathExists(BlockNode start, BlockNode end) {
-		if (start == end || end.isDominator(start)) {
+		if (start == end
+				|| end.isDominator(start)
+				|| start.getCleanSuccessors().contains(end)) {
 			return true;
 		}
+		if (start.getPredecessors().contains(end)) {
+			return false;
+		}
 		return traverseSuccessorsUntil(start, end, new BitSet());
 	}
 
@@ -257,6 +262,12 @@ public class BlockUtils {
 				return end;
 			}
 		}
+		if (isPathExists(b1, b2)) {
+			return b2;
+		}
+		if (isPathExists(b2, b1)) {
+			return b1;
+		}
 		return null;
 	}
 

jadx-core/src/main/java/jadx/core/utils/RegionUtils.java

@@ -39,7 +39,7 @@ public class RegionUtils {
 	 */
 	public static boolean hasExitBlock(IContainer container) {
 		if (container instanceof BlockNode) {
-			return ((BlockNode) container).getSuccessors().size() == 0;
+			return ((BlockNode) container).getSuccessors().isEmpty();
 		} else if (container instanceof IRegion) {
 			List<IContainer> blocks = ((IRegion) container).getSubBlocks();
 			return !blocks.isEmpty()

jadx-core/src/test/java/jadx/tests/internal/conditions/TestConditions10.java

@@ -0,0 +1,45 @@
+package jadx.tests.internal.conditions;
+
+import jadx.api.InternalJadxTest;
+import jadx.core.dex.nodes.ClassNode;
+
+import org.junit.Test;
+
+import static jadx.tests.utils.JadxMatchers.containsOne;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertThat;
+
+public class TestConditions10 extends InternalJadxTest {
+
+	public static class TestCls {
+
+		public void test(boolean a, int b) throws Exception {
+			if (a || b > 2) {
+				b++;
+			}
+			if (!a || (b >= 0 && b <= 11)) {
+				System.out.println("1");
+			} else {
+				System.out.println("2");
+			}
+			System.out.println("3");
+		}
+	}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+		System.out.println(code);
+
+		assertThat(code, not(containsString("return")));
+		assertThat(code, containsOne("if (a || b > 2) {"));
+		assertThat(code, containsOne("b++;"));
+		assertThat(code, containsOne("if (!a || (b >= 0 && b <= 11)) {"));
+		assertThat(code, containsOne("System.out.println(\"1\");"));
+		assertThat(code, containsOne("} else {"));
+		assertThat(code, containsOne("System.out.println(\"2\");"));
+		assertThat(code, containsOne("System.out.println(\"3\");"));
+	}
+}

jadx-core/src/test/java/jadx/tests/internal/conditions/TestConditions11.java

@@ -0,0 +1,39 @@
+package jadx.tests.internal.conditions;
+
+import jadx.api.InternalJadxTest;
+import jadx.core.dex.nodes.ClassNode;
+
+import org.junit.Test;
+
+import static jadx.tests.utils.JadxMatchers.containsOne;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertThat;
+
+public class TestConditions11 extends InternalJadxTest {
+
+	public static class TestCls {
+
+		public void test(boolean a, int b) {
+			if (a || b > 2) {
+				f();
+			}
+		}
+
+		private void f() {
+		}
+	}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+		System.out.println(code);
+
+		assertThat(code, containsOne("if (a || b > 2) {"));
+		assertThat(code, containsOne("f();"));
+		assertThat(code, not(containsString("return")));
+		assertThat(code, not(containsString("else")));
+
+	}
+}

jadx-core/src/test/java/jadx/tests/internal/conditions/TestConditions12.java

@@ -0,0 +1,71 @@
+package jadx.tests.internal.conditions;
+
+import jadx.api.InternalJadxTest;
+import jadx.core.dex.nodes.ClassNode;
+
+import org.junit.Test;
+
+import static jadx.tests.utils.JadxMatchers.containsOne;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertThat;
+
+public class TestConditions12 extends InternalJadxTest {
+
+	public static class TestCls {
+		static boolean autoStop = true;
+		static boolean qualityReading = false;
+		static int lastValidRaw = -1;
+
+		public static void main(String[] args) throws Exception {
+			int a = 5;
+			int b = 30;
+			dataProcess(a, b);
+		}
+
+		public static void dataProcess(int raw, int quality) {
+			if (quality >= 10 && raw != 0) {
+				System.out.println("OK" + raw);
+				qualityReading = false;
+			} else if (raw == 0 || quality < 6 || !qualityReading) {
+				System.out.println("Not OK" + raw);
+			} else {
+				System.out.println("Quit OK" + raw);
+			}
+			if (quality < 30) {
+				int timeLeft = 30 - quality;
+				if (quality >= 10) {
+					System.out.println("Processing" + timeLeft);
+				}
+			} else {
+				System.out.println("Finish Processing");
+				if (raw > 0) {
+					lastValidRaw = raw;
+				}
+			}
+			if (quality >= 30 && autoStop) {
+				System.out.println("Finished");
+			}
+			if (!autoStop && lastValidRaw > -1 && quality < 10) {
+				System.out.println("Finished");
+			}
+		}
+	}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+		System.out.println(code);
+
+		assertThat(code, containsOne("if (quality >= 10 && raw != 0) {"));
+		assertThat(code, containsOne("} else if (raw == 0 || quality < 6 || !qualityReading) {"));
+		assertThat(code, containsOne("if (quality < 30) {"));
+		assertThat(code, containsOne("if (quality >= 10) {"));
+		assertThat(code, containsOne("if (raw > 0) {"));
+		assertThat(code, containsOne("if (quality >= 30 && autoStop) {"));
+		assertThat(code, containsOne("if (!autoStop && lastValidRaw > -1 && quality < 10) {"));
+		assertThat(code, not(containsString("return")));
+
+	}
+}

jadx-core/src/test/java/jadx/tests/internal/conditions/TestConditions13.java

@@ -0,0 +1,43 @@
+package jadx.tests.internal.conditions;
+
+import jadx.api.InternalJadxTest;
+import jadx.core.dex.nodes.ClassNode;
+
+import org.junit.Test;
+
+import static jadx.tests.utils.JadxMatchers.containsOne;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertThat;
+
+public class TestConditions13 extends InternalJadxTest {
+
+	public static class TestCls {
+		static boolean qualityReading;
+
+		public static void dataProcess(int raw, int quality) {
+			if (quality >= 10 && raw != 0) {
+				System.out.println("OK" + raw);
+				qualityReading = false;
+			} else if (raw == 0 || quality < 6 || !qualityReading) {
+				System.out.println("Not OK" + raw);
+			} else {
+				System.out.println("Quit OK" + raw);
+			}
+		}
+	}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+		System.out.println(code);
+
+		assertThat(code, containsOne("if (quality >= 10 && raw != 0) {"));
+		assertThat(code, containsOne("System.out.println(\"OK\" + raw);"));
+		assertThat(code, containsOne("qualityReading = false;"));
+		assertThat(code, containsOne("} else if (raw == 0 || quality < 6 || !qualityReading) {"));
+		assertThat(code, not(containsString("return")));
+
+	}
+}

jadx-core/src/test/java/jadx/tests/internal/conditions/TestConditions2.java

@@ -8,7 +8,6 @@ import org.junit.Test;
 public class TestConditions2 extends InternalJadxTest {
 
 	public static class TestCls {
-
 		int c;
 		String d;
 		String f;

jadx-core/src/test/java/jadx/tests/internal/conditions/TestConditions5.java

@@ -21,6 +21,18 @@ public class TestConditions5 extends InternalJadxTest {
 				throw new AssertionError(a1 + " != " + a2);
 			}
 		}
+
+		public static void assertEquals2(Object a1, Object a2) {
+			if (a1 != null) {
+				if (!a1.equals(a2)) {
+					throw new AssertionError(a1 + " != " + a2);
+				}
+			} else {
+				if (a2 != null) {
+					throw new AssertionError(a1 + " != " + a2);
+				}
+			}
+		}
 	}
 
 	@Test

jadx-core/src/test/java/jadx/tests/internal/conditions/TestConditions9.java

@@ -0,0 +1,37 @@
+package jadx.tests.internal.conditions;
+
+import jadx.api.InternalJadxTest;
+import jadx.core.dex.nodes.ClassNode;
+
+import org.junit.Test;
+
+import static jadx.tests.utils.JadxMatchers.containsOne;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertThat;
+
+public class TestConditions9 extends InternalJadxTest {
+
+	public static class TestCls {
+		public void test(boolean a, int b) throws Exception {
+			if (!a || (b >= 0 && b <= 11)) {
+				System.out.println('1');
+			} else {
+				System.out.println('2');
+			}
+		}
+	}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+		System.out.println(code);
+
+		assertThat(code, containsOne("if (!a || (b >= 0 && b <= 11)) {"));
+		assertThat(code, containsOne("System.out.println('1');"));
+		assertThat(code, containsOne("} else {"));
+		assertThat(code, containsOne("System.out.println('2');"));
+		assertThat(code, not(containsString("return;")));
+	}
+}

jadx-core/src/test/java/jadx/tests/internal/conditions/TestSimpleConditions.java

@@ -0,0 +1,32 @@
+package jadx.tests.internal.conditions;
+
+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 TestSimpleConditions extends InternalJadxTest {
+
+	public static class TestCls {
+		public boolean test1(boolean[] a) {
+			return (a[0] && a[1] && a[2]) || (a[3] && a[4]);
+		}
+
+		public boolean test2(boolean[] a) {
+			return a[0] || a[1] || a[2] || a[3];
+		}
+	}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+		System.out.println(code);
+
+		assertThat(code, containsString("return (a[0] && a[1] && a[2]) || (a[3] && a[4]);"));
+		assertThat(code, containsString("return a[0] || a[1] || a[2] || a[3];"));
+	}
+}

jadx-core/src/test/java/jadx/tests/internal/loops/TestLoopCondition2.java

@@ -5,7 +5,7 @@ import jadx.core.dex.nodes.ClassNode;
 
 import org.junit.Test;
 
-import static org.hamcrest.CoreMatchers.containsString;
+import static jadx.tests.utils.JadxMatchers.containsOne;
 import static org.junit.Assert.assertThat;
 
 public class TestLoopCondition2 extends InternalJadxTest {
@@ -27,6 +27,9 @@ public class TestLoopCondition2 extends InternalJadxTest {
 		String code = cls.getCode().toString();
 		System.out.println(code);
 
-		assertThat(code, containsString("while (a && i < 10) {"));
+		assertThat(code, containsOne("int i = 0;"));
+		assertThat(code, containsOne("while (a && i < 10) {"));
+		assertThat(code, containsOne("i++;"));
+		assertThat(code, containsOne("return i;"));
 	}
 }