Re: [PATCH] c++: Fix ICE due to shared BLOCK node in coroutine generation [PR103328]

[Jason Merrill <jason@redhat.com>]

On 3/30/22 09:06, Benno Evers via Gcc-patches wrote:
> From: Benno Evers <benno.evers@tenzir.com>
> 
> When finishing a function that is a coroutine, the function is
> transformed into a "ramp" function, and the original user-provided
> function body gets moved into a newly created "actor" function.
> 
> In this case `current_function_decl` points to the ramp function,
> but `current_binding_level->blocks` would still point to the
> scope block of the user-provided function body in the actor function,
> so when the ramp function was finished during `poplevel()` in decl.cc,
> we could end up with that block being reused as the `DECL_INITIAL()` of
> the ramp function:
> 
>      subblocks = functionbody >= 0 ? current_binding_level->blocks : 0;
>      // [...]
>      DECL_INITIAL (current_function_decl) = block ? block : subblocks;
> 
> This block would then be independently modified by subsequent passes
> touching either the ramp or the actor function, potentially causing
> an ICE depending on the order and function of these passes.
> 
> gcc/cp/ChangeLog:
> 
>          PR c++/103328
>      * coroutines.cc (morph_fn_to_coro): Reset
>        current_binding_level->blocks.
> 
> gcc/testsuite/ChangeLog:
> 
>          PR c++/103328
>          * g++.dg/coroutines/pr103328.C: New test.
> 
> Co-Authored-By: Iain Sandoe <iain@sandoe.co.uk>

Looks like you also need a DCO sign-off; see

https://gcc.gnu.org/contribute.html#legal

for more information.

> ---
>   gcc/cp/coroutines.cc                       |  3 ++
>   gcc/testsuite/g++.dg/coroutines/pr103328.C | 32 ++++++++++++++++++++++
>   2 files changed, 35 insertions(+)
>   create mode 100644 gcc/testsuite/g++.dg/coroutines/pr103328.C
> 
> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> index 23dc28271a4..ece30c905e8 100644
> --- a/gcc/cp/coroutines.cc
> +++ b/gcc/cp/coroutines.cc
> @@ -4541,6 +4541,9 @@ morph_fn_to_coro (tree orig, tree *resumer, tree
> *destroyer)

gmail is breaking your patch with word wrap; see

https://www.kernel.org/doc/html/v4.17/process/email-clients.html

for information about ways to work around this, or just use an attachment.

>     BLOCK_VARS (top_block) = BIND_EXPR_VARS (ramp_bind);
>     BLOCK_SUBBLOCKS (top_block) = NULL_TREE;
> 
> +  /* Reset the current binding level to the ramp function */
> +  current_binding_level->blocks = top_block;
> +
>     /* The decl_expr for the coro frame pointer, initialize to zero so that we
>        can pass it to the IFN_CO_FRAME (since there's no way to pass a type,
>        directly apparently).  This avoids a "used uninitialized" warning.  */
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr103328.C
> b/gcc/testsuite/g++.dg/coroutines/pr103328.C
> new file mode 100644
> index 00000000000..56fb54ab316
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr103328.C
> @@ -0,0 +1,32 @@
> +// { dg-additional-options "-g" }
> +
> +#include <coroutine>
> +
> +struct task {
> +  struct promise_type {
> +    task get_return_object() { return {}; }
> +    std::suspend_never initial_suspend() { return {}; }
> +    std::suspend_never final_suspend() noexcept { return {}; }
> +    void unhandled_exception() {}
> +  };
> +  bool await_ready() { return false; }
> +  void await_suspend(std::coroutine_handle<> h) {}
> +  void await_resume() {}
> +};
> +
> +template <typename Func>
> +void call(Func func) { func(); }
> +
> +class foo {
> +  void f();
> +  task g();
> +};
> +
> +void foo::f() {
> +  auto lambda = [this]() noexcept -> task {
> +      co_await g();
> +  };
> +  (void)call<decltype(lambda)>;
> +}
> +
> +int main() {}