[-Wunsafe-buffer-usage] Add unsafe buffer checking opt-out pragmas

Add a pair of clang pragmas:
- `#pragma clang unsafe_buffer_usage begin` and
- `#pragma clang unsafe_buffer_usage end`,
which specify the start and end of an (unsafe buffer checking) opt-out
region, respectively.

Behaviors of opt-out regions conform to the following rules:

- No nested nor overlapped opt-out regions are allowed. One cannot
  start an opt-out region with `... unsafe_buffer_usage begin` but never
  close it with `... unsafe_buffer_usage end`. Mis-use of the pragmas
  will be warned.
- Warnings raised from unsafe buffer operations inside such an opt-out
  region will always be suppressed. This behavior CANNOT be changed by
  `clang diagnostic` pragmas or command-line flags.
- Warnings raised from unsafe operations outside of such opt-out
  regions may be reported on declarations inside opt-out
  regions. These warnings are NOT suppressed.
- An un-suppressed unsafe operation warning may be attached with
  notes. These notes are NOT suppressed as well regardless of whether
  they are in opt-out regions.

The implementation maintains a separate sequence of location pairs
representing opt-out regions in `Preprocessor`.  The `UnsafeBufferUsage`
analyzer reads the region sequence to check if an unsafe operation is
in an opt-out region. If it is, discard the warning raised from the
operation immediately.

This is a re-land after I reverting it at 9aa00c8.
The compilation error should be resolved.

Reviewed by: NoQ

Differential revision: https://reviews.llvm.org/D140179

Author: ziqingluo-90

clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h

@@ -38,6 +38,9 @@ class UnsafeBufferUsageHandler {
   virtual void handleFixableVariable(const VarDecl *Variable,
                                      FixItList &&List) = 0;
 
+  /// Returns a reference to the `Preprocessor`:
+  virtual bool isSafeBufferOptOut(const SourceLocation &Loc) const = 0;
+
   /// Returns the text indicating that the user needs to provide input there:
   virtual std::string
   getUserFillPlaceHolder(StringRef HintTextToUser = "placeholder") {

clang/include/clang/Basic/DiagnosticLexKinds.td

@@ -945,4 +945,15 @@ def err_dep_source_scanner_unexpected_tokens_at_import : Error<
 
 }
 
+def err_pp_double_begin_pragma_unsafe_buffer_usage :
+Error<"already inside '#pragma unsafe_buffer_usage'">;
+
+def err_pp_unmatched_end_begin_pragma_unsafe_buffer_usage :
+Error<"not currently inside '#pragma unsafe_buffer_usage'">;
+
+def err_pp_unclosed_pragma_unsafe_buffer_usage :
+Error<"'#pragma unsafe_buffer_usage' was not ended">;
+
+def err_pp_pragma_unsafe_buffer_usage_syntax :
+Error<"Expected 'begin' or 'end'">;
 }

clang/include/clang/Lex/Preprocessor.h

@@ -2688,6 +2688,51 @@ class Preprocessor {
   void emitMacroDeprecationWarning(const Token &Identifier) const;
   void emitRestrictExpansionWarning(const Token &Identifier) const;
   void emitFinalMacroWarning(const Token &Identifier, bool IsUndef) const;
+
+  /// This boolean state keeps track if the current scanned token (by this PP)
+  /// is in an "-Wunsafe-buffer-usage" opt-out region. Assuming PP scans a
+  /// translation unit in a linear order.
+  bool InSafeBufferOptOutRegion = 0;
+
+  /// Hold the start location of the current "-Wunsafe-buffer-usage" opt-out
+  /// region if PP is currently in such a region.  Hold undefined value
+  /// otherwise.
+  SourceLocation CurrentSafeBufferOptOutStart; // It is used to report the start location of an never-closed region.
+
+  // An ordered sequence of "-Wunsafe-buffer-usage" opt-out regions in one
+  // translation unit. Each region is represented by a pair of start and end
+  // locations.  A region is "open" if its' start and end locations are
+  // identical.
+  SmallVector<std::pair<SourceLocation, SourceLocation>, 8> SafeBufferOptOutMap;
+
+public:
+  /// \return true iff the given `Loc` is in a "-Wunsafe-buffer-usage" opt-out
+  /// region.  This `Loc` must be a source location that has been pre-processed.
+  bool isSafeBufferOptOut(const SourceManager&SourceMgr, const SourceLocation &Loc) const;
+
+  /// Alter the state of whether this PP currently is in a
+  /// "-Wunsafe-buffer-usage" opt-out region.
+  ///
+  /// \param isEnter: true if this PP is entering a region; otherwise, this PP
+  /// is exiting a region
+  /// \param Loc: the location of the entry or exit of a
+  /// region
+  /// \return true iff it is INVALID to enter or exit a region, i.e.,
+  /// attempt to enter a region before exiting a previous region, or exiting a
+  /// region that PP is not currently in.
+  bool enterOrExitSafeBufferOptOutRegion(bool isEnter,
+                                         const SourceLocation &Loc);
+
+  /// \return true iff this PP is currently in a "-Wunsafe-buffer-usage"
+  ///          opt-out region
+  bool isPPInSafeBufferOptOutRegion();
+
+  /// \param StartLoc: output argument. It will be set to the start location of
+  /// the current "-Wunsafe-buffer-usage" opt-out region iff this function
+  /// returns true.
+  /// \return true iff this PP is currently in a "-Wunsafe-buffer-usage"
+  ///          opt-out region
+  bool isPPInSafeBufferOptOutRegion(SourceLocation &StartLoc);
 };
 
 /// Abstract base class that describes a handler that will receive

clang/lib/Analysis/UnsafeBufferUsage.cpp

@@ -9,6 +9,7 @@
 #include "clang/Analysis/Analyses/UnsafeBufferUsage.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/SmallVector.h"
 #include <memory>
@@ -117,6 +118,11 @@ AST_MATCHER_P(Stmt, forEveryDescendant, internal::Matcher<Stmt>, innerMatcher) {
   return Visitor.findMatch(DynTypedNode::create(Node));
 }
 
+// Matches a `Stmt` node iff the node is in a safe-buffer opt-out region
+AST_MATCHER_P(Stmt, notInSafeBufferOptOut, const UnsafeBufferUsageHandler *, Handler) {
+  return !Handler->isSafeBufferOptOut(Node.getBeginLoc());
+}
+
 AST_MATCHER_P(CastExpr, castSubExpr, internal::Matcher<Expr>, innerMatcher) {
   return innerMatcher.matches(*Node.getSubExpr(), Finder, Builder);
 }
@@ -548,7 +554,8 @@ class Strategy {
 } // namespace
 
 /// Scan the function and return a list of gadgets found with provided kits.
-static std::tuple<FixableGadgetList, WarningGadgetList, DeclUseTracker> findGadgets(const Decl *D) {
+static std::tuple<FixableGadgetList, WarningGadgetList, DeclUseTracker>
+findGadgets(const Decl *D, const UnsafeBufferUsageHandler &Handler) {
 
   struct GadgetFinderCallback : MatchFinder::MatchCallback {
     FixableGadgetList FixableGadgets;
@@ -620,7 +627,7 @@ static std::tuple<FixableGadgetList, WarningGadgetList, DeclUseTracker> findGadg
       stmt(anyOf(
         // Add Gadget::matcher() for every gadget in the registry.
 #define WARNING_GADGET(x)                                                              \
-        x ## Gadget::matcher().bind(#x),
+        allOf(x ## Gadget::matcher().bind(#x), notInSafeBufferOptOut(&Handler)),
 #include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def"
         // In parallel, match all DeclRefExprs so that to find out
         // whether there are any uncovered by gadgets.
@@ -1009,7 +1016,7 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
   DeclUseTracker Tracker;
 
   {
-    auto [FixableGadgets, WarningGadgets, TrackerRes] = findGadgets(D);
+    auto [FixableGadgets, WarningGadgets, TrackerRes] = findGadgets(D, Handler);
     UnsafeOps = groupWarningGadgetsByVar(std::move(WarningGadgets));
     FixablesForUnsafeVars = groupFixablesByVar(std::move(FixableGadgets));
     Tracker = std::move(TrackerRes);

clang/lib/Lex/PPLexerChange.cpp

@@ -333,6 +333,15 @@ bool Preprocessor::HandleEndOfFile(Token &Result, bool isEndOfMacro) {
   assert(!CurTokenLexer &&
          "Ending a file when currently in a macro!");
 
+  SourceLocation UnclosedSafeBufferOptOutLoc;
+
+  if (IncludeMacroStack.empty() &&
+      isPPInSafeBufferOptOutRegion(UnclosedSafeBufferOptOutLoc)) {
+    // To warn if a "-Wunsafe-buffer-usage" opt-out region is still open by the
+    // end of a file.
+    Diag(UnclosedSafeBufferOptOutLoc,
+         diag::err_pp_unclosed_pragma_unsafe_buffer_usage);
+  }
   // If we have an unclosed module region from a pragma at the end of a
   // module, complain and close it now.
   const bool LeavingSubmodule = CurLexer && CurLexerSubmodule;

clang/lib/Lex/Pragma.cpp

@@ -1243,6 +1243,32 @@ struct PragmaDebugHandler : public PragmaHandler {
 #endif
 };
 
+struct PragmaUnsafeBufferUsageHandler : public PragmaHandler {
+  PragmaUnsafeBufferUsageHandler() : PragmaHandler("unsafe_buffer_usage") {}
+  void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
+                    Token &FirstToken) override {
+    Token Tok;
+
+    PP.LexUnexpandedToken(Tok);
+    if (Tok.isNot(tok::identifier)) {
+      PP.Diag(Tok, diag::err_pp_pragma_unsafe_buffer_usage_syntax);
+      return;
+    }
+
+    IdentifierInfo *II = Tok.getIdentifierInfo();
+    SourceLocation Loc = Tok.getLocation();
+
+    if (II->isStr("begin")) {
+      if (PP.enterOrExitSafeBufferOptOutRegion(true, Loc))
+        PP.Diag(Loc, diag::err_pp_double_begin_pragma_unsafe_buffer_usage);
+    } else if (II->isStr("end")) {
+      if (PP.enterOrExitSafeBufferOptOutRegion(false, Loc))
+        PP.Diag(Loc, diag::err_pp_unmatched_end_begin_pragma_unsafe_buffer_usage);
+    } else
+      PP.Diag(Tok, diag::err_pp_pragma_unsafe_buffer_usage_syntax);
+  }
+};
+
 /// PragmaDiagnosticHandler - e.g. '\#pragma GCC diagnostic ignored "-Wformat"'
 struct PragmaDiagnosticHandler : public PragmaHandler {
 private:
@@ -2128,6 +2154,9 @@ void Preprocessor::RegisterBuiltinPragmas() {
   ModuleHandler->AddPragma(new PragmaModuleBuildHandler());
   ModuleHandler->AddPragma(new PragmaModuleLoadHandler());
 
+  // Safe Buffers pragmas
+  AddPragmaHandler("clang", new PragmaUnsafeBufferUsageHandler);
+
   // Add region pragmas.
   AddPragmaHandler(new PragmaRegionHandler("region"));
   AddPragmaHandler(new PragmaRegionHandler("endregion"));

clang/lib/Lex/Preprocessor.cpp

@@ -1462,6 +1462,75 @@ void Preprocessor::emitFinalMacroWarning(const Token &Identifier,
   Diag(*A.FinalAnnotationLoc, diag::note_pp_macro_annotation) << 2;
 }
 
+bool Preprocessor::isSafeBufferOptOut(const SourceManager &SourceMgr,
+                                           const SourceLocation &Loc) const {
+  // Try to find a region in `SafeBufferOptOutMap` where `Loc` is in:
+  auto FirstRegionEndingAfterLoc = llvm::partition_point(
+      SafeBufferOptOutMap,
+      [&SourceMgr,
+       &Loc](const std::pair<SourceLocation, SourceLocation> &Region) {
+        return SourceMgr.isBeforeInTranslationUnit(Region.second, Loc);
+      });
+
+  if (FirstRegionEndingAfterLoc != SafeBufferOptOutMap.end()) {
+    // To test if the start location of the found region precedes `Loc`:
+    return SourceMgr.isBeforeInTranslationUnit(FirstRegionEndingAfterLoc->first,
+                                               Loc);
+  }
+  // If we do not find a region whose end location passes `Loc`, we want to
+  // check if the current region is still open:
+  if (!SafeBufferOptOutMap.empty() &&
+      SafeBufferOptOutMap.back().first == SafeBufferOptOutMap.back().second)
+    return SourceMgr.isBeforeInTranslationUnit(SafeBufferOptOutMap.back().first,
+                                               Loc);
+  return false;
+}
+
+bool Preprocessor::enterOrExitSafeBufferOptOutRegion(
+    bool isEnter, const SourceLocation &Loc) {
+  if (isEnter) {
+    if (isPPInSafeBufferOptOutRegion())
+      return true; // invalid enter action
+    InSafeBufferOptOutRegion = true;
+    CurrentSafeBufferOptOutStart = Loc;
+
+    // To set the start location of a new region:
+
+    if (!SafeBufferOptOutMap.empty()) {
+      auto *PrevRegion = &SafeBufferOptOutMap.back();
+      assert(PrevRegion->first != PrevRegion->second &&
+             "Shall not begin a safe buffer opt-out region before closing the "
+             "previous one.");
+    }
+    // If the start location equals to the end location, we call the region a
+    // open region or a unclosed region (i.e., end location has not been set
+    // yet).
+    SafeBufferOptOutMap.emplace_back(Loc, Loc);
+  } else {
+    if (!isPPInSafeBufferOptOutRegion())
+      return true; // invalid enter action
+    InSafeBufferOptOutRegion = false;
+
+    // To set the end location of the current open region:
+
+    assert(!SafeBufferOptOutMap.empty() &&
+           "Misordered safe buffer opt-out regions");
+    auto *CurrRegion = &SafeBufferOptOutMap.back();
+    assert(CurrRegion->first == CurrRegion->second &&
+           "Set end location to a closed safe buffer opt-out region");
+    CurrRegion->second = Loc;
+  }
+  return false;
+}
+
+bool Preprocessor::isPPInSafeBufferOptOutRegion() {
+  return InSafeBufferOptOutRegion;
+}
+bool Preprocessor::isPPInSafeBufferOptOutRegion(SourceLocation &StartLoc) {
+  StartLoc = CurrentSafeBufferOptOutStart;
+  return InSafeBufferOptOutRegion;
+}
+
 ModuleLoader::~ModuleLoader() = default;
 
 CommentHandler::~CommentHandler() = default;

clang/lib/Sema/AnalysisBasedWarnings.cpp

@@ -2213,6 +2213,10 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
         FD << F;
     }
   }
+
+  bool isSafeBufferOptOut(const SourceLocation &Loc) const override {
+    return S.PP.isSafeBufferOptOut(S.getSourceManager(), Loc);
+  }
 };
 } // namespace
 

clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma-fixit.cpp

@@ -0,0 +1,107 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+void basic(int * x) {
+  int tmp;
+  int *p1 = new int[10];  // no fix
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+  int *p2 = new int[10];
+  // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span<int> p2"
+  // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
+  // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 10}"
+#pragma clang unsafe_buffer_usage begin
+  tmp = p1[5];
+#pragma clang unsafe_buffer_usage end
+  tmp = p2[5];
+}
+
+void withDiagnosticWarning() {
+  int tmp;
+  int *p1 = new int[10]; // no fix
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+  int *p2 = new int[10];
+  // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span<int> p2"
+  // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
+  // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 10}"
+
+  // diagnostics in opt-out region
+#pragma clang unsafe_buffer_usage begin
+  tmp = p1[5];  // not to warn
+  tmp = p2[5];  // not to warn
+#pragma clang diagnostic push
+#pragma clang diagnostic warning "-Wunsafe-buffer-usage"
+  tmp = p1[5];  // not to warn
+  tmp = p2[5];  // not to warn
+#pragma clang diagnostic warning "-Weverything"
+  tmp = p1[5];  // not to warn
+  tmp = p2[5];  // not to warn
+#pragma clang diagnostic pop
+#pragma clang unsafe_buffer_usage end
+
+  // opt-out region under diagnostic warning
+#pragma clang diagnostic push
+#pragma clang diagnostic warning "-Wunsafe-buffer-usage"
+#pragma clang unsafe_buffer_usage begin
+  tmp = p1[5];  // not to warn
+  tmp = p2[5];  // not to warn
+#pragma clang unsafe_buffer_usage end
+#pragma clang diagnostic pop
+
+  tmp = p2[5];
+}
+
+
+void withDiagnosticIgnore() {
+  int tmp;
+  int *p1 = new int[10];
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+  int *p2 = new int[10];
+  // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span<int> p2"
+  // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
+  // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 10}"
+  int *p3 = new int[10];
+  // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span<int> p3"
+  // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
+  // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 10}"
+
+#pragma clang unsafe_buffer_usage begin
+  tmp = p1[5];  // not to warn
+  tmp = p2[5];  // not to warn
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunsafe-buffer-usage"
+  tmp = p1[5];  // not to warn
+  tmp = p2[5];  // not to warn
+#pragma clang diagnostic ignored "-Weverything"
+  tmp = p1[5];  // not to warn
+  tmp = p2[5];  // not to warn
+#pragma clang diagnostic pop
+#pragma clang unsafe_buffer_usage end
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunsafe-buffer-usage"
+#pragma clang unsafe_buffer_usage begin
+  tmp = p1[5];  // not to warn
+  tmp = p2[5];  // not to warn
+#pragma clang unsafe_buffer_usage end
+#pragma clang diagnostic pop
+
+  tmp = p2[5];
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunsafe-buffer-usage"
+#pragma clang unsafe_buffer_usage begin
+  tmp = p1[5];  // not to warn
+  tmp = p2[5];  // not to warn
+#pragma clang unsafe_buffer_usage end
+  tmp = p3[5];  // expected-note{{used in buffer access here}}
+#pragma clang diagnostic pop
+}
+
+void noteGoesWithVarDeclWarning() {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunsafe-buffer-usage"
+  int *p = new int[10]; // not to warn
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+#pragma clang diagnostic pop
+
+  p[5]; // not to note since the associated warning is suppressed
+}