Bug 97452 - [coroutines] incorrect sequencing of await_resume() when multiple co_await expressions occur in a single statement
Summary: [coroutines] incorrect sequencing of await_resume() when multiple co_await ex...
Status: ASSIGNED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 10.2.0
: P3 normal
Target Milestone: 10.4
Assignee: Iain Sandoe
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2020-10-16 01:40 UTC by Lewis Baker
Modified: 2021-05-04 12:31 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-10-16 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Lewis Baker 2020-10-16 01:40:26 UTC
See https://godbolt.org/z/9Kj9o8

Compile the following program with GCC trunk and flags: -std=c++20

----
#include <coroutine>
#include <exception>
#include <utility>
#include <cassert>
#include <cstdio>

struct task {
    struct promise_type {
        task get_return_object() noexcept {
            return task{std::coroutine_handle<promise_type>::from_promise(*this)};
        }
        std::suspend_always initial_suspend() noexcept {
            std::puts("task initial_suspend");
            return {};
        }
        
        struct yield_awaiter {
            bool await_ready() noexcept { return false; }
            auto await_suspend(std::coroutine_handle<promise_type> h) noexcept {
                std::puts("task yielding/returning value");
                return std::exchange(h.promise().continuation, {});
            }
            void await_resume() noexcept {
                std::puts("task resumed");
            }
        };

        yield_awaiter yield_value(int x) noexcept {
            value = x;
            return {};
        }

        void return_value(int x) noexcept {
            value = x;
        }

        yield_awaiter final_suspend() noexcept {
            return {};
        }

        [[noreturn]] void unhandled_exception() noexcept {
            std::terminate();
        }

        int value;
        std::coroutine_handle<> continuation;
    };

    explicit task(std::coroutine_handle<promise_type> coro) noexcept
    : coro(coro)
    {}

    task(task&& t) noexcept
    : coro(std::exchange(t.coro, {}))
    {}

    ~task() {
        if (coro) coro.destroy();
    }

    __attribute__((noinline)) bool await_ready() {
        std::puts("task::await_ready");
        return false;
    }

    __attribute__((noinline)) auto await_suspend(std::coroutine_handle<> h) noexcept {
        std::puts("task::await_suspend");
        coro.promise().continuation = h;
        return coro;
    }

    __attribute__((noinline)) int await_resume() {
        std::puts("task::await_resume");
        return coro.promise().value;
    }

    std::coroutine_handle<promise_type> coro;
};

task two_values() {
    co_yield 1;
    co_return 2;
}

task example() {
    task t = two_values();
    int result = co_await t + co_await t;
    std::printf("result = %i (should be 3)\n", result);
    std::fflush(stdout);
    co_return result;
}

int main() {
    task t = example();
    t.coro.resume();
    assert(t.coro.done());
}
----

Expected output:
----
task initial_suspend
task initial_suspend
task::await_ready
task::await_suspend
task yielding/returning value
task::await_resume
task::await_ready
task::await_suspend
task resumed
task yielding/returning value
task::await_resume
result = 3 (should be 3)

Actual output:
----
task initial_suspend
task initial_suspend
task::await_ready
task::await_suspend
task yielding/returning value
task::await_ready
task::await_suspend
task resumed
task yielding/returning value
task::await_resume
task::await_resume
result = 4 (should be 3)


Note that the output indicates the the compiler is only calling await_resume()
on both awaiters immediately before evaluating the plus-expression rather than
evaluating await_resume() on the first awaiter immediately upon resuming from
the first suspend-point and before evaluating await_ready() for the second
awaiter.
Comment 1 Iain Sandoe 2020-10-16 07:04:33 UTC
probably a dup of 97433.
Comment 2 David Ledger 2020-10-17 03:15:40 UTC
I'm happy to use this thread for the issue, I can just repost my link to the same issue here.

My reporting of the issue is here, but Lewis Bakers example is seperate.

https://stackoverflow.com/questions/64348125/c20-coroutines-unexpected-reordering-of-await-resume-return-value-and-yield

Is there anything I can do to help this issue get resolved? I've been throwing things at the wall trying to get something to stick (for a work around) but so far nothing helps with this particular case.
Comment 3 Iain Sandoe 2020-10-17 07:30:10 UTC
(In reply to David Ledger from comment #2)
> I'm happy to use this thread for the issue, I can just repost my link to the
> same issue here.
> 
> My reporting of the issue is here, but Lewis Bakers example is seperate.
> 
> https://stackoverflow.com/questions/64348125/c20-coroutines-unexpected-
> reordering-of-await-resume-return-value-and-yield
> 
> Is there anything I can do to help this issue get resolved? I've been
> throwing things at the wall trying to get something to stick (for a work
> around) but so far nothing helps with this particular case.

I have reproduced the problem(s) - the next thing is to do some analysis of the generated code and try to figure out where the bug is.
Comment 4 David Ledger 2020-10-29 06:11:29 UTC
@Iain Sandoe
In terms of the standard do you think this is technically undefined behaviour?
I tried bring this up with Std-Proposals but got no response at all.

I believe either I'm very bad at writing (likely), or nobody knows whether or not this is undefined behaviour and therefore the conversation didn't start. If this is undefined behaviour I believe a defect report is in order.
Comment 5 Iain Sandoe 2020-10-29 07:52:53 UTC
(In reply to David Ledger from comment #4)
> @Iain Sandoe
> In terms of the standard do you think this is technically undefined
> behaviour?

no, AFAICT, it's just a regular bug in the implementation.
(it's just a question of finding resources to deal it).
Comment 6 David Ledger 2020-12-02 09:01:26 UTC
Is this the right place for me to track this bug?
Comment 7 Iain Sandoe 2020-12-02 09:02:54 UTC
(In reply to David Ledger from comment #6)
> Is this the right place for me to track this bug?

yes - it's just waiting for someone to have time to address it.
Comment 8 Richard Biener 2021-04-08 12:02:12 UTC
GCC 10.3 is being released, retargeting bugs to GCC 10.4.
Comment 9 Lewis Baker 2021-04-09 10:23:08 UTC
> In terms of the standard do you think this is technically undefined behaviour?

Yes, I think this is something that Gor was looking into as a wording issue that could do with some clarification.

I think the suggestion was something along the lines of adding some wording to ensure that the evaluation of a an await-expression was sequenced atomically with respect to the evaluation of other expressions in the statement.