[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.

Author: ostannard

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
 ----------------------------------
 

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.
   }];

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<

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(

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

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;
 }
 

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);
+}