[clang] [analyzer] MallocChecker: Recognize std::atomics in smart pointer suppression. (PR #90918)

Balazs Benics via cfe-commits cfe-commits at lists.llvm.org
Fri May 3 01:56:27 PDT 2024


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


More information about the cfe-commits mailing list