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

Fix always-false condition in <AsmMethodSource: void assignReadOps(Local l)> #2046

Merged

Conversation

Momo-Not-Emo
Copy link
Contributor

Problem Description:

The always-false condition was initially reported in Soot PR #1834, resulting in a branch that is never reached or covered. This issue may lead to redundant Jimple statements, as addressed by the fix in SootUp PR #472 in SootUp. Unfortunately, Soot PR #1834 was closed without being merged due to the absence of test cases.

Solution Overview:

I have ported the test cases from SootUp PR #472 to Soot and observed that the current Soot implementation does not generate redundant statements despite retaining the always-false condition.

Changes Made:

I corrected the always-false condition by referencing both Soot PR #1834 and SootUp PR #472. Following these modifications, all tests, including the ported ones, pass successfully when executing mvn -B clean test -PJava8, mvn -B clean test -PJava9, and mvn -B clean test -PJava11. However, one ported test consistently produces redundant Jimple statements, as reported in Issue #2045.

Further Insights:

The results suggest that the current Soot implementation may not require the if-structure, as it has no impact on the test cases, whether the condition is reachable or not. This conclusion is supported by tracking the line coverage in Jacoco reports before and after the modification.

@StevenArzt
Copy link
Contributor

Thank you for this effort, much appreciated. Could you take a look at the failed style check in our build pipeline and fix that?

@StevenArzt StevenArzt merged commit 95e95bb into soot-oss:develop Feb 9, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants