Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----------------------------------

Expand Down Expand Up @@ -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
----------------------------------

Expand Down
6 changes: 6 additions & 0 deletions clang/include/clang/Basic/AttrDocs.td
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}];
Expand Down
6 changes: 5 additions & 1 deletion clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -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<
Expand Down
19 changes: 17 additions & 2 deletions clang/lib/Sema/CheckExprLifetime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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)) {
Expand All @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 6 additions & 0 deletions clang/lib/Sema/CheckExprLifetime.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
10 changes: 10 additions & 0 deletions clang/lib/Sema/SemaStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
//
//===----------------------------------------------------------------------===//

#include "CheckExprLifetime.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTDiagnostic.h"
#include "clang/AST/ASTLambda.h"
Expand Down Expand Up @@ -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;
}

Expand Down
31 changes: 31 additions & 0 deletions clang/test/SemaCXX/attr-musttail.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a test where a function call is used in the argument? something like

int foo();
void test(int a, const int &b) {
  int c;
  [[clang::musttail]] return test(0, foo()); 
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, and there was a different diagnostic message used for temporaries, so added a better wording for that.

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