Skip to content
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

[[coro_lifetimebound]] does not catch lifetime issues with lambda captures #76995

Closed
usx95 opened this issue Jan 4, 2024 · 1 comment · Fixed by #77066
Closed

[[coro_lifetimebound]] does not catch lifetime issues with lambda captures #76995

usx95 opened this issue Jan 4, 2024 · 1 comment · Fixed by #77066
Assignees
Labels
coroutines C++20 coroutines

Comments

@usx95
Copy link
Contributor

usx95 commented Jan 4, 2024

The pattern of interest is

co_task<int> coro() {
    int a = 1;
    auto lamb = [a](int x, const int& y) -> co_task<int> {
        co_return x + y + a; // 'a' in the lambda object dies after the iniital_suspend.
    }(1, a);
    co_return co_await lamb;
}

https://godbolt.org/z/GWPEovWWc

Lambda captures (even by value) are prone to use after free once the lambda object dies. In the above example, the lambda object appears only as a temporary in the call expression.

It should be fine if this is used in a co_await expression due to lifetime extension of temporaries. Example:

co_task<int> coro() {
    int a = 1;
    int res = co_await [a](int x, const int& y) -> co_task<int> {
        co_return x + y + a;
    }(1, a);
    co_return res;
}
@usx95 usx95 self-assigned this Jan 4, 2024
@usx95 usx95 added coroutines C++20 coroutines and removed new issue labels Jan 4, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 4, 2024

@llvm/issue-subscribers-coroutines

Author: Utkarsh Saxena (usx95)

The pattern of interest is ```cpp co_task<int> coro() { int a = 1; auto lamb = [a](int x, const int& y) -> co_task<int> { co_return x + y + a; // 'a' in the lambda object dies after the iniital_suspend. }(1, a); co_return co_await lamb; } ``` https://godbolt.org/z/GWPEovWWc

Lambda captures (even by value) are prone to use after free once the lambda object dies. In the above example, it appears only as a temporary in the call expression.

It should be fine if this is used in a co_await expression. Example:

co_task&lt;int&gt; coro() {
    int a = 1;
    int res = co_await [a](int x, const int&amp; y) -&gt; co_task&lt;int&gt; {
        co_return x + y + a;
    }(1, a);
    co_return res;
}

usx95 added a commit that referenced this issue Jan 18, 2024
…aptures (#77066)

### Problem

```cpp
co_task<int> coro() {
    int a = 1;
    auto lamb = [a]() -> co_task<int> {
        co_return a; // 'a' in the lambda object dies after the iniital_suspend in the lambda coroutine.
    }();
    co_return co_await lamb;
}
```
[use-after-free](https://godbolt.org/z/GWPEovWWc)

Lambda captures (even by value) are prone to use-after-free once the
lambda object dies. In the above example, the lambda object appears only
as a temporary in the call expression. It dies after the first
suspension (`initial_suspend`) in the lambda.
On resumption in `co_await lamb`, the lambda accesses `a` which is part
of the already-dead lambda object.

---

### Solution

This problem can be formulated by saying that the `this` parameter of
the lambda call operator is a lifetimebound parameter. The lambda object
argument should therefore live atleast as long as the return object.
That said, this requirement does not hold if the lambda does not have a
capture list. In principle, the coroutine frame still has a reference to
a dead lambda object, but it is easy to see that the object would not be
used in the lambda-coroutine body due to no capture list.

It is safe to use this pattern inside a`co_await` expression due to the
lifetime extension of temporaries. Example:

```cpp
co_task<int> coro() {
    int a = 1;
    int res = co_await [a]() -> co_task<int> { co_return a; }();
    co_return res;
}
```
---
### Background

This came up in the discussion with seastar folks on
[RFC](https://discourse.llvm.org/t/rfc-lifetime-bound-check-for-parameters-of-coroutines/74253/19?u=usx95).
This is a fairly common pattern in continuation-style-passing (CSP)
async programming involving futures and continuations. Document ["Lambda
coroutine
fiasco"](https://github.com/scylladb/seastar/blob/master/doc/lambda-coroutine-fiasco.md)
by Seastar captures the problem.
This pattern makes the migration from CSP-style async programming to
coroutines very bugprone.


Fixes #76995

---------

Co-authored-by: Chuanqi Xu <yedeng.yd@linux.alibaba.com>
ampandey-1995 pushed a commit to ampandey-1995/llvm-project that referenced this issue Jan 19, 2024
…aptures (llvm#77066)

### Problem

```cpp
co_task<int> coro() {
    int a = 1;
    auto lamb = [a]() -> co_task<int> {
        co_return a; // 'a' in the lambda object dies after the iniital_suspend in the lambda coroutine.
    }();
    co_return co_await lamb;
}
```
[use-after-free](https://godbolt.org/z/GWPEovWWc)

Lambda captures (even by value) are prone to use-after-free once the
lambda object dies. In the above example, the lambda object appears only
as a temporary in the call expression. It dies after the first
suspension (`initial_suspend`) in the lambda.
On resumption in `co_await lamb`, the lambda accesses `a` which is part
of the already-dead lambda object.

---

### Solution

This problem can be formulated by saying that the `this` parameter of
the lambda call operator is a lifetimebound parameter. The lambda object
argument should therefore live atleast as long as the return object.
That said, this requirement does not hold if the lambda does not have a
capture list. In principle, the coroutine frame still has a reference to
a dead lambda object, but it is easy to see that the object would not be
used in the lambda-coroutine body due to no capture list.

It is safe to use this pattern inside a`co_await` expression due to the
lifetime extension of temporaries. Example:

```cpp
co_task<int> coro() {
    int a = 1;
    int res = co_await [a]() -> co_task<int> { co_return a; }();
    co_return res;
}
```
---
### Background

This came up in the discussion with seastar folks on
[RFC](https://discourse.llvm.org/t/rfc-lifetime-bound-check-for-parameters-of-coroutines/74253/19?u=usx95).
This is a fairly common pattern in continuation-style-passing (CSP)
async programming involving futures and continuations. Document ["Lambda
coroutine
fiasco"](https://github.com/scylladb/seastar/blob/master/doc/lambda-coroutine-fiasco.md)
by Seastar captures the problem.
This pattern makes the migration from CSP-style async programming to
coroutines very bugprone.


Fixes llvm#76995

---------

Co-authored-by: Chuanqi Xu <yedeng.yd@linux.alibaba.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
coroutines C++20 coroutines
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants