<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>Issue</th>
<td>
<a href=https://github.com/llvm/llvm-project/issues/65054>65054</a>
</td>
</tr>
<tr>
<th>Summary</th>
<td>
[bug] clang incorrectly uses coroutine frame for scratch space after suspending (take 2)
</td>
</tr>
<tr>
<th>Labels</th>
<td>
new issue
</td>
</tr>
<tr>
<th>Assignees</th>
<td>
</td>
</tr>
<tr>
<th>Reporter</th>
<td>
jacobsa
</td>
</tr>
</table>
<pre>
Starting a new thread for this, because the previous threads have become long and confused by rollbacks and cherrypicks. Previous history here:
* #56301: this issue was fixed for a reproducer I filed, but not for programs/optimization levels (see below).
* #65018: a regression caused by the first try at fixing #56301. It was fixed by reverting.
---
I've found another case where clang incorrectly writes to the coroutine frame after suspending, introducing a data race. To recap, **clang must not write to the coroutine frame after suspending, because another thread may have already resumed/destroyed the coroutine** by the time the write happens.
Here is the program, a slight variant of the one in #65018 (with a fix for the "caller always leaks" issue discussed in #65030):
```c++
#include <coroutine>
// A simple awaiter type with an await_suspend method that can't be
// inlined.
struct Awaiter {
const int& x;
bool await_ready() { return false; }
std::coroutine_handle<> await_suspend(const std::coroutine_handle<> h);
void await_resume() {}
};
struct MyTask {
// A lazy promise with an await_transform method that supports awaiting
// integer references using the Awaiter struct above.
struct promise_type {
MyTask get_return_object() {
return MyTask{
std::coroutine_handle<promise_type>::from_promise(*this),
};
}
std::suspend_always initial_suspend() { return {}; }
std::suspend_always final_suspend() noexcept { return {}; }
void unhandled_exception();
auto await_transform(const int& x) { return Awaiter{x}; }
};
std::coroutine_handle<> h;
};
// A global array of integers.
int g_array[32];
// A coroutine that awaits each integer in the global array.
MyTask FooBar() {
for (const int& x : g_array) {
co_await x;
}
}
```
([Compiler Explorer](https://godbolt.org/z/6r5a5f17f))
Compiler Explorer isn't yet up to date with recent commits, but I built clang manually at `b32aa72afc` and then ran `./bin/clang -std=c++20 -O0 -S -masm=intel foo.cc`. This gives me the following [output](https://gist.github.com/jacobsa/a0fc8b5a303853a1fceba85639a6d6c4):
```asm
; (In the preamble before modifying rdi)
;
; Stash the address 64 bytes into the coroutine frame in the local thread's
; stack.
mov rax, rdi
add rax, 64
mov qword ptr [rbp - 96], rax # 8-byte Spill
[...]
; Call await_suspend, then store its result on the local thread's stack.
call _ZN7Awaiter13await_suspendENSt7__n486116coroutine_handleIvEE@PLT
.Ltmp16:
mov qword ptr [rbp - 200], rax # 8-byte Spill
jmp .LBB19_17
.LBB19_17:
; Reload `&coroutine_frame + 64` into rdi.
mov rdi, qword ptr [rbp - 96] # 8-byte Reload
; Reload the result of await_suspend into rax.
mov rax, qword ptr [rbp - 200] # 8-byte Reload
; Write the result of await_suspend to `&coroutine_frame + 64`.
; ------> This is the bug!
mov qword ptr [rdi], rax
; Call std::coroutine_handle::address handing it a `this` pointer that points
; at the coroutine stack. Use the result as an indirect jump target.
call _ZNKSt7__n486116coroutine_handleIvE7addressEv
mov rdi, rax
mov rax, qword ptr [rax]
add rsp, 256
pop rbp
.cfi_def_cfa rsp, 8
jmp rax # TAILCALL
```
@ChuanqiXu9
</pre>
<img width="1px" height="1px" alt="" src="http://email.email.llvm.org/o/eJyUWFtv67gR_jXMy8CGTFmy_ZCHOInRoOl20ZOiRV-MkTSyeEKRWpKy4_31xUjyPcnBBjkXS-Twm2--udDovdoYonuRLEXydIdtqKy7_4m5zTzeZbbY3_8I6IIyG0AwtINQOcICSusgVMoL-QgZ5dh6glARNI62yrZ-WOehwi3xClsTaMtmTAG5NWXrqYBsD85qnWH-7vs3FTm3b1T-7sfw-8FYpXywbg8VORLxg4ieRHT4Wz6AkHGSxtFExA8dKFDetwQ79FCqD-rRIjhqnC3anBy8QKk0FR36NoCxoVvTOLtxWHshV7YJqlZ_YlDWgKYtaQ9Czj2xN9ruhFyMP8GRJtFkzjj4uI0j73l_x0_nLXNUKucDBLcHDAyQyT24MIaXcAac-aEtdQG4OG40Gp1_fBFytiUobWsKQGNDRQ5y9AQ75gxyjWYDyuTWOcqD3sPOqUAegu0g5dbZNihDUDqsCbAM5MC3viFTKLNhopQJHX29FgoMCA5zGsObBUc5NrxISGaiP65ufU9td9ZfOeqgqIMng-hq3Pd6Qs2fmRvf1hzGVUE-OLun4vKMHs6B-KDqXqU9oAqbhoy_4PVvzJbyg5Y7NTAgBK_VpgqwRafQBLBlt8QaAmWOgWeF7FSoADl-Q5IQCClz1JocoN7h3oMmfPdCykGohfJ561khR1txJOTiWupp1P_mQi75dxBerEyu24JAxI8nz-PnS3muhFzBA3hVN5oAd6iY97BvCHrIpn-4HkIBNYXKMp8YIEcj5CxARhfmlNHKUDEw6INr8wAPg2kxGxAC57sPLCAhU_gQ8fIcGkBmrR4O7-Iq5FzIBRsAR6F1BkrUnkS8BDF7OuzyoWCC4oejz-sKTaFJxI8ifr70Rsh5D-JXu6qO9yPyrVXFERqL7YTtCIX_c-nSwMQ_9m_o38-JOIZB4597Flit_DX_waHxpXX1RQR82zTWBd8v4jy5NKlMoA05cFSSI5OTh9ZzqrIADyEZcGFmtzQ-8dg9HMCsO0WcQYaDGxtiDjgca5v9pDycUXFcC4eI9ZtO777j_fxo1m23rnS2Xg9vupMe-n6zEPLx_Lxz9k_yOK04HjwoYT3koDIqKNRnArnS3BDiC9F9ZaxU5saUsfSRUxN-ZRMGlbWmJ6RY9_uUNb2lm3wBwDbYa7kcJX7Ms0t_BhGI2fLjFsOthn-dYNVpx83uo9A32maoAZ3DPVfNQaeHqqtMgM26eyuSZSxF8qWlU-PoEqLz3gNhXh3Fr0wn9_Mzh3MGDa-sXaK7FS5X6hv-gPv4Ady10HO77hCclTOAi5JwWbMvXZqLZPlo60ZxU3j-aLR15Nh3Oa9CaDzz3vm9sUVmdRhbtxFy9aeQq9QlmJSTWdmlwuLc7o1FUL6v23sK0DbcggsMQ8FxlJMJkNu6VsEfRqEXyFqlwzAx1Gha1LqbVEQaZbFEnEksc5FG3bwWKjLg0PDbsZCrTBkhV_3mUSegp6FbyQhG_4xg9ANGNfpaxE8cNQ2lteOc7Y3hjWe3jdqSh6FRl1Zru-sGpGRp29C04VOWlA_jjQpVm41zWwu5GkZYIVcYlfk8SzCO4nkS46TMKcN5ksYLTIs0n37TZxnmUFc4W-T8xRyGXKwzzZNgaR1BbQtV7hmlK9QxJgDnyoiX8COgrzoDWBQ8GkI6hWzPY5gyX0xHg6K1zVEPY5CQM39u1gfM3w_VvLZbES0cfnA8Gc2wEIvi9DydHh73y__YWVdAExyz7LIGRrBIO54fweEHDIVWxjAfMV740SitB7KS5Xg85sUXtYOBPaLW1134sVcMj_MEnL_cVXUA-7mfl87xECWixfp_v82GYjaJL-w___YjzNZrM52nk0l6XbZets_PYhr9_vrW2xu_hrqZpMfof82GjKJrOj5nA37WjYgW49flcrJYT2bnpJwexhc0_Yu0xYITSMgzzH38hVxyuNKol4gr1Pgydp3iHuGrEN7Grj_uCOAWCMfhEJXyaibsQeDHNYheV18y9xdA_Ke_K3yDIdhfcTU-NzjqfrhfvfVXw8541m6EnHyXBoU6hfwzoJ26v2mR_PSQ5_yMy4MKgAy-m2TSCBrLNdD1Da37cJHZGK5qQp8N8G9_wRDy1RmUKRTf7eBnWzcQ0G0o3CbO33-RILMB8vP2U5mduPgm8PhxLAfHwuO726FM0sPzxnKiuKw5PBjnpVoXVK7zEmFYPz-87NPqVIs-_2GBvT28vD4-vL5-136n0WPVovlD_bddwF1xHxeLeIF3dD9JF7GcyWk0uavuszymLKZCZhH_mZcLytK0nJQySZKklHfqXkYyjuZyEU1kOp2MKcmQyjyV04lclAmKaUQ1Kj3WeltzC7_r7nv3aRIl0zuNGWnfffUipaFdfxkUkmegO3fPe0ZZu_FiGmnlgz9ZCSro7jsbVnHy9MntvvXkb3oJDzk-dxjyCnyD-e3dm3tcwHcCKeTirnX6_rrPnrVYhjP8M2qc7W8Eq84Jz3MKO_n_AAAA__-DR6gQ">