Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] There is a bug in the decompile "while" loop in version 1.3.x #1472

Closed
threerog opened this issue Apr 29, 2022 · 8 comments
Closed

[core] There is a bug in the decompile "while" loop in version 1.3.x #1472

threerog opened this issue Apr 29, 2022 · 8 comments
Labels
bug Core Issues in jadx-core module regression

Comments

@threerog
Copy link

This is the result comparison between version 1.3 and 1.2:
Missing jump out statements and reverse the order of loop variables
1
2

@threerog threerog added bug Core Issues in jadx-core module labels Apr 29, 2022
@skylot
Copy link
Owner

skylot commented Apr 29, 2022

@threerog it will be much easier to reproduce your issue if you share sample dex or apk.

@threerog
Copy link
Author

hc3.b.j
mk3.a.Y

classes6.zip

@threerog
Copy link
Author

I'm using jadx 1.3.x, we often encounter the error problem of "while" context. Is this the first time.

@skylot skylot removed the no-sample label Apr 29, 2022
@skylot
Copy link
Owner

skylot commented Apr 29, 2022

@threerog method in class hc3.b looks fine for me in version 1.3.5:

    public static final int j(byte[] bArr, int i) {
        int i2;
        while (true) {
            i2 = i - 1;
            if (i == 0 || bArr[i2] != 0) {
                break;
            }
            i = i2;
        }
        return i2 + 1;
    }

Class mk3.a not exist in supplied sample 😢

@threerog
Copy link
Author

Carefully compare the pictures. The logic of version 1.3 is verified to be wrong, and version 1.2 is correct

classes7.zip

@skylot
Copy link
Owner

skylot commented Apr 29, 2022

The logic of version 1.3 is verified to be wrong

I disagree, check smali:

.method public static final j([BI)I
    .registers 3

    :goto_0
    add-int/lit8 v0, p1, -0x1
    if-eqz p1, :cond_a

    aget-byte p1, p0, v0
    if-nez p1, :cond_a
    move p1, v0
    goto :goto_0

    :cond_a
    add-int/lit8 v0, v0, 0x1
    return v0
.end method

If p1 equals zero, execution goes to return (cond_a). Anyway, endless loop looks wrong 🙂

For other case I commit a workaround fix to restore correct order of operations. For complete fix I don't have negative case, so I postpone it until new cases will be found.

@threerog thanks for report! 👍

@skylot skylot closed this as completed Apr 29, 2022
@nitram84
Copy link
Contributor

@skylot: Sorry for answering to a closed ticket. This is just an idea: If you are able to detect those missing cases or patterns, you could add a debug message to the generated code (e.g. "Please review this decompiled code carefully - in case of decompilation errors report this issue with reference ." I remember the "multi-entry loop pattern" message which is a good indicator for broken, but often compilable code. Without a message errors like this are hard to find and would require a code review. To reduce false-positive reports of those issues, messages for missing patterns could be limited to certain conditions (e.g. on active in debug mode, ...). I'm sure all missing patterns and cases can be found.

@skylot
Copy link
Owner

skylot commented Apr 30, 2022

@nitram84

If you are able to detect those missing cases or patterns

Unfortunately, I can't in most cases 😢
Because most issues are "unexpected": transformations break code semantics because of incorrect checks or unknown to me cases. To fix that, I can only add additional checks on result IR, like "Code restructure failed: missing block" and even these checks due to bugs can show false-positive reports.
Some time ago I started work on control flow verifier, but it produces too many issues even on jadx test suite and need a lot of work to complete, also it is very resource consuming. Maybe I will try again to finish it, these checks should cover issues like missing break and other code restructure problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Core Issues in jadx-core module regression
Projects
None yet
Development

No branches or pull requests

3 participants