public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c++/95736] New: coroutine method improperly copies awaitable
@ 2020-06-18 8:07 mizvekov at gmail dot com
2020-06-18 15:50 ` [Bug c++/95736] " lewissbaker.opensource at gmail dot com
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: mizvekov at gmail dot com @ 2020-06-18 8:07 UTC (permalink / raw)
To: gcc-bugs
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug c++/95736] coroutine method improperly copies awaitable
2020-06-18 8:07 [Bug c++/95736] New: coroutine method improperly copies awaitable mizvekov at gmail dot com
@ 2020-06-18 15:50 ` lewissbaker.opensource at gmail dot com
2020-06-21 12:56 ` iains at gcc dot gnu.org
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: lewissbaker.opensource at gmail dot com @ 2020-06-18 15:50 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug c++/95736] coroutine method improperly copies awaitable
2020-06-18 8:07 [Bug c++/95736] New: coroutine method improperly copies awaitable mizvekov at gmail dot com
2020-06-18 15:50 ` [Bug c++/95736] " lewissbaker.opensource at gmail dot com
@ 2020-06-21 12:56 ` iains at gcc dot gnu.org
2020-06-27 9:40 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: iains at gcc dot gnu.org @ 2020-06-21 12:56 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug c++/95736] coroutine method improperly copies awaitable
2020-06-18 8:07 [Bug c++/95736] New: coroutine method improperly copies awaitable mizvekov at gmail dot com
2020-06-18 15:50 ` [Bug c++/95736] " lewissbaker.opensource at gmail dot com
2020-06-21 12:56 ` iains at gcc dot gnu.org
@ 2020-06-27 9:40 ` cvs-commit at gcc dot gnu.org
2020-06-30 12:46 ` cvs-commit at gcc dot gnu.org
2020-06-30 12:49 ` iains at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-06-27 9:40 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug c++/95736] coroutine method improperly copies awaitable
2020-06-18 8:07 [Bug c++/95736] New: coroutine method improperly copies awaitable mizvekov at gmail dot com
` (2 preceding siblings ...)
2020-06-27 9:40 ` cvs-commit at gcc dot gnu.org
@ 2020-06-30 12:46 ` cvs-commit at gcc dot gnu.org
2020-06-30 12:49 ` iains at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-06-30 12:46 UTC (permalink / raw)
To: gcc-bugs
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)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug c++/95736] coroutine method improperly copies awaitable
2020-06-18 8:07 [Bug c++/95736] New: coroutine method improperly copies awaitable mizvekov at gmail dot com
` (3 preceding siblings ...)
2020-06-30 12:46 ` cvs-commit at gcc dot gnu.org
@ 2020-06-30 12:49 ` iains at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: iains at gcc dot gnu.org @ 2020-06-30 12:49 UTC (permalink / raw)
To: gcc-bugs
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-30 12:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-18 8:07 [Bug c++/95736] New: coroutine method improperly copies awaitable mizvekov at gmail dot com
2020-06-18 15:50 ` [Bug c++/95736] " lewissbaker.opensource at gmail dot com
2020-06-21 12:56 ` iains at gcc dot gnu.org
2020-06-27 9:40 ` cvs-commit at gcc dot gnu.org
2020-06-30 12:46 ` cvs-commit at gcc dot gnu.org
2020-06-30 12:49 ` iains at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).