https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/107627
>From e9af4e3d8f629fff790f2f573ceccdf01fcb9495 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 | 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 6cdd4dc629e50a..d3423b50f3b051 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 716d8ed1fae4f8..754f36fa03f1d1 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 dd8f37a5c967ab14a5d9b9c6a4ef84be2d970491 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 754f36fa03f1d1..716d8ed1fae4f8 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 74b0e5ad23bd48..2d82275509ebd8 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 8da0a21981bd2df7a9e47a0c8fff040960003dc7 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 | 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 d3423b50f3b051..c25a8df2c7dab3 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -499,6 +499,7 @@ static bool isNormalAssignmentOperator(const FunctionDecl *FD) { } 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 2d82275509ebd8..02e4d973e15338 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. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits