[patch] info threads sort by name and name regex matching

[patch] info threads sort by name and name regex matching

[Aaron Gamble]

[-- Attachment #1: Type: text/plain, Size: 1272 bytes --]

Hello all,

Here is a patch for adding sorting by name in 'info threads' and regex
pattern matching of thread names for filtering threads.

With this patch sorting by name will happen always. (Perhaps a switch
to enable sorting would be better?)

Regex matching is specified by doing 'info threads r<regex>'. This is
not ambiguous with previous behavior where parameters to info threads
were only numbers. (spaces between 'r' and <regex> are ignored)

e.g. info threads r ^name_prefix

Regards,

gdb/ChangeLog:
    * gdbthread.h: Add char *cached_name to struct thread_info for use
when sorting.
    * thread.c: #include <stdlib.h>.
    (thread_list_size): New global.
    (init_Thread_list): Initialize it.
    (free_thread): Decrement it.
    (new_thread): Increment it.
    (print_thread_filter_check): New function.
    (print_thread_sort_cmp): New function.
    (print_thread_info_single): New function.
    (print_thread_info): Sort threads by name and filter by regex match.

    testsuite/
    * gdb.threads/Makefile.in (EXECUTABLES): Add info-thread-sort-regex.
    * gdb.threads/info-thread-sort-regex.c: New file.
    * gdb.threads/info-thread-sort-regex.exp: New file.

    doc/
    * gdb.texinfo (Threads): Add information about sorting and regex parameters.

[-- Attachment #2: info_threads_sort_and_regex.patch --]
[-- Type: application/octet-stream, Size: 22486 bytes --]

From b66a50452170bda7f696cea24a459bd8ecc37764 Mon Sep 17 00:00:00 2001
From: Aaron Gamble <agamble@google.com>
Date: Tue, 21 Aug 2012 12:03:06 -0700
Subject: [PATCH] info threads sort and regex

---
 gdb/doc/gdb.texinfo                                |   10 +-
 gdb/gdbthread.h                                    |    4 +
 gdb/testsuite/gdb.threads/Makefile.in              |    2 +-
 gdb/testsuite/gdb.threads/info-thread-sort-regex.c |  122 +++++++
 .../gdb.threads/info-thread-sort-regex.exp         |   64 ++++
 gdb/thread.c                                       |  378 +++++++++++++-------
 6 files changed, 438 insertions(+), 142 deletions(-)
 create mode 100644 gdb/testsuite/gdb.threads/info-thread-sort-regex.c
 create mode 100644 gdb/testsuite/gdb.threads/info-thread-sort-regex.exp

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 08ba92d..53a45d7 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -2751,10 +2751,12 @@ number---always a single integer---with each thread in your program.
 
 @table @code
 @kindex info threads
-@item info threads @r{[}@var{id}@dots{}@r{]}
-Display a summary of all threads currently in your program.  Optional 
-argument @var{id}@dots{} is one or more thread ids separated by spaces, and
-means to print information only about the specified thread or threads.
+@item info threads @r{[}@var{id}@dots{}@b{|}r<@var{regex}>@r{]}
+Display a summary of all threads currently in your program.  Optional
+argument for specifying threads is either @code{r} followed by a regular
+expression or @var{id}@dots{} one or more numeric thread ids separated by
+spaces. The list of threads is sorted alphabetically by thread name.
+
 @value{GDBN} displays for each thread (in this order):
 
 @enumerate
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 0250555..c54d1ee 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -138,6 +138,10 @@ struct thread_info
      if the thread does not have a user-given name.  */
   char *name;
 
+  /* The name of the thread, as cached by print_thread_info.
+     Used for sorting info threads output.  */
+  char *cached_name;
+
   /* Non-zero means the thread is executing.  Note: this is different
      from saying that there is an active target and we are stopped at
      a breakpoint, for instance.  This is a real indicator whether the
diff --git a/gdb/testsuite/gdb.threads/Makefile.in b/gdb/testsuite/gdb.threads/Makefile.in
index 8f9f867..2d24ba4 100644
--- a/gdb/testsuite/gdb.threads/Makefile.in
+++ b/gdb/testsuite/gdb.threads/Makefile.in
@@ -5,7 +5,7 @@ EXECUTABLES = attach-into-signal-nothr attach-into-signal-thr \
 	attach-stopped attachstop-mt \
 	bp_in_thread current-lwp-dead execl execl1 fork-child-threads \
 	fork-thread-pending gcore-pthreads hand-call-in-threads \
-	ia64-sigill interrupted-hand-call killed linux-dp \
+	ia64-sigill info-thread-sort-regex interrupted-hand-call killed linux-dp \
 	local-watch-wrong-thread manythreads multi-create pending-step \
 	print-threads pthreads pthread_cond_wait schedlock sigthread \
 	staticthreads switch-threads thread-execl thread-specific \
diff --git a/gdb/testsuite/gdb.threads/info-thread-sort-regex.c b/gdb/testsuite/gdb.threads/info-thread-sort-regex.c
new file mode 100644
index 0000000..98979ce
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/info-thread-sort-regex.c
@@ -0,0 +1,122 @@
+/* Test case for info threads sort and regex matching.
+
+   Copyright 2008, 2010-2012 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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 <stdio.h>
+#include <stdlib.h>
+#include <time.h>
+#include <unistd.h>
+
+#ifndef NR_THREADS
+#define NR_THREADS 4
+#endif
+
+int thread_count;
+
+pthread_mutex_t thread_count_mutex;
+
+pthread_cond_t thread_count_condvar;
+
+void
+incr_thread_count (void)
+{
+  pthread_mutex_lock (&thread_count_mutex);
+  ++thread_count;
+  if (thread_count == NR_THREADS)
+    pthread_cond_signal (&thread_count_condvar);
+  pthread_mutex_unlock (&thread_count_mutex);
+}
+
+void
+cond_wait (pthread_cond_t *cond, pthread_mutex_t *mut)
+{
+  pthread_mutex_lock (mut);
+  pthread_cond_wait (cond, mut);
+  pthread_mutex_unlock (mut);
+}
+
+void
+noreturn (void)
+{
+  pthread_mutex_t mut;
+  pthread_cond_t cond;
+
+  pthread_mutex_init (&mut, NULL);
+  pthread_cond_init (&cond, NULL);
+
+  /* Wait for a condition that will never be signaled, so we effectively
+     block the thread here.  */
+  cond_wait (&cond, &mut);
+}
+
+void *
+forever_pthread (void *unused)
+{
+  incr_thread_count ();
+  noreturn ();
+}
+
+/* Wait until all threads are running.  */
+
+void
+wait_all_threads_running (void)
+{
+  pthread_mutex_lock (&thread_count_mutex);
+  if (thread_count == NR_THREADS)
+    {
+      pthread_mutex_unlock (&thread_count_mutex);
+      return;
+    }
+  pthread_cond_wait (&thread_count_condvar, &thread_count_mutex);
+  if (thread_count == NR_THREADS)
+    {
+      pthread_mutex_unlock (&thread_count_mutex);
+      return;
+    }
+  pthread_mutex_unlock (&thread_count_mutex);
+  printf ("failed waiting for all threads to start\n");
+  abort ();
+}
+
+/* Called when all threads are running.
+   Easy place for a breakpoint.  */
+
+void
+all_threads_running (void)
+{
+}
+
+int
+main (void)
+{
+  pthread_t forever[NR_THREADS];
+  int i;
+
+  pthread_mutex_init (&thread_count_mutex, NULL);
+  pthread_cond_init (&thread_count_condvar, NULL);
+
+  for (i = 0; i < NR_THREADS; ++i)
+    pthread_create (&forever[i], NULL, forever_pthread, NULL);
+
+  wait_all_threads_running ();
+  all_threads_running ();
+
+  return 0;
+}
+
diff --git a/gdb/testsuite/gdb.threads/info-thread-sort-regex.exp b/gdb/testsuite/gdb.threads/info-thread-sort-regex.exp
new file mode 100644
index 0000000..bd9e21d
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/info-thread-sort-regex.exp
@@ -0,0 +1,64 @@
+# Copyright (C) 2004, 2007-2008, 2010-2012 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 sorting and regex matching in info threads.
+
+set NR_THREADS 4
+
+standard_testfile
+
+# Start with a fresh gdb.
+gdb_exit
+gdb_start
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug "additional_flags=-DNR_THREADS=$NR_THREADS"]] != "" } {
+    return -1
+}
+
+clean_restart ${binfile}
+
+if { ![runto_main] } {
+    fail "Can't run to main"
+    return 0
+}
+
+gdb_test "break all_threads_running" \
+         "Breakpoint 2 at .*: file .*${srcfile}, line .*" \
+         "breakpoint on all_threads_running"
+
+# Run the program and make sure GDB reports that we stopped after
+# hitting breakpoint 2 in all_threads_running().
+
+gdb_test "continue" \
+         ".*Breakpoint 2, all_threads_running ().*" \
+         "run to all_threads_running"
+
+# Test that the no threads match message prints correctly.
+gdb_test "info threads r \$foo" \
+         "No threads match '\\\$foo'\." \
+         "info thread no match test"
+
+gdb_test_no_output "thread name a" "set thread name to a"
+
+gdb_test "info threads" \
+   "\\s+Id\\s+Target\\s+Id\\s+Frame \r\n\\*\\\s+\\\d+\\\s+Thread\\\s+0x\[A-Fa-f0-9\]+ \\\(LWP \\\d+\\\) \"a\".*" \
+   "check thread a is first"
+
+gdb_test "info threads r ^info" \
+   "\\s+Id\\s+Target\\s+Id\\s+Frame \r\n\\\s+\\\d+\\\s+Thread\\\s+0x\[A-Fa-f0-9\]+ \\\(LWP \\\d+\\\) \"info.*\".*" \
+   "filter out thread named a"
+
+return 0
diff --git a/gdb/thread.c b/gdb/thread.c
index 7e8eec5..68d306e 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -38,6 +38,7 @@
 #include <ctype.h>
 #include <sys/types.h>
 #include <signal.h>
+#include <stdlib.h>
 #include "ui-out.h"
 #include "observer.h"
 #include "annotate.h"
@@ -55,7 +56,10 @@ void _initialize_thread (void);
 /* Prototypes for local functions.  */
 
 struct thread_info *thread_list = NULL;
+static int thread_list_size = 0;
 static int highest_thread_num;
+static int print_thread_info_regex_cflags = 0;
+static int print_thread_info_regex_eflags = 0;
 
 static void thread_command (char *tidstr, int from_tty);
 static void thread_apply_all_command (char *, int);
@@ -132,6 +136,7 @@ free_thread (struct thread_info *tp)
 	xfree (tp->private);
     }
 
+  thread_list_size--;
   xfree (tp->name);
   xfree (tp);
 }
@@ -153,6 +158,7 @@ init_thread_list (void)
     }
 
   thread_list = NULL;
+  thread_list_size = 0;
 }
 
 /* Allocate a new thread with target id PTID and add it to the thread
@@ -169,6 +175,7 @@ new_thread (ptid_t ptid)
   tp->num = ++highest_thread_num;
   tp->next = thread_list;
   thread_list = tp;
+  thread_list_size++;
 
   /* Nothing to follow yet.  */
   tp->pending_follow.kind = TARGET_WAITKIND_SPURIOUS;
@@ -748,25 +755,208 @@ finish_thread_state_cleanup (void *arg)
   finish_thread_state (*ptid_p);
 }
 
+/* Returns true if user requested the thread.  */
+
+static int
+print_thread_filter_check (struct thread_info *tp, char *requested_threads,
+                           int pid, regex_t *preg)
+{
+  if (preg)
+    {
+      int err = regexec (preg, tp->cached_name, 0, NULL,
+                         print_thread_info_regex_eflags);
+      if (err == 0)
+        return TRUE;
+      else
+        return FALSE;
+    }
+
+  /* No regular expression was supplied, see if there's a match the list
+     of thread id's.  */
+  if (!number_is_in_list (requested_threads, tp->num))
+    return FALSE;
+
+  if (pid != -1 && PIDGET (tp->ptid) != pid)
+    {
+      if (requested_threads != NULL && *requested_threads != '\0')
+        error (_("Requested thread not found in requested process"));
+      return FALSE;
+    }
+
+  if (tp->state == THREAD_EXITED)
+    return FALSE;
+
+  return TRUE;
+}
+
+
+/* Used to Sort threads in lexicographic order by name.  */
+
+static int
+print_thread_sort_cmp (const void *p1, const void *p2)
+{
+  const char *name1 = (*(struct thread_info **)p1)->cached_name;
+  const char *name2 = (*(struct thread_info **)p2)->cached_name;
+  if (name1 && name2)
+    return strcoll (name1, name2);
+
+  /* Incase we receive null pointers instead of strings.
+     NULL pointers are sorted as empty strings.  */
+  if (!name1 && name2)
+    return -1;
+  if (!name2 && name1)
+    return 1;
+
+  return 0;
+}
+
+/* Prints the info for a single thread.
+   Called by print_thread_info for each thread.  */
+
+static void
+print_thread_info_single (struct ui_out *uiout, struct thread_info *tp,
+                               ptid_t current_ptid)
+{
+  struct cleanup *uiout_cleanup
+    = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+  int core;
+  char *extra_info, *name, *target_id;
+
+  if (ui_out_is_mi_like_p (uiout))
+    {
+      /* Compatibility.  */
+      if (ptid_equal (tp->ptid, current_ptid))
+        ui_out_text (uiout, "* ");
+      else
+        ui_out_text (uiout, "  ");
+    }
+    else
+    {
+      if (ptid_equal (tp->ptid, current_ptid))
+        ui_out_field_string (uiout, "current", "*");
+      else
+        ui_out_field_skip (uiout, "current");
+    }
+
+  ui_out_field_int (uiout, "id", tp->num);
+
+  /* For the CLI, we stuff everything into the target-id field.
+     This is a gross hack to make the output come out looking
+     correct.  The underlying problem here is that ui-out has no
+     way to specify that a field's space allocation should be
+     shared by several fields.  For MI, we do the right thing
+     instead.  */
+
+  target_id = target_pid_to_str (tp->ptid);
+  extra_info = target_extra_thread_info (tp);
+  if (tp->cached_name)
+    name = tp->cached_name;
+  else
+    name = tp->name ? tp->name : target_thread_name (tp);
+
+  if (ui_out_is_mi_like_p (uiout))
+    {
+      ui_out_field_string (uiout, "target-id", target_id);
+      if (extra_info)
+        ui_out_field_string (uiout, "details", extra_info);
+      if (name)
+        ui_out_field_string (uiout, "name", name);
+    }
+  else
+    {
+      struct cleanup *str_cleanup;
+      char *contents;
+
+      if (extra_info && name)
+        contents = xstrprintf ("%s \"%s\" (%s)", target_id,
+                               name, extra_info);
+      else if (extra_info)
+        contents = xstrprintf ("%s (%s)", target_id, extra_info);
+      else if (name)
+        contents = xstrprintf ("%s \"%s\"", target_id, name);
+      else
+        contents = xstrdup (target_id);
+      str_cleanup = make_cleanup (xfree, contents);
+
+      ui_out_field_string (uiout, "target-id", contents);
+      do_cleanups (str_cleanup);
+    }
+
+  if (tp->state == THREAD_RUNNING)
+    ui_out_text (uiout, "(running)\n");
+  else
+    {
+      /* The switch below puts us at the top of the stack (leaf
+         frame).  */
+      switch_to_thread (tp->ptid);
+      print_stack_frame (get_selected_frame (NULL),
+          /* For MI output, print frame level.  */
+          ui_out_is_mi_like_p (uiout),
+          LOCATION);
+    }
+
+  if (ui_out_is_mi_like_p (uiout))
+    {
+      char *state = "stopped";
+
+      if (tp->state == THREAD_RUNNING)
+        state = "running";
+      ui_out_field_string (uiout, "state", state);
+    }
+
+  core = target_core_of_thread (tp->ptid);
+  if (ui_out_is_mi_like_p (uiout) && core != -1)
+    ui_out_field_int (uiout, "core", core);
+
+  do_cleanups (uiout_cleanup);
+}
+
+/* Caches the name that will be shown to the user for a thread.
+   We keep track of this for sorting purposes.  */
+
+static void
+thread_cache_name (struct thread_info *tp)
+{
+    tp->cached_name = tp->name ? tp->name : target_thread_name (tp);
+}
+
 /* Prints the list of threads and their details on UIOUT.
    This is a version of 'info_threads_command' suitable for
    use from MI.
-   If REQUESTED_THREAD is not -1, it's the GDB id of the thread
-   that should be printed.  Otherwise, all threads are
-   printed.
-   If PID is not -1, only print threads from the process PID.
-   Otherwise, threads from all attached PIDs are printed.
-   If both REQUESTED_THREAD and PID are not -1, then the thread
-   is printed if it belongs to the specified process.  Otherwise,
-   an error is raised.  */
+   REQUESTED_THREADS is either a list of numeric thread Id's
+   or the character 'r' followed by a regular expression to
+   match with each thread's name. Any spaces between the 'r' and
+   the regular expression are ignored.
+   If neither is supplied then all threads are printed.
+   Threads are sorted by name unless we are sending to MI.  */
+
 void
 print_thread_info (struct ui_out *uiout, char *requested_threads, int pid)
 {
   struct thread_info *tp;
   ptid_t current_ptid;
   struct cleanup *old_chain;
-  char *extra_info, *name, *target_id;
   int current_thread = -1;
+  struct thread_info **threads = NULL;
+  int n_threads, i, ret;
+  regex_t preg_buffer;
+  regex_t *preg = NULL;
+
+  if (requested_threads && requested_threads[0] == 'r')
+    {
+      /* User has supplied a regular expression.  */
+      requested_threads = skip_spaces (&requested_threads[1]);
+      ret = regcomp (&preg_buffer, requested_threads,
+                     print_thread_info_regex_cflags);
+      if (ret == 0)
+        preg = &preg_buffer;
+      else
+        {
+          char *err =  get_regcomp_error (ret, &preg_buffer);
+          make_cleanup (xfree, err);
+          error (_("Invalid regexp (%s): %s"), err, requested_threads);
+        }
+    }
 
   update_thread_list ();
   current_ptid = inferior_ptid;
@@ -776,37 +966,52 @@ print_thread_info (struct ui_out *uiout, char *requested_threads, int pid)
 
   /* For backward compatibility, we make a list for MI.  A table is
      preferable for the CLI, though, because it shows table
-     headers.  */
+     headers.
+     We do not sort the list for MI.  */
   if (ui_out_is_mi_like_p (uiout))
-    make_cleanup_ui_out_list_begin_end (uiout, "threads");
-  else
     {
-      int n_threads = 0;
+      make_cleanup_ui_out_list_begin_end (uiout, "threads");
 
       for (tp = thread_list; tp; tp = tp->next)
-	{
-	  if (!number_is_in_list (requested_threads, tp->num))
-	    continue;
+        {
+          if (ptid_equal (tp->ptid, current_ptid))
+            current_thread = tp->num;
 
-	  if (pid != -1 && PIDGET (tp->ptid) != pid)
-	    continue;
+          if (print_thread_filter_check (tp, requested_threads, pid, NULL))
+            print_thread_info_single (uiout, tp, current_ptid);
+        }
+    }
+  else
+    {
+      threads = xmalloc (sizeof (*threads) * thread_list_size);
+      make_cleanup (free, threads);
 
-	  if (tp->state == THREAD_EXITED)
-	    continue;
 
-	  ++n_threads;
-	}
+      n_threads = 0;
+      for (i = 0, tp = thread_list; tp; tp = tp->next, ++i)
+        {
+          thread_cache_name (tp);
+          if (print_thread_filter_check (tp, requested_threads, pid, preg))
+            {
+              threads[n_threads] = tp;
+              ++n_threads;
+            }
+
+          if (ptid_equal (tp->ptid, current_ptid))
+            current_thread = tp->num;
+
+        }
 
       if (n_threads == 0)
-	{
-	  if (requested_threads == NULL || *requested_threads == '\0')
-	    ui_out_message (uiout, 0, _("No threads.\n"));
-	  else
-	    ui_out_message (uiout, 0, _("No threads match '%s'.\n"),
-			    requested_threads);
-	  do_cleanups (old_chain);
-	  return;
-	}
+        {
+          if (requested_threads == NULL || *requested_threads == '\0')
+            ui_out_message (uiout, 0, _("No threads.\n"));
+          else
+            ui_out_message (uiout, 0, _("No threads match '%s'.\n"),
+                                        requested_threads);
+          do_cleanups (old_chain);
+          return;
+        }
 
       make_cleanup_ui_out_table_begin_end (uiout, 4, n_threads, "threads");
 
@@ -815,115 +1020,14 @@ print_thread_info (struct ui_out *uiout, char *requested_threads, int pid)
       ui_out_table_header (uiout, 17, ui_left, "target-id", "Target Id");
       ui_out_table_header (uiout, 1, ui_left, "frame", "Frame");
       ui_out_table_body (uiout);
-    }
-
-  for (tp = thread_list; tp; tp = tp->next)
-    {
-      struct cleanup *chain2;
-      int core;
-
-      if (!number_is_in_list (requested_threads, tp->num))
-	continue;
-
-      if (pid != -1 && PIDGET (tp->ptid) != pid)
-	{
-	  if (requested_threads != NULL && *requested_threads != '\0')
-	    error (_("Requested thread not found in requested process"));
-	  continue;
-	}
-
-      if (ptid_equal (tp->ptid, current_ptid))
-	current_thread = tp->num;
-
-      if (tp->state == THREAD_EXITED)
-	continue;
-
-      chain2 = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
 
-      if (ui_out_is_mi_like_p (uiout))
-	{
-	  /* Compatibility.  */
-	  if (ptid_equal (tp->ptid, current_ptid))
-	    ui_out_text (uiout, "* ");
-	  else
-	    ui_out_text (uiout, "  ");
-	}
-      else
-	{
-	  if (ptid_equal (tp->ptid, current_ptid))
-	    ui_out_field_string (uiout, "current", "*");
-	  else
-	    ui_out_field_skip (uiout, "current");
-	}
-
-      ui_out_field_int (uiout, "id", tp->num);
-
-      /* For the CLI, we stuff everything into the target-id field.
-	 This is a gross hack to make the output come out looking
-	 correct.  The underlying problem here is that ui-out has no
-	 way to specify that a field's space allocation should be
-	 shared by several fields.  For MI, we do the right thing
-	 instead.  */
-
-      target_id = target_pid_to_str (tp->ptid);
-      extra_info = target_extra_thread_info (tp);
-      name = tp->name ? tp->name : target_thread_name (tp);
-
-      if (ui_out_is_mi_like_p (uiout))
-	{
-	  ui_out_field_string (uiout, "target-id", target_id);
-	  if (extra_info)
-	    ui_out_field_string (uiout, "details", extra_info);
-	  if (name)
-	    ui_out_field_string (uiout, "name", name);
-	}
-      else
-	{
-	  struct cleanup *str_cleanup;
-	  char *contents;
-
-	  if (extra_info && name)
-	    contents = xstrprintf ("%s \"%s\" (%s)", target_id,
-				   name, extra_info);
-	  else if (extra_info)
-	    contents = xstrprintf ("%s (%s)", target_id, extra_info);
-	  else if (name)
-	    contents = xstrprintf ("%s \"%s\"", target_id, name);
-	  else
-	    contents = xstrdup (target_id);
-	  str_cleanup = make_cleanup (xfree, contents);
-
-	  ui_out_field_string (uiout, "target-id", contents);
-	  do_cleanups (str_cleanup);
-	}
-
-      if (tp->state == THREAD_RUNNING)
-	ui_out_text (uiout, "(running)\n");
-      else
-	{
-	  /* The switch below puts us at the top of the stack (leaf
-	     frame).  */
-	  switch_to_thread (tp->ptid);
-	  print_stack_frame (get_selected_frame (NULL),
-			     /* For MI output, print frame level.  */
-			     ui_out_is_mi_like_p (uiout),
-			     LOCATION);
-	}
-
-      if (ui_out_is_mi_like_p (uiout))
-	{
-	  char *state = "stopped";
-
-	  if (tp->state == THREAD_RUNNING)
-	    state = "running";
-	  ui_out_field_string (uiout, "state", state);
-	}
 
-      core = target_core_of_thread (tp->ptid);
-      if (ui_out_is_mi_like_p (uiout) && core != -1)
-	ui_out_field_int (uiout, "core", core);
+      qsort (threads, n_threads, sizeof (*threads), print_thread_sort_cmp);
 
-      do_cleanups (chain2);
+      for (i = 0; i < n_threads; i++)
+        {
+          print_thread_info_single (uiout, threads[i], current_ptid);
+        }
     }
 
   /* Restores the current thread and the frame selected before
-- 
1.7.7.3

RE: [patch] info threads sort by name and name regex matching

[Abid, Hafiz]

>+set NR_THREADS 4
>+
>+standard_testfile
>+
>+# Start with a fresh gdb.
>+gdb_exit
>+gdb_start

I think you don't need gdb_exit and gdb_start when you are doing clean_restart below.

>+
>+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug "additional_flags=-DNR_THREADS=$NR_THREADS"]] != "" } {
>+    return -1
>+}
>+
>+clean_restart ${binfile}
>+
>+if { ![runto_main] } {
>+    fail "Can't run to main"
>+    return 0
>+}
>+

Regards,
Abid

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Aaron Gamble
> Sent: Tuesday, August 21, 2012 10:19 PM
> To: gdb-patches@sourceware.org
> Subject: [patch] info threads sort by name and name regex matching
> 
> Hello all,
> 
> Here is a patch for adding sorting by name in 'info threads' and regex
> pattern matching of thread names for filtering threads.
> 
> With this patch sorting by name will happen always. (Perhaps a switch
> to enable sorting would be better?)
> 
> Regex matching is specified by doing 'info threads r<regex>'. This is
> not ambiguous with previous behavior where parameters to info threads
> were only numbers. (spaces between 'r' and <regex> are ignored)
> 
> e.g. info threads r ^name_prefix
> 
> Regards,
> 
> gdb/ChangeLog:
>     * gdbthread.h: Add char *cached_name to struct thread_info for use
> when sorting.
>     * thread.c: #include <stdlib.h>.
>     (thread_list_size): New global.
>     (init_Thread_list): Initialize it.
>     (free_thread): Decrement it.
>     (new_thread): Increment it.
>     (print_thread_filter_check): New function.
>     (print_thread_sort_cmp): New function.
>     (print_thread_info_single): New function.
>     (print_thread_info): Sort threads by name and filter by regex
> match.
> 
>     testsuite/
>     * gdb.threads/Makefile.in (EXECUTABLES): Add info-thread-sort-
> regex.
>     * gdb.threads/info-thread-sort-regex.c: New file.
>     * gdb.threads/info-thread-sort-regex.exp: New file.
> 
>     doc/
>     * gdb.texinfo (Threads): Add information about sorting and regex
> parameters.

Re: [patch] info threads sort by name and name regex matching

[Eli Zaretskii]

> Date: Tue, 21 Aug 2012 14:18:42 -0700
> From: Aaron Gamble <agamble@google.com>
> 
> Here is a patch for adding sorting by name in 'info threads' and regex
> pattern matching of thread names for filtering threads.

Thanks.

> Regex matching is specified by doing 'info threads r<regex>'. This is
> not ambiguous with previous behavior where parameters to info threads
> were only numbers. (spaces between 'r' and <regex> are ignored)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This should be documented in the manual.

> +@item info threads @r{[}@var{id}@dots{}@b{|}r<@var{regex}>@r{]}

No need for <...> around "regex", the @var markup does the equivalent
already.

> +Display a summary of all threads currently in your program.  Optional
> +argument for specifying threads is either @code{r} followed by a regular
> +expression or @var{id}@dots{} one or more numeric thread ids separated by
                               ^^^
Something's missing here, perhaps "for" or an em-dash "---".

> +spaces. The list of threads is sorted alphabetically by thread name.
         ^^
Two spaces between sentences, please.

The documentation part is OK with these changes.

Re: [patch] info threads sort by name and name regex matching

[Tom Tromey]

>>>>> "Aaron" == Aaron Gamble <agamble@google.com> writes:

Aaron> Here is a patch for adding sorting by name in 'info threads' and regex
Aaron> pattern matching of thread names for filtering threads.

Thanks.  I think this is a good idea.

Aaron> With this patch sorting by name will happen always. (Perhaps a switch
Aaron> to enable sorting would be better?)

Yes, I think so.  I think sorting by name makes sense but it isn't
perhaps always what you want.  Sorting by number has the nice feature
that it is stable.

Aaron> Regex matching is specified by doing 'info threads r<regex>'. This is
Aaron> not ambiguous with previous behavior where parameters to info threads
Aaron> were only numbers. (spaces between 'r' and <regex> are ignored)

I'm curious why you chose this particular spelling.
Other possible approaches would be a subcommand, or a flag like "-r".
Either of these is perhaps more in keeping with gdb tradition.

I'm interested in other opinions here too.

Expect some bikeshedding on this point.

Aaron> +@item info threads @r{[}@var{id}@dots{}@b{|}r<@var{regex}>@r{]}
Aaron> +Display a summary of all threads currently in your program.  Optional
Aaron> +argument for specifying threads is either @code{r} followed by a regular
Aaron> +expression or @var{id}@dots{} one or more numeric thread ids separated by
Aaron> +spaces. The list of threads is sorted alphabetically by thread name.

I think the documentation should say what the regular expression matches
against.

Aaron> +# Start with a fresh gdb.
Aaron> +gdb_exit
Aaron> +gdb_start

What Hafiz said :)

Aaron> +return 0

There's no need for this in the .exp file.

Aaron> diff --git a/gdb/thread.c b/gdb/thread.c
[...]
Aaron> +#include <stdlib.h>

I think defs.h will already include this if it is available.

Aaron> +static int print_thread_info_regex_cflags = 0;
Aaron> +static int print_thread_info_regex_eflags = 0;

I don't think you need these.  Just put the constants directly in the
re* calls.  Also, I'm guessing you want REG_NOSUB at least.

Aaron> +  if (preg)
Aaron> +    {
Aaron> +      int err = regexec (preg, tp->cached_name, 0, NULL,
Aaron> +                         print_thread_info_regex_eflags);

I think cached_name can be NULL, because target_thread_name can return
NULL.

Aaron> +static int
Aaron> +print_thread_sort_cmp (const void *p1, const void *p2)
Aaron> +{
Aaron> +  const char *name1 = (*(struct thread_info **)p1)->cached_name;
Aaron> +  const char *name2 = (*(struct thread_info **)p2)->cached_name;

GNU style requires some spaces in here, before "p1" and "p2".

Aaron> +  if (name1 && name2)
Aaron> +    return strcoll (name1, name2);

strcoll isn't used in gdb yet.  So, you have to look to see whether
configury is required.  I usually check gnulib.

Using strcmp seems just as good though.

Aaron> +  /* Incase we receive null pointers instead of strings.

s/Incase/In case/ and s/null/NULL/

Aaron> +/* Caches the name that will be shown to the user for a thread.
Aaron> +   We keep track of this for sorting purposes.  */
Aaron> +
Aaron> +static void
Aaron> +thread_cache_name (struct thread_info *tp)
Aaron> +{
Aaron> +    tp->cached_name = tp->name ? tp->name : target_thread_name (tp);

Too much indentation.

I think the cached_name field should have a comment explaining that it
doesn't need to be freed, and that it is just transient.

Aaron>  void
Aaron>  print_thread_info (struct ui_out *uiout, char *requested_threads, int pid)
Aaron>  {
Aaron>    struct thread_info *tp;
Aaron>    ptid_t current_ptid;
Aaron>    struct cleanup *old_chain;
Aaron> -  char *extra_info, *name, *target_id;
Aaron>    int current_thread = -1;
Aaron> +  struct thread_info **threads = NULL;
Aaron> +  int n_threads, i, ret;
Aaron> +  regex_t preg_buffer;
Aaron> +  regex_t *preg = NULL;
Aaron> +
Aaron> +  if (requested_threads && requested_threads[0] == 'r')
Aaron> +    {
Aaron> +      /* User has supplied a regular expression.  */
Aaron> +      requested_threads = skip_spaces (&requested_threads[1]);
Aaron> +      ret = regcomp (&preg_buffer, requested_threads,
Aaron> +                     print_thread_info_regex_cflags);

Nothing ever calls regfree on preg_buffer.
You probably want to use make_regfree_cleanup.
You may want to rearrange the code so that either the assignment to
old_chain happens before this, or to make a null cleanup first.

Aaron> +      threads = xmalloc (sizeof (*threads) * thread_list_size);
Aaron> +      make_cleanup (free, threads);

I think making a VEC here would be better.
Then you wouldn't need thread_list_size at all.

Tom

Re: [patch] info threads sort by name and name regex matching

[Aaron Gamble]

On Wed, Aug 22, 2012 at 11:51 AM, Tom Tromey <tromey@redhat.com> wrote:

*snip*

> Aaron> With this patch sorting by name will happen always. (Perhaps a switch
> Aaron> to enable sorting would be better?)
>
> Yes, I think so.  I think sorting by name makes sense but it isn't
> perhaps always what you want.  Sorting by number has the nice feature
> that it is stable.
>
> Aaron> Regex matching is specified by doing 'info threads r<regex>'. This is
> Aaron> not ambiguous with previous behavior where parameters to info threads
> Aaron> were only numbers. (spaces between 'r' and <regex> are ignored)
>
> I'm curious why you chose this particular spelling.
> Other possible approaches would be a subcommand, or a flag like "-r".
> Either of these is perhaps more in keeping with gdb tradition.
>
> I'm interested in other opinions here too.
>
> Expect some bikeshedding on this point.
I picked a single letter for it to be quick and easy to type. I
suppose it's possible 'info threads' could be expanded to do other
sorts of matching where different flags would be useful.

How about 'info threads [ -a ] [ id.. | -n <name regex> ]'

-a - Sort alphabetically by name
-n regex - Match thread names with regex

Of course then we could do sorting based on the name of the function a
thread is in or other sorts of sorting. So other sorting flags would
need to be introduced.

*snip*
> Aaron> +      threads = xmalloc (sizeof (*threads) * thread_list_size);
> Aaron> +      make_cleanup (free, threads);
>
> I think making a VEC here would be better.
> Then you wouldn't need thread_list_size at all.

Hmm, not sure if this would improve performance at all. A one time
allocation bounded by the number of threads vs VEC's implementation of
expanding arrays. I'll wait for others feedback.

>
> Tom

Thanks for the feedback. I'll fix the other things you mentioned I snipped out.


-Aaron

Re: [patch] info threads sort by name and name regex matching

[Sergio Durigan Junior]

On Wednesday, August 22 2012, Aaron Gamble wrote:

> On Wed, Aug 22, 2012 at 11:51 AM, Tom Tromey <tromey@redhat.com> wrote:

>> Aaron> With this patch sorting by name will happen always. (Perhaps a switch
>> Aaron> to enable sorting would be better?)
>>
>> Yes, I think so.  I think sorting by name makes sense but it isn't
>> perhaps always what you want.  Sorting by number has the nice feature
>> that it is stable.
>>
>> Aaron> Regex matching is specified by doing 'info threads r<regex>'. This is
>> Aaron> not ambiguous with previous behavior where parameters to info threads
>> Aaron> were only numbers. (spaces between 'r' and <regex> are ignored)
>>
>> I'm curious why you chose this particular spelling.
>> Other possible approaches would be a subcommand, or a flag like "-r".
>> Either of these is perhaps more in keeping with gdb tradition.
>>
>> I'm interested in other opinions here too.
>>
>> Expect some bikeshedding on this point.
> I picked a single letter for it to be quick and easy to type. I
> suppose it's possible 'info threads' could be expanded to do other
> sorts of matching where different flags would be useful.
>
> How about 'info threads [ -a ] [ id.. | -n <name regex> ]'
>
> -a - Sort alphabetically by name
> -n regex - Match thread names with regex
>
> Of course then we could do sorting based on the name of the function a
> thread is in or other sorts of sorting. So other sorting flags would
> need to be introduced.

Why not `-r regex'?  I think it would be more clear.

Other commands (`sharedlibrary', `info variables', etc) take a regex as
their first argument, without requiring a modifier like `-r'.  If the
"pipe" patch were already in, this alphabetical sorting would not be
needed...  Anyway, just thinking here.

> *snip*
>> Aaron> +      threads = xmalloc (sizeof (*threads) * thread_list_size);
>> Aaron> +      make_cleanup (free, threads);
>>
>> I think making a VEC here would be better.
>> Then you wouldn't need thread_list_size at all.
>
> Hmm, not sure if this would improve performance at all. A one time
> allocation bounded by the number of threads vs VEC's implementation of
> expanding arrays. I'll wait for others feedback.

I guess Tom did not suggest this because of performance per se, but
rather because if you have to make a list in GDB then it is already a
convention to use VEC for these things.

BTW, I noticed one minor nit in your patch: you do not obey the "TAB
vs. space" rule.  Basically, if you have 8 spaces, then you should
convert it to a TAB.  Emacs does this for free, but if you're using Vim
you can ask me offlist and I can send you a little function that does
that.

Thanks,

-- 
Sergio

Re: [patch] info threads sort by name and name regex matching

[Tom Tromey]

Sergio> Why not `-r regex'?  I think it would be more clear.

Me too.

Sergio> Other commands (`sharedlibrary', `info variables', etc) take a regex as
Sergio> their first argument, without requiring a modifier like `-r'.  If the
Sergio> "pipe" patch were already in, this alphabetical sorting would not be
Sergio> needed...  Anyway, just thinking here.

Piping is nice but I think it is also good to have options for common
cases.

Sergio> I guess Tom did not suggest this because of performance per se, but
Sergio> rather because if you have to make a list in GDB then it is already a
Sergio> convention to use VEC for these things.

Yeah.  I thought it would make the code simpler.

Tom

Re: [patch] info threads sort by name and name regex matching

[Aaron Gamble]

[-- Attachment #1: Type: text/plain, Size: 1165 bytes --]

Here is an updated patch. I have changed the options to be as follows.

'info threads [-a] [ id ... | -r regex ]'

-a enables sorting by thread name
-r regex matches regex with thread names.

I have not changed the threads array to a VEC as of yet. At this point
it seems like more trouble than it's worth. But I suppose it could be
done if people think it's necessary.

On Thu, Aug 23, 2012 at 9:00 AM, Tom Tromey <tromey@redhat.com> wrote:
>
> Sergio> Why not `-r regex'?  I think it would be more clear.
>
> Me too.
>
> Sergio> Other commands (`sharedlibrary', `info variables', etc) take a regex as
> Sergio> their first argument, without requiring a modifier like `-r'.  If the
> Sergio> "pipe" patch were already in, this alphabetical sorting would not be
> Sergio> needed...  Anyway, just thinking here.
>
> Piping is nice but I think it is also good to have options for common
> cases.
>
> Sergio> I guess Tom did not suggest this because of performance per se, but
> Sergio> rather because if you have to make a list in GDB then it is already a
> Sergio> convention to use VEC for these things.
>
> Yeah.  I thought it would make the code simpler.
>
> Tom

[-- Attachment #2: thread_sorting_regex_with-a-r-options.patch --]
[-- Type: application/octet-stream, Size: 22268 bytes --]

From efef7b796f528835d18bb778304805e6228695fd Mon Sep 17 00:00:00 2001
From: Aaron Gamble <agamble@google.com>
Date: Thu, 23 Aug 2012 18:04:03 -0700
Subject: [PATCH] sorting with -a -r regex options

---
 gdb/doc/gdb.texinfo                                |   11 +-
 gdb/gdbthread.h                                    |    6 +-
 gdb/testsuite/gdb.threads/Makefile.in              |    2 +-
 gdb/testsuite/gdb.threads/info-thread-sort-regex.c |  122 +++++++
 .../gdb.threads/info-thread-sort-regex.exp         |   58 +++
 gdb/thread.c                                       |  374 +++++++++++++-------
 6 files changed, 441 insertions(+), 132 deletions(-)
 create mode 100644 gdb/testsuite/gdb.threads/info-thread-sort-regex.c
 create mode 100644 gdb/testsuite/gdb.threads/info-thread-sort-regex.exp

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 08ba92d..3e6c108 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -2751,10 +2751,13 @@ number---always a single integer---with each thread in your program.
 
 @table @code
 @kindex info threads
-@item info threads @r{[}@var{id}@dots{}@r{]}
-Display a summary of all threads currently in your program.  Optional 
-argument @var{id}@dots{} is one or more thread ids separated by spaces, and
-means to print information only about the specified thread or threads.
+@item info threads @r{[}-a@r{]} @r{[}@var{id}@dots{}@b{|}-r @var{regex}@r{]}
+Display a summary of all threads currently in your program.  Use the option
+-a to sort threads alphabetically by name.  Selection of threads can be done
+by supplying a list or ranges of thread ID's, or by using the -r @var{regex}
+option to display only threads with names matching @var{regex}.  Spaces
+between the -r and @var{regex} are ignored.
+
 @value{GDBN} displays for each thread (in this order):
 
 @enumerate
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 0250555..2463cea 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -138,6 +138,10 @@ struct thread_info
      if the thread does not have a user-given name.  */
   char *name;
 
+  /* The name of the thread, as cached by print_thread_info.
+     Used for sorting info threads output.  */
+  char *cached_name;
+
   /* Non-zero means the thread is executing.  Note: this is different
      from saying that there is an active target and we are stopped at
      a breakpoint, for instance.  This is a real indicator whether the
@@ -385,7 +389,7 @@ extern struct cmd_list_element *thread_cmd_list;
    `set print thread-events'.  */
 extern int print_thread_events;
 
-extern void print_thread_info (struct ui_out *uiout, char *threads,
+extern void print_thread_info (struct ui_out *uiout, char *args,
 			       int pid);
 
 extern struct cleanup *make_cleanup_restore_current_thread (void);
diff --git a/gdb/testsuite/gdb.threads/Makefile.in b/gdb/testsuite/gdb.threads/Makefile.in
index 8f9f867..2d24ba4 100644
--- a/gdb/testsuite/gdb.threads/Makefile.in
+++ b/gdb/testsuite/gdb.threads/Makefile.in
@@ -5,7 +5,7 @@ EXECUTABLES = attach-into-signal-nothr attach-into-signal-thr \
 	attach-stopped attachstop-mt \
 	bp_in_thread current-lwp-dead execl execl1 fork-child-threads \
 	fork-thread-pending gcore-pthreads hand-call-in-threads \
-	ia64-sigill interrupted-hand-call killed linux-dp \
+	ia64-sigill info-thread-sort-regex interrupted-hand-call killed linux-dp \
 	local-watch-wrong-thread manythreads multi-create pending-step \
 	print-threads pthreads pthread_cond_wait schedlock sigthread \
 	staticthreads switch-threads thread-execl thread-specific \
diff --git a/gdb/testsuite/gdb.threads/info-thread-sort-regex.c b/gdb/testsuite/gdb.threads/info-thread-sort-regex.c
new file mode 100644
index 0000000..98979ce
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/info-thread-sort-regex.c
@@ -0,0 +1,122 @@
+/* Test case for info threads sort and regex matching.
+
+   Copyright 2008, 2010-2012 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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 <stdio.h>
+#include <stdlib.h>
+#include <time.h>
+#include <unistd.h>
+
+#ifndef NR_THREADS
+#define NR_THREADS 4
+#endif
+
+int thread_count;
+
+pthread_mutex_t thread_count_mutex;
+
+pthread_cond_t thread_count_condvar;
+
+void
+incr_thread_count (void)
+{
+  pthread_mutex_lock (&thread_count_mutex);
+  ++thread_count;
+  if (thread_count == NR_THREADS)
+    pthread_cond_signal (&thread_count_condvar);
+  pthread_mutex_unlock (&thread_count_mutex);
+}
+
+void
+cond_wait (pthread_cond_t *cond, pthread_mutex_t *mut)
+{
+  pthread_mutex_lock (mut);
+  pthread_cond_wait (cond, mut);
+  pthread_mutex_unlock (mut);
+}
+
+void
+noreturn (void)
+{
+  pthread_mutex_t mut;
+  pthread_cond_t cond;
+
+  pthread_mutex_init (&mut, NULL);
+  pthread_cond_init (&cond, NULL);
+
+  /* Wait for a condition that will never be signaled, so we effectively
+     block the thread here.  */
+  cond_wait (&cond, &mut);
+}
+
+void *
+forever_pthread (void *unused)
+{
+  incr_thread_count ();
+  noreturn ();
+}
+
+/* Wait until all threads are running.  */
+
+void
+wait_all_threads_running (void)
+{
+  pthread_mutex_lock (&thread_count_mutex);
+  if (thread_count == NR_THREADS)
+    {
+      pthread_mutex_unlock (&thread_count_mutex);
+      return;
+    }
+  pthread_cond_wait (&thread_count_condvar, &thread_count_mutex);
+  if (thread_count == NR_THREADS)
+    {
+      pthread_mutex_unlock (&thread_count_mutex);
+      return;
+    }
+  pthread_mutex_unlock (&thread_count_mutex);
+  printf ("failed waiting for all threads to start\n");
+  abort ();
+}
+
+/* Called when all threads are running.
+   Easy place for a breakpoint.  */
+
+void
+all_threads_running (void)
+{
+}
+
+int
+main (void)
+{
+  pthread_t forever[NR_THREADS];
+  int i;
+
+  pthread_mutex_init (&thread_count_mutex, NULL);
+  pthread_cond_init (&thread_count_condvar, NULL);
+
+  for (i = 0; i < NR_THREADS; ++i)
+    pthread_create (&forever[i], NULL, forever_pthread, NULL);
+
+  wait_all_threads_running ();
+  all_threads_running ();
+
+  return 0;
+}
+
diff --git a/gdb/testsuite/gdb.threads/info-thread-sort-regex.exp b/gdb/testsuite/gdb.threads/info-thread-sort-regex.exp
new file mode 100644
index 0000000..a49eedf
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/info-thread-sort-regex.exp
@@ -0,0 +1,58 @@
+# Copyright (C) 2004, 2007-2008, 2010-2012 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 sorting and regex matching in info threads.
+
+set NR_THREADS 4
+
+standard_testfile
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug "additional_flags=-DNR_THREADS=$NR_THREADS"]] != "" } {
+    return -1
+}
+
+clean_restart ${binfile}
+
+if { ![runto_main] } {
+    fail "Can't run to main"
+    return 0
+}
+
+gdb_test "break all_threads_running" \
+	 "Breakpoint 2 at .*: file .*${srcfile}, line .*" \
+	 "breakpoint on all_threads_running"
+
+# Run the program and make sure GDB reports that we stopped after
+# hitting breakpoint 2 in all_threads_running().
+
+gdb_test "continue" \
+	 ".*Breakpoint 2, all_threads_running ().*" \
+	 "run to all_threads_running"
+
+# Test that the no threads match message prints correctly.
+gdb_test "info threads -r \$foo" \
+	 "No threads match '\\\$foo'\." \
+	 "info thread no match test"
+
+gdb_test_no_output "thread name a" "set thread name to a"
+
+gdb_test "info threads -a" \
+   "\\s+Id\\s+Target\\s+Id\\s+Frame \r\n\\*\\\s+\\\d+\\\s+Thread\\\s+0x\[A-Fa-f0-9\]+ \\\(LWP \\\d+\\\) \"a\".*" \
+   "check thread a is first"
+
+gdb_test "info threads -r ^info" \
+   "\\s+Id\\s+Target\\s+Id\\s+Frame \r\n\\\s+\\\d+\\\s+Thread\\\s+0x\[A-Fa-f0-9\]+ \\\(LWP \\\d+\\\) \"info.*\".*" \
+   "filter out thread named a"
diff --git a/gdb/thread.c b/gdb/thread.c
index 7e8eec5..68b2dac 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -55,6 +55,7 @@ void _initialize_thread (void);
 /* Prototypes for local functions.  */
 
 struct thread_info *thread_list = NULL;
+static int thread_list_size = 0;
 static int highest_thread_num;
 
 static void thread_command (char *tidstr, int from_tty);
@@ -132,6 +133,7 @@ free_thread (struct thread_info *tp)
 	xfree (tp->private);
     }
 
+  thread_list_size--;
   xfree (tp->name);
   xfree (tp);
 }
@@ -153,6 +155,7 @@ init_thread_list (void)
     }
 
   thread_list = NULL;
+  thread_list_size = 0;
 }
 
 /* Allocate a new thread with target id PTID and add it to the thread
@@ -169,6 +172,7 @@ new_thread (ptid_t ptid)
   tp->num = ++highest_thread_num;
   tp->next = thread_list;
   thread_list = tp;
+  thread_list_size++;
 
   /* Nothing to follow yet.  */
   tp->pending_follow.kind = TARGET_WAITKIND_SPURIOUS;
@@ -748,25 +752,195 @@ finish_thread_state_cleanup (void *arg)
   finish_thread_state (*ptid_p);
 }
 
+/* Returns true if user requested the thread.  */
+
+static int
+print_thread_filter_check (struct thread_info *tp, char *requested_threads,
+			   int pid, regex_t *preg)
+{
+  if (preg)
+    {
+      int err = regexec (preg, tp->cached_name ? tp->cached_name : "",
+			 0, NULL, 0);
+      if (err == 0)
+	return TRUE;
+      else
+	return FALSE;
+    }
+
+  /* No regular expression was supplied, see if there's a match the list
+     of thread id's.  */
+  if (!number_is_in_list (requested_threads, tp->num))
+    return FALSE;
+
+  if (pid != -1 && PIDGET (tp->ptid) != pid)
+    {
+      if (requested_threads != NULL && *requested_threads != '\0')
+	error (_("Requested thread not found in requested process"));
+      return FALSE;
+    }
+
+  if (tp->state == THREAD_EXITED)
+    return FALSE;
+
+  return TRUE;
+}
+
+
+/* Used to Sort threads in lexicographic order by name.  */
+
+static int
+print_thread_sort_cmp (const void *p1, const void *p2)
+{
+  const char *name1 = (*(struct thread_info **) p1)->cached_name;
+  const char *name2 = (*(struct thread_info **) p2)->cached_name;
+  if (name1 && name2)
+    return strcmp (name1, name2);
+
+  /* In case we receive NULL pointers instead of strings.
+     NULL pointers are sorted as empty strings.  */
+  if (!name1 && name2)
+    return -1;
+  if (!name2 && name1)
+    return 1;
+
+  return 0;
+}
+
+/* Prints the info for a single thread.
+   Called by print_thread_info for each thread.  */
+
+static void
+print_thread_info_single (struct ui_out *uiout, struct thread_info *tp,
+			       ptid_t current_ptid)
+{
+  struct cleanup *uiout_cleanup
+    = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+  int core;
+  char *extra_info, *name, *target_id;
+
+  if (ui_out_is_mi_like_p (uiout))
+    {
+      /* Compatibility.  */
+      if (ptid_equal (tp->ptid, current_ptid))
+	ui_out_text (uiout, "* ");
+      else
+	ui_out_text (uiout, "  ");
+    }
+    else
+    {
+      if (ptid_equal (tp->ptid, current_ptid))
+	ui_out_field_string (uiout, "current", "*");
+      else
+	ui_out_field_skip (uiout, "current");
+    }
+
+  ui_out_field_int (uiout, "id", tp->num);
+
+  /* For the CLI, we stuff everything into the target-id field.
+     This is a gross hack to make the output come out looking
+     correct.  The underlying problem here is that ui-out has no
+     way to specify that a field's space allocation should be
+     shared by several fields.  For MI, we do the right thing
+     instead.  */
+
+  target_id = target_pid_to_str (tp->ptid);
+  extra_info = target_extra_thread_info (tp);
+  if (tp->cached_name)
+    name = tp->cached_name;
+  else
+    name = tp->name ? tp->name : target_thread_name (tp);
+
+  if (ui_out_is_mi_like_p (uiout))
+    {
+      ui_out_field_string (uiout, "target-id", target_id);
+      if (extra_info)
+	ui_out_field_string (uiout, "details", extra_info);
+      if (name)
+	ui_out_field_string (uiout, "name", name);
+    }
+  else
+    {
+      struct cleanup *str_cleanup;
+      char *contents;
+
+      if (extra_info && name)
+	contents = xstrprintf ("%s \"%s\" (%s)", target_id,
+			       name, extra_info);
+      else if (extra_info)
+	contents = xstrprintf ("%s (%s)", target_id, extra_info);
+      else if (name)
+	contents = xstrprintf ("%s \"%s\"", target_id, name);
+      else
+	contents = xstrdup (target_id);
+      str_cleanup = make_cleanup (xfree, contents);
+
+      ui_out_field_string (uiout, "target-id", contents);
+      do_cleanups (str_cleanup);
+    }
+
+  if (tp->state == THREAD_RUNNING)
+    ui_out_text (uiout, "(running)\n");
+  else
+    {
+      /* The switch below puts us at the top of the stack (leaf
+	 frame).  */
+      switch_to_thread (tp->ptid);
+      print_stack_frame (get_selected_frame (NULL),
+	  /* For MI output, print frame level.  */
+	  ui_out_is_mi_like_p (uiout),
+	  LOCATION);
+    }
+
+  if (ui_out_is_mi_like_p (uiout))
+    {
+      char *state = "stopped";
+
+      if (tp->state == THREAD_RUNNING)
+	state = "running";
+      ui_out_field_string (uiout, "state", state);
+    }
+
+  core = target_core_of_thread (tp->ptid);
+  if (ui_out_is_mi_like_p (uiout) && core != -1)
+    ui_out_field_int (uiout, "core", core);
+
+  do_cleanups (uiout_cleanup);
+}
+
+/* Caches the name that will be shown to the user for a thread.
+   We keep track of this for sorting purposes.  */
+
+static void
+thread_cache_name (struct thread_info *tp)
+{
+  /* Does not need to be freed, is only transient.  */
+  tp->cached_name = tp->name ? tp->name : target_thread_name (tp);
+}
+
 /* Prints the list of threads and their details on UIOUT.
    This is a version of 'info_threads_command' suitable for
    use from MI.
-   If REQUESTED_THREAD is not -1, it's the GDB id of the thread
-   that should be printed.  Otherwise, all threads are
-   printed.
-   If PID is not -1, only print threads from the process PID.
-   Otherwise, threads from all attached PIDs are printed.
-   If both REQUESTED_THREAD and PID are not -1, then the thread
-   is printed if it belongs to the specified process.  Otherwise,
-   an error is raised.  */
+   REQUESTED_THREADS is either a list of numeric thread Id's
+   or the character 'r' followed by a regular expression to
+   match with each thread's name. Any spaces between the 'r' and
+   the regular expression are ignored.
+   If neither is supplied then all threads are printed.
+   Threads are sorted by name unless we are sending to MI.  */
+
 void
-print_thread_info (struct ui_out *uiout, char *requested_threads, int pid)
+print_thread_info (struct ui_out *uiout, char *args, int pid)
 {
   struct thread_info *tp;
   ptid_t current_ptid;
   struct cleanup *old_chain;
-  char *extra_info, *name, *target_id;
   int current_thread = -1;
+  struct thread_info **threads = NULL;
+  int n_threads, i, ret;
+  int enable_sort = FALSE;
+  regex_t preg_buffer;
+  regex_t *preg = NULL;
+  char *requested_threads = args;
 
   update_thread_list ();
   current_ptid = inferior_ptid;
@@ -774,27 +948,75 @@ print_thread_info (struct ui_out *uiout, char *requested_threads, int pid)
   /* We'll be switching threads temporarily.  */
   old_chain = make_cleanup_restore_current_thread ();
 
+  /* Process argument string args for options.  */
+  args = skip_spaces (args);
+  while (args && *args != '\0')
+    {
+      if (*args != '-')
+	break;
+      if (args[1] == 'a')
+	{
+	  enable_sort = TRUE;
+	  args = skip_spaces (args + 2);
+	  requested_threads = args;
+	  continue;
+	}
+      if (args[1] == 'r')
+	{
+	  args = skip_spaces (&args[2]);
+	  ret = regcomp (&preg_buffer, args, REG_NOSUB);
+	  if (ret == 0)
+	    {
+	      preg = &preg_buffer;
+	      make_regfree_cleanup (preg);
+	    }
+	  else
+	    {
+	      char *err =  get_regcomp_error (ret, &preg_buffer);
+	      make_cleanup (xfree, err);
+	      error (_("Invalid regexp (%s): %s"), err, requested_threads);
+	    }
+	  break;
+	}
+	error (_("Bad option (%s)"), &args[1]);
+	break;
+    }
+
   /* For backward compatibility, we make a list for MI.  A table is
      preferable for the CLI, though, because it shows table
-     headers.  */
+     headers.
+     We do not sort the list for MI.  */
   if (ui_out_is_mi_like_p (uiout))
-    make_cleanup_ui_out_list_begin_end (uiout, "threads");
-  else
     {
-      int n_threads = 0;
+      make_cleanup_ui_out_list_begin_end (uiout, "threads");
 
       for (tp = thread_list; tp; tp = tp->next)
 	{
-	  if (!number_is_in_list (requested_threads, tp->num))
-	    continue;
+	  if (ptid_equal (tp->ptid, current_ptid))
+	    current_thread = tp->num;
+
+	  if (print_thread_filter_check (tp, requested_threads, pid, NULL))
+	    print_thread_info_single (uiout, tp, current_ptid);
+	}
+    }
+  else
+    {
+      threads = xmalloc (sizeof (*threads) * thread_list_size);
+      make_cleanup (free, threads);
 
-	  if (pid != -1 && PIDGET (tp->ptid) != pid)
-	    continue;
+      n_threads = 0;
+      for (i = 0, tp = thread_list; tp; tp = tp->next, ++i)
+	{
+	  thread_cache_name (tp);
+	  if (print_thread_filter_check (tp, requested_threads, pid, preg))
+	    {
+	      threads[n_threads] = tp;
+	      ++n_threads;
+	    }
 
-	  if (tp->state == THREAD_EXITED)
-	    continue;
+	  if (ptid_equal (tp->ptid, current_ptid))
+	    current_thread = tp->num;
 
-	  ++n_threads;
 	}
 
       if (n_threads == 0)
@@ -803,7 +1025,7 @@ print_thread_info (struct ui_out *uiout, char *requested_threads, int pid)
 	    ui_out_message (uiout, 0, _("No threads.\n"));
 	  else
 	    ui_out_message (uiout, 0, _("No threads match '%s'.\n"),
-			    requested_threads);
+					args);
 	  do_cleanups (old_chain);
 	  return;
 	}
@@ -815,115 +1037,15 @@ print_thread_info (struct ui_out *uiout, char *requested_threads, int pid)
       ui_out_table_header (uiout, 17, ui_left, "target-id", "Target Id");
       ui_out_table_header (uiout, 1, ui_left, "frame", "Frame");
       ui_out_table_body (uiout);
-    }
 
-  for (tp = thread_list; tp; tp = tp->next)
-    {
-      struct cleanup *chain2;
-      int core;
 
-      if (!number_is_in_list (requested_threads, tp->num))
-	continue;
+      if (enable_sort)
+	qsort (threads, n_threads, sizeof (*threads), print_thread_sort_cmp);
 
-      if (pid != -1 && PIDGET (tp->ptid) != pid)
+      for (i = 0; i < n_threads; i++)
 	{
-	  if (requested_threads != NULL && *requested_threads != '\0')
-	    error (_("Requested thread not found in requested process"));
-	  continue;
+	  print_thread_info_single (uiout, threads[i], current_ptid);
 	}
-
-      if (ptid_equal (tp->ptid, current_ptid))
-	current_thread = tp->num;
-
-      if (tp->state == THREAD_EXITED)
-	continue;
-
-      chain2 = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
-
-      if (ui_out_is_mi_like_p (uiout))
-	{
-	  /* Compatibility.  */
-	  if (ptid_equal (tp->ptid, current_ptid))
-	    ui_out_text (uiout, "* ");
-	  else
-	    ui_out_text (uiout, "  ");
-	}
-      else
-	{
-	  if (ptid_equal (tp->ptid, current_ptid))
-	    ui_out_field_string (uiout, "current", "*");
-	  else
-	    ui_out_field_skip (uiout, "current");
-	}
-
-      ui_out_field_int (uiout, "id", tp->num);
-
-      /* For the CLI, we stuff everything into the target-id field.
-	 This is a gross hack to make the output come out looking
-	 correct.  The underlying problem here is that ui-out has no
-	 way to specify that a field's space allocation should be
-	 shared by several fields.  For MI, we do the right thing
-	 instead.  */
-
-      target_id = target_pid_to_str (tp->ptid);
-      extra_info = target_extra_thread_info (tp);
-      name = tp->name ? tp->name : target_thread_name (tp);
-
-      if (ui_out_is_mi_like_p (uiout))
-	{
-	  ui_out_field_string (uiout, "target-id", target_id);
-	  if (extra_info)
-	    ui_out_field_string (uiout, "details", extra_info);
-	  if (name)
-	    ui_out_field_string (uiout, "name", name);
-	}
-      else
-	{
-	  struct cleanup *str_cleanup;
-	  char *contents;
-
-	  if (extra_info && name)
-	    contents = xstrprintf ("%s \"%s\" (%s)", target_id,
-				   name, extra_info);
-	  else if (extra_info)
-	    contents = xstrprintf ("%s (%s)", target_id, extra_info);
-	  else if (name)
-	    contents = xstrprintf ("%s \"%s\"", target_id, name);
-	  else
-	    contents = xstrdup (target_id);
-	  str_cleanup = make_cleanup (xfree, contents);
-
-	  ui_out_field_string (uiout, "target-id", contents);
-	  do_cleanups (str_cleanup);
-	}
-
-      if (tp->state == THREAD_RUNNING)
-	ui_out_text (uiout, "(running)\n");
-      else
-	{
-	  /* The switch below puts us at the top of the stack (leaf
-	     frame).  */
-	  switch_to_thread (tp->ptid);
-	  print_stack_frame (get_selected_frame (NULL),
-			     /* For MI output, print frame level.  */
-			     ui_out_is_mi_like_p (uiout),
-			     LOCATION);
-	}
-
-      if (ui_out_is_mi_like_p (uiout))
-	{
-	  char *state = "stopped";
-
-	  if (tp->state == THREAD_RUNNING)
-	    state = "running";
-	  ui_out_field_string (uiout, "state", state);
-	}
-
-      core = target_core_of_thread (tp->ptid);
-      if (ui_out_is_mi_like_p (uiout) && core != -1)
-	ui_out_field_int (uiout, "core", core);
-
-      do_cleanups (chain2);
     }
 
   /* Restores the current thread and the frame selected before
-- 
1.7.7.3

Re: [patch] info threads sort by name and name regex matching

[Tom Tromey]

>>>>> "Aaron" == Aaron Gamble <agamble@google.com> writes:

Aaron> -a enables sorting by thread name

Is '-a' mnemonic for something?

Aaron> +static int
Aaron> +print_thread_sort_cmp (const void *p1, const void *p2)
Aaron> +{
Aaron> +  const char *name1 = (*(struct thread_info **) p1)->cached_name;
Aaron> +  const char *name2 = (*(struct thread_info **) p2)->cached_name;
Aaron> +  if (name1 && name2)
Aaron> +    return strcmp (name1, name2);

The gdb style is to have a blank line between declarations and code.

Aaron> +static void
Aaron> +thread_cache_name (struct thread_info *tp)
Aaron> +{
Aaron> +  /* Does not need to be freed, is only transient.  */
Aaron> +  tp->cached_name = tp->name ? tp->name : target_thread_name (tp);

I realized while re-reading the patch that this isn't safe.
target_thread_name has a funny contract, as you note, where it returns
static data.  But this means that the result can't be cached --
otherwise all threads with non-NULL names will end up with the same
cached name.

Either you have to copy the name here or change target_thread_name to do so;
and then make sure to free at the right spots, etc.

Tom

Re: [patch] info threads sort by name and name regex matching

[Aaron Gamble]

On Fri, Aug 24, 2012 at 10:57 AM, Tom Tromey <tromey@redhat.com> wrote:
>
>>>>>> "Aaron" == Aaron Gamble <agamble@google.com> writes:
>
> Aaron> -a enables sorting by thread name
>
> Is '-a' mnemonic for something?
-a is for alphabetical. Would -s be more appropriate?


> Aaron> +static int
> Aaron> +print_thread_sort_cmp (const void *p1, const void *p2)
> Aaron> +{
> Aaron> +  const char *name1 = (*(struct thread_info **) p1)->cached_name;
> Aaron> +  const char *name2 = (*(struct thread_info **) p2)->cached_name;
> Aaron> +  if (name1 && name2)
> Aaron> +    return strcmp (name1, name2);
>
> The gdb style is to have a blank line between declarations and code.
Ack.


> Aaron> +static void
> Aaron> +thread_cache_name (struct thread_info *tp)
> Aaron> +{
> Aaron> +  /* Does not need to be freed, is only transient.  */
> Aaron> +  tp->cached_name = tp->name ? tp->name : target_thread_name (tp);
>
> I realized while re-reading the patch that this isn't safe.
> target_thread_name has a funny contract, as you note, where it returns
> static data.  But this means that the result can't be cached --
> otherwise all threads with non-NULL names will end up with the same
> cached name.
>
> Either you have to copy the name here or change target_thread_name to do so;
> and then make sure to free at the right spots, etc.
Would it be safe to use target_thread_name in print_thread_sort_cmp
each time the name is used in the sort function? I'm not entirely sure
why it's unsafe to store the NULL pointers when the name could not be
determined.

Re: [patch] info threads sort by name and name regex matching

[Sergio Durigan Junior]

On Friday, August 24 2012, Aaron Gamble wrote:

> On Fri, Aug 24, 2012 at 10:57 AM, Tom Tromey <tromey@redhat.com> wrote:
>>
>>>>>>> "Aaron" == Aaron Gamble <agamble@google.com> writes:
>>
>> Aaron> -a enables sorting by thread name
>>
>> Is '-a' mnemonic for something?
> -a is for alphabetical. Would -s be more appropriate?

I prefer `-a' FWIW.  I could understand that it meant `alphabetical'
since the beginning, and I could not find any convention for this on GNU
manuals.  BTW, `-s' is for `string'?  This time I didn't understand it :-).

-- 
Sergio

Re: [patch] info threads sort by name and name regex matching

[Aaron Gamble]

-s would be for 'sorted'

On Fri, Aug 24, 2012 at 3:31 PM, Sergio Durigan Junior
<sergiodj@redhat.com> wrote:
>
> On Friday, August 24 2012, Aaron Gamble wrote:
>
>> On Fri, Aug 24, 2012 at 10:57 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>
>>>>>>>> "Aaron" == Aaron Gamble <agamble@google.com> writes:
>>>
>>> Aaron> -a enables sorting by thread name
>>>
>>> Is '-a' mnemonic for something?
>> -a is for alphabetical. Would -s be more appropriate?
>
> I prefer `-a' FWIW.  I could understand that it meant `alphabetical'
> since the beginning, and I could not find any convention for this on GNU
> manuals.  BTW, `-s' is for `string'?  This time I didn't understand it :-).
>
> --
> Sergio

Re: [patch] info threads sort by name and name regex matching

[Aaron Gamble]

-s would be for 'sorted'

On Fri, Aug 24, 2012 at 3:31 PM, Sergio Durigan Junior
<sergiodj@redhat.com> wrote:
>
> On Friday, August 24 2012, Aaron Gamble wrote:
>
>> On Fri, Aug 24, 2012 at 10:57 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>
>>>>>>>> "Aaron" == Aaron Gamble <agamble@google.com> writes:
>>>
>>> Aaron> -a enables sorting by thread name
>>>
>>> Is '-a' mnemonic for something?
>> -a is for alphabetical. Would -s be more appropriate?
>
> I prefer `-a' FWIW.  I could understand that it meant `alphabetical'
> since the beginning, and I could not find any convention for this on GNU
> manuals.  BTW, `-s' is for `string'?  This time I didn't understand it :-).
>
> --
> Sergio

Re: [patch] info threads sort by name and name regex matching

[Sergio Durigan Junior]

On Friday, August 24 2012, Aaron Gamble wrote:

> -s would be for 'sorted'

I don't want to make this a big thread on a small thing, but `-s' or
`sorted' does not specify what is the criteria for sorting.

Anyway, although I prefer `-a', either of them would work as long as it
is documented.

-- 
Sergio