[PATCH] D98726: [analyzer] Enabling MallocChecker to take up after SmartPtrModelling

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D98726#2798325 <https://reviews.llvm.org/D98726#2798325>, @RedDocMD wrote:

> This is what causes the false suppression. To be more specific, the analyzer tries to follow the logic of the destructor of unique_ptr into the standard library. And since that is in the std namespace, it causes LikelyFalsePositiveSuppressionBRVisitor::finalizeVisitor() to squelch the report. Now there are two problems here:

Which of the two reports are you describing here?

> - Why is there a bug in `unique_ptr.h`? (This is the worse of the two, IMO). I am going to take a look at the standard library code (sigh) and see if that's an actual bug or another false positive.

If the warning is / the bug path ends in `unique_ptr.h` it doesn't mean that the bug is in `unique_ptr.h` in particular or in the standard library in general. That's the thing with path sensitive analysis: all nodes of the path are potentially responsible for the bug. Take away any statement in the middle and the entire thing might never happen. This is why path-sensitive checkers will most likely never provide fix-it hints: we aren't supposed to know at all which part of the path was //the// problem, all we know is that the path in its entirety leads to an instance of problematic behavior.

For this reason we're also not massively suppressing all warnings that are displayed against the C standard library headers (unlike clang-tidy). In case of clang-tidy even if there's a bug in the standard library the user will probably be unable to fix it, so it's entirely their job to suppress such warnings. In case of the static analyzer the warning in the standard library isn't an indication that the standard library needs to be fixed at all; most of the time the user can handle it in their code. What we do do, however, is avoid analyzing standard library functions as analysis entry points - as it's a good indication that the entire bug path (and, therefore, the entire list of things to blame) is going to be in the standard library.

We are suppressing warnings against C++ standard library headers though, as you've correctly pointed out. This was a judgement call because it looks like there are too many false positives among these warnings. We should really reconsider this suppression in the future, it could most likely be made much more fine-grained.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98726/new/

https://reviews.llvm.org/D98726