diff mbox series

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

Message ID CAEQVFRFkapOFWy1b8XR3rXvBH4yO9OtubQS=2OEfECMLB2qfEQ@mail.gmail.com
State New
Headers show
Series c++: Fix ICE due to shared BLOCK node in coroutine generation [PR103328] | expand

Commit Message

Benno Evers March 30, 2022, 1:06 p.m. UTC
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>
---
 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

Comments

Jason Merrill April 1, 2022, 7:07 p.m. UTC | #1
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() {}
diff mbox series

Patch

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)
   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() {}