vsavchenko updated this revision to Diff 320068.
vsavchenko added a comment.
Herald added a subscriber: jdoerfert.

Fix failing tests


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/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaCXX/attr-analyzer-suppress.cpp
  clang/test/SemaCXX/suppress.cpp
  clang/test/SemaObjC/attr-analyzer-suppress.m

Index: clang/test/SemaObjC/attr-analyzer-suppress.m
===================================================================
--- /dev/null
+++ clang/test/SemaObjC/attr-analyzer-suppress.m
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks %s -verify
+
+#define SUPPRESS1 __attribute__((analyzer_suppress))
+#define SUPPRESS2(...) __attribute__((analyzer_suppress(__VA_ARGS__)))
+
+SUPPRESS1 int global = 42;
+
+SUPPRESS1 void foo() {
+  // expected-warning@-1 {{'analyzer_suppress' attribute only applies to variables}}
+  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@-1 {{'analyzer_suppress' attribute requires a string}}
+}
+
+union SUPPRESS2("type.1") U {
+  // expected-warning@-1 {{'analyzer_suppress' attribute only applies to variables}}
+  int i;
+  float f;
+};
+
+SUPPRESS1 @interface Test {
+  // expected-warning@-1 {{'analyzer_suppress' attribute only applies to variables}}
+}
+@property SUPPRESS2("prop") int *prop;
+// expected-warning@-1 {{'analyzer_suppress' attribute only applies to variables}}
+- (void)bar:(int) x SUPPRESS1;
+// expected-warning@-1 {{'analyzer_suppress' attribute only applies to variables}}
+@end
Index: clang/test/SemaCXX/suppress.cpp
===================================================================
--- clang/test/SemaCXX/suppress.cpp
+++ clang/test/SemaCXX/suppress.cpp
@@ -6,16 +6,15 @@
   [[gsl::suppress("in-a-namespace")]];
 }
 
-[[gsl::suppress("readability-identifier-naming")]]
-void f_() {
+[[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]] 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}}
 }
 
Index: clang/test/SemaCXX/attr-analyzer-suppress.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/attr-analyzer-suppress.cpp
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only %s -verify
+
+[[clang::analyzer_suppress]];
+// expected-warning@-1 {{'analyzer_suppress' attribute only applies to variables}}
+
+namespace N {
+[[clang::analyzer_suppress("in-a-namespace")]];
+// expected-warning@-1 {{'analyzer_suppress' attribute only applies to variables}}
+}
+
+[[clang::analyzer_suppress]] int global = 42;
+
+[[clang::analyzer_suppress]] void foo() {
+  // expected-warning@-1 {{'analyzer_suppress' attribute only applies to variables}}
+  [[clang::analyzer_suppress]] int *p;
+
+  [[clang::analyzer_suppress]] int a = 0;           // no-warning
+  [[clang::analyzer_suppress()]] int b = 1;         // no-warning
+  [[clang::analyzer_suppress("a")]] int c = a + b;  // no-warning
+  [[clang::analyzer_suppress("a", "b")]] b = c - a; // no-warning
+
+  [[clang::analyzer_suppress("a", "b")]] if (b == 10) a += 4; // no-warning
+  [[clang::analyzer_suppress]] while (true) {}                // no-warning
+  [[clang::analyzer_suppress]] switch (a) {                   // no-warning
+  default:
+    c -= 10;
+  }
+
+  int [[clang::analyzer_suppress("r")]] z;
+  // expected-error@-1 {{'analyzer_suppress' attribute cannot be applied to types}}
+  [[clang::analyzer_suppress(foo)]] float f;
+  // expected-error@-1 {{'analyzer_suppress' attribute requires a string}}
+}
+
+class [[clang::analyzer_suppress("type.1")]] U {
+  // expected-warning@-1 {{'analyzer_suppress' attribute only applies to variables}}
+  int i;
+  float f;
+};
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===================================================================
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -15,6 +15,7 @@
 // CHECK-NEXT: AllocSize (SubjectMatchRule_function)
 // CHECK-NEXT: AlwaysDestroy (SubjectMatchRule_variable)
 // CHECK-NEXT: AlwaysInline (SubjectMatchRule_function)
+// CHECK-NEXT: AnalyzerSuppress (SubjectMatchRule_variable)
 // CHECK-NEXT: Annotate ()
 // CHECK-NEXT: AnyX86NoCfCheck (SubjectMatchRule_hasType_functionType)
 // CHECK-NEXT: ArcWeakrefUnavailable (SubjectMatchRule_objc_interface)
Index: clang/test/Analysis/suppression-attr.m
===================================================================
--- /dev/null
+++ clang/test/Analysis/suppression-attr.m
@@ -0,0 +1,240 @@
+// 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__((analyzer_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 {
+  // expected-warning@-1 {{'analyzer_suppress' attribute only applies to variables}}
+  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() {
+  // expected-warning@-1 {{'analyzer_suppress' attribute only applies to variables}}
+  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 {{'analyzer_suppress' attribute only applies to variables}}
+// expected-warning@-2 {{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 {
+  // expected-warning@-1 {{'analyzer_suppress' attribute only applies to variables}}
+  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 {{'analyzer_suppress' attribute only applies to variables}}
+  // expected-warning@-2 {{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,159 @@
+//===- 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) {
+  return D->hasAttr<AnalyzerSuppressAttr>();
+}
+inline bool hasSuppression(const AttributedStmt *S) {
+  return llvm::any_of(S->getAttrs(), [](const Attr *A) {
+    return isa<AnalyzerSuppressAttr>(A);
+  });
+}
+
+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"
@@ -2383,11 +2386,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();
@@ -2404,6 +2406,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);
@@ -2877,13 +2885,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
@@ -51,13 +51,9 @@
   return ::new (S.Context) FallThroughAttr(S.Context, A);
 }
 
-static Attr *handleSuppressAttr(Sema &S, Stmt *St, const ParsedAttr &A,
+template <class SuppressAttrTy>
+static Attr *createSuppressAttr(Sema &S, Stmt *St, const ParsedAttr &A,
                                 SourceRange Range) {
-  if (A.getNumArgs() < 1) {
-    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;
@@ -65,15 +61,28 @@
     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);
   }
 
-  return ::new (S.Context) SuppressAttr(
+  return ::new (S.Context) SuppressAttrTy(
       S.Context, A, DiagnosticIdentifiers.data(), DiagnosticIdentifiers.size());
 }
 
+static Attr *handleSuppressAttr(Sema &S, Stmt *St, const ParsedAttr &A,
+                                SourceRange Range) {
+  if (A.isCXX11Attribute() && A.getNumArgs() < 1) {
+    S.Diag(A.getLoc(), diag::err_attribute_too_few_arguments) << A << 1;
+    return nullptr;
+  }
+
+  return createSuppressAttr<SuppressAttr>(S, St, A, Range);
+}
+
+static Attr *handleAnalyzerSuppressAttr(Sema &S, Stmt *St, const ParsedAttr &A,
+                                        SourceRange Range) {
+  return createSuppressAttr<AnalyzerSuppressAttr>(S, St, A, Range);
+}
+
 static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const ParsedAttr &A,
                                 SourceRange) {
   IdentifierLoc *PragmaNameLoc = A.getArgAsIdent(0);
@@ -419,6 +428,8 @@
     return handleOpenCLUnrollHint(S, St, A, Range);
   case ParsedAttr::AT_Suppress:
     return handleSuppressAttr(S, St, A, Range);
+  case ParsedAttr::AT_AnalyzerSuppress:
+    return handleAnalyzerSuppressAttr(S, St, A, Range);
   case ParsedAttr::AT_NoMerge:
     return handleNoMergeAttr(S, St, A, Range);
   case ParsedAttr::AT_Likely:
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4664,10 +4664,8 @@
   }
 }
 
-static void handleSuppressAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
-  if (!checkAttributeAtLeastNumArgs(S, AL, 1))
-    return;
-
+template <class SuppressAttrTy>
+static void addSuppressAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   std::vector<StringRef> DiagnosticIdentifiers;
   for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) {
     StringRef RuleName;
@@ -4675,13 +4673,22 @@
     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)
-                 SuppressAttr(S.Context, AL, DiagnosticIdentifiers.data(),
-                              DiagnosticIdentifiers.size()));
+                 SuppressAttrTy(S.Context, AL, DiagnosticIdentifiers.data(),
+                                DiagnosticIdentifiers.size()));
+}
+
+static void handleSuppressAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  if (!checkAttributeAtLeastNumArgs(S, AL, 1))
+    return;
+
+  addSuppressAttr<SuppressAttr>(S, D, AL);
+}
+
+static void handleAnalyzerSuppressAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  addSuppressAttr<AnalyzerSuppressAttr>(S, D, AL);
 }
 
 static void handleLifetimeCategoryAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
@@ -8004,6 +8011,9 @@
   case ParsedAttr::AT_Suppress:
     handleSuppressAttr(S, D, AL);
     break;
+  case ParsedAttr::AT_AnalyzerSuppress:
+    handleAnalyzerSuppressAttr(S, D, AL);
+    break;
   case ParsedAttr::AT_Owner:
   case ParsedAttr::AT_Pointer:
     handleLifetimeCategoryAttr(S, D, AL);
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"
@@ -588,6 +589,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
@@ -4512,6 +4512,42 @@
   }];
 }
 
+def AnalyzerSuppressDocs : Documentation {
+  let Category = DocCatStmt;
+  let Content = [{
+The ``analyzer_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
+  }
+
+  }];
+}
+
 def AbiTagsDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -2390,6 +2390,13 @@
   let Documentation = [SuppressDocs];
 }
 
+def AnalyzerSuppress : DeclOrStmtAttr {
+  let Spellings = [Clang<"analyzer_suppress">];
+  let Args = [VariadicStringArgument<"DiagnosticIdentifiers">];
+  let Subjects = SubjectList<[Var]>;
+  let Documentation = [AnalyzerSuppressDocs];
+}
+
 def SysVABI : DeclOrTypeAttr {
   let Spellings = [GCC<"sysv_abi">];
 //  let Subjects = [Function, ObjCMethod];
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to