https://github.com/ostannard updated
https://github.com/llvm/llvm-project/pull/109255
>From 85ac319257785f88fd27a533fffde7aab20c8d8d Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stann...@arm.com>
Date: Wed, 18 Sep 2024 16:22:41 +0100
Subject: [PATCH 1/5] [clang] Lifetime of locals must end before musttail call
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.
---
clang/include/clang/Basic/AttrDocs.td | 5 ++++
.../clang/Basic/DiagnosticSemaKinds.td | 3 ++-
clang/lib/Sema/CheckExprLifetime.cpp | 16 +++++++++--
clang/lib/Sema/CheckExprLifetime.h | 6 +++++
clang/lib/Sema/SemaStmt.cpp | 10 +++++++
clang/test/SemaCXX/attr-musttail.cpp | 27 +++++++++++++++++++
6 files changed, 64 insertions(+), 3 deletions(-)
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);
+}
>From c9ed9b8bd0fc065fa4c17cee41a1804897176068 Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stann...@arm.com>
Date: Thu, 19 Sep 2024 10:12:24 +0100
Subject: [PATCH 2/5] clang-format
---
clang/lib/Sema/CheckExprLifetime.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp
b/clang/lib/Sema/CheckExprLifetime.cpp
index 211c1cc7bc81f9..149880301d4a93 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -1271,8 +1271,8 @@ void checkExprLifetime(Sema &SemaRef, const
InitializedEntity &Entity,
/*AEntity*/ nullptr, Init);
}
-void checkExprLifetimeMustTailArg(Sema &SemaRef, const InitializedEntity
&Entity,
- Expr *Init) {
+void checkExprLifetimeMustTailArg(Sema &SemaRef,
+ const InitializedEntity &Entity, Expr *Init)
{
checkExprLifetimeImpl(SemaRef, &Entity, nullptr, LK_MustTail,
/*AEntity*/ nullptr, Init);
}
>From 78075ad7f5adc47fa8e30878a378854df3cf1751 Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stann...@arm.com>
Date: Thu, 19 Sep 2024 15:16:41 +0100
Subject: [PATCH 3/5] Add release notes
---
clang/docs/ReleaseNotes.rst | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b8816d7b555e87..b7fa28e3ae1f57 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -260,6 +260,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
-----------------------------------
@@ -316,6 +321,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
----------------------------------
>From 0115234c45182dd633ed0be2cbd76e5a23f549ac Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stann...@arm.com>
Date: Fri, 20 Sep 2024 10:16:17 +0100
Subject: [PATCH 4/5] Better message for passing reference to temporary
---
clang/include/clang/Basic/DiagnosticSemaKinds.td | 3 +++
clang/lib/Sema/CheckExprLifetime.cpp | 3 +++
clang/test/SemaCXX/attr-musttail.cpp | 12 ++++++++----
3 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 75c7f9e0eb7de0..e4e04bff8b5120 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10107,6 +10107,9 @@ def warn_ret_stack_addr_ref : Warning<
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 149880301d4a93..e9e39c11ffbaab 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -1187,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;
diff --git a/clang/test/SemaCXX/attr-musttail.cpp
b/clang/test/SemaCXX/attr-musttail.cpp
index d84b97a4f04d86..12cfd89c64a6fd 100644
--- a/clang/test/SemaCXX/attr-musttail.cpp
+++ b/clang/test/SemaCXX/attr-musttail.cpp
@@ -283,14 +283,18 @@ void PassValues(int a, int *b) {
[[clang::musttail]] return TakesIntAndPtr(a, b);
}
-void TakesIntAndRef(int, int &);
-void PassRefOfLocal(int a, int &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, int &b) {
+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}}
}
-void PassValuesRef(int a, int &b) {
+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);
}
>From c46e0a8d5bbceebee25b7069727cd9e3dd974d67 Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stann...@arm.com>
Date: Fri, 20 Sep 2024 10:17:57 +0100
Subject: [PATCH 5/5] Mention warning in attribute docs
---
clang/include/clang/Basic/AttrDocs.td | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index 7226871074ee7e..f23a148e546fa3 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -640,7 +640,8 @@ 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.
+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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
