[PATCH] D154880: [-Wunsafe-buffer-usage][WIP] Add a facility for debugging low fixit coverage.
ziqingluo-90 added a comment. 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? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154880/new/ https://reviews.llvm.org/D154880 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154880: [-Wunsafe-buffer-usage][WIP] Add a facility for debugging low fixit coverage.
NoQ added inline comments. Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:2248-2249 +#ifndef NDEBUG +// FIXME: F->getBaseStmt() should never be null! +// (Or we should build a better interface for this.) +Handler.addDebugNoteForVar( 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. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154880/new/ https://reviews.llvm.org/D154880 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154880: [-Wunsafe-buffer-usage][WIP] Add a facility for debugging low fixit coverage.
NoQ added a comment.
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!
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:1745-1746
+const StringRef UserFillPlaceHolder,
+UnsafeBufferUsageHandler ) {
const QualType = D->getType()->getPointeeType();
assert(!SpanEltT.isNull() && "Trying to fix a non-pointer type variable!");
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
```
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:1760-1761
+#ifndef NDEBUG
+ // FIXME: F->getBaseStmt() should never be null!
+ // (Or we should build a better interface for this.)
+ Handler.addDebugNoteForVar(
This comment is probably duplicated everywhere by accident; it doesn't make
much sense in most of these places.
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:1790
+ ("failed to produce fixit for declaration '" + D->getName()
+ + "' : fale dto get end char loc (macro)").str());
+#endif
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:2395
+ it->first, it->first->getBeginLoc(),
+ ("'" + it->first->getName() +
+ "' is neither local nor a parameter").str());
`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.
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:2396
+ ("'" + it->first->getName() +
+ "' is neither local nor a parameter").str());
+#endif
Do you want to include a "failde to produce fixit..." text in this message and
the next message, for consistency?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154880/new/
https://reviews.llvm.org/D154880
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154880: [-Wunsafe-buffer-usage][WIP] Add a facility for debugging low fixit coverage.
t-rasmud updated this revision to Diff 541745.
t-rasmud added a comment.
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.
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,68 @@
+// 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 parm var decl '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: '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: 'b' has an unclaimed use}}
+}
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2276,6 +2276,12 @@
for (const auto : Fixes)
FD << F;
}
+
+#ifndef NDEBUG
+if (areDebugNotesRequested())
+ for (const DebugNote : DebugNotesByVar[Variable])
+S.Diag(Note.first, diag::note_safe_buffer_debug_mode) << Note.second;
+#endif
}
bool isSafeBufferOptOut(const SourceLocation ) const override {
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -319,6 +319,15 @@
Kind
[PATCH] D154880: [-Wunsafe-buffer-usage][WIP] Add a facility for debugging low fixit coverage.
t-rasmud added a comment. I will add debug notes for the rest of the cases not addressed in this patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154880/new/ https://reviews.llvm.org/D154880 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154880: [-Wunsafe-buffer-usage][WIP] Add a facility for debugging low fixit coverage.
NoQ updated this revision to Diff 538862.
NoQ added a comment.
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.
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 : Fixes)
FD << F;
}
+
+#ifndef NDEBUG
+if (areDebugNotesRequested())
+ for (const DebugNote : DebugNotesByVar[Variable])
+S.Diag(Note.first, diag::note_safe_buffer_debug_mode) << Note.second;
+#endif
}
bool isSafeBufferOptOut(const SourceLocation ) 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,11 @@
virtual std::optional getFixits(const Strategy ) 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 ) 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 : 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 =
[PATCH] D154880: [-Wunsafe-buffer-usage][WIP] Add a facility for debugging low fixit coverage.
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 : Fixes)
FD << F;
}
+
+#ifndef NDEBUG
+if (areDebugNotesRequested())
+ for (const DebugNote : DebugNotesByVar[Variable])
+S.Diag(Note.first, diag::note_safe_buffer_debug_mode) << Note.second;
+#endif
}
bool isSafeBufferOptOut(const SourceLocation ) 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 getFixits(const Strategy ) 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 : 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() ? 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 ,
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
[PATCH] D154880: [-Wunsafe-buffer-usage][WIP] Add a facility for debugging low fixit coverage.
NoQ added inline comments.
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:578
- virtual const Stmt *getBaseStmt() const override { return nullptr; }
+ virtual const Stmt *getBaseStmt() const override { return PtrInitRHS; }
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.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154880/new/
https://reviews.llvm.org/D154880
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154880: [-Wunsafe-buffer-usage][WIP] Add a facility for debugging low fixit coverage.
NoQ created this revision.
NoQ added reviewers: jkorous, t-rasmud, ziqingluo-90, malavikasamak.
Herald added subscribers: steakhal, martong.
Herald added a project: All.
NoQ requested review of this revision.
Herald added a subscriber: wangpc.
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).
Repository:
rC Clang
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 : Fixes)
FD << F;
}
+
+#ifndef NDEBUG
+if (areDebugNotesRequested())
+ for (const DebugNote : DebugNotesByVar[Variable])
+S.Diag(Note.first, diag::note_safe_buffer_debug_mode) << Note.second;
+#endif
}
bool isSafeBufferOptOut(const SourceLocation ) 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 getFixits(const Strategy ) const override;
- virtual const Stmt *getBaseStmt() const override { return nullptr; }
+ virtual const Stmt *getBaseStmt() const override { return PtrInitRHS; }
virtual
