https://github.com/steakhal commented:

Hah!
I've just debugged an identical case last week, wrt. llvm intrusive pointers, 
which also used std::atomics inside for the refcount and lead to a Leak FP.

When I dag into the case, I realized that the root cause is that we can't track 
the value of the `std::atomic` from zero, inc, dec and then fail to prove that 
it must be equal to zero and take the "delete" path.

This is not strictly related to leak reports, probably that's the most affected 
checker - but one can craft any other case with other checkers too. This is why 
I wanted a solution not special-casing the leak checker, but something more 
generic.

I considered modeling the atomic member functions, until the first place they 
escape, after which we can no longer ever reason about them. This made me to 
look into suppressions.

My first idea was to check what "interesting symbols" we have after a BR 
traversal and check if any is a conjured one conjured for a callexpr statement 
calling a std::atomic-related function - and suppress the report in that case.
I realized that this wouldn't be enough, as control dependencies such as 
conditions, wouldn't be considered for leaks, thus not suppress the motivating 
FP example.

My next idea was to also check the `TrackedConditions` of the bug report, and 
look for expressions that refer to a subexpression for which we associate a 
conjured symbol, etc. and apply the same logic as before.
It didn't work because it would suppress all paths crossing that condition, no 
matter if the branch was taken or not.
Now, the final idea was to only consider conditions, which dominate the basic 
block of the bug report itself. This wouldn't solve the leak FP per-say, but 
would work for any other bug report.

I believe, that such a heuristic with dominators could work for the rest of the 
checkers - where the bug report is attached to some given statement - and not 
delayed until some other future statement like in the leak checker.

All in all, I think the suppression you propose here makes sense.
I've just left here what my thought process was when I dig into a similar case. 
It might be useful, who knows.

https://github.com/llvm/llvm-project/pull/90918
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to