[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 Dec 13 16:12:24 PST 2024
https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627
>From 3764d3e1062c6b748cea1faa9e4bd9628a1a5dea 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 | 20 ++++++-------
clang/lib/Sema/SemaAttr.cpp | 34 +++++++++++++++--------
clang/test/SemaCXX/attr-lifetimebound.cpp | 5 ++++
3 files changed, 37 insertions(+), 22 deletions(-)
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 843fdb4a65cd73..8ba59dfde3d69b 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -636,9 +636,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)) {
@@ -646,11 +646,11 @@ 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 (const auto *CaptureAttr =
- Callee->getParamDecl(I)->getAttr<LifetimeCaptureByAttr>();
- CaptureAttr && isa<CXXConstructorDecl>(Callee) &&
+ CanonCallee->getParamDecl(I)->getAttr<LifetimeCaptureByAttr>();
+ CaptureAttr && isa<CXXConstructorDecl>(CanonCallee) &&
llvm::any_of(CaptureAttr->params(), [](int ArgIdx) {
return ArgIdx == LifetimeCaptureByAttr::THIS;
}))
@@ -667,11 +667,11 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
// `lifetimebound` and shares the same code path. This implies the emitted
// diagnostics will be emitted under `-Wdangling`, not
// `-Wdangling-capture`.
- VisitLifetimeBoundArg(Callee->getParamDecl(I), Arg);
+ 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 44485e71d57a01..3fd122b20d7b12 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 f89b556f5bba08..c4a4e5415252e7 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 8d5ff5534e406cbd0bd004f35adba632e0e00e72 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 3fd122b20d7b12..44485e71d57a01 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 55e891e3acf20d..0b5146df86c3a0 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 8afd704bfb257e4bbfd4116fce232f33c149491e 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 | 13 +++++++-
clang/lib/Sema/SemaDecl.cpp | 49 +++++++++++++---------------
2 files changed, 34 insertions(+), 28 deletions(-)
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 8ba59dfde3d69b..85e571a1c8785b 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -523,7 +523,17 @@ static bool isNormalAssignmentOperator(const FunctionDecl *FD) {
return false;
}
+static const FunctionDecl *getDeclWithMergedLifetimeBoundAttrs(const FunctionDecl *FD) {
+ return FD->getMostRecentDecl();
+}
+
+static const CXXMethodDecl *getDeclWithMergedLifetimeBoundAttrs(const CXXMethodDecl *CMD) {
+ const FunctionDecl* FD = CMD;
+ return cast<CXXMethodDecl>(getDeclWithMergedLifetimeBoundAttrs(FD));
+}
+
bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
+ FD = getDeclWithMergedLifetimeBoundAttrs(FD);
const TypeSourceInfo *TSI = FD->getTypeSourceInfo();
if (!TSI)
return false;
@@ -636,7 +646,7 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
}
}
- const FunctionDecl *CanonCallee = Callee->getCanonicalDecl();
+ const FunctionDecl *CanonCallee = getDeclWithMergedLifetimeBoundAttrs(Callee);
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];
@@ -1236,6 +1246,7 @@ static AnalysisResult analyzePathForGSLPointer(const IndirectLocalPath &Path,
}
static bool isAssignmentOperatorLifetimeBound(CXXMethodDecl *CMD) {
+ CMD = getDeclWithMergedLifetimeBoundAttrs(CMD);
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 0b5146df86c3a0..9312b3472f01df 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -3227,15 +3227,16 @@ void Sema::mergeDeclAttributes(NamedDecl *New, Decl *Old,
if (!foundAny) New->dropAttrs();
}
+// Returns the number of added attributes.
template <class T>
-static unsigned propagateAttribute(ParmVarDecl *toDecl,
- const ParmVarDecl *fromDecl, Sema &S) {
+static unsigned propagateAttribute(ParmVarDecl *To,
+ const ParmVarDecl *From, Sema &S) {
unsigned found = 0;
- for (const auto *I : fromDecl->specific_attrs<T>()) {
- if (!DeclHasAttr(toDecl, I)) {
+ for (const auto *I : From->specific_attrs<T>()) {
+ if (!DeclHasAttr(To, I)) {
T *newAttr = cast<T>(I->clone(S.Context));
newAttr->setInherited(true);
- toDecl->addAttr(newAttr);
+ To->addAttr(newAttr);
++found;
}
}
@@ -3243,28 +3244,29 @@ static unsigned propagateAttribute(ParmVarDecl *toDecl,
}
template <class F>
-static void propagateAttributes(ParmVarDecl *toDecl,
- const ParmVarDecl *fromDecl, F &&propagator) {
- if (!fromDecl->hasAttrs()) {
+static void propagateAttributes(ParmVarDecl *To,
+ const ParmVarDecl *From, F &&propagator) {
+ if (!From->hasAttrs()) {
return;
}
- bool foundAny = toDecl->hasAttrs();
+ bool foundAny = To->hasAttrs();
// Ensure that any moving of objects within the allocated map is
// done before we process them.
if (!foundAny)
- toDecl->setAttrs(AttrVec());
+ To->setAttrs(AttrVec());
- foundAny |= std::forward<F>(propagator)(toDecl, fromDecl) != 0;
+ foundAny |= std::forward<F>(propagator)(To, From) != 0;
if (!foundAny)
- toDecl->dropAttrs();
+ To->dropAttrs();
}
/// 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,23 +3286,15 @@ 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) {
+ newDecl, oldDecl, [&S](ParmVarDecl *To, const ParmVarDecl *From) {
unsigned found = 0;
+ found += propagateAttribute<InheritableParamAttr>(To, From, S);
// 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);
+ found += propagateAttribute<LifetimeBoundAttr>(To, From, S);
return found;
});
}
@@ -4356,8 +4350,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 +6975,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