[core] TernaryMod proceed when shouldn't

[ewt45]

Issue details

The content of else if (i == 2) is replaced with ternary.
In TernaryMod, insn in else block (k = 0) is removed. However, this insn is also used in other places. In this case ternary shouldn't be processed.

Relevant log output or stacktrace

Provide sample and class/method full name

example code

public static void test(int i, int j) {
    int k;
    if (i == 1) {
        if (j == 2) {
            k = 3;
        } else {
            k = 0;
        }
    } else if (i == 2) {
        if (j == 3) {
            k = 4;
        } else {
            k = 0;
        }
    } else if (i == 3) {
        if (j == 4) {
            k = 5;
        } else {
            k = 0;
        }
    } else {
        k = 0;
    }
    System.out.println("k = " + k);
}

jadx decompile:

/* JADX WARN: Found duplicated region for block: B:6:0x0009  */
public static void test(int i, int i2) {
    int i3 = 3;
    if (i == 1) {
        if (i2 != 2) {
        }
    } else if (i == 2) {
        i3 = i2 == 3 ? 4 : 0;
    } else if (i == 3 && i2 == 4) {
        i3 = 5;
    }
    System.out.println("k = " + i3);
}

Jadx version

latest commit (7ce40ba)

[skylot]

@ewt45 thanks for test case.
Indeed, this is a weird issue, and it only appears using d8, will try to fix this.

Anyway, I agree, jadx sometimes is too aggressive with ternary usage, for example, this test case decompiled from java bytecode (without debug info) results to this:

    public static void test(int i, int i2) {
        int i3 = i == 1 ? i2 == 2 ? 3 : 0 : i == 2 ? i2 == 3 ? 4 : 0 : (i == 3 && i2 == 4) ? 5 : 0;
        System.out.println("k = " + i3);
    }

and this code is correct 🤣

[ewt45]

Oh that looks like a mess... maybe you can limit the times of ternary being wrapped into another ternary?

btw, This is the smali of the java code, compiled by android studio, release variant.

Ternary.smali.txt

[skylot]

Fixed. Ternary limited to one wrapped level and disabled for duplicated blocks.

Although, for this case it might be better to correctly copy constant instruction to avoid block duplication in regions, but this will be harder to implement and will work only for similar cases.

[ewt45]

Thanks for the fix！