[rfc][patch] Allow GDB to display user-defined thread names

[rfc][patch] Allow GDB to display user-defined thread names

[Karen Osmond]

Hello!

When debugging programs with posix threads, it often makes me sad that the 
threads don't have any meaningful name associated with them.  I use a 
workaround involving a new gdb command for setting a thread name together 
with a script for use with the "-x" option.  The workaround should be 
generic enough to be useful to other people too, so I'm hoping the patch 
might be considered for inclusion.

The new gdb command takes the form "thread name NAME" and associates NAME 
with the current thread, so you can issue something like:

(gdb) thread 3
(gdb) thread name "producer"
etc

Then anytime gdb needs to identify the thread to the user, it can be 
supplemented with the name, e.g. on a thread info:
  3 Thread ["producer"] 0xb678cb90 (LWP 10286)  0x00a46416 in __kernel_vsyscall ()
  2 Thread ["consumer"] 0xb718db90 (LWP 10285)  0x00a46416 in __kernel_vsyscall ()
* 1 Thread ["main"] 0x7f2b9d0 (LWP 10282)  0x00a46416 in __kernel_vsyscall ()

It's not much fun to name the threads by hand, but it can be automated 
with a script, so long as you know a place to set a breakpoint that is 
unique to the thread.  For example, to name "main":

tbreak main
commands
silent
thread name "main"
c
end

In the codebase I debug most often, thread creation is wrapped for 
portability, and the thread name is available in the code itself at a 
thread's entrypoint, for use with Windows threads.  So my script looks a 
bit like the following:

break Thread::ThreadProc
commands
silent
thread name ((ThreadProcData*)pParam)->name
c
end

I attach a patch for the "thread name" command functionality.  Despite its 
small size, I know it will need improvement - at very least I doubt it is 
good style to expose language / valprint stuff in thread.c.

There are also a few caveats / potential improvements:
 - Thread names aren't known at the point gdb issues a "New thread..." 
notification, and initial "Switching to...".
 - There is a slight problem with pagination on, since the 
prompt_for_continue prompt will appear after several threads are created.  
I think this happens without my patch, and is caused by the "Switching..." 
messages that happen on the automated "continue".
 - It would be good to get this working with MI.
 - Maybe the script could be done better with python - I haven't yet tried 
the python stuff in gdb.

So, if this is of interest, I'd like to spend the time to improve it but 
if anybody has time, it would be of great help if a gdb bod could 
volunteer to mentor me a bit... please? :)

cheers,
-karen

Index: gdb/gdbthread.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbthread.h,v
retrieving revision 1.49
diff -c -p -r1.49 gdbthread.h
*** gdb/gdbthread.h	24 May 2009 21:06:53 -0000	1.49
--- gdb/gdbthread.h	4 Jun 2009 16:32:14 -0000
*************** struct thread_info
*** 180,185 ****
--- 180,188 ----
    /* True if this thread has been explicitly requested to stop.  */
    int stop_requested;
  
+   /* Thread name as optionally set by user command. */
+   char *name;
+ 
    /* Private data used by the target vector implementation.  */
    struct private_thread_info *private;
  };
Index: gdb/linux-thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-thread-db.c,v
retrieving revision 1.60
diff -c -p -r1.60 linux-thread-db.c
*** gdb/linux-thread-db.c	24 May 2009 21:06:53 -0000	1.60
--- gdb/linux-thread-db.c	4 Jun 2009 16:32:17 -0000
*************** thread_db_pid_to_str (struct target_ops 
*** 1373,1380 ****
        thread_t tid;
  
        tid = thread_info->private->tid;
!       snprintf (buf, sizeof (buf), "Thread 0x%lx (LWP %ld)",
! 		tid, GET_LWP (ptid));
  
        return buf;
      }
--- 1373,1384 ----
        thread_t tid;
  
        tid = thread_info->private->tid;
!       if (thread_info->name == NULL)
!         snprintf (buf, sizeof (buf), "Thread 0x%lx (LWP %ld)",
!                   tid, GET_LWP (ptid));
!       else
!         snprintf (buf, sizeof (buf), "Thread [%s] 0x%lx (LWP %ld)", 
!                   thread_info->name, tid, GET_LWP (ptid));
  
        return buf;
      }
Index: gdb/thread.c
===================================================================
RCS file: /cvs/src/src/gdb/thread.c,v
retrieving revision 1.113
diff -c -p -r1.113 thread.c
*** gdb/thread.c	24 May 2009 21:06:53 -0000	1.113
--- gdb/thread.c	4 Jun 2009 16:32:17 -0000
***************
*** 35,40 ****
--- 35,42 ----
  #include "regcache.h"
  #include "gdb.h"
  #include "gdb_string.h"
+ #include "language.h"
+ #include "valprint.h"
  
  #include <ctype.h>
  #include <sys/types.h>
*************** static int highest_thread_num;
*** 57,62 ****
--- 59,65 ----
  
  static void thread_command (char *tidstr, int from_tty);
  static void thread_apply_all_command (char *, int);
+ static void thread_name_command (char *, int);
  static int thread_alive (struct thread_info *);
  static void info_threads_command (char *, int);
  static void thread_apply_command (char *, int);
*************** free_thread (struct thread_info *tp)
*** 114,119 ****
--- 117,124 ----
  {
    clear_thread_inferior_resources (tp);
  
+   xfree (tp->name);
+ 
    /* FIXME: do I ever need to call the back-end to give it a
       chance at this private data before deleting the thread?  */
    if (tp->private)
*************** thread_apply_all_command (char *cmd, int
*** 1059,1064 ****
--- 1064,1095 ----
  }
  
  static void
+ thread_name_command (char *arg, int from_tty)
+ {
+   struct value *val;
+   struct ui_file *mem_file;
+   char *str;
+   long len;
+   struct thread_info *tp;
+   struct value_print_options opts;
+ 
+   tp = find_thread_ptid (inferior_ptid);
+ 
+   get_raw_print_options (&opts);
+   opts.addressprint = 0;
+ 
+   val = parse_and_eval (arg);
+ 
+   mem_file = mem_fileopen ();
+   LA_VALUE_PRINT (val, mem_file, &opts);
+   str = ui_file_xstrdup (mem_file, &len);
+   ui_file_delete (mem_file);
+ 
+   xfree (tp->name);
+   tp->name = str;
+ }
+ 
+ static void
  thread_apply_command (char *tidlist, int from_tty)
  {
    char *cmd;
*************** The new thread ID must be currently know
*** 1242,1247 ****
--- 1273,1282 ----
  		  _("Apply a command to a list of threads."),
  		  &thread_apply_list, "thread apply ", 1, &thread_cmd_list);
  
+   add_cmd ("name", class_run, thread_name_command,
+            _("Name current thread."),
+            &thread_cmd_list);
+ 
    add_cmd ("all", class_run, thread_apply_all_command,
  	   _("Apply a command to all threads."), &thread_apply_list);
  

Re: [rfc][patch] Allow GDB to display user-defined thread names

[J.T. Conklin]

Karen Osmond <karen.osmond@gmail.com> writes:
> So, if this is of interest, I'd like to spend the time to improve it but 
> if anybody has time, it would be of great help if a gdb bod could 
> volunteer to mentor me a bit... please? :)

Some systems have names that can be set programmatically w/
pthread_attr_setname() or pthread_attr_setname_np().  On such systems,
it might be useful for GDB's initial thread name be inherited from the
target.

    --jtc

-- 
J.T. Conklin

Re: [rfc][patch] Allow GDB to display user-defined thread names

[Tom Tromey]

>>>>> "Karen" == Karen Osmond <karen.osmond@gmail.com> writes:

Karen> When debugging programs with posix threads, it often makes me
Karen> sad that the threads don't have any meaningful name associated
Karen> with them.

GDB 7.0, now with more happiness ;)

Karen> The new gdb command takes the form "thread name NAME" and
Karen> associates NAME with the current thread, so you can issue
Karen> something like:

It seems like a nice idea to me.  As J.T. says, it would be nice to
have this automatically work for targets that define a thread name --
but I think it is ok to build up the feature piecemeal.

Karen> I attach a patch for the "thread name" command functionality.
Karen> Despite its small size, I know it will need improvement - at
Karen> very least I doubt it is good style to expose language /
Karen> valprint stuff in thread.c.

Actually, I don't think that is such a big deal.  The implementation
of user commands often needs this functionality.

Karen>  - Thread names aren't known at the point gdb issues a "New thread..." 
Karen> notification, and initial "Switching to...".

This could be handled with J.T.'s idea, or...

Karen>  - Maybe the script could be done better with python - I
Karen> haven't yet tried the python stuff in gdb.

... a python hook run when a thread is created.

Karen>  - It would be good to get this working with MI.

I think this should not be too difficult.

Karen> So, if this is of interest, I'd like to spend the time to improve it but 
Karen> if anybody has time, it would be of great help if a gdb bod could 
Karen> volunteer to mentor me a bit... please? :)

Sure.

Karen> !       if (thread_info->name == NULL)
Karen> !         snprintf (buf, sizeof (buf), "Thread 0x%lx (LWP %ld)",
Karen> !                   tid, GET_LWP (ptid));
Karen> !       else
Karen> !         snprintf (buf, sizeof (buf), "Thread [%s] 0x%lx (LWP %ld)", 
Karen> !                   thread_info->name, tid, GET_LWP (ptid));

I think you will at least want to make 'buf' bigger.  But, probably
you want to dynamically allocate instead.

Actually, it seems to me that the name-printing should be done by some
generic code, not in linux-thread-db.c.

One important missing piece is an update to the manual.
All new user commands require documentation.

Otherwise I think it looks pretty reasonable.

Tom

Re: [rfc][patch] Allow GDB to display user-defined thread names

[Paul Pluzhnikov]

On Fri, Jun 5, 2009 at 3:29 PM, Tom Tromey <tromey@redhat.com> wrote:

> Actually, it seems to me that the name-printing should be done by some
> generic code, not in linux-thread-db.c.

Here is a head of previous thread on this subject with some possibly useful
pointers: http://sourceware.org/ml/archer/2008-q4/msg00534.html

-- 
Paul Pluzhnikov

Re: [rfc][patch] Allow GDB to display user-defined thread names

[Karen Osmond]

On Fri, 5 Jun 2009, Tom Tromey wrote:
> >>>>> "Karen" == Karen Osmond <karen.osmond@gmail.com> writes:
> Karen> The new gdb command takes the form "thread name NAME" and
> Karen> associates NAME with the current thread, so you can issue
> Karen> something like:
> 
> It seems like a nice idea to me.  As J.T. says, it would be nice to
> have this automatically work for targets that define a thread name --

Yup, I'll try to find out a bit more about those posix thread extensions.  
Also, I think the Windows way involves throwing a particular exception to 
get the thread name into the debugger, and it looks like gdb doesn't 
support that currently (though i only had a very cursory browse for it).

> but I think it is ok to build up the feature piecemeal.

I'll leave the above and python and MI for now...

> Karen> So, if this is of interest, I'd like to spend the time to improve it but 
> Karen> if anybody has time, it would be of great help if a gdb bod could 
> Karen> volunteer to mentor me a bit... please? :)
> 
> Sure.

Wonderful, thank you!

> Actually, it seems to me that the name-printing should be done by some
> generic code, not in linux-thread-db.c.

Yes, true... and stepping back a level, adding it to target_pid_to_str 
seems wrong also.  Perhaps it is best to make a new function to print the 
thread's id, name and pid_to_str stuff and to call that from the 
appropriate places in thread.c, such as after the thread id in "thread 
info" output.  I think it is also helpful in a few other places like 
[Switching to...] and [New...] and [... exited].  Those don't currently 
include the thread id, but perhaps it makes sense to add that as well (I 
spotted a FIXME-implementors in the manual to that effect, at least for 
[New...]).
 
> One important missing piece is an update to the manual.
> All new user commands require documentation.

I'll make sure to include that in the next patch.

cheers,
-karen