diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -2792,9 +2792,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]; } diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -5267,7 +5267,74 @@ def SuppressDocs : Documentation { let Category = DocCatStmt; let Content = [{ -The ``[[gsl::suppress]]`` attribute suppresses specific +The ``suppress`` attribute suppresses unwanted warnings coming from static +analysis tools such as the Clang Static Analyzer. The tool will not report +any issues in source code annotated with the attribute. + +The attribute cannot be used to suppress traditional Clang warnings, because +many such warnings are emitted before the attribute is fully parsed. +Consider using ``#pragma clang diagnostic`` to control such diagnostics, +as described in `Controlling Diagnostics via Pragmas +`_. + +The ``suppress`` attribute can be placed on an individual statement in order to +suppress warnings about undesirable behavior occurring at that statement: + +.. code-block:: c++ + + int foo() { + int *x = nullptr; + ... + [[clang::suppress]] + return *x; // null pointer dereference warning suppressed here + } + +Putting the attribute on a compound statement suppresses all warnings in scope: + +.. code-block:: c++ + + int foo() { + [[clang::suppress]] { + int *x = nullptr; + ... + return *x; // warnings suppressed in the entire scope + } + } + +Some static analysis warnings are accompanied by one or more notes, and the +line of code against which the warning is emitted isn't necessarily the best +for suppression purposes. In such cases the tools are allowed to implement +additional ways to suppress specific warnings based on the attribute attached +to a note location. + +For example, the Clang Static Analyzer suppresses memory leak warnings when +the suppression attribute is placed at the allocation site (highlited by +a "note: memory is allocated"), which may be different from the line of code +at which the program "loses track" of the pointer (where the warning +is ultimately emitted): + +.. code-block:: c + + int bar1(bool coin_flip) { + __attribute__((suppress)) + int *result = (int *)malloc(sizeof(int)); + if (coin_flip) + return 1; // warning about this leak path is suppressed + + return *result; // warning about this leak path is also suppressed + } + + int bar2(bool coin_flip) { + int *result = (int *)malloc(sizeof(int)); + if (coin_flip) + return 1; // leak warning on this path NOT suppressed + + __attribute__((suppress)) + return *result; // leak warning is suppressed only on this path + } + + +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. diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h --- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h +++ b/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" @@ -594,6 +595,9 @@ /// A vector of BugReports for tracking the allocated pointers and cleanup. std::vector EQClassesVector; + /// User-provided in-code suppressions. + BugSuppression UserSuppressions; + public: BugReporter(BugReporterData &d); virtual ~BugReporter(); diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugSuppression.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugSuppression.h new file mode 100644 --- /dev/null +++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugSuppression.h @@ -0,0 +1,53 @@ +//===- 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 encapsulating +// all user provided in-code suppressions. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_SUPPRESSION_H +#define LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_SUPPRESSION_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: + using DiagnosticIdentifierList = llvm::ArrayRef; + + /// 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, + DiagnosticIdentifierList DiagnosticIdentification); + +private: + // Overly pessimistic number, to be honest. + static constexpr unsigned EXPECTED_NUMBER_OF_SUPPRESSIONS = 8; + using CachedRanges = + llvm::SmallVector; + + llvm::DenseMap CachedSuppressionLocations; +}; + +} // end namespace ento +} // end namespace clang + +#endif // LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_SUPPRESSION_H diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -5225,8 +5225,16 @@ } static void handleSuppressAttr(Sema &S, Decl *D, const ParsedAttr &AL) { - if (!AL.checkAtLeastNumArgs(S, 1)) + if (AL.getAttributeSpellingListIndex() == SuppressAttr::CXX11_gsl_suppress) { + // Suppression attribute with GSL spelling requires at least 1 argument. + if (!AL.checkAtLeastNumArgs(S, 1)) + return; + } else if (!isa(D)) { + // Analyzer suppression applies only to variables and statements. + S.Diag(AL.getLoc(), diag::err_attribute_wrong_decl_type_str) + << AL << 0 << "variables and statements"; return; + } std::vector DiagnosticIdentifiers; for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) { @@ -5235,8 +5243,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) diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp --- a/clang/lib/Sema/SemaStmtAttr.cpp +++ b/clang/lib/Sema/SemaStmtAttr.cpp @@ -53,6 +53,13 @@ static Attr *handleSuppressAttr(Sema &S, Stmt *St, const ParsedAttr &A, SourceRange Range) { + if (A.getAttributeSpellingListIndex() == SuppressAttr::CXX11_gsl_suppress && + 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 DiagnosticIdentifiers; for (unsigned I = 0, E = A.getNumArgs(); I != E; ++I) { StringRef RuleName; @@ -60,8 +67,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); } diff --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp --- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/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" @@ -2424,6 +2427,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(S)) + S = AS->getSubStmt(); + // For member expressions, return the location of the '.' or '->'. if (const auto *ME = dyn_cast(S)) return PathDiagnosticLocation::createMemberLoc(ME, SM); @@ -2896,6 +2905,10 @@ 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); diff --git a/clang/lib/StaticAnalyzer/Core/BugSuppression.cpp b/clang/lib/StaticAnalyzer/Core/BugSuppression.cpp new file mode 100644 --- /dev/null +++ b/clang/lib/StaticAnalyzer/Core/BugSuppression.cpp @@ -0,0 +1,169 @@ +//===- 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; + +inline bool hasSuppression(const Decl *D) { + // FIXME: Implement diagnostic identifier arguments + // (checker names, "hashtags"). + if (const auto *Suppression = D->getAttr()) + return !Suppression->isGSL() && + (Suppression->diagnosticIdentifiers().empty()); + return false; +} +inline bool hasSuppression(const AttributedStmt *S) { + // FIXME: Implement diagnostic identifier arguments + // (checker names, "hashtags"). + return llvm::any_of(S->getAttrs(), [](const Attr *A) { + const auto *Suppression = dyn_cast(A); + return Suppression && !Suppression->isGSL() && + (Suppression->diagnosticIdentifiers().empty()); + }); +} + +template 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 { +public: + static void initialize(const Decl *D, Ranges &ToInit) { + CacheInitializer(ToInit).TraverseDecl(const_cast(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 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, + DiagnosticIdentifierList Hashtags) { + 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); + }); +} diff --git a/clang/lib/StaticAnalyzer/Core/CMakeLists.txt b/clang/lib/StaticAnalyzer/Core/CMakeLists.txt --- a/clang/lib/StaticAnalyzer/Core/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Core/CMakeLists.txt @@ -11,6 +11,7 @@ BlockCounter.cpp BugReporter.cpp BugReporterVisitors.cpp + BugSuppression.cpp CallDescription.cpp CallEvent.cpp Checker.cpp diff --git a/clang/test/Analysis/suppression-attr-doc.cpp b/clang/test/Analysis/suppression-attr-doc.cpp new file mode 100644 --- /dev/null +++ b/clang/test/Analysis/suppression-attr-doc.cpp @@ -0,0 +1,54 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix \ +// RUN: -analyzer-disable-checker=core.uninitialized \ +// RUN: -verify %s + +// NOTE: These tests correspond to examples provided in documentation +// of [[clang::suppress]]. If you break them intentionally, it's likely that +// you need to update the documentation! + +typedef __typeof(sizeof(int)) size_t; +void *malloc(size_t); + +int foo_initial() { + int *x = nullptr; + return *x; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}} +} + +int foo1() { + int *x = nullptr; + [[clang::suppress]] + return *x; // null pointer dereference warning suppressed here +} + +int foo2() { + [[clang::suppress]] { + int *x = nullptr; + return *x; // null pointer dereference warning suppressed here + } +} + +int bar_initial(bool coin_flip) { + int *result = (int *)malloc(sizeof(int)); + if (coin_flip) + return 1; // There's no warning here YET, but it will show up if the other one is suppressed. + + return *result; // expected-warning{{Potential leak of memory pointed to by 'result'}} +} + +int bar1(bool coin_flip) { + __attribute__((suppress)) + int *result = (int *)malloc(sizeof(int)); + if (coin_flip) + return 1; // warning about this leak path is suppressed + + return *result; // warning about this leak path also suppressed +} + +int bar2(bool coin_flip) { + int *result = (int *)malloc(sizeof(int)); + if (coin_flip) + return 1; // expected-warning{{Potential leak of memory pointed to by 'result'}} + + __attribute__((suppress)) + return *result; // leak warning is suppressed only on this path +} diff --git a/clang/test/Analysis/suppression-attr.m b/clang/test/Analysis/suppression-attr.m new file mode 100644 --- /dev/null +++ b/clang/test/Analysis/suppression-attr.m @@ -0,0 +1,271 @@ +// 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)) +#define SUPPRESS_SPECIFIC(...) __attribute__((suppress(__VA_ARGS__))) + +@protocol NSObject +- (id)retain; +- (oneway void)release; +@end +@interface 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 { +} ++ (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_suppression_2a() { + int *x = 0; + // FIXME: Implement suppressing individual checkers. + SUPPRESS_SPECIFIC("core.NullDereference") if (*x) { // expected-warning{{Dereference of null pointer (loaded from variable 'x')}} + } +} + +void dereference_suppression_2b() { + int *x = 0; + // This is not a MallocChecker issue so it shouldn't be suppressed. (Though the attribute + // doesn't really understand any of those arguments yet.) + SUPPRESS_SPECIFIC("unix.Malloc") if (*x) { // expected-warning{{Dereference of null pointer (loaded from variable 'x')}} + } +} + +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 +} + +void do_deref(int *y) { + *y = 1; // expected-warning{{Dereference of null pointer (loaded from variable 'y')}} +} + +void dereference_interprocedural() { + int *x = 0; + do_deref(x); +} + +void do_deref_suppressed(int *y) { + SUPPRESS *y = 1; // no-warning +} + +void dereference_interprocedural_suppressed() { + int *x = 0; + do_deref_suppressed(x); +} + +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; + // FIXME: The warning should be suppressed but dead symbol elimination + // happens too late. +} // expected-warning{{Potential leak of memory pointed to by 'x'}} + +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; } +} diff --git a/clang/test/SemaCXX/attr-suppress.cpp b/clang/test/SemaCXX/attr-suppress.cpp new file mode 100644 --- /dev/null +++ b/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(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 {{expected string literal as argument of 'suppress' attribute}} +} + +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 {{expected string literal as argument of 'suppress' attribute}} +} + +class [[clang::suppress("type.1")]] V { + // expected-error@-1 {{'suppress' attribute only applies to variables and statements}} + int i; + float f; +}; diff --git a/clang/test/SemaCXX/suppress.cpp b/clang/test/SemaCXX/suppress.cpp deleted file mode 100644 --- a/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(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 {{expected string literal as argument of 'suppress' attribut}} -} - -union [[gsl::suppress("type.1")]] U { - int i; - float f; -}; diff --git a/clang/test/SemaObjC/attr-suppress.m b/clang/test/SemaObjC/attr-suppress.m new file mode 100644 --- /dev/null +++ b/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 {{expected string literal as argument of 'suppress' attribute}} +} + +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 diff --git a/clang/www/analyzer/faq.html b/clang/www/analyzer/faq.html --- a/clang/www/analyzer/faq.html +++ b/clang/www/analyzer/faq.html @@ -195,12 +195,45 @@

Q: How can I suppress a specific analyzer warning?

-

There is currently no solid mechanism for suppressing an analyzer warning, -although this is currently being investigated. When you encounter an analyzer -bug/false positive, check if it's one of the issues discussed above or if the -analyzer annotations can -resolve the issue. Second, please report it to -help us improve user experience. As the last resort, consider using __clang_analyzer__ macro +

When you encounter an analyzer bug/false positive, check if it's one of the +issues discussed above or if the analyzer +annotations can +resolve the issue by helping the static analyzer understand the code better. +Second, please report it to help us improve +user experience.

+ +

Sometimes there's really no "good" way to eliminate the issue. In such cases +you can "silence" it directly by annotating the problematic line of code with +the help of Clang attribute 'suppress': + +

+int foo() {
+  int *x = nullptr;
+  ...
+  [[clang::suppress]] {
+    // all warnings in this scope are suppressed
+    int y = *x;
+  }
+
+  // null pointer dereference warning suppressed on the next line
+  [[clang::suppress]]
+  return *x
+}
+
+int bar(bool coin_flip) {
+  // suppress all memory leak warnings about this allocation
+  [[clang::suppress]]
+  int *result = (int *)malloc(sizeof(int));
+
+  if (coin_flip)
+    return 0;      // including this leak path
+
+  return *result;  // as well as this leak path
+}
+
+ + +

You can also consider using __clang_analyzer__ macro described below.

Q: How can I selectively exclude code the analyzer examines?