[clang] 4cc9bf1 - Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (#107627)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 14 01:22:24 PST 2025
Author: higher-performance
Date: 2025-01-14T10:22:20+01:00
New Revision: 4cc9bf149f07edec5ea910af8b3ead17ae8b29b7
URL: https://github.com/llvm/llvm-project/commit/4cc9bf149f07edec5ea910af8b3ead17ae8b29b7
DIFF: https://github.com/llvm/llvm-project/commit/4cc9bf149f07edec5ea910af8b3ead17ae8b29b7.diff
LOG: Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (#107627)
This partially fixes #62072 by making sure that re-declarations of a
function do not have the effect of removing lifetimebound from the
canonical declaration.
It doesn't handle the implicit 'this' parameter, but that can be
addressed in a separate fix.
Added:
Modified:
clang/lib/Sema/CheckExprLifetime.cpp
clang/lib/Sema/SemaDecl.cpp
clang/test/SemaCXX/attr-lifetimebound.cpp
Removed:
################################################################################
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 837414c4840d75..27e6b5b2cb3930 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -525,7 +525,20 @@ static bool isNormalAssignmentOperator(const FunctionDecl *FD) {
return false;
}
+static const FunctionDecl *
+getDeclWithMergedLifetimeBoundAttrs(const FunctionDecl *FD) {
+ return FD != nullptr ? FD->getMostRecentDecl() : nullptr;
+}
+
+static const CXXMethodDecl *
+getDeclWithMergedLifetimeBoundAttrs(const CXXMethodDecl *CMD) {
+ const FunctionDecl *FD = CMD;
+ return cast_if_present<CXXMethodDecl>(
+ getDeclWithMergedLifetimeBoundAttrs(FD));
+}
+
bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
+ FD = getDeclWithMergedLifetimeBoundAttrs(FD);
const TypeSourceInfo *TSI = FD->getTypeSourceInfo();
if (!TSI)
return false;
@@ -647,9 +660,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 = 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];
RevertToOldSizeRAII RAII(Path);
if (auto *DAE = dyn_cast<CXXDefaultArgExpr>(Arg)) {
@@ -657,11 +670,12 @@ 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;
}))
@@ -678,11 +692,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);
@@ -1245,7 +1259,8 @@ static AnalysisResult analyzePathForGSLPointer(const IndirectLocalPath &Path,
return Report;
}
-static bool isAssignmentOperatorLifetimeBound(CXXMethodDecl *CMD) {
+static bool isAssignmentOperatorLifetimeBound(const 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 5b7275c316f74a..f5e57988b7fa8d 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -3239,6 +3239,42 @@ void Sema::mergeDeclAttributes(NamedDecl *New, Decl *Old,
if (!foundAny) New->dropAttrs();
}
+// Returns the number of added attributes.
+template <class T>
+static unsigned propagateAttribute(ParmVarDecl *To, const ParmVarDecl *From,
+ Sema &S) {
+ unsigned found = 0;
+ for (const auto *I : From->specific_attrs<T>()) {
+ if (!DeclHasAttr(To, I)) {
+ T *newAttr = cast<T>(I->clone(S.Context));
+ newAttr->setInherited(true);
+ To->addAttr(newAttr);
+ ++found;
+ }
+ }
+ return found;
+}
+
+template <class F>
+static void propagateAttributes(ParmVarDecl *To, const ParmVarDecl *From,
+ F &&propagator) {
+ if (!From->hasAttrs()) {
+ return;
+ }
+
+ bool foundAny = To->hasAttrs();
+
+ // Ensure that any moving of objects within the allocated map is
+ // done before we process them.
+ if (!foundAny)
+ To->setAttrs(AttrVec());
+
+ foundAny |= std::forward<F>(propagator)(To, From) != 0;
+
+ if (!foundAny)
+ To->dropAttrs();
+}
+
/// mergeParamDeclAttributes - Copy attributes from the old parameter
/// to the new one.
static void mergeParamDeclAttributes(ParmVarDecl *newDecl,
@@ -3262,26 +3298,17 @@ 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;
- }
- }
-
- if (!foundAny) newDecl->dropAttrs();
+ propagateAttributes(
+ 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
+ // 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>(To, From, S);
+ return found;
+ });
}
static bool EquivalentArrayTypes(QualType Old, QualType New,
@@ -6960,6 +6987,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.
diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp b/clang/test/SemaCXX/attr-lifetimebound.cpp
index 896793f9966666..e7c8b35cb0c481 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -33,6 +33,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]]);
@@ -62,6 +66,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}}
More information about the cfe-commits
mailing list