From patchwork Sat Sep 3 20:57:29 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Arsen_Arsenovi=C4=87?= X-Patchwork-Id: 951 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:ecc5:0:0:0:0:0 with SMTP id s5csp1320029wro; Sat, 3 Sep 2022 13:58:52 -0700 (PDT) X-Google-Smtp-Source: AA6agR603to66cdXGe1+/cOEawMDERggNde35xqksVzgLOBwrEuR70aXSTrirj3KSVYkHYkN/atl X-Received: by 2002:a05:6402:1445:b0:44d:844d:e76e with SMTP id d5-20020a056402144500b0044d844de76emr2476792edx.313.1662238732388; Sat, 03 Sep 2022 13:58:52 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1662238732; cv=none; d=google.com; s=arc-20160816; b=Bvo/3QctZjl8wdMTOhu8vbmziaDTk2bK/XoLxW10gyM0NTAUmy4tgkBXbA1v2hh4M/ pkijAWfztbt2qC/xAyW68pGNTUb4c8wO9MfCM0J7j+JsF8P7WT7UupSSG0w+/zby/YWh 5tfr9lSbyhTBbhULdFdgFCH84FhCNiWeJxWu0gkFx5D/NSRV5QOReLcG/8s+P/67cvZq mNg7sA4QpLvRiqaR1w+as9jqhoZEuowYQdvairQ0/RALnM1ut2Ogc1932pmp2SI8pspZ HY9EX/7z4sK0d9k/R5ga5QuC2Y9Y3UttnFyzMif5rwkiS9pc2mfo/Hi1UDP9qegdwe+1 XyKA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:reply-to:from:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence :content-transfer-encoding:mime-version:message-id:date:subject:to :dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=UIYjhHtBZnJE02tYN1DPhS4vDXKn94EU70HvJKPwhu0=; b=cnk6KuFMzG3URtGwwKXdjUjLw8zmuJpIveJytPNYtKpBbiltuzFlQdt6elR7uCZGZL iImWYON6fK5xs3XX6A/JvVr48gKhknP8HWSu7z95cd0sB2IBcyO2njmYJIg+10FoRK+L gWQ/sdwVoeeBIrbYG9Mwxeim0U+IyaT3yPkKZr8aGxo4XMOqN+wpo8/HAfoWbO/QAgm0 ZcfCS9+j/AMHTwIpVhgjftVMAQdeLVatEHL4j9/7EDsm493EB6f0eD6XGnGOX4eO0IgC uuaONZYK+ntLgFOuQHN/pNGVatGF0HVuNK0MRqVwhW1t14CnbV0sTPYoUhLQwLkPGBKW BV0w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b="Y/Wz1NDr"; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from sourceware.org (server2.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id nc20-20020a1709071c1400b0073d67d3313dsi5402472ejc.364.2022.09.03.13.58.52 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 03 Sep 2022 13:58:52 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b="Y/Wz1NDr"; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 66C49385829B for ; Sat, 3 Sep 2022 20:58:51 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 66C49385829B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1662238731; bh=UIYjhHtBZnJE02tYN1DPhS4vDXKn94EU70HvJKPwhu0=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=Y/Wz1NDrz8+rf/v/U9qTKO5YyR1QjDEW7eZhjQHemYO/juESC61MmW1wLZ4dGTGRE XAQu4pjYO8Rd66xBtD61JWq3AnlPPmsAYa6THuXxVvsf7cf3J8eINSwS6k36Y6PdaI H74NR0Mgx/5FZULhXIWqF4TAfK5dWG1B/t79GH/g= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mout-p-201.mailbox.org (mout-p-201.mailbox.org [IPv6:2001:67c:2050:0:465::201]) by sourceware.org (Postfix) with ESMTPS id 7FAC13858CDB for ; Sat, 3 Sep 2022 20:58:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7FAC13858CDB Received: from smtp2.mailbox.org (smtp2.mailbox.org [10.196.197.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-384) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-201.mailbox.org (Postfix) with ESMTPS id 4MKnDY3bbrz9sR6; Sat, 3 Sep 2022 22:58:05 +0200 (CEST) To: gcc-patches@gcc.gnu.org Subject: [PATCH] coroutines: Wrap awaiting if/swich in a BIND_EXPR, if needed [PR106188] Date: Sat, 3 Sep 2022 22:57:29 +0200 Message-Id: <20220903205728.944089-1-arsen@aarsen.me> MIME-Version: 1.0 X-Spam-Status: No, score=-10.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_INFOUSMEBIZ, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: =?utf-8?q?Arsen_Arsenovi=C4=87_via_Gcc-patches?= From: =?utf-8?q?Arsen_Arsenovi=C4=87?= Reply-To: =?utf-8?q?Arsen_Arsenovi=C4=87?= Cc: iain@sandoe.co.uk, nathan@acm.org Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1742983641437509239?= X-GMAIL-MSGID: =?utf-8?q?1742983641437509239?= In the edge case of a coroutine not containing any locals, the ifcd/swch temporaries would get added to the coroutine frame, corrupting its layout. We can prevent this by ensuring that in cases where the scope of the if/switch is the same as the coroutine frame one, we insert a wrapping BIND_EXPR and try again. PR c++/106188 - [coroutines] Incorrect frame layout after transforming conditional statement without top-level bind expression PR c++/106713 - [11/12/13 Regression] Coroutine regression in GCC 11.3.0: if (co_await ...) crashes with a jump to ud2 since r12-3529-g70ee703c479081ac gcc/cp/ChangeLog: PR c++/106188 PR c++/106713 * coroutines.cc (struct susp_frame_data): Add fn_body for informing the await_statement_walker of the coroutine body. (maybe_add_bind): New function. (await_statement_walker): Call maybe_add_bind when necessary. (morph_fn_to_coro): Pass fnbody into susp_frame_data. gcc/testsuite/ChangeLog: * g++.dg/coroutines/pr106188.C: New test. Signed-off-by: Arsen Arsenović --- gcc/cp/coroutines.cc | 45 ++++++++++++++++++++-- gcc/testsuite/g++.dg/coroutines/pr106188.C | 35 +++++++++++++++++ 2 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/coroutines/pr106188.C diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index edb3b706ddc..2e88fb99d7d 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -2586,6 +2586,7 @@ struct susp_frame_data bool captures_temporary; /* This expr captures temps by ref. */ bool needs_truth_if_exp; /* We must expand a truth_if expression. */ bool has_awaiter_init; /* We must handle initializing an awaiter. */ + tree *fn_body; /* Original function body */ }; /* If this is an await expression, then count it (both uniquely within the @@ -3326,6 +3327,33 @@ add_var_to_bind (tree& bind, tree var_type, return newvar; } +/* Helper to ensure we have at least one bind to add vars to. */ +static bool +maybe_add_bind(susp_frame_data *awpts, tree *stmt, location_t sloc) +{ + /* bind_stack is already asserted to be nonempty */ + tree &bind = awpts->bind_stack->last(); + gcc_checking_assert(TREE_CODE (*awpts->fn_body) == BIND_EXPR); + if (BIND_EXPR_VARS (bind) != BIND_EXPR_VARS (*awpts->fn_body)) + { + /* Alright, we have a unique (enough) scope, bail early. */ + return false; + } + + /* In this case, we'd be pushing variables to the start of the coroutine + * unless we add a new BIND_EXPR here. + */ + tree ifsw_bind = build3_loc (sloc, BIND_EXPR, void_type_node, + NULL, NULL, NULL); + BIND_EXPR_BODY (ifsw_bind) = *stmt; + *stmt = ifsw_bind; + /* We don't push this BIND_EXPR to the walkers bind stack, it will be handled + * by a call to cp_walk_tree, since we start the next walk off from this + * bind node. + */ + return true; +} + /* Helper to build and add if (!cond) break; */ static void @@ -3456,6 +3484,12 @@ await_statement_walker (tree *stmt, int *do_subtree, void *d) return NULL_TREE; /* Nothing special to do here. */ gcc_checking_assert (!awpts->bind_stack->is_empty()); + location_t sloc = EXPR_LOCATION (IF_COND (if_stmt)); + if (maybe_add_bind (awpts, stmt, sloc)) + { + *do_subtree = false; /* Done inside maybe_add_bind. */ + return cp_walk_tree (stmt, await_statement_walker, awpts, NULL); + } tree& bind_expr = awpts->bind_stack->last (); tree newvar = add_var_to_bind (bind_expr, boolean_type_node, "ifcd", awpts->cond_number++); @@ -3464,7 +3498,6 @@ await_statement_walker (tree *stmt, int *do_subtree, void *d) if (TREE_CODE (cond_inner) == CLEANUP_POINT_EXPR) cond_inner = TREE_OPERAND (cond_inner, 0); add_decl_expr (newvar); - location_t sloc = EXPR_LOCATION (IF_COND (if_stmt)); /* We want to initialize the new variable with the expression that contains the await(s) and potentially also needs to have truth_if expressions expanded. */ @@ -3640,6 +3673,12 @@ await_statement_walker (tree *stmt, int *do_subtree, void *d) return NULL_TREE; /* Nothing special to do here. */ gcc_checking_assert (!awpts->bind_stack->is_empty()); + location_t sloc = EXPR_LOCATION (SWITCH_STMT_COND (sw_stmt)); + if (maybe_add_bind (awpts, stmt, sloc)) + { + *do_subtree = false; /* Done inside maybe_add_bind. */ + return cp_walk_tree (stmt, await_statement_walker, awpts, NULL); + } /* Build a variable to hold the condition, this will be included in the frame as a local var. */ tree& bind_expr = awpts->bind_stack->last (); @@ -3652,7 +3691,6 @@ await_statement_walker (tree *stmt, int *do_subtree, void *d) tree cond_inner = SWITCH_STMT_COND (sw_stmt); if (TREE_CODE (cond_inner) == CLEANUP_POINT_EXPR) cond_inner = TREE_OPERAND (cond_inner, 0); - location_t sloc = EXPR_LOCATION (SWITCH_STMT_COND (sw_stmt)); tree new_s = build2_loc (sloc, INIT_EXPR, sw_type, newvar, cond_inner); finish_expr_stmt (new_s); @@ -4477,7 +4515,8 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) vars) they will get added to the coro frame along with other locals. */ susp_frame_data body_aw_points = {&field_list, handle_type, fs_label, NULL, NULL, 0, 0, - hash_set (), NULL, NULL, 0, false, false, false}; + hash_set (), NULL, NULL, 0, false, false, false, + &fnbody}; body_aw_points.block_stack = make_tree_vector (); body_aw_points.bind_stack = make_tree_vector (); body_aw_points.to_replace = make_tree_vector (); diff --git a/gcc/testsuite/g++.dg/coroutines/pr106188.C b/gcc/testsuite/g++.dg/coroutines/pr106188.C new file mode 100644 index 00000000000..1de3d4cceca --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr106188.C @@ -0,0 +1,35 @@ +// { dg-additional-options "-std=c++20 -w" } +// { dg-do run } +// test case from pr106188, w/o workaround +#include + +struct task { + struct promise_type { + task get_return_object() { return task{}; } + void return_void() {} + void unhandled_exception() {} + auto initial_suspend() noexcept { return std::suspend_never{}; } + auto final_suspend() noexcept { return std::suspend_never{}; } + }; +}; + +struct suspend_and_resume { + bool await_ready() const { return false; } + void await_suspend(std::coroutine_handle<> h) { h.resume(); } + void await_resume() {} +}; + +task f() { + if (co_await suspend_and_resume{}, false) {} +} + +task g() { + switch (co_await suspend_and_resume{}, 0) { + default: break; + } +} + +int main() { + f(); + g(); +}