-
Notifications
You must be signed in to change notification settings - Fork 351
[-Wunsafe-buffer-usage] Check safe assignment patterns #11680
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[-Wunsafe-buffer-usage] Check safe assignment patterns #11680
Conversation
clang/test/SemaCXX/warn-unsafe-buffer-usage-count-attributed-pointer-assignment.cpp
Outdated
Show resolved
Hide resolved
clang/test/SemaCXX/warn-unsafe-buffer-usage-count-attributed-pointer-assignment.cpp
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM other than what I've commented on
a73715e to
77d721c
Compare
| QualType Ty = VD->getType(); | ||
| const auto *CAT = Ty->getAs<CountAttributedType>(); | ||
| if (!CAT && Ty->isPointerType()) | ||
| CAT = Ty->getPointeeType()->getAs<CountAttributedType>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have tests covering this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, those cases happen when we have a dereference on LHS (*ptr = ). We have tests like this:
void good_inout_span(int *__counted_by(*count) *p, size_t *count, std::span<int> sp) {
*p = sp.data();
*count = sp.size();
}|
|
||
| const BinaryOperator *Assign = Group.Assignments[I]; | ||
|
|
||
| // TODO: Move this logic to isCountAttributedPointerArgumentSafeImpl. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, the parameters of isCountAttributedPointerArgumentSafeImpl aren't the most intuitive. Technically, we do not need the caller to pass CountArg because the count expression with DependentValues already contain that information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will take care of this TODO in my refactoring patch.
|
@swift-ci test llvm |
Check safe assignment patterns. This uses the infrastructure that is already available for count-attributed arguments, and checks for each assigned pointer in the group that the RHS has enough elements. rdar://161608493
77d721c to
12d943e
Compare
|
@swift-ci test llvm |
Check safe assignment patterns. This uses the infrastructure that is already available for count-attributed arguments, and checks for each assigned pointer in the group that the RHS has enough elements.
rdar://161608493