From 016d0f9e43c1d2bd8227751b5b20a309c94edc90 Mon Sep 17 00:00:00 2001 From: Bin Cheng Date: Mon, 9 Mar 2020 18:54:57 +0800 Subject: [PATCH] Insert default return_void at the end of coroutine body Exception in coroutine is not correctly handled because the default return_void call is now inserted before the finish suspend point, rather than at the end of the original coroutine body. This patch fixes the issue by expanding code as following: co_await promise.initial_suspend(); try { // The original coroutine body promise.return_void(); // The default return_void call. } catch (...) { promise.unhandled_exception(); } final_suspend: // ... gcc/cp/ * coroutines.cc (build_actor_fn): Factor out code inserting the default return_void call to... (morph_fn_to_coro): ...here, also hoist local var declarations. gcc/testsuite/ * g++.dg/coroutines/torture/co-ret-15-default-return_void.C: New. --- gcc/cp/ChangeLog | 6 ++ gcc/cp/coroutines.cc | 68 ++++++++++++---------- gcc/testsuite/ChangeLog | 4 ++ .../torture/co-ret-15-default-return_void.C | 55 +++++++++++++++++ 4 files changed, 102 insertions(+), 31 deletions(-) create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/co-ret-15-default-return_void.C diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index f32c27e..128880b 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,9 @@ +2020-03-09 Bin Cheng + + * coroutines.cc (build_actor_fn): Factor out code inserting the + default return_void call to... + (morph_fn_to_coro): ...here, also hoist local var declarations. + 2020-03-08 Patrick Palka PR c++/93729 diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index bca4f1e..920575b 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -2161,21 +2161,6 @@ build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody, r = coro_build_expr_stmt (initial_await, loc); add_stmt (r); - /* Now we've built the promise etc, process fnbody for co_returns. - We want the call to return_void () below and it has no params so - we can create it once here. - Calls to return_value () will have to be checked and created as - required. */ - - tree return_void = NULL_TREE; - tree rvm - = lookup_promise_method (orig, coro_return_void_identifier, loc, - /*musthave=*/false); - if (rvm && rvm != error_mark_node) - return_void - = build_new_method_call (ap, rvm, NULL, NULL_TREE, LOOKUP_NORMAL, NULL, - tf_warning_or_error); - /* co_return branches to the final_suspend label, so declare that now. */ tree fs_label = create_named_label_with_ctx (loc, "final.suspend", actor); @@ -2190,15 +2175,6 @@ build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody, /* Add in our function body with the co_returns rewritten to final form. */ add_stmt (fnbody); - /* [stmt.return.coroutine] (2.2 : 3) if p.return_void() is a valid - expression, flowing off the end of a coroutine is equivalent to - co_return; otherwise UB. - We just inject the call to p.return_void() here, and fall through to - the final_suspend: label (eliding the goto). If the function body has - a co_return, then this statement will be unreachable and DCEd. */ - if (return_void != NULL_TREE) - add_stmt (return_void); - /* Final suspend starts here. */ r = build_stmt (loc, LABEL_EXPR, fs_label); add_stmt (r); @@ -3815,18 +3791,48 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) BIND_EXPR_BLOCK (first) = replace_blk; } + /* actor's version of the promise. */ + tree actor_frame = build1_loc (fn_start, INDIRECT_REF, coro_frame_type, + DECL_ARGUMENTS (actor)); + tree ap_m = lookup_member (coro_frame_type, get_identifier ("__p"), 1, 0, + tf_warning_or_error); + tree ap = build_class_member_access_expr (actor_frame, ap_m, NULL_TREE, + false, tf_warning_or_error); + + /* Now we've built the promise etc, process fnbody for co_returns. + We want the call to return_void () below and it has no params so + we can create it once here. + Calls to return_value () will have to be checked and created as + required. */ + + tree return_void = NULL_TREE; + tree rvm + = lookup_promise_method (orig, coro_return_void_identifier, fn_start, + /*musthave=*/false); + if (rvm && rvm != error_mark_node) + return_void + = build_new_method_call (ap, rvm, NULL, NULL_TREE, LOOKUP_NORMAL, NULL, + tf_warning_or_error); + + /* [stmt.return.coroutine] (2.2 : 3) if p.return_void() is a valid + expression, flowing off the end of a coroutine is equivalent to + co_return; otherwise UB. + We just inject the call to p.return_void() here, and fall through to + the final_suspend: label (eliding the goto). If the function body has + a co_return, then this statement will be unreachable and DCEd. */ + if (return_void != NULL_TREE) + { + tree append = push_stmt_list (); + add_stmt (fnbody); + add_stmt (return_void); + fnbody = pop_stmt_list(append); + } + if (flag_exceptions) { tree ueh_meth = lookup_promise_method (orig, coro_unhandled_exception_identifier, fn_start, /*musthave=*/true); - /* actor's version of the promise. */ - tree actor_frame = build1_loc (fn_start, INDIRECT_REF, coro_frame_type, - DECL_ARGUMENTS (actor)); - tree ap_m = lookup_member (coro_frame_type, get_identifier ("__p"), 1, 0, - tf_warning_or_error); - tree ap = build_class_member_access_expr (actor_frame, ap_m, NULL_TREE, - false, tf_warning_or_error); /* Build promise.unhandled_exception(); */ tree ueh = build_new_method_call (ap, ueh_meth, NULL, NULL_TREE, LOOKUP_NORMAL, diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index b72b9cb..cf7243d 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2020-03-09 Bin Cheng + + * g++.dg/coroutines/torture/co-ret-15-default-return_void.C: New. + 2020-03-09 Kewen Lin PR testsuite/94019 diff --git a/gcc/testsuite/g++.dg/coroutines/torture/co-ret-15-default-return_void.C b/gcc/testsuite/g++.dg/coroutines/torture/co-ret-15-default-return_void.C new file mode 100644 index 0000000..e600fea --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/torture/co-ret-15-default-return_void.C @@ -0,0 +1,55 @@ +// { dg-do run } +// +// Check if default return_void is insert at correct position. +#include +#include "../coro.h" + +class resumable { +public: + class promise_type; + using coro_handle = std::coroutine_handle; + resumable(coro_handle handle) : handle_(handle) { assert(handle); } + resumable(resumable&) = delete; + resumable(resumable&&) = delete; + bool resume() { + if (!handle_.done()) + handle_.resume(); + return !handle_.done(); + } + int recent_val(); + ~resumable() { handle_.destroy(); } +private: + coro_handle handle_; +}; + +class resumable::promise_type { +public: + friend class resumable; + using coro_handle = std::coroutine_handle; + auto get_return_object() { return coro_handle::from_promise(*this); } + auto initial_suspend() { return std::suspend_always(); } + auto final_suspend() { return std::suspend_always(); } + void return_void() { value_ = -1; } + void unhandled_exception() {} +private: + int value_ = 0; +}; + +int resumable::recent_val() {return handle_.promise().value_;} + +resumable foo(int n){ + co_await std::suspend_always(); + throw 1; +} + +int bar (int n) { + resumable res = foo(n); + while(res.resume()); + return res.recent_val(); +} + +int main() { + int res = bar(3); + assert(res == 0); + return 0; +} -- 2.7.4