From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1698) id E9F3C385700B; Mon, 15 Mar 2021 15:56:05 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E9F3C385700B MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Iain D Sandoe To: gcc-cvs@gcc.gnu.org Subject: [gcc r11-7676] coroutines : Handle rethrow from unhandled_exception [PR98704]. X-Act-Checkin: gcc X-Git-Author: Iain Sandoe X-Git-Refname: refs/heads/master X-Git-Oldrev: 26e0eb1071e318728bcd33f28d055729ac48792c X-Git-Newrev: 020b286c769f4dc8a6b45491351f6bc2e69d7a7f Message-Id: <20210315155605.E9F3C385700B@sourceware.org> Date: Mon, 15 Mar 2021 15:56:05 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Mar 2021 15:56:06 -0000 https://gcc.gnu.org/g:020b286c769f4dc8a6b45491351f6bc2e69d7a7f commit r11-7676-g020b286c769f4dc8a6b45491351f6bc2e69d7a7f Author: Iain Sandoe Date: Thu Mar 11 17:04:14 2021 +0000 coroutines : Handle rethrow from unhandled_exception [PR98704]. Although there is still some discussion in CWG 2451 on this, the implementors are agreed on the intent. When promise.unhandled_exception () is entered, the coroutine is considered to be still running - returning from the method will cause the final await expression to be evaluated. If the method throws, that action is considered to make the coroutine suspend (since, otherwise, it would be impossible to reclaim its resources, since one cannot destroy a running coro). The wording issue is to do with how to represent the place at which the coroutine should be considered suspended. For the implementation here, that place is immediately before the promise life-time ends. A handler for the rethrown exception, can thus call xxxx.destroy() which will run DTORs for the promise and any parameter copies [as needed] then the coroutine frame will be deallocated. At present, we also set "done=true" in this case (for compatibility with other current implementations). One might consider 'done()' to be misleading in the case of an abnormal termination - that is also part of the CWG 2451 discussion. gcc/cp/ChangeLog: PR c++/98704 * coroutines.cc (build_actor_fn): Make destroy index 1 correspond to the abnormal unhandled_exception() exit. Substitute the proxy for the resume index. (coro_rewrite_function_body): Arrange to reset the resume index and make done = true for a rethrown exception from unhandled_exception (). (morph_fn_to_coro): Adjust calls to build_actor_fn and coro_rewrite_function_body. gcc/testsuite/ChangeLog: PR c++/98704 * g++.dg/coroutines/torture/pr98704.C: New test. Diff: --- gcc/cp/coroutines.cc | 75 +++++++++++++------ gcc/testsuite/g++.dg/coroutines/torture/pr98704.C | 91 +++++++++++++++++++++++ 2 files changed, 145 insertions(+), 21 deletions(-) diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index 438538a0b4f..ea714da146b 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -2145,7 +2145,7 @@ build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody, tree orig, hash_map *param_uses, hash_map *local_var_uses, vec *param_dtor_list, tree resume_fn_field, - unsigned body_count, tree frame_size) + tree resume_idx_field, unsigned body_count, tree frame_size) { verify_stmt_tree (fnbody); /* Some things we inherit from the original function. */ @@ -2267,6 +2267,17 @@ build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody, b = coro_build_cvt_void_expr_stmt (b, loc); add_stmt (b); + /* The destroy point numbered #1 is special, in that it is reached from a + coroutine that is suspended after re-throwing from unhandled_exception(). + This label just invokes the cleanup of promise, param copies and the + frame itself. */ + tree del_promise_label + = create_named_label_with_ctx (loc, "coro.delete.promise", actor); + b = build_case_label (build_int_cst (short_unsigned_type_node, 1), NULL_TREE, + create_anon_label_with_ctx (loc, actor)); + add_stmt (b); + add_stmt (build_stmt (loc, GOTO_EXPR, del_promise_label)); + short unsigned lab_num = 3; for (unsigned destr_pt = 0; destr_pt < body_count; destr_pt++) { @@ -2371,9 +2382,10 @@ build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody, p_data.to = ap; cp_walk_tree (&fnbody, replace_proxy, &p_data, NULL); - /* Set the actor pointer to null, so that 'done' will work. - Resume from here is UB anyway - although a 'ready' await will - branch to the final resume, and fall through to the destroy. */ + /* The rewrite of the function adds code to set the __resume field to + nullptr when the coroutine is done and also the index to zero when + calling an unhandled exception. These are represented by two proxies + in the function, so rewrite them to the proper frame access. */ tree resume_m = lookup_member (coro_frame_type, get_identifier ("__resume"), /*protect=*/1, /*want_type=*/0, tf_warning_or_error); @@ -2383,12 +2395,14 @@ build_actor_fn (location_t loc, tree coro_frame_type, tree actor, tree fnbody, p_data.to = res_x; cp_walk_tree (&fnbody, replace_proxy, &p_data, NULL); + p_data.from = resume_idx_field; + p_data.to = rat; + cp_walk_tree (&fnbody, replace_proxy, &p_data, NULL); + /* Add in our function body with the co_returns rewritten to final form. */ add_stmt (fnbody); /* now do the tail of the function. */ - tree del_promise_label - = create_named_label_with_ctx (loc, "coro.delete.promise", actor); r = build_stmt (loc, LABEL_EXPR, del_promise_label); add_stmt (r); @@ -4022,9 +4036,9 @@ act_des_fn (tree orig, tree fn_type, tree coro_frame_ptr, const char* name) /* Re-write the body as per [dcl.fct.def.coroutine] / 5. */ static tree -coro_rewrite_function_body (location_t fn_start, tree fnbody, - tree orig, tree resume_fn_ptr_type, - tree& resume_fn_field, tree& fs_label) +coro_rewrite_function_body (location_t fn_start, tree fnbody, tree orig, + tree resume_fn_ptr_type, tree& resume_fn_field, + tree& resume_idx_field, tree& fs_label) { /* This will be our new outer scope. */ tree update_body = build3 (BIND_EXPR, void_type_node, NULL, NULL, NULL); @@ -4068,6 +4082,25 @@ coro_rewrite_function_body (location_t fn_start, tree fnbody, tree return_void = get_coroutine_return_void_expr (current_function_decl, fn_start, false); + /* We will need to be able to set the resume function pointer to nullptr + to signal that the coroutine is 'done'. */ + resume_fn_field + = build_lang_decl (VAR_DECL, get_identifier ("resume.fn.ptr.proxy"), + resume_fn_ptr_type); + DECL_ARTIFICIAL (resume_fn_field) = true; + tree zero_resume + = build1 (CONVERT_EXPR, resume_fn_ptr_type, integer_zero_node); + zero_resume + = build2 (INIT_EXPR, resume_fn_ptr_type, resume_fn_field, zero_resume); + /* Likewise, the resume index needs to be reset. */ + resume_idx_field + = build_lang_decl (VAR_DECL, get_identifier ("resume.index.proxy"), + short_unsigned_type_node); + DECL_ARTIFICIAL (resume_idx_field) = true; + tree zero_resume_idx = build_int_cst (short_unsigned_type_node, 0); + zero_resume_idx = build2 (INIT_EXPR, short_unsigned_type_node, + resume_idx_field, zero_resume_idx); + if (flag_exceptions) { /* Build promise.unhandled_exception(); */ @@ -4126,7 +4159,13 @@ coro_rewrite_function_body (location_t fn_start, tree fnbody, IF_SCOPE (not_iarc_if) = NULL; not_iarc_if = do_poplevel (iarc_scope); add_stmt (not_iarc_if); - /* ... else call the promise unhandled exception method. */ + /* ... else call the promise unhandled exception method + but first we set done = true and the resume index to 0. + If the unhandled exception method returns, then we continue + to the final await expression (which duplicates the clearing of + the field). */ + finish_expr_stmt (zero_resume); + finish_expr_stmt (zero_resume_idx); ueh = maybe_cleanup_point_expr_void (ueh); add_stmt (ueh); finish_handler (handler); @@ -4163,14 +4202,6 @@ coro_rewrite_function_body (location_t fn_start, tree fnbody, /* Before entering the final suspend point, we signal that this point has been reached by setting the resume function pointer to zero (this is what the 'done()' builtin tests) as per the current ABI. */ - resume_fn_field - = build_lang_decl (VAR_DECL, get_identifier ("resume.fn.ptr.proxy"), - resume_fn_ptr_type); - DECL_ARTIFICIAL (resume_fn_field) = true; - tree zero_resume - = build1 (CONVERT_EXPR, resume_fn_ptr_type, integer_zero_node); - zero_resume - = build2 (INIT_EXPR, resume_fn_ptr_type, resume_fn_field, zero_resume); finish_expr_stmt (zero_resume); finish_expr_stmt (build_init_or_final_await (fn_start, true)); BIND_EXPR_BODY (update_body) = pop_stmt_list (BIND_EXPR_BODY (update_body)); @@ -4314,9 +4345,11 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) the requirements for the coroutine frame. */ tree resume_fn_field = NULL_TREE; + tree resume_idx_field = NULL_TREE; tree fs_label = NULL_TREE; - fnbody = coro_rewrite_function_body (fn_start, fnbody, orig, act_des_fn_ptr, - resume_fn_field, fs_label); + fnbody = coro_rewrite_function_body (fn_start, fnbody, orig, + act_des_fn_ptr, resume_fn_field, + resume_idx_field, fs_label); /* Build our dummy coro frame layout. */ coro_frame_type = begin_class_definition (coro_frame_type); @@ -5198,7 +5231,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) /* Build the actor... */ build_actor_fn (fn_start, coro_frame_type, actor, fnbody, orig, param_uses, &local_var_uses, param_dtor_list, resume_fn_field, - body_aw_points.await_number, frame_size); + resume_idx_field, body_aw_points.await_number, frame_size); /* Destroyer ... */ build_destroy_fn (fn_start, coro_frame_type, destroy, actor); diff --git a/gcc/testsuite/g++.dg/coroutines/torture/pr98704.C b/gcc/testsuite/g++.dg/coroutines/torture/pr98704.C new file mode 100644 index 00000000000..15db250b4a2 --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/torture/pr98704.C @@ -0,0 +1,91 @@ +// { dg-do run } +#include "../coro.h" + +#include + +int frame_live = 0; +int promise_live = 0; +int task_live = 0; + +struct Task +{ + struct promise_type; + using handle = std::coroutine_handle; + + struct promise_type + { + promise_type () { promise_live++; PRINT ("promise_type ()"); } + ~promise_type () { promise_live--; PRINT ("~promise_type ()"); } + void* operator new(size_t sz) { + PRINT("operator new()"); + frame_live++; + return ::operator new(sz); + } + void operator delete(void* p, size_t sz) { + PRINT("operator delete"); + frame_live--; + return ::operator delete(p, sz); + } + + Task get_return_object() { return handle::from_promise(*this); } + auto initial_suspend() noexcept { return std::suspend_always{}; } + auto final_suspend() noexcept { return std::suspend_always{}; } + void return_void() noexcept {} + + auto yield_value(int x) noexcept + { + PRINTF ("yield_value(%d)\n", x); + return std::suspend_always{}; + } + + void unhandled_exception() + { + PRINT ("unhandled_exception()"); + throw; + } + }; + + Task(handle h) : coro(h) { task_live++; PRINT ("Task(handle h)"); } + ~Task() { task_live--; PRINT ("~Task()"); if (coro) coro.destroy(); } + + handle coro; +}; + +Task myco() +{ + co_yield 42; + throw std::out_of_range("TEST EXCEPTION"); +} + +int main() +{ + { + Task task = myco(); + PRINT ("START"); + try { + PRINTF ("done #0 = %d\n", task.coro.done()); + if (task.coro.done()) + abort(); + task.coro.resume(); // will yield 42 + PRINTF ("done #1 = %d\n", task.coro.done()); + if (task.coro.done()) + abort(); + task.coro.resume(); // will throw exception + PRINT ("should not be reached"); + abort (); + } + catch (const std::exception&) { + PRINTF ("done exc = %d\n", task.coro.done()); + if (!task.coro.done()) + abort(); + } + if (!task.coro.done()) + abort(); + } // should cause cause the destroy () to run. + if (task_live || promise_live || frame_live) + { + PRINTF ("task_live = %d, promise_live = %d, frame_live = %d\n", + task_live, promise_live, frame_live); + abort (); + } +}