llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Oliver Stannard (ostannard) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/109255.diff 6 Files Affected: - (modified) clang/include/clang/Basic/AttrDocs.td (+5) - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2-1) - (modified) clang/lib/Sema/CheckExprLifetime.cpp (+14-2) - (modified) clang/lib/Sema/CheckExprLifetime.h (+6) - (modified) clang/lib/Sema/SemaStmt.cpp (+10) - (modified) clang/test/SemaCXX/attr-musttail.cpp (+27) ``````````diff diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 8ef151b3f2fddb..7226871074ee7e 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -637,6 +637,11 @@ 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::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..75c7f9e0eb7de0 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10101,7 +10101,8 @@ 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">, diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index c98fbca849faba..211c1cc7bc81f9 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, @@ -1265,6 +1271,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..d84b97a4f04d86 100644 --- a/clang/test/SemaCXX/attr-musttail.cpp +++ b/clang/test/SemaCXX/attr-musttail.cpp @@ -267,3 +267,30 @@ 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, int &); +void PassRefOfLocal(int a, 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, int &b) { + [[clang::musttail]] return TakesIntAndRef(0, a); // expected-warning {{address of stack memory associated with parameter 'a' passed to musttail function}} +} +void PassValuesRef(int a, int &b) { + [[clang::musttail]] return TakesIntAndRef(a, b); +} `````````` </details> https://github.com/llvm/llvm-project/pull/109255 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
