diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h index eccb64fd4d3ed..9377a66c110b2 100644 --- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h +++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h @@ -181,6 +181,11 @@ class UnsafeBufferUsageHandler { ASTContext &Ctx) { handleUnsafeOperation(Assign, IsRelatedToDecl, Ctx); } + + virtual void handleUnsafeCountAttributedPointerAssignment( + const BinaryOperator *Assign, bool IsRelatedToDecl, ASTContext &Ctx) { + handleUnsafeOperation(Assign, IsRelatedToDecl, Ctx); + } /* TO_UPSTREAM(BoundsSafety) OFF */ /// Invoked when a fix is suggested against a variable. This function groups diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index a08c04434ff30..8559722a56fcb 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -14289,6 +14289,9 @@ def warn_missing_assignments_in_bounds_attributed_group : Warning< def warn_duplicated_assignment_in_bounds_attributed_group : Warning< "duplicated assignment to %select{variable|parameter|member}0 %1 in " "bounds-attributed group">, InGroup, DefaultIgnore; +def warn_unsafe_count_attributed_pointer_assignment : Warning< + "unsafe assignment to count-attributed pointer">, + InGroup, DefaultIgnore; #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). diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 87d9c1c2bf4a5..1ad69c63816f3 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -5947,6 +5947,60 @@ static bool checkMissingAndDuplicatedAssignments( return IsGroupSafe; } +// Checks if each assignment to count-attributed pointer in the group is safe. +static bool +checkAssignmentPatterns(const BoundsAttributedAssignmentGroup &Group, + UnsafeBufferUsageHandler &Handler, ASTContext &Ctx) { + // Collect dependent values. + DependentValuesTy DependentValues; + for (size_t I = 0, N = Group.AssignedObjects.size(); I < N; ++I) { + const ValueDecl *VD = Group.AssignedObjects[I].Decl; + const auto *Attr = VD->getAttr(); + if (!Attr) + continue; + + const BinaryOperator *Assign = Group.Assignments[I]; + const Expr *Value = Assign->getRHS(); + + [[maybe_unused]] bool Inserted = + DependentValues.insert({{VD, Attr->getIsDeref()}, Value}).second; + // Previous checks in `checkBoundsAttributedGroup` should have validated + // that we have only a single assignment. + assert(Inserted); + } + + bool IsGroupSafe = true; + + // Check every pointer in the group. + for (size_t I = 0, N = Group.AssignedObjects.size(); I < N; ++I) { + const ValueDecl *VD = Group.AssignedObjects[I].Decl; + + QualType Ty = VD->getType(); + const auto *CAT = Ty->getAs(); + if (!CAT && Ty->isPointerType()) + CAT = Ty->getPointeeType()->getAs(); + if (!CAT) + continue; + + const BinaryOperator *Assign = Group.Assignments[I]; + + // TODO: Move this logic to isCountAttributedPointerArgumentSafeImpl. + const Expr *CountArg = + DependentValues.size() == 1 ? DependentValues.begin()->second : nullptr; + + bool IsSafe = isCountAttributedPointerArgumentSafeImpl( + Ctx, Assign->getRHS(), CountArg, CAT, CAT->getCountExpr(), + CAT->isCountInBytes(), CAT->isOrNull(), &DependentValues); + if (!IsSafe) { + Handler.handleUnsafeCountAttributedPointerAssignment( + Assign, /*IsRelatedToDecl=*/false, Ctx); + IsGroupSafe = false; + } + } + + return IsGroupSafe; +} + // Checks if the bounds-attributed group is safe. This function returns false // iff the assignment group is unsafe and diagnostics have been emitted. static bool @@ -5956,8 +6010,7 @@ checkBoundsAttributedGroup(const BoundsAttributedAssignmentGroup &Group, return false; if (!checkMissingAndDuplicatedAssignments(Group, Handler, Ctx)) return false; - // TODO: Add more checks. - return true; + return checkAssignmentPatterns(Group, Handler, Ctx); } static void diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 61a48546989ca..35f3413ed7127 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2685,6 +2685,14 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler { S.Diag(PrevAssign->getOperatorLoc(), diag::note_bounds_safety_previous_assignment); } + + void + handleUnsafeCountAttributedPointerAssignment(const BinaryOperator *Assign, + bool IsRelatedToDecl, + ASTContext &Ctx) override { + S.Diag(Assign->getOperatorLoc(), + diag::warn_unsafe_count_attributed_pointer_assignment); + } /* TO_UPSTREAM(BoundsSafety) OFF */ void handleUnsafeVariableGroup(const VarDecl *Variable, diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-count-attributed-pointer-assignment.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-count-attributed-pointer-assignment.cpp index 226ec6e29a090..755caae5a8a42 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-count-attributed-pointer-assignment.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-count-attributed-pointer-assignment.cpp @@ -20,6 +20,12 @@ struct cb { size_t count; }; +struct cb_multi { + int *__counted_by(m * n) p; + size_t m; + size_t n; +}; + // Simple pointer and count void good_null(int *__counted_by(count) p, int count) { @@ -27,6 +33,46 @@ void good_null(int *__counted_by(count) p, int count) { count = 0; } +void good_simple_self(int *__counted_by(count) p, int count) { + p = p; + count = count; +} + +void good_simple_other(int *__counted_by(count) p, int count, int *__counted_by(len) q, int len) { + p = q; + count = len; +} + +void good_simple_span(int *__counted_by(count) p, size_t count, std::span sp) { + p = sp.data(); + count = sp.size(); +} + +void bad_simple_span(int *__counted_by(count) p, size_t count, std::span sp) { + p = sp.data(); // expected-warning{{unsafe assignment to count-attributed pointer}} + count = 42; +} + +void good_simple_subspan_const(int *__counted_by(count) p, int count, std::span sp) { + p = sp.first(42).data(); + count = 42; +} + +void bad_simple_subspan_const(int *__counted_by(count) p, int count, std::span sp) { + p = sp.first(42).data(); // expected-warning{{unsafe assignment to count-attributed pointer}} + count = 43; +} + +void good_simple_subspan_var(int *__counted_by(count) p, int count, std::span sp, int new_count) { + p = sp.first(new_count).data(); + count = new_count; +} + +void bad_simple_subspan_var(int *__counted_by(count) p, int count, std::span sp, int new_count, int new_count2) { + p = sp.first(new_count).data(); // expected-warning{{unsafe assignment to count-attributed pointer}} + count = new_count2; +} + void good_null_loop(int *__counted_by(count) p, int count) { for (int i = 0; i < 2; i++) { p = nullptr; @@ -34,6 +80,13 @@ void good_null_loop(int *__counted_by(count) p, int count) { } } +void good_simple_loop(int *__counted_by(count) p, int count, std::span sp) { + for (int i = 0; i < 2; i++) { + p = sp.data(); + count = sp.size(); + } +} + void good_null_ifelse(int *__counted_by(count) p, int count) { if (count > 10) { p = nullptr; @@ -44,6 +97,16 @@ void good_null_ifelse(int *__counted_by(count) p, int count) { } } +void good_simple_ifelse(int *__counted_by(count) p, size_t count, std::span a, std::span b) { + if (count % 2 == 0) { + p = a.data(); + count = a.size(); + } else { + count = b.size(); + p = b.data(); + } +} + void good_null_loop_if(int *__counted_by(count) p, int count) { for (int i = 0; i < 2; ++i) { if (i == 0) { @@ -62,6 +125,41 @@ void good_struct_self(cb *c) { c->count = c->count; } +void good_struct_other_struct(cb *c, cb *d) { + c->p = d->p; + c->count = d->count; +} + +void good_struct_other_param(cb *c, int *__counted_by(count) p, int count) { + c->p = p; + c->count = count; +} + +void good_struct_span(cb *c, std::span sp) { + c->p = sp.data(); + c->count = sp.size(); +} + +void bad_struct_span(cb *c, std::span sp) { + c->p = sp.data(); // expected-warning{{unsafe assignment to count-attributed pointer}} + c->count = 42; +} + +void good_struct_subspan(cb *c, std::span sp) { + c->p = sp.first(42).data(); + c->count = 42; +} + +void bad_struct_subspan(cb *c, std::span sp) { + c->p = sp.first(42).data(); // expected-warning{{unsafe assignment to count-attributed pointer}} + c->count = 43; +} + +void bad_struct_unrelated_structs(cb *c, cb *d, cb *e) { + c->p = d->p; // expected-warning{{unsafe assignment to count-attributed pointer}} + c->count = e->count; +} + void good_struct_self_loop(cb *c) { for (int i = 0; i < 2; i++) { c->p = c->p; @@ -69,6 +167,150 @@ void good_struct_self_loop(cb *c) { } } +// Pointer with multiple counts + +void bad_multicounts_span(int *__counted_by(a + b) p, size_t a, size_t b, std::span sp) { + p = sp.data(); // expected-warning{{unsafe assignment to count-attributed pointer}} + a = sp.size(); + b = sp.size(); +} + +void good_multicounts_subspan_const(int *__counted_by(a + b) p, int a, int b, std::span sp) { + p = sp.first(42 + 100).data(); + a = 42; + b = 100; +} + +void bad_multicounts_subspan_const(int *__counted_by(a + b) p, int a, int b, std::span sp) { + p = sp.first(42 + 100).data(); // expected-warning{{unsafe assignment to count-attributed pointer}} + a = 42; + b = 99; +} + +void good_multicounts_subspan_var(int *__counted_by(a + b) p, int a, int b, std::span sp, int n, int m) { + p = sp.first(n + m).data(); + a = n; + b = m; +} + +void bad_multicounts_subspan_var(int *__counted_by(a + b) p, int a, int b, std::span sp, int n, int m) { + p = sp.first(n + m).data(); // expected-warning{{unsafe assignment to count-attributed pointer}} + a = n; + b = n; +} + +// Pointer with multiple counts in struct + +void bad_multicount_struct_span(cb_multi *cbm, std::span sp) { + cbm->p = sp.data(); // expected-warning{{unsafe assignment to count-attributed pointer}} + cbm->m = sp.size(); + cbm->n = sp.size(); +} + +void good_multicount_struct_subspan_const(cb_multi *cbm, std::span sp) { + cbm->p = sp.first(42 * 100).data(); + cbm->m = 42; + cbm->n = 100; +} + +void bad_multicount_struct_subspan_const(cb_multi *cbm, std::span sp) { + cbm->p = sp.first(42 * 100).data(); // expected-warning{{unsafe assignment to count-attributed pointer}} + cbm->m = 43; + cbm->n = 100; +} + +void good_multicount_struct_subspan_var(cb_multi *cbm, std::span sp, size_t a, size_t b) { + cbm->p = sp.first(a * b).data(); + cbm->m = a; + cbm->n = b; +} + +void bad_multicount_struct_subspan_var(cb_multi *cbm, std::span sp, size_t a, size_t b) { + cbm->p = sp.first(a * b).data(); // expected-warning{{unsafe assignment to count-attributed pointer}} + cbm->m = a; + cbm->n = a; +} + +bool good_multicount_struct_realistic(cb_multi *cbm, std::span sp, size_t stride) { + if (sp.size() % stride == 0) + return false; + size_t length = sp.size() / stride; + cbm->p = sp.first(length * stride).data(); + cbm->m = length; + cbm->n = stride; + return true; +} + +// Multiple pointers + +void good_multiptr_span(int *__counted_by(count) p, int *__counted_by(count) q, size_t count, std::span sp) { + p = sp.data(); + q = sp.data(); + count = sp.size(); +} + +void bad_multiptr_span(int *__counted_by(count) p, int *__counted_by(count) q, size_t count, std::span sp) { + p = sp.data(); // expected-warning{{unsafe assignment to count-attributed pointer}} + q = sp.data(); // expected-warning{{unsafe assignment to count-attributed pointer}} + count = 42; +} + +void bad_multiptr_span_subspan(int *__counted_by(count) p, int *__counted_by(count) q, size_t count, std::span sp) { + p = sp.data(); // expected-warning{{unsafe assignment to count-attributed pointer}} + q = sp.first(42).data(); + count = 42; +} + +void bad_multiptr_span_subspan2(int *__counted_by(count) p, int *__counted_by(count) q, size_t count, std::span sp) { + p = sp.data(); + q = sp.first(42).data(); // expected-warning{{unsafe assignment to count-attributed pointer}} + count = sp.size(); +} + +void good_multiptr_subspan(int *__counted_by(count) p, int *__counted_by(count) q, size_t count, std::span sp) { + p = sp.first(42).data(); + q = sp.last(42).data(); + count = 42; +} + +// Multiple pointers with multiple counts + +void good_multimix_subspan_complex(int *__counted_by(a * b) p, int *__counted_by((a + b) * c) q, size_t a, size_t b, size_t c, + std::span sp, size_t i, size_t j, size_t k) { + p = sp.first(i * j).data(); + q = sp.last((i + j) * k).data(); + a = i; + b = j; + c = k; +} + +void bad_multimix_subspan_complex(int *__counted_by(a * b) p, int *__counted_by((a + b) * c) q, size_t a, size_t b, size_t c, + std::span sp, size_t i, size_t j, size_t k) { + p = sp.first(i * j).data(); // expected-warning{{unsafe assignment to count-attributed pointer}} + q = sp.last((i + j) * k).data(); // expected-warning{{unsafe assignment to count-attributed pointer}} + a = k; + b = j; + c = i; +} + +void good_multimix_subspan_complex2(int *__counted_by(a * b) p, int *__counted_by((a + b) * c) q, size_t a, size_t b, size_t c, + std::span sp, size_t i, size_t j, size_t k) { + a = i * 2; + b = j; + c = j + k; + p = sp.first((i * 2) * j).data(); + q = sp.last(((i * 2) + j) * (j + k)).data(); +} + +void good_multimix_subspan_complex_multispan(int *__counted_by(a * b) p, int *__counted_by((a + b) * c) q, size_t a, size_t b, size_t c, + std::span sp, std::span sp2, size_t i, size_t j, size_t k) { + a = i; + b = j; + c = k; + p = sp.first(i * j).data(); + q = sp2.first((i + j) * k).data(); +} + // Inout pointer and count void good_inout_span(int *__counted_by(*count) *p, size_t *count, std::span sp) { @@ -77,7 +319,7 @@ void good_inout_span(int *__counted_by(*count) *p, size_t *count, std::span } void bad_inout_span(int *__counted_by(*count) *p, size_t *count, std::span sp) { - *p = sp.data(); // TODO-expected-warning{{unsafe assignment to count-attributed pointer}} + *p = sp.data(); // expected-warning{{unsafe assignment to count-attributed pointer}} *count = 42; } @@ -105,7 +347,7 @@ void good_inout_span_if(int *__counted_by(*count) *p, size_t *count, std::span sp, size_t size) { if (p && count) { - *p = sp.data(); // TODO-expected-warning{{unsafe assignment to count-attributed pointer}} + *p = sp.data(); // expected-warning{{unsafe assignment to count-attributed pointer}} *count = size; } } @@ -117,7 +359,7 @@ class inout_class { } void bad_inout_span(int *__counted_by(*count) *p, size_t *count, std::span sp) { - *p = sp.data(); // TODO-expected-warning{{unsafe assignment to count-attributed pointer}} + *p = sp.data(); // expected-warning{{unsafe assignment to count-attributed pointer}} *count = 42; } @@ -130,7 +372,7 @@ class inout_class { // Inout pointer void bad_inout_ptr_span(int *__counted_by(count) *p, int count, std::span sp) { - *p = sp.data(); // TODO-expected-warning{{unsafe assignment to count-attributed pointer}} + *p = sp.data(); // expected-warning{{unsafe assignment to count-attributed pointer}} } void good_inout_ptr_subspan(int *__counted_by(count) *p, size_t count, std::span sp) { @@ -147,7 +389,7 @@ void good_inout_ptr_multi_subspan(int *__counted_by(a + b) *p, size_t a, size_t class inout_ptr_class { void bad_inout_ptr_span(int *__counted_by(count) *p, int count, std::span sp) { - *p = sp.data(); // TODO-expected-warning{{unsafe assignment to count-attributed pointer}} + *p = sp.data(); // expected-warning{{unsafe assignment to count-attributed pointer}} } void good_inout_ptr_subspan(int *__counted_by(count) *p, size_t count, std::span sp) {