From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2a07:de40:b251:101:10:150:64:2]) by sourceware.org (Postfix) with ESMTPS id 743EC3858D37 for ; Wed, 28 Feb 2024 03:23:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 743EC3858D37 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 743EC3858D37 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a07:de40:b251:101:10:150:64:2 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709090631; cv=none; b=kfRpcMaq2VrZtbX4Q1oWCaQWDBYR0B/N3EgI9UrWfYFQlfecVdeO5sr3J9dwAC4ocHat6g3KX8QzwXRJNSOCZjkK0GzotO7Du4jsIfA9FYNOm2iBzwyzjHQ/mXvPc+M7+p5UziGElmGGK89V0c991NuBzInluZVe7DFI3NuBe7Y= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709090631; c=relaxed/simple; bh=GWmn0m/3NiieI4PczLvtGptsOedLVoh4JTnFB5KwP14=; h=DKIM-Signature:DKIM-Signature:DKIM-Signature:DKIM-Signature: Message-ID:Date:MIME-Version:Subject:To:From; b=f7ushIs5BFx5wov+yFgyvJ5VNVF2vbmQkuOd4l5PDd5UK80SOZLxtl5qAJlMMiSBQpiY3nE6YEZARBRbw2qHberL8dYv9AEeEx+ai1cKkO/fwFcB7TfwVAGAil/3Kdbvv7hsS29deJxC6vPsNvneYg+MzdlTy1Y4rAAXYcEkIyM= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [10.150.64.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 642DD1FDA4; Wed, 28 Feb 2024 03:23:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1709090628; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=+JzEfZeOAYhsCwCTy5DRh4uO6xRokGQbVl+Cr4r0dvA=; b=rNyPtGKLLa1DOJIrka752aLgskz9qJFZg0uRqnCO6CtTyx3cVdeXhUleWcPP5AIMX3SV0H syC8k/XZhhcxG5XstXCyk40QJjxrP2xmvOkvxfprB13AwwDKX+JfpY6pDIzWJLZBGan4gI vw7el1D6iVuq201Ey7N8XbueNPRHGmo= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1709090628; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=+JzEfZeOAYhsCwCTy5DRh4uO6xRokGQbVl+Cr4r0dvA=; b=HkIZmwAcTlqgWjOXRJkIUaYOWCiI5WM4JvZ0JNCQuZnJPLl1Rt16ppbY6Ji4ll2njcyY6V y7rRWiPxnh/vyvBQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1709090626; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=+JzEfZeOAYhsCwCTy5DRh4uO6xRokGQbVl+Cr4r0dvA=; b=nangn7AiO45ied6FijgOwAUlnYaW/S5OE1SNI1PP2LdJ2T/8SopLqw0a86cegiEoec+4u3 sftFvJztpjFdOoRcvSoF4zhV5dQRTpHFZhPndhH0dahIyeg62Qnyp5hyqaVeyZAWsXKSLN DiFaDMTTnQUQv7VCyB/xM8cQDdplc6E= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1709090626; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=+JzEfZeOAYhsCwCTy5DRh4uO6xRokGQbVl+Cr4r0dvA=; b=sc573dSmacw58Wx4rb7SXMTaY4k8Q5v59cGku+0PUcjBXMUcH/2pAbLezBp8gh2m6mOjtA eE+8wpK2yeZbVhCw== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 4D6B613A92; Wed, 28 Feb 2024 03:23:46 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id yIOTEUKn3mU3YgAAD6G6ig (envelope-from ); Wed, 28 Feb 2024 03:23:46 +0000 Message-ID: <9dcb4480-57a8-4d2d-8acf-01f6d19481bb@suse.de> Date: Wed, 28 Feb 2024 04:23:59 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] Fix gdb.interrupt race Content-Language: en-US To: Tom Tromey Cc: gdb-patches@sourceware.org References: <20240223162031.3568167-1-tromey@adacore.com> <871q8xvjm8.fsf@tromey.com> From: Tom de Vries In-Reply-To: <871q8xvjm8.fsf@tromey.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Authentication-Results: smtp-out2.suse.de; none X-Spam-Level: X-Spam-Score: -1.10 X-Spamd-Result: default: False [-1.10 / 50.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; XM_UA_NO_VERSION(0.01)[]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; BAYES_HAM(-3.00)[100.00%]; MIME_GOOD(-0.10)[text/plain]; NEURAL_SPAM_SHORT(2.99)[0.997]; NEURAL_HAM_LONG(-1.00)[-1.000]; RCVD_COUNT_THREE(0.00)[3]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; RCPT_COUNT_TWO(0.00)[2]; DBL_BLOCKED_OPENRESOLVER(0.00)[adacore.com:email,sourceware.org:url]; FUZZY_BLOCKED(0.00)[rspamd.com]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_TLS_ALL(0.00)[]; MID_RHS_MATCH_FROM(0.00)[] X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,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 List-Id: On 2/27/24 19:37, Tom Tromey wrote: > Tom> I vote disable. If somebody has such a platform and wants DAP, they > Tom> can submit a patch that enables it for them in a way that is safe. > > Easily done, see the appended. > > I think this is really only an issue for mingw with certain versions of > GCC -- it's fixed in FSF GCC now. > > Let me know what you think. > LGTM, just the "let this race" comment needs updating. Thanks, - Tom > Tom > > commit eb5e215bfb810b9c8771462562365b2fae18afe6 > Author: Tom Tromey > Date: Fri Feb 23 08:59:40 2024 -0700 > > Fix gdb.interrupt race > > gdb.interrupt was introduced to implement DAP request cancellation. > However, because it can be run from another thread, and because I > didn't look deeply enough at the implementation, it turns out to be > racy. > > The fix here is to lock accesses to certain globals in extension.c. > > Note that this won't work in the case where configure detects that the > C++ compiler doesn't provide thread support. This version of the > patch disables DAP entirely in this situation. > > Regression tested on x86-64 Fedora 38. I also ran gdb.dap/pause.exp > in a thread-sanitizer build tree to make sure the reported race is > gone. > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31263 > > diff --git a/gdb/extension.c b/gdb/extension.c > index 9f403500497..b420350926d 100644 > --- a/gdb/extension.c > +++ b/gdb/extension.c > @@ -646,6 +646,21 @@ breakpoint_ext_lang_cond_says_stop (struct breakpoint *b) > This requires cooperation with the extension languages so the support > is defined here. */ > > +#if CXX_STD_THREAD > + > +#include > + > +/* DAP needs a way to interrupt the main thread, so we added > + gdb.interrupt. However, as this can run from any thread, we need > + locking for the current extension language. If threading is not > + available, we simply let this race. > + > + This lock is held for accesses to quit_flag, active_ext_lang, and > + cooperative_sigint_handling_disabled. */ > +static std::recursive_mutex ext_lang_mutex; > + > +#endif /* CXX_STD_THREAD */ > + > /* This flag tracks quit requests when we haven't called out to an > extension language. it also holds quit requests when we transition to > an extension language that doesn't have cooperative SIGINT handling. */ > @@ -701,6 +716,10 @@ static bool cooperative_sigint_handling_disabled = false; > > scoped_disable_cooperative_sigint_handling::scoped_disable_cooperative_sigint_handling () > { > +#if CXX_STD_THREAD > + std::lock_guard guard (ext_lang_mutex); > +#endif /* CXX_STD_THREAD */ > + > /* Force the active extension language to the GDB scripting > language. This ensures that a previously saved SIGINT is moved > to the quit_flag global, as well as ensures that future SIGINTs > @@ -718,6 +737,10 @@ scoped_disable_cooperative_sigint_handling::scoped_disable_cooperative_sigint_ha > > scoped_disable_cooperative_sigint_handling::~scoped_disable_cooperative_sigint_handling () > { > +#if CXX_STD_THREAD > + std::lock_guard guard (ext_lang_mutex); > +#endif /* CXX_STD_THREAD */ > + > cooperative_sigint_handling_disabled = m_prev_cooperative_sigint_handling_disabled; > restore_active_ext_lang (m_prev_active_ext_lang_state); > } > @@ -756,6 +779,10 @@ scoped_disable_cooperative_sigint_handling::~scoped_disable_cooperative_sigint_h > struct active_ext_lang_state * > set_active_ext_lang (const struct extension_language_defn *now_active) > { > +#if CXX_STD_THREAD > + std::lock_guard guard (ext_lang_mutex); > +#endif /* CXX_STD_THREAD */ > + > #if GDB_SELF_TEST > if (selftests::hook_set_active_ext_lang) > selftests::hook_set_active_ext_lang (); > @@ -808,6 +835,10 @@ set_active_ext_lang (const struct extension_language_defn *now_active) > void > restore_active_ext_lang (struct active_ext_lang_state *previous) > { > +#if CXX_STD_THREAD > + std::lock_guard guard (ext_lang_mutex); > +#endif /* CXX_STD_THREAD */ > + > if (cooperative_sigint_handling_disabled) > { > /* See set_active_ext_lang. */ > @@ -844,6 +875,10 @@ restore_active_ext_lang (struct active_ext_lang_state *previous) > void > set_quit_flag (void) > { > +#if CXX_STD_THREAD > + std::lock_guard guard (ext_lang_mutex); > +#endif /* CXX_STD_THREAD */ > + > if (active_ext_lang->ops != NULL > && active_ext_lang->ops->set_quit_flag != NULL) > active_ext_lang->ops->set_quit_flag (active_ext_lang); > @@ -868,6 +903,10 @@ set_quit_flag (void) > int > check_quit_flag (void) > { > +#if CXX_STD_THREAD > + std::lock_guard guard (ext_lang_mutex); > +#endif /* CXX_STD_THREAD */ > + > int result = 0; > > for (const struct extension_language_defn *extlang : extension_languages) > diff --git a/gdb/python/py-dap.c b/gdb/python/py-dap.c > index 2034105c939..9a00130fe90 100644 > --- a/gdb/python/py-dap.c > +++ b/gdb/python/py-dap.c > @@ -92,10 +92,14 @@ call_dap_fn (const char *fn_name) > void > dap_interp::init (bool top_level) > { > +#if CXX_STD_THREAD > call_dap_fn ("run"); > > current_ui->input_fd = -1; > current_ui->m_input_interactive_p = false; > +#else > + error (_("GDB was compiled without threading, which DAP requires")); > +#endif > } > > void