Skip to content

[BoundsSafety][NFC] Simplify the interface of BoundsSafetyCheckAssignmentToCountAttrPtrWithIncompletePointeeTy #9807

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 8, 2025
Merged

[BoundsSafety][NFC] Simplify the interface of BoundsSafetyCheckAssignmentToCountAttrPtrWithIncompletePointeeTy #9807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions clang/include/clang/Sema/Sema.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2228,16 +2228,18 @@ class Sema final : public SemaBase {
/// \param RHSExpr The expression being assigned from.
/// \param Action The type assignment being performed
/// \param Loc The SourceLocation to use for error diagnostics
/// \param ComputeAssignee If provided this function will be called before
/// emitting a diagnostic. The function should return the name of
/// entity being assigned to or an empty string if this cannot be
/// determined.
/// \param Assignee The ValueDecl being assigned. This is used to compute
/// the name of the assignee. If the assignee isn't known this can
/// be set to nullptr.
/// \param ShowFullyQualifiedAssigneeName If set to true when using \p
/// Assignee to compute the name of the assignee use the fully
/// qualified name, otherwise use the unqualified name.
///
/// \returns True iff no diagnostic where emitted, false otherwise.
bool BoundsSafetyCheckAssignmentToCountAttrPtr(
QualType LHSTy, Expr *RHSExpr, AssignmentAction Action,
SourceLocation Loc,
std::function<std::string()> ComputeAssignee = nullptr);
SourceLocation Loc, const ValueDecl *Assignee,
bool ShowFullyQualifiedAssigneeName);

/// Perform Checks for assigning to a `__counted_by` or
/// `__counted_by_or_null` pointer type \param LHSTy where the pointee type
Expand All @@ -2248,15 +2250,18 @@ class Sema final : public SemaBase {
/// \param RHSExpr The expression being assigned from.
/// \param Action The type assignment being performed
/// \param Loc The SourceLocation to use for error diagnostics
/// \param ComputeAssignee If provided this function will be called before
/// emitting a diagnostic. The function should return the name of
/// entity being assigned to or an empty string if this cannot be
/// determined.
/// \param Assignee The ValueDecl being assigned. This is used to compute
/// the name of the assignee. If the assignee isn't known this can
/// be set to nullptr.
/// \param ShowFullyQualifiedAssigneeName If set to true when using \p
/// Assignee to compute the name of the assignee use the fully
/// qualified name, otherwise use the unqualified name.
///
/// \returns True iff no diagnostic where emitted, false otherwise.
bool BoundsSafetyCheckAssignmentToCountAttrPtrWithIncompletePointeeTy(
QualType LHSTy, Expr *RHSExpr, AssignmentAction Action,
SourceLocation Loc, std::function<std::string()> ComputeAssignee);
SourceLocation Loc, const ValueDecl *Assignee,
bool ShowFullyQualifiedAssigneeName);

/// Perform Bounds Safety Semantic checks for initializing a Bounds Safety
/// pointer.
Expand Down
45 changes: 24 additions & 21 deletions clang/lib/Sema/SemaBoundsSafety.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,8 +463,8 @@ HasCountedByAttrOnIncompletePointee(QualType Ty, NamedDecl **ND,
}

bool Sema::BoundsSafetyCheckAssignmentToCountAttrPtrWithIncompletePointeeTy(
QualType LHSTy, Expr *RHSExpr, AssignmentAction Action,
SourceLocation Loc, std::function<std::string()> ComputeAssignee) {
QualType LHSTy, Expr *RHSExpr, AssignmentAction Action, SourceLocation Loc,
const ValueDecl *Assignee, bool ShowFullyQualifiedAssigneeName) {
NamedDecl *IncompleteTyDecl = nullptr;
const CountAttributedType *CATy = nullptr;
QualType PointeeTy;
Expand All @@ -481,13 +481,20 @@ bool Sema::BoundsSafetyCheckAssignmentToCountAttrPtrWithIncompletePointeeTy(
Action != AssignmentAction::Casting &&
Action != AssignmentAction::Passing_CFAudited);

// By having users provide a function we only pay the cost of allocation the
// string and computing when a diagnostic is emitted.
std::string Assignee = ComputeAssignee ? ComputeAssignee() : "";
std::string AssigneeStr;
if (Assignee) {
if (ShowFullyQualifiedAssigneeName) {
AssigneeStr = Assignee->getQualifiedNameAsString();
} else {
AssigneeStr = Assignee->getNameAsString();
}
}
{
auto D =
Diag(Loc, diag::err_bounds_safety_counted_by_on_incomplete_type_on_assign)
<< /*0*/ (int)Action << /*1*/ Assignee << /*2*/ (Assignee.size() > 0)
Diag(Loc,
diag::err_bounds_safety_counted_by_on_incomplete_type_on_assign)
<< /*0*/ (int)Action << /*1*/ AssigneeStr
<< /*2*/ (AssigneeStr.size() > 0)
<< /*3*/ isa<ImplicitValueInitExpr>(RHSExpr) << /*4*/ LHSTy
<< /*5*/ CATy->GetAttributeName(/*WithMacroPrefix=*/true)
<< /*6*/ PointeeTy << /*7*/ CATy->isOrNull();
Expand All @@ -501,13 +508,13 @@ bool Sema::BoundsSafetyCheckAssignmentToCountAttrPtrWithIncompletePointeeTy(
}

bool Sema::BoundsSafetyCheckAssignmentToCountAttrPtr(
QualType LHSTy, Expr *RHSExpr, AssignmentAction Action,
SourceLocation Loc, std::function<std::string()> ComputeAssignee) {
QualType LHSTy, Expr *RHSExpr, AssignmentAction Action, SourceLocation Loc,
const ValueDecl *Assignee, bool ShowFullyQualifiedAssigneeName) {
if (!getLangOpts().hasBoundsSafety())
return true;

return BoundsSafetyCheckAssignmentToCountAttrPtrWithIncompletePointeeTy(
LHSTy, RHSExpr, Action, Loc, ComputeAssignee);
LHSTy, RHSExpr, Action, Loc, Assignee, ShowFullyQualifiedAssigneeName);
}

bool Sema::BoundsSafetyCheckInitialization(const InitializedEntity &Entity,
Expand All @@ -531,13 +538,10 @@ bool Sema::BoundsSafetyCheckInitialization(const InitializedEntity &Entity,
Entity.getKind() != InitializedEntity::EK_Variable) {

if (!BoundsSafetyCheckAssignmentToCountAttrPtrWithIncompletePointeeTy(
LHSType, RHSExpr, Action, SL, [&Entity]() -> std::string {
if (const auto *VD =
dyn_cast_or_null<ValueDecl>(Entity.getDecl())) {
return VD->getQualifiedNameAsString();
}
return "";
})) {
LHSType, RHSExpr, Action, SL,
dyn_cast_or_null<ValueDecl>(Entity.getDecl()),
/*ShowFullQualifiedAssigneeName=*/true)) {

ChecksPassed = false;

// It's not necessarily bad if this assert fails but we should catch
Expand Down Expand Up @@ -629,13 +633,12 @@ bool Sema::BoundsSafetyCheckResolvedCall(FunctionDecl *FDecl, CallExpr *Call,

for (size_t ArgIdx = 0; ArgIdx < MinNumArgs; ++ArgIdx) {
Expr *CallArg = Call->getArg(ArgIdx); // FIXME: IgnoreImpCast()?
StringRef ParamName;
const ValueDecl *ParamVarDecl = nullptr;
QualType ParamTy;
if (FDecl) {
// Direct call
auto *ParamVarDecl = FDecl->getParamDecl(ArgIdx);
ParamVarDecl = FDecl->getParamDecl(ArgIdx);
ParamTy = ParamVarDecl->getType();
ParamName = ParamVarDecl->getName();
} else {
// Indirect call. The parameter name isn't known
ParamTy = ProtoType->getParamType(ArgIdx);
Expand All @@ -644,7 +647,7 @@ bool Sema::BoundsSafetyCheckResolvedCall(FunctionDecl *FDecl, CallExpr *Call,
// Assigning to the parameter type is treated as a "use" of the type.
if (!BoundsSafetyCheckAssignmentToCountAttrPtr(
ParamTy, CallArg, AssignmentAction::Passing, CallArg->getBeginLoc(),
[&ParamName]() { return ParamName.str(); }))
ParamVarDecl, /*ShowFullQualifiedAssigneeName=*/false))
ChecksPassed = false;
}
return ChecksPassed;
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Sema/SemaChecking.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9503,8 +9503,8 @@ Sema::CheckReturnValExpr(Expr *RetValExp, QualType lhsType,

/* TO_UPSTREAM(BoundsSafety) ON*/
BoundsSafetyCheckAssignmentToCountAttrPtr(
lhsType, RetValExp, AssignmentAction::Returning,
RetValExp->getBeginLoc());
lhsType, RetValExp, AssignmentAction::Returning, RetValExp->getBeginLoc(),
/*Assignee=*/nullptr, /*ShowFullQualifiedAssigneeName=*/false);

// For a count-attributed return type, its dependent count variables can be
// assigned in arbitrary places. Don't try to find the assigned values, just
Expand Down
27 changes: 13 additions & 14 deletions clang/lib/Sema/SemaExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16730,20 +16730,19 @@ QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, ExprResult &RHS,
// Even if this check fails don't return early to allow the best
// possible error recovery and to allow any subsequent diagnostics to
// work.
const ValueDecl *Assignee = nullptr;
bool ShowFullyQualifiedAssigneeName = false;
// In simple cases describe what is being assigned to
if (auto *DR = dyn_cast<DeclRefExpr>(LHSExpr->IgnoreParenCasts())) {
Assignee = DR->getDecl();
} else if (auto *ME = dyn_cast<MemberExpr>(LHSExpr->IgnoreParenCasts())) {
Assignee = ME->getMemberDecl();
ShowFullyQualifiedAssigneeName = true;
}

(void)BoundsSafetyCheckAssignmentToCountAttrPtr(
LHSType, RHS.get(), AssignmentAction::Assigning, Loc,
[&LHSExpr]() -> std::string {
// In simple cases describe what is being assigned to
if (auto *DR = dyn_cast<DeclRefExpr>(LHSExpr->IgnoreParenCasts())) {
auto *II = DR->getDecl()->getDeclName().getAsIdentifierInfo();
if (II)
return II->getName().str();
} else if (auto *ME =
dyn_cast<MemberExpr>(LHSExpr->IgnoreParenCasts())) {
return ME->getMemberDecl()->getQualifiedNameAsString();
}
return "";
});
LHSType, RHS.get(), AssignmentAction::Assigning, Loc, Assignee,
ShowFullyQualifiedAssigneeName);
}
/* TO_UPSTREAM(BoundsSafety) OFF */

Expand Down Expand Up @@ -26280,4 +26279,4 @@ ExprResult Sema::ActOnUnsafeTerminatedByFromIndexable(
PointerToTerminatorExpr, BuiltinLoc,
RParenLoc);
}
/* TO_UPSTREAM(BoundsSafety) OFF*/
/* TO_UPSTREAM(BoundsSafety) OFF*/