[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
Tue Nov 26 09:13:19 PST 2024
https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627
>From dffc6abf0a1e4b1e8977bf9476bb65d808b46986 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/3] 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 | 5 ++++
3 files changed, 35 insertions(+), 19 deletions(-)
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 8886e5e307ddf86..e7c8a7591804efb 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -611,9 +611,9 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
}
}
- for (unsigned I = 0,
- N = std::min<unsigned>(Callee->getNumParams(), Args.size());
- I != N; ++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) {
Expr *Arg = Args[I];
RevertToOldSizeRAII RAII(Path);
if (auto *DAE = dyn_cast<CXXDefaultArgExpr>(Arg)) {
@@ -621,12 +621,13 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
{IndirectLocalPathEntry::DefaultArg, DAE, DAE->getParam()});
Arg = DAE->getExpr();
}
- if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr<LifetimeBoundAttr>())
- VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg);
+ if (CheckCoroCall ||
+ CanonCallee->getParamDecl(I)->hasAttr<LifetimeBoundAttr>())
+ VisitLifetimeBoundArg(CanonCallee->getParamDecl(I), Arg);
else if (EnableGSLAnalysis && I == 0) {
// Perform GSL analysis for the first argument
- if (shouldTrackFirstArgument(Callee)) {
- VisitGSLPointerArg(Callee, Arg);
+ if (shouldTrackFirstArgument(CanonCallee)) {
+ VisitGSLPointerArg(CanonCallee, Arg);
} else if (auto *Ctor = dyn_cast<CXXConstructExpr>(Call);
Ctor && shouldTrackFirstArgumentForConstructor(Ctor)) {
VisitGSLPointerArg(Ctor->getConstructor(), Arg);
diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp
index 9fbad7ed67ccbe2..85274aee785a58f 100644
--- a/clang/lib/Sema/SemaAttr.cpp
+++ b/clang/lib/Sema/SemaAttr.cpp
@@ -216,7 +216,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()) {
@@ -238,18 +239,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);
@@ -265,6 +261,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 f89b556f5bba088..c4a4e5415252e7f 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -19,6 +19,10 @@ 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); // 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]]);
int &classparam(IntRef param [[clang::lifetimebound]]);
@@ -48,6 +52,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 f27341b32d53a7fe8cfe905f5bcb7ee5fd2045a0 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/3] Undo changes to inferLifetimeBoundAttribute and use
mergeParamDeclAttributes instead
---
clang/lib/Sema/SemaAttr.cpp | 34 ++++++----------
clang/lib/Sema/SemaDecl.cpp | 79 ++++++++++++++++++++++++++-----------
2 files changed, 68 insertions(+), 45 deletions(-)
diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp
index 85274aee785a58f..9fbad7ed67ccbe2 100644
--- a/clang/lib/Sema/SemaAttr.cpp
+++ b/clang/lib/Sema/SemaAttr.cpp
@@ -216,8 +216,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()) {
@@ -239,13 +238,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);
@@ -261,20 +265,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 be570f3a1829d0f..0c0aa9de47d6789 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -3227,10 +3227,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
@@ -3250,26 +3284,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,
@@ -4323,8 +4356,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)
>From e7afe88bec8e46f3edad9b7d4e3864b2515b99d0 Mon Sep 17 00:00:00 2001
From: higher-performance <higher.performance.github at gmail.com>
Date: Tue, 26 Nov 2024 18:12:41 +0100
Subject: [PATCH 3/3] Use the most recent declaration as the canonical one for
lifetimebound
---
clang/lib/Sema/CheckExprLifetime.cpp | 4 +++-
clang/lib/Sema/SemaDecl.cpp | 18 ++++++------------
2 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index e7c8a7591804efb..0fbedb052f6a600 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -499,6 +499,7 @@ static bool isNormalAssignmentOperator(const FunctionDecl *FD) {
}
static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
+ FD = FD->getMostRecentDecl();
const TypeSourceInfo *TSI = FD->getTypeSourceInfo();
if (!TSI)
return false;
@@ -611,7 +612,7 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
}
}
- const FunctionDecl *CanonCallee = Callee->getCanonicalDecl();
+ const FunctionDecl *CanonCallee = Callee->getMostRecentDecl();
unsigned NP = std::min(Callee->getNumParams(), CanonCallee->getNumParams());
for (unsigned I = 0, N = std::min<unsigned>(NP, Args.size()); I != N; ++I) {
Expr *Arg = Args[I];
@@ -1100,6 +1101,7 @@ static bool pathOnlyHandlesGslPointer(const IndirectLocalPath &Path) {
}
static bool isAssignmentOperatorLifetimeBound(CXXMethodDecl *CMD) {
+ CMD = CMD->getMostRecentDecl();
return CMD && isNormalAssignmentOperator(CMD) && CMD->param_size() == 1 &&
CMD->getParamDecl(0)->hasAttr<LifetimeBoundAttr>();
}
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 0c0aa9de47d6789..ee655d3f9079bde 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -3264,7 +3264,8 @@ static void propagateAttributes(ParmVarDecl *toDecl,
/// mergeParamDeclAttributes - Copy attributes from the old parameter
/// to the new one.
-static void mergeParamDeclAttributes(ParmVarDecl *newDecl, ParmVarDecl *oldDecl,
+static void mergeParamDeclAttributes(ParmVarDecl *newDecl,
+ const ParmVarDecl *oldDecl,
Sema &S) {
// C++11 [dcl.attr.depend]p2:
// The first declaration of a function shall specify the
@@ -3284,20 +3285,12 @@ static void mergeParamDeclAttributes(ParmVarDecl *newDecl, ParmVarDecl *oldDecl,
diag::note_carries_dependency_missing_first_decl) << 1/*Param*/;
}
- // 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;
- });
-
- // 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
+ // most recent 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);
@@ -4356,8 +4349,8 @@ void Sema::mergeObjCMethodDecls(ObjCMethodDecl *newMethod,
mergeDeclAttributes(newMethod, oldMethod, MergeKind);
// Merge attributes from the parameters.
- ObjCMethodDecl::param_iterator oi = oldMethod->param_begin(),
- oe = oldMethod->param_end();
+ ObjCMethodDecl::param_const_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)
@@ -6981,6 +6974,7 @@ static void checkInheritableAttr(Sema &S, NamedDecl &ND) {
static void checkLifetimeBoundAttr(Sema &S, NamedDecl &ND) {
// Check the attributes on the function type and function params, if any.
if (const auto *FD = dyn_cast<FunctionDecl>(&ND)) {
+ FD = FD->getMostRecentDecl();
// Don't declare this variable in the second operand of the for-statement;
// GCC miscompiles that by ending its lifetime before evaluating the
// third operand. See gcc.gnu.org/PR86769.
More information about the cfe-commits
mailing list