[clang] [coroutines] Introduce [[clang::coro_lifetimebound]] (PR #72851)

Utkarsh Saxena via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 20 18:40:03 PST 2023


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

>From 28e9fda4b78e1e60287048891cc92bafdef3ac4c Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Mon, 20 Nov 2023 12:17:30 +0100
Subject: [PATCH 1/5] Introduce [[clang::coro_lifetimebound]]

---
 clang/include/clang/Basic/Attr.td             |  8 ++
 clang/include/clang/Basic/AttrDocs.td         |  8 +-
 clang/lib/Sema/SemaInit.cpp                   |  7 +-
 ...a-attribute-supported-attributes-list.test |  3 +-
 clang/test/SemaCXX/coro-lifetimebound.cpp     | 93 +++++++++++++++++++
 5 files changed, 116 insertions(+), 3 deletions(-)
 create mode 100644 clang/test/SemaCXX/coro-lifetimebound.cpp

diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index f7a2b83b15ef5bc..dfe6f8999f832a3 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1110,6 +1110,14 @@ def CoroWrapper : InheritableAttr {
   let SimpleHandler = 1;
 }
 
+def CoroLifetimeBound : InheritableAttr {
+  let Spellings = [Clang<"coro_lifetimebound">];
+  let Subjects = SubjectList<[CXXRecord]>;
+  let LangOpts = [CPlusPlus];
+  let Documentation = [CoroLifetimeBoundDoc];
+  let SimpleHandler = 1;
+}
+
 // OSObject-based attributes.
 def OSConsumed : InheritableParamAttr {
   let Spellings = [Clang<"os_consumed">];
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 438d846c39eaa57..39efaee20694a43 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -7483,7 +7483,6 @@ generation of the other destruction cases, optimizing the above `foo.destroy` to
   }];
 }
 
-
 def CoroReturnTypeAndWrapperDoc : Documentation {
   let Category = DocCatDecl;
   let Content = [{
@@ -7540,3 +7539,10 @@ Note: ``a_promise_type::get_return_object`` is exempted from this analysis as it
 implementation detail of any coroutine library.
 }];
 }
+
+def CoroLifetimeBoundDoc : Documentation {
+  let Category = DocCatDecl;
+  let Content = [{
+asdam
+}];
+}
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 80b51b09bf5445f..631b6a266412ccb 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -7580,10 +7580,15 @@ static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call,
   if (ObjectArg && implicitObjectParamIsLifetimeBound(Callee))
     VisitLifetimeBoundArg(Callee, ObjectArg);
 
+  bool checkCoroCall = false;
+  if (const auto *RD = Callee->getReturnType()->getAsRecordDecl()) {
+    checkCoroCall |= RD->hasAttr<CoroLifetimeBoundAttr>() &&
+                     RD->hasAttr<CoroReturnTypeAttr>();
+  }
   for (unsigned I = 0,
                 N = std::min<unsigned>(Callee->getNumParams(), Args.size());
        I != N; ++I) {
-    if (Callee->getParamDecl(I)->hasAttr<LifetimeBoundAttr>())
+    if (checkCoroCall || Callee->getParamDecl(I)->hasAttr<LifetimeBoundAttr>())
       VisitLifetimeBoundArg(Callee->getParamDecl(I), Args[I]);
   }
 }
diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
index a57bc011c059483..dd91f4f88ad685b 100644
--- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -56,9 +56,10 @@
 // CHECK-NEXT: ConsumableAutoCast (SubjectMatchRule_record)
 // CHECK-NEXT: ConsumableSetOnRead (SubjectMatchRule_record)
 // CHECK-NEXT: Convergent (SubjectMatchRule_function)
+// CHECK-NEXT: CoroLifetimeBound (SubjectMatchRule_record)
 // CHECK-NEXT: CoroOnlyDestroyWhenComplete (SubjectMatchRule_record)
 // CHECK-NEXT: CoroReturnType (SubjectMatchRule_record)
-// CHECK-NEXT: CoroWrapper
+// CHECK-NEXT: CoroWrapper (SubjectMatchRule_function)
 // CHECK-NEXT: CountedBy (SubjectMatchRule_field)
 // CHECK-NEXT: DLLExport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
 // CHECK-NEXT: DLLImport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
diff --git a/clang/test/SemaCXX/coro-lifetimebound.cpp b/clang/test/SemaCXX/coro-lifetimebound.cpp
new file mode 100644
index 000000000000000..3f719866eae0ee4
--- /dev/null
+++ b/clang/test/SemaCXX/coro-lifetimebound.cpp
@@ -0,0 +1,93 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++20 -fsyntax-only -verify -Wall -Wextra -Wno-error=unreachable-code -Wno-unused
+
+#include "Inputs/std-coroutine.h"
+
+using std::suspend_always;
+using std::suspend_never;
+
+template <typename T> struct [[clang::coro_lifetimebound, clang::coro_return_type]] Gen {
+  struct promise_type {
+    Gen<T> get_return_object() {
+      return {};
+    }
+    suspend_always initial_suspend();
+    suspend_always final_suspend() noexcept;
+    void unhandled_exception();
+    void return_value(const T &t);
+
+    template <typename U>
+    auto await_transform(const Gen<U> &) {
+      struct awaitable {
+        bool await_ready() noexcept { return false; }
+        void await_suspend(std::coroutine_handle<>) noexcept {}
+        U await_resume() noexcept { return {}; }
+      };
+      return awaitable{};
+    }
+  };
+};
+
+template <typename T> using Co = Gen<T>;
+
+Gen<int> foo_coro(const int& b);
+
+Gen<int> foo_coro(const int& b) {
+  if (b > 0)
+    co_return 1;
+  co_return 2;
+}
+
+int getInt() { return 0; }
+
+Co<int> bar_coro(const int &b, int c) {
+  int x = co_await foo_coro(b);
+  int y = co_await foo_coro(1);
+  int z = co_await foo_coro(getInt());
+  auto unsafe1 = foo_coro(1); // expected-warning {{temporary whose address is used as value of local variable}}
+  auto unsafe2 = foo_coro(getInt()); // expected-warning {{temporary whose address is used as value of local variable}}
+  auto  safe1 = foo_coro(b);
+  auto  safe2 = foo_coro(c);
+  co_return co_await foo_coro(co_await foo_coro(1));
+}
+
+[[clang::coro_wrapper]] Gen<int> plain_return_co(int b) {
+  return foo_coro(b); // expected-warning {{address of stack memory associated with parameter}}
+}
+
+[[clang::coro_wrapper]] Gen<int> safe_forwarding(const int& b) {
+  return foo_coro(b);
+}
+
+[[clang::coro_wrapper]] Gen<int> unsafe_wrapper(int b) {
+  return safe_forwarding(b); // expected-warning {{address of stack memory associated with parameter}}
+}
+
+[[clang::coro_wrapper]] Co<int> complex_plain_return(int b) {
+  return b > 0 
+      ? foo_coro(1)   // expected-warning {{returning address of local temporary object}}
+      : bar_coro(0, 1); // expected-warning {{returning address of local temporary object}}
+}
+
+#define CORO_WRAPPER \
+  _Pragma("clang diagnostic push") \
+  _Pragma("clang diagnostic ignored \"-Wc++23-extensions\"") \
+  [[clang::coro_wrapper]] \
+  _Pragma("clang diagnostic pop")
+
+void lambdas() {
+  auto unsafe_lambda = [] CORO_WRAPPER (int b) {
+    return foo_coro(b); // expected-warning {{address of stack memory associated with parameter}}
+  };
+  auto safe_lambda = [](int b) -> Co<int> {
+    int x = co_await foo_coro(1);
+    co_return x + co_await foo_coro(b);
+  };
+}
+// =============================================================================
+// Safe usage when parameters are value
+// =============================================================================
+namespace by_value {
+Gen<int> value_coro(int b) { co_return co_await foo_coro(b); }
+[[clang::coro_wrapper]] Gen<int> wrapper1(int b) { return value_coro(b); }
+[[clang::coro_wrapper]] Gen<int> wrapper2(const int& b) { return value_coro(b); }
+}

>From cfc5fa4e59e551a391ecc9be8db6d4b98d37e45a Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Mon, 20 Nov 2023 16:14:25 +0100
Subject: [PATCH 2/5] add docs

---
 clang/include/clang/Basic/AttrDocs.td     | 49 ++++++++++++++++++++++-
 clang/test/SemaCXX/coro-lifetimebound.cpp | 35 +++++++++-------
 2 files changed, 69 insertions(+), 15 deletions(-)

diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 39efaee20694a43..534f0f628ecaf34 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -7543,6 +7543,53 @@ implementation detail of any coroutine library.
 def CoroLifetimeBoundDoc : Documentation {
   let Category = DocCatDecl;
   let Content = [{
-asdam
+The ``[[clang::coro_lifetimebound]]`` is a class attribute which can be applied
+to a `coroutine return type (CRT) <https://clang.llvm.org/docs/AttributeReference.html#coro-return-type>` _ (i.e.
+it should also be annotated with ``[[clang::coro_return_type]]``).
+
+All arguments to a function are considered to be lifetime bound if the function
+returns a coroutine return type (CRT) annotated with ``[[clang::coro_lifetimebound]]``.
+
+Reference parameters of a coroutine are susceptible to capturing references to temporaries or local variables.
+
+For example,
+
+.. code-block:: c++
+  task<int> coro(const int& a) { co_return a + 1; }
+  task<int> dangling_refs(int a) {
+    // `coro` captures reference to a temporary. `foo` would now contain a dangling reference to `a`.
+    auto foo = coro(1);
+    // `coro` captures reference to local variable `a` which is destroyed after the return. 
+    return coro(a);
+  }
+
+`Lifetime bound <https://clang.llvm.org/docs/AttributeReference.html#lifetimebound> _` static analysis
+can be used to detect such instances when coroutines capture references which may die earlier than the
+coroutine frame itself. In the above example, if the CRT `task` is annotated with 
+``[[clang::coro_lifetimebound]]``, then lifetime bound analysis would detect capturing reference to
+temporaries or return address of a local variable.
+
+Both coroutines and coroutine wrappers are part of this analysis.
+
+.. code-block:: c++
+  template <typename T> struct [[clang::coro_return_type, clang::coro_lifetimebound]] Task {
+    using promise_type = some_promise_type;
+  };
+
+  Task<int> coro(const int& a) { co_return a + 1; }
+  Task<int> [[clang::coro_wrapper]] coro_wrapper(const int& a, const int& b) { 
+    return a > b ? coro(a) : coro(b);
+  }
+  Task<int> temporary_reference() {
+    auto foo = coro(1); // warning: capturing reference to a temporary which would die after the expression.
+
+    int a = 1;
+    auto bar = coro_wrapper(a, 0); // warning: `b` captures reference to a temporary.
+
+    co_return co_await coro(1); // fine.
+  }
+  Task<int> stack_reference(int a) {
+    return coro(a); // returning address of stack variable `a`.
+  }
 }];
 }
diff --git a/clang/test/SemaCXX/coro-lifetimebound.cpp b/clang/test/SemaCXX/coro-lifetimebound.cpp
index 3f719866eae0ee4..a12315d17096199 100644
--- a/clang/test/SemaCXX/coro-lifetimebound.cpp
+++ b/clang/test/SemaCXX/coro-lifetimebound.cpp
@@ -5,9 +5,9 @@
 using std::suspend_always;
 using std::suspend_never;
 
-template <typename T> struct [[clang::coro_lifetimebound, clang::coro_return_type]] Gen {
+template <typename T> struct [[clang::coro_lifetimebound, clang::coro_return_type]] Co {
   struct promise_type {
-    Gen<T> get_return_object() {
+    Co<T> get_return_object() {
       return {};
     }
     suspend_always initial_suspend();
@@ -16,7 +16,7 @@ template <typename T> struct [[clang::coro_lifetimebound, clang::coro_return_typ
     void return_value(const T &t);
 
     template <typename U>
-    auto await_transform(const Gen<U> &) {
+    auto await_transform(const Co<U> &) {
       struct awaitable {
         bool await_ready() noexcept { return false; }
         void await_suspend(std::coroutine_handle<>) noexcept {}
@@ -27,11 +27,7 @@ template <typename T> struct [[clang::coro_lifetimebound, clang::coro_return_typ
   };
 };
 
-template <typename T> using Co = Gen<T>;
-
-Gen<int> foo_coro(const int& b);
-
-Gen<int> foo_coro(const int& b) {
+Co<int> foo_coro(const int& b) {
   if (b > 0)
     co_return 1;
   co_return 2;
@@ -50,15 +46,15 @@ Co<int> bar_coro(const int &b, int c) {
   co_return co_await foo_coro(co_await foo_coro(1));
 }
 
-[[clang::coro_wrapper]] Gen<int> plain_return_co(int b) {
+[[clang::coro_wrapper]] Co<int> plain_return_co(int b) {
   return foo_coro(b); // expected-warning {{address of stack memory associated with parameter}}
 }
 
-[[clang::coro_wrapper]] Gen<int> safe_forwarding(const int& b) {
+[[clang::coro_wrapper]] Co<int> safe_forwarding(const int& b) {
   return foo_coro(b);
 }
 
-[[clang::coro_wrapper]] Gen<int> unsafe_wrapper(int b) {
+[[clang::coro_wrapper]] Co<int> unsafe_wrapper(int b) {
   return safe_forwarding(b); // expected-warning {{address of stack memory associated with parameter}}
 }
 
@@ -78,6 +74,17 @@ void lambdas() {
   auto unsafe_lambda = [] CORO_WRAPPER (int b) {
     return foo_coro(b); // expected-warning {{address of stack memory associated with parameter}}
   };
+  auto coro_lambda = [] (const int&) -> Co<int> {
+    co_return 0;
+  };
+  auto unsafe_coro_lambda = [&] (const int& b) -> Co<int> {
+    int x = co_await coro_lambda(b);
+    auto safe = coro_lambda(b);
+    auto unsafe1 = coro_lambda(1); // expected-warning {{temporary whose address is used as value of local variable}}
+    auto unsafe2 = coro_lambda(getInt()); // expected-warning {{temporary whose address is used as value of local variable}}
+    auto unsafe3 = coro_lambda(co_await coro_lambda(b)); // expected-warning {{temporary whose address is used as value of local variable}}
+    co_return co_await safe;
+  };
   auto safe_lambda = [](int b) -> Co<int> {
     int x = co_await foo_coro(1);
     co_return x + co_await foo_coro(b);
@@ -87,7 +94,7 @@ void lambdas() {
 // Safe usage when parameters are value
 // =============================================================================
 namespace by_value {
-Gen<int> value_coro(int b) { co_return co_await foo_coro(b); }
-[[clang::coro_wrapper]] Gen<int> wrapper1(int b) { return value_coro(b); }
-[[clang::coro_wrapper]] Gen<int> wrapper2(const int& b) { return value_coro(b); }
+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); }
 }

>From 3f5f1a370300ff41483efd6b68c4dd9d85ed025a Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Mon, 20 Nov 2023 16:23:23 +0100
Subject: [PATCH 3/5] add Release notes

---
 clang/docs/ReleaseNotes.rst           | 4 ++++
 clang/include/clang/Basic/AttrDocs.td | 4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 52c9d6eb69617b0..3fdb93fda8eec7f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -311,6 +311,10 @@ Attribute Changes in Clang
   should be a coroutine. A non-coroutine function marked with ``[[clang::coro_wrapper]]``
   is still allowed to return the such a type. This is helpful for analyzers to recognize coroutines from the function signatures.
 
+- Clang now introduced ``[[clang::coro_lifetimebound]]`` attribute.
+  All arguments to a function are considered to be lifetime bound if the function 
+  returns a type annotated with ``[[clang::coro_lifetimebound]]`` and ``[[clang::coro_return_type]]``.
+
 Improvements to Clang's diagnostics
 -----------------------------------
 - Clang constexpr evaluator now prints template arguments when displaying
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 534f0f628ecaf34..516ef76615f40f1 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -7547,8 +7547,8 @@ The ``[[clang::coro_lifetimebound]]`` is a class attribute which can be applied
 to a `coroutine return type (CRT) <https://clang.llvm.org/docs/AttributeReference.html#coro-return-type>` _ (i.e.
 it should also be annotated with ``[[clang::coro_return_type]]``).
 
-All arguments to a function are considered to be lifetime bound if the function
-returns a coroutine return type (CRT) annotated with ``[[clang::coro_lifetimebound]]``.
+All arguments to a function are considered to be `lifetime bound <https://clang.llvm.org/docs/AttributeReference.html#lifetimebound> _`
+if the function returns a coroutine return type (CRT) annotated with ``[[clang::coro_lifetimebound]]``.
 
 Reference parameters of a coroutine are susceptible to capturing references to temporaries or local variables.
 

>From 54c62257caaa3c5070b977bad9bc96bdd6c8c69e Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Mon, 20 Nov 2023 17:41:03 +0100
Subject: [PATCH 4/5] fix codeblock formatting

---
 clang/include/clang/Basic/AttrDocs.td | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 516ef76615f40f1..f54ae048dfa4874 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -7555,6 +7555,7 @@ Reference parameters of a coroutine are susceptible to capturing references to t
 For example,
 
 .. code-block:: c++
+
   task<int> coro(const int& a) { co_return a + 1; }
   task<int> dangling_refs(int a) {
     // `coro` captures reference to a temporary. `foo` would now contain a dangling reference to `a`.
@@ -7572,6 +7573,7 @@ temporaries or return address of a local variable.
 Both coroutines and coroutine wrappers are part of this analysis.
 
 .. code-block:: c++
+
   template <typename T> struct [[clang::coro_return_type, clang::coro_lifetimebound]] Task {
     using promise_type = some_promise_type;
   };

>From 932ddd98e6b46b21f1d85834d2586c6de8e024aa Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Tue, 21 Nov 2023 03:39:40 +0100
Subject: [PATCH 5/5] address comments and add negative test

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

diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 631b6a266412ccb..d8d7d2ee4f48c77 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -7582,8 +7582,8 @@ static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call,
 
   bool checkCoroCall = false;
   if (const auto *RD = Callee->getReturnType()->getAsRecordDecl()) {
-    checkCoroCall |= RD->hasAttr<CoroLifetimeBoundAttr>() &&
-                     RD->hasAttr<CoroReturnTypeAttr>();
+    checkCoroCall = RD->hasAttr<CoroLifetimeBoundAttr>() &&
+                    RD->hasAttr<CoroReturnTypeAttr>();
   }
   for (unsigned I = 0,
                 N = std::min<unsigned>(Callee->getNumParams(), Args.size());
diff --git a/clang/test/SemaCXX/coro-lifetimebound.cpp b/clang/test/SemaCXX/coro-lifetimebound.cpp
index a12315d17096199..92229b615866fd7 100644
--- a/clang/test/SemaCXX/coro-lifetimebound.cpp
+++ b/clang/test/SemaCXX/coro-lifetimebound.cpp
@@ -98,3 +98,23 @@ 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); }
 }
+
+// =============================================================================
+// Lifetime bound but not a Coroutine Return Type: No analysis.
+// =============================================================================
+namespace not_a_crt {
+template <typename T> struct [[clang::coro_lifetimebound]] Co {
+  struct promise_type {
+    Co<T> get_return_object() {
+      return {};
+    }
+    suspend_always initial_suspend();
+    suspend_always final_suspend() noexcept;
+    void unhandled_exception();
+    void return_value(const T &t);
+  };
+};
+
+Co<int> foo_coro(const int& a) { co_return a; }
+Co<int> bar(int a) { return foo_coro(a); }
+} // namespace not_a_crt



More information about the cfe-commits mailing list