Index: clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h =================================================================== --- clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h +++ clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h @@ -16,6 +16,7 @@ #include "clang/AST/Decl.h" #include "clang/AST/Stmt.h" +#include "llvm/Support/Debug.h" namespace clang { @@ -24,6 +25,18 @@ /// 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; + using DebugNoteList = std::vector; + using DebugNoteByVar = std::map; + DebugNoteByVar DebugNotesByVar; +#endif + public: UnsafeBufferUsageHandler() = default; virtual ~UnsafeBufferUsageHandler() = default; @@ -43,6 +56,26 @@ 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; Index: clang/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -11841,6 +11841,12 @@ "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">; Index: clang/lib/Analysis/UnsafeBufferUsage.cpp =================================================================== --- clang/lib/Analysis/UnsafeBufferUsage.cpp +++ clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -319,6 +319,15 @@ 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; @@ -566,7 +575,11 @@ virtual std::optional 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}; @@ -614,7 +627,11 @@ virtual std::optional 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}; @@ -2113,6 +2130,14 @@ for (const auto &F : Fixables) { std::optional Fixits = F->getFixits(S); if (!Fixits) { +#ifndef NDEBUG + // FIXME: F->getBaseStmt() should never be null! + // (Or we should build a better interface for this.) + Handler.addDebugNoteForVar( + VD, F->getBaseStmt()->getBeginLoc(), + ("gadget '" + F->getDebugName() + "' refused to produce a fix") + .str()); +#endif ImpossibleToFix = true; break; } else { @@ -2198,6 +2223,10 @@ void clang::checkUnsafeBufferUsage(const Decl *D, UnsafeBufferUsageHandler &Handler, bool EmitSuggestions) { +#ifndef NDEBUG + Handler.clearDebugNotes(); +#endif + assert(D && D->getBody()); // Do not emit fixit suggestions for functions declared in an Index: clang/lib/Sema/AnalysisBasedWarnings.cpp =================================================================== --- clang/lib/Sema/AnalysisBasedWarnings.cpp +++ clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2276,6 +2276,12 @@ 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 { Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp @@ -0,0 +1,33 @@ +// 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}} +}