From: Pedro Alves <pedro@palves.net>
To: gdb-patches@sourceware.org
Subject: [PATCH v2 21/29] Don't resume new threads if scheduler-locking is in effect
Date: Wed, 13 Jul 2022 23:24:25 +0100 [thread overview]
Message-ID: <20220713222433.374898-22-pedro@palves.net> (raw)
In-Reply-To: <20220713222433.374898-1-pedro@palves.net>
If scheduler-locking is in effect, like e.g., with "set
scheduler-locking on", and you step over a function that spawns a new
thread, the new thread is allowed to run free, at least until some
event is hit, at which point, whether the new thread is re-resumed
depends on a number of seemingly random factors. E.g., if the target
is all-stop, and the parent thread hits a breakpoint, and gdb decides
the breakpoint isn't interesting to report to the user, then the
parent thread is resumed, but the new thread is left stopped.
I think that letting the new threads run with scheduler-locking
enabled is a defect. This commit fixes that, making use of the new
clone events on Linux, and of target_thread_events() on targets where
new threads have no connection to the thread that spawned them.
Testcase and documentation changes included.
Change-Id: Ie12140138b37534b7fc1d904da34f0f174aa11ce
---
gdb/NEWS | 7 +++
gdb/doc/gdb.texinfo | 4 +-
gdb/infrun.c | 41 +++++++++---
.../gdb.threads/schedlock-new-thread.c | 46 ++++++++++++++
.../gdb.threads/schedlock-new-thread.exp | 63 +++++++++++++++++++
5 files changed, 152 insertions(+), 9 deletions(-)
create mode 100644 gdb/testsuite/gdb.threads/schedlock-new-thread.c
create mode 100644 gdb/testsuite/gdb.threads/schedlock-new-thread.exp
diff --git a/gdb/NEWS b/gdb/NEWS
index 742f4fe47fb..f0f6990dc59 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,13 @@
*** Changes since GDB 12
+* Scheduler-locking and new threads
+
+ When scheduler-locking is in effect, only the current thread may run
+ when the inferior is resumed. However, previously, new threads
+ created by the resumed thread would still be able to run free. Now,
+ they are held stopped.
+
* "info breakpoints" now displays enabled breakpoint locations of
disabled breakpoints as in the "y-" state. For example:
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9a8b010476a..a15ef0ed0db 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -6947,7 +6947,9 @@ the following:
There is no locking and any thread may run at any time.
@item on
-Only the current thread may run when the inferior is resumed.
+Only the current thread may run when the inferior is resumed. New
+threads created by the resumed thread are held stopped at their entry
+point, before they execute any instruction.
@item step
Behaves like @code{on} when stepping, and @code{off} otherwise.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 50270ec4759..6df70e374b4 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -103,6 +103,8 @@ static bool start_step_over (void);
static bool step_over_info_valid_p (void);
+static bool schedlock_applies (struct thread_info *tp);
+
/* Asynchronous signal handler registered as event loop source for
when we have pending events ready to be passed to the core. */
static struct async_event_handler *infrun_async_inferior_event_token;
@@ -1823,7 +1825,13 @@ static void
update_thread_events_after_step_over (thread_info *event_thread,
const target_waitstatus &event_status)
{
- if (target_supports_set_thread_options (0))
+ if (schedlock_applies (event_thread))
+ {
+ /* If scheduler-locking applies, continue reporting
+ thread-created/thread-cloned events. */
+ return;
+ }
+ else if (target_supports_set_thread_options (0))
{
/* We can control per-thread options. Disable events for the
event thread, unless the thread is gone. */
@@ -2395,9 +2403,14 @@ do_target_resume (ptid_t resume_ptid, bool step, enum gdb_signal sig)
to start stopped. We need to release the displaced stepping
buffer if the stepped thread exits, so we also enable
thread-exit events.
+
+ - If scheduler-locking applies, threads that the current thread
+ spawns should remain halted. It's not strictly necessary to
+ enable thread-exit events in this case, but it doesn't hurt.
*/
if (step_over_info_valid_p ()
- || displaced_step_in_progress_thread (tp))
+ || displaced_step_in_progress_thread (tp)
+ || schedlock_applies (tp))
{
gdb_thread_options options = GDB_TO_CLONE | GDB_TO_EXIT;
if (target_supports_set_thread_options (options))
@@ -2405,6 +2418,13 @@ do_target_resume (ptid_t resume_ptid, bool step, enum gdb_signal sig)
else
target_thread_events (true);
}
+ else
+ {
+ if (target_supports_set_thread_options (0))
+ tp->set_thread_options (0);
+ else if (!displaced_step_in_progress_any_thread ())
+ target_thread_events (false);
+ }
/* If we're resuming more than one thread simultaneously, then any
thread other than the leader is being set to run free. Clear any
@@ -6007,16 +6027,21 @@ handle_inferior_event (struct execution_control_state *ecs)
parent->set_running (false);
/* If resuming the child, mark it running. */
- if (ecs->ws.kind () == TARGET_WAITKIND_THREAD_CLONED
- || (follow_child || (!detach_fork && (non_stop || sched_multi))))
+ if ((ecs->ws.kind () == TARGET_WAITKIND_THREAD_CLONED
+ && !schedlock_applies (ecs->event_thread))
+ || (ecs->ws.kind () != TARGET_WAITKIND_THREAD_CLONED
+ && (follow_child
+ || (!detach_fork && (non_stop || sched_multi)))))
child->set_running (true);
/* In non-stop mode, also resume the other branch. */
if ((ecs->ws.kind () == TARGET_WAITKIND_THREAD_CLONED
- && target_is_non_stop_p ())
- || (!detach_fork && (non_stop
- || (sched_multi
- && target_is_non_stop_p ()))))
+ && target_is_non_stop_p ()
+ && !schedlock_applies (ecs->event_thread))
+ || (ecs->ws.kind () != TARGET_WAITKIND_THREAD_CLONED
+ && (!detach_fork && (non_stop
+ || (sched_multi
+ && target_is_non_stop_p ())))))
{
if (follow_child)
switch_to_thread (parent);
diff --git a/gdb/testsuite/gdb.threads/schedlock-new-thread.c b/gdb/testsuite/gdb.threads/schedlock-new-thread.c
new file mode 100644
index 00000000000..67a2ef61a7b
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/schedlock-new-thread.c
@@ -0,0 +1,46 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2021-2022 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include <pthread.h>
+#include <assert.h>
+#include <unistd.h>
+
+static void *
+thread_func (void *arg)
+{
+ while (1)
+ sleep (1);
+}
+
+int
+main (void)
+{
+ pthread_t thread;
+ int ret;
+
+ ret = pthread_create (&thread, NULL, thread_func, NULL); /* set break 1 here */
+ assert (ret == 0);
+
+ /* When testing with schedlock enabled, the new thread won't run, so
+ we can't join it, as that would hang forever. Instead, sleep for
+ a bit, enough that if the spawned thread is scheduled, it hits
+ the thread_func breakpoint before the main thread reaches the
+ "return 0" line below. */
+ sleep (3);
+
+ return 0; /* set break 2 here */
+}
diff --git a/gdb/testsuite/gdb.threads/schedlock-new-thread.exp b/gdb/testsuite/gdb.threads/schedlock-new-thread.exp
new file mode 100644
index 00000000000..8952cb7531c
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/schedlock-new-thread.exp
@@ -0,0 +1,63 @@
+# Copyright 2021-2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test continuing over a thread spawn with scheduler-locking on.
+
+standard_testfile .c
+
+set syscalls_src $srcdir/lib/my-syscalls.S
+
+if { [build_executable "failed to prepare" $testfile \
+ [list $srcfile $syscalls_src] {debug pthreads}] == -1 } {
+ return
+}
+
+proc test {non-stop schedlock} {
+ save_vars ::GDBFLAGS {
+ append ::GDBFLAGS " -ex \"set non-stop ${non-stop}\""
+ clean_restart $::binfile
+ }
+
+ set linenum1 [gdb_get_line_number "set break 1 here"]
+
+ if { ![runto $::srcfile:$linenum1] } {
+ return
+ }
+
+ delete_breakpoints
+
+ set linenum2 [gdb_get_line_number "set break 2 here"]
+ gdb_breakpoint $linenum2
+
+ gdb_breakpoint "thread_func"
+
+ gdb_test_no_output "set scheduler-locking $schedlock"
+
+ if {$schedlock} {
+ gdb_test "continue" \
+ "return 0.*set break 2 here .*" \
+ "continue does not stop in new thread"
+ } else {
+ gdb_test "continue" \
+ "thread_func .*" \
+ "continue stops in new thread"
+ }
+}
+
+foreach_with_prefix non-stop {off on} {
+ foreach_with_prefix schedlock {off on} {
+ test ${non-stop} ${schedlock}
+ }
+}
--
2.36.0
next prev parent reply other threads:[~2022-07-13 22:25 UTC|newest]
Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-13 22:24 [PATCH v2 00/29] Step over thread clone and thread exit Pedro Alves
2022-07-13 22:24 ` [PATCH v2 01/29] displaced step: pass down target_waitstatus instead of gdb_signal Pedro Alves
2022-07-20 20:21 ` Simon Marchi
2022-07-13 22:24 ` [PATCH v2 02/29] linux-nat: introduce pending_status_str Pedro Alves
2022-07-20 20:38 ` Simon Marchi
2022-07-13 22:24 ` [PATCH v2 03/29] gdb/linux: Delete all other LWPs immediately on ptrace exec event Pedro Alves
2022-07-21 0:45 ` Simon Marchi
2022-07-13 22:24 ` [PATCH v2 04/29] Step over clone syscall w/ breakpoint, TARGET_WAITKIND_THREAD_CLONED Pedro Alves
2022-07-21 1:35 ` Simon Marchi
2022-10-17 18:54 ` Pedro Alves
2022-10-18 12:40 ` [PATCH] Don't explicitly set clone child ptrace options (was: Re: [PATCH v2 04/29] Step over clone syscall w/ breakpoint, TARGET_WAITKIND_THREAD_CLONED) Pedro Alves
2022-10-28 14:50 ` Simon Marchi
2022-12-12 20:13 ` [PATCH v2 04/29] Step over clone syscall w/ breakpoint, TARGET_WAITKIND_THREAD_CLONED Pedro Alves
2022-07-13 22:24 ` [PATCH v2 05/29] Support clone events in the remote protocol Pedro Alves
2022-07-21 2:25 ` Simon Marchi
2022-12-12 20:14 ` Pedro Alves
2022-07-13 22:24 ` [PATCH v2 06/29] Avoid duplicate QThreadEvents packets Pedro Alves
2022-07-21 2:30 ` Simon Marchi
2022-12-12 20:14 ` Pedro Alves
2022-07-13 22:24 ` [PATCH v2 07/29] Thread options & clone events (core + remote) Pedro Alves
2022-07-21 3:14 ` Simon Marchi
2022-10-27 19:55 ` [PATCH] enum_flags to_string (was: Re: [PATCH v2 07/29] Thread options & clone events (core + remote)) Pedro Alves
2022-10-28 10:26 ` [PATCH v2] " Pedro Alves
2022-10-28 11:08 ` [PATCH v3] " Pedro Alves
2022-10-28 15:59 ` Simon Marchi
2022-10-28 18:23 ` Pedro Alves
2022-10-31 12:47 ` Simon Marchi
2022-11-07 17:26 ` [PATCH v5] " Pedro Alves
2022-11-07 18:29 ` Simon Marchi
2022-11-08 14:56 ` Pedro Alves
2022-12-12 20:15 ` [PATCH v2 07/29] Thread options & clone events (core + remote) Pedro Alves
2022-07-13 22:24 ` [PATCH v2 08/29] Thread options & clone events (native Linux) Pedro Alves
2022-07-21 12:38 ` Simon Marchi
2022-12-12 20:16 ` Pedro Alves
2022-07-13 22:24 ` [PATCH v2 09/29] Thread options & clone events (Linux GDBserver) Pedro Alves
2022-07-21 13:11 ` Simon Marchi
2022-12-12 20:16 ` Pedro Alves
2022-07-13 22:24 ` [PATCH v2 10/29] gdbserver: Hide and don't detach pending clone children Pedro Alves
2022-07-13 22:24 ` [PATCH v2 11/29] Remove gdb/19675 kfails (displaced stepping + clone) Pedro Alves
2022-07-13 22:24 ` [PATCH v2 12/29] Add test for stepping over clone syscall Pedro Alves
2022-07-13 22:24 ` [PATCH v2 13/29] all-stop/synchronous RSP support thread-exit events Pedro Alves
2022-07-13 22:24 ` [PATCH v2 14/29] gdbserver/linux-low.cc: Ignore event_ptid if TARGET_WAITKIND_IGNORE Pedro Alves
2022-07-13 22:24 ` [PATCH v2 15/29] Introduce GDB_TO_EXIT thread option, fix step-over-thread-exit Pedro Alves
2022-07-13 22:24 ` [PATCH v2 16/29] Implement GDB_TO_EXIT support for Linux GDBserver Pedro Alves
2022-07-13 22:24 ` [PATCH v2 17/29] Implement GDB_TO_EXIT support for native Linux Pedro Alves
2022-07-21 15:26 ` Simon Marchi
2022-12-12 20:17 ` Pedro Alves
2022-07-13 22:24 ` [PATCH v2 18/29] gdb: clear step over information on thread exit (PR gdb/27338) Pedro Alves
2022-07-21 18:12 ` Simon Marchi
2022-12-12 20:18 ` Pedro Alves
2022-07-13 22:24 ` [PATCH v2 19/29] stop_all_threads: (re-)enable async before waiting for stops Pedro Alves
2022-07-21 18:21 ` Simon Marchi
2022-12-12 20:18 ` Pedro Alves
2022-07-13 22:24 ` [PATCH v2 20/29] gdbserver: Queue no-resumed event after thread exit Pedro Alves
2022-07-13 22:24 ` Pedro Alves [this message]
2022-07-14 5:28 ` [PATCH v2 21/29] Don't resume new threads if scheduler-locking is in effect Eli Zaretskii
2022-07-21 18:49 ` Simon Marchi
2022-12-12 20:19 ` Pedro Alves
2022-07-13 22:24 ` [PATCH v2 22/29] Report thread exit event for leader if reporting thread exit events Pedro Alves
2022-07-13 22:24 ` [PATCH v2 23/29] Ignore failure to read PC when resuming Pedro Alves
2022-07-13 22:24 ` [PATCH v2 24/29] gdb/testsuite/lib/my-syscalls.S: Refactor new SYSCALL macro Pedro Alves
2022-07-13 22:24 ` [PATCH v2 25/29] Testcases for stepping over thread exit syscall (PR gdb/27338) Pedro Alves
2022-07-13 22:24 ` [PATCH v2 26/29] Document remote clone events, and QThreadOptions packet Pedro Alves
2022-07-14 5:27 ` Eli Zaretskii
2022-07-13 22:24 ` [PATCH v2 27/29] inferior::clear_thread_list always silent Pedro Alves
2022-07-13 22:24 ` [PATCH v2 28/29] Centralize "[Thread ...exited]" notifications Pedro Alves
2022-07-13 22:24 ` [PATCH v2 29/29] Cancel execution command on thread exit, when stepping, nexting, etc Pedro Alves
2022-07-21 19:28 ` [PATCH v2 00/29] Step over thread clone and thread exit Simon Marchi
2022-10-03 13:46 ` Tom de Vries
2022-10-03 18:37 ` Tom de Vries
2022-12-12 20:20 ` Pedro Alves
2022-12-12 20:19 ` Pedro Alves
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=20220713222433.374898-22-pedro@palves.net \
--to=pedro@palves.net \
--cc=gdb-patches@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: link
Be 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).