Hi!

On 2025-04-04 14:36, Tankut Baris Aktemur wrote:
> 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.
> 
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
> Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
> ---
>  gdb/NEWS                                      |   5 +
>  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      | 107 ++++++++++++++++++
>  gdb/thread.c                                  |  17 ++-
>  6 files changed, 220 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
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 6a557bb4af9..d5c55c3c938 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -56,6 +56,11 @@ info sharedlibrary
>    command are now for the full memory range allocated to the shared
>    library.
>  
> +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.


Cool!

(I would have added -running too while at it, and make "thread apply -stopped" work too.

 And then:

  "info threads -stopped -running"

 would print both stopped and running.

 Same logic as "info lanes -active -inactive" including both active and inactive in
 our offlist discussions.)

> +
>  * Python API
>  
>    ** New class gdb.Color for dealing with colors.
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index e034ac53295..3c03761dd00 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -3807,7 +3807,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
> @@ -3857,6 +3857,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 8760a918082..90902e94086 100644
> --- a/gdb/testsuite/gdb.base/options.exp
> +++ b/gdb/testsuite/gdb.base/options.exp
> @@ -509,12 +509,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..2c38ecca074
> --- /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-2025 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);

This usleep isn't in a loop, so why is it needed?

> +
> +  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);

This is 10 seconds ...

> +
> +  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);

... and this is 10 seconds too.  Shouldn't the alarm clock be larger to give it a chance of clean exit?

Actually better is to not have a timer at all.  We can do that with a barrier that guarantees that
we reach a breakpoint only after all threads have been seen.  That fixes an issue which is that the
test is expecting to hit two breakpoints, but it isn't waiting until the threads that are not supposed
to hit a breakpoint actually start!  Due to scheduling, you could end up doing "info threads" before all
those threads are seen by GDB.  Using a barrier fixes that.

> +  should_spin = 0;
> +
> +  for (int i = 0; i < NUM; i++)
> +    pthread_join(threads[i], &thread_result);

Space missing: "pthread_join (".

You are not using thread_result for anything, so that could be:

    pthread_join (threads[i], NULL);

But really, with a barrier and given the alarm, this is dead code, basically.  Since we need a breakpoint after
the barrier, might as well make the main thread hit the something() breakpoint too and be considered a stopped thread.

(See attached patch.)

> +
> +  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..37d6622697c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/info-threads-stopped.exp
> @@ -0,0 +1,107 @@
> +# Copyright (C) 2022-2025 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
> +    }
> +}

It's better to write this in a way that explicitly always consumes the prompt.

> +gdb_assert {$num_hits == 2} "two threads hit the bp"
> +
> +# We are in non-stop mode.
> +# Send a simple command to resync the command prompt.
> +gdb_test "p 42" " = 42"

Then this shouldn't be needed.

> +
> +# Count the number of running/stopped threads reported
> +# by the "info threads" command.  We also capture thread ids
> +# for additional tests.
> +set running_tid "invalid"
> +set stopped_tid "invalid"
> +
> +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
> +	    set running_tid $expect_out(1,string)
> +	    exp_continue
> +	}
> +	-re "^\r\n. (${decimal})${fill}Thread ${fill}something ${fill}${srcfile}:${decimal}" {
> +	    incr num_stopped
> +	    set stopped_tid $expect_out(1,string)
> +	    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"

It's better practice have the same number of PASSes and FAILs.  They won't match if the prompt
regexp doesn't match.

> +	    }
> +	}
> +    }

This needs some fixing around regexp matching.  It is currently racy wrt to how much expect happens to
manage to read into in the expect buffer at a time.  If you run the test with "make check-read1" to force
one byte at a time, it exposes the races reliably:

 $ make check-read1 TESTS="gdb.threads/info-threads-stopped.exp"
 ...
 FAIL: gdb.threads/info-threads-stopped.exp: info threads
 FAIL: gdb.threads/info-threads-stopped.exp: info threads -stopped
 
                 === gdb Summary ===
 
 # of expected passes            8
 # of unexpected failures        2

I've attached a patch that fixes all the testcase issues that you can squash into yours.


> +}
> +
> +gdb_assert {$running_tid != "invalid"} "found a running thread"
> +gdb_assert {$stopped_tid != "invalid"} "found a stopped thread"
> +
> +# Test specifying thread ids.
> +gdb_test "info threads -stopped $running_tid" \
> +    "No stopped threads match '$running_tid'\." \
> +    "info thread -stopped for a running thread"
> +
> +set fill "\[^\r\n\]+"
> +set ws "\[ \t\]+"
> +gdb_test "info threads -stopped $stopped_tid" \
> +    [multi_line \
> +	 "${ws}Id${ws}Target Id${ws}Frame${ws}" \
> +	 "${ws}${stopped_tid}${ws}Thread ${fill} something ${fill}"] \
> +    "info thread -stopped for a stopped thread"
> +
> +gdb_test "info threads -stopped $running_tid $stopped_tid" \
> +    [multi_line \
> +	 "${ws}Id${ws}Target Id${ws}Frame${ws}" \
> +	 "${ws}${stopped_tid}${ws}Thread ${fill} something ${fill}"] \
> +    "info thread -stopped for a running and a stopped thread"
> diff --git a/gdb/thread.c b/gdb/thread.c
> index 7dc8e7018c5..87ee9f66680 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -1044,6 +1044,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[] = {
> @@ -1053,6 +1055,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."),
> +  },
>  
>  };
>  
> @@ -1095,6 +1102,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;
>  }
>  
> @@ -1286,7 +1298,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);

I don't think we should do this.  For one, it's not i18n friendly how that is written.

Then, once we add a filter like "-running" too, then it'll get awkward with:

  (gdb) info threads -stopped -running
  *what would we say here?*

I think the easiest is to just drop the %s part, and just say that no threads matched, like:

  	    if (!any_threads)
  	      uiout->message (_("No threads.\n"));
            else
	      uiout->message (_("No threads matched.\n"));

I've also attached a attached patch for this to make it easier to see what I mean.

Note that "\." in your regexp does not match a period.  It needs to be two back slashes "\\." as in my patch.

Thanks,
Pedro Alves