Skip to content

Commit 237ca43

Browse files
jkorous-applejkorous-apple
committed
[-Wunsafe-buffer-usage] Group diagnostics by variable
Differential Revision: https://reviews.llvm.org/D141356
1 parent 69a9bbf commit 237ca43

File tree

7 files changed

+343
-172
lines changed

7 files changed

+343
-172
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ class UnsafeBufferUsageHandler {
3131
using FixItList = llvm::SmallVectorImpl<FixItHint>;
3232

3333
/// Invoked when an unsafe operation over raw pointers is found.
34-
virtual void handleUnsafeOperation(const Stmt *Operation) = 0;
34+
virtual void handleUnsafeOperation(const Stmt *Operation,
35+
bool IsRelatedToDecl) = 0;
3536

3637
/// Invoked when a fix is suggested against a variable.
3738
virtual void handleFixableVariable(const VarDecl *Variable,

‎clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11778,12 +11778,16 @@ def err_loongarch_builtin_requires_la64 : Error<
1177811778
"this builtin requires target: loongarch64">;
1177911779

1178011780
// Unsafe buffer usage diagnostics.
11781-
def warn_unsafe_buffer_expression : Warning<
11782-
"unchecked operation on raw buffer in expression">,
11783-
InGroup<UnsafeBufferUsage>, DefaultIgnore;
1178411781
def warn_unsafe_buffer_variable : Warning<
11785-
"variable %0 participates in unchecked buffer operations">,
11782+
"%0 is an %select{unsafe pointer used for buffer access|unsafe buffer that "
11783+
"does not perform bounds checks}1">,
11784+
InGroup<UnsafeBufferUsage>, DefaultIgnore;
11785+
def warn_unsafe_buffer_operation : Warning<
11786+
"%select{unsafe pointer operation|unsafe pointer arithmetic|"
11787+
"unsafe buffer access}0">,
1178611788
InGroup<UnsafeBufferUsage>, DefaultIgnore;
11789+
def note_unsafe_buffer_operation : Note<
11790+
"used%select{| in pointer arithmetic| in buffer access}0 here">;
1178711791
def err_loongarch_builtin_requires_la32 : Error<
1178811792
"this builtin requires target: loongarch32">;
1178911793
} // end of sema component.

‎clang/lib/Analysis/UnsafeBufferUsage.cpp

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -680,17 +680,16 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
680680
// FIXME Detect overlapping FixIts.
681681

682682
for (const auto &G : UnsafeOps.noVar) {
683-
Handler.handleUnsafeOperation(G->getBaseStmt());
683+
Handler.handleUnsafeOperation(G->getBaseStmt(), /*IsRelatedToDecl=*/false);
684684
}
685685

686686
for (const auto &[VD, WarningGadgets] : UnsafeOps.byVar) {
687687
auto FixItsIt = FixItsForVariable.find(VD);
688-
if (FixItsIt != FixItsForVariable.end()) {
689-
Handler.handleFixableVariable(VD, std::move(FixItsIt->second));
690-
} else {
691-
for (const auto &G : WarningGadgets) {
692-
Handler.handleUnsafeOperation(G->getBaseStmt());
693-
}
688+
Handler.handleFixableVariable(VD, FixItsIt != FixItsForVariable.end()
689+
? std::move(FixItsIt->second)
690+
: FixItList{});
691+
for (const auto &G : WarningGadgets) {
692+
Handler.handleUnsafeOperation(G->getBaseStmt(), /*IsRelatedToDecl=*/true);
694693
}
695694
}
696695
}

‎clang/lib/Sema/AnalysisBasedWarnings.cpp

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2153,12 +2153,15 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
21532153
public:
21542154
UnsafeBufferUsageReporter(Sema &S) : S(S) {}
21552155

2156-
void handleUnsafeOperation(const Stmt *Operation) override {
2156+
void handleUnsafeOperation(const Stmt *Operation,
2157+
bool IsRelatedToDecl) override {
21572158
SourceLocation Loc;
21582159
SourceRange Range;
2160+
unsigned MsgParam = 0;
21592161
if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(Operation)) {
21602162
Loc = ASE->getBase()->getExprLoc();
21612163
Range = ASE->getBase()->getSourceRange();
2164+
MsgParam = 2;
21622165
} else if (const auto *BO = dyn_cast<BinaryOperator>(Operation)) {
21632166
BinaryOperator::Opcode Op = BO->getOpcode();
21642167
if (Op == BO_Add || Op == BO_AddAssign || Op == BO_Sub ||
@@ -2170,27 +2173,35 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
21702173
Loc = BO->getRHS()->getExprLoc();
21712174
Range = BO->getRHS()->getSourceRange();
21722175
}
2176+
MsgParam = 1;
21732177
}
21742178
} else if (const auto *UO = dyn_cast<UnaryOperator>(Operation)) {
21752179
UnaryOperator::Opcode Op = UO->getOpcode();
21762180
if (Op == UO_PreInc || Op == UO_PreDec || Op == UO_PostInc ||
21772181
Op == UO_PostDec) {
21782182
Loc = UO->getSubExpr()->getExprLoc();
21792183
Range = UO->getSubExpr()->getSourceRange();
2184+
MsgParam = 1;
21802185
}
21812186
} else {
21822187
Loc = Operation->getBeginLoc();
21832188
Range = Operation->getSourceRange();
21842189
}
2185-
S.Diag(Loc, diag::warn_unsafe_buffer_expression) << Range;
2190+
if (IsRelatedToDecl)
2191+
S.Diag(Loc, diag::note_unsafe_buffer_operation) << MsgParam << Range;
2192+
else
2193+
S.Diag(Loc, diag::warn_unsafe_buffer_operation) << MsgParam << Range;
21862194
}
21872195

2196+
// FIXME: rename to handleUnsafeVariable
21882197
void handleFixableVariable(const VarDecl *Variable,
21892198
FixItList &&Fixes) override {
21902199
const auto &D =
2191-
S.Diag(Variable->getBeginLoc(), diag::warn_unsafe_buffer_variable);
2192-
D << Variable << Variable->getSourceRange();
2193-
for (const auto &F: Fixes)
2200+
S.Diag(Variable->getLocation(), diag::warn_unsafe_buffer_variable);
2201+
D << Variable;
2202+
D << (Variable->getType()->isPointerType() ? 0 : 1);
2203+
D << Variable->getSourceRange();
2204+
for (const auto &F : Fixes)
21942205
D << F;
21952206
}
21962207
};
@@ -2489,7 +2500,7 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
24892500
checkThrowInNonThrowingFunc(S, FD, AC);
24902501

24912502
// Emit unsafe buffer usage warnings and fixits.
2492-
if (!Diags.isIgnored(diag::warn_unsafe_buffer_expression, D->getBeginLoc()) ||
2503+
if (!Diags.isIgnored(diag::warn_unsafe_buffer_operation, D->getBeginLoc()) ||
24932504
!Diags.isIgnored(diag::warn_unsafe_buffer_variable, D->getBeginLoc())) {
24942505
UnsafeBufferUsageReporter R(S);
24952506
checkUnsafeBufferUsage(D, R);

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

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage -verify %s
2+
3+
namespace localVar {
4+
void testRefersPtrLocalVarDecl(int i) {
5+
int * ptr; // expected-warning{{'ptr' is an unsafe pointer used for buffer access}}
6+
ptr + i; // expected-note{{used in pointer arithmetic here}}
7+
ptr[i]; // expected-note{{used in buffer access here}}
8+
}
9+
10+
void testRefersArrayLocalVarDecl(int i) {
11+
int array[i]; // expected-warning{{'array' is an unsafe buffer that does not perform bounds}}
12+
array[i/2]; // expected-note{{used in buffer access here}}
13+
}
14+
}
15+
16+
namespace globalVar {
17+
int * ptr; // expected-warning{{'ptr' is an unsafe pointer used for buffer access}}
18+
void testRefersPtrGlobalVarDecl(int i) {
19+
ptr + i; // expected-note{{used in pointer arithmetic here}}
20+
ptr[i]; // expected-note{{used in buffer access here}}
21+
}
22+
23+
int array[10]; // expected-warning{{'array' is an unsafe buffer that does not perform bounds}}
24+
void testRefersArrayGlobalVarDecl(int i) {
25+
array[i/2]; // expected-note{{used in buffer access here}}
26+
}
27+
}
28+
29+
namespace functionParm {
30+
void testRefersPtrParmVarDecl(int * ptr) {
31+
// expected-warning@-1{{'ptr' is an unsafe pointer used for buffer access}}
32+
ptr + 5; // expected-note{{used in pointer arithmetic here}}
33+
ptr[5]; // expected-note{{used in buffer access here}}
34+
}
35+
36+
// FIXME: shall we explain the array to pointer decay to make the warning more understandable?
37+
void testRefersArrayParmVarDecl(int array[10]) {
38+
// expected-warning@-1{{'array' is an unsafe pointer used for buffer access}}
39+
array[2]; // expected-note{{used in buffer access here}}
40+
}
41+
}
42+
43+
namespace structField {
44+
struct Struct1 {
45+
int * ptr; // FIXME: per-declaration warning aggregated at the struct definition?
46+
};
47+
48+
void testRefersPtrStructFieldDecl(int i) {
49+
Struct1 s1;
50+
s1.ptr + i; // expected-warning{{unsafe pointer arithmetic}}
51+
s1.ptr[i]; // expected-warning{{unsafe buffer access}}
52+
}
53+
54+
struct Struct2 {
55+
int array[10]; // FIXME: per-declaration warning aggregated at the struct definition?
56+
};
57+
58+
void testRefersArrayStructFieldDecl(int i) {
59+
Struct2 s2;
60+
s2.array[i/2]; // expected-warning{{unsafe buffer access}}
61+
}
62+
}
63+
64+
namespace structFieldFromMethod {
65+
struct Struct1 {
66+
int * ptr; // FIXME: per-declaration warning aggregated at the struct definition
67+
68+
void testRefersPtrStructFieldDecl(int i) {
69+
ptr + i; // expected-warning{{unsafe pointer arithmetic}}
70+
ptr[i]; // expected-warning{{unsafe buffer access}}
71+
}
72+
};
73+
74+
struct Struct2 {
75+
int array[10]; // FIXME: per-declaration warning aggregated at the struct definition
76+
77+
void testRefersArrayStructFieldDecl(int i) {
78+
Struct2 s2;
79+
s2.array[i/2]; // expected-warning{{unsafe buffer access}}
80+
}
81+
};
82+
}
83+
84+
namespace staticStructField {
85+
struct Struct1 {
86+
static int * ptr; // expected-warning{{'ptr' is an unsafe pointer used for buffer access}}
87+
};
88+
89+
void testRefersPtrStructFieldDecl(int i) {
90+
Struct1::ptr + i; // expected-note{{used in pointer arithmetic here}}
91+
Struct1::ptr[i]; // expected-note{{used in buffer access here}}
92+
}
93+
94+
struct Struct2 {
95+
static int array[10]; // expected-warning{{'array' is an unsafe buffer that does not perform bounds}}
96+
};
97+
98+
void testRefersArrayStructFieldDecl(int i) {
99+
Struct2::array[i/2]; // expected-note{{used in buffer access here}}
100+
}
101+
}
102+
103+
int * return_ptr();
104+
105+
void testNoDeclRef(int i) {
106+
return_ptr() + i; // expected-warning{{unsafe pointer arithmetic}}
107+
return_ptr()[i]; // expected-warning{{unsafe buffer access}}
108+
}

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

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,47 +3,46 @@
33
void foo(int i) {
44
int * ptr;
55

6-
76
ptr++;
8-
// CHECK-DAG: {7:3-7:6}{{.*}}[-Wunsafe-buffer-usage]
7+
// CHECK-DAG: {[[@LINE-1]]:3-[[@LINE-1]]:6}
98
ptr--;
10-
// CHECK-DAG: {9:3-9:6}{{.*}}[-Wunsafe-buffer-usage]
9+
// CHECK-DAG: {[[@LINE-1]]:3-[[@LINE-1]]:6}
1110
++ptr;
12-
// CHECK-DAG: {11:5-11:8}{{.*}}[-Wunsafe-buffer-usage]
11+
// CHECK-DAG: {[[@LINE-1]]:5-[[@LINE-1]]:8}
1312
--ptr;
14-
// CHECK-DAG: {13:5-13:8}{{.*}}[-Wunsafe-buffer-usage]
13+
// CHECK-DAG: {[[@LINE-1]]:5-[[@LINE-1]]:8}
1514

1615

1716
ptr + 1;
18-
// CHECK-DAG: {17:3-17:6}{{.*}}[-Wunsafe-buffer-usage]
17+
// CHECK-DAG: {[[@LINE-1]]:3-[[@LINE-1]]:6}
1918
2 + ptr;
20-
// CHECK-DAG: {19:7-19:10}{{.*}}[-Wunsafe-buffer-usage]
19+
// CHECK-DAG: {[[@LINE-1]]:7-[[@LINE-1]]:10}
2120
ptr + i;
22-
// CHECK-DAG: {21:3-21:6}{{.*}}[-Wunsafe-buffer-usage]
21+
// CHECK-DAG: {[[@LINE-1]]:3-[[@LINE-1]]:6}
2322
i + ptr;
24-
// CHECK-DAG: {23:7-23:10}{{.*}}[-Wunsafe-buffer-usage]
23+
// CHECK-DAG: {[[@LINE-1]]:7-[[@LINE-1]]:10}
2524

2625

2726
ptr - 3;
28-
// CHECK-DAG: {27:3-27:6}{{.*}}[-Wunsafe-buffer-usage]
27+
// CHECK-DAG: {[[@LINE-1]]:3-[[@LINE-1]]:6}
2928
ptr - i;
30-
// CHECK-DAG: {29:3-29:6}{{.*}}[-Wunsafe-buffer-usage]
29+
// CHECK-DAG: {[[@LINE-1]]:3-[[@LINE-1]]:6}
3130

3231

3332
ptr += 4;
34-
// CHECK-DAG: {33:3-33:6}{{.*}}[-Wunsafe-buffer-usage]
33+
// CHECK-DAG: {[[@LINE-1]]:3-[[@LINE-1]]:6}
3534
ptr += i;
36-
// CHECK-DAG: {35:3-35:6}{{.*}}[-Wunsafe-buffer-usage]
35+
// CHECK-DAG: {[[@LINE-1]]:3-[[@LINE-1]]:6}
3736

3837

3938
ptr -= 5;
40-
// CHECK-DAG: {39:3-39:6}{{.*}}[-Wunsafe-buffer-usage]
39+
// CHECK-DAG: {[[@LINE-1]]:3-[[@LINE-1]]:6}
4140
ptr -= i;
42-
// CHECK-DAG: {41:3-41:6}{{.*}}[-Wunsafe-buffer-usage]
41+
// CHECK-DAG: {[[@LINE-1]]:3-[[@LINE-1]]:6}
4342

4443

4544
ptr[5];
46-
// CHECK-DAG: {45:3-45:6}{{.*}}[-Wunsafe-buffer-usage]
45+
// CHECK-DAG: {[[@LINE-1]]:3-[[@LINE-1]]:6}
4746
5[ptr];
48-
// CHECK-DAG: {47:5-47:8}{{.*}}[-Wunsafe-buffer-usage]
47+
// CHECK-DAG: {[[@LINE-1]]:5-[[@LINE-1]]:8}
4948
}

0 commit comments

Comments
 (0)