Skip to content

Commit 12d943e

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 61571ad commit 12d943e

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
@@ -6020,6 +6020,60 @@ static bool checkAssignedAndUsed(const BoundsAttributedAssignmentGroup &Group,
60206020
return IsGroupSafe;
60216021
}
60226022

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

60386091
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)