[clang] [coroutines] Detect lifetime issues with coroutine lambda captures (PR #77066)

Utkarsh Saxena via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 5 02:39:52 PST 2024


https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/77066

>From 3e0d0ab6c4fc6cba68285816a95e423bc18e8e55 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Fri, 5 Jan 2024 10:11:20 +0100
Subject: [PATCH 1/2] [coroutines] Detect lifetime issues with coroutine lambda
 captures

---
 clang/lib/Sema/SemaInit.cpp               | 20 +++++--
 clang/test/SemaCXX/coro-lifetimebound.cpp | 64 +++++++++++++++++++++--
 2 files changed, 76 insertions(+), 8 deletions(-)

diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 60c0e3e74204ec..c100bf11454786 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -12,6 +12,7 @@
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclObjC.h"
+#include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExprOpenMP.h"
@@ -33,6 +34,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -7575,15 +7577,27 @@ static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call,
     Path.pop_back();
   };
 
-  if (ObjectArg && implicitObjectParamIsLifetimeBound(Callee))
-    VisitLifetimeBoundArg(Callee, ObjectArg);
-
   bool CheckCoroCall = false;
   if (const auto *RD = Callee->getReturnType()->getAsRecordDecl()) {
     CheckCoroCall = RD->hasAttr<CoroLifetimeBoundAttr>() &&
                     RD->hasAttr<CoroReturnTypeAttr>() &&
                     !Callee->hasAttr<CoroDisableLifetimeBoundAttr>();
   }
+
+  if (ObjectArg) {
+    bool CheckCoroObjArg = CheckCoroCall;
+    // Ignore `__promise.get_return_object()` as it not lifetimebound.
+    if (Callee->getDeclName().isIdentifier() &&
+        Callee->getName() == "get_return_object")
+      CheckCoroObjArg = false;
+    // Coroutine lambda objects with empty capture list are not lifetimebound.
+    if (auto *LE = dyn_cast<LambdaExpr>(ObjectArg->IgnoreImplicit());
+        LE && LE->captures().empty())
+      CheckCoroObjArg = false;
+    if (implicitObjectParamIsLifetimeBound(Callee) || CheckCoroObjArg)
+      VisitLifetimeBoundArg(Callee, ObjectArg);
+  }
+
   for (unsigned I = 0,
                 N = std::min<unsigned>(Callee->getNumParams(), Args.size());
        I != N; ++I) {
diff --git a/clang/test/SemaCXX/coro-lifetimebound.cpp b/clang/test/SemaCXX/coro-lifetimebound.cpp
index 3fc7ca70a14a12..319134450e4b6f 100644
--- a/clang/test/SemaCXX/coro-lifetimebound.cpp
+++ b/clang/test/SemaCXX/coro-lifetimebound.cpp
@@ -64,6 +64,10 @@ Co<int> bar_coro(const int &b, int c) {
       : bar_coro(0, 1); // expected-warning {{returning address of local temporary object}}
 }
 
+// =============================================================================
+// Lambdas
+// =============================================================================
+namespace lambdas {
 void lambdas() {
   auto unsafe_lambda = [] [[clang::coro_wrapper]] (int b) {
     return foo_coro(b); // expected-warning {{address of stack memory associated with parameter}}
@@ -84,15 +88,47 @@ void lambdas() {
     co_return x + co_await foo_coro(b);
   };
 }
+
+Co<int> lambda_captures() {
+  int a = 1;
+  // Temporary lambda object dies.
+  auto lamb = [a](int x, const int& y) -> Co<int> { // expected-warning {{temporary whose address is used as value of local variable 'lamb'}}
+    co_return x + y + a;
+  }(1, a);
+  // Object dies but it has no capture.
+  auto no_capture = []() -> Co<int> { co_return 1; }();
+  auto bad_no_capture = [](const int& a) -> Co<int> { co_return a; }(1); // expected-warning {{temporary}}
+  // Temporary lambda object with lifetime extension under co_await.
+  int res = co_await [a](int x, const int& y) -> Co<int> {
+    co_return x + y + a;
+  }(1, a);
+  co_return 1;
+}
+} // namespace lambdas
+
 // =============================================================================
-// Safe usage when parameters are value
+// Member coroutines
 // =============================================================================
-namespace by_value {
-Co<int> value_coro(int b) { co_return co_await foo_coro(b); }
-[[clang::coro_wrapper]] Co<int> wrapper1(int b) { return value_coro(b); }
-[[clang::coro_wrapper]] Co<int> wrapper2(const int& b) { return value_coro(b); }
+namespace member_coroutines{
+struct S {
+  Co<int> member(const int& a) { co_return a; }  
+};
+
+Co<int> use() {
+  S s;
+  int a = 1;
+  auto test1 = s.member(1);  // expected-warning {{temporary whose address is used as value of local variable}}
+  auto test2 = s.member(a);
+  auto test3 = S{}.member(a);  // expected-warning {{temporary whose address is used as value of local variable}}
+  co_return 1;
 }
 
+[[clang::coro_wrapper]] Co<int> wrapper(const int& a) {
+  S s;
+  return s.member(a); // expected-warning {{address of stack memory}}
+}
+} // member_coroutines
+
 // =============================================================================
 // Lifetime bound but not a Coroutine Return Type: No analysis.
 // =============================================================================
@@ -129,4 +165,22 @@ Co<int> foo_wrapper(const int& x) { return foo(x); }
   // The call to foo_wrapper is wrapper is safe.
   return foo_wrapper(1);
 }
+
+struct S{
+[[clang::coro_wrapper, clang::coro_disable_lifetimebound]] 
+Co<int> member(const int& x) { return foo(x); }
+};
+
+Co<int> use() {
+  S s;
+  int a = 1;
+  auto test1 = s.member(1); // param is not flagged.
+  auto test2 = S{}.member(a); // 'this' is not flagged.
+  co_return 1;
+}
+
+[[clang::coro_wrapper]] Co<int> return_stack_addr(const int& a) {
+  S s;
+  return s.member(a); // return of stack addr is not flagged.
+}
 } // namespace disable_lifetimebound

>From 0d8d78e30dff896880e674240c8bf47b13c527f5 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Fri, 5 Jan 2024 11:39:41 +0100
Subject: [PATCH 2/2] add more test

---
 clang/lib/Sema/SemaInit.cpp               |  4 ++--
 clang/test/SemaCXX/coro-lifetimebound.cpp | 17 +++++++++++++++--
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index c100bf11454786..1aa6be826699bb 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -7586,8 +7586,8 @@ static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call,
 
   if (ObjectArg) {
     bool CheckCoroObjArg = CheckCoroCall;
-    // Ignore `__promise.get_return_object()` as it not lifetimebound.
-    if (Callee->getDeclName().isIdentifier() &&
+    // Ignore `__promise.get_return_object()` as it is not lifetimebound.
+    if (CheckCoroObjArg && Callee->getDeclName().isIdentifier() &&
         Callee->getName() == "get_return_object")
       CheckCoroObjArg = false;
     // Coroutine lambda objects with empty capture list are not lifetimebound.
diff --git a/clang/test/SemaCXX/coro-lifetimebound.cpp b/clang/test/SemaCXX/coro-lifetimebound.cpp
index 319134450e4b6f..b61013ec057381 100644
--- a/clang/test/SemaCXX/coro-lifetimebound.cpp
+++ b/clang/test/SemaCXX/coro-lifetimebound.cpp
@@ -102,6 +102,10 @@ Co<int> lambda_captures() {
   int res = co_await [a](int x, const int& y) -> Co<int> {
     co_return x + y + a;
   }(1, a);
+  // Lambda object on stack should be fine.
+  auto lamb2 = [a]() -> Co<int> { co_return a; };
+  auto on_stack = lamb2();
+  auto res2 = co_await on_stack;
   co_return 1;
 }
 } // namespace lambdas
@@ -111,7 +115,7 @@ Co<int> lambda_captures() {
 // =============================================================================
 namespace member_coroutines{
 struct S {
-  Co<int> member(const int& a) { co_return a; }  
+  Co<int> member(const int& a) { co_return a; }
 };
 
 Co<int> use() {
@@ -129,6 +133,15 @@ Co<int> use() {
 }
 } // member_coroutines
 
+// =============================================================================
+// Safe usage when parameters are value
+// =============================================================================
+namespace by_value {
+Co<int> value_coro(int b) { co_return co_await foo_coro(b); }
+[[clang::coro_wrapper]] Co<int> wrapper1(int b) { return value_coro(b); }
+[[clang::coro_wrapper]] Co<int> wrapper2(const int& b) { return value_coro(b); }
+} // namespace by_value
+
 // =============================================================================
 // Lifetime bound but not a Coroutine Return Type: No analysis.
 // =============================================================================
@@ -158,7 +171,7 @@ CoNoCRT<int> bar(int a) {
 namespace disable_lifetimebound {
 Co<int> foo(int x) {  co_return x; }
 
-[[clang::coro_wrapper, clang::coro_disable_lifetimebound]] 
+[[clang::coro_wrapper, clang::coro_disable_lifetimebound]]
 Co<int> foo_wrapper(const int& x) { return foo(x); }
 
 [[clang::coro_wrapper]] Co<int> caller() {



More information about the cfe-commits mailing list