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.git...@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.git...@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.git...@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. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits