Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

C++20 Coroutines, Unexpected reordering of await_resume, return_value and yield_value

Tags:

Background

I have a task type that can both co_return and co_yield. In LLVM the task works as expected and passes some early tests. In MSVC and GCC the code fails in the same manner (coincidence?).


Brief Problem

With the following test function:

Task<int> test_yielding()
{
    co_yield 1;
    co_return 2;
}

There are two values retrieved from a Task object.

auto a = co_await fn;
auto b = co_await fn;

The value of a is expected to be 1, the value of b is expected to be 2.

The result is tested against a + b == 3.

The above test passes, however the following test fails:

auto res = co_await fn + co_await fn

The value of res for GCC and MSVC is 4. Both retrieved from the final co_return. As I understand it the first and second calls of co_await fn should be 1 and 2 in either order.

In MSVC and GCC the code fails as they seems to reorder await_resume, return_value and yield_value.


Details

I have run the code through clang tidy, PVS studio, enabled all the available sanitisers in LLVM, GCC, MSVC and nothing relevant pops up (just comments about destroy and resume not being noexcept).

I have several very similar tests: The relevant tests are:

Function:

Task<int> test_yielding()
{
    co_yield 1;
    co_return 2;
}

Test 1 (PASS):

Title("Test co_yield + co_return lvalue");
auto fn = test_yielding();
auto a = co_await fn;
auto b = co_await fn;
ASSERT(a + b == 3);

Test 2 (FAIL):

Title("Test co_yield + co_return rvalue");
auto fn = test_yielding();
auto res =
(
    co_await fn +
    co_await fn
);
ASSERT(res == 3);

The result of the test MSVC 1 (PASS):

---------------------------------
Title   Test co_yield + co_return lvalue
---------------------------------
        get_return_object: 02F01DA0
        initial_suspend: 02F01DA0
        await_transform: 02D03C80
        AwaitAwaitable: await_ready: 02F01DA0
        AwaitAwaitable: await_suspend: 02F01DA0
        SetCurrent: 02F01DA0
        ContinueWith: 02F01DA0
        yield_value: 02F01DA0
        SetValue: 02F01DA0
        YieldAwaitable: await_ready: 02F01DA0
        YieldAwaitable: await_suspend: 02F01DA0
        ContinueWith: 02F01DA0
        AwaitAwaitable: await_resume: 02F01DA0
        GetValue: 02F01DA0
        await_transform: 02D03C80
        AwaitAwaitable: await_ready: 02F01DA0
        AwaitAwaitable: await_suspend: 02F01DA0
        SetCurrent: 02F01DA0
        ContinueWith: 02F01DA0
        YieldAwaitable: await_resume: 02F01DA0
        return_value: 02F01DA0
        SetValue: 02F01DA0
        final_suspend: 02F01DA0
        YieldAwaitable: await_ready: 02F01DA0
        YieldAwaitable: await_suspend: 02F01DA0
        ContinueWith: 02F01DA0
        AwaitAwaitable: await_resume: 02F01DA0
        GetValue: 02F01DA0
PASS    test_task:323 a + b == 3
        [ result = 3, expected = 3 ]
        Destroy: 02F01DA0

The result of the test MSVC 2 (FAIL):

---------------------------------
Title   Test co_yield + co_return rvalue
---------------------------------
        get_return_object: 02F01CA0
        initial_suspend: 02F01CA0
        await_transform: 02D03C80
        AwaitAwaitable: await_ready: 02F01CA0
        AwaitAwaitable: await_suspend: 02F01CA0
        SetCurrent: 02F01CA0
        ContinueWith: 02F01CA0
        yield_value: 02F01CA0
        SetValue: 02F01CA0
        YieldAwaitable: await_ready: 02F01CA0
        YieldAwaitable: await_suspend: 02F01CA0
        ContinueWith: 02F01CA0
        await_transform: 02D03C80
        AwaitAwaitable: await_ready: 02F01CA0
        AwaitAwaitable: await_suspend: 02F01CA0
        SetCurrent: 02F01CA0
        ContinueWith: 02F01CA0
        YieldAwaitable: await_resume: 02F01CA0
        return_value: 02F01CA0
        SetValue: 02F01CA0
        final_suspend: 02F01CA0
        YieldAwaitable: await_ready: 02F01CA0
        YieldAwaitable: await_suspend: 02F01CA0
        ContinueWith: 02F01CA0
        AwaitAwaitable: await_resume: 02F01CA0
        GetValue: 02F01CA0
        AwaitAwaitable: await_resume: 02F01CA0
        GetValue: 02F01CA0
FAIL    test_task:342 res == 3
        [ result = 4, expected = 3 ]
        Destroy: 02F01CA0

If you look at the diff between working MSVC FAIL and MSVC PASS (with addresses corrected, the following appears): enter image description here Which makes it clear that the following lines were reordered:

        AwaitAwaitable: await_resume: 02901E20  
        GetValue: 02901E20

The source and results for LLVM and GCC are here.

Looking at the test 2 diff between the GCC FAIL and LLVM PASS: GCC vs LLVM A very similar reording is occuring in GCC.

The highlighted lines in the diff are produced be the following source:

template <typename Promise>
struct AwaitAwaitable
{
    Promise & m_promise;

    bool await_ready() const noexcept
    {
        WriteLine("AwaitAwaitable: ", __func__, ": ", &m_promise);
        return false;
    }

    void await_suspend(default_handle handle) noexcept
    {
        WriteLine("AwaitAwaitable: ", __func__, ": ", &m_promise);
        m_promise.SetCurrent( m_promise.Handle() );
        m_promise.ContinueWith( handle );
    }

    auto await_resume() const noexcept
    {
        WriteLine("AwaitAwaitable: ", __func__, ": ", &m_promise);
        return m_promise.GetValue();
    }
};

Does anybody know what is going on here, is this a compiler/library/user error?

like image 554
David Ledger Avatar asked Oct 14 '20 06:10

David Ledger


1 Answers

The observed behaviour appears to be due to similar bugs in both GCC and MSVC in their handling of the addition-operator where the arguments are both co_await expressions.

  • GCC Bug Report
  • MSVC Bug Report

In this instance, both GCC and MSVC seem to be incorrectly sequencing the call to await_resume() for both co_await expressions after resumption from the second suspend-point (ie. just before the addition is performed).

They should instead be sequencing the call to await_resume() for the first co_await expression (it is indeterminate which one) immediately after resuming from the first suspend-point and before beginning to evaluate the second co_await expression.

like image 89
Lewis Baker Avatar answered Sep 18 '22 13:09

Lewis Baker