From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp001-out.apm-internet.net (smtp001-out.apm-internet.net [85.119.248.222]) by sourceware.org (Postfix) with ESMTPS id 9453C3858C56 for ; Mon, 28 Mar 2022 06:57:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9453C3858C56 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=sandoe.co.uk Authentication-Results: sourceware.org; spf=none smtp.mailfrom=sandoe.co.uk Received: (qmail 90876 invoked from network); 28 Mar 2022 06:57:47 -0000 X-APM-Out-ID: 16484506669087 X-APM-Authkey: 257869/1(257869/1) 3 Received: from unknown (HELO ?192.168.1.214?) (81.138.1.83) by smtp001.apm-internet.net with SMTP; 28 Mar 2022 06:57:47 -0000 Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.21\)) Subject: Re: [PATCH RFC] Fix ICE due to shared BLOCK node in coroutine generation [PR103328] From: Iain Sandoe In-Reply-To: Date: Mon, 28 Mar 2022 07:57:46 +0100 Cc: GCC Patches Content-Transfer-Encoding: quoted-printable Message-Id: <4B530314-E417-418C-9F4F-0F12F3C51A42@sandoe.co.uk> References: To: Jason Merrill , Benno Evers X-Mailer: Apple Mail (2.3445.104.21) X-Spam-Status: No, score=-14.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_COUK, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, KAM_SHORT, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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-List-Received-Date: Mon, 28 Mar 2022 06:57:50 -0000 Hi Folks > On 27 Mar 2022, at 02:33, Jason Merrill wrote: >=20 > On 3/17/22 07:37, Benno Evers via Gcc-patches wrote: >> The coroutine transformation moves the original function body into a >> newly created actor function, but the block of the >> `current_binding_level` still points into the original function, >> causing the block to be shared between the two functions if it is >> subsequently used. This may cause havoc later on, as subsequent >> compiler passes for one function will also implicitly modify the >> other. See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D103328#c19 = for >> a more detailed writeup. >> This patch fixes the issue locally, but I'm not familiar with the GCC >> code base so I'm not sure if this is the right way to go about it or >> if there's a cleaner way to reset the current binding level. If this >> is the way to go I'm happy to extend the patch with a testcase and >> changelog entry. >=20 > Please do, it looks like a good fix to me. Iain, does it make sense = to you as well? Yes, LGTM, thanks for the patch, I have a testcase for this already locally (diff attatched) The patch needs a changelog entry and to reference the PR that=E2=80=99s = fixed. Benno, do you have write access? If not I can take care of this for you if you like? thanks Iain diff --git a/gcc/testsuite/g++.dg/coroutines/pr103328.C = b/gcc/testsuite/g++.dg/coroutines/pr103328.C new file mode 100644 index 00000000000..56fb54ab316 --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr103328.C @@ -0,0 +1,32 @@ +// { dg-additional-options "-g" } + +#include + +struct task { + struct promise_type { + task get_return_object() { return {}; } + std::suspend_never initial_suspend() { return {}; } + std::suspend_never final_suspend() noexcept { return {}; } + void unhandled_exception() {} + }; + bool await_ready() { return false; } + void await_suspend(std::coroutine_handle<> h) {} + void await_resume() {} +}; + +template +void call(Func func) { func(); } + +class foo { + void f(); + task g(); +}; + +void foo::f() { + auto lambda =3D [this]() noexcept -> task { + co_await g(); + }; + (void)call; +} + +int main() {} >=20 >> --- >> gcc/cp/coroutines.cc | 2 ++ >> 1 file changed, 2 insertions(+) >> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc >> index b1bfdc767a4..eb5f80f499b 100644 >> --- a/gcc/cp/coroutines.cc >> +++ b/gcc/cp/coroutines.cc >> @@ -4541,6 +4541,8 @@ morph_fn_to_coro (tree orig, tree *resumer, = tree >> *destroyer) >> BLOCK_VARS (top_block) =3D BIND_EXPR_VARS (ramp_bind); >> BLOCK_SUBBLOCKS (top_block) =3D NULL_TREE; >> + current_binding_level->blocks =3D top_block; >> + >> /* The decl_expr for the coro frame pointer, initialize to zero so = that we >> can pass it to the IFN_CO_FRAME (since there's no way to pass a = type, >> directly apparently). This avoids a "used uninitialized" = warning. */