fix: preserve shared if branch targets

jadx-core/src/main/java/jadx/core/dex/visitors/regions/maker/IfRegionMaker.java

@@ -188,10 +188,14 @@ IfInfo restructureIf(BlockNode block, IfInfo info) {
 			return info;
 		}
 		// init outblock, which will be used in isBadBranchBlock to compare with branch block
-		info.setOutBlock(findOutBlock(mth, thenBlock, elseBlock));
+		BlockNode outBlock = findOutBlock(mth, thenBlock, elseBlock);
+		if (!isOutBlockInCurrentScope(outBlock)) {
+			outBlock = null;
+		}
+		info.setOutBlock(outBlock);
 
-		boolean badThen = isBadBranchBlock(info, thenBlock);
-		boolean badElse = isBadBranchBlock(info, elseBlock);
+		boolean badThen = isBadBranchBlock(block, info, thenBlock, elseBlock);
+		boolean badElse = isBadBranchBlock(block, info, elseBlock, thenBlock);
 		if (badThen && badElse) {
 			if (Consts.DEBUG_RESTRUCTURE) {
 				LOG.debug("Stop processing blocks after 'if': {}, method: {}", info.getMergedBlocks(), mth);
@@ -227,6 +231,18 @@ IfInfo restructureIf(BlockNode block, IfInfo info) {
 		return info;
 	}
 
+	private boolean isOutBlockInCurrentScope(@Nullable BlockNode outBlock) {
+		if (outBlock == null) {
+			return true;
+		}
+		for (BlockNode exit : regionMaker.getStack().getExits()) {
+			if (BlockUtils.isPathExists(exit, outBlock)) {
+				return false;
+			}
+		}
+		return true;
+	}
+
 	static @Nullable BlockNode findOutBlock(MethodNode mth, BlockNode thenBlock, BlockNode elseBlock) {
 		if (thenBlock == elseBlock) {
 			return thenBlock;
@@ -321,12 +337,12 @@ static boolean isCandidateForOutBlock(MethodNode mth, BlockNode thenBlock, Block
 		return true;
 	}
 
-	private static boolean isBadBranchBlock(IfInfo info, BlockNode block) {
+	private boolean isBadBranchBlock(BlockNode ifBlock, IfInfo info, BlockNode branchBlock, BlockNode siblingBlock) {
 		// check if block at end of loop edge
-		if (block.contains(AFlag.LOOP_START) && block.getPredecessors().size() == 1) {
-			BlockNode pred = block.getPredecessors().get(0);
+		if (branchBlock.contains(AFlag.LOOP_START) && branchBlock.getPredecessors().size() == 1) {
+			BlockNode pred = branchBlock.getPredecessors().get(0);
 			if (pred.contains(AFlag.LOOP_END)) {
-				List<LoopInfo> startLoops = block.getAll(AType.LOOP);
+				List<LoopInfo> startLoops = branchBlock.getAll(AType.LOOP);
 				List<LoopInfo> endLoops = pred.getAll(AType.LOOP);
 				// search for same loop
 				for (LoopInfo startLoop : startLoops) {
@@ -340,9 +356,15 @@ private static boolean isBadBranchBlock(IfInfo info, BlockNode block) {
 		}
 		// if branch block itself is outblock
 		if (info.getOutBlock() != null) {
-			return block == info.getOutBlock();
+			return branchBlock == info.getOutBlock();
+		}
+		if (allPathsFromIf(branchBlock, info)) {
+			return false;
 		}
-		return !allPathsFromIf(block, info);
+		// An incoming edge from outside the condition doesn't make a branch an out block by itself.
+		// Promote it only when every sibling path which can fall through stays inside this if's
+		// dominated scope until it reaches the branch. Other paths may terminate or loop.
+		return isBranchContinuation(ifBlock, siblingBlock, branchBlock);
 	}
 
 	private static boolean allPathsFromIf(BlockNode block, IfInfo info) {
@@ -361,6 +383,37 @@ private static boolean allPathsFromIf(BlockNode block, IfInfo info) {
 		return true;
 	}
 
+	private boolean isBranchContinuation(BlockNode ifBlock, BlockNode siblingBlock, BlockNode branchBlock) {
+		BitSet visited = newBlocksBitSet(mth);
+		return allFallThroughPathsLeadToBranch(ifBlock, siblingBlock, branchBlock, visited);
+	}
+
+	private boolean allFallThroughPathsLeadToBranch(
+			BlockNode ifBlock, BlockNode block, BlockNode branchBlock, BitSet visited) {
+		if (block == branchBlock || BlockUtils.isExitBlock(mth, block)) {
+			return true;
+		}
+		// Reaching a block not dominated by this if means this path escaped through a different
+		// shared target, so moving branchBlock after the if would add execution to that path.
+		if (!block.isDominator(ifBlock)) {
+			return false;
+		}
+		int pos = block.getPos();
+		if (visited.get(pos)) {
+			return true;
+		}
+		visited.set(pos);
+		for (BlockNode successor : block.getCleanSuccessors()) {
+			if (BlockUtils.isBackEdge(block, successor)) {
+				continue;
+			}
+			if (!allFallThroughPathsLeadToBranch(ifBlock, successor, branchBlock, visited)) {
+				return false;
+			}
+		}
+		return true;
+	}
+
 	/**
 	 * if startBlock is in a (try) scope, find the scope end as outBlock
 	 */

jadx-core/src/test/java/jadx/tests/integration/conditions/TestSharedIfBranchTarget.java

@@ -0,0 +1,16 @@
+package jadx.tests.integration.conditions;
+
+import org.junit.jupiter.api.Test;
+
+import jadx.tests.api.SmaliTest;
+
+import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
+
+public class TestSharedIfBranchTarget extends SmaliTest {
+	@Test
+	public void testSmali() {
+		allowWarnInCode();
+		assertThat(getClassNodeFromSmali())
+				.runDecompiledAutoCheck(this);
+	}
+}

jadx-core/src/test/java/jadx/tests/integration/switches/TestSwitchSharedCaseTargets.java

@@ -0,0 +1,22 @@
+package jadx.tests.integration.switches;
+
+import org.junit.jupiter.api.Test;
+
+import jadx.tests.api.SmaliTest;
+
+import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
+
+public class TestSwitchSharedCaseTargets extends SmaliTest {
+	@Test
+	public void testSmali() {
+		allowWarnInCode();
+		assertThat(getClassNodeFromSmali())
+				.code()
+				.containsOne("case 3:")
+				.containsOne("if (i2 != 3) {")
+				.containsOne("} else {")
+				.containsOne("stop();")
+				.countString(2, "fail();")
+				.countString(2, "complete();");
+	}
+}

jadx-core/src/test/smali/conditions/TestSharedIfBranchTarget.smali

@@ -0,0 +1,64 @@
+.class public Lconditions/TestSharedIfBranchTarget;
+.super Ljava/lang/Object;
+
+.method public static test(II)I
+    .registers 3
+
+    const/4 v0, 0x0
+    if-eqz p0, :outer_complete
+
+    const/4 v1, 0x3
+    if-ne p1, v1, :do_stop
+    goto :complete
+
+    :outer_complete
+    add-int/lit8 v0, v0, 0x1
+    goto :complete
+
+    :do_stop
+    add-int/lit8 v0, v0, 0x2
+    goto :fail
+
+    :complete
+    add-int/lit8 v0, v0, 0x4
+    goto :after
+
+    :fail
+    add-int/lit8 v0, v0, 0x8
+
+    :after
+    add-int/lit8 v0, v0, 0x10
+    return v0
+.end method
+
+.method public check()V
+    .registers 4
+
+    const/4 v0, 0x0
+    const/4 v1, 0x0
+    invoke-static {v0, v1}, Lconditions/TestSharedIfBranchTarget;->test(II)I
+    move-result v2
+    const/16 v3, 0x15
+    if-ne v2, v3, :fail
+
+    const/4 v0, 0x1
+    const/4 v1, 0x3
+    invoke-static {v0, v1}, Lconditions/TestSharedIfBranchTarget;->test(II)I
+    move-result v2
+    const/16 v3, 0x14
+    if-ne v2, v3, :fail
+
+    const/4 v0, 0x1
+    const/4 v1, 0x0
+    invoke-static {v0, v1}, Lconditions/TestSharedIfBranchTarget;->test(II)I
+    move-result v2
+    const/16 v3, 0x1a
+    if-ne v2, v3, :fail
+
+    return-void
+
+    :fail
+    new-instance v0, Ljava/lang/AssertionError;
+    invoke-direct {v0}, Ljava/lang/AssertionError;-><init>()V
+    throw v0
+.end method

jadx-core/src/test/smali/switches/TestSwitchSharedCaseTargets.smali

@@ -0,0 +1,50 @@
+.class public Lswitches/TestSwitchSharedCaseTargets;
+.super Ljava/lang/Object;
+
+.method public static test(II)V
+    .registers 4
+
+    packed-switch p0, :pswitch_data
+    goto :end
+
+    :case_cond
+    const/4 v0, 0x3
+    if-ne p1, v0, :do_stop
+    goto :complete
+
+    :do_stop
+    invoke-static {}, Lswitches/TestSwitchSharedCaseTargets;->stop()V
+    goto :fail
+
+    :complete
+    invoke-static {}, Lswitches/TestSwitchSharedCaseTargets;->complete()V
+    goto :end
+
+    :fail
+    invoke-static {}, Lswitches/TestSwitchSharedCaseTargets;->fail()V
+
+    :end
+    return-void
+
+    :pswitch_data
+    .packed-switch 0x1
+        :fail
+        :complete
+        :case_cond
+    .end packed-switch
+.end method
+
+.method private static stop()V
+    .registers 0
+    return-void
+.end method
+
+.method private static fail()V
+    .registers 0
+    return-void
+.end method
+
+.method private static complete()V
+    .registers 0
+    return-void
+.end method