Skip to content

Commit 27d8aa4

Browse files
committed
[attributes][analyzer] Implement [[clang::suppress]] - suppress static analysis warnings.
The new attribute can be placed on statements in order to suppress arbitrary warnings produced by static analysis tools at those statements. Previously such suppressions were implemented as either informal comments (eg. clang-tidy `// NOLINT:`) or with preprocessor macros (eg. clang static analyzer's `#ifdef __clang_analyzer__`). The attribute provides a universal, formal, flexible and neat-looking suppression mechanism. Implement support for the new attribute in the clang static analyzer; clang-tidy coming soon. The attribute allows specifying which specific warnings to suppress, in the form of free-form strings that are intended to be specific to the tools, but currently none are actually supported; so this is also going to be a future improvement. Differential Revision: https://reviews.llvm.org/D93110 (cherry picked from commit ef3f476) Conflicts: clang/test/SemaCXX/suppress.cpp
1 parent 2a31eae commit 27d8aa4

File tree

15 files changed

+803
-39
lines changed

15 files changed

+803
-39
lines changed

clang/include/clang/Basic/Attr.td

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2742,9 +2742,10 @@ def SwiftAsyncError : InheritableAttr {
27422742
let Documentation = [SwiftAsyncErrorDocs];
27432743
}
27442744

2745-
def Suppress : StmtAttr {
2746-
let Spellings = [CXX11<"gsl", "suppress">];
2745+
def Suppress : DeclOrStmtAttr {
2746+
let Spellings = [CXX11<"gsl", "suppress">, Clang<"suppress">];
27472747
let Args = [VariadicStringArgument<"DiagnosticIdentifiers">];
2748+
let Accessors = [Accessor<"isGSL", [CXX11<"gsl", "suppress">]>];
27482749
let Documentation = [SuppressDocs];
27492750
}
27502751

clang/include/clang/Basic/AttrDocs.td

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5295,7 +5295,74 @@ the ``int`` parameter is the one that represents the error.
52955295
def SuppressDocs : Documentation {
52965296
let Category = DocCatStmt;
52975297
let Content = [{
5298-
The ``[[gsl::suppress]]`` attribute suppresses specific
5298+
The ``suppress`` attribute suppresses unwanted warnings coming from static
5299+
analysis tools such as the Clang Static Analyzer. The tool will not report
5300+
any issues in source code annotated with the attribute.
5301+
5302+
The attribute cannot be used to suppress traditional Clang warnings, because
5303+
many such warnings are emitted before the attribute is fully parsed.
5304+
Consider using ``#pragma clang diagnostic`` to control such diagnostics,
5305+
as described in `Controlling Diagnostics via Pragmas
5306+
<https://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-via-pragmas>`_.
5307+
5308+
The ``suppress`` attribute can be placed on an individual statement in order to
5309+
suppress warnings about undesirable behavior occurring at that statement:
5310+
5311+
.. code-block:: c++
5312+
5313+
int foo() {
5314+
int *x = nullptr;
5315+
...
5316+
[[clang::suppress]]
5317+
return *x; // null pointer dereference warning suppressed here
5318+
}
5319+
5320+
Putting the attribute on a compound statement suppresses all warnings in scope:
5321+
5322+
.. code-block:: c++
5323+
5324+
int foo() {
5325+
[[clang::suppress]] {
5326+
int *x = nullptr;
5327+
...
5328+
return *x; // warnings suppressed in the entire scope
5329+
}
5330+
}
5331+
5332+
Some static analysis warnings are accompanied by one or more notes, and the
5333+
line of code against which the warning is emitted isn't necessarily the best
5334+
for suppression purposes. In such cases the tools are allowed to implement
5335+
additional ways to suppress specific warnings based on the attribute attached
5336+
to a note location.
5337+
5338+
For example, the Clang Static Analyzer suppresses memory leak warnings when
5339+
the suppression attribute is placed at the allocation site (highlited by
5340+
a "note: memory is allocated"), which may be different from the line of code
5341+
at which the program "loses track" of the pointer (where the warning
5342+
is ultimately emitted):
5343+
5344+
.. code-block:: c
5345+
5346+
int bar1(bool coin_flip) {
5347+
__attribute__((suppress))
5348+
int *result = (int *)malloc(sizeof(int));
5349+
if (coin_flip)
5350+
return 1; // warning about this leak path is suppressed
5351+
5352+
return *result; // warning about this leak path is also suppressed
5353+
}
5354+
5355+
int bar2(bool coin_flip) {
5356+
int *result = (int *)malloc(sizeof(int));
5357+
if (coin_flip)
5358+
return 1; // leak warning on this path NOT suppressed
5359+
5360+
__attribute__((suppress))
5361+
return *result; // leak warning is suppressed only on this path
5362+
}
5363+
5364+
5365+
When written as ``[[gsl::suppress]]``, this attribute suppresses specific
52995366
clang-tidy diagnostics for rules of the `C++ Core Guidelines`_ in a portable
53005367
way. The attribute can be attached to declarations, statements, and at
53015368
namespace scope.

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "clang/Basic/SourceLocation.h"
2020
#include "clang/Lex/Preprocessor.h"
2121
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
22+
#include "clang/StaticAnalyzer/Core/BugReporter/BugSuppression.h"
2223
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
2324
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
2425
#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
@@ -595,6 +596,9 @@ class BugReporter {
595596
/// A vector of BugReports for tracking the allocated pointers and cleanup.
596597
std::vector<BugReportEquivClass *> EQClassesVector;
597598

599+
/// User-provided in-code suppressions.
600+
BugSuppression UserSuppressions;
601+
598602
public:
599603
BugReporter(BugReporterData &d);
600604
virtual ~BugReporter();
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
//===- BugSuppression.h - Suppression interface -----------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
//
9+
// This file defines BugSuppression, a simple interface class encapsulating
10+
// all user provided in-code suppressions.
11+
//
12+
//===----------------------------------------------------------------------===//
13+
14+
#ifndef LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_SUPPRESSION_H
15+
#define LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_SUPPRESSION_H
16+
17+
#include "clang/Basic/SourceLocation.h"
18+
#include "llvm/ADT/DenseMap.h"
19+
#include "llvm/ADT/SmallVector.h"
20+
21+
namespace clang {
22+
class Decl;
23+
24+
namespace ento {
25+
class BugReport;
26+
class PathDiagnosticLocation;
27+
28+
class BugSuppression {
29+
public:
30+
using DiagnosticIdentifierList = llvm::ArrayRef<llvm::StringRef>;
31+
32+
/// Return true if the given bug report was explicitly suppressed by the user.
33+
bool isSuppressed(const BugReport &);
34+
35+
/// Return true if the bug reported at the given location was explicitly
36+
/// suppressed by the user.
37+
bool isSuppressed(const PathDiagnosticLocation &Location,
38+
const Decl *DeclWithIssue,
39+
DiagnosticIdentifierList DiagnosticIdentification);
40+
41+
private:
42+
// Overly pessimistic number, to be honest.
43+
static constexpr unsigned EXPECTED_NUMBER_OF_SUPPRESSIONS = 8;
44+
using CachedRanges =
45+
llvm::SmallVector<SourceRange, EXPECTED_NUMBER_OF_SUPPRESSIONS>;
46+
47+
llvm::DenseMap<const Decl *, CachedRanges> CachedSuppressionLocations;
48+
};
49+
50+
} // end namespace ento
51+
} // end namespace clang
52+
53+
#endif // LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_SUPPRESSION_H

clang/lib/Sema/SemaDeclAttr.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5209,8 +5209,16 @@ static void handleCallConvAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
52095209
}
52105210

52115211
static void handleSuppressAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
5212-
if (!AL.checkAtLeastNumArgs(S, 1))
5212+
if (AL.getAttributeSpellingListIndex() == SuppressAttr::CXX11_gsl_suppress) {
5213+
// Suppression attribute with GSL spelling requires at least 1 argument.
5214+
if (!AL.checkAtLeastNumArgs(S, 1))
5215+
return;
5216+
} else if (!isa<VarDecl>(D)) {
5217+
// Analyzer suppression applies only to variables and statements.
5218+
S.Diag(AL.getLoc(), diag::err_attribute_wrong_decl_type_str)
5219+
<< AL << 0 << "variables and statements";
52135220
return;
5221+
}
52145222

52155223
std::vector<StringRef> DiagnosticIdentifiers;
52165224
for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) {
@@ -5219,8 +5227,6 @@ static void handleSuppressAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
52195227
if (!S.checkStringLiteralArgumentAttr(AL, I, RuleName, nullptr))
52205228
return;
52215229

5222-
// FIXME: Warn if the rule name is unknown. This is tricky because only
5223-
// clang-tidy knows about available rules.
52245230
DiagnosticIdentifiers.push_back(RuleName);
52255231
}
52265232
D->addAttr(::new (S.Context)

clang/lib/Sema/SemaStmtAttr.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,20 @@ static Attr *handleFallThroughAttr(Sema &S, Stmt *St, const ParsedAttr &A,
5353

5454
static Attr *handleSuppressAttr(Sema &S, Stmt *St, const ParsedAttr &A,
5555
SourceRange Range) {
56+
if (A.getAttributeSpellingListIndex() == SuppressAttr::CXX11_gsl_suppress &&
57+
A.getNumArgs() < 1) {
58+
// Suppression attribute with GSL spelling requires at least 1 argument.
59+
S.Diag(A.getLoc(), diag::err_attribute_too_few_arguments) << A << 1;
60+
return nullptr;
61+
}
62+
5663
std::vector<StringRef> DiagnosticIdentifiers;
5764
for (unsigned I = 0, E = A.getNumArgs(); I != E; ++I) {
5865
StringRef RuleName;
5966

6067
if (!S.checkStringLiteralArgumentAttr(A, I, RuleName, nullptr))
6168
return nullptr;
6269

63-
// FIXME: Warn if the rule name is unknown. This is tricky because only
64-
// clang-tidy knows about available rules.
6570
DiagnosticIdentifiers.push_back(RuleName);
6671
}
6772

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,15 @@
1212
//===----------------------------------------------------------------------===//
1313

1414
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
15+
#include "clang/AST/ASTTypeTraits.h"
16+
#include "clang/AST/Attr.h"
1517
#include "clang/AST/Decl.h"
1618
#include "clang/AST/DeclBase.h"
1719
#include "clang/AST/DeclObjC.h"
1820
#include "clang/AST/Expr.h"
1921
#include "clang/AST/ExprCXX.h"
2022
#include "clang/AST/ParentMap.h"
23+
#include "clang/AST/ParentMapContext.h"
2124
#include "clang/AST/Stmt.h"
2225
#include "clang/AST/StmtCXX.h"
2326
#include "clang/AST/StmtObjC.h"
@@ -2427,6 +2430,12 @@ PathSensitiveBugReport::getLocation() const {
24272430
}
24282431

24292432
if (S) {
2433+
// Attributed statements usually have corrupted begin locations,
2434+
// it's OK to ignore attributes for our purposes and deal with
2435+
// the actual annotated statement.
2436+
if (const auto *AS = dyn_cast<AttributedStmt>(S))
2437+
S = AS->getSubStmt();
2438+
24302439
// For member expressions, return the location of the '.' or '->'.
24312440
if (const auto *ME = dyn_cast<MemberExpr>(S))
24322441
return PathDiagnosticLocation::createMemberLoc(ME, SM);
@@ -2899,6 +2908,10 @@ void BugReporter::emitReport(std::unique_ptr<BugReport> R) {
28992908
if (!ValidSourceLoc)
29002909
return;
29012910

2911+
// If the user asked to suppress this report, we should skip it.
2912+
if (UserSuppressions.isSuppressed(*R))
2913+
return;
2914+
29022915
// Compute the bug report's hash to determine its equivalence class.
29032916
llvm::FoldingSetNodeID ID;
29042917
R->Profile(ID);

0 commit comments

Comments
 (0)