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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to