[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 13 09:25:06 PDT 2024
https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627
>From 2d9cc933aa8dc6819c3b0a08f5457bad98e8e6df Mon Sep 17 00:00:00 2001
From: higher-performance <higher.performance.github at gmail.com>
Date: Fri, 6 Sep 2024 14:16:15 -0400
Subject: [PATCH 1/2] Propagate lifetimebound from formal parameters to those
in the canonical declaration, then use the canonical declaration for analysis
Note that this doesn't handle the implicit 'this' parameter; that can be addressed in a separate commit.
---
clang/lib/Sema/CheckExprLifetime.cpp | 15 +++++-----
clang/lib/Sema/SemaAttr.cpp | 34 +++++++++++++++--------
clang/test/SemaCXX/attr-lifetimebound.cpp | 4 +++
3 files changed, 34 insertions(+), 19 deletions(-)
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index f62e18543851c1..1b1220456ded85 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -462,14 +462,15 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
}
}
- for (unsigned I = 0,
- N = std::min<unsigned>(Callee->getNumParams(), Args.size());
- I != N; ++I) {
- if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr<LifetimeBoundAttr>())
- VisitLifetimeBoundArg(Callee->getParamDecl(I), Args[I]);
+ const FunctionDecl *CanonCallee = Callee->getCanonicalDecl();
+ unsigned NP = std::min(Callee->getNumParams(), CanonCallee->getNumParams());
+ for (unsigned I = 0, N = std::min<unsigned>(NP, Args.size()); I != N; ++I) {
+ if (CheckCoroCall ||
+ CanonCallee->getParamDecl(I)->hasAttr<LifetimeBoundAttr>())
+ VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Args[I]);
else if (EnableGSLAnalysis && I == 0) {
- if (shouldTrackFirstArgument(Callee)) {
- VisitGSLPointerArg(Callee, Args[0]);
+ if (shouldTrackFirstArgument(CanonCallee)) {
+ VisitGSLPointerArg(CanonCallee, Args[0]);
} else if (auto *CCE = dyn_cast<CXXConstructExpr>(Call);
CCE &&
CCE->getConstructor()->getParent()->hasAttr<PointerAttr>()) {
diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp
index fb594e6f13c0b3..acdfe190d9f19b 100644
--- a/clang/lib/Sema/SemaAttr.cpp
+++ b/clang/lib/Sema/SemaAttr.cpp
@@ -217,7 +217,8 @@ void Sema::inferGslOwnerPointerAttribute(CXXRecordDecl *Record) {
}
void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) {
- if (FD->getNumParams() == 0)
+ unsigned NumParams = FD->getNumParams();
+ if (NumParams == 0)
return;
if (unsigned BuiltinID = FD->getBuiltinID()) {
@@ -239,18 +240,13 @@ void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) {
default:
break;
}
- return;
- }
- if (auto *CMD = dyn_cast<CXXMethodDecl>(FD)) {
- const auto *CRD = CMD->getParent();
- if (!CRD->isInStdNamespace() || !CRD->getIdentifier())
- return;
-
- if (isa<CXXConstructorDecl>(CMD)) {
+ } else if (auto *CMD = dyn_cast<CXXMethodDecl>(FD)) {
+ const CXXRecordDecl *CRD = CMD->getParent();
+ if (CRD->isInStdNamespace() && CRD->getIdentifier() &&
+ isa<CXXConstructorDecl>(CMD)) {
auto *Param = CMD->getParamDecl(0);
- if (Param->hasAttr<LifetimeBoundAttr>())
- return;
- if (CRD->getName() == "basic_string_view" &&
+ if (!Param->hasAttr<LifetimeBoundAttr>() &&
+ CRD->getName() == "basic_string_view" &&
Param->getType()->isPointerType()) {
// construct from a char array pointed by a pointer.
// basic_string_view(const CharT* s);
@@ -266,6 +262,20 @@ void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) {
LifetimeBoundAttr::CreateImplicit(Context, FD->getLocation()));
}
}
+ } else if (auto *CanonDecl = FD->getCanonicalDecl(); FD != CanonDecl) {
+ // Propagate the lifetimebound attribute from parameters to the canonical
+ // declaration.
+ // Note that this doesn't include the implicit 'this' parameter, as the
+ // attribute is applied to the function type in that case.
+ unsigned NP = std::min(NumParams, CanonDecl->getNumParams());
+ for (unsigned I = 0; I < NP; ++I) {
+ auto *CanonParam = CanonDecl->getParamDecl(I);
+ if (!CanonParam->hasAttr<LifetimeBoundAttr>() &&
+ FD->getParamDecl(I)->hasAttr<LifetimeBoundAttr>()) {
+ CanonParam->addAttr(LifetimeBoundAttr::CreateImplicit(
+ Context, CanonParam->getLocation()));
+ }
+ }
}
}
diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp b/clang/test/SemaCXX/attr-lifetimebound.cpp
index 0fb997a5671085..c0255e9a9f7280 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -19,6 +19,9 @@ namespace usage_invalid {
namespace usage_ok {
struct IntRef { int *target; };
+ const int &crefparam(const int ¶m); // Omitted in first decl
+ const int &crefparam(const int ¶m [[clang::lifetimebound]]); // Add LB
+ const int &crefparam(const int ¶m) { return param; } // Omit in impl
int &refparam(int ¶m [[clang::lifetimebound]]);
int &classparam(IntRef param [[clang::lifetimebound]]);
@@ -41,6 +44,7 @@ namespace usage_ok {
int *p = A().class_member(); // expected-warning {{temporary whose address is used as value of local variable 'p' will be destroyed at the end of the full-expression}}
int *q = A(); // expected-warning {{temporary whose address is used as value of local variable 'q' will be destroyed at the end of the full-expression}}
int *r = A(1); // expected-warning {{temporary whose address is used as value of local variable 'r' will be destroyed at the end of the full-expression}}
+ const int& s = crefparam(2); // expected-warning {{temporary bound to local reference 's' will be destroyed at the end of the full-expression}}
void test_assignment() {
p = A().class_member(); // expected-warning {{object backing the pointer p will be destroyed at the end of the full-expression}}
>From 4b621584e91f60c4aa2e0b6159bd1c0db7aef839 Mon Sep 17 00:00:00 2001
From: higher-performance <higher.performance.github at gmail.com>
Date: Fri, 13 Sep 2024 12:17:52 -0400
Subject: [PATCH 2/2] Undo changes to inferLifetimeBoundAttribute and use
mergeParamDeclAttributes instead
---
clang/lib/Sema/SemaAttr.cpp | 34 ++++------
clang/lib/Sema/SemaDecl.cpp | 79 ++++++++++++++++-------
clang/test/SemaCXX/attr-lifetimebound.cpp | 1 +
3 files changed, 69 insertions(+), 45 deletions(-)
diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp
index acdfe190d9f19b..fb594e6f13c0b3 100644
--- a/clang/lib/Sema/SemaAttr.cpp
+++ b/clang/lib/Sema/SemaAttr.cpp
@@ -217,8 +217,7 @@ void Sema::inferGslOwnerPointerAttribute(CXXRecordDecl *Record) {
}
void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) {
- unsigned NumParams = FD->getNumParams();
- if (NumParams == 0)
+ if (FD->getNumParams() == 0)
return;
if (unsigned BuiltinID = FD->getBuiltinID()) {
@@ -240,13 +239,18 @@ void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) {
default:
break;
}
- } else if (auto *CMD = dyn_cast<CXXMethodDecl>(FD)) {
- const CXXRecordDecl *CRD = CMD->getParent();
- if (CRD->isInStdNamespace() && CRD->getIdentifier() &&
- isa<CXXConstructorDecl>(CMD)) {
+ return;
+ }
+ if (auto *CMD = dyn_cast<CXXMethodDecl>(FD)) {
+ const auto *CRD = CMD->getParent();
+ if (!CRD->isInStdNamespace() || !CRD->getIdentifier())
+ return;
+
+ if (isa<CXXConstructorDecl>(CMD)) {
auto *Param = CMD->getParamDecl(0);
- if (!Param->hasAttr<LifetimeBoundAttr>() &&
- CRD->getName() == "basic_string_view" &&
+ if (Param->hasAttr<LifetimeBoundAttr>())
+ return;
+ if (CRD->getName() == "basic_string_view" &&
Param->getType()->isPointerType()) {
// construct from a char array pointed by a pointer.
// basic_string_view(const CharT* s);
@@ -262,20 +266,6 @@ void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) {
LifetimeBoundAttr::CreateImplicit(Context, FD->getLocation()));
}
}
- } else if (auto *CanonDecl = FD->getCanonicalDecl(); FD != CanonDecl) {
- // Propagate the lifetimebound attribute from parameters to the canonical
- // declaration.
- // Note that this doesn't include the implicit 'this' parameter, as the
- // attribute is applied to the function type in that case.
- unsigned NP = std::min(NumParams, CanonDecl->getNumParams());
- for (unsigned I = 0; I < NP; ++I) {
- auto *CanonParam = CanonDecl->getParamDecl(I);
- if (!CanonParam->hasAttr<LifetimeBoundAttr>() &&
- FD->getParamDecl(I)->hasAttr<LifetimeBoundAttr>()) {
- CanonParam->addAttr(LifetimeBoundAttr::CreateImplicit(
- Context, CanonParam->getLocation()));
- }
- }
}
}
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 8557c25b93a8da..e136faf12d677c 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -3226,10 +3226,44 @@ void Sema::mergeDeclAttributes(NamedDecl *New, Decl *Old,
if (!foundAny) New->dropAttrs();
}
+template <class T>
+static unsigned propagateAttribute(ParmVarDecl *toDecl,
+ const ParmVarDecl *fromDecl, Sema &S) {
+ unsigned found = 0;
+ for (const auto *I : fromDecl->specific_attrs<T>()) {
+ if (!DeclHasAttr(toDecl, I)) {
+ T *newAttr = cast<T>(I->clone(S.Context));
+ newAttr->setInherited(true);
+ toDecl->addAttr(newAttr);
+ ++found;
+ }
+ }
+ return found;
+}
+
+template <class F>
+static void propagateAttributes(ParmVarDecl *toDecl,
+ const ParmVarDecl *fromDecl, F &&propagator) {
+ if (!fromDecl->hasAttrs()) {
+ return;
+ }
+
+ bool foundAny = toDecl->hasAttrs();
+
+ // Ensure that any moving of objects within the allocated map is
+ // done before we process them.
+ if (!foundAny)
+ toDecl->setAttrs(AttrVec());
+
+ foundAny |= std::forward<F>(propagator)(toDecl, fromDecl) != 0;
+
+ if (!foundAny)
+ toDecl->dropAttrs();
+}
+
/// mergeParamDeclAttributes - Copy attributes from the old parameter
/// to the new one.
-static void mergeParamDeclAttributes(ParmVarDecl *newDecl,
- const ParmVarDecl *oldDecl,
+static void mergeParamDeclAttributes(ParmVarDecl *newDecl, ParmVarDecl *oldDecl,
Sema &S) {
// C++11 [dcl.attr.depend]p2:
// The first declaration of a function shall specify the
@@ -3249,26 +3283,25 @@ static void mergeParamDeclAttributes(ParmVarDecl *newDecl,
diag::note_carries_dependency_missing_first_decl) << 1/*Param*/;
}
- if (!oldDecl->hasAttrs())
- return;
-
- bool foundAny = newDecl->hasAttrs();
-
- // Ensure that any moving of objects within the allocated map is
- // done before we process them.
- if (!foundAny) newDecl->setAttrs(AttrVec());
-
- for (const auto *I : oldDecl->specific_attrs<InheritableParamAttr>()) {
- if (!DeclHasAttr(newDecl, I)) {
- InheritableAttr *newAttr =
- cast<InheritableParamAttr>(I->clone(S.Context));
- newAttr->setInherited(true);
- newDecl->addAttr(newAttr);
- foundAny = true;
- }
- }
+ // Forward propagation (from old parameter to new)
+ propagateAttributes(
+ newDecl, oldDecl, [&S](ParmVarDecl *toDecl, const ParmVarDecl *fromDecl) {
+ unsigned found = 0;
+ found += propagateAttribute<InheritableParamAttr>(toDecl, fromDecl, S);
+ return found;
+ });
- if (!foundAny) newDecl->dropAttrs();
+ // Backward propagation (from new parameter to old)
+ propagateAttributes(
+ oldDecl, newDecl, [&S](ParmVarDecl *toDecl, const ParmVarDecl *fromDecl) {
+ unsigned found = 0;
+ // Propagate the lifetimebound attribute from parameters to the
+ // canonical declaration. Note that this doesn't include the implicit
+ // 'this' parameter, as the attribute is applied to the function type in
+ // that case.
+ found += propagateAttribute<LifetimeBoundAttr>(toDecl, fromDecl, S);
+ return found;
+ });
}
static bool EquivalentArrayTypes(QualType Old, QualType New,
@@ -4319,8 +4352,8 @@ void Sema::mergeObjCMethodDecls(ObjCMethodDecl *newMethod,
mergeDeclAttributes(newMethod, oldMethod, MergeKind);
// Merge attributes from the parameters.
- ObjCMethodDecl::param_const_iterator oi = oldMethod->param_begin(),
- oe = oldMethod->param_end();
+ ObjCMethodDecl::param_iterator oi = oldMethod->param_begin(),
+ oe = oldMethod->param_end();
for (ObjCMethodDecl::param_iterator
ni = newMethod->param_begin(), ne = newMethod->param_end();
ni != ne && oi != oe; ++ni, ++oi)
diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp b/clang/test/SemaCXX/attr-lifetimebound.cpp
index c0255e9a9f7280..38b6407f0cd03c 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -20,6 +20,7 @@ namespace usage_ok {
struct IntRef { int *target; };
const int &crefparam(const int ¶m); // Omitted in first decl
+ const int &crefparam(const int ¶m); // Omitted in second decl
const int &crefparam(const int ¶m [[clang::lifetimebound]]); // Add LB
const int &crefparam(const int ¶m) { return param; } // Omit in impl
int &refparam(int ¶m [[clang::lifetimebound]]);
More information about the cfe-commits
mailing list