[3/3] gdb: add a '-stopped' option to "info threads"

Message ID de95477f0d1022b67e539e15bc6c4dacc0108c85.1680686221.git.tankut.baris.aktemur@intel.com
State New
Headers
Series Option to show stopped threads only |

Commit Message

Aktemur, Tankut Baris April 5, 2023, 9:20 a.m. UTC
  Add a '-stopped' option to the "info threads" command to print stopped
threads only and skip the running ones.  This is a convenience flag to
filter out the running threads in programs that have many threads.

Suppose we have an application with 5 threads, 2 of which have hit a
breakpoint.  The "info threads" command in the non-stop mode gives:

  (gdb) info threads
    Id   Target Id             Frame
  * 1    Thread 0x7ffff7d99740 (running)
    2    Thread 0x7ffff7d98700 something () at file.c:30
    3    Thread 0x7ffff7597700 (running)
    4    Thread 0x7ffff6d96700 something () at file.c:30
    5    Thread 0x7ffff6595700 (running)
  (gdb)

Using the "-stopped" flag, we get

  (gdb) info threads -stopped
    Id   Target Id             Frame
    2    Thread 0x7ffff7d98700 something () at file.c:30
    4    Thread 0x7ffff6d96700 something () at file.c:30
  (gdb)

When combined with a thread ID, the behavior is as follows:

  (gdb) info threads 3
    Id   Target Id             Frame
    3    Thread 0x7ffff7597700 (running)
  (gdb) info threads -stopped 3
  No stopped threads match '3'.
  (gdb)

Regression-tested on X86_64 Linux.
---
 gdb/NEWS                                      |  7 ++
 gdb/doc/gdb.texinfo                           |  6 +-
 gdb/testsuite/gdb.base/options.exp            | 11 ++-
 .../gdb.threads/info-threads-stopped.c        | 78 +++++++++++++++++++
 .../gdb.threads/info-threads-stopped.exp      | 75 ++++++++++++++++++
 gdb/thread.c                                  | 17 +++-
 6 files changed, 190 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.threads/info-threads-stopped.c
 create mode 100644 gdb/testsuite/gdb.threads/info-threads-stopped.exp
  

Comments

Eli Zaretskii April 5, 2023, 10 a.m. UTC | #1
> Date: Wed,  5 Apr 2023 11:20:02 +0200
> From: Tankut Baris Aktemur via Gdb-patches <gdb-patches@sourceware.org>
> 
>  gdb/NEWS                                      |  7 ++
>  gdb/doc/gdb.texinfo                           |  6 +-
>  gdb/testsuite/gdb.base/options.exp            | 11 ++-
>  .../gdb.threads/info-threads-stopped.c        | 78 +++++++++++++++++++
>  .../gdb.threads/info-threads-stopped.exp      | 75 ++++++++++++++++++
>  gdb/thread.c                                  | 17 +++-
>  6 files changed, 190 insertions(+), 4 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.threads/info-threads-stopped.c
>  create mode 100644 gdb/testsuite/gdb.threads/info-threads-stopped.exp

Thanks, the documentation parts are OK.

But I have a question about the new behavior:

>   (gdb) info threads -stopped 3
>   No stopped threads match '3'.

Is it really useful to show nothing and emit an error message if
explicit thread IDs were specified by the user?  Wouldn't it be better
to ignore the -stopped switch in that case?

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
  
Terekhov, Mikhail via Gdb-patches April 5, 2023, 10:19 a.m. UTC | #2
On Wednesday, April 5, 2023 12:01 PM, Eli Zaretskii wrote:
> > Date: Wed,  5 Apr 2023 11:20:02 +0200
> > From: Tankut Baris Aktemur via Gdb-patches <gdb-patches@sourceware.org>
> >
> >  gdb/NEWS                                      |  7 ++
> >  gdb/doc/gdb.texinfo                           |  6 +-
> >  gdb/testsuite/gdb.base/options.exp            | 11 ++-
> >  .../gdb.threads/info-threads-stopped.c        | 78 +++++++++++++++++++
> >  .../gdb.threads/info-threads-stopped.exp      | 75 ++++++++++++++++++
> >  gdb/thread.c                                  | 17 +++-
> >  6 files changed, 190 insertions(+), 4 deletions(-)
> >  create mode 100644 gdb/testsuite/gdb.threads/info-threads-stopped.c
> >  create mode 100644 gdb/testsuite/gdb.threads/info-threads-stopped.exp
> 
> Thanks, the documentation parts are OK.
> 
> But I have a question about the new behavior:
> 
> >   (gdb) info threads -stopped 3
> >   No stopped threads match '3'.
> 
> Is it really useful to show nothing and emit an error message if
> explicit thread IDs were specified by the user?  Wouldn't it be better
> to ignore the -stopped switch in that case?
> 
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>

The user may have given a thread id range or a wildcard, like
"info threads -stopped 1-999" or "info threads -stopped 2.*".
Ignoring the flag for these cases can output a very long list.

I'm fine if we make the single thread id a special case.  But then
the question is, where do we draw the line?  If the user gave just a
few thread ids, do we still ignore the flag?  What is the limit to
the acceptable list length?  Because of these questions, consistently
applying the flag made more sense to me.

Regards
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  
Eli Zaretskii April 5, 2023, 10:50 a.m. UTC | #3
> From: "Aktemur, Tankut Baris" <tankut.baris.aktemur@intel.com>
> Date: Wed, 5 Apr 2023 10:19:26 +0000
> 
> I'm fine if we make the single thread id a special case.

Maybe that's all we should do.

> But then the question is, where do we draw the line?  If the user
> gave just a few thread ids, do we still ignore the flag?  What is
> the limit to the acceptable list length?  Because of these
> questions, consistently applying the flag made more sense to me.

We don't have to be 100% consistent, we just need to be useful.
  
Terekhov, Mikhail via Gdb-patches April 5, 2023, 11:31 a.m. UTC | #4
On Wednesday, April 5, 2023 12:51 PM, Eli Zaretskii wrote:
> > From: "Aktemur, Tankut Baris" <tankut.baris.aktemur@intel.com>
> > Date: Wed, 5 Apr 2023 10:19:26 +0000
> >
> > I'm fine if we make the single thread id a special case.
> 
> Maybe that's all we should do.
> 
> > But then the question is, where do we draw the line?  If the user
> > gave just a few thread ids, do we still ignore the flag?  What is
> > the limit to the acceptable list length?  Because of these
> > questions, consistently applying the flag made more sense to me.
> 
> We don't have to be 100% consistent, we just need to be useful.

Let's please wait a bit in case other maintainers want to chime in.
I don't have an objection to treating the single thread id case specially.

-Baris

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  
Eli Zaretskii April 5, 2023, 11:56 a.m. UTC | #5
> From: "Aktemur, Tankut Baris" <tankut.baris.aktemur@intel.com>
> Date: Wed, 5 Apr 2023 11:31:16 +0000
> 
> On Wednesday, April 5, 2023 12:51 PM, Eli Zaretskii wrote:
> > > From: "Aktemur, Tankut Baris" <tankut.baris.aktemur@intel.com>
> > > Date: Wed, 5 Apr 2023 10:19:26 +0000
> > >
> > > I'm fine if we make the single thread id a special case.
> > 
> > Maybe that's all we should do.
> > 
> > > But then the question is, where do we draw the line?  If the user
> > > gave just a few thread ids, do we still ignore the flag?  What is
> > > the limit to the acceptable list length?  Because of these
> > > questions, consistently applying the flag made more sense to me.
> > 
> > We don't have to be 100% consistent, we just need to be useful.
> 
> Let's please wait a bit in case other maintainers want to chime in.

Sure.  I just expressed my personal opinion, not a request to make
changes in your patches right away.
  

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index 10a1a70fa52..33b104fda72 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -96,6 +96,13 @@  info main
    $2 = 1
    (gdb) break func if $_shell("some command") == 0
 
+* Changed commands
+
+info threads [-gid] [-stopped] [ID]...
+  This command now takes an optional flag, '-stopped', that causes only
+  the stopped threads to be printed.  The flag can be useful to get a
+  reduced list when there is a large number of unstopped threads.
+
 * MI changes
 
 ** mi now reports 'no-history' as a stop reason when hitting the end of the
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index ed14888b77b..02f6a9eb49a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -3678,7 +3678,7 @@  Thread 1 "main" received signal SIGINT, Interrupt.
 @table @code
 @anchor{info_threads}
 @kindex info threads
-@item info threads @r{[}-gid@r{]} @r{[}@var{thread-id-list}@r{]}
+@item info threads @r{[}-gid@r{]} @r{[}-stopped@r{]} @r{[}@var{thread-id-list}@r{]}
 
 Display information about one or more threads.  With no arguments
 displays information about all threads.  You can specify the list of
@@ -3728,6 +3728,10 @@  If you're debugging multiple inferiors, @value{GDBN} displays thread
 IDs using the qualified @var{inferior-num}.@var{thread-num} format.
 Otherwise, only @var{thread-num} is shown.
 
+If you specify the @samp{-stopped} option, @value{GDBN} displays the
+stopped threads only.  This can be helpful to reduce the output list
+if there is a large number of unstopped threads.
+
 If you specify the @samp{-gid} option, @value{GDBN} displays a column
 indicating each thread's global thread ID:
 
diff --git a/gdb/testsuite/gdb.base/options.exp b/gdb/testsuite/gdb.base/options.exp
index 6d90615710e..58c85567a4e 100644
--- a/gdb/testsuite/gdb.base/options.exp
+++ b/gdb/testsuite/gdb.base/options.exp
@@ -492,12 +492,21 @@  proc_with_prefix test-thread-apply {} {
 proc_with_prefix test-info-threads {} {
     test_gdb_complete_multiple "info threads " "" "" {
 	"-gid"
+	"-stopped"
 	"ID"
     }
 
+    test_gdb_complete_multiple "info threads " "-" "" {
+	"-gid"
+	"-stopped"
+    }
+
     test_gdb_complete_unique \
-	"info threads -" \
+	"info threads -g" \
 	"info threads -gid"
+    test_gdb_complete_unique \
+	"info threads -s" \
+	"info threads -stopped"
 
     # "ID" isn't really something the user can type.
     test_gdb_complete_none "info threads I"
diff --git a/gdb/testsuite/gdb.threads/info-threads-stopped.c b/gdb/testsuite/gdb.threads/info-threads-stopped.c
new file mode 100644
index 00000000000..42ca8cf0b69
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/info-threads-stopped.c
@@ -0,0 +1,78 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2022-2023 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 <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <pthread.h>
+
+#define NUM 4
+
+volatile int should_spin = 1;
+
+static void
+something ()
+{
+}
+
+static void
+spin ()
+{
+  while (should_spin)
+    usleep (1);
+}
+
+static void *
+work (void *arg)
+{
+  int id = *((int *) arg);
+
+  /* Sleep a bit to give the other threads a chance to run.  */
+  usleep (1);
+
+  if (id % 2 == 0)
+    something (); /* break-here */
+  else
+    spin ();
+
+  pthread_exit (NULL);
+}
+
+int
+main ()
+{
+  /* Ensure we stop if GDB crashes and DejaGNU fails to kill us.  */
+  alarm (10);
+
+  pthread_t threads[NUM];
+  void *thread_result;
+  int ids[NUM];
+
+  for (int i = 0; i < NUM; i++)
+    {
+      ids[i] = i + 2;
+      pthread_create (&threads[i], NULL, work, &(ids[i]));
+    }
+
+  sleep (10);
+  should_spin = 0;
+
+  for (int i = 0; i < NUM; i++)
+    pthread_join(threads[i], &thread_result);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/info-threads-stopped.exp b/gdb/testsuite/gdb.threads/info-threads-stopped.exp
new file mode 100644
index 00000000000..5643a513aff
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/info-threads-stopped.exp
@@ -0,0 +1,75 @@ 
+# Copyright (C) 2022-2023 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 for the '-stopped' flag of the "info threads" command.
+
+standard_testfile
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+	 executable debug] != "" } {
+    return -1
+}
+
+save_vars { GDBFLAGS } {
+    append GDBFLAGS " -ex \"set non-stop on\""
+    clean_restart $binfile
+}
+
+gdb_breakpoint "something"
+gdb_run_cmd
+
+# Two threads hit the bp.
+set fill "\[^\r\n\]+"
+set num_hits 0
+gdb_test_multiple "" "hit the breakpoint" -lbl {
+    -re "\r\nThread ${fill} hit Breakpoint 1${fill}" {
+	incr num_hits
+	if {$num_hits < 2} {
+	    exp_continue
+	}
+    }
+    -re "\r\n$gdb_prompt " {
+	exp_continue
+    }
+}
+gdb_assert {$num_hits == 2} "two threads hit the bp"
+
+# Count the number of running/stopped threads reported
+# by the "info threads" command.
+foreach flag {"" "-stopped"} {
+    set num_running 0
+    set num_stopped 0
+    gdb_test_multiple "info threads $flag" "info threads $flag" {
+	-re "Id${fill}Target Id${fill}Frame${fill}" {
+	    exp_continue
+	}
+	-re "^\r\n. ${decimal}${fill}Thread ${fill}.running." {
+	    incr num_running
+	    exp_continue
+	}
+	-re "^\r\n. ${decimal}${fill}Thread ${fill}something ${fill}" {
+	    incr num_stopped
+	    exp_continue
+	}
+	-re "^\r\n$gdb_prompt $" {
+	    gdb_assert {$num_stopped == 2} "$gdb_test_name: num stopped"
+	    if {$flag eq ""} {
+		gdb_assert {$num_running == 3} "$gdb_test_name: num running"
+	    } else {
+		gdb_assert {$num_running == 0} "$gdb_test_name: num running"
+	    }
+	}
+    }
+}
diff --git a/gdb/thread.c b/gdb/thread.c
index 57b3f7611e2..8886bd45674 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -957,6 +957,8 @@  struct info_threads_opts
 {
   /* For "-gid".  */
   bool show_global_ids = false;
+  /* For "-stopped".  */
+  bool show_stopped_threads = false;
 };
 
 static const gdb::option::option_def info_threads_option_defs[] = {
@@ -966,6 +968,11 @@  static const gdb::option::option_def info_threads_option_defs[] = {
     [] (info_threads_opts *opts) { return &opts->show_global_ids; },
     N_("Show global thread IDs."),
   },
+  gdb::option::flag_option_def<info_threads_opts> {
+    "stopped",
+    [] (info_threads_opts *opts) { return &opts->show_stopped_threads; },
+    N_("Show stopped threads only."),
+  },
 
 };
 
@@ -1008,6 +1015,11 @@  should_print_thread (const char *requested_threads, int default_inf_num,
   if (thr->state == THREAD_EXITED)
     return false;
 
+  /* Skip a running thread if the user wants stopped threads only.  */
+  bool is_stopped = (thr->state == THREAD_STOPPED);
+  if (opts.show_stopped_threads && !is_stopped)
+    return false;
+
   return true;
 }
 
@@ -1094,7 +1106,8 @@  print_thread_info_1 (struct ui_out *uiout, const char *requested_threads,
 	    if (requested_threads == NULL || *requested_threads == '\0')
 	      uiout->message (_("No threads.\n"));
 	    else
-	      uiout->message (_("No threads match '%s'.\n"),
+	      uiout->message (_("No %sthreads match '%s'.\n"),
+			      (opts.show_stopped_threads ? "stopped " : ""),
 			      requested_threads);
 	    return;
 	  }
@@ -1219,7 +1232,7 @@  void
 print_thread_info (struct ui_out *uiout, const char *requested_threads,
 		   int pid)
 {
-  info_threads_opts opts {false};
+  info_threads_opts opts {false, false};
   print_thread_info_1 (uiout, requested_threads, 1, pid, opts);
 }