Author: Haojian Wu Date: 2024-09-04T10:09:04+02:00 New Revision: 01e56849001b4ace984e9557abc82bc051e03677
URL: https://github.com/llvm/llvm-project/commit/01e56849001b4ace984e9557abc82bc051e03677 DIFF: https://github.com/llvm/llvm-project/commit/01e56849001b4ace984e9557abc82bc051e03677.diff LOG: [clang] Respect the lifetimebound in assignment operator. (#106997) Fixes #106372 Added: Modified: clang/docs/ReleaseNotes.rst clang/lib/Sema/CheckExprLifetime.cpp clang/lib/Sema/CheckExprLifetime.h clang/lib/Sema/SemaOverload.cpp clang/test/SemaCXX/attr-lifetimebound.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 45b08128600efb..511724c73015ea 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -268,6 +268,8 @@ Improvements to Clang's diagnostics - Improved diagnostic when trying to overload a function in an ``extern "C"`` context. (#GH80235) +- 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..f7540a6e3a8979 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,22 @@ static bool pathOnlyHandlesGslPointer(IndirectLocalPath &Path) { return false; } +static bool isAssginmentOperatorLifetimeBound(CXXMethodDecl *CMD) { + if (!CMD) + return false; + 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 +1287,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 +1320,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..0fb997a5671085 100644 --- a/clang/test/SemaCXX/attr-lifetimebound.cpp +++ b/clang/test/SemaCXX/attr-lifetimebound.cpp @@ -287,3 +287,23 @@ 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 {}; + +class NonAnnotatedFoo {}; +class NonAnnotatedFooView {}; + +template <typename T> +struct StatusOr { + template <typename U = T> + StatusOr& operator=(U&& v [[clang::lifetimebound]]); +}; + +void test(StatusOr<FooView> foo1, StatusOr<NonAnnotatedFooView> foo2) { + foo1 = Foo(); // expected-warning {{object backing the pointer foo1 will be destroyed at the end}} + // No warning on non-gsl annotated types. + foo2 = NonAnnotatedFoo(); +} +} // namespace GH106372 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
