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

[ziqingluo-90]

Summary

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

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

1. 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.
2. 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.
3. 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.
4. 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.

Examples,

void f() {
  int * p = new int [10];
#pragma clang unsafe_buffer_usage begin
  p[5];
#pragma clang unsafe_buffer_usage end
}

Nothing will be warned in the code above since the only unsafe operation p[5] is in an opt-out region.

void f() {
#pragma clang unsafe_buffer_usage begin
   int * p = new int [10];  // expect a warning on `p` for the unsafe operation below
#pragma clang unsafe_buffer_usage end
   p[5]; // may have a note here which is an attachment of the warning above
}

In the example above, a warning triggered by p[5], which is not in an opt-out region, will be reported on the declaration. Although the declaration is in an opt-out region, the warning is NOT suppressed.

[NoQ]

clang/include/clang/Basic/DiagnosticLexKinds.td

947

IIUC ExtWarn means it's a warning of the form "warning: XXX is a language extension". It's not just a warning that has something to do with language extensions, it's a warning that tries to warn the user about the very fact that it's an extension. So we should just use Warning or Error.

Also about wording: we already have a somewhat similar #pragma assume_nonnull, can we use similar text? And probably prefer Error because that's what the other pragma uses too:

def err_pp_assume_nonnull_syntax : Error<"expected 'begin' or 'end'">;
def err_pp_double_begin_of_assume_nonnull : Error<
  "already inside '#pragma clang assume_nonnull'">;
def err_pp_unmatched_end_of_assume_nonnull : Error<
  "not currently inside '#pragma clang assume_nonnull'">;
def err_pp_include_in_assume_nonnull : Error<
  "cannot %select{#include files|import headers}0 "
  "inside '#pragma clang assume_nonnull'">;
def err_pp_eof_in_assume_nonnull : Error<
  "'#pragma clang assume_nonnull' was not ended within this file">;

clang/lib/Sema/AnalysisBasedWarnings.cpp

2379 ↗

(On Diff #483332)

I believe this check should be performed *much earlier*. It's not about how we display unsafe usages to the user; we can exclude variables from analysis entirely when all their unsafe uses are guarded by the pragma. I suspect that we can drop these gadgets as early as in findGadgets() phase (assuming that D140062 causes us to never rely on unsafe gadgets for fixits).

[ziqingluo-90]

Fixing bugs in my test.

[jkorous]

clang/lib/Sema/AnalysisBasedWarnings.cpp

2379 ↗

(On Diff #483332)

Let's addres this in a follow-up patch.

[ziqingluo-90]

Addressing comments.

[ziqingluo-90]

clang/include/clang/Basic/DiagnosticLexKinds.td

947

Fixed!

clang/lib/Sema/AnalysisBasedWarnings.cpp

2379 ↗

(On Diff #483332)

I agree to both of you. Moving the check into UnsafeBufferUsage.cpp may depends on this https://reviews.llvm.org/D140062 patch. So I will make it a follow-up patch.

[ziqingluo-90]

Rebase the patch.

Move the check of whether a node is in an opt-out region to an earlier stage---the AST matching stage.

[NoQ]

clang/include/clang/Basic/Diagnostic.h

1040–1043

Ok, now I no longer see why this data should live in DiagnosticEngine. It's mostly about analysis, right? The pragma simply makes our analysis produce different results, regardless of whether these results are used for producing diagnostics or something else. Maybe let's keep it all in Preprocessor?

clang/lib/Analysis/UnsafeBufferUsage.cpp

543

This prevents safe fixable gadgets from being found in the opt-out zone. I think this clause should only apply to warning gadgets.

551–555

I think we should match all DeclStmts as well, because otherwise we may be unable to find the variable to fix.

[ziqingluo-90]

clang/include/clang/Basic/Diagnostic.h

1040–1043

make sense to me!

clang/lib/Analysis/UnsafeBufferUsage.cpp

543

You are right! Fixables should be found regardless of whether they are in an opt-out zone. A Fixable could later be immediately discarded once we know that the variable declaration associated to the Fixable is in an opt-out zone.

551–555

In case we are unable to find the variable to fix, it means that the variable declaration is in an opt-out zone. So we don't fix the variable anyway, right?

Or do you mean that a variable may still get fixed even if its declaration is in an opt-out zone? I could imagine it is possible if the variable is involved in some assignments that we want to fix.

[NoQ]

clang/lib/Analysis/UnsafeBufferUsage.cpp

551–555

do you mean that a variable may still get fixed even if its declaration is in an opt-out zone?

Yes I think that's the case. We do have tests about this right?:

#pragma clang unsafe_buffer_usage begin
  ...
  int *p3 = new int[10]; // expected-warning{{'p3' is an unsafe pointer used for buffer access}}

#pragma clang unsafe_buffer_usage end
  ...
  p3[5]; //expected-note{{used in buffer access here}}

And I think it's safe to assume that every time we emit a warning, we also want to emit a fixit.

If only we had any fixits implemented, this code would have been much easier to refactor because we'd have some actual tests covering it 🤔

[ziqingluo-90]

clang/lib/Analysis/UnsafeBufferUsage.cpp

543

Oh wait, scratch what I said above.

FixableGadgets should be found regardless of whether they are in an opt-out zone. They will be attached to variable declarations. The emission of an Unsafe Buffer diagnostic of a variable declaration only depends on WarningGadgets. So FixableGadgets have nothing to do with opt-out regions.

[ziqingluo-90]

clang/lib/Analysis/UnsafeBufferUsage.cpp

551–555

sorry I didn't think it through when I reply the comments. You are right.
Fixables and variable declarations should be irrelevant to opt-out regions.

I'll rebase this patch w.r.t. https://reviews.llvm.org/D139737 so that we will have tests covering it.

[ziqingluo-90]

Put the implementation of the opt-out pragma completely in Preprocessor.
Add an opt-out pragma test for fix-its.

[NoQ]

Ok I think I'm happy with the patch! Let's give other folks a few days to comment and then land?

The commit message will need to be updated to remove mentions of DiagnosticsEngine.

I'm somewhat surprised that there is no centralized documentation for pragmas, like there is for attributes. But I hope we'll provide enough documentation as part of D136811, which would also supply release notes.

---

It's worth saying out loud that this patch doesn't implement the opt-in pragma (#pragma only_safe_buffers begin ... end) as proposed in D136811, but only the opt-out pragma. We're actually downprioritizing it (possibly dropping entirely) because we've discovered a complete lack of symmetry between these two pragmas. While the opt-out pragma is straightforward, the opt-in pragma is very hard to get right, as it implies the possibility to turn on compiler warnings that were disabled by all other means. Moreover, because warnings enabled this way don't necessarily live between begin and end, this makes it tricky to avoid running unsafe buffer usage analysis on the entire translation unit even when the warning is completely disabled and there are no opt-in pragmas anywhere in the translation unit. So there are a lot of potential scary interactions that we'll have to address before we consider implementing the opt-in pragma. They probably can be addressed, but it'll take us some time to come up with a good design.

[ziqingluo-90]

Change the fix-it test style

clang/include/clang/Basic/Diagnostic.h

private:
 
   /// Used to report a diagnostic that is finally fully formed.
   ///
   /// \returns true if the diagnostic was emitted, false if it was suppressed.
   bool ProcessDiag() {
     return Diags->ProcessDiag(*this);
   }
 
+  // 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.
+  SmallVector<std::pair<SourceLocation, SourceLocation>, 8> SafeBufferOptOutMap;
+
+public:
+  /// \return true iff the given `Loc` is in a "-Wunsafe-buffer-usage" opt-out
+  /// region
+  bool isSafeBufferOptOut(const SourceManager&SourceMgr, const SourceLocation &Loc) const;
+
+  /// Mark a new "-Wunsafe-buffer-usage" opt-out region and set its' start
+  /// location to `StartLoc`
+  void setSafeBufferOptOutStartLoc(const SourceLocation &StartLoc);
+
+  /// Set the end location of the current "-Wunsafe-buffer-usage" opt-out region
+  /// to `EndLoc`
+  void setSafeBufferOptOutEndLoc(const SourceLocation &EndLoc);
+
   /// @name Diagnostic Emission
   /// @{
 protected:
   friend class ASTReader;
   friend class ASTWriter;
 
   // Sema requires access to the following functions because the current design
   // of SFINAE requires it to use its own SemaDiagnosticBuilder, which needs to

clang/include/clang/Basic/DiagnosticLexKinds.td

 
 def err_dep_source_scanner_missing_semi_after_at_import : Error<
   "could not find ';' after @import">;
 def err_dep_source_scanner_unexpected_tokens_at_import : Error<
   "unexpected extra tokens at end of @import declaration">;
 
 }
 
+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

public:
   static void processPathForFileMacro(SmallVectorImpl<char> &Path,
                                       const LangOptions &LangOpts,
                                       const TargetInfo &TI);
 
 private:
   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.
+
+public:
+  /// 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 isInSafeBufferOptOutRegion();
+
+  /// \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 isInSafeBufferOptOutRegion(SourceLocation &StartLoc);
 };
 
 /// Abstract base class that describes a handler that will receive
 /// source ranges for each of the comments encountered in the source file.
 class CommentHandler {
 public:
   virtual ~CommentHandler();
 

clang/lib/Analysis/UnsafeBufferUsage.cpp

 #include "clang/AST/RecursiveASTVisitor.h"
 #include "llvm/ADT/SmallVector.h"
 #include <memory>
 
 using namespace llvm;
 using namespace clang;
 using namespace ast_matchers;
 
+// Returns true iff `Loc` is in a safe-buffer opt-out region
+bool isSafeBufferOptOut(const SourceManager &SM, const SourceLocation &Loc) {
+  return SM.getDiagnostics().isSafeBufferOptOut(SM, Loc);
+}
+
 namespace clang::ast_matchers {
 // A `RecursiveASTVisitor` that traverses all descendants of a given node "n"
 // except for those belonging to a different callable of "n".
 class MatchDescendantVisitor
     : public RecursiveASTVisitor<MatchDescendantVisitor> {
 public:
   typedef RecursiveASTVisitor<MatchDescendantVisitor> VisitorBase;
 

 };
 
 AST_MATCHER_P(Stmt, forEveryDescendant, internal::Matcher<Stmt>, innerMatcher) {
   const DynTypedMatcher &DTM = static_cast<DynTypedMatcher>(innerMatcher);
   
   MatchDescendantVisitor Visitor(&DTM, Finder, Builder, ASTMatchFinder::BK_All);  
   return Visitor.findMatch(DynTypedNode::create(Node));
 }
+
+// Matches a `Stmt` node iff the node is in a safe-buffer opt-out region
+AST_MATCHER(Stmt, notInSafeBufferOptOut) {
+  const SourceManager &SM = Finder->getASTContext().getSourceManager();
+  return !isSafeBufferOptOut(SM, Node.getBeginLoc());
+}
 } // namespace clang::ast_matchers
 
 namespace {
 // Because the analysis revolves around variables and their types, we'll need to
 // track uses of variables (aka DeclRefExprs).
 using DeclUseList = SmallVector<const DeclRefExpr *, 1>;
 
 // Convenience typedef.
#include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def"
   GadgetFinderCallback CB;
 
   // clang-format off
   M.addMatcher(
     stmt(forEveryDescendant(
       stmt(anyOf(
         // Add Gadget::matcher() for every gadget in the registry.
 #define GADGET(x)                                                              \
-        x ## Gadget::matcher().bind(#x),
+        allOf(x ## Gadget::matcher().bind(#x), notInSafeBufferOptOut()),
 #include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def"
         // In parallel, match all DeclRefExprs so that to find out
         // whether there are any uncovered by gadgets.
         declRefExpr(anyOf(hasPointerType(), hasArrayType()), to(varDecl())).bind("any_dre"),
         // Also match DeclStmts because we'll need them when fixing
         // their underlying VarDecls that otherwise don't have
         // any backreferences to DeclStmts.
-        declStmt().bind("any_ds")
+        allOf(declStmt().bind("any_ds"), notInSafeBufferOptOut())
+        // We match all DREs regardless of whether they are in safe-buffer
+        // opt-out region. Because an unclaimed DRE 'd', regardless of where it is,
+        // should prevent a Fixable associated to the same variable as 'd'
+        // from being emitting.
       ))
       // FIXME: Idiomatically there should be a forCallable(equalsNode(D))
       // here, to make sure that the statement actually belongs to the
       // function and not to a nested function. However, forCallable uses
       // ParentMap which can't be used before the AST is fully constructed.
       // The original problem doesn't sound like it needs ParentMap though,
       // maybe there's a more direct solution?
     )),

clang/lib/Basic/Diagnostic.cpp

bool DiagnosticsEngine::EmitCurrentDiagnostic(bool Force) {
 
   // If there was a delayed diagnostic, emit it now.
   if (!Force && DelayedDiagID)
     ReportDelayed();
 
   return Emitted;
 }
 
+bool DiagnosticsEngine::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;
+}
+
+void DiagnosticsEngine::setSafeBufferOptOutStartLoc(
+    const SourceLocation &StartLoc) {
+  if (!SafeBufferOptOutMap.empty()) {
+    auto *PrevRegion = &SafeBufferOptOutMap.back();
+
+    // To check if the pre-condition is satisfied:
+    assert(PrevRegion->first != PrevRegion->second &&
+           "Shall not begin a safe buffer opt-out region before closing the "
+           "previous one.");
+    assert(!hasSourceManager() || getSourceManager().isBeforeInTranslationUnit(
+                                      PrevRegion->second, StartLoc) &&
+                                      "Misordered safe buffer opt-out regions");
+  }
+  // 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(StartLoc, StartLoc);
+}
+
+void DiagnosticsEngine::setSafeBufferOptOutEndLoc(
+    const SourceLocation &EndLoc) {
+  assert(!SafeBufferOptOutMap.empty() &&
+         "Misordered safe buffer opt-out regions");
+
+  auto *CurrRegion = &SafeBufferOptOutMap.back();
+
+  // To check if the pre-condition is satisfied:
+  assert(CurrRegion->first == CurrRegion->second &&
+         "Set end location to a closed safe buffer opt-out region");
+  assert(!hasSourceManager() || getSourceManager().isBeforeInTranslationUnit(
+                                    CurrRegion->first, EndLoc) &&
+                                    "Misordered safe buffer opt-out regions");
+  CurrRegion->second = EndLoc;
+}
+
 DiagnosticConsumer::~DiagnosticConsumer() = default;
 
 void DiagnosticConsumer::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
                                         const Diagnostic &Info) {
   if (!IncludeInDiagnosticCounts())
     return;
 
   if (DiagLevel == DiagnosticsEngine::Warning)

clang/lib/Lex/PPLexerChange.cpp

 
 /// HandleEndOfFile - This callback is invoked when the lexer hits the end of
 /// the current file.  This either returns the EOF token or pops a level off
 /// the include stack and keeps going.
 bool Preprocessor::HandleEndOfFile(Token &Result, bool isEndOfMacro) {
   assert(!CurTokenLexer &&
          "Ending a file when currently in a macro!");
 
+  SourceLocation UnclosedSafeBufferOptOutLoc;
+
+  if (IncludeMacroStack.empty() &&
+      isInSafeBufferOptOutRegion(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;
   if ((LeavingSubmodule || IncludeMacroStack.empty()) &&
       !BuildingSubmoduleStack.empty() &&
       BuildingSubmoduleStack.back().IsPragma) {
     Diag(BuildingSubmoduleStack.back().ImportLoc,
          diag::err_pp_module_begin_without_module_end);

clang/lib/Lex/Pragma.cpp

static void DebugOverflowStack(void (*P)() = nullptr) {
     void (*volatile Self)(void(*P)()) = DebugOverflowStack;
     Self(reinterpret_cast<void(*)()>(Self));
   }
 #ifdef _MSC_VER
     #pragma warning(default : 4717)
 #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:
   const char *Namespace;
 
 public:
   explicit PragmaDiagnosticHandler(const char *NS)
       : PragmaHandler("diagnostic"), Namespace(NS) {}
void Preprocessor::RegisterBuiltinPragmas() {
   auto *ModuleHandler = new PragmaNamespace("module");
   AddPragmaHandler("clang", ModuleHandler);
   ModuleHandler->AddPragma(new PragmaModuleImportHandler());
   ModuleHandler->AddPragma(new PragmaModuleBeginHandler());
   ModuleHandler->AddPragma(new PragmaModuleEndHandler());
   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"));
 
   // MS extensions.
   if (LangOpts.MicrosoftExt) {
     AddPragmaHandler(new PragmaWarningHandler());
     AddPragmaHandler(new PragmaExecCharsetHandler());

clang/lib/Lex/Preprocessor.cpp

void Preprocessor::emitFinalMacroWarning(const Token &Identifier,
   assert(A.FinalAnnotationLoc &&
          "Final macro warning without recorded annotation!");
 
   Diag(Identifier, diag::warn_pragma_final_macro)
       << Identifier.getIdentifierInfo() << (IsUndef ? 0 : 1);
   Diag(*A.FinalAnnotationLoc, diag::note_pp_macro_annotation) << 2;
 }
 
+bool Preprocessor::enterOrExitSafeBufferOptOutRegion(
+    bool isEnter, const SourceLocation &Loc) {
+  if (isEnter) {
+    if (isInSafeBufferOptOutRegion())
+      return true; // invalid enter action
+    InSafeBufferOptOutRegion = true;
+    CurrentSafeBufferOptOutStart = Loc;
+    getDiagnostics().setSafeBufferOptOutStartLoc(Loc);
+  } else {
+    if (!isInSafeBufferOptOutRegion())
+      return true; // invalid enter action
+    InSafeBufferOptOutRegion = false;
+    getDiagnostics().setSafeBufferOptOutEndLoc(Loc);
+  }
+  return false;
+}
+
+bool Preprocessor::isInSafeBufferOptOutRegion() {
+  return InSafeBufferOptOutRegion;
+}
+bool Preprocessor::isInSafeBufferOptOutRegion(SourceLocation &StartLoc) {
+  StartLoc = CurrentSafeBufferOptOutStart;
+  return InSafeBufferOptOutRegion;
+}
+
 ModuleLoader::~ModuleLoader() = default;
 
 CommentHandler::~CommentHandler() = default;
 
 EmptylineHandler::~EmptylineHandler() = default;
 
 CodeCompletionHandler::~CodeCompletionHandler() = default;
 
 void Preprocessor::createPreprocessingRecord() {
   if (Record)
     return;
 
   Record = new PreprocessingRecord(getSourceManager());
   addPPCallbacks(std::unique_ptr<PPCallbacks>(Record));
 }

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

+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -verify %s
+
+void beginUnclosed(int * x) {
+#pragma clang unsafe_buffer_usage begin
+
+#pragma clang unsafe_buffer_usage begin  // expected-error{{already inside '#pragma unsafe_buffer_usage'}}
+  x++;
+#pragma clang unsafe_buffer_usage end
+}
+
+void endUnopened(int *x) {
+#pragma clang unsafe_buffer_usage end    // expected-error{{not currently inside '#pragma unsafe_buffer_usage'}}
+
+#pragma clang unsafe_buffer_usage begin
+  x++;
+#pragma clang unsafe_buffer_usage end
+}
+
+void wrongOption() {
+#pragma clang unsafe_buffer_usage start // expected-error{{Expected 'begin' or 'end'}}
+#pragma clang unsafe_buffer_usage close // expected-error{{Expected 'begin' or 'end'}}
+}
+
+void unclosed(int * p1) {
+#pragma clang unsafe_buffer_usage begin
+// End of the included file will not raise the unclosed region warning:
+#define _INCLUDE_NO_WARN
+#include "warn-unsafe-buffer-usage-pragma.h"
+#pragma clang unsafe_buffer_usage end
+
+// End of this file raises the warning:
+#pragma clang unsafe_buffer_usage begin  // expected-error{{'#pragma unsafe_buffer_usage' was not ended}}
+}

clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma.h

+#ifdef _INCLUDE_NO_WARN
+// the snippet will be included in an opt-out region
+p1++;
+
+#undef _INCLUDE_NO_WARN
+
+#elif defined(_INCLUDE_WARN)
+// the snippet will be included in a location where warnings are expected
+p2++; // expected-note{{used in pointer arithmetic here}}
+#undef _INCLUDE_WARN
+
+#else
+
+#endif

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

+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -Wno-unused-value -verify %s
+
+void basic(int * x) {    // expected-warning{{'x' is an unsafe pointer used for buffer access}}
+  int *p1 = new int[10]; // not to warn
+  int *p2 = new int[10]; // expected-warning{{'p2' is an unsafe pointer used for buffer access}}
+
+#pragma clang unsafe_buffer_usage begin
+  p1[5];  // not to warn
+
+#define _INCLUDE_NO_WARN
+#include "warn-unsafe-buffer-usage-pragma.h" // increment p1 in header
+
+  int *p3 = new int[10]; // expected-warning{{'p3' is an unsafe pointer used for buffer access}}
+
+#pragma clang unsafe_buffer_usage end
+  p2[5]; //expected-note{{used in buffer access here}}
+  p3[5]; //expected-note{{used in buffer access here}}
+  x++;   //expected-note{{used in pointer arithmetic here}}
+#define _INCLUDE_WARN
+#include "warn-unsafe-buffer-usage-pragma.h" // increment p2 in header
+}
+
+
+void withDiagnosticWarning() {
+  int *p1 = new int[10]; // not to warn
+  int *p2 = new int[10]; // expected-warning{{'p2' is an unsafe pointer used for buffer access}}
+
+  // diagnostics in opt-out region
+#pragma clang unsafe_buffer_usage begin
+  p1[5];  // not to warn
+  p2[5];  // not to warn
+#pragma clang diagnostic push
+#pragma clang diagnostic warning "-Wunsafe-buffer-usage"
+  p1[5];  // not to warn
+  p2[5];  // not to warn
+#pragma clang diagnostic warning "-Weverything"
+  p1[5];  // not to warn expected-warning{{expression result unused}}
+  p2[5];  // not to warn expected-warning{{expression result unused}}
+#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
+  p1[5];  // not to warn
+  p2[5];  // not to warn
+#pragma clang unsafe_buffer_usage end
+#pragma clang diagnostic pop
+
+  p2[5]; // expected-note{{used in buffer access here}}
+}
+
+
+void withDiagnosticIgnore() {
+  int *p1 = new int[10]; // not to warn
+  int *p2 = new int[10]; // expected-warning{{'p2' is an unsafe pointer used for buffer access}}
+  int *p3 = new int[10]; // expected-warning{{'p3' is an unsafe pointer used for buffer access}}
+
+#pragma clang unsafe_buffer_usage begin
+  p1[5];  // not to warn
+  p2[5];  // not to warn
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunsafe-buffer-usage"
+  p1[5];  // not to warn
+  p2[5];  // not to warn
+#pragma clang diagnostic ignored "-Weverything"
+  p1[5];  // not to warn
+  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
+  p1[5];  // not to warn
+  p2[5];  // not to warn
+#pragma clang unsafe_buffer_usage end
+#pragma clang diagnostic pop
+
+  p2[5]; // expected-note{{used in buffer access here}}
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunsafe-buffer-usage"
+#pragma clang unsafe_buffer_usage begin
+  p1[5];  // not to warn
+  p2[5];  // not to warn
+#pragma clang unsafe_buffer_usage end
+  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
+#pragma clang diagnostic pop
+
+  p[5]; // not to note since the associated warning is suppressed
+}