From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd2e.google.com (mail-io1-xd2e.google.com [IPv6:2607:f8b0:4864:20::d2e]) by sourceware.org (Postfix) with ESMTPS id 8F92D3858D20 for ; Tue, 27 Feb 2024 18:37:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8F92D3858D20 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 8F92D3858D20 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::d2e ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709059061; cv=none; b=ijWblAzmkliZGSKf7xA1lgJPE4Xo4enOaq0WO5DzkLOSsorjdbVQnJ7OGkum7CKE8mUM7RiSoG89IWRxFpeJTwP+ueFlUTT0hjM4DKcHoZR56NO9vbzPGfavcnPMV3PGu/hpJYLbFWc5B0VSUkTW4peIFHWXghCrOCyqaovRUAE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709059061; c=relaxed/simple; bh=kXLOudroIhdFUyo1+9XcNzajcGhHti1FjRlJ0Cnus00=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=oflbTE2e304k+pKRbeXLkp0pEveui1gMS6plA/TvJTcqDZ1AqNz+pN9SzdshqmwWjvH0jWr/bc88e1NDO21qIX9bg9OjMgDzFYPCZ4DWXNpZ+ykpjo5Gc4iK4Bmax4By76Te0tLq4pZr/iXueMMP+eDYvWZvccU/Njscdr0mFls= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-io1-xd2e.google.com with SMTP id ca18e2360f4ac-7c78573f294so209885639f.0 for ; Tue, 27 Feb 2024 10:37:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore.com; s=google; t=1709059057; x=1709663857; darn=sourceware.org; h=mime-version:user-agent:message-id:in-reply-to:date:references :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=JhDeOneWqMBLnCnk8sQv0PlVxbGJF8QfAbvygKbGpPo=; b=j/daagonaDF2zTOpJ6ELJ6l1cGgtu63mCNdfagS2oXSaBlTWXufhiqMwe/YLnmXjIx 2ZRY6E5v+0b6QCa4rVGyCfpWt3FltbzuBTgiFHACOH1wzT8FPrOzeeRdMqAZnNGKfMAN HheonMJLr7UyjQbWCblaL5XhINZIBkhWF7P2KH1kR8OR62qjX39KKB8ywmIrPuKU0LKZ 3TFqk324jMujyEBAG0b+420xSU7iO1RC6qd+pUFngaO2DGcjFGgrjZUP+5yczG0a69CB g2Tc2QBwApx6ty0ozhXyBHq+I5TV7fnozj7UkRkjRQXA53wDbC8oRuaX++l9jrZMHUCC ys4g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709059057; x=1709663857; h=mime-version:user-agent:message-id:in-reply-to:date:references :subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=JhDeOneWqMBLnCnk8sQv0PlVxbGJF8QfAbvygKbGpPo=; b=ZsXkxCYr0G32qEO7p76IG8A2WtQUOzHBM7y3K+k1+mYaiHwRXKXCSA6XdoNvBbBnJf 3OgQe+aYtBEXROfAi7ZHkLUr+r/eB1SLkDfCaverEUhvz9JyNh/CCgNScBS09YshiPQu WI4SAJEPzgqs0kgHRTeFmcfWg2gHeFizW07qMrzN5i3A3uf6HJywYPmpSS4je52UvaTh VKIhP6aeLOfYh3/dfhioE3UDbGIDSvgEBpyELiYhnFFYkAR+xkVswnxHzrYOVUc1kD+2 Udgew28FJZGCu+8v5lSYz0jObXA3wh20mzd4tEw3/UJpCSHp7Bt4oWPJjavo3tqxy59z 8JkQ== X-Forwarded-Encrypted: i=1; AJvYcCXsysZWqLnP4mohTNTk2ctoEzKG0t8QCTG3T1CE/dz1pZrSMXEr0qnZU3A2BykfrWZyT8bpq/j0tKNlSVhw6aL4goCgROQ4xMHM0A== X-Gm-Message-State: AOJu0Yz6CUJruS0WoiJXXpsdjO6MTZz5AUjrvj0TguCNuFRyQbdl2Znw ROx3fNP2/QXZBHB8N5tn85rpEdB3NTkrPLHt6/SUEwhfQ0S5Mxk3H2NBdPA08cwAfwLU/Hz08OM = X-Google-Smtp-Source: AGHT+IHHA0T6HmJVM/RDymMRHLM3XkX9GP5Yt9KwNkq64rWGHd9zirRkRiJol20O8jtsTys9cyfLxA== X-Received: by 2002:a92:db4e:0:b0:365:69a:86b2 with SMTP id w14-20020a92db4e000000b00365069a86b2mr11865554ilq.17.1709059056938; Tue, 27 Feb 2024 10:37:36 -0800 (PST) Received: from murgatroyd (71-211-170-195.hlrn.qwest.net. [71.211.170.195]) by smtp.gmail.com with ESMTPSA id m2-20020a924a02000000b00363c8049f30sm115773ilf.50.2024.02.27.10.37.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 27 Feb 2024 10:37:36 -0800 (PST) From: Tom Tromey To: Tom de Vries Cc: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [PATCH] Fix gdb.interrupt race References: <20240223162031.3568167-1-tromey@adacore.com> X-Attribution: Tom Date: Tue, 27 Feb 2024 11:37:35 -0700 In-Reply-To: (Tom de Vries's message of "Tue, 27 Feb 2024 16:07:45 +0100") Message-ID: <871q8xvjm8.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,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: 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. 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