Skip to content

Commit a73715e

Browse files
[-Wunsafe-buffer-usage] Check safe assignment patterns
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
1 parent 3147c6e commit a73715e

File tree

5 files changed

+325
-7
lines changed

5 files changed

+325
-7
lines changed

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,11 @@ class UnsafeBufferUsageHandler {
181181
ASTContext &Ctx) {
182182
handleUnsafeOperation(Assign, IsRelatedToDecl, Ctx);
183183
}
184+
185+
virtual void handleUnsafeCountAttributedPointerAssignment(
186+
const BinaryOperator *Assign, bool IsRelatedToDecl, ASTContext &Ctx) {
187+
handleUnsafeOperation(Assign, IsRelatedToDecl, Ctx);
188+
}
184189
/* TO_UPSTREAM(BoundsSafety) OFF */
185190

186191
/// Invoked when a fix is suggested against a variable. This function groups

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14291,6 +14291,9 @@ def warn_missing_assignments_in_bounds_attributed_group : Warning<
1429114291
def warn_duplicated_assignment_in_bounds_attributed_group : Warning<
1429214292
"duplicated assignment to %select{variable|parameter|member}0 %1 in "
1429314293
"bounds-attributed group">, InGroup<UnsafeBufferUsage>, DefaultIgnore;
14294+
def warn_unsafe_count_attributed_pointer_assignment : Warning<
14295+
"unsafe assignment to count-attributed pointer">,
14296+
InGroup<UnsafeBufferUsage>, DefaultIgnore;
1429414297
#ifndef NDEBUG
1429514298
// Not a user-facing diagnostic. Useful for debugging false negatives in
1429614299
// -fsafe-buffer-usage-suggestions (i.e. lack of -Wunsafe-buffer-usage fixits).

clang/lib/Analysis/UnsafeBufferUsage.cpp

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5947,6 +5947,60 @@ static bool checkMissingAndDuplicatedAssignments(
59475947
return IsGroupSafe;
59485948
}
59495949

5950+
// Checks if each assignment to count-attributed pointer in the group is safe.
5951+
static bool
5952+
checkAssignmentPatterns(const BoundsAttributedAssignmentGroup &Group,
5953+
UnsafeBufferUsageHandler &Handler, ASTContext &Ctx) {
5954+
// Collect dependent values.
5955+
DependentValuesTy DependentValues;
5956+
for (size_t I = 0, N = Group.AssignedObjects.size(); I < N; ++I) {
5957+
const ValueDecl *VD = Group.AssignedObjects[I].Decl;
5958+
const auto *Attr = VD->getAttr<DependerDeclsAttr>();
5959+
if (!Attr)
5960+
continue;
5961+
5962+
const BinaryOperator *Assign = Group.Assignments[I];
5963+
const Expr *Value = Assign->getRHS();
5964+
5965+
[[maybe_unused]] bool Inserted =
5966+
DependentValues.insert({{VD, Attr->getIsDeref()}, Value}).second;
5967+
// Previous checks in `checkBoundsAttributedGroup` should have validated
5968+
// that we have only a single assignment.
5969+
assert(Inserted);
5970+
}
5971+
5972+
bool IsGroupSafe = true;
5973+
5974+
// Check every pointer in the group.
5975+
for (size_t I = 0, N = Group.AssignedObjects.size(); I < N; ++I) {
5976+
const ValueDecl *VD = Group.AssignedObjects[I].Decl;
5977+
5978+
QualType Ty = VD->getType();
5979+
const auto *CAT = Ty->getAs<CountAttributedType>();
5980+
if (!CAT && Ty->isPointerType())
5981+
CAT = Ty->getPointeeType()->getAs<CountAttributedType>();
5982+
if (!CAT)
5983+
continue;
5984+
5985+
const BinaryOperator *Assign = Group.Assignments[I];
5986+
5987+
// TODO: Move this logic to isCountAttributedPointerArgumentSafeImpl.
5988+
const Expr *CountArg =
5989+
DependentValues.size() == 1 ? DependentValues.begin()->second : nullptr;
5990+
5991+
bool IsSafe = isCountAttributedPointerArgumentSafeImpl(
5992+
Ctx, Assign->getRHS(), CountArg, CAT, CAT->getCountExpr(),
5993+
CAT->isCountInBytes(), CAT->isOrNull(), &DependentValues);
5994+
if (!IsSafe) {
5995+
Handler.handleUnsafeCountAttributedPointerAssignment(
5996+
Assign, /*IsRelatedToDecl=*/false, Ctx);
5997+
IsGroupSafe = false;
5998+
}
5999+
}
6000+
6001+
return IsGroupSafe;
6002+
}
6003+
59506004
// Checks if the bounds-attributed group is safe. This function returns false
59516005
// iff the assignment group is unsafe and diagnostics have been emitted.
59526006
static bool
@@ -5956,8 +6010,7 @@ checkBoundsAttributedGroup(const BoundsAttributedAssignmentGroup &Group,
59566010
return false;
59576011
if (!checkMissingAndDuplicatedAssignments(Group, Handler, Ctx))
59586012
return false;
5959-
// TODO: Add more checks.
5960-
return true;
6013+
return checkAssignmentPatterns(Group, Handler, Ctx);
59616014
}
59626015

59636016
static void

clang/lib/Sema/AnalysisBasedWarnings.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2685,6 +2685,13 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
26852685
S.Diag(PrevAssign->getOperatorLoc(),
26862686
diag::note_bounds_safety_previous_assignment);
26872687
}
2688+
2689+
void handleUnsafeCountAttributedPointerAssignment(
2690+
const BinaryOperator *Assign, [[maybe_unused]] bool IsRelatedToDecl,
2691+
ASTContext &Ctx) override {
2692+
S.Diag(Assign->getOperatorLoc(),
2693+
diag::warn_unsafe_count_attributed_pointer_assignment);
2694+
}
26882695
/* TO_UPSTREAM(BoundsSafety) OFF */
26892696

26902697
void handleUnsafeVariableGroup(const VarDecl *Variable,

0 commit comments

Comments
 (0)