NoQ updated this revision to Diff 538852.
NoQ added a comment.
`private:` => `public:`
(for some reason it didn't complain until I did a full rebuild)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154880/new/
https://reviews.llvm.org/D154880
Files:
clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp
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}}
+}
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/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,7 @@
virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
- virtual const Stmt *getBaseStmt() const override { return nullptr; }
+ virtual const Stmt *getBaseStmt() const override { return PtrInitRHS; }
virtual DeclUseList getClaimedVarUseSites() const override {
return DeclUseList{PtrInitRHS};
@@ -2113,6 +2122,16 @@
for (const auto &F : Fixables) {
std::optional<FixItList> 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() ? F->getBaseStmt()->getBeginLoc()
+ : VD->getBeginLoc(),
+ ("gadget '" + F->getDebugName() + "' refused to produce a fix")
+ .str());
+#endif
ImpossibleToFix = true;
break;
} else {
@@ -2198,6 +2217,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/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/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<SourceLocation, std::string>;
+ using DebugNoteList = std::vector<DebugNote>;
+ using DebugNoteByVar = std::map<const VarDecl *, DebugNoteList>;
+ 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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
