-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[coroutines] Introduce [[clang::coro_lifetimebound]] #72851
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-clang Author: Utkarsh Saxena (usx95) ChangesAdds attribute All arguments to a function are considered to be lifetime bound if the function Full diff: https://github.com/llvm/llvm-project/pull/72851.diff 6 Files Affected:
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/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..516ef76615f40f1 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,57 @@ 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 = [{
+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 <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.
+
+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/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..a12315d17096199
--- /dev/null
+++ b/clang/test/SemaCXX/coro-lifetimebound.cpp
@@ -0,0 +1,100 @@
+// 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]] 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);
+
+ template <typename U>
+ auto await_transform(const Co<U> &) {
+ struct awaitable {
+ bool await_ready() noexcept { return false; }
+ void await_suspend(std::coroutine_handle<>) noexcept {}
+ U await_resume() noexcept { return {}; }
+ };
+ return awaitable{};
+ }
+ };
+};
+
+Co<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]] Co<int> plain_return_co(int b) {
+ return foo_coro(b); // expected-warning {{address of stack memory associated with parameter}}
+}
+
+[[clang::coro_wrapper]] Co<int> safe_forwarding(const int& b) {
+ return foo_coro(b);
+}
+
+[[clang::coro_wrapper]] Co<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 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);
+ };
+}
+// =============================================================================
+// 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); }
+}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Please wait for few days in case there are other comments.
✅ With the latest revision this PR passed the C/C++ code formatter. |
2ecc4b5
to
0d77978
Compare
(cherry picked from commit 28e9fda4b78e1e60287048891cc92bafdef3ac4c)
(cherry picked from commit 3f5f1a370300ff41483efd6b68c4dd9d85ed025a)
(cherry picked from commit 54c62257caaa3c5070b977bad9bc96bdd6c8c69e)
(cherry picked from commit 932ddd98e6b46b21f1d85834d2586c6de8e024aa)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good from my side. Some nits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a few suggestiosn.
One is in the comment.
I also suggest to add a warning when users mark the type as coro_lifetimebound
without coro_return_type
.
The analysis will be disabled in this case and it may be hard to understand why.
However, I don't think that's a reason to block this change. This could be a follow-up.
I would add the mentioned warning in a follow-up change. |
Adds attribute
[[clang::coro_lifetimebound]]
.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]]
.