Symmetric transfer does not prevent stack-overflow for C++20 coroutines

[Quuxplusone]

Bugzilla Link	PR50544
Status	CONFIRMED
Importance	P normal
Reported by	Leonard von Merzljak (leonard.von-merzljak@tum.de)
Reported on	2021-05-31 15:00:52 -0700
Last modified on	2021-06-03 06:45:18 -0700
Version	12.0
Hardware	PC Linux
CC	blitzrakete@gmail.com, erik.pilkington@gmail.com, llvm-bugs@lists.llvm.org, richard-llvm@metafoo.co.uk, yedeng.yd@linux.alibaba.com, yuanfang.chen@sony.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

Even though the following code uses symmetric transfer, it crashes due to a stack-overflow. Please note that the code below is a minimal example to reproduce the stack-overflow: e.g., if I include the definition of the destructor of type Type in the header file, then I don't get a stack-overflow. The crash can also be reproduced when using the task<> type of the cppcoro library.

// type.h
#pragma once

struct Type {
  ~Type();
};

// type.cc
#include "type.h"

Type::~Type() {}

// main.cc
#include <cstdint>
#include <exception>
#include <type_traits>
#include <utility>

#include "type.h"

#if __has_include(<coroutine>)  // when using g++
#include <coroutine>
namespace coro {
using std::coroutine_handle;
using std::noop_coroutine;
using std::suspend_always;
}  // namespace coro
#elif __has_include(<experimental/coroutine>)  // when using clang++
#include <experimental/coroutine>
namespace coro {
using std::experimental::coroutine_handle;
using std::experimental::noop_coroutine;
using std::experimental::suspend_always;
}  // namespace coro
#endif

template <typename T = void>
class Task {
 public:
  struct PromiseBase {
    friend struct final_awaitable;

    struct final_awaitable {
      bool await_ready() const noexcept { return false; }

      template <typename PROMISE>
      coro::coroutine_handle<> await_suspend(
          coro::coroutine_handle<PROMISE> coro) noexcept {
        if (coro.promise().m_continuation) {
          return coro.promise().m_continuation;
        } else {
          // The top-level task started from within main() does not have a
          // continuation. This will give control back to the main function.
          return coro::noop_coroutine();
        }
      }

      void await_resume() noexcept {}
    };

    coro::suspend_always initial_suspend() noexcept { return {}; }

    auto final_suspend() noexcept { return final_awaitable{}; }

    void unhandled_exception() noexcept { std::terminate(); }

    void set_continuation(coro::coroutine_handle<> continuation) noexcept {
      m_continuation = continuation;
    }

   private:
    coro::coroutine_handle<> m_continuation;
  };

  struct PromiseVoid : public PromiseBase {
    auto get_return_object() { return coroutine_handle_t::from_promise(*this); }

    void return_void() noexcept {}

    void result() {}
  };

  struct PromiseT : public PromiseBase {
    auto get_return_object() { return coroutine_handle_t::from_promise(*this); }

    void return_value(T&& v) { value = std::move(v); }

    T&& result() && { return std::move(value); }

    T value;
  };

  using promise_type =
      std::conditional_t<std::is_same_v<T, void>, PromiseVoid, PromiseT>;

  using coroutine_handle_t = coro::coroutine_handle<promise_type>;

  Task(coroutine_handle_t coroutine) : m_coroutine(coroutine) {}

  ~Task() {
    if (m_coroutine) {
      m_coroutine.destroy();
    }
  }

  void start() noexcept { m_coroutine.resume(); }

  auto operator co_await() const noexcept { return awaitable{m_coroutine}; }

 private:
  struct awaitable {
    coroutine_handle_t m_coroutine;

    awaitable(coroutine_handle_t coroutine) noexcept : m_coroutine(coroutine) {}

    bool await_ready() const noexcept { return false; }

    coro::coroutine_handle<> await_suspend(
        coro::coroutine_handle<> awaitingCoroutine) noexcept {
      m_coroutine.promise().set_continuation(awaitingCoroutine);
      return m_coroutine;
    }

    auto await_resume() { return std::move(m_coroutine.promise()).result(); }
  };
  coroutine_handle_t m_coroutine;
};

Task<Type> coro2() { co_return Type{}; }

Task<> coro1() { auto s = co_await coro2(); }

Task<> test() {
  for (std::uint64_t i = 0; i != 50000000; ++i) {
    co_await coro1();
  }
}

int main() {
  auto task = test();
  task.start();
}

I compile the code with clang++-12 main.cc type.cc -std=c++20 -stdlib=libc++ -O3 -fsanitize=address.

Here is the output:

$ ./a.out 

AddressSanitizer:DEADLYSIGNAL
=================================================================
==20846==ERROR: AddressSanitizer: stack-overflow on address 0x7ffc76b1aff8 (pc 0x0000004cb7ab bp 0x7ffc76b1b050 sp 0x7ffc76b1afa0 T0)
    #0 0x4cb7ab in coro1() (.resume) (/home/leonard/Desktop/hiwi/async_io_uring/stack_overflow/a.out+0x4cb7ab)
    #1 0x4cbe4a in test() (.resume) (/home/leonard/Desktop/hiwi/async_io_uring/stack_overflow/a.out+0x4cbe4a)
    #2 0x4cbe4a in test() (.resume) (/home/leonard/Desktop/hiwi/async_io_uring/stack_overflow/a.out+0x4cbe4a)
    #3 0x4cbe4a in test() (.resume) (/home/leonard/Desktop/hiwi/async_io_uring/stack_overflow/a.out+0x4cbe4a)
    #4 0x4cbe4a in test() (.resume) (/home/leonard/Desktop/hiwi/async_io_uring/stack_overflow/a.out+0x4cbe4a)
    #5 0x4cbe4a in test() (.resume) (/home/leonard/Desktop/hiwi/async_io_uring/stack_overflow/a.out+0x4cbe4a)
    #6 0x4cbe4a in test() (.resume) (/home/leonard/Desktop/hiwi/async_io_uring/stack_overflow/a.out+0x4cbe4a)
    #7 0x4cbe4a in test() (.resume) (/home/leonard/Desktop/hiwi/async_io_uring/stack_overflow/a.out+0x4cbe4a)
    #8 0x4cbe4a in test() (.resume) (/home/leonard/Desktop/hiwi/async_io_uring/stack_overflow/a.out+0x4cbe4a)
    #9 0x4cbe4a in test() (.resume) (/home/leonard/Desktop/hiwi/async_io_uring/stack_overflow/a.out+0x4cbe4a)
    #10 0x4cbe4a in test() (.resume) (/home/leonard/Desktop/hiwi/async_io_uring/stack_overflow/a.out+0x4cbe4a)
...

Interestingly, if I compile with -O0 the program does not trigger a stack-overflow and exits without any errors.

Further information about the problem and a workaround can be found here: https://stackoverflow.com/questions/67446478/symmetric-transfer-does-not-prevent-stack-overflow-for-c20-coroutines

[Quuxplusone]

If the program would crash with O3 and Ok with O0, it should be a compiler bug. I would try to fix it. Thanks for reporting.

[Quuxplusone]

By the way, symmetric transfer isn't defined in the language standard. In N4878, it says:

If the type of await-suspend is std::coroutine_handle<Z>, await-suspend.resume() is evaluated

It didn't say anything about symmetric transfer and the style of symmetric transfer could prevent stack overflow.

This may not be related to this issue : ).

[Quuxplusone]

(In reply to Chuanqi Xu from comment #1)
> If the program would crash with O3 and Ok with O0, it should be a compiler
> bug. I would try to fix it. Thanks for reporting.

Great. Thank you very much :)