soot-oss / soot

Soot - A Java optimization framework
GNU Lesser General Public License v2.1
2.87k stars 706 forks source link

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

Closed Momo-Not-Emo closed 8 months ago

Momo-Not-Emo commented 8 months ago

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 commented 8 months ago

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