Author: Oliver Stannard Date: 2024-09-23T10:04:08+01:00 New Revision: 0b0a37e158bcf6d0667b4744b3d335f91578e0c9
URL: https://github.com/llvm/llvm-project/commit/0b0a37e158bcf6d0667b4744b3d335f91578e0c9 DIFF: https://github.com/llvm/llvm-project/commit/0b0a37e158bcf6d0667b4744b3d335f91578e0c9.diff LOG: [clang] Lifetime of locals must end before musttail call (#109255) The lifetimes of local variables and function parameters must end before the call to a [[clang::musttail]] function, instead of before the return, because we will not have a stack frame to hold them when doing the call. This documents this limitation, and adds diagnostics to warn about some code which is invalid because of it. Added: Modified: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/AttrDocs.td clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/CheckExprLifetime.cpp clang/lib/Sema/CheckExprLifetime.h clang/lib/Sema/SemaStmt.cpp clang/test/SemaCXX/attr-musttail.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index da5205087fd821..8f7772baafcdb1 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -268,6 +268,11 @@ Attribute Changes in Clang - Introduced a new attribute ``[[clang::coro_await_elidable_argument]]`` on function parameters to propagate safe elide context to arguments if such function is also under a safe elide context. +- The documentation of the ``[[clang::musttail]]`` attribute was updated to + note that the lifetimes of all local variables end before the call. This does + not change the behaviour of the compiler, as this was true for previous + versions. + Improvements to Clang's diagnostics ----------------------------------- @@ -324,6 +329,10 @@ Improvements to Clang's diagnostics - Don't emit bogus dangling diagnostics when ``[[gsl::Owner]]`` and `[[clang::lifetimebound]]` are used together (#GH108272). +- The ``-Wreturn-stack-address`` warning now also warns about addresses of + local variables passed to function calls using the ``[[clang::musttail]]`` + attribute. + Improvements to Clang's time-trace ---------------------------------- diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 8ef151b3f2fddb..f23a148e546fa3 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -637,6 +637,12 @@ return value must be trivially destructible. The calling convention of the caller and callee must match, and they must not be variadic functions or have old style K&R C function declarations. +The lifetimes of all local variables and function parameters end immediately +before the call to the function. This means that it is undefined behaviour to +pass a pointer or reference to a local variable to the called function, which +is not the case without the attribute. Clang will emit a warning in common +cases where this happens. + ``clang::musttail`` provides assurances that the tail call can be optimized on all targets, not just one. }]; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index ba813af960af6f..e4e04bff8b5120 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10101,11 +10101,15 @@ def err_lifetimebound_ctor_dtor : Error< // CHECK: returning address/reference of stack memory def warn_ret_stack_addr_ref : Warning< "%select{address of|reference to}0 stack memory associated with " - "%select{local variable|parameter|compound literal}2 %1 returned">, + "%select{local variable|parameter|compound literal}2 %1 " + "%select{returned|passed to musttail function}3">, InGroup<ReturnStackAddress>; def warn_ret_local_temp_addr_ref : Warning< "returning %select{address of|reference to}0 local temporary object">, InGroup<ReturnStackAddress>; +def warn_musttail_local_temp_addr_ref : Warning< + "passing %select{address of|reference to}0 local temporary object to musttail function">, + InGroup<ReturnStackAddress>; def err_ret_local_temp_ref : Error< "returning reference to local temporary object">; def warn_ret_addr_label : Warning< diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index c98fbca849faba..e9e39c11ffbaab 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -33,6 +33,10 @@ enum LifetimeKind { /// the entity is a return object. LK_Return, + /// The lifetime of a temporary bound to this entity ends too soon, because + /// the entity passed to a musttail function call. + LK_MustTail, + /// The lifetime of a temporary bound to this entity ends too soon, because /// the entity is the result of a statement expression. LK_StmtExprResult, @@ -1150,6 +1154,7 @@ static void checkExprLifetimeImpl(Sema &SemaRef, break; case LK_Return: + case LK_MustTail: case LK_StmtExprResult: if (auto *DRE = dyn_cast<DeclRefExpr>(L)) { // We can't determine if the local variable outlives the statement @@ -1158,7 +1163,8 @@ static void checkExprLifetimeImpl(Sema &SemaRef, return false; SemaRef.Diag(DiagLoc, diag::warn_ret_stack_addr_ref) << InitEntity->getType()->isReferenceType() << DRE->getDecl() - << isa<ParmVarDecl>(DRE->getDecl()) << DiagRange; + << isa<ParmVarDecl>(DRE->getDecl()) << (LK == LK_MustTail) + << DiagRange; } else if (isa<BlockExpr>(L)) { SemaRef.Diag(DiagLoc, diag::err_ret_local_block) << DiagRange; } else if (isa<AddrLabelExpr>(L)) { @@ -1170,7 +1176,7 @@ static void checkExprLifetimeImpl(Sema &SemaRef, } else if (auto *CLE = dyn_cast<CompoundLiteralExpr>(L)) { SemaRef.Diag(DiagLoc, diag::warn_ret_stack_addr_ref) << InitEntity->getType()->isReferenceType() << CLE->getInitializer() - << 2 << DiagRange; + << 2 << (LK == LK_MustTail) << DiagRange; } else { // P2748R5: Disallow Binding a Returned Glvalue to a Temporary. // [stmt.return]/p6: In a function whose return type is a reference, @@ -1181,6 +1187,9 @@ static void checkExprLifetimeImpl(Sema &SemaRef, InitEntity->getType()->isReferenceType()) SemaRef.Diag(DiagLoc, diag::err_ret_local_temp_ref) << InitEntity->getType()->isReferenceType() << DiagRange; + else if (LK == LK_MustTail) + SemaRef.Diag(DiagLoc, diag::warn_musttail_local_temp_addr_ref) + << InitEntity->getType()->isReferenceType() << DiagRange; else SemaRef.Diag(DiagLoc, diag::warn_ret_local_temp_addr_ref) << InitEntity->getType()->isReferenceType() << DiagRange; @@ -1265,6 +1274,12 @@ void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity, /*AEntity*/ nullptr, Init); } +void checkExprLifetimeMustTailArg(Sema &SemaRef, + const InitializedEntity &Entity, Expr *Init) { + checkExprLifetimeImpl(SemaRef, &Entity, nullptr, LK_MustTail, + /*AEntity*/ nullptr, Init); +} + void checkExprLifetime(Sema &SemaRef, const AssignedEntity &Entity, Expr *Init) { bool EnableDanglingPointerAssignment = !SemaRef.getDiagnostics().isIgnored( diff --git a/clang/lib/Sema/CheckExprLifetime.h b/clang/lib/Sema/CheckExprLifetime.h index 8c8d0806dee0a3..903f312f3533e5 100644 --- a/clang/lib/Sema/CheckExprLifetime.h +++ b/clang/lib/Sema/CheckExprLifetime.h @@ -35,6 +35,12 @@ void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity, /// sufficient for assigning to the entity. void checkExprLifetime(Sema &SemaRef, const AssignedEntity &Entity, Expr *Init); +/// Check that the lifetime of the given expr (and its subobjects) is +/// sufficient, assuming that it is passed as an argument to a musttail +/// function. +void checkExprLifetimeMustTailArg(Sema &SemaRef, + const InitializedEntity &Entity, Expr *Init); + } // namespace clang::sema #endif // LLVM_CLANG_SEMA_CHECK_EXPR_LIFETIME_H diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 9664287b9a3fe9..9e235a46707cd4 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// +#include "CheckExprLifetime.h" #include "clang/AST/ASTContext.h" #include "clang/AST/ASTDiagnostic.h" #include "clang/AST/ASTLambda.h" @@ -889,6 +890,15 @@ bool Sema::checkMustTailAttr(const Stmt *St, const Attr &MTA) { return false; } + // The lifetimes of locals and incoming function parameters must end before + // the call, because we can't have a stack frame to store them, so diagnose + // any pointers or references to them passed into the musttail call. + for (auto ArgExpr : CE->arguments()) { + InitializedEntity Entity = InitializedEntity::InitializeParameter( + Context, ArgExpr->getType(), false); + checkExprLifetimeMustTailArg(*this, Entity, const_cast<Expr *>(ArgExpr)); + } + return true; } diff --git a/clang/test/SemaCXX/attr-musttail.cpp b/clang/test/SemaCXX/attr-musttail.cpp index 561184e7a24f94..12cfd89c64a6fd 100644 --- a/clang/test/SemaCXX/attr-musttail.cpp +++ b/clang/test/SemaCXX/attr-musttail.cpp @@ -267,3 +267,34 @@ namespace ns {} void TestCallNonValue() { [[clang::musttail]] return ns; // expected-error {{unexpected namespace name 'ns': expected expression}} } + +// Test diagnostics for lifetimes of local variables, which end earlier for a +// musttail call than for a nowmal one. + +void TakesIntAndPtr(int, int *); +void PassAddressOfLocal(int a, int *b) { + int c; + [[clang::musttail]] return TakesIntAndPtr(0, &c); // expected-warning {{address of stack memory associated with local variable 'c' passed to musttail function}} +} +void PassAddressOfParam(int a, int *b) { + [[clang::musttail]] return TakesIntAndPtr(0, &a); // expected-warning {{address of stack memory associated with parameter 'a' passed to musttail function}} +} +void PassValues(int a, int *b) { + [[clang::musttail]] return TakesIntAndPtr(a, b); +} + +void TakesIntAndRef(int, const int &); +void PassRefOfLocal(int a, const int &b) { + int c; + [[clang::musttail]] return TakesIntAndRef(0, c); // expected-warning {{address of stack memory associated with local variable 'c' passed to musttail function}} +} +void PassRefOfParam(int a, const int &b) { + [[clang::musttail]] return TakesIntAndRef(0, a); // expected-warning {{address of stack memory associated with parameter 'a' passed to musttail function}} +} +int ReturnInt(); +void PassRefOfTemporary(int a, const int &b) { + [[clang::musttail]] return TakesIntAndRef(0, ReturnInt()); // expected-warning {{passing address of local temporary object to musttail function}} +} +void PassValuesRef(int a, const int &b) { + [[clang::musttail]] return TakesIntAndRef(a, b); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
