Skip to content

Commit cb7dcc9

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 9ae86a7 commit cb7dcc9

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
@@ -187,6 +187,11 @@ class UnsafeBufferUsageHandler {
187187
bool IsRelatedToDecl, ASTContext &Ctx) {
188188
handleUnsafeOperation(Assign, IsRelatedToDecl, Ctx);
189189
}
190+
191+
virtual void handleUnsafeCountAttributedPointerAssignment(
192+
const BinaryOperator *Assign, bool IsRelatedToDecl, ASTContext &Ctx) {
193+
handleUnsafeOperation(Assign, IsRelatedToDecl, Ctx);
194+
}
190195
/* TO_UPSTREAM(BoundsSafety) OFF */
191196

192197
/// 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
@@ -14296,6 +14296,9 @@ def warn_duplicated_assignment_in_bounds_attributed_group : Warning<
1429614296
def warn_assigned_and_used_in_bounds_attributed_group : Warning<
1429714297
"%select{variable|parameter|member}0 %1 is assigned and used in the same "
1429814298
"bounds-attributed group">, InGroup<UnsafeBufferUsage>, DefaultIgnore;
14299+
def warn_unsafe_count_attributed_pointer_assignment : Warning<
14300+
"unsafe assignment to count-attributed pointer">,
14301+
InGroup<UnsafeBufferUsage>, DefaultIgnore;
1429914302
#ifndef NDEBUG
1430014303
// Not a user-facing diagnostic. Useful for debugging false negatives in
1430114304
// -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
@@ -6021,6 +6021,60 @@ static bool checkAssignedAndUsed(const BoundsAttributedAssignmentGroup &Group,
60216021
return IsGroupSafe;
60226022
}
60236023

6024+
// Checks if each assignment to count-attributed pointer in the group is safe.
6025+
static bool
6026+
checkAssignmentPatterns(const BoundsAttributedAssignmentGroup &Group,
6027+
UnsafeBufferUsageHandler &Handler, ASTContext &Ctx) {
6028+
// Collect dependent values.
6029+
DependentValuesTy DependentValues;
6030+
for (size_t I = 0, N = Group.AssignedObjects.size(); I < N; ++I) {
6031+
const ValueDecl *VD = Group.AssignedObjects[I].Decl;
6032+
const auto *Attr = VD->getAttr<DependerDeclsAttr>();
6033+
if (!Attr)
6034+
continue;
6035+
6036+
const BinaryOperator *Assign = Group.Assignments[I];
6037+
const Expr *Value = Assign->getRHS();
6038+
6039+
[[maybe_unused]] bool Inserted =
6040+
DependentValues.insert({{VD, Attr->getIsDeref()}, Value}).second;
6041+
// Previous checks in `checkBoundsAttributedGroup` should have validated
6042+
// that we have only a single assignment.
6043+
assert(Inserted);
6044+
}
6045+
6046+
bool IsGroupSafe = true;
6047+
6048+
// Check every pointer in the group.
6049+
for (size_t I = 0, N = Group.AssignedObjects.size(); I < N; ++I) {
6050+
const ValueDecl *VD = Group.AssignedObjects[I].Decl;
6051+
6052+
QualType Ty = VD->getType();
6053+
const auto *CAT = Ty->getAs<CountAttributedType>();
6054+
if (!CAT && Ty->isPointerType())
6055+
CAT = Ty->getPointeeType()->getAs<CountAttributedType>();
6056+
if (!CAT)
6057+
continue;
6058+
6059+
const BinaryOperator *Assign = Group.Assignments[I];
6060+
6061+
// TODO: Move this logic to isCountAttributedPointerArgumentSafeImpl.
6062+
const Expr *CountArg =
6063+
DependentValues.size() == 1 ? DependentValues.begin()->second : nullptr;
6064+
6065+
bool IsSafe = isCountAttributedPointerArgumentSafeImpl(
6066+
Ctx, Assign->getRHS(), CountArg, CAT, CAT->getCountExpr(),
6067+
CAT->isCountInBytes(), CAT->isOrNull(), &DependentValues);
6068+
if (!IsSafe) {
6069+
Handler.handleUnsafeCountAttributedPointerAssignment(
6070+
Assign, /*IsRelatedToDecl=*/false, Ctx);
6071+
IsGroupSafe = false;
6072+
}
6073+
}
6074+
6075+
return IsGroupSafe;
6076+
}
6077+
60246078
// Checks if the bounds-attributed group is safe. This function returns false
60256079
// iff the assignment group is unsafe and diagnostics have been emitted.
60266080
static bool
@@ -6032,8 +6086,7 @@ checkBoundsAttributedGroup(const BoundsAttributedAssignmentGroup &Group,
60326086
return false;
60336087
if (!checkAssignedAndUsed(Group, Handler, Ctx))
60346088
return false;
6035-
// TODO: Add more checks.
6036-
return true;
6089+
return checkAssignmentPatterns(Group, Handler, Ctx);
60376090
}
60386091

60396092
static void

clang/lib/Sema/AnalysisBasedWarnings.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2695,6 +2695,13 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
26952695
<< getBoundsAttributedObjectKind(VD) << VD;
26962696
S.Diag(Use->getBeginLoc(), diag::note_used_here);
26972697
}
2698+
2699+
void handleUnsafeCountAttributedPointerAssignment(
2700+
const BinaryOperator *Assign, [[maybe_unused]] bool IsRelatedToDecl,
2701+
[[maybe_unused]] ASTContext &Ctx) override {
2702+
S.Diag(Assign->getOperatorLoc(),
2703+
diag::warn_unsafe_count_attributed_pointer_assignment);
2704+
}
26982705
/* TO_UPSTREAM(BoundsSafety) OFF */
26992706

27002707
void handleUnsafeVariableGroup(const VarDecl *Variable,

0 commit comments

Comments
 (0)