Re: [PATCH gcc-{11,12}] c++: top level bind when rewriting coroutines [PR106188]

[Jason Merrill <jason@redhat.com>]

On 3/13/23 02:57, Arsen Arsenović wrote:
> In the edge case of a coroutine not containing any locals, the ifcd/switch
> temporaries would get added to the coroutine frame, corrupting its
> layout. To prevent this, we can make sure there is always a BIND_EXPR at
> the top of the function body, and thus, always a place for our new
> temporaries to go without interfering with the coroutine frame.
> 
> PR c++/106188 - Incorrect frame layout after transforming conditional statement without top-level bind expression
> PR c++/106713 - if (co_await ...) crashes with a jump to ud2
> 
> 	PR c++/106188
> 	PR c++/106713
> 
> gcc/cp/ChangeLog:
> 
> 	* coroutines.cc (coro_rewrite_function_body): Ensure we have a
> 	BIND_EXPR wrapping the function body.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/coroutines/pr106188.C: New test.
> ---
> Morning,
> 
> A while back, we merged this patch into GCC 13 to fix PR106188, but
> never backported it into GCC 11 and 12, which both still suffer from
> this bug.
> 
> I've tested the same patch against releases/gcc-{11,12} and they apply
> and fix the testcase (and the testcase fails without them, as expected)
> on x86_64-pc-linux-gnu.
> 
> OK for backporting?

OK.

> Thanks in advance.
> 
>   gcc/cp/coroutines.cc                       |  9 ++++++
>   gcc/testsuite/g++.dg/coroutines/pr106188.C | 34 ++++++++++++++++++++++
>   2 files changed, 43 insertions(+)
>   create mode 100644 gcc/testsuite/g++.dg/coroutines/pr106188.C
> 
> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> index e982cdb89a7..ea3850082cf 100644
> --- a/gcc/cp/coroutines.cc
> +++ b/gcc/cp/coroutines.cc
> @@ -4067,6 +4067,15 @@ coro_rewrite_function_body (location_t fn_start, tree fnbody, tree orig,
>         BLOCK_SUPERCONTEXT (replace_blk) = top_block;
>         BLOCK_SUBBLOCKS (top_block) = replace_blk;
>       }
> +  else
> +    {
> +      /* We are missing a top level BIND_EXPR. We need one to ensure that we
> +	 don't shuffle around the coroutine frame and corrupt it.  */
> +      tree bind_wrap = build3_loc (fn_start, BIND_EXPR, void_type_node,
> +				   NULL, NULL, NULL);
> +      BIND_EXPR_BODY (bind_wrap) = fnbody;
> +      fnbody = bind_wrap;
> +    }
>   
>     /* Wrap the function body in a try {} catch (...) {} block, if exceptions
>        are enabled.  */
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr106188.C b/gcc/testsuite/g++.dg/coroutines/pr106188.C
> new file mode 100644
> index 00000000000..9db3778d079
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr106188.C
> @@ -0,0 +1,34 @@
> +// { dg-do run { target c++20 } }
> +// test case from pr106188, w/o workaround
> +#include <coroutine>
> +
> +struct task {
> +  struct promise_type {
> +    task get_return_object() { return task{}; }
> +    void return_void() {}
> +    void unhandled_exception() {}
> +    auto initial_suspend() noexcept { return std::suspend_never{}; }
> +    auto final_suspend() noexcept { return std::suspend_never{}; }
> +  };
> +};
> +
> +struct suspend_and_resume {
> +  bool await_ready() const { return false; }
> +  void await_suspend(std::coroutine_handle<> h) { h.resume(); }
> +  void await_resume() {}
> +};
> +
> +task f() {
> +  if (co_await suspend_and_resume{}, false) {}
> +}
> +
> +task g() {
> +  switch (co_await suspend_and_resume{}, 0) {
> +    default: break;
> +  }
> +}
> +
> +int main() {
> +  f();
> +  g();
> +}