diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/maker/IfRegionMaker.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/maker/IfRegionMaker.java index 79e9b3813f1..eb9b1c24280 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/maker/IfRegionMaker.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/maker/IfRegionMaker.java @@ -68,17 +68,15 @@ BlockNode process(IRegion currentRegion, BlockNode block, IfNode ifnode, RegionS IfInfo mergedIf = mergeNestedIfNodes(currentIf); if (mergedIf != null) { currentIf = mergedIf; - } else { + } else if (!block.contains(AFlag.DONT_INVERT)) { // invert simple condition (compiler often do it) - // ensure that we only ever invert once, because if multiple regions contain this block - // we'll change the block after it's already been included in a region, which can cause - // other regions containing the block to believe the condition has been flipped when it - // has not, or vice versa. - if (!block.contains(AFlag.DONT_INVERT)) { - currentIf = IfInfo.invert(currentIf); - block.add(AFlag.DONT_INVERT); - } + currentIf = IfInfo.invert(currentIf); } + // Guard the simple inversion above from being applied again on a later region-head pass over + // this shared block (see AFlag.DUPLICATED): its 'if' instruction is inverted in place, so a + // second inversion would flip an operand of the condition already built here. Set the flag + // after a merge too, since that branch skips the simple inversion above. + block.add(AFlag.DONT_INVERT); IfInfo modifiedIf = restructureIf(block, currentIf); if (modifiedIf != null) { currentIf = modifiedIf; diff --git a/jadx-core/src/test/java/jadx/tests/integration/conditions/TestDuplicatedBlockCondition.java b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestDuplicatedBlockCondition.java new file mode 100644 index 00000000000..dae680eab09 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestDuplicatedBlockCondition.java @@ -0,0 +1,19 @@ +package jadx.tests.integration.conditions; + +import org.junit.jupiter.api.Test; + +import jadx.tests.api.RaungTest; + +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; + +public class TestDuplicatedBlockCondition extends RaungTest { + + @Test + public void test() { + allowWarnInCode(); // block is duplicated, see TestConditions22 + assertThat(getClassNodeFromRaung()) + .code() + .containsOne("if (z3 || z4) {") + .doesNotContain("!z3 || z4"); + } +} diff --git a/jadx-core/src/test/raung/conditions/TestDuplicatedBlockCondition.raung b/jadx-core/src/test/raung/conditions/TestDuplicatedBlockCondition.raung new file mode 100644 index 00000000000..effe8695359 --- /dev/null +++ b/jadx-core/src/test/raung/conditions/TestDuplicatedBlockCondition.raung @@ -0,0 +1,41 @@ +.version 50 +.class public super conditions/TestDuplicatedBlockCondition +.super java/lang/Object +.auto frames + +# Java equivalent: +# static int test(boolean a, boolean b, boolean x, boolean y) { +# int r = 2; +# if (a || b) { # 'if' body is a block with two predecessors -> DUPLICATED +# r = (x || y) ? 1 : 0; +# } +# return r; +# } +# +# The '(x || y)' block heads a merged short-circuit condition and is also reached from +# two predecessors, so it is processed as a region head twice. The second (standalone) +# pass must not invert its shared 'if' instruction again, otherwise the first operand of +# the already built condition gets flipped ('x || y' -> '!x || y'). + +.method public static test(ZZZZ)I + iload 0 + ifne :L_body + iconst_2 + istore 4 + iload 1 + ifeq :L_return + :L_body + iload 2 + ifne :L_one + iload 3 + ifne :L_one + iconst_0 + istore 4 + goto :L_return + :L_one + iconst_1 + istore 4 + :L_return + iload 4 + ireturn +.end method