llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Haojian Wu (hokein) <details> <summary>Changes</summary> Fixes #<!-- -->106372 --- Full diff: https://github.com/llvm/llvm-project/pull/106997.diff 5 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+2) - (modified) clang/lib/Sema/CheckExprLifetime.cpp (+42-23) - (modified) clang/lib/Sema/CheckExprLifetime.h (+1) - (modified) clang/lib/Sema/SemaOverload.cpp (+3-1) - (modified) clang/test/SemaCXX/attr-lifetimebound.cpp (+15) ``````````diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index fc940db4813948..f87080955b3302 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -266,6 +266,8 @@ Improvements to Clang's diagnostics compilation speed with modules. This warning is disabled by default and it needs to be explicitly enabled or by ``-Weverything``. +- Clang now respects lifetimebound attribute for the assignment operator parameter. (#GH106372). + Improvements to Clang's time-trace ---------------------------------- diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index f28789dba34e10..776d0e8b07a30b 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -326,24 +326,11 @@ static bool shouldTrackFirstArgument(const FunctionDecl *FD) { return false; } -static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) { - const TypeSourceInfo *TSI = FD->getTypeSourceInfo(); - if (!TSI) - return false; - // 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. - AttributedTypeLoc ATL; - for (TypeLoc TL = TSI->getTypeLoc(); - (ATL = TL.getAsAdjusted<AttributedTypeLoc>()); - TL = ATL.getModifiedLoc()) { - if (ATL.getAttrAs<LifetimeBoundAttr>()) - return true; - } - - // Assume that all assignment operators with a "normal" return type return - // *this, that is, an lvalue reference that is the same type as the implicit - // object parameter (or the LHS for a non-member operator$=). +// Return true if this is an "normal" assignment operator. +// We assuments that a normal assingment operator always returns *this, that is, +// an lvalue reference that is the same type as the implicit object parameter +// (or the LHS for a non-member operator$=). +static bool isNormalAsisgnmentOperator(const FunctionDecl *FD) { OverloadedOperatorKind OO = FD->getDeclName().getCXXOverloadedOperator(); if (OO == OO_Equal || isCompoundAssignmentOperator(OO)) { QualType RetT = FD->getReturnType(); @@ -359,10 +346,27 @@ static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) { return true; } } - return false; } +static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) { + const TypeSourceInfo *TSI = FD->getTypeSourceInfo(); + if (!TSI) + return false; + // 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. + AttributedTypeLoc ATL; + for (TypeLoc TL = TSI->getTypeLoc(); + (ATL = TL.getAsAdjusted<AttributedTypeLoc>()); + TL = ATL.getModifiedLoc()) { + if (ATL.getAttrAs<LifetimeBoundAttr>()) + return true; + } + + return isNormalAsisgnmentOperator(FD); +} + // Visit lifetimebound or gsl-pointer arguments. static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, LocalVisitor Visit, @@ -968,6 +972,23 @@ static bool pathOnlyHandlesGslPointer(IndirectLocalPath &Path) { return false; } +static bool isAssginmentOperatorLifetimeBound(CXXMethodDecl *CMD) { + if (!CMD) + return false; + assert(CMD->getOverloadedOperator() == OverloadedOperatorKind::OO_Equal); + return isNormalAsisgnmentOperator(CMD) && CMD->param_size() == 1 && + CMD->getParamDecl(0)->hasAttr<LifetimeBoundAttr>(); +} + +static bool shouldRunGSLAssignmentAnalysis(const Sema &SemaRef, + const AssignedEntity &Entity) { + bool EnableGSLAssignmentWarnings = !SemaRef.getDiagnostics().isIgnored( + diag::warn_dangling_lifetime_pointer_assignment, SourceLocation()); + return (EnableGSLAssignmentWarnings && + (isRecordWithAttr<PointerAttr>(Entity.LHS->getType()) || + isAssginmentOperatorLifetimeBound(Entity.AssignmentOperator))); +} + static void checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity, const InitializedEntity *ExtendingEntity, @@ -1267,8 +1288,7 @@ static void checkExprLifetimeImpl(Sema &SemaRef, }; llvm::SmallVector<IndirectLocalPathEntry, 8> Path; - if (EnableLifetimeWarnings && LK == LK_Assignment && - isRecordWithAttr<PointerAttr>(AEntity->LHS->getType())) + if (LK == LK_Assignment && shouldRunGSLAssignmentAnalysis(SemaRef, *AEntity)) Path.push_back({IndirectLocalPathEntry::GslPointerAssignment, Init}); if (Init->isGLValue()) @@ -1301,8 +1321,7 @@ void checkExprLifetime(Sema &SemaRef, const AssignedEntity &Entity, diag::warn_dangling_pointer_assignment, SourceLocation()); bool RunAnalysis = (EnableDanglingPointerAssignment && Entity.LHS->getType()->isPointerType()) || - (EnableLifetimeWarnings && - isRecordWithAttr<PointerAttr>(Entity.LHS->getType())); + shouldRunGSLAssignmentAnalysis(SemaRef, Entity); if (!RunAnalysis) return; diff --git a/clang/lib/Sema/CheckExprLifetime.h b/clang/lib/Sema/CheckExprLifetime.h index af381fb96c4d68..8c8d0806dee0a3 100644 --- a/clang/lib/Sema/CheckExprLifetime.h +++ b/clang/lib/Sema/CheckExprLifetime.h @@ -22,6 +22,7 @@ namespace clang::sema { struct AssignedEntity { // The left-hand side expression of the assignment. Expr *LHS = nullptr; + CXXMethodDecl *AssignmentOperator = nullptr; }; /// Check that the lifetime of the given expr (and its subobjects) is diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 95551173df91a5..861b0a91240b3b 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -14768,7 +14768,9 @@ ExprResult Sema::CreateOverloadedBinOp(SourceLocation OpLoc, // Check for a self move. DiagnoseSelfMove(Args[0], Args[1], OpLoc); // lifetime check. - checkExprLifetime(*this, AssignedEntity{Args[0]}, Args[1]); + checkExprLifetime( + *this, AssignedEntity{Args[0], dyn_cast<CXXMethodDecl>(FnDecl)}, + Args[1]); } if (ImplicitThis) { QualType ThisType = Context.getPointerType(ImplicitThis->getType()); diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp b/clang/test/SemaCXX/attr-lifetimebound.cpp index 6566ed6270cd14..0bc06ef9cab3ab 100644 --- a/clang/test/SemaCXX/attr-lifetimebound.cpp +++ b/clang/test/SemaCXX/attr-lifetimebound.cpp @@ -287,3 +287,18 @@ std::span<int> test2() { return abc; // expected-warning {{address of stack memory associated with local variable}} } } // namespace ctor_cases + +namespace GH106372 { +class [[gsl::Owner]] Foo {}; +class [[gsl::Pointer]] FooView {}; + +template <typename T> +struct StatusOr { + template <typename U = T> + StatusOr& operator=(U&& v [[clang::lifetimebound]]); +}; + +void test(StatusOr<FooView> foo) { + foo = Foo(); // expected-warning {{object backing the pointer foo will be destroyed at the end}} +} +} // namespace GH106372 `````````` </details> https://github.com/llvm/llvm-project/pull/106997 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
