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 S2583/2589 FP: Return inside lock and using causes FP after the block #8495

Closed
pavel-mikula-sonarsource opened this issue Dec 21, 2023 · 0 comments · Fixed by #8761
Closed
Assignees
Labels
Area: C# C# rules related issues. Area: CFG/SE CFG and SE related issues. Area: VB.NET VB.NET rules related issues. Type: CFG/SE FPs Rule IS triggered when it shouldn't be for CFG and SE rules.
Milestone

Comments

@pavel-mikula-sonarsource
Copy link
Contributor

pavel-mikula-sonarsource commented Dec 21, 2023

We have FP when flow exists with return from within the explicit (try/finally) or implicit (foreach, lock, using) TryRegion/FinallyRegion.

Flow if(a)

The return true visits finally block with flag=true state, and then goes to exit block.

Flow if(b)

flag is set to false, normal exit of implicit try/finally visits the finally block with flag=false state,and then goes to exit block.

Flow with b==false

flag remains true and normal exist of implicit try/finally visits the finally block. And because this state has already been visited, it's thrown away.

ToDo

Take closer look on why it doesn't reproduce on the try/finally scenario, that's part of the reproducer. It could involve LVA, where the a is or is not disposed properly.

Possible solution

To fix this, we need to consider what are the finally blocks that should be visited after as a part of the unique key.

We need to consider FinallyPoint of the node, including all FinallyPoint.Previous nodes, as the path could be different in nested scenarios.

public bool Method(bool a, bool b)
{
    bool flag = true;
    lock (Lock)
    {
        if (a)
        {
            return true;
        }
        if (b)
        {
            flag = false;
        }
    }
    if (flag)           // Noncompliant FP
    {
        return true;    // Secondary FP
    }
    else
    {
        return false;
    }
}
@pavel-mikula-sonarsource pavel-mikula-sonarsource added Area: CFG/SE CFG and SE related issues. Area: VB.NET VB.NET rules related issues. Area: C# C# rules related issues. Type: CFG/SE FPs Rule IS triggered when it shouldn't be for CFG and SE rules. labels Dec 21, 2023
@pavel-mikula-sonarsource pavel-mikula-sonarsource changed the title Fix S2583/2589 FP: Return inside lock, using and try/finally causes FP after the block Fix S2583/2589 FP: Return inside lock and using causes FP after the block Dec 21, 2023
@pavel-mikula-sonarsource pavel-mikula-sonarsource added this to the 9.17 milestone Dec 21, 2023
@pavel-mikula-sonarsource pavel-mikula-sonarsource removed this from the 9.17 milestone Dec 21, 2023
@pavel-mikula-sonarsource pavel-mikula-sonarsource added this to the 9.20 milestone Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: C# C# rules related issues. Area: CFG/SE CFG and SE related issues. Area: VB.NET VB.NET rules related issues. Type: CFG/SE FPs Rule IS triggered when it shouldn't be for CFG and SE rules.
Projects
None yet
3 participants