vsavchenko updated this revision to Diff 367788.
vsavchenko added a comment.
Herald added a subscriber: manas.
Join 'suppress' and 'analyzer_suppress' attributes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93110/new/
https://reviews.llvm.org/D93110
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugSuppression.h
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaStmtAttr.cpp
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
clang/lib/StaticAnalyzer/Core/BugSuppression.cpp
clang/lib/StaticAnalyzer/Core/CMakeLists.txt
clang/test/Analysis/suppression-attr.m
clang/test/SemaCXX/attr-suppress.cpp
clang/test/SemaCXX/suppress.cpp
clang/test/SemaObjC/attr-suppress.m
Index: clang/test/SemaObjC/attr-suppress.m
===================================================================
--- /dev/null
+++ clang/test/SemaObjC/attr-suppress.m
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks %s -verify
+
+#define SUPPRESS1 __attribute__((suppress))
+#define SUPPRESS2(...) __attribute__((suppress(__VA_ARGS__)))
+
+SUPPRESS1 int global = 42;
+
+SUPPRESS1 void foo() {
+ // expected-error@-1 {{'suppress' attribute only applies to variables and statements}}
+ SUPPRESS1 int *p;
+
+ SUPPRESS1 int a = 0; // no-warning
+ SUPPRESS2()
+ int b = 1; // no-warning
+ SUPPRESS2("a")
+ int c = a + b; // no-warning
+ SUPPRESS2("a", "b") { b = c - a; } // no-warning
+
+ SUPPRESS2("a", "b")
+ if (b == 10)
+ a += 4; // no-warning
+ SUPPRESS1 while (1) {} // no-warning
+ SUPPRESS1 switch (a) { // no-warning
+ default:
+ c -= 10;
+ }
+
+ // GNU-style attributes and C++11 attributes apply to different things when
+ // written like this. GNU attribute gets attached to the declaration, while
+ // C++11 attribute ends up on the type.
+ int SUPPRESS2("r") z;
+ SUPPRESS2(foo)
+ float f;
+ // expected-error@-2 {{'suppress' attribute requires a string}}
+}
+
+union SUPPRESS2("type.1") U {
+ // expected-error@-1 {{'suppress' attribute only applies to variables and statements}}
+ int i;
+ float f;
+};
+
+SUPPRESS1 @interface Test {
+ // expected-error@-1 {{'suppress' attribute only applies to variables and statements}}
+}
+@property SUPPRESS2("prop") int *prop;
+// expected-error@-1 {{'suppress' attribute only applies to variables and statements}}
+- (void)bar:(int)x SUPPRESS1;
+// expected-error@-1 {{'suppress' attribute only applies to variables and statements}}
+@end
Index: clang/test/SemaCXX/suppress.cpp
===================================================================
--- clang/test/SemaCXX/suppress.cpp
+++ /dev/null
@@ -1,25 +0,0 @@
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only %s -verify
-
-[[gsl::suppress("globally")]];
-
-namespace N {
- [[gsl::suppress("in-a-namespace")]];
-}
-
-[[gsl::suppress("readability-identifier-naming")]]
-void f_() {
- int *p;
- [[gsl::suppress("type", "bounds")]] {
- p = reinterpret_cast<int *>(7);
- }
-
- [[gsl::suppress]] int x; // expected-error {{'suppress' attribute takes at least 1 argument}}
- [[gsl::suppress()]] int y; // expected-error {{'suppress' attribute takes at least 1 argument}}
- int [[gsl::suppress("r")]] z; // expected-error {{'suppress' attribute cannot be applied to types}}
- [[gsl::suppress(f_)]] float f; // expected-error {{'suppress' attribute requires a string}}
-}
-
-union [[gsl::suppress("type.1")]] U {
- int i;
- float f;
-};
Index: clang/test/SemaCXX/attr-suppress.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/attr-suppress.cpp
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only %s -verify
+
+[[gsl::suppress("globally")]];
+
+namespace N {
+[[gsl::suppress("in-a-namespace")]];
+}
+
+[[gsl::suppress("readability-identifier-naming")]] void f_() {
+ int *p;
+ [[gsl::suppress("type", "bounds")]] {
+ p = reinterpret_cast<int *>(7);
+ }
+
+ [[gsl::suppress]] int x; // expected-error {{'suppress' attribute takes at least 1 argument}}
+ [[gsl::suppress()]] int y; // expected-error {{'suppress' attribute takes at least 1 argument}}
+ int [[gsl::suppress("r")]] z; // expected-error {{'suppress' attribute cannot be applied to types}}
+ [[gsl::suppress(f_)]] float f; // expected-error {{'suppress' attribute requires a string}}
+}
+
+union [[gsl::suppress("type.1")]] U {
+ int i;
+ float f;
+};
+
+[[clang::suppress]];
+// expected-error@-1 {{'suppress' attribute only applies to variables and statements}}
+
+namespace N {
+[[clang::suppress("in-a-namespace")]];
+// expected-error@-1 {{'suppress' attribute only applies to variables and statements}}
+} // namespace N
+
+[[clang::suppress]] int global = 42;
+
+[[clang::suppress]] void foo() {
+ // expected-error@-1 {{'suppress' attribute only applies to variables and statements}}
+ [[clang::suppress]] int *p;
+
+ [[clang::suppress]] int a = 0; // no-warning
+ [[clang::suppress()]] int b = 1; // no-warning
+ [[clang::suppress("a")]] int c = a + b; // no-warning
+ [[clang::suppress("a", "b")]] b = c - a; // no-warning
+
+ [[clang::suppress("a", "b")]] if (b == 10) a += 4; // no-warning
+ [[clang::suppress]] while (true) {} // no-warning
+ [[clang::suppress]] switch (a) { // no-warning
+ default:
+ c -= 10;
+ }
+
+ int [[clang::suppress("r")]] z;
+ // expected-error@-1 {{'suppress' attribute cannot be applied to types}}
+ [[clang::suppress(foo)]] float f;
+ // expected-error@-1 {{'suppress' attribute requires a string}}
+}
+
+class [[clang::suppress("type.1")]] V {
+ // expected-error@-1 {{'suppress' attribute only applies to variables and statements}}
+ int i;
+ float f;
+};
Index: clang/test/Analysis/suppression-attr.m
===================================================================
--- /dev/null
+++ clang/test/Analysis/suppression-attr.m
@@ -0,0 +1,235 @@
+// RUN: %clang_analyze_cc1 -fblocks \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=osx.cocoa.MissingSuperCall \
+// RUN: -analyzer-checker=osx.cocoa.NSError \
+// RUN: -analyzer-checker=osx.ObjCProperty \
+// RUN: -analyzer-checker=osx.cocoa.RetainCount \
+// RUN: -analyzer-checker=unix.Malloc \
+// RUN: -analyzer-checker=alpha.core.CastToStruct \
+// RUN: -Wno-unused-value -Wno-objc-root-class -verify %s
+
+#define SUPPRESS __attribute__((suppress))
+
+@protocol NSObject
+- (id)retain;
+- (oneway void)release;
+@end
+@interface NSObject <NSObject> {
+}
+- (id)init;
++ (id)alloc;
+@end
+typedef int NSInteger;
+typedef char BOOL;
+typedef struct _NSZone NSZone;
+@class NSInvocation, NSMethodSignature, NSCoder, NSString, NSEnumerator;
+@protocol NSCopying
+- (id)copyWithZone:(NSZone *)zone;
+@end
+@protocol NSCoding
+- (void)encodeWithCoder:(NSCoder *)aCoder;
+@end
+@class NSDictionary;
+@interface NSError : NSObject <NSCopying, NSCoding> {
+}
++ (id)errorWithDomain:(NSString *)domain code:(NSInteger)code userInfo:(NSDictionary *)dict;
+@end
+
+@interface NSMutableString : NSObject
+@end
+
+typedef __typeof__(sizeof(int)) size_t;
+void *malloc(size_t);
+void free(void *);
+
+void dereference_1() {
+ int *x = 0;
+ *x; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+}
+
+void dereference_suppression_1() {
+ int *x = 0;
+ SUPPRESS { *x; } // no-warning
+}
+
+void dereference_2() {
+ int *x = 0;
+ if (*x) { // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+ }
+}
+
+void dereference_suppression_2() {
+ int *x = 0;
+ SUPPRESS if (*x) { // no-warning
+ }
+}
+
+void dereference_3(int cond) {
+ int *x = 0;
+ if (cond) {
+ (*x)++; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+ }
+}
+
+void dereference_suppression_3(int cond) {
+ int *x = 0;
+ SUPPRESS if (cond) {
+ (*x)++; // no-warning
+ }
+}
+
+void dereference_4() {
+ int *x = 0;
+ int y = *x; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+}
+
+void dereference_suppression_4() {
+ int *x = 0;
+ SUPPRESS int y = *x; // no-warning
+}
+
+void dereference_5() {
+ int *x = 0;
+ int y = *x; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+ int z = *x; // no-warning (duplicate)
+}
+
+void dereference_suppression_5_1() {
+ int *x = 0;
+ SUPPRESS int y = *x; // no-warning
+ int z = *x; // no-warning (duplicate)
+}
+
+void dereference_suppression_5_2() {
+ int *x = 0;
+ int y = *x; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+ SUPPRESS int z = *x; // no-warning
+}
+
+int malloc_leak_1() {
+ int *x = (int *)malloc(sizeof(int));
+ *x = 42;
+ return *x; // expected-warning{{Potential leak of memory pointed to by 'x'}}
+}
+
+int malloc_leak_suppression_1_1() {
+ SUPPRESS int *x = (int *)malloc(sizeof(int));
+ *x = 42;
+ return *x;
+}
+
+int malloc_leak_suppression_1_2() {
+ int *x = (int *)malloc(sizeof(int));
+ *x = 42;
+ SUPPRESS return *x;
+}
+
+void malloc_leak_2() {
+ int *x = (int *)malloc(sizeof(int));
+ *x = 42;
+} // expected-warning{{Potential leak of memory pointed to by 'x'}}
+
+void malloc_leak_suppression_2_1() {
+ SUPPRESS int *x = (int *)malloc(sizeof(int));
+ *x = 42;
+}
+
+// TODO: reassess when we decide what to do with declaration annotations
+void malloc_leak_suppression_2_2() /* SUPPRESS */ {
+ int *x = (int *)malloc(sizeof(int));
+ *x = 42;
+} // expected-warning{{Potential leak of memory pointed to by 'x'}}
+
+// TODO: reassess when we decide what to do with declaration annotations
+/* SUPPRESS */ void malloc_leak_suppression_2_3() {
+ int *x = (int *)malloc(sizeof(int));
+ *x = 42;
+} // expected-warning{{Potential leak of memory pointed to by 'x'}}
+
+void malloc_leak_suppression_2_4(int cond) {
+ int *x = (int *)malloc(sizeof(int));
+ *x = 42;
+ SUPPRESS;
+}
+
+void retain_release_leak_1() {
+ [[NSMutableString alloc] init]; // expected-warning{{Potential leak of an object of type 'NSMutableString *'}}
+}
+
+void retain_release_leak_suppression_1() {
+ SUPPRESS { [[NSMutableString alloc] init]; }
+}
+
+void retain_release_leak_2(int cond) {
+ id obj = [[NSMutableString alloc] init]; // expected-warning{{Potential leak of an object stored into 'obj'}}
+ if (cond) {
+ [obj release];
+ }
+}
+
+void retain_release_leak__suppression_2(int cond) {
+ SUPPRESS id obj = [[NSMutableString alloc] init];
+ if (cond) {
+ [obj release];
+ }
+}
+
+@interface UIResponder : NSObject {
+}
+- (char)resignFirstResponder;
+@end
+
+@interface Test : UIResponder {
+}
+@property(copy) NSMutableString *mutableStr;
+// expected-warning@-1 {{Property of mutable type 'NSMutableString' has 'copy' attribute; an immutable object will be stored instead}}
+@end
+@implementation Test
+
+- (BOOL)resignFirstResponder {
+ return 0;
+} // expected-warning {{The 'resignFirstResponder' instance method in UIResponder subclass 'Test' is missing a [super resignFirstResponder] call}}
+
+- (void)methodWhichMayFail:(NSError **)error {
+ // expected-warning@-1 {{Method accepting NSError** should have a non-void return value to indicate whether or not an error occurred}}
+}
+@end
+
+@interface TestSuppress : UIResponder {
+}
+// TODO: reassess when we decide what to do with declaration annotations
+@property(copy) /* SUPPRESS */ NSMutableString *mutableStr;
+// expected-warning@-1 {{Property of mutable type 'NSMutableString' has 'copy' attribute; an immutable object will be stored instead}}
+@end
+@implementation TestSuppress
+
+// TODO: reassess when we decide what to do with declaration annotations
+- (BOOL)resignFirstResponder /* SUPPRESS */ {
+ return 0;
+} // expected-warning {{The 'resignFirstResponder' instance method in UIResponder subclass 'TestSuppress' is missing a [super resignFirstResponder] call}}
+
+// TODO: reassess when we decide what to do with declaration annotations
+- (void)methodWhichMayFail:(NSError **)error /* SUPPRESS */ {
+ // expected-warning@-1 {{Method accepting NSError** should have a non-void return value to indicate whether or not an error occurred}}
+}
+@end
+
+struct AB {
+ int A, B;
+};
+
+struct ABC {
+ int A, B, C;
+};
+
+void ast_checker_1() {
+ struct AB Ab;
+ struct ABC *Abc;
+ Abc = (struct ABC *)&Ab; // expected-warning {{Casting data to a larger structure type and accessing a field can lead to memory access errors or data corruption}}
+}
+
+void ast_checker_suppress_1() {
+ struct AB Ab;
+ struct ABC *Abc;
+ SUPPRESS { Abc = (struct ABC *)&Ab; }
+}
Index: clang/lib/StaticAnalyzer/Core/CMakeLists.txt
===================================================================
--- clang/lib/StaticAnalyzer/Core/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/Core/CMakeLists.txt
@@ -11,6 +11,7 @@
BlockCounter.cpp
BugReporter.cpp
BugReporterVisitors.cpp
+ BugSuppression.cpp
CallEvent.cpp
Checker.cpp
CheckerContext.cpp
Index: clang/lib/StaticAnalyzer/Core/BugSuppression.cpp
===================================================================
--- /dev/null
+++ clang/lib/StaticAnalyzer/Core/BugSuppression.cpp
@@ -0,0 +1,162 @@
+//===- BugSuppression.cpp - Suppression interface -------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/StaticAnalyzer/Core/BugReporter/BugSuppression.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+using Ranges = llvm::SmallVectorImpl<SourceRange>;
+
+inline bool hasSuppression(const Decl *D) {
+ if (const auto *Suppression = D->getAttr<SuppressAttr>())
+ return !Suppression->isGSL();
+ return false;
+}
+inline bool hasSuppression(const AttributedStmt *S) {
+ return llvm::any_of(S->getAttrs(), [](const Attr *A) {
+ const auto *Suppression = dyn_cast<SuppressAttr>(A);
+ return Suppression && !Suppression->isGSL();
+ });
+}
+
+template <class NodeType> inline SourceRange getRange(const NodeType *Node) {
+ return Node->getSourceRange();
+}
+template <> inline SourceRange getRange(const AttributedStmt *S) {
+ // Begin location for attributed statement node seems to be ALWAYS invalid.
+ //
+ // It is unlikely that we ever report any warnings on suppression
+ // attribute itself, but even if we do, we wouldn't want that warning
+ // to be suppressed by that same attribute.
+ //
+ // Long story short, we can use inner statement and it's not going to break
+ // anything.
+ return getRange(S->getSubStmt());
+}
+
+inline bool isLessOrEqual(SourceLocation LHS, SourceLocation RHS,
+ const SourceManager &SM) {
+ // SourceManager::isBeforeInTranslationUnit tests for strict
+ // inequality, when we need a non-strict comparison (bug
+ // can be reported directly on the annotated note).
+ // For this reason, we use the following equivalence:
+ //
+ // A <= B <==> !(B < A)
+ //
+ return !SM.isBeforeInTranslationUnit(RHS, LHS);
+}
+
+inline bool fullyContains(SourceRange Larger, SourceRange Smaller,
+ const SourceManager &SM) {
+ // Essentially this means:
+ //
+ // Larger.fullyContains(Smaller)
+ //
+ // However, that method has a very trivial implementation and couldn't
+ // compare regular locations and locations from macro expansions.
+ // We could've converted everything into regular locations as a solution,
+ // but the following solution seems to be the most bulletproof.
+ return isLessOrEqual(Larger.getBegin(), Smaller.getBegin(), SM) &&
+ isLessOrEqual(Smaller.getEnd(), Larger.getEnd(), SM);
+}
+
+class CacheInitializer : public RecursiveASTVisitor<CacheInitializer> {
+public:
+ static void initialize(const Decl *D, Ranges &ToInit) {
+ CacheInitializer(ToInit).TraverseDecl(const_cast<Decl *>(D));
+ }
+
+ bool VisitVarDecl(VarDecl *VD) {
+ // Bug location could be somewhere in the init value of
+ // a freshly declared variable. Even though it looks like the
+ // user applied attribute to a statement, it will apply to a
+ // variable declaration, and this is where we check for it.
+ return VisitAttributedNode(VD);
+ }
+
+ bool VisitAttributedStmt(AttributedStmt *AS) {
+ // When we apply attributes to statements, it actually creates
+ // a wrapper statement that only contains attributes and the wrapped
+ // statement.
+ return VisitAttributedNode(AS);
+ }
+
+private:
+ template <class NodeType> bool VisitAttributedNode(NodeType *Node) {
+ if (hasSuppression(Node)) {
+ // TODO: In the future, when we come up with good stable IDs for checkers
+ // we can return a list of kinds to ignore, or all if no arguments
+ // were provided.
+ addRange(getRange(Node));
+ }
+ // We should keep traversing AST.
+ return true;
+ }
+
+ void addRange(SourceRange R) {
+ if (R.isValid()) {
+ Result.push_back(R);
+ }
+ }
+
+ CacheInitializer(Ranges &R) : Result(R) {}
+ Ranges &Result;
+};
+
+} // end anonymous namespace
+
+// TODO: Introduce stable IDs for checkers and check for those here
+// to be more specific. Attribute without arguments should still
+// be considered as "suppress all".
+// It is already much finer granularity than what we have now
+// (i.e. removing the whole function from the analysis).
+bool BugSuppression::isSuppressed(const BugReport &R) {
+ PathDiagnosticLocation Location = R.getLocation();
+ PathDiagnosticLocation UniqueingLocation = R.getUniqueingLocation();
+ const Decl *DeclWithIssue = R.getDeclWithIssue();
+
+ return isSuppressed(Location, DeclWithIssue) ||
+ isSuppressed(UniqueingLocation, DeclWithIssue);
+}
+
+bool BugSuppression::isSuppressed(const PathDiagnosticLocation &Location,
+ const Decl *DeclWithIssue) {
+ if (!Location.isValid() || DeclWithIssue == nullptr)
+ return false;
+
+ // While some warnings are attached to AST nodes (mostly path-sensitive
+ // checks), others are simply associated with a plain source location
+ // or range. Figuring out the node based on locations can be tricky,
+ // so instead, we traverse the whole body of the declaration and gather
+ // information on ALL suppressions. After that we can simply check if
+ // any of those suppressions affect the warning in question.
+ //
+ // Traversing AST of a function is not a heavy operation, but for
+ // large functions with a lot of bugs it can make a dent in performance.
+ // In order to avoid this scenario, we cache traversal results.
+ auto InsertionResult = CachedSuppressionLocations.insert(
+ std::make_pair(DeclWithIssue, CachedRanges{}));
+ Ranges &SuppressionRanges = InsertionResult.first->second;
+ if (InsertionResult.second) {
+ // We haven't checked this declaration for suppressions yet!
+ CacheInitializer::initialize(DeclWithIssue, SuppressionRanges);
+ }
+
+ SourceRange BugRange = Location.asRange();
+ const SourceManager &SM = Location.getManager();
+
+ return llvm::any_of(SuppressionRanges,
+ [BugRange, &SM](SourceRange Suppression) {
+ return fullyContains(Suppression, BugRange, SM);
+ });
+}
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -12,12 +12,15 @@
//===----------------------------------------------------------------------===//
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Attr.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclBase.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/ParentMap.h"
+#include "clang/AST/ParentMapContext.h"
#include "clang/AST/Stmt.h"
#include "clang/AST/StmtCXX.h"
#include "clang/AST/StmtObjC.h"
@@ -2412,11 +2415,10 @@
return Ranges;
}
-PathDiagnosticLocation
-PathSensitiveBugReport::getLocation() const {
+PathDiagnosticLocation PathSensitiveBugReport::getLocation() const {
assert(ErrorNode && "Cannot create a location with a null node.");
const Stmt *S = ErrorNode->getStmtForDiagnostics();
- ProgramPoint P = ErrorNode->getLocation();
+ ProgramPoint P = ErrorNode->getLocation();
const LocationContext *LC = P.getLocationContext();
SourceManager &SM =
ErrorNode->getState()->getStateManager().getContext().getSourceManager();
@@ -2433,6 +2435,12 @@
}
if (S) {
+ // Attributed statements usually have corrupted begin locations,
+ // it's OK to ignore attributes for our purposes and deal with
+ // the actual annotated statement.
+ if (const auto *AS = dyn_cast<AttributedStmt>(S))
+ S = AS->getSubStmt();
+
// For member expressions, return the location of the '.' or '->'.
if (const auto *ME = dyn_cast<MemberExpr>(S))
return PathDiagnosticLocation::createMemberLoc(ME, SM);
@@ -2906,13 +2914,17 @@
if (!ValidSourceLoc)
return;
+ // If the user asked to suppress this report, we should skip it.
+ if (UserSuppressions.isSuppressed(*R))
+ return;
+
// Compute the bug report's hash to determine its equivalence class.
llvm::FoldingSetNodeID ID;
R->Profile(ID);
// Lookup the equivance class. If there isn't one, create it.
void *InsertPos;
- BugReportEquivClass* EQ = EQClasses.FindNodeOrInsertPos(ID, InsertPos);
+ BugReportEquivClass *EQ = EQClasses.FindNodeOrInsertPos(ID, InsertPos);
if (!EQ) {
EQ = new BugReportEquivClass(std::move(R));
Index: clang/lib/Sema/SemaStmtAttr.cpp
===================================================================
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -52,6 +52,12 @@
static Attr *handleSuppressAttr(Sema &S, Stmt *St, const ParsedAttr &A,
SourceRange Range) {
+ if (A.getAttributeSpellingListIndex() == 0 && A.getNumArgs() < 1) {
+ // Suppression attribute with GSL spelling requires at least 1 argument.
+ S.Diag(A.getLoc(), diag::err_attribute_too_few_arguments) << A << 1;
+ return nullptr;
+ }
+
std::vector<StringRef> DiagnosticIdentifiers;
for (unsigned I = 0, E = A.getNumArgs(); I != E; ++I) {
StringRef RuleName;
@@ -59,8 +65,6 @@
if (!S.checkStringLiteralArgumentAttr(A, I, RuleName, nullptr))
return nullptr;
- // FIXME: Warn if the rule name is unknown. This is tricky because only
- // clang-tidy knows about available rules.
DiagnosticIdentifiers.push_back(RuleName);
}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4705,8 +4705,16 @@
}
static void handleSuppressAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
- if (!AL.checkAtLeastNumArgs(S, 1))
+ if (AL.getAttributeSpellingListIndex() == 0) {
+ // Suppression attribute with GSL spelling requires at least 1 argument.
+ if (!AL.checkAtLeastNumArgs(S, 1))
+ return;
+ } else if (!isa<VarDecl>(D)) {
+ // Analyzer suppression applies only to variables and statements.
+ S.Diag(AL.getLoc(), diag::err_attribute_wrong_decl_type_str)
+ << AL << "variables and statements";
return;
+ }
std::vector<StringRef> DiagnosticIdentifiers;
for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) {
@@ -4715,8 +4723,6 @@
if (!S.checkStringLiteralArgumentAttr(AL, I, RuleName, nullptr))
return;
- // FIXME: Warn if the rule name is unknown. This is tricky because only
- // clang-tidy knows about available rules.
DiagnosticIdentifiers.push_back(RuleName);
}
D->addAttr(::new (S.Context)
Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugSuppression.h
===================================================================
--- /dev/null
+++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugSuppression.h
@@ -0,0 +1,49 @@
+//===- BugSuppression.h - Suppression interface -----------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines BugSuppression, a simple interface class incapsulating
+// all user provided in-code suppressions.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_SUPPTRESSION_H
+#define LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_SUPPTRESSION_H
+
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallVector.h"
+
+namespace clang {
+class Decl;
+
+namespace ento {
+class BugReport;
+class PathDiagnosticLocation;
+
+class BugSuppression {
+public:
+ /// Return true if the given bug report was explicitly suppressed by the user.
+ bool isSuppressed(const BugReport &);
+ /// Return true if the bug reported at the given location was explicitly
+ /// suppressed by the user.
+ bool isSuppressed(const PathDiagnosticLocation &Location,
+ const Decl *DeclWithIssue);
+
+private:
+ // Overly pessimistic number, to be honest.
+ static constexpr unsigned EXPECTED_NUMBER_OF_SUPPRESSIONS = 8;
+ using CachedRanges =
+ llvm::SmallVector<SourceRange, EXPECTED_NUMBER_OF_SUPPRESSIONS>;
+
+ llvm::DenseMap<const Decl *, CachedRanges> CachedSuppressionLocations;
+};
+
+} // end namespace ento
+} // end namespace clang
+
+#endif // LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_SUPPTRESSION_H
Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -19,6 +19,7 @@
#include "clang/Basic/SourceLocation.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugSuppression.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
@@ -597,6 +598,9 @@
/// A vector of BugReports for tracking the allocated pointers and cleanup.
std::vector<BugReportEquivClass *> EQClassesVector;
+ /// User-provided in-code suppressions.
+ BugSuppression UserSuppressions;
+
public:
BugReporter(BugReporterData &d);
virtual ~BugReporter();
Index: clang/include/clang/Basic/AttrDocs.td
===================================================================
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -4743,7 +4743,37 @@
def SuppressDocs : Documentation {
let Category = DocCatStmt;
let Content = [{
-The ``[[gsl::suppress]]`` attribute suppresses specific
+The ``suppress`` attribute can be applied to variable declarations and statements
+to suppess warnings from the Clang Static Analyzer. The analyzer will not report
+any issues on the annotated constructs.
+
+.. code-block:: c++
+
+ void foo() {
+ int *x = nullptr;
+ ...
+ [[clang::analyzer_suppress]] int y = *x; // no warning from the analyzer
+ ...
+ }
+
+For leaks, suppressions can work both when placed on the statement where the
+issue is reported and on the allocation site.
+
+.. code-block:: c
+
+ int bar1() {
+ __attribute__((analyzer_suppress)) int *result = (int *)malloc(sizeof(int));
+ ...
+ return *result; // no leak warning
+ }
+
+ int bar2() {
+ int *result = (int *)malloc(sizeof(int));
+ ...
+ __attribute__((analyzer_suppress)) return *result; // no leak warning
+ }
+
+When written as ``[[gsl::suppress]]``, this attribute suppresses specific
clang-tidy diagnostics for rules of the `C++ Core Guidelines`_ in a portable
way. The attribute can be attached to declarations, statements, and at
namespace scope.
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -2518,9 +2518,10 @@
let Documentation = [SwiftAsyncErrorDocs];
}
-def Suppress : StmtAttr {
- let Spellings = [CXX11<"gsl", "suppress">];
+def Suppress : DeclOrStmtAttr {
+ let Spellings = [CXX11<"gsl", "suppress">, Clang<"suppress">];
let Args = [VariadicStringArgument<"DiagnosticIdentifiers">];
+ let Accessors = [Accessor<"isGSL", [CXX11<"gsl", "suppress">]>];
let Documentation = [SuppressDocs];
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
