Skip to content

Commit

Permalink
[coroutines] Introduce [[clang::coro_disable_lifetimebound]] (#76818)
Browse files Browse the repository at this point in the history
Lifetime-bound analysis of reference parameters of coroutines and
coroutine wrappers is helpful in surfacing memory bugs associated with
using temporaries and stack variables in call expressions in plain
return statements.

This is the default semantics of `[[clang::coro_lifetimebound]]`. But it
should be okay to relax the requirements for a function when the
reference arguments are not lifetime bound. For example:

A coroutine wrapper accepts a reference parameter but does not pass it
to the underlying coroutine call.
```cpp
[[clang::coro_wrapper]] Task<int> wrapper(const Request& req) {
  return req.shouldCallA() ? coroA() : coroB();
}
```
Or passes it the coroutine by value
```cpp
Task<int> coro(std::string s) { co_return s.size(); }
[[clang::coro_wrapper]] wrapper(const std::string& s) { return coro(s); }
```

This patch allows functions to be annotated with
`[[clang::coro_disable_lifetime_bound]]` to disable lifetime bound
analysis for all calls to this function.

---
One missing piece here is a note suggesting using this annotation in
cases of lifetime warnings. This would require some more tweaks in the
lifetimebound analysis to recognize violations involving coroutines only
and produce this note only in those cases.
  • Loading branch information
usx95 committed Jan 5, 2024
1 parent 2a1e390 commit 190a75b
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 5 deletions.
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ Attribute Changes in Clang
- Clang now introduced ``[[clang::coro_lifetimebound]]`` attribute.
All parameters of a function are considered to be lifetime bound if the function
returns a type annotated with ``[[clang::coro_lifetimebound]]`` and ``[[clang::coro_return_type]]``.
This analysis can be disabled for a function by annotating the function with ``[[clang::coro_disable_lifetimebound]]``.

Improvements to Clang's diagnostics
-----------------------------------
Expand Down
8 changes: 8 additions & 0 deletions clang/include/clang/Basic/Attr.td
Original file line number Diff line number Diff line change
Expand Up @@ -1121,6 +1121,14 @@ def CoroLifetimeBound : InheritableAttr {
let SimpleHandler = 1;
}

def CoroDisableLifetimeBound : InheritableAttr {
let Spellings = [Clang<"coro_disable_lifetimebound">];
let Subjects = SubjectList<[Function]>;
let LangOpts = [CPlusPlus];
let Documentation = [CoroLifetimeBoundDoc];
let SimpleHandler = 1;
}

// OSObject-based attributes.
def OSConsumed : InheritableParamAttr {
let Spellings = [Clang<"os_consumed">];
Expand Down
26 changes: 22 additions & 4 deletions clang/include/clang/Basic/AttrDocs.td
Original file line number Diff line number Diff line change
Expand Up @@ -7671,9 +7671,12 @@ The ``[[clang::coro_lifetimebound]]`` is a class attribute which can be applied
to a coroutine return type (`CRT`_) (i.e.
it should also be annotated with ``[[clang::coro_return_type]]``).

All parameters of a function are considered to be lifetime bound. See `documentation`_
of ``[[clang::lifetimebound]]`` for more details.
if the function returns a coroutine return type (CRT) annotated with ``[[clang::coro_lifetimebound]]``.
All parameters of a function are considered to be lifetime bound if the function returns a
coroutine return type (CRT) annotated with ``[[clang::coro_lifetimebound]]``.
This lifetime bound analysis can be disabled for a coroutine wrapper or a coroutine by annotating the function
with ``[[clang::coro_disable_lifetimebound]]`` function attribute .
See `documentation`_ of ``[[clang::lifetimebound]]`` for details about lifetime bound analysis.


Reference parameters of a coroutine are susceptible to capturing references to temporaries or local variables.

Expand Down Expand Up @@ -7703,7 +7706,7 @@ Both coroutines and coroutine wrappers are part of this analysis.
};

Task<int> coro(const int& a) { co_return a + 1; }
Task<int> [[clang::coro_wrapper]] coro_wrapper(const int& a, const int& b) {
[[clang::coro_wrapper]] Task<int> coro_wrapper(const int& a, const int& b) {
return a > b ? coro(a) : coro(b);
}
Task<int> temporary_reference() {
Expand All @@ -7718,6 +7721,21 @@ Both coroutines and coroutine wrappers are part of this analysis.
return coro(a); // warning: returning address of stack variable `a`.
}

This analysis can be disabled for all calls to a particular function by annotating the function
with function attribute ``[[clang::coro_disable_lifetimebound]]``.
For example, this could be useful for coroutine wrappers which accept reference parameters
but do not pass them to the underlying coroutine or pass them by value.

.. code-block:: c++

Task<int> coro(int a) { co_return a + 1; }
[[clang::coro_wrapper, clang::coro_disable_lifetimebound]] Task<int> coro_wrapper(const int& a) {
return coro(a + 1);
}
void use() {
auto task = coro_wrapper(1); // use of temporary is fine as the argument is not lifetime bound.
}

.. _`documentation`: https://clang.llvm.org/docs/AttributeReference.html#lifetimebound
.. _`CRT`: https://clang.llvm.org/docs/AttributeReference.html#coro-return-type
}];
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Sema/SemaInit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7581,7 +7581,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>();
RD->hasAttr<CoroReturnTypeAttr>() &&
!Callee->hasAttr<CoroDisableLifetimeBoundAttr>();
}
for (unsigned I = 0,
N = std::min<unsigned>(Callee->getNumParams(), Args.size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
// CHECK-NEXT: ConsumableAutoCast (SubjectMatchRule_record)
// CHECK-NEXT: ConsumableSetOnRead (SubjectMatchRule_record)
// CHECK-NEXT: Convergent (SubjectMatchRule_function)
// CHECK-NEXT: CoroDisableLifetimeBound (SubjectMatchRule_function)
// CHECK-NEXT: CoroLifetimeBound (SubjectMatchRule_record)
// CHECK-NEXT: CoroOnlyDestroyWhenComplete (SubjectMatchRule_record)
// CHECK-NEXT: CoroReturnType (SubjectMatchRule_record)
Expand Down
15 changes: 15 additions & 0 deletions clang/test/SemaCXX/coro-lifetimebound.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,18 @@ CoNoCRT<int> bar(int a) {
co_return 1;
}
} // namespace not_a_crt

// =============================================================================
// Not lifetime bound coroutine wrappers: [[clang::coro_disable_lifetimebound]].
// =============================================================================
namespace disable_lifetimebound {
Co<int> foo(int x) { co_return x; }

[[clang::coro_wrapper, clang::coro_disable_lifetimebound]]
Co<int> foo_wrapper(const int& x) { return foo(x); }

[[clang::coro_wrapper]] Co<int> caller() {
// The call to foo_wrapper is wrapper is safe.
return foo_wrapper(1);
}
} // namespace disable_lifetimebound

0 comments on commit 190a75b

Please sign in to comment.