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