Skip to content

Commit ef3f476

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
1 parent c76c00f commit ef3f476

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
@@ -2792,9 +2792,10 @@ def SwiftAsyncError : InheritableAttr {
27922792
let Documentation = [SwiftAsyncErrorDocs];
27932793
}
27942794

2795-
def Suppress : StmtAttr {
2796-
let Spellings = [CXX11<"gsl", "suppress">];
2795+
def Suppress : DeclOrStmtAttr {
2796+
let Spellings = [CXX11<"gsl", "suppress">, Clang<"suppress">];
27972797
let Args = [VariadicStringArgument<"DiagnosticIdentifiers">];
2798+
let Accessors = [Accessor<"isGSL", [CXX11<"gsl", "suppress">]>];
27982799
let Documentation = [SuppressDocs];
27992800
}
28002801

clang/include/clang/Basic/AttrDocs.td

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5267,7 +5267,74 @@ the ``int`` parameter is the one that represents the error.
52675267
def SuppressDocs : Documentation {
52685268
let Category = DocCatStmt;
52695269
let Content = [{
5270-
The ``[[gsl::suppress]]`` attribute suppresses specific
5270+
The ``suppress`` attribute suppresses unwanted warnings coming from static
5271+
analysis tools such as the Clang Static Analyzer. The tool will not report
5272+
any issues in source code annotated with the attribute.
5273+
5274+
The attribute cannot be used to suppress traditional Clang warnings, because
5275+
many such warnings are emitted before the attribute is fully parsed.
5276+
Consider using ``#pragma clang diagnostic`` to control such diagnostics,
5277+
as described in `Controlling Diagnostics via Pragmas
5278+
<https://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-via-pragmas>`_.
5279+
5280+
The ``suppress`` attribute can be placed on an individual statement in order to
5281+
suppress warnings about undesirable behavior occurring at that statement:
5282+
5283+
.. code-block:: c++
5284+
5285+
int foo() {
5286+
int *x = nullptr;
5287+
...
5288+
[[clang::suppress]]
5289+
return *x; // null pointer dereference warning suppressed here
5290+
}
5291+
5292+
Putting the attribute on a compound statement suppresses all warnings in scope:
5293+
5294+
.. code-block:: c++
5295+
5296+
int foo() {
5297+
[[clang::suppress]] {
5298+
int *x = nullptr;
5299+
...
5300+
return *x; // warnings suppressed in the entire scope
5301+
}
5302+
}
5303+
5304+
Some static analysis warnings are accompanied by one or more notes, and the
5305+
line of code against which the warning is emitted isn't necessarily the best
5306+
for suppression purposes. In such cases the tools are allowed to implement
5307+
additional ways to suppress specific warnings based on the attribute attached
5308+
to a note location.
5309+
5310+
For example, the Clang Static Analyzer suppresses memory leak warnings when
5311+
the suppression attribute is placed at the allocation site (highlited by
5312+
a "note: memory is allocated"), which may be different from the line of code
5313+
at which the program "loses track" of the pointer (where the warning
5314+
is ultimately emitted):
5315+
5316+
.. code-block:: c
5317+
5318+
int bar1(bool coin_flip) {
5319+
__attribute__((suppress))
5320+
int *result = (int *)malloc(sizeof(int));
5321+
if (coin_flip)
5322+
return 1; // warning about this leak path is suppressed
5323+
5324+
return *result; // warning about this leak path is also suppressed
5325+
}
5326+
5327+
int bar2(bool coin_flip) {
5328+
int *result = (int *)malloc(sizeof(int));
5329+
if (coin_flip)
5330+
return 1; // leak warning on this path NOT suppressed
5331+
5332+
__attribute__((suppress))
5333+
return *result; // leak warning is suppressed only on this path
5334+
}
5335+
5336+
5337+
When written as ``[[gsl::suppress]]``, this attribute suppresses specific
52715338
clang-tidy diagnostics for rules of the `C++ Core Guidelines`_ in a portable
52725339
way. The attribute can be attached to declarations, statements, and at
52735340
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"
@@ -594,6 +595,9 @@ class BugReporter {
594595
/// A vector of BugReports for tracking the allocated pointers and cleanup.
595596
std::vector<BugReportEquivClass *> EQClassesVector;
596597

598+
/// User-provided in-code suppressions.
599+
BugSuppression UserSuppressions;
600+
597601
public:
598602
BugReporter(BugReporterData &d);
599603
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
@@ -5225,8 +5225,16 @@ static void handleCallConvAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
52255225
}
52265226

52275227
static void handleSuppressAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
5228-
if (!AL.checkAtLeastNumArgs(S, 1))
5228+
if (AL.getAttributeSpellingListIndex() == SuppressAttr::CXX11_gsl_suppress) {
5229+
// Suppression attribute with GSL spelling requires at least 1 argument.
5230+
if (!AL.checkAtLeastNumArgs(S, 1))
5231+
return;
5232+
} else if (!isa<VarDecl>(D)) {
5233+
// Analyzer suppression applies only to variables and statements.
5234+
S.Diag(AL.getLoc(), diag::err_attribute_wrong_decl_type_str)
5235+
<< AL << 0 << "variables and statements";
52295236
return;
5237+
}
52305238

52315239
std::vector<StringRef> DiagnosticIdentifiers;
52325240
for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) {
@@ -5235,8 +5243,6 @@ static void handleSuppressAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
52355243
if (!S.checkStringLiteralArgumentAttr(AL, I, RuleName, nullptr))
52365244
return;
52375245

5238-
// FIXME: Warn if the rule name is unknown. This is tricky because only
5239-
// clang-tidy knows about available rules.
52405246
DiagnosticIdentifiers.push_back(RuleName);
52415247
}
52425248
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"
@@ -2424,6 +2427,12 @@ PathSensitiveBugReport::getLocation() const {
24242427
}
24252428

24262429
if (S) {
2430+
// Attributed statements usually have corrupted begin locations,
2431+
// it's OK to ignore attributes for our purposes and deal with
2432+
// the actual annotated statement.
2433+
if (const auto *AS = dyn_cast<AttributedStmt>(S))
2434+
S = AS->getSubStmt();
2435+
24272436
// For member expressions, return the location of the '.' or '->'.
24282437
if (const auto *ME = dyn_cast<MemberExpr>(S))
24292438
return PathDiagnosticLocation::createMemberLoc(ME, SM);
@@ -2896,6 +2905,10 @@ void BugReporter::emitReport(std::unique_ptr<BugReport> R) {
28962905
if (!ValidSourceLoc)
28972906
return;
28982907

2908+
// If the user asked to suppress this report, we should skip it.
2909+
if (UserSuppressions.isSuppressed(*R))
2910+
return;
2911+
28992912
// Compute the bug report's hash to determine its equivalence class.
29002913
llvm::FoldingSetNodeID ID;
29012914
R->Profile(ID);

0 commit comments

Comments
 (0)