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
This commit is contained in:
Tom Tromey 2024-02-23 08:59:40 -07:00
parent 4b72a278f4
commit 8bb8f83467
2 changed files with 43 additions and 0 deletions

View file

@ -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)

View file

@ -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