Skip to content

Commit aa7bded

Browse files
[-Wunsafe-buffer-usage] Fix false positive when assigning in C++ class
Fix false positive when assigning to __counted_by() that is a class member. The analysis fails to see through `CXXThisExpr` and thinks we are assigning 2 different groups. This is caused by me, failing to copy/paste the updated version of `isSameMemberBase()`. ``` struct cb { int *__counted_by(count) p; size_t count; }; class struct_test { cb cb_; void set_cb(std::span<int> sp) { cb_.p = sp.data(); // warning: missing assignment to count cb_.count = sp.size(); // warning: missing assignment to p } }; ``` rdar://164535516 (cherry picked from commit 3cd72d6)
1 parent d854d17 commit aa7bded

File tree

2 files changed

+114
-2
lines changed

2 files changed

+114
-2
lines changed

clang/lib/Analysis/UnsafeBufferUsage.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5385,8 +5385,10 @@ static bool isSameMemberBase(const Expr *Self, const Expr *Other) {
53855385

53865386
const auto *SelfICE = dyn_cast<ImplicitCastExpr>(Self);
53875387
const auto *OtherICE = dyn_cast<ImplicitCastExpr>(Other);
5388-
if (SelfICE && OtherICE && SelfICE->getCastKind() == CK_LValueToRValue &&
5389-
OtherICE->getCastKind() == CK_LValueToRValue) {
5388+
if (SelfICE && OtherICE &&
5389+
SelfICE->getCastKind() == OtherICE->getCastKind() &&
5390+
(SelfICE->getCastKind() == CK_LValueToRValue ||
5391+
SelfICE->getCastKind() == CK_UncheckedDerivedToBase)) {
53905392
Self = SelfICE->getSubExpr();
53915393
Other = OtherICE->getSubExpr();
53925394
}
@@ -5396,6 +5398,12 @@ static bool isSameMemberBase(const Expr *Self, const Expr *Other) {
53965398
if (SelfDRE && OtherDRE)
53975399
return SelfDRE->getDecl() == OtherDRE->getDecl();
53985400

5401+
if (isa<CXXThisExpr>(Self) && isa<CXXThisExpr>(Other)) {
5402+
// `Self` and `Other` should be evaluated at the same state so `this` must
5403+
// mean the same thing for both:
5404+
return true;
5405+
}
5406+
53995407
const auto *SelfME = dyn_cast<MemberExpr>(Self);
54005408
const auto *OtherME = dyn_cast<MemberExpr>(Other);
54015409
if (!SelfME || !OtherME ||

clang/test/SemaCXX/warn-unsafe-buffer-usage-count-attributed-pointer-assignment.cpp

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#include <ptrcheck.h>
44
#include <stddef.h>
5+
#include <stdint.h>
56

67
namespace std {
78
template <typename T> struct span {
@@ -26,6 +27,10 @@ struct cb_multi {
2627
size_t n;
2728
};
2829

30+
struct cb_nested {
31+
cb nested;
32+
};
33+
2934
// Simple pointer and count
3035

3136
void good_null(int *__counted_by(count) p, int count) {
@@ -118,6 +123,16 @@ void good_null_loop_if(int *__counted_by(count) p, int count) {
118123
}
119124
}
120125

126+
void good_size_bytes_char(char *__counted_by(count) p, size_t count, std::span<char> sp) {
127+
p = sp.data();
128+
count = sp.size_bytes();
129+
}
130+
131+
void good_size_bytes_void(void *__sized_by(size) p, size_t size, std::span<uint8_t> sp) {
132+
p = sp.data();
133+
size = sp.size_bytes();
134+
}
135+
121136
// Simple pointer and count in struct
122137

123138
void good_struct_self(cb *c) {
@@ -167,6 +182,54 @@ void good_struct_self_loop(cb *c) {
167182
}
168183
}
169184

185+
void good_struct_nested_span(cb_nested *n, std::span<int> sp) {
186+
n->nested.p = sp.data();
187+
n->nested.count = sp.size();
188+
}
189+
190+
void bad_struct_nested_span(cb_nested *n, std::span<int> sp, size_t unrelated_size) {
191+
n->nested.p = sp.data(); // expected-warning{{unsafe assignment to count-attributed pointer}}
192+
n->nested.count = unrelated_size;
193+
}
194+
195+
class struct_test {
196+
int *__counted_by(count_) data_;
197+
size_t count_;
198+
cb cb_;
199+
cb_nested nested_;
200+
cb *p_cb_;
201+
202+
void set_data(std::span<int> sp) {
203+
data_ = sp.data();
204+
count_ = sp.size();
205+
}
206+
207+
void bad_set_data(std::span<int> sp) {
208+
data_ = sp.data(); // expected-warning{{unsafe assignment to count-attributed pointer}}
209+
count_ = 42;
210+
}
211+
212+
void set_cb(std::span<int> sp) {
213+
cb_.p = sp.data();
214+
cb_.count = sp.size();
215+
}
216+
217+
void bad_set_cb(std::span<int> sp) {
218+
cb_.p = sp.data(); // expected-warning{{unsafe assignment to count-attributed pointer}}
219+
cb_.count = 42;
220+
}
221+
222+
void set_nested(std::span<int> sp) {
223+
nested_.nested.p = sp.data();
224+
nested_.nested.count = sp.size();
225+
}
226+
227+
void set_p_cb(std::span<int> sp) {
228+
p_cb_->p = sp.data();
229+
p_cb_->count = sp.size();
230+
}
231+
};
232+
170233
// Pointer with multiple counts
171234

172235
void bad_multicounts_span(int *__counted_by(a + b) p, size_t a, size_t b, std::span<int> sp) {
@@ -249,6 +312,16 @@ bool good_multicount_struct_realistic(cb_multi *cbm, std::span<int> sp, size_t s
249312
return true;
250313
}
251314

315+
struct multicount_struct_test {
316+
cb_multi multi_;
317+
318+
void set_multi(std::span<int> sp, size_t a, size_t b) {
319+
multi_.p = sp.first(a * b).data();
320+
multi_.m = a;
321+
multi_.n = b;
322+
}
323+
};
324+
252325
// Multiple pointers
253326

254327
void good_multiptr_span(int *__counted_by(count) p, int *__counted_by(count) q, size_t count, std::span<int> sp) {
@@ -552,6 +625,37 @@ void missing_struct_unrelated(cb *c, cb *d) {
552625
d->count = 0; // expected-warning{{bounds-attributed group requires assigning 'count, p', assignments to 'p' missing}}
553626
}
554627

628+
void missing_struct_nested_ptr(cb_nested *c) {
629+
c->nested.count = 0; // expected-warning{{bounds-attributed group requires assigning 'count, p', assignments to 'p' missing}}
630+
}
631+
632+
void missing_struct_nested_unrelated(cb_nested *c, cb_nested *d) {
633+
c->nested.p = nullptr; // expected-warning{{bounds-attributed group requires assigning 'count, p', assignments to 'count' missing}}
634+
d->nested.count = 0; // expected-warning{{bounds-attributed group requires assigning 'count, p', assignments to 'p' missing}}
635+
}
636+
637+
struct missing_struct_test {
638+
cb cb_;
639+
cb_nested nested_;
640+
641+
void set_cb_missing_ptr(std::span<int> sp) {
642+
cb_.count = sp.size(); // expected-warning{{bounds-attributed group requires assigning 'count, p', assignments to 'p' missing}}
643+
}
644+
645+
void set_cb_missing_count(std::span<int> sp) {
646+
cb_.p = sp.data(); // expected-warning{{bounds-attributed group requires assigning 'count, p', assignments to 'count' missing}}
647+
}
648+
649+
void set_nested_missing_ptr(std::span<int> sp) {
650+
nested_.nested.count = sp.size(); // expected-warning{{bounds-attributed group requires assigning 'count, p', assignments to 'p' missing}}
651+
}
652+
653+
void set_missing_unrelated(std::span<int> sp) {
654+
cb_.p = sp.data(); // expected-warning{{bounds-attributed group requires assigning 'count, p', assignments to 'count' missing}}
655+
nested_.nested.count = sp.size(); // expected-warning{{bounds-attributed group requires assigning 'count, p', assignments to 'p' missing}}
656+
}
657+
};
658+
555659
// Duplicated assignments
556660

557661
void duplicated_ptr(int *__counted_by(count) p, int count) {

0 commit comments

Comments
 (0)