[clang] ef3f476 - [attributes][analyzer] Implement [[clang::suppress]] - suppress static analysis warnings.

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 13 18:09:34 PST 2023


Author: Artem Dergachev
Date: 2023-12-13T18:09:16-08:00
New Revision: ef3f476097c7a13c0578e331e44b584b706089ed

URL: https://github.com/llvm/llvm-project/commit/ef3f476097c7a13c0578e331e44b584b706089ed
DIFF: https://github.com/llvm/llvm-project/commit/ef3f476097c7a13c0578e331e44b584b706089ed.diff

LOG: [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

Added: 
    clang/include/clang/StaticAnalyzer/Core/BugReporter/BugSuppression.h
    clang/lib/StaticAnalyzer/Core/BugSuppression.cpp
    clang/test/Analysis/suppression-attr-doc.cpp
    clang/test/Analysis/suppression-attr.m
    clang/test/SemaCXX/attr-suppress.cpp
    clang/test/SemaObjC/attr-suppress.m

Modified: 
    clang/include/clang/Basic/Attr.td
    clang/include/clang/Basic/AttrDocs.td
    clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
    clang/lib/Sema/SemaDeclAttr.cpp
    clang/lib/Sema/SemaStmtAttr.cpp
    clang/lib/StaticAnalyzer/Core/BugReporter.cpp
    clang/lib/StaticAnalyzer/Core/CMakeLists.txt
    clang/www/analyzer/faq.html

Removed: 
    clang/test/SemaCXX/suppress.cpp


################################################################################
diff  --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index c8b17cbe8ab7e7..d47a59b3f7ca4f 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -2792,9 +2792,10 @@ def SwiftAsyncError : InheritableAttr {
   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
index 1a98196834cefc..77950ab6d877ea 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -5267,7 +5267,74 @@ the ``int`` parameter is the one that represents the error.
 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
+<https://clang.llvm.org/docs/UsersManual.html#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 
diff erent 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
index 8956552e7bfc21..e762f7548e0b54 100644
--- 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 @@ class BugReporter {
   /// 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();

diff  --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugSuppression.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugSuppression.h
new file mode 100644
index 00000000000000..4fd81b62751974
--- /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<llvm::StringRef>;
+
+  /// 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<SourceRange, EXPECTED_NUMBER_OF_SUPPRESSIONS>;
+
+  llvm::DenseMap<const Decl *, CachedRanges> 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
index 22f291ee113384..5b29b05dee54b3 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -5225,8 +5225,16 @@ static void handleCallConvAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
 }
 
 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<VarDecl>(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<StringRef> DiagnosticIdentifiers;
   for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) {
@@ -5235,8 +5243,6 @@ static void handleSuppressAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
     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
index 725d8efe3828d6..0d0a7bcebab4e8 100644
--- a/clang/lib/Sema/SemaStmtAttr.cpp
+++ b/clang/lib/Sema/SemaStmtAttr.cpp
@@ -53,6 +53,13 @@ static Attr *handleFallThroughAttr(Sema &S, Stmt *St, const ParsedAttr &A,
 
 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<StringRef> DiagnosticIdentifiers;
   for (unsigned I = 0, E = A.getNumArgs(); I != E; ++I) {
     StringRef RuleName;
@@ -60,8 +67,6 @@ static Attr *handleSuppressAttr(Sema &S, Stmt *St, const ParsedAttr &A,
     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
index a253f2b637f5a3..f3e0a5f9f314ad 100644
--- 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 @@ PathSensitiveBugReport::getLocation() const {
   }
 
   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);
@@ -2896,6 +2905,10 @@ void BugReporter::emitReport(std::unique_ptr<BugReport> R) {
   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
index 00000000000000..b5991e47a53887
--- /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<SourceRange>;
+
+inline bool hasSuppression(const Decl *D) {
+  // FIXME: Implement diagnostic identifier arguments
+  // (checker names, "hashtags").
+  if (const auto *Suppression = D->getAttr<SuppressAttr>())
+    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<SuppressAttr>(A);
+    return Suppression && !Suppression->isGSL() &&
+           (Suppression->diagnosticIdentifiers().empty());
+  });
+}
+
+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,
+                                  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
index 3df93374dada75..8672876c0608d0 100644
--- a/clang/lib/StaticAnalyzer/Core/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Core/CMakeLists.txt
@@ -11,6 +11,7 @@ add_clang_library(clangStaticAnalyzerCore
   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
index 00000000000000..1208842799ed9a
--- /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
index 00000000000000..8ba8dda722721b
--- /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__)))
+
+ at protocol NSObject
+- (id)retain;
+- (oneway void)release;
+ at end
+ at interface NSObject <NSObject> {
+}
+- (id)init;
++ (id)alloc;
+ at end
+typedef int NSInteger;
+typedef char BOOL;
+typedef struct _NSZone NSZone;
+ at class NSInvocation, NSMethodSignature, NSCoder, NSString, NSEnumerator;
+ at protocol NSCopying
+- (id)copyWithZone:(NSZone *)zone;
+ at end
+ at protocol NSCoding
+- (void)encodeWithCoder:(NSCoder *)aCoder;
+ at end
+ at class NSDictionary;
+ at interface NSError : NSObject <NSCopying, NSCoding> {
+}
++ (id)errorWithDomain:(NSString *)domain code:(NSInteger)code userInfo:(NSDictionary *)dict;
+ at end
+
+ at interface NSMutableString : NSObject
+ at 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];
+  }
+}
+
+ at interface UIResponder : NSObject {
+}
+- (char)resignFirstResponder;
+ at end
+
+ at interface Test : UIResponder {
+}
+ at property(copy) NSMutableString *mutableStr;
+// expected-warning at -1 {{Property of mutable type 'NSMutableString' has 'copy' attribute; an immutable object will be stored instead}}
+ at end
+ at 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 at -1 {{Method accepting NSError** should have a non-void return value to indicate whether or not an error occurred}}
+}
+ at end
+
+ at interface TestSuppress : UIResponder {
+}
+// TODO: reassess when we decide what to do with declaration annotations
+ at property(copy) /* SUPPRESS */ NSMutableString *mutableStr;
+// expected-warning at -1 {{Property of mutable type 'NSMutableString' has 'copy' attribute; an immutable object will be stored instead}}
+ at end
+ at 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 at -1 {{Method accepting NSError** should have a non-void return value to indicate whether or not an error occurred}}
+}
+ at 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
index 00000000000000..fb5e2ac7ce2066
--- /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<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 {{expected string literal as argument of 'suppress' attribute}}
+}
+
+union [[gsl::suppress("type.1")]] U {
+  int i;
+  float f;
+};
+
+[[clang::suppress]];
+// expected-error at -1 {{'suppress' attribute only applies to variables and statements}}
+
+namespace N {
+[[clang::suppress("in-a-namespace")]];
+// expected-error at -1 {{'suppress' attribute only applies to variables and statements}}
+} // namespace N
+
+[[clang::suppress]] int global = 42;
+
+[[clang::suppress]] void foo() {
+  // expected-error at -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 at -1 {{'suppress' attribute cannot be applied to types}}
+  [[clang::suppress(foo)]] float f;
+  // expected-error at -1 {{expected string literal as argument of 'suppress' attribute}}
+}
+
+class [[clang::suppress("type.1")]] V {
+  // expected-error at -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
index 29544b3c573ccc..00000000000000
--- 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<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 {{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
index 00000000000000..ade8f94ec5895e
--- /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 at -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 
diff erent 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 at -2 {{expected string literal as argument of 'suppress' attribute}}
+}
+
+union SUPPRESS2("type.1") U {
+  // expected-error at -1 {{'suppress' attribute only applies to variables and statements}}
+  int i;
+  float f;
+};
+
+SUPPRESS1 @interface Test {
+  // expected-error at -1 {{'suppress' attribute only applies to variables and statements}}
+}
+ at property SUPPRESS2("prop") int *prop;
+// expected-error at -1 {{'suppress' attribute only applies to variables and statements}}
+- (void)bar:(int)x SUPPRESS1;
+// expected-error at -1 {{'suppress' attribute only applies to variables and statements}}
+ at end

diff  --git a/clang/www/analyzer/faq.html b/clang/www/analyzer/faq.html
index 72ca27eb8c36b9..156d383db2f234 100644
--- a/clang/www/analyzer/faq.html
+++ b/clang/www/analyzer/faq.html
@@ -195,12 +195,45 @@ <h4 id="use_assert" class="faq">Q: The analyzer assumes that a loop body is neve
 
 <h4 id="suppress_issue" class="faq">Q: How can I suppress a specific analyzer warning?</h4>
 
-<p>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 <a href = "annotations.html#custom_assertions" >annotations</a> can
-resolve the issue. Second, please <a href = "filing_bugs.html">report it</a> to
-help us improve user experience. As the last resort, consider using <tt>__clang_analyzer__</tt> macro
+<p>When you encounter an analyzer bug/false positive, check if it's one of the
+issues discussed above or if the analyzer
+<a href = "annotations.html#custom_assertions" >annotations</a> can
+resolve the issue by helping the static analyzer understand the code better.
+Second, please <a href = "filing_bugs.html">report it</a> to help us improve
+user experience.</p>
+
+<p>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 '<a href="https://clang.llvm.org/docs/AttributeReference.html#suppress">suppress</a>':
+
+<pre class="code_example">
+int foo() {
+  int *x = nullptr;
+  ...
+  <span class="code_highlight">[[clang::suppress]]</span> {
+    // all warnings in this scope are suppressed
+    int y = *x;
+  }
+
+  // null pointer dereference warning suppressed on the next line
+  <span class="code_highlight">[[clang::suppress]]</span>
+  return *x
+}
+
+int bar(bool coin_flip) {
+  // suppress all memory leak warnings about this allocation
+  <span class="code_highlight">[[clang::suppress]]</span>
+  int *result = (int *)malloc(sizeof(int));
+
+  if (coin_flip)
+    return 0;      // including this leak path
+
+  return *result;  // as well as this leak path
+}
+</pre>
+
+
+<p>You can also consider using <tt>__clang_analyzer__</tt> macro
 <a href = "faq.html#exclude_code" >described below</a>.</p>
 
 <h4 id="exclude_code" class="faq">Q: How can I selectively exclude code the analyzer examines?</h4>


        


More information about the cfe-commits mailing list