public inbox for gdb-cvs@sourceware.org help / color / mirror / Atom feed
From: Tom Tromey <tromey@sourceware.org> To: gdb-cvs@sourceware.org Subject: [binutils-gdb] Fix gdb.interrupt race Date: Wed, 28 Feb 2024 16:13:26 +0000 (GMT) [thread overview] Message-ID: <20240228161326.0C3713858D39@sourceware.org> (raw) https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=8bb8f8346729c35433c961f7f1ed3a801776a362 commit 8bb8f8346729c35433c961f7f1ed3a801776a362 Author: Tom Tromey <tromey@adacore.com> 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: --- gdb/extension.c | 39 +++++++++++++++++++++++++++++++++++++++ gdb/python/py-dap.c | 4 ++++ 2 files changed, 43 insertions(+) diff --git a/gdb/extension.c b/gdb/extension.c index 9f403500497..9db8b53a087 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 <mutex> + +/* 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, DAP will not start. + + 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
reply other threads:[~2024-02-28 16:13 UTC|newest] Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20240228161326.0C3713858D39@sourceware.org \ --to=tromey@sourceware.org \ --cc=gdb-cvs@sourceware.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).