[Bug c++/95736] New: coroutine method improperly copies awaitable

[Bug c++/95736] New: coroutine method improperly copies awaitable

[mizvekov at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95736

            Bug ID: 95736
           Summary: coroutine method improperly copies awaitable
           Product: gcc
           Version: 10.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: mizvekov at gmail dot com
  Target Milestone: ---

Created attachment 48752
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48752&action=edit
example reproduction

Compiling attached program with `g++ -std=c++20 -fcoroutines -O0 -ggdb3
test.cc` and then running gives unexpected result:

```
coro begin
suspending to footable 0x7fffee14dee0
operator() on 0x7ffff5c307e0
a.out: test.cc:33: void footable::operator()(): Assertion `handle' failed.
fish: “./a.out” terminated by signal SIGABRT (Abort)
```

Expected result, as obtained from clang with `clang++ -std=c++20 -stdlib=libc++
-O0 -ggdb3 test.cc`:
```
coro begin
suspending to footable 0x7ffff6441330
operator() on 0x7ffff6441330
resuming from footable 0x7ffff6441330
coro end
coro returns
final suspend
releasing
done
```

Notice how on the gcc (first) case, it fails because `await_suspend` and
`operator()` are operating on different copies of the awaiter, even though the
awaiter is not copy constructible.

I failed to reproduce this problem if the coroutine body was not a class
method.

Compiler explorer workspace for convenience: https://godbolt.org/z/_yqwCV

[Bug c++/95736] coroutine method improperly copies awaitable

[lewissbaker.opensource at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95736

Lewis Baker <lewissbaker.opensource at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |lewissbaker.opensource@gmai
                   |                            |l.com

--- Comment #1 from Lewis Baker <lewissbaker.opensource at gmail dot com> ---
I've also investigated this a bit more and have annotated the assembly of the
code being generated by GCC in this example (slightly modified).

https://godbolt.org/z/_HJNHC

Immediately before the 'co_await foo' expression the compiler generates
a copy of the this->foo member into the coroutine frame. But it does
this doing the equivalent of a memcpy() rather than calling the
copy-constructor.

The compiler should not be making this copy of the operand to 'co_await'
and should instead be passing the address of the 'this->foo' member to the
as the implicit object parameter in the call to
await_ready/await_suspend/await_resume.

[Bug c++/95736] coroutine method improperly copies awaitable

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95736

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |10.2
           Assignee|unassigned at gcc dot gnu.org      |iains at gcc dot gnu.org
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2020-06-21

--- Comment #2 from Iain Sandoe <iains at gcc dot gnu.org> ---
thanks for the report.

[Bug c++/95736] coroutine method improperly copies awaitable

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95736

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Iain D Sandoe <iains@gcc.gnu.org>:

https://gcc.gnu.org/g:daaed0199ee57013ae011421a7e90b7bdd295373

commit r11-1684-gdaaed0199ee57013ae011421a7e90b7bdd295373
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Sat Jun 27 08:18:34 2020 +0100

    coroutines: Handle awaiters that are sub-objects [PR95736]

    Move deciding on initializers for awaitables to the build of the
    co_await, this allows us to analyse cases that do not need
    a temporary at that point.

    As the PR shows, the late analysis meant that we  were not
    checking properly for the case that an awaiter is a sub-object
    of an existing variable outside the current function scope (and
    therefore does not need to be duplicated in the frame).

    gcc/cp/ChangeLog:

            PR c++/95736
            * coroutines.cc (get_awaitable_var): New helper.
            (build_co_await): Check more carefully before
            copying an awaitable.
            (expand_one_await_expression): No initializer
            is required when the awaitable is not a temp.
            (register_awaits): Remove handling that is now
            completed when the await expression is built.

    gcc/testsuite/ChangeLog:

            PR c++/95736
            * g++.dg/coroutines/pr95736.C: New test.

[Bug c++/95736] coroutine method improperly copies awaitable

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95736

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Iain D Sandoe
<iains@gcc.gnu.org>:

https://gcc.gnu.org/g:c5a90f61ace5aca821224a658c223881d5b02388

commit r10-8394-gc5a90f61ace5aca821224a658c223881d5b02388
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Tue Jun 30 08:18:34 2020 +0100

    coroutines: Handle awaiters that are sub-objects [PR95736]

    Move deciding on initializers for awaitables to the build of the
    co_await, this allows us to analyse cases that do not need
    a temporary at that point.

    As the PR shows, the late analysis meant that we  were not
    checking properly for the case that an awaiter is a sub-object
    of an existing variable outside the current function scope (and
    therefore does not need to be duplicated in the frame).

    gcc/cp/ChangeLog:

            PR c++/95736
            * coroutines.cc (get_awaitable_var): New helper.
            (build_co_await): Check more carefully before
            copying an awaitable.
            (expand_one_await_expression): No initializer
            is required when the awaitable is not a temp.
            (register_awaits): Remove handling that is now
            completed when the await expression is built.

    gcc/testsuite/ChangeLog:

            PR c++/95736
            * g++.dg/coroutines/pr95736.C: New test.

    (cherry picked from commit daaed0199ee57013ae011421a7e90b7bdd295373)

[Bug c++/95736] coroutine method improperly copies awaitable

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95736

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #5 from Iain Sandoe <iains at gcc dot gnu.org> ---
fixed for master and 10.2