[cfe-dev] [analyzer][RFC] Attribute(s) to enhance/configure the analysis

[Aaron Ballman via cfe-dev]

On Tue, Oct 20, 2020 at 11:34 AM Gábor Márton <martongabesz at gmail.com> wrote:
>
> Hi,
>
> There is an evolving need to configure the Clang Static Analyzer within the analyzed source code itself. We'd like to
> 1) suppress specific checkers (we already have an ongoing discussion at D89638)
> 2) express summaries (mainly argument constraints)
> 3) express taint propagation rules for functions (or for global variables like std::cin)
>
> What if we had one attribute for CSA with a StringArgument?
> (Actually, we already have that with the `annotate` attribute.)
>
> So we'd have something like this:
> 1) [[clang::csa("supress.somecheck.somefunctionality")]]
> 2) [[clang::csa("summary.std::fread.BufferSize.Buffer(0).BufSize(1).BufSizeMultiplier(2)")]]
> 3) [[clang::csa("taint.sink.myNamespace::mySink")]]
>
> Disadvantages: we must process strings whenever a node has the 'csa' attr attached, we have to come up with a "DSL".
> Advantages: total flexibility.
>
> I'd like to explore the possible approaches that we could have. For example, Aaron suggested alternatively for the suppression:
> [[clang::suppress("analyzer.somecheck.somefunctionality")]]
> [[clang::suppress("compiler.warning.12345")]]
> [[clang::suppress("tidy.check-name.whatever")]]

Thank you for exploring the options in this space -- I think having a
uniform way for users to suppress diagnostics using attributes is a
really interesting idea. I have personally never really liked using
pragmas to do this work because that approach is usually quite verbose
when applied to a single line of code.

I think there are some questions we should keep in mind:
* How do we name diagnostics for the various components that issue
diagnostics? Does Clang need to introduce a per-warning diagnostic
identifier (e.g., C1234 like done in MSVC)
* Do we want to suppress at the declaration/statement level, or do we
want to support suppressing a range of lines? (I think we only want
decl/stmt level as pragma can be used for a range of lines.)
* Do we want the user to be able to suppress more than diagnostics by
identifier in the same attribute? e.g.,
[[clang::suppress("compiler.warning.12345", "tidy.frobble.wonky")]]
(My answer is yes.)
* Do we want the user to be able to suppress all diagnostics? e.g.,
[[clang::suppress]] (My answer is yes.)
* Do we want the user to be able to suppress diagnostics using some
grouping technique? e.g., [[clang::suppress("tidy.cert.*")]] or
[[clang::suppress("compiler.attributes")]] akin to -Wno-attributes
* If the user has a suppression attribute and the diagnostic listed is
*not* issued, do we want to give a user a way to notice that so they
can remove potentially stale annotations? e.g.,
-Wsuppressed-but-not-diagnosed
* Should the user be able to suppress warnings that have been upgraded
into an error via the command line?
* Does suppressing a diagnostic also suppress associated notes? (I
would assume the answer is yes.)

Regarding the name of the attribute, I would recommend avoiding "csa"
as that's not likely to be an acronym users will have a lot of
experience with. Given that all the tools that emit diagnostics which
would use this functionality are clang-based, I think using the
"clang" vendor namespace is sufficient (I'd rather not have
csa::suppress, clang::suppress, and tidy::suppress unless there's a
really good reason for it).

~Aaron