[cfe-dev] Clang Static Analyzer: False Positive Suppression Support

[Alexander Kornienko via cfe-dev]

In clang-tidy we're trying to provide natural ways to mute diagnostics
(e.g. warnings related to implicit casts can be suppressed by adding the
corresponding explicit cast). It's the preferred way, but it's not always
possible, so additionally clang-tidy supports `// NOLINT` suppression
comments without a way to silence specific diagnostics/checks. `// NOLINT`
mutes all diagnostics on the same line.

Cpplint <https://github.com/google/styleguide/tree/gh-pages/cpplint>, where
we took the idea from, additionally supports `// NOLINT(category)` that
allows targeted suppression of diagnostics, and `// NOLINTNEXTLINE` (with
an optional `(category)` as well), which mutes diagnostics on the following
line.

NOLINT was a reasonable solution for clang-tidy, since many cpplint checks
are implemented in clang-tidy as well and suppressions (where needed by
both tools) should not be repeated with a different syntax.

I'm not particularly interested in significantly improving suppression
mechanisms, since in most cases it's better to either improve the analysis
or modify the code. The frequency of suppressions should stay low to avoid
warning blindness and letting the code with suppression comments.

For compiler diagnostics I'm against introducing a comment-based
suppression system at all. It's not the compiler's business to analyze
comments.

On 18 Aug 2016 10:17 AM, "Gábor Horváth" <xazax.hun at gmail.com> wrote:

> Hi!
>
> In the CodeChecker tool (https://github.com/Ericsson/codechecker) we
> support suppressing false positive bugs as a post processing step. After
> all the issues were reported, CodeChecker filters out the suppressed ones,
> and those will not be displayed to the user.
>
> In our experience, however, and external tool only suppression suppport is
> not sufficient for certian reasons:
> * When a checker generates a sink node during analysis, the rest of the
> path will not be covered by the Static Analyzer. Unfortunately, if the user
> suppress the bug in an external tool, the coverage will not get better.
> This way false positive results can hide true positive results regardless
> of suppression.
> * The compiler could do better job diagnosing ill formed suppressions
> (invalid source range, typo in checker name etc).
> * Tools that are developed on top of the compiler need not to introduce
> their own customized solution for suppression.
>
> It is beneficial to have a suppression format that is standard across all
> clang tools. So the same format could be used to:
> * Suppress clang warnings
> * Suppress clang static analyzer warnings
> * Suppress clang tidy warnings
>
> There are two main approaches to suppress warnings that I can think of
> right now:
>
> Suppress using comments (or pragma):
> * This is a good solution for code evolution. The comments are moving
> together with the code. Edits are less likely to invalidate comments.
> * This is an intrusive solution, the source code needs to be changed to
> suppress an issue. This is both good, because the suppressions are version
> controlled and bad, because suppressing in 3rd party libraries might be
> problematic.
> * For path sensitive checks, it is not always self evident at which point
> of the path should a suppression comment be added.
>
>
> Suppress using hashes:
> * It is hard to come up with a reliable hash to identify bugs. There
> always might be corner cases when the hashes of different bugs collide or
> the hash of a bug changes due to an edit that in fact should not affect the
> bug.
> * It is non intrusive.
> * The user do not need to think about at which point should the
> suppression comment be inserted.
>
>
> I would suggest the suppress comment road.
> The syntax could be something like:
> // clang suppress warning-or-checker-name [optional line offset][optional
> column range] [comment]
>
> The warning-or-checker-name could be a regex, so multiple checks can be
> suppressed at the same time.
> Column ranges are useful when multiple errors are reported for the same
> line.
> Line offsets are useful when one do not want to break the flow of a multi
> line code snippet with a comment, so this way warnings could be suppressed
> without too much negative effects on the readability.
> Comment is for documentation purposes, the user can document why does she
> think that this is a false positive.
>
> What do you think?
>
> Regards,
> Gábor
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20160818/97fec240/attachment.html>