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 <u...@google.com> Date: Mon, 20 Nov 2023 12:17:30 +0100 Subject: [PATCH 1/4] 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 <u...@google.com> Date: Mon, 20 Nov 2023 16:14:25 +0100 Subject: [PATCH 2/4] 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 <u...@google.com> Date: Mon, 20 Nov 2023 16:23:23 +0100 Subject: [PATCH 3/4] 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 <u...@google.com> Date: Mon, 20 Nov 2023 17:41:03 +0100 Subject: [PATCH 4/4] 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; }; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits