[-Wunsafe-buffer-usage] Add a facility for debugging low fixit coverage.

[t-rasmud]

Summary

This patch adds extra notes to -Wunsafe-buffer-usage warnings, which explain why a fixit wasn't produced. When applied to a large body of real-world code, it'll help us gather statistics that will help us figure out which fixable gadgets (or other features of the fixit machine) to "invest" into.

This is a debugging facility intended for developer use only; it is activated by passing -mllvm -debug-only=SafeBuffers to clang, so it's carefully hidden and undiscoverable, and it's only available in builds with assertions.

Offline we've identified the following sources of false negatives which these notes can help us categorize:

1. unsafe operation not performed on a supported kind of variable (eg. member variable);
2. use site of the unsafe variable not claimed by any fixable gadgets (so we need to cover it with a new fixable);
3. one of the "implicated" variables has unclaimed uses (so we can't build the implication graph);
4. fixit generation for the declaration of the variable has failed (eg. declaration is in a macro);
5. fixit generation for one of the fixable gadgets has failed (eg. the use is in a macro).

Currently this patch covers #5; it probably makes sense to cover all of these except maybe #1 (this one's usually obvious).

[NoQ]

clang/lib/Analysis/UnsafeBufferUsage.cpp

577–581

I changed this to make F->getBaseStmt() available more often, but ran into more problems of this kind that were much more strange than this one. I'll probably investigate more.

[NoQ]

private: => public:

(for some reason it didn't complain until I did a full rebuild)

[NoQ]

Add the other missing base statement. Debugger was acting weirdly so I thought we had bigger problems, but it's just the other thing missing. It's likely that we ultimately want a better interface for that anyway.

[t-rasmud]

I will add debug notes for the rest of the cases not addressed in this patch.

[t-rasmud]

This patch addresses cases 2, 3, and 4 described in the summary (i.e) adds debug notes for unclaimed uses of variables and for failed fixit generation of variable declarations.

[NoQ]

Awesome!!

Did you try running it on some real code? Does this actually cover most cases? (I suspect that (1.) is going to be the most popular case, but that's also the easiest case to diagnose visually. We might still want a note if we wanted to prioritize among non-variables, but that's some work, maybe we can get back to this later.)

I have a few minor nitpicks, but generally LGTM!

clang/lib/Analysis/UnsafeBufferUsage.cpp

1755–1756

We might want to silence -Wunused-parameter in release builds.

Maybe it's better to put the entire parameter under #ifndef NDEBUG, but it's definitely more clumsy.

There's also __attribute__((unused)) aka [[maybe_unused]] but it looks like we're not supposed to use it in LLVM for variables:

173 // Some compilers warn about unused functions. When a function is sometimes
174 // used or not depending on build settings (e.g. a function only called from
175 // within "assert"), this attribute can be used to suppress such warnings.
176 //
177 // However, it shouldn't be used for unused *variables*, as those have a much
178 // more portable solution:
179 //   (void)unused_var_name;
180 // Prefer cast-to-void wherever it is sufficient.
181 #if __has_attribute(unused)
182 #define LLVM_ATTRIBUTE_UNUSED __attribute__((__unused__))
183 #else
184 #define LLVM_ATTRIBUTE_UNUSED
185 #endif

1771–1772

This comment is probably duplicated everywhere by accident; it doesn't make much sense in most of these places.

1796

2353

getName() crashes on various anonymous variables, whereas getNameAsString() always gives an answer (even if it's not always a "good" answer), which is more suitable for diagnostics.

2354

Do you want to include a "failde to produce fixit..." text in this message and the next message, for consistency?

[NoQ]

clang/lib/Analysis/UnsafeBufferUsage.cpp

2174–2175

This is my original comment right? It's actually also outdated and should be removed, I already erased the runtime check, and I already moved the architectural recommendation to implementation of getBaseStmt() in individual gadgets for which it doesn't make sense.

[ziqingluo-90]

This is a lot of work, thank you @t-rasmud & @NoQ !

I have a minor suggestion: can we use some macros to make the debug stub even shorter?
The prefix "failed to produce fixit for declaration" is used in many places so probably we do not have to repeat it everywhere. And, maybe some prefixes could be a bit more blurry so that they can be shared. For example, we can just replace "failed to produce fixit for parm var decl" with "failed to produce fixit for declaration". We have source location and more specific message attached to the note so we are not losing information I think.

I'm imagining something like this:

#define DEBUG_NOTE_DECL_FAIL(D, Msg)  \
Handler.addDebugNoteForVar((D), (D)->getBeginLoc(),  "failed to produce fixit for declaration "##Msg)

#define DEBUG_NOTE_GADGET_FAIL(Gadget, Msg)  ...

Does it make sense to you?

[t-rasmud]

In D154880#4533035, @ziqingluo-90 wrote:

This is a lot of work, thank you @t-rasmud & @NoQ !

I have a minor suggestion: can we use some macros to make the debug stub even shorter?
The prefix "failed to produce fixit for declaration" is used in many places so probably we do not have to repeat it everywhere. And, maybe some prefixes could be a bit more blurry so that they can be shared. For example, we can just replace "failed to produce fixit for parm var decl" with "failed to produce fixit for declaration". We have source location and more specific message attached to the note so we are not losing information I think.

I'm imagining something like this:

#define DEBUG_NOTE_DECL_FAIL(D, Msg)  \
Handler.addDebugNoteForVar((D), (D)->getBeginLoc(),  "failed to produce fixit for declaration "##Msg)

#define DEBUG_NOTE_GADGET_FAIL(Gadget, Msg)  ...

Does it make sense to you?

I like this suggestion. I've made changes to replace Handler.addDebugNoteForVar for declarations. The Gadget case appears just once as of now, so I've left it as is.

[NoQ]

clang/lib/Analysis/UnsafeBufferUsage.cpp

1733–1734

Ooo that's actually really nice! Maybe you can go even further and add this extra harness, so that to eliminate the need for #ifndef NDEBUG at every use.

[NoQ]

LGTM! I'm excited to learn what this new facility discovers!

clang/lib/Analysis/UnsafeBufferUsage.cpp

1733–1738

An extra bit of paranoia.

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

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_ANALYSIS_ANALYSES_UNSAFEBUFFERUSAGE_H
 #define LLVM_CLANG_ANALYSIS_ANALYSES_UNSAFEBUFFERUSAGE_H
 
 #include "clang/AST/Decl.h"
 #include "clang/AST/Stmt.h"
+#include "llvm/Support/Debug.h"
 
 namespace clang {
 
 using DefMapTy = llvm::DenseMap<const VarDecl *, std::vector<const VarDecl *>>;
 
 /// The interface that lets the caller handle unsafe buffer usage analysis
 /// results by overriding this class's handle... methods.
 class UnsafeBufferUsageHandler {
+#ifndef NDEBUG
+public:
+  // A self-debugging facility that you can use to notify the user when
+  // suggestions or fixits are incomplete.
+  // Uses std::function to avoid computing the message when it won't
+  // actually be displayed.
+  using DebugNote = std::pair<SourceLocation, std::string>;
+  using DebugNoteList = std::vector<DebugNote>;
+  using DebugNoteByVar = std::map<const VarDecl *, DebugNoteList>;
+  DebugNoteByVar DebugNotesByVar;
+#endif
+
 public:
   UnsafeBufferUsageHandler() = default;
   virtual ~UnsafeBufferUsageHandler() = default;
 
   /// This analyses produces large fixits that are organized into lists
   /// of primitive fixits (individual insertions/removals/replacements).
   using FixItList = llvm::SmallVectorImpl<FixItHint>;
 
   /// Invoked when an unsafe operation over raw pointers is found.
   virtual void handleUnsafeOperation(const Stmt *Operation,
                                      bool IsRelatedToDecl) = 0;
 
   /// Invoked when a fix is suggested against a variable. This function groups
   /// all variables that must be fixed together (i.e their types must be changed to the
   /// same target type to prevent type mismatches) into a single fixit.
   virtual void handleUnsafeVariableGroup(const VarDecl *Variable,
                                          const DefMapTy &VarGrpMap,
                                          FixItList &&Fixes) = 0;
 
+#ifndef NDEBUG
+public:
+  bool areDebugNotesRequested() {
+    DEBUG_WITH_TYPE("SafeBuffers", return true);
+    return false;
+  }
+
+  void addDebugNoteForVar(const VarDecl *VD, SourceLocation Loc,
+                          std::string Text) {
+    if (areDebugNotesRequested())
+      DebugNotesByVar[VD].push_back(std::make_pair(Loc, Text));
+  }
+
+  void clearDebugNotes() {
+    if (areDebugNotesRequested())
+      DebugNotesByVar.clear();
+  }
+#endif
+
+public:
   /// Returns a reference to the `Preprocessor`:
   virtual bool isSafeBufferOptOut(const SourceLocation &Loc) const = 0;
 
   virtual std::string
   getUnsafeBufferUsageAttributeTextAt(SourceLocation Loc,
                                       StringRef WSSuffix = "") const = 0;
 };
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

def warn_unsafe_buffer_operation : Warning<
   "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_group : Note<
   "change type of %0 to '%select{std::span|std::array|std::span::iterator}1' to preserve bounds information%select{|, and change %2 to '%select{std::span|std::array|std::span::iterator}1' to propagate bounds information between them}3">;
 def note_safe_buffer_usage_suggestions_disabled : Note<
   "pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions">;
+#ifndef NDEBUG
+// Not a user-facing diagnostic. Useful for debugging false negatives in
+// -fsafe-buffer-usage-suggestions (i.e. lack of -Wunsafe-buffer-usage fixits).
+def note_safe_buffer_debug_mode : Note<"safe buffers debug: %0">;
+#endif
+
 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.

clang/lib/Analysis/UnsafeBufferUsage.cpp

#include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def"
   /// Useful for implementing the static matcher() methods
   /// that are expected from all non-abstract subclasses.
   using Matcher = decltype(stmt());
 
   Gadget(Kind K) : K(K) {}
 
   Kind getKind() const { return K; }
 
+#ifndef NDEBUG
+  StringRef getDebugName() const {
+    switch (K) {
+#define GADGET(x) case Kind::x: return #x;
+#include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def"
+    }
+  }
+#endif
+
   virtual bool isWarningGadget() const = 0;
   virtual const Stmt *getBaseStmt() const = 0;
 
   /// Returns the list of pointer-type variables on which this gadget performs
   /// its operation. Typically, there's only one variable. This isn't a list
   /// of all DeclRefExprs in the gadget's AST!
   virtual DeclUseList getClaimedVarUseSites() const = 0;
 
auto PtrInitStmt = declStmt(hasSingleDecl(varDecl(
                                                   bind(PointerInitRHSTag)))).
                                               bind(PointerInitLHSTag)));
 
     return stmt(PtrInitStmt);
   }
 
   virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
 
-  virtual const Stmt *getBaseStmt() const override { return nullptr; }
+  virtual const Stmt *getBaseStmt() const override {
+    // FIXME: This needs to be the entire DeclStmt, assuming that this method
+    // makes sense at all on a FixableGadget.
+    return PtrInitRHS;
+  }
 
   virtual DeclUseList getClaimedVarUseSites() const override {
     return DeclUseList{PtrInitRHS};
   }
 
   virtual std::optional<std::pair<const VarDecl *, const VarDecl *>>
   getStrategyImplications() const override {
       return std::make_pair(PtrInitLHS,
auto PtrAssignExpr = binaryOperator(allOf(hasOperatorName("="),
                                                       to(varDecl())).
                                           bind(PointerAssignLHSTag))));
 
     return stmt(isInUnspecifiedUntypedContext(PtrAssignExpr));
   }
 
   virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
 
-  virtual const Stmt *getBaseStmt() const override { return nullptr; }
+  virtual const Stmt *getBaseStmt() const override {
+    // FIXME: This should be the binary operator, assuming that this method
+    // makes sense at all on a FixableGadget.
+    return PtrLHS;
+  }
 
   virtual DeclUseList getClaimedVarUseSites() const override {
     return DeclUseList{PtrLHS, PtrRHS};
   }
 
   virtual std::optional<std::pair<const VarDecl *, const VarDecl *>>
   getStrategyImplications() const override {
     return std::make_pair(cast<VarDecl>(PtrLHS->getDecl()),
public:
   // A variable is unclaimed if at least one use is unclaimed.
   bool hasUnclaimedUses(const VarDecl *VD) const {
     // FIXME: Can this be less linear? Maybe maintain a map from VDs to DREs?
     return any_of(*Uses, [VD](const DeclRefExpr *DRE) {
       return DRE->getDecl()->getCanonicalDecl() == VD->getCanonicalDecl();
     });
   }
 
+  UseSetTy getUnclaimedUses(const VarDecl *VD) const {
+    UseSetTy ReturnSet;
+    for (auto use : *Uses) {
+      if (use->getDecl()->getCanonicalDecl() == VD->getCanonicalDecl()) {
+        ReturnSet.insert(use);
+      }
+    }
+    return ReturnSet;
+  }
+  
   void discoverDecl(const DeclStmt *DS) {
     for (const Decl *D : DS->decls()) {
       if (const auto *VD = dyn_cast<VarDecl>(D)) {
         // FIXME: Assertion temporarily disabled due to a bug in
         // ASTMatcher internal behavior in presence of GNU
         // statement-expressions. We need to properly investigate this
         // because it can screw up our algorithm in other ways.
         // assert(Defs.count(VD) == 0 && "Definition already discovered!");
populateInitializerFixItWithSpan(const Expr *Init, ASTContext &Ctx,
 
   StrBuffer.append(", ");
   StrBuffer.append(ExtentText);
   StrBuffer.append("}");
   FixIts.push_back(FixItHint::CreateInsertion(*LocPassInit, StrBuffer.str()));
   return FixIts;
 }
 
+#ifndef NDEBUG
+#define DEBUG_NOTE_DECL_FAIL(D, Msg)  \
+Handler.addDebugNoteForVar((D), (D)->getBeginLoc(), "failed to produce fixit for declaration '" + (D)->getNameAsString() + "'" + (Msg))
+#else
+#define DEBUG_NOTE_DECL_FAIL(D, Msg)
+#endif
+
 // For a `VarDecl` of the form `T  * var (= Init)?`, this
 // function generates a fix-it for the declaration, which re-declares `var` to
 // be of `span<T>` type and transforms the initializer, if present, to a span
 // constructor---`span<T> var {Init, Extent}`, where `Extent` may need the user
 // to fill in.
 //
 // FIXME: support Multi-level pointers
 //
 // Parameters:
 //   `D` a pointer the variable declaration node
 //   `Ctx` a reference to the ASTContext
 // Returns:
 //    the generated fix-it
 static FixItList fixVarDeclWithSpan(const VarDecl *D, ASTContext &Ctx,
-                                    const StringRef UserFillPlaceHolder) {
+                                    const StringRef UserFillPlaceHolder,
+                                    UnsafeBufferUsageHandler &Handler) {
+  (void)Handler; // Suppress unused variable warning in release builds.
   const QualType &SpanEltT = D->getType()->getPointeeType();
   assert(!SpanEltT.isNull() && "Trying to fix a non-pointer type variable!");
 
   FixItList FixIts{};
   std::optional<SourceLocation>
       ReplacementLastLoc; // the inclusive end location of the replacement
   const SourceManager &SM = Ctx.getSourceManager();
 
   if (const Expr *Init = D->getInit()) {
     FixItList InitFixIts =
         populateInitializerFixItWithSpan(Init, Ctx, UserFillPlaceHolder);
 
-    if (InitFixIts.empty())
+    if (InitFixIts.empty()) {
+      DEBUG_NOTE_DECL_FAIL(D, " : empty initializer");
       return {};
+    }
 
     // The loc right before the initializer:
     ReplacementLastLoc = Init->getBeginLoc().getLocWithOffset(-1);
     FixIts.insert(FixIts.end(), std::make_move_iterator(InitFixIts.begin()),
                   std::make_move_iterator(InitFixIts.end()));
   } else
     ReplacementLastLoc = getEndCharLoc(D, SM, Ctx.getLangOpts());
 
   // Producing fix-it for variable declaration---make `D` to be of span type:
   SmallString<32> Replacement;
   raw_svector_ostream OS(Replacement);
 
   OS << "std::span<" << SpanEltT.getAsString() << "> " << D->getName();
 
-  if (!ReplacementLastLoc)
+  if (!ReplacementLastLoc) {
+    DEBUG_NOTE_DECL_FAIL(D, " : failed to get end char loc (macro)");
     return {};
+  }
 
   FixIts.push_back(FixItHint::CreateReplacement(
       SourceRange{D->getBeginLoc(), *ReplacementLastLoc}, OS.str()));
   return FixIts;
 }
 
 static bool hasConflictingOverload(const FunctionDecl *FD) {
   return !FD->getDeclContext()->lookup(FD->getDeclName()).isSingleResult();
 }
 
 // For a `FunDecl`, one of whose `ParmVarDecl`s is being changed to have a new
 // type, this function produces fix-its to make the change self-contained.  Let
 // 'F' be the entity defined by the original `FunDecl` and "NewF" be the entity
 // defined by the `FunDecl` after the change to the parameter.  Fix-its produced
createOverloadsForFixedParams(unsigned ParmIdx, StringRef NewTyText,
   return FixIts;
 }
 
 // To fix a `ParmVarDecl` to be of `std::span` type.  In addition, creating a
 // new overload of the function so that the change is self-contained (see
 // `createOverloadsForFixedParams`).
 static FixItList fixParamWithSpan(const ParmVarDecl *PVD, const ASTContext &Ctx,
                                   UnsafeBufferUsageHandler &Handler) {
-  if (PVD->hasDefaultArg())
+  if (PVD->hasDefaultArg()) {
     // FIXME: generate fix-its for default values:
+    DEBUG_NOTE_DECL_FAIL(PVD, " : has default arg");
     return {};
+  }
+  
   assert(PVD->getType()->isPointerType());
   auto *FD = dyn_cast<FunctionDecl>(PVD->getDeclContext());
 
-  if (!FD)
+  if (!FD) {
+    DEBUG_NOTE_DECL_FAIL(PVD, " : invalid func decl");
     return {};
+  }
 
   std::optional<Qualifiers> PteTyQualifiers = std::nullopt;
   std::optional<std::string> PteTyText = getPointeeTypeText(
       PVD, Ctx.getSourceManager(), Ctx.getLangOpts(), &PteTyQualifiers);
 
-  if (!PteTyText)
+  if (!PteTyText) {
+    DEBUG_NOTE_DECL_FAIL(PVD, " : invalid pointee type");
     return {};
+  }
 
   std::optional<StringRef> PVDNameText = PVD->getIdentifier()->getName();
 
-  if (!PVDNameText)
+  if (!PVDNameText) {
+    DEBUG_NOTE_DECL_FAIL(PVD, " : invalid identifier name");
     return {};
+  }
 
   std::string SpanOpen = "std::span<";
   std::string SpanClose = ">";
   std::string SpanTyText;
   std::stringstream SS;
 
   SS << SpanOpen << *PteTyText;
   // Append qualifiers to span element type:
for (auto *ParmIter : FD->parameters()) {
     ParmIdx++;
   }
   if (ParmIdx < FD->getNumParams())
     if (auto OverloadFix = createOverloadsForFixedParams(ParmIdx, SpanTyText,
                                                          FD, Ctx, Handler)) {
       Fixes.append(*OverloadFix);
       return Fixes;
     }
+  DEBUG_NOTE_DECL_FAIL(PVD, " : invalid number of parameters");
   return {};
 }
 
 static FixItList fixVariableWithSpan(const VarDecl *VD,
                                      const DeclUseTracker &Tracker,
                                      ASTContext &Ctx,
                                      UnsafeBufferUsageHandler &Handler) {
   const DeclStmt *DS = Tracker.lookupDecl(VD);
   assert(DS && "Fixing non-local variables not implemented yet!");
   if (!DS->isSingleDecl()) {
     // FIXME: to support handling multiple `VarDecl`s in a single `DeclStmt`
+    DEBUG_NOTE_DECL_FAIL(VD, " : multiple VarDecls");
     return {};
   }
   // Currently DS is an unused variable but we'll need it when
   // non-single decls are implemented, where the pointee type name
   // and the '*' are spread around the place.
   (void)DS;
 
   // FIXME: handle cases where DS has multiple declarations
-  return fixVarDeclWithSpan(VD, Ctx, getUserFillPlaceHolder());
+  return fixVarDeclWithSpan(VD, Ctx, getUserFillPlaceHolder(),
+                            Handler);
 }
 
 // TODO: we should be consistent to use `std::nullopt` to represent no-fix due
 // to any unexpected problem.
 static FixItList
 fixVariable(const VarDecl *VD, Strategy::Kind K,
             /* The function decl under analysis */ const Decl *D,
             const DeclUseTracker &Tracker, ASTContext &Ctx,
             UnsafeBufferUsageHandler &Handler) {
   if (const auto *PVD = dyn_cast<ParmVarDecl>(VD)) {
     auto *FD = dyn_cast<clang::FunctionDecl>(PVD->getDeclContext());
-    if (!FD || FD != D)
+    if (!FD || FD != D) {
       // `FD != D` means that `PVD` belongs to a function that is not being
       // analyzed currently.  Thus `FD` may not be complete.
+      DEBUG_NOTE_DECL_FAIL(VD, " : function not currently analyzed");
       return {};
+    }
 
     // TODO If function has a try block we can't change params unless we check
     // also its catch block for their use.
     // FIXME We might support static class methods, some select methods,
     // operators and possibly lamdas.
     if (FD->isMain() || FD->isConstexpr() ||
         FD->getTemplatedKind() != FunctionDecl::TemplatedKind::TK_NonTemplate ||
         FD->isVariadic() ||
         // also covers call-operator of lamdas
         isa<CXXMethodDecl>(FD) ||
         // skip when the function body is a try-block
         (FD->hasBody() && isa<CXXTryStmt>(FD->getBody())) ||
-        FD->isOverloadedOperator())
+        FD->isOverloadedOperator()) {
+      DEBUG_NOTE_DECL_FAIL(VD, " : unsupported function decl");
       return {}; // TODO test all these cases
     }
+  }
 
   switch (K) {
   case Strategy::Kind::Span: {
     if (VD->getType()->isPointerType()) {
       if (const auto *PVD = dyn_cast<ParmVarDecl>(VD))
         return fixParamWithSpan(PVD, Ctx, Handler);
 
       if (VD->isLocalVarDecl())
         return fixVariableWithSpan(VD, Tracker, Ctx, Handler);
     }
+    DEBUG_NOTE_DECL_FAIL(VD, " : not a pointer");
     return {};
   }
   case Strategy::Kind::Iterator:
   case Strategy::Kind::Array:
   case Strategy::Kind::Vector:
     llvm_unreachable("Strategy not implemented yet!");
   case Strategy::Kind::Wontfix:
     llvm_unreachable("Invalid strategy!");
if (FixItsForVariable[VD].empty()) {
       FixItsForVariable.erase(VD);
       continue;
     }
     bool ImpossibleToFix = false;
     llvm::SmallVector<FixItHint, 16> FixItsForVD;
     for (const auto &F : Fixables) {
       std::optional<FixItList> Fixits = F->getFixits(S);
       if (!Fixits) {
+#ifndef NDEBUG
+        Handler.addDebugNoteForVar(
+            VD, F->getBaseStmt()->getBeginLoc(),
+            ("gadget '" + F->getDebugName() + "' refused to produce a fix")
+                .str());
+#endif
         ImpossibleToFix = true;
         break;
       } else {
         const FixItList CorrectFixes = Fixits.value();
         FixItsForVD.insert(FixItsForVD.end(), CorrectFixes.begin(),
                            CorrectFixes.end());
       }
     }
for (const VarDecl *VD : UnsafeVars) {
     S.set(VD, Strategy::Kind::Span);
   }
   return S;
 }
 
 void clang::checkUnsafeBufferUsage(const Decl *D,
                                    UnsafeBufferUsageHandler &Handler,
                                    bool EmitSuggestions) {
+#ifndef NDEBUG
+  Handler.clearDebugNotes();
+#endif
+
   assert(D && D->getBody());
 
   // We do not want to visit a Lambda expression defined inside a method independently.
   // Instead, it should be visited along with the outer method.
   if (const auto *fd = dyn_cast<CXXMethodDecl>(D)) {
     if (fd->getParent()->isLambda() && fd->getParent()->isLocalClass())
       return;
   }
#endif
 
   std::map<const VarDecl *, FixItList> FixItsForVariableGroup;
   DefMapTy VariableGroupsMap{};
 
   // Filter out non-local vars and vars with unclaimed DeclRefExpr-s.
   for (auto it = FixablesForAllVars.byVar.cbegin();
        it != FixablesForAllVars.byVar.cend();) {
       // FIXME: need to deal with global variables later
-      if ((!it->first->isLocalVarDecl() && !isa<ParmVarDecl>(it->first)) ||
-          Tracker.hasUnclaimedUses(it->first) || it->first->isInitCapture()) {
+      if ((!it->first->isLocalVarDecl() && !isa<ParmVarDecl>(it->first))) {
+#ifndef NDEBUG
+          Handler.addDebugNoteForVar(
+              it->first, it->first->getBeginLoc(),
+              ("failed to produce fixit for '" + it->first->getNameAsString() +
+               "' : neither local nor a parameter"));
+#endif
+        it = FixablesForAllVars.byVar.erase(it);
+      } else if (Tracker.hasUnclaimedUses(it->first)) {
+#ifndef NDEBUG
+        auto AllUnclaimed = Tracker.getUnclaimedUses(it->first);
+        for (auto UnclaimedDRE : AllUnclaimed) {
+          Handler.addDebugNoteForVar(
+              it->first, UnclaimedDRE->getBeginLoc(),
+                                     ("failed to produce fixit for '" + it->first->getNameAsString() +
+                                      "' : has an unclaimed use"));
+          }
+#endif
+        it = FixablesForAllVars.byVar.erase(it);
+      } else if (it->first->isInitCapture()) {
+#ifndef NDEBUG
+        Handler.addDebugNoteForVar(
+            it->first, it->first->getBeginLoc(),
+                                   ("failed to produce fixit for '" + it->first->getNameAsString() +
+                                    "' : init capture"));
+#endif
         it = FixablesForAllVars.byVar.erase(it);
-    } else {
+      }else {
       ++it;
     }
   }
 
   llvm::SmallVector<const VarDecl *, 16> UnsafeVars;
   for (const auto &[VD, ignore] : FixablesForAllVars.byVar)
     UnsafeVars.push_back(VD);
 

clang/lib/Sema/AnalysisBasedWarnings.cpp

if (!Fixes.empty()) {
         FD << AllVars << 1;
       } else {
         FD << "" << 0;
       }
 
       for (const auto &F : Fixes)
         FD << F;
     }
+
+#ifndef NDEBUG
+    if (areDebugNotesRequested())
+      for (const DebugNote &Note: DebugNotesByVar[Variable])
+        S.Diag(Note.first, diag::note_safe_buffer_debug_mode) << Note.second;
+#endif
   }
 
   bool isSafeBufferOptOut(const SourceLocation &Loc) const override {
     return S.PP.isSafeBufferOptOut(S.getSourceManager(), Loc);
   }
 
   // Returns the text representation of clang::unsafe_buffer_usage attribute.
   // `WSSuffix` holds customized "white-space"s, e.g., newline or whilespace

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

+// RUN: %clang_cc1 -Wunsafe-buffer-usage -fsafe-buffer-usage-suggestions \
+// RUN:            -std=c++20 -verify=expected %s
+// RUN: %clang_cc1 -Wunsafe-buffer-usage -fsafe-buffer-usage-suggestions \
+// RUN:            -mllvm -debug-only=SafeBuffers \
+// RUN:            -std=c++20 -verify=expected,debug %s
+
+// A generic -debug would also enable our notes. This is probably fine.
+//
+// RUN: %clang_cc1 -Wunsafe-buffer-usage -fsafe-buffer-usage-suggestions \
+// RUN:            -std=c++20 -mllvm -debug \
+// RUN:            -verify=expected,debug %s
+
+// This test file checks the behavior under the assumption that no fixits
+// were emitted for the test cases. If -Wunsafe-buffer-usage is improved
+// to support these cases (thus failing the test), the test should be changed
+// to showcase a different unsupported example.
+//
+// RUN: %clang_cc1 -Wunsafe-buffer-usage -fsafe-buffer-usage-suggestions \
+// RUN:            -mllvm -debug-only=SafeBuffers \
+// RUN:            -std=c++20 -fdiagnostics-parseable-fixits %s \
+// RUN:            2>&1 | FileCheck %s
+// CHECK-NOT: fix-it:
+
+// This debugging facility is only available in debug builds.
+//
+// REQUIRES: asserts
+
+void foo() {
+  int *x = new int[10]; // expected-warning{{'x' is an unsafe pointer used for buffer access}}
+  x[5] = 10;            // expected-note{{used in buffer access here}}
+  int z = x[-1];        // expected-note{{used in buffer access here}} \
+                        // debug-note{{safe buffers debug: gadget 'ULCArraySubscript' refused to produce a fix}}
+}
+
+void failed_decl() {
+  int a[10];  // expected-warning{{'a' is an unsafe buffer that does not perform bounds checks}} \
+              // debug-note{{safe buffers debug: failed to produce fixit for declaration 'a' : not a pointer}}
+  
+  for (int i = 0; i < 10; i++) {
+    a[i] = i;  // expected-note{{used in buffer access here}}
+  }
+}
+
+void failed_multiple_decl() {
+  int *a = new int[4], b;  // expected-warning{{'a' is an unsafe pointer used for buffer access}} \
+                          // debug-note{{safe buffers debug: failed to produce fixit for declaration 'a' : multiple VarDecls}}
+  a[4] = 3;  // expected-note{{used in buffer access here}}
+}
+
+void failed_param_var_decl(int *a =new int[3]) {  // expected-warning{{'a' is an unsafe pointer used for buffer access}} \
+  // debug-note{{safe buffers debug: failed to produce fixit for declaration 'a' : has default arg}}
+  a[4] = 6;  // expected-note{{used in buffer access here}}
+}
+
+void unclaimed_use() {
+  int *a = new int[3];  // expected-warning{{'a' is an unsafe pointer used for buffer access}}
+  a[2] = 9;  // expected-note{{used in buffer access here}}
+  int *b = a++;  // expected-note{{used in pointer arithmetic here}} \
+  // debug-note{{safe buffers debug: failed to produce fixit for 'a' : has an unclaimed use}}
+}
+
+void implied_unclaimed_var(int *b) {  // expected-warning{{'b' is an unsafe pointer used for buffer access}}
+  int *a = new int[3];  // expected-warning{{'a' is an unsafe pointer used for buffer access}}
+  a[4] = 7;  // expected-note{{used in buffer access here}}
+  a = b;  // debug-note{{safe buffers debug: gadget 'PointerAssignment' refused to produce a fix}}
+  b++;  // expected-note{{used in pointer arithmetic here}} \
+        // debug-note{{safe buffers debug: failed to produce fixit for 'b' : has an unclaimed use}}
+}