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