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).