Merge pull request #7893 from apple/static-analyzer-cherrypicks-2023-12

Static analyzer cherrypicks 2023/12

Author: haoNoQ

clang/include/clang/Basic/Attr.td

@@ -2742,9 +2742,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];
 }
 

clang/include/clang/Basic/AttrDocs.td

@@ -5295,7 +5295,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 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.

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"
@@ -595,6 +596,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();

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

clang/lib/Sema/SemaDeclAttr.cpp

@@ -5209,8 +5209,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) {
@@ -5219,8 +5227,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)

clang/lib/Sema/SemaStmtAttr.cpp

@@ -53,15 +53,20 @@ 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;
 
     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);
   }
 

clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp

@@ -18,24 +18,15 @@ using namespace clang;
 
 namespace {
 
-bool hasPublicRefAndDeref(const CXXRecordDecl *R) {
+bool hasPublicMethodInBaseClass(const CXXRecordDecl *R,
+                                const char *NameToMatch) {
   assert(R);
   assert(R->hasDefinition());
 
-  bool hasRef = false;
-  bool hasDeref = false;
   for (const CXXMethodDecl *MD : R->methods()) {
     const auto MethodName = safeGetName(MD);
-
-    if (MethodName == "ref" && MD->getAccess() == AS_public) {
-      if (hasDeref)
-        return true;
-      hasRef = true;
-    } else if (MethodName == "deref" && MD->getAccess() == AS_public) {
-      if (hasRef)
-        return true;
-      hasDeref = true;
-    }
+    if (MethodName == NameToMatch && MD->getAccess() == AS_public)
+      return true;
   }
   return false;
 }
@@ -44,9 +35,8 @@ bool hasPublicRefAndDeref(const CXXRecordDecl *R) {
 
 namespace clang {
 
-std::optional<const clang::CXXRecordDecl*>
-isRefCountable(const CXXBaseSpecifier* Base)
-{
+std::optional<const clang::CXXRecordDecl *>
+hasPublicMethodInBase(const CXXBaseSpecifier *Base, const char *NameToMatch) {
   assert(Base);
 
   const Type *T = Base->getType().getTypePtrOrNull();
@@ -59,7 +49,7 @@ isRefCountable(const CXXBaseSpecifier* Base)
   if (!R->hasDefinition())
     return std::nullopt;
 
-  return hasPublicRefAndDeref(R) ? R : nullptr;
+  return hasPublicMethodInBaseClass(R, NameToMatch) ? R : nullptr;
 }
 
 std::optional<bool> isRefCountable(const CXXRecordDecl* R)
@@ -70,29 +60,45 @@ std::optional<bool> isRefCountable(const CXXRecordDecl* R)
   if (!R)
     return std::nullopt;
 
-  if (hasPublicRefAndDeref(R))
+  bool hasRef = hasPublicMethodInBaseClass(R, "ref");
+  bool hasDeref = hasPublicMethodInBaseClass(R, "deref");
+  if (hasRef && hasDeref)
     return true;
 
   CXXBasePaths Paths;
   Paths.setOrigin(const_cast<CXXRecordDecl *>(R));
 
   bool AnyInconclusiveBase = false;
-  const auto isRefCountableBase =
-      [&AnyInconclusiveBase](const CXXBaseSpecifier* Base, CXXBasePath&) {
-          std::optional<const clang::CXXRecordDecl*> IsRefCountable = clang::isRefCountable(Base);
-          if (!IsRefCountable) {
-              AnyInconclusiveBase = true;
-              return false;
-          }
-          return (*IsRefCountable) != nullptr;
+  const auto hasPublicRefInBase =
+      [&AnyInconclusiveBase](const CXXBaseSpecifier *Base, CXXBasePath &) {
+        auto hasRefInBase = clang::hasPublicMethodInBase(Base, "ref");
+        if (!hasRefInBase) {
+          AnyInconclusiveBase = true;
+          return false;
+        }
+        return (*hasRefInBase) != nullptr;
       };
 
-  bool BasesResult = R->lookupInBases(isRefCountableBase, Paths,
+  hasRef = hasRef || R->lookupInBases(hasPublicRefInBase, Paths,
                                       /*LookupInDependent =*/true);
   if (AnyInconclusiveBase)
     return std::nullopt;
 
-  return BasesResult;
+  const auto hasPublicDerefInBase =
+      [&AnyInconclusiveBase](const CXXBaseSpecifier *Base, CXXBasePath &) {
+        auto hasDerefInBase = clang::hasPublicMethodInBase(Base, "deref");
+        if (!hasDerefInBase) {
+          AnyInconclusiveBase = true;
+          return false;
+        }
+        return (*hasDerefInBase) != nullptr;
+      };
+  hasDeref = hasDeref || R->lookupInBases(hasPublicDerefInBase, Paths,
+                                          /*LookupInDependent =*/true);
+  if (AnyInconclusiveBase)
+    return std::nullopt;
+
+  return hasRef && hasDeref;
 }
 
 bool isCtorOfRefCounted(const clang::FunctionDecl *F) {
@@ -186,8 +192,7 @@ bool isPtrConversion(const FunctionDecl *F) {
   // FIXME: check # of params == 1
   const auto FunctionName = safeGetName(F);
   if (FunctionName == "getPtr" || FunctionName == "WeakPtr" ||
-      FunctionName == "makeWeakPtr"
-
+      FunctionName == "dynamicDowncast"
       || FunctionName == "downcast" || FunctionName == "bitwise_cast")
     return true;
 

clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h

@@ -26,10 +26,10 @@ class Type;
 // In WebKit there are two ref-counted templated smart pointers: RefPtr<T> and
 // Ref<T>.
 
-/// \returns CXXRecordDecl of the base if the type is ref-countable, nullptr if
-/// not, std::nullopt if inconclusive.
-std::optional<const clang::CXXRecordDecl*>
-isRefCountable(const clang::CXXBaseSpecifier* Base);
+/// \returns CXXRecordDecl of the base if the type has ref as a public method,
+/// nullptr if not, std::nullopt if inconclusive.
+std::optional<const clang::CXXRecordDecl *>
+hasPublicMethodInBase(const CXXBaseSpecifier *Base, const char *NameToMatch);
 
 /// \returns true if \p Class is ref-countable, false if not, std::nullopt if
 /// inconclusive.