Skip to content

Commit 68a5ecf

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 (cherry picked from commit cb7dcc9)
1 parent 4a93e41 commit 68a5ecf

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

191196
/// 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
@@ -14130,6 +14130,9 @@ def warn_duplicated_assignment_in_bounds_attributed_group : Warning<
1413014130
def warn_assigned_and_used_in_bounds_attributed_group : Warning<
1413114131
"%select{variable|parameter|member}0 %1 is assigned and used in the same "
1413214132
"bounds-attributed group">, InGroup<UnsafeBufferUsage>, DefaultIgnore;
14133+
def warn_unsafe_count_attributed_pointer_assignment : Warning<
14134+
"unsafe assignment to count-attributed pointer">,
14135+
InGroup<UnsafeBufferUsage>, DefaultIgnore;
1413314136
#ifndef NDEBUG
1413414137
// Not a user-facing diagnostic. Useful for debugging false negatives in
1413514138
// -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
@@ -5857,6 +5857,60 @@ static bool checkAssignedAndUsed(const BoundsAttributedAssignmentGroup &Group,
58575857
return IsGroupSafe;
58585858
}
58595859

5860+
// Checks if each assignment to count-attributed pointer in the group is safe.
5861+
static bool
5862+
checkAssignmentPatterns(const BoundsAttributedAssignmentGroup &Group,
5863+
UnsafeBufferUsageHandler &Handler, ASTContext &Ctx) {
5864+
// Collect dependent values.
5865+
DependentValuesTy DependentValues;
5866+
for (size_t I = 0, N = Group.AssignedObjects.size(); I < N; ++I) {
5867+
const ValueDecl *VD = Group.AssignedObjects[I].Decl;
5868+
const auto *Attr = VD->getAttr<DependerDeclsAttr>();
5869+
if (!Attr)
5870+
continue;
5871+
5872+
const BinaryOperator *Assign = Group.Assignments[I];
5873+
const Expr *Value = Assign->getRHS();
5874+
5875+
[[maybe_unused]] bool Inserted =
5876+
DependentValues.insert({{VD, Attr->getIsDeref()}, Value}).second;
5877+
// Previous checks in `checkBoundsAttributedGroup` should have validated
5878+
// that we have only a single assignment.
5879+
assert(Inserted);
5880+
}
5881+
5882+
bool IsGroupSafe = true;
5883+
5884+
// Check every pointer in the group.
5885+
for (size_t I = 0, N = Group.AssignedObjects.size(); I < N; ++I) {
5886+
const ValueDecl *VD = Group.AssignedObjects[I].Decl;
5887+
5888+
QualType Ty = VD->getType();
5889+
const auto *CAT = Ty->getAs<CountAttributedType>();
5890+
if (!CAT && Ty->isPointerType())
5891+
CAT = Ty->getPointeeType()->getAs<CountAttributedType>();
5892+
if (!CAT)
5893+
continue;
5894+
5895+
const BinaryOperator *Assign = Group.Assignments[I];
5896+
5897+
// TODO: Move this logic to isCountAttributedPointerArgumentSafeImpl.
5898+
const Expr *CountArg =
5899+
DependentValues.size() == 1 ? DependentValues.begin()->second : nullptr;
5900+
5901+
bool IsSafe = isCountAttributedPointerArgumentSafeImpl(
5902+
Ctx, Assign->getRHS(), CountArg, CAT, CAT->getCountExpr(),
5903+
CAT->isCountInBytes(), CAT->isOrNull(), &DependentValues);
5904+
if (!IsSafe) {
5905+
Handler.handleUnsafeCountAttributedPointerAssignment(
5906+
Assign, /*IsRelatedToDecl=*/false, Ctx);
5907+
IsGroupSafe = false;
5908+
}
5909+
}
5910+
5911+
return IsGroupSafe;
5912+
}
5913+
58605914
// Checks if the bounds-attributed group is safe. This function returns false
58615915
// iff the assignment group is unsafe and diagnostics have been emitted.
58625916
static bool
@@ -5868,8 +5922,7 @@ checkBoundsAttributedGroup(const BoundsAttributedAssignmentGroup &Group,
58685922
return false;
58695923
if (!checkAssignedAndUsed(Group, Handler, Ctx))
58705924
return false;
5871-
// TODO: Add more checks.
5872-
return true;
5925+
return checkAssignmentPatterns(Group, Handler, Ctx);
58735926
}
58745927

58755928
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
<< getBoundsAttributedObjectKind(VD) << VD;
26862686
S.Diag(Use->getBeginLoc(), diag::note_used_here);
26872687
}
2688+
2689+
void handleUnsafeCountAttributedPointerAssignment(
2690+
const BinaryOperator *Assign, [[maybe_unused]] bool IsRelatedToDecl,
2691+
[[maybe_unused]] 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)