[-Wunsafe-buffer-usage] Hide fixits/suggestions behind an extra flag, -fsafe-buffer-usage-suggestions.

[NoQ]

Summary

This patch implements a new clang driver flag -fsafe-buffer-usage-suggestions which allows turning the smart suggestion machine on and off (defaults to off). This is valuable for stability reasons, as the machine is being rapidly improved and we don't want accidental breakages to ruin the build for innocent users. It is also arguably useful in general because it enables separation of concerns between project contributors: some users will actively update the code to conform to the programming model, while others simply want to make sure that they aren't regressing it. Finally, there could be other valid reasons to opt out of suggestions entirely on some codebases (while continuing to enforce -Wunsafe-buffer-usage warnings), such as lack of access to hardened libc++ (or even to the C++ standard library in general) on the target platform.

When the flag is disabled, the unsafe buffer usage analysis is reduced to an extremely minimal mode of operation that contains virtually no smarts: not only it doesn't offer automatic fixits, but also textual suggestions such as "change the type of this variable to std::span to preserve bounds information" are not displayed*, and in fact the machine doesn't even try to blame specific variables in the first place, it simply warns on the operations and leaves everything else to the user. So this flag turns off a lot more of our complex machinery than what we already turn off in presence of say -fno-diagnostic-fixit-info.

The flag is discoverable: when it's off, the warnings are accompanied by a note: telling the user that there's a flag they can use. (Do we really need that? Maybe we should somehow throttle it to reduce noise?)

[NoQ]

Add some nice assertions.

[jkorous]

LGTM.

clang/include/clang/Basic/DiagnosticSemaKinds.td

11828

I like this idea!

Thinking about the questions you raised:

Do we really need that? Maybe we should somehow throttle it to reduce noise?

The only case I can see value in not emitting this note is when the user passes -fno-safe-buffer-usage-suggestions. In that case it can be argued that the user is well aware of the flag since they explicitly turned it off.

That being said I am fine with the current behavior and in either case expect we will tweak this once we get feedback from the users.

[ziqingluo-90]

clang/lib/Analysis/UnsafeBufferUsage.cpp

1614

nitpick: I'm afraid that we will forgot the difference between EmitSuggestions and EmitFixits later if there is no document about it.

[ziqingluo-90]

clang/lib/Sema/AnalysisBasedWarnings.cpp

2210

nitpick:
I was a bit confused by the name of the variable. My understand of
!SuggestSuggestions here means "we are suggesting suggestions now".
Then what does SuggestSuggestions mean?

[t-rasmud]

clang/lib/Sema/AnalysisBasedWarnings.cpp

2164

Was there a reason for naming this variable SuggestSuggestions? Can this be called EmitSuggestions? I think that would make it uniform and easy to follow the code.

[NoQ]

Explain the weird variable name.

Confirm that even though warn_unsafe_buffer_operation and note_unsafe_buffer_operation have a different number of modes, the mismatch doesn't cause problems yet. Add an assert to catch the problem when it starts happening. Add tests to confirm that messages make sense.

clang/include/clang/Basic/DiagnosticSemaKinds.td

11828

The only case I can see value in not emitting this note is when the user passes -fno-safe-buffer-usage-suggestions. In that case it can be argued that the user is well aware of the flag since they explicitly turned it off.

Yeah fair enough. This will need some rework though, because right now -fno-... is purely a Driver flag, so -cc1 invocations aren't even aware of it being passed this way. So it turns from a normal flag to a somewhat quirky flag.

clang/lib/Sema/AnalysisBasedWarnings.cpp

2164

It means "Should I emit a note reminding the user to enable -fsafe-buffer-usage-suggestions if they want suggestions?", so it's quite different from EmitSuggestions (in fact it's the opposite). Added a comment.

2210

Yeah it means "Should I emit a note reminding the user to enable -fsafe-buffer-usage-suggestions if they want suggestions?". Added a comment to the variable.

[NoQ]

Don't recommend enabling suggestions when suggestions are impossible (eg. due to lack of C++20).

[NoQ]

So in a nutshell, this is the intended behavior:

code	diagnostic	EmitFixits && EmitSuggestions	!EmitFixits && EmitSuggestions	!EmitFixits && !EmitSuggestions
void foo(int *p) {	note: 'p' declared here	yes	yes	no
int *q;	warning: 'q' is unsafe pointer to raw buffer	yes	yes	no
	note with fixit: change type of 'q' and 'p' to span to propagate bounds information	yes	yes (no fixit)	no
q = p;	note: bounds information propagates from 'p' to 'q' here	yes	yes	no
q[5] = 10;	note: unsafe raw buffer operation here	yes	yes	yes (warning)
	note: pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions	no	no	yes
}

The first mode, "EmitFixits && EmitSuggestions", is the default behavior before this patch, and the behavior under -fsafe-buffer-usage-suggestions after this patch. The third mode, "!EmitFixits && !EmitSuggestions", is the default behavior after the patch.

The second mode, "!EmitFixits && EmitSuggestions" (the special behavior under -fno-diagnostics-fixit-info) was implemented for two purposes:

• Recover some performance when -fno-diagnostics-fixit-info is passed (don't run the sophisticated fixit machinery if fixits are going to be discarded);
• As a possible workaround for potential crashes in the fixit machinery.

We initially hoped that this mode will make "!EmitSuggestions" unnecessary, but as the table above demonstrates, a very large portion of the fixit machine's logic is still being invoked for the purposes of building notes, even when fixits aren't attached. And we cannot really disable this logic under that compiler flag, given that -fno-diagnostics-fixit-info already has a well-defined meaning which doesn't allow us to change the shape of the diagnostics in response. So we needed a new, more powerful flag, so here we are. I suspect that explicit support for !EmitFixits is no longer necessary. It is now useful for performance purposes only, and we don't any concrete data to support the claim that it's a valuable optimization, nor do we believe that this flag is ever intentionally passed in practice, so I guess I'll go ahead and remove it.

[NoQ]

• Rebase! (I'll update related revisions soon but not immediately, need to make sense out of them first.)
• Eliminate the EmitFixits flag as discussed above (so the middle mode is now gone).

[NoQ]

Before I forget: Update tests that didn't fail due to my patch because they were testing absence of things (warnings or fixits) and my patch only made them more absent. They need to keep testing absence of these things even when the flag is passed.

[NoQ]

Rebase!

[NoQ]

Rebase *correctly*!

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

getUserFillPlaceHolder(StringRef HintTextToUser = "placeholder") const {
     s += " #>";
     return s;
   }
 };
 
 // This function invokes the analysis and allows the caller to react to it
 // through the handler class.
 void checkUnsafeBufferUsage(const Decl *D, UnsafeBufferUsageHandler &Handler,
-                            bool EmitFixits);
+                            bool EmitSuggestions);
 
 namespace internal {
 // Tests if any two `FixItHint`s in `FixIts` conflict.  Two `FixItHint`s
 // conflict if they have overlapping source ranges.
 bool anyConflict(const llvm::SmallVectorImpl<FixItHint> &FixIts,
                  const SourceManager &SM);
 } // namespace internal
 } // end namespace clang
 
 #endif /* LLVM_CLANG_ANALYSIS_ANALYSES_UNSAFEBUFFERUSAGE_H */

clang/include/clang/Basic/DiagnosticOptions.def

 VALUE_DIAGOPT(SpellCheckingLimit, 32, DefaultSpellCheckingLimit)
 /// Limit number of lines shown in a snippet.
 VALUE_DIAGOPT(SnippetLineLimit, 32, DefaultSnippetLineLimit)
 
 VALUE_DIAGOPT(TabStop, 32, DefaultTabStop) /// The distance between tab stops.
 /// Column limit for formatting message diagnostics, or 0 if unused.
 VALUE_DIAGOPT(MessageLength, 32, 0)
 
+DIAGOPT(ShowSafeBufferUsageSuggestions, 1, 0)
+
 #undef DIAGOPT
 #undef ENUM_DIAGOPT
 #undef VALUE_DIAGOPT
 #undef SEMANTIC_DIAGOPT
 #undef SEMANTIC_ENUM_DIAGOPT
 #undef SEMANTIC_VALUE_DIAGOPT

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def warn_unsafe_buffer_operation : Warning<
   "%select{unsafe pointer operation|unsafe pointer arithmetic|"
   "unsafe buffer access|function introduces unsafe buffer manipulation}0">,
   InGroup<UnsafeBufferUsage>, DefaultIgnore;
 def note_unsafe_buffer_operation : Note<
   "used%select{| in pointer arithmetic| in buffer access}0 here">;
 def note_unsafe_buffer_variable_fixit : Note<
   "change type of '%0' to '%select{std::span|std::array|std::span::iterator}1' to preserve bounds information">;
+def note_safe_buffer_usage_suggestions_disabled : Note<
+  "pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions">;
 def err_loongarch_builtin_requires_la32 : Error<
   "this builtin requires target: loongarch32">;
 
 def err_builtin_pass_in_regs_non_class : Error<
   "argument %0 is not an unqualified class type">;
 
 
 // WebAssembly reference type and table diagnostics.
 def err_wasm_reference_pr : Error<
   "%select{pointer|reference}0 to WebAssembly reference type is not allowed">;
 def err_wasm_ca_reference : Error<
   "cannot %select{capture|take address of}0 WebAssembly reference">;
 def err_wasm_funcref_not_wasm : Error<
   "invalid use of '__funcref' keyword outside the WebAssembly triple">;
 } // end of sema component.

clang/include/clang/Driver/Options.td

defm diagnostics_show_note_include_stack : BoolFOption<"diagnostics-show-note-include-stack",
     PosFlag<SetTrue, [], "Display include stacks for diagnostic notes">,
     NegFlag<SetFalse>, BothFlags<[CC1Option]>>;
 def fdiagnostics_format_EQ : Joined<["-"], "fdiagnostics-format=">, Group<f_clang_Group>;
 def fdiagnostics_show_category_EQ : Joined<["-"], "fdiagnostics-show-category=">, Group<f_clang_Group>;
 def fdiagnostics_show_template_tree : Flag<["-"], "fdiagnostics-show-template-tree">,
     Group<f_Group>, Flags<[CC1Option]>,
     HelpText<"Print a template comparison tree for differing templates">,
     MarshallingInfoFlag<DiagnosticOpts<"ShowTemplateTree">>;
+defm safe_buffer_usage_suggestions : BoolFOption<"safe-buffer-usage-suggestions",
+  DiagnosticOpts<"ShowSafeBufferUsageSuggestions">, DefaultFalse,
+  PosFlag<SetTrue, [CC1Option],
+          "Display suggestions to update code associated with -Wunsafe-buffer-usage warnings">,
+  NegFlag<SetFalse>>;
 def fdiscard_value_names : Flag<["-"], "fdiscard-value-names">, Group<f_clang_Group>,
   HelpText<"Discard value names in LLVM IR">, Flags<[NoXarchOption]>;
 def fno_discard_value_names : Flag<["-"], "fno-discard-value-names">, Group<f_clang_Group>,
   HelpText<"Do not discard value names in LLVM IR">, Flags<[NoXarchOption]>;
 defm dollars_in_identifiers : BoolFOption<"dollars-in-identifiers",
   LangOpts<"DollarIdents">, Default<!strconcat("!", asm_preprocessor.KeyPath)>,
   PosFlag<SetTrue, [], "Allow">, NegFlag<SetFalse, [], "Disallow">,
   BothFlags<[CC1Option], " '$' in identifiers">>;

clang/lib/Analysis/UnsafeBufferUsage.cpp

public:
 
   virtual DeclUseList getClaimedVarUseSites() const final {
     return {BaseDeclRefExpr};
   }
 };
 
 /// Scan the function and return a list of gadgets found with provided kits.
 static std::tuple<FixableGadgetList, WarningGadgetList, DeclUseTracker>
-findGadgets(const Decl *D, const UnsafeBufferUsageHandler &Handler) {
+findGadgets(const Decl *D, const UnsafeBufferUsageHandler &Handler,
+            bool EmitSuggestions) {
 
   struct GadgetFinderCallback : MatchFinder::MatchCallback {
     FixableGadgetList FixableGadgets;
     WarningGadgetList WarningGadgets;
     DeclUseTracker Tracker;
 
     void run(const MatchFinder::MatchResult &Result) override {
       // In debug mode, assert that we've found exactly one gadget.
#include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def"
     }
   };
 
   MatchFinder M;
   GadgetFinderCallback CB;
 
   // clang-format off
   M.addMatcher(
-    stmt(eachOf(
-      // A `FixableGadget` matcher and a `WarningGadget` matcher should not disable
-      // each other (they could if they were put in the same `anyOf` group).
-      // We also should make sure no two `FixableGadget` (resp. `WarningGadget`) matchers
-      // match for the same node, so that we can group them
-      // in one `anyOf` group (for better performance via short-circuiting).
-      forEachDescendantStmt(stmt(eachOf(
-#define FIXABLE_GADGET(x)                                                              \
-        x ## Gadget::matcher().bind(#x),
-#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")
-      ))),
+      stmt(
         forEachDescendantEvaluatedStmt(stmt(anyOf(
           // Add Gadget::matcher() for every gadget in the registry.
-#define WARNING_GADGET(x)                                                              \
-        allOf(x ## Gadget::matcher().bind(#x), notInSafeBufferOptOut(&Handler)),
+#define WARNING_GADGET(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.
+            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")
-      ))
-    ))),
+        )))
+    ),
     &CB
   );
   // clang-format on
 
+  if (EmitSuggestions) {
+    // clang-format off
+    M.addMatcher(
+        stmt(
+          forEachDescendantStmt(stmt(eachOf(
+#define FIXABLE_GADGET(x)                                                      \
+            x ## Gadget::matcher().bind(#x),
+#include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def"
+            // Avoid a hanging comma.
+            unless(stmt())
+          )))
+      ),
+      &CB
+    );
+    // clang-format on
+  }
+
   M.match(*D->getBody(), D->getASTContext());
 
   // Gadgets "claim" variables they're responsible for. Once this loop finishes,
   // the tracker will only track DREs that weren't claimed by any gadgets,
   // i.e. not understood by the analysis.
   for (const auto &G : CB.FixableGadgets) {
     for (const auto *DRE : G->getClaimedVarUseSites()) {
       CB.Tracker.claimUse(DRE);
getNaiveStrategy(const llvm::SmallVectorImpl<const VarDecl *> &UnsafeVars) {
   for (const VarDecl *VD : UnsafeVars) {
     S.set(VD, Strategy::Kind::Span);
   }
   return S;
 }
 
 void clang::checkUnsafeBufferUsage(const Decl *D,
                                    UnsafeBufferUsageHandler &Handler,
-                                   bool EmitFixits) {
+                                   bool EmitSuggestions) {
   assert(D && D->getBody());
-
   WarningGadgetSets UnsafeOps;
   FixableGadgetSets FixablesForUnsafeVars;
   DeclUseTracker Tracker;
 
   {
-    auto [FixableGadgets, WarningGadgets, TrackerRes] = findGadgets(D, Handler);
+    auto [FixableGadgets, WarningGadgets, TrackerRes] =
+      findGadgets(D, Handler, EmitSuggestions);
+
+    if (!EmitSuggestions) {
+      // Our job is very easy without suggestions. Just warn about
+      // every problematic operation and consider it done. No need to deal
+      // with fixable gadgets, no need to group operations by variable.
+      for (const auto &G : WarningGadgets) {
+        Handler.handleUnsafeOperation(G->getBaseStmt(),
+                                      /*IsRelatedToDecl=*/false);
+      }
+
+      // This return guarantees that most of the machine doesn't run when
+      // suggestions aren't requested.
+      assert(FixableGadgets.size() == 0 &&
+             "Fixable gadgets found but suggestions not requested!");
+      return;
+    }
+
     UnsafeOps = groupWarningGadgetsByVar(std::move(WarningGadgets));
     FixablesForUnsafeVars = groupFixablesByVar(std::move(FixableGadgets));
     Tracker = std::move(TrackerRes);
   }
 
   std::map<const VarDecl *, FixItList> FixItsForVariable;
 
-  if (EmitFixits) {
-    // Filter out non-local vars and vars with unclaimed DeclRefExpr-s.
-    for (auto it = FixablesForUnsafeVars.byVar.cbegin();
-         it != FixablesForUnsafeVars.byVar.cend();) {
-      // FIXME: Support ParmVarDecl as well.
-      if (!it->first->isLocalVarDecl() || Tracker.hasUnclaimedUses(it->first)) {
-        it = FixablesForUnsafeVars.byVar.erase(it);
-      } else {
-        ++it;
-      }
-    }
 
-    llvm::SmallVector<const VarDecl *, 16> UnsafeVars;
-    for (const auto &[VD, ignore] : FixablesForUnsafeVars.byVar)
-      UnsafeVars.push_back(VD);
 
-    Strategy NaiveStrategy = getNaiveStrategy(UnsafeVars);
-    FixItsForVariable = getFixIts(FixablesForUnsafeVars, NaiveStrategy, Tracker,
-                                  D->getASTContext(), Handler);
 
-    // FIXME Detect overlapping FixIts.
-  }
 
   for (const auto &G : UnsafeOps.noVar) {
     Handler.handleUnsafeOperation(G->getBaseStmt(), /*IsRelatedToDecl=*/false);
   }
 
   for (const auto &[VD, WarningGadgets] : UnsafeOps.byVar) {
-    auto FixItsIt =
-        EmitFixits ? FixItsForVariable.find(VD) : FixItsForVariable.end();
+    auto FixItsIt = FixItsForVariable.find(VD);
     Handler.handleFixableVariable(VD, FixItsIt != FixItsForVariable.end()
                                           ? std::move(FixItsIt->second)
                                           : FixItList{});
     for (const auto &G : WarningGadgets) {
       Handler.handleUnsafeOperation(G->getBaseStmt(), /*IsRelatedToDecl=*/true);
     }
   }
 }

clang/lib/Driver/ToolChains/Clang.cpp

 
   // Forward --vfsoverlay to -cc1.
   for (const Arg *A : Args.filtered(options::OPT_vfsoverlay)) {
     CmdArgs.push_back("--vfsoverlay");
     CmdArgs.push_back(A->getValue());
     A->claim();
   }
 
+  Args.addOptInFlag(CmdArgs, options::OPT_fsafe_buffer_usage_suggestions,
+                    options::OPT_fno_safe_buffer_usage_suggestions);
+
   // Setup statistics file output.
   SmallString<128> StatsFile = getStatsFileName(Args, Output, Input, D);
   if (!StatsFile.empty()) {
     CmdArgs.push_back(Args.MakeArgString(Twine("-stats-file=") + StatsFile));
     if (D.CCPrintInternalStats)
       CmdArgs.push_back("-stats-file-append");
   }
 

clang/lib/Sema/AnalysisBasedWarnings.cpp

 
 //===----------------------------------------------------------------------===//
 // Unsafe buffer usage analysis.
 //===----------------------------------------------------------------------===//
 
 namespace {
 class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
   Sema &S;
+  bool SuggestSuggestions;  // Recommend -fsafe-buffer-usage-suggestions?
 
 public:
-  UnsafeBufferUsageReporter(Sema &S) : S(S) {}
+  UnsafeBufferUsageReporter(Sema &S, bool SuggestSuggestions)
+    : S(S), SuggestSuggestions(SuggestSuggestions) {}
 
   void handleUnsafeOperation(const Stmt *Operation,
                              bool IsRelatedToDecl) override {
     SourceLocation Loc;
     SourceRange Range;
     unsigned MsgParam = 0;
     if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(Operation)) {
       Loc = ASE->getBase()->getExprLoc();
if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(Operation)) {
       if (Op == UO_PreInc || Op == UO_PreDec || Op == UO_PostInc ||
           Op == UO_PostDec) {
         Loc = UO->getSubExpr()->getExprLoc();
         Range = UO->getSubExpr()->getSourceRange();
         MsgParam = 1;
       }
     } else {
       if (isa<CallExpr>(Operation)) {
+        // note_unsafe_buffer_operation doesn't have this mode yet.
+        assert(!IsRelatedToDecl && "Not implemented yet!");
         MsgParam = 3;
       }
       Loc = Operation->getBeginLoc();
       Range = Operation->getSourceRange();
     }
-    if (IsRelatedToDecl)
+    if (IsRelatedToDecl) {
+      assert(!SuggestSuggestions &&
+             "Variables blamed for unsafe buffer usage without suggestions!");
       S.Diag(Loc, diag::note_unsafe_buffer_operation) << MsgParam << Range;
-    else
+    } else {
       S.Diag(Loc, diag::warn_unsafe_buffer_operation) << MsgParam << Range;
+      if (SuggestSuggestions) {
+        S.Diag(Loc, diag::note_safe_buffer_usage_suggestions_disabled);
+      }
+    }
   }
 
   // FIXME: rename to handleUnsafeVariable
   void handleFixableVariable(const VarDecl *Variable,
                              FixItList &&Fixes) override {
+    assert(!SuggestSuggestions &&
+           "Unsafe buffer usage fixits displayed without suggestions!");
     S.Diag(Variable->getLocation(), diag::warn_unsafe_buffer_variable)
         << Variable << (Variable->getType()->isPointerType() ? 0 : 1)
         << Variable->getSourceRange();
     if (!Fixes.empty()) {
       unsigned FixItStrategy = 0; // For now we only has 'std::span' strategy
       const auto &FD = S.Diag(Variable->getLocation(),
                               diag::note_unsafe_buffer_variable_fixit);
 
if (!TU)
     return; // This is unexpected, give up quietly.
 
   DiagnosticsEngine &Diags = S.getDiagnostics();
 
   if (S.hasUncompilableErrorOccurred() || Diags.getIgnoreAllWarnings())
     // exit if having uncompilable errors or ignoring all warnings:
     return;
 
-  // Whether -Wunsafe-buffer-usage should emit fix-its:
-  const bool UnsafeBufferEmitFixits =
-      Diags.getDiagnosticOptions().ShowFixits && S.getLangOpts().CPlusPlus20;
-  UnsafeBufferUsageReporter R(S);
+  DiagnosticOptions &DiagOpts = Diags.getDiagnosticOptions();
+
+  // UnsafeBufferUsage analysis settings.
+  bool UnsafeBufferUsageCanEmitSuggestions = S.getLangOpts().CPlusPlus20;
+  // Should != Can.
+  bool UnsafeBufferUsageShouldEmitSuggestions =
+      UnsafeBufferUsageCanEmitSuggestions &&
+      DiagOpts.ShowSafeBufferUsageSuggestions;
+  bool UnsafeBufferUsageShouldSuggestSuggestions =
+      UnsafeBufferUsageCanEmitSuggestions &&
+      !DiagOpts.ShowSafeBufferUsageSuggestions;
+  UnsafeBufferUsageReporter R(S, UnsafeBufferUsageShouldSuggestSuggestions);
 
   // The Callback function that performs analyses:
   auto CallAnalyzers = [&](const Decl *Node) -> void {
-    // Perform unsafe buffer analysis:
+    // Perform unsafe buffer usage analysis:
     if (!Diags.isIgnored(diag::warn_unsafe_buffer_operation,
                          Node->getBeginLoc()) ||
         !Diags.isIgnored(diag::warn_unsafe_buffer_variable,
-                         Node->getBeginLoc()))
-      clang::checkUnsafeBufferUsage(Node, R, UnsafeBufferEmitFixits);
+                         Node->getBeginLoc())) {
+      clang::checkUnsafeBufferUsage(Node, R,
+                                    UnsafeBufferUsageCanEmitSuggestions);
+    }
 
     // More analysis ...
   };
   // Emit per-function analysis-based warnings that require the whole-TU
   // reasoning. Check if any of them is enabled at all before scanning the AST:
   if (!Diags.isIgnored(diag::warn_unsafe_buffer_operation, SourceLocation()) ||
       !Diags.isIgnored(diag::warn_unsafe_buffer_variable, SourceLocation())) {
     CallableVisitor(CallAnalyzers).TraverseTranslationUnitDecl(TU);

clang/test/SemaCXX/unsafe-buffer-usage-diag-type.cpp

-// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage -verify %s
+// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions -verify %s
 
 namespace localVar {
 void testRefersPtrLocalVarDecl(int i) {
   int * ptr;    // expected-warning{{'ptr' is an unsafe pointer used for buffer access}}
   ptr + i;      // expected-note{{used in pointer arithmetic here}}
   ptr[i];       // expected-note{{used in buffer access here}}
 }
 

clang/test/SemaCXX/warn-unsafe-buffer-usage-crashes.c

-// RUN: %clang_cc1 -Wunsafe-buffer-usage %s -verify %s
+// RUN: %clang_cc1 -Wunsafe-buffer-usage -fsafe-buffer-usage-suggestions \
+// RUN:            %s -verify %s
 
 void gnu_stmtexpr_crash(void) {
   struct A {};
   struct B {
     struct A a;
   };
 
   struct B b = {{
     // This is a statement-expression (GNU extension).
     ({ int x; }) // no-crash // expected-warning{{excess elements in struct initializer}}
   }};
 }

clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-addressof-arraysubscript.cpp

-// RUN: %clang_cc1 -triple=arm-apple -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -triple=arm-apple \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 int f(unsigned long, void *);
 
 [[clang::unsafe_buffer_usage]]
 int unsafe_f(unsigned long, void *);
 
 void address_to_integer(int x) {
   int * p = new int[10];

clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-assign-to-array-subscr-on-ptr.cpp

-// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 // TODO cases where we don't want fixits
 
 // The Fix-It for unsafe operation is trivially empty.
 // In order to test that our machinery recognizes that we can test if the variable declaration gets a Fix-It.
 // If the operation wasn't handled propertly the declaration won't get Fix-It.
 // By testing presence of the declaration Fix-It we indirectly test presence of the trivial Fix-It for its operations.
 void test() {
   int *p = new int[10];
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
   p[5] = 1;
   // CHECK-NOT: fix-it:
 }

clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-deref-simple-ptr-arith.cpp

-// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits -fsyntax-only %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits -fsyntax-only \
+// RUN:            %s 2>&1 | FileCheck %s
 
 // TODO test we don't mess up vertical whitespace
 // TODO test different whitespaces
 // TODO test different contexts
   // when it's on the right side
 
 void basic() {
   int *ptr;

clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp

-// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 typedef int * Int_ptr_t;
 typedef int Int_t;
 
 #define DEFINE_PTR(X) int* ptr = (X);
 
 void local_array_subscript_simple() {
   int tmp;
   int *p = new int[10];

clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp

-// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 void foo(int* v) {
 }
 
 void m1(int* v1, int* v2, int) {
 
 }
 

clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-deref.cpp

-// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 void basic_dereference() {
   int tmp;
   auto p = new int[10];
   // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
   // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
   // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
   tmp = p[5];

clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pre-increment.cpp

-// RUN: %clang_cc1 -triple=arm-apple -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -triple=arm-apple \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 void foo(int * , int *);
 
 void simple() {
   int * p = new int[10];
   // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span<int> p"
   // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
   // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 10}"

clang/test/SemaCXX/warn-unsafe-buffer-usage-function-attr.cpp

-// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -verify %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions -verify %s
 
 [[clang::unsafe_buffer_usage]]
 void deprecatedFunction3();
 
 void deprecatedFunction4(int z);
 
 void someFunction();
 

clang/test/SemaCXX/warn-unsafe-buffer-usage-no-fixits.cpp

-// RUN: %clang_cc1 -x c -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-
-// RUN: %clang_cc1 -x c -std=c89 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -x c -std=gnu89 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -x c -std=iso9899:1990 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-
-// RUN: %clang_cc1 -x c -std=c17 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -x c -std=gnu17 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -x c -std=iso9899:2017 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -x c -std=c2x -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-
-// RUN: %clang_cc1 -x c++ -std=c++98 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -x c++ -std=gnu++98 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -x c++ -std=c++17 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -x c++ -std=gnu++17 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-
-// RUN: %clang_cc1 -x objective-c++ -std=c++98 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -x objective-c++ -std=gnu++98 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -x objective-c++ -std=c++17 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -x objective-c++ -std=gnu++17 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+// RUN: %clang_cc1 -x c -std=c89 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c -std=gnu89 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c -std=iso9899:1990 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+// RUN: %clang_cc1 -x c -std=c17 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c -std=gnu17 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c -std=iso9899:2017 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c -std=c2x -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+// RUN: %clang_cc1 -x c++ -std=c++98 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c++ -std=gnu++98 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c++ -std=c++17 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x c++ -std=gnu++17 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+// RUN: %clang_cc1 -x objective-c++ -std=c++98 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x objective-c++ -std=gnu++98 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x objective-c++ -std=c++17 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -x objective-c++ -std=gnu++17 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 // CHECK-NOT: fix-it:
 
 typedef int * Int_ptr_t;
 typedef int Int_t;
 
 void local_array_subscript_simple() {
   int tmp;

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

-// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -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}:"{"

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

-// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -verify %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions -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
 }

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

-// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -Wno-unused-value -verify %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -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
 

clang/test/SemaCXX/warn-unsafe-buffer-usage-source-ranges.cpp

-// RUN: %clang_cc1 -Wno-everything -Wunsafe-buffer-usage -fdiagnostics-print-source-range-info %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -Wno-everything -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fdiagnostics-print-source-range-info %s 2>&1 | FileCheck %s
 
 void foo(int i) {
   int * ptr;
 
   ptr++;
   // CHECK-DAG: {[[@LINE-1]]:3-[[@LINE-1]]:6}
   ptr--;
   // CHECK-DAG: {[[@LINE-1]]:3-[[@LINE-1]]:6}

clang/test/SemaCXX/warn-unsafe-buffer-usage-suggestions-flag.cpp

+// Test the -cc1 flag. There's no -fno-... option in -cc1 invocations,
+// just the positive option.
+
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -verify=OFF %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -verify=ON %s \
+// RUN:            -fsafe-buffer-usage-suggestions
+
+// Test driver flags. Now there's both -f... and -fno-... to worry about.
+
+// RUN: %clang -fsyntax-only -std=c++20 -Wunsafe-buffer-usage \
+// RUN:        -Xclang -verify=OFF %s
+// RUN: %clang -fsyntax-only -std=c++20 -Wunsafe-buffer-usage \
+// RUN:        -fsafe-buffer-usage-suggestions \
+// RUN:        -Xclang -verify=ON %s
+// RUN: %clang -fsyntax-only -std=c++20 -Wunsafe-buffer-usage \
+// RUN:        -fno-safe-buffer-usage-suggestions \
+// RUN:        -Xclang -verify=OFF %s
+
+// In case of driver flags, last flag takes precedence.
+
+// RUN: %clang -fsyntax-only -std=c++20 -Wunsafe-buffer-usage \
+// RUN:        -fsafe-buffer-usage-suggestions \
+// RUN:        -fno-safe-buffer-usage-suggestions \
+// RUN:        -Xclang -verify=OFF %s
+// RUN: %clang -fsyntax-only -std=c++20 -Wunsafe-buffer-usage \
+// RUN:        -fno-safe-buffer-usage-suggestions \
+// RUN:        -fsafe-buffer-usage-suggestions \
+// RUN:        -Xclang -verify=ON %s
+
+// Passing through -Xclang.
+
+// RUN: %clang -fsyntax-only -std=c++20 -Wunsafe-buffer-usage \
+// RUN:        -Xclang -fsafe-buffer-usage-suggestions \
+// RUN:        -Xclang -verify=ON %s
+
+// -Xclang flags take precedence over driver flags.
+
+// RUN: %clang -fsyntax-only -std=c++20 -Wunsafe-buffer-usage \
+// RUN:        -Xclang -fsafe-buffer-usage-suggestions \
+// RUN:        -fno-safe-buffer-usage-suggestions \
+// RUN:        -Xclang -verify=ON %s
+// RUN: %clang -fsyntax-only -std=c++20 -Wunsafe-buffer-usage \
+// RUN:        -fno-safe-buffer-usage-suggestions \
+// RUN:        -Xclang -fsafe-buffer-usage-suggestions \
+// RUN:        -Xclang -verify=ON %s
+
+[[clang::unsafe_buffer_usage]] void bar(int *);
+
+void foo(int *x) { // \
+  // ON-warning{{'x' is an unsafe pointer used for buffer access}}
+  // FIXME: Better "OFF" warning?
+  x[5] = 10; // \
+  // ON-note    {{used in buffer access here}} \
+  // OFF-warning{{unsafe buffer access}} \
+  // OFF-note   {{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
+
+  x += 5;  // \
+  // ON-note    {{used in pointer arithmetic here}} \
+  // OFF-warning{{unsafe pointer arithmetic}} \
+  // OFF-note   {{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
+
+  bar(x);  // \
+  // ON-warning{{function introduces unsafe buffer manipulation}} \
+  // OFF-warning{{function introduces unsafe buffer manipulation}} \
+  // OFF-note   {{pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions}}
+}

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

-// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage -fblocks -include %s -verify %s
+// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage \
+// RUN:            -fsafe-buffer-usage-suggestions \
+// RUN:            -fblocks -include %s -verify %s
 
 // RUN: %clang -x c++ -fsyntax-only -fblocks -include %s %s 2>&1 | FileCheck --allow-empty %s
 // RUN: %clang_cc1 -std=c++11 -fblocks -include %s %s 2>&1 | FileCheck --allow-empty %s
 // RUN: %clang_cc1 -std=c++20 -fblocks -include %s %s 2>&1 | FileCheck --allow-empty %s
 // CHECK-NOT: [-Wunsafe-buffer-usage]
 
 #ifndef INCLUDED
 #define INCLUDED