[PATCH 19/25] Don't resume new threads if scheduler-locking is in effect

Pedro Alves pedro@palves.net
Mon Jun 20 22:54:13 GMT 2022


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                                      |  3 +
 gdb/doc/gdb.texinfo                           |  3 +
 gdb/infrun.c                                  | 41 +++++++++---
 .../gdb.threads/schedlock-new-thread.c        | 46 ++++++++++++++
 .../gdb.threads/schedlock-new-thread.exp      | 63 +++++++++++++++++++
 5 files changed, 148 insertions(+), 8 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 5576c355b7a..2e842cb00f9 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,9 @@
 
 *** Changes since GDB 12
 
+* If supported by the target, when scheduler-locking is in effect, new
+  threads created by the resumed thread 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 b3e995f166b..775c4b1347b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -6932,6 +6932,9 @@ current thread away from the thread that you are debugging.  The
 @code{replay} mode behaves like @code{off} in record mode and like
 @code{on} in replay mode.
 
+If supported by the target, when scheduler-locking is in effect, new
+threads created by the resumed thread are held stopped.
+
 @item show scheduler-locking
 Display the current scheduler locking mode.
 @end table
diff --git a/gdb/infrun.c b/gdb/infrun.c
index c7d5acf2326..6c273fd8cf2 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))
+	target_set_thread_options (tp, 0);
+      else if (!displaced_step_in_progress_any_thread ())
+	target_thread_events (false);
+    }
 
   infrun_debug_printf ("resume_ptid=%s, step=%d, sig=%s",
 		       resume_ptid.to_string ().c_str (),
@@ -5995,16 +6015,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



More information about the Gdb-patches mailing list