[RFA] improved thread id reporting

[RFA] improved thread id reporting

[Doug Evans]

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

Blech.  Right list this time.


On Sun, Apr 5, 2009 at 8:33 PM, Daniel Jacobowitz <drow@false.org> wrote:
> On Sun, Apr 05, 2009 at 01:36:06PM -0700, Doug Evans wrote:
>> Attached is a simplistic patch to help illustrate the challenge.
>>
>> Here is an example session that prints from/to and the thread number
>> in "[New ...", "[Switching ...", etc. messages.
>> I can think of two issues with the patch:
>> 1) Printing "[tT]hread" twice in one line is a bit annoying.
>> 2) Spreading from/to over two lines is a bit annoying.
>
> What do you think of this?  On the theory that you can go look up
> thread #2, either in 'info threads' or in a previous notification:
>
> [Switching from thread #2 to thread #3, Thread 0x41001960 (LWP 14407)]

"works for me"

> Or, migrating the "Thread" out:
>
> [Switching from thread #2 to thread #3, 0x41001960 (LWP 14407)]
>
> But that might be tricky with multi-process, some ptid_t's are not
> threads.

It's not clear how multi-process is going to work yet (or more likely
I've forgotten).  I played with attaching and running several
processes via gdbserver and all processes appear in "info threads".
[sidebar: I wouldn't mind "info threads" just showing the threads of
the current process, otherwise it might get confusing.  And given that
"info inferiors" is used to show all the processes IWBN if "inferior
N" switched to the specified process; a straightforward and intuitive
mapping from "info threads" + "thread N".]

How about this?

[-- Attachment #2: gdb-090507-thread-id-1.patch.txt --]
[-- Type: text/plain, Size: 4857 bytes --]

2009-05-07  Doug Evans  <dje@google.com>

	* infrun.c (normal_stop): Print gdb thread number of thread being
	switched away from and to.
	* inf-ttrace.c (inf_ttrace_wait): Include gdb thread number in
	thread event messages.
	* linux-nat.c (exit_lwp): Ditto.
	* procfs.c (procfs_wait): Ditto.
	* thread.c (add_thread_with_info): Ditto.

Index: inf-ttrace.c
===================================================================
RCS file: /cvs/src/src/gdb/inf-ttrace.c,v
retrieving revision 1.44
diff -u -p -r1.44 inf-ttrace.c
--- inf-ttrace.c	17 Mar 2009 19:28:09 -0000	1.44
+++ inf-ttrace.c	8 May 2009 04:53:39 -0000
@@ -1081,10 +1081,11 @@ inf_ttrace_wait (struct target_ops *ops,
       return ptid;
 
     case TTEVT_LWP_EXIT:
-      if (print_thread_events)
-	printf_unfiltered (_("[%s exited]\n"), target_pid_to_str (ptid));
       ti = find_thread_pid (ptid);
       gdb_assert (ti != NULL);
+      if (print_thread_events)
+	printf_unfiltered (_("[Thread #%d, %s exited]\n"),
+			   ti->num, target_pid_to_str (ptid));
       ((struct inf_ttrace_private_thread_info *)ti->private)->dying = 1;
       inf_ttrace_num_lwps--;
       /* Let the thread really exit.  */
@@ -1097,11 +1098,11 @@ inf_ttrace_wait (struct target_ops *ops,
     case TTEVT_LWP_TERMINATE:
       lwpid = tts.tts_u.tts_thread.tts_target_lwpid;
       ptid = ptid_build (tts.tts_pid, lwpid, 0);
-      if (print_thread_events)
-	printf_unfiltered(_("[%s has been terminated]\n"),
-			  target_pid_to_str (ptid));
       ti = find_thread_pid (ptid);
       gdb_assert (ti != NULL);
+      if (print_thread_events)
+	printf_unfiltered (_("[Thread #%d, %s has been terminated]\n"),
+			   ti->num, target_pid_to_str (ptid));
       ((struct inf_ttrace_private_thread_info *)ti->private)->dying = 1;
       inf_ttrace_num_lwps--;
 
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.371
diff -u -p -r1.371 infrun.c
--- infrun.c	5 May 2009 13:24:48 -0000	1.371
+++ infrun.c	8 May 2009 04:53:39 -0000
@@ -4299,7 +4299,11 @@ normal_stop (void)
       && last.kind != TARGET_WAITKIND_EXITED)
     {
       target_terminal_ours_for_output ();
-      printf_filtered (_("[Switching to %s]\n"),
+      /* It is useful to the user to print the gdb thread numbers we're
+	 switching away from and to.  */
+      printf_filtered (_("[Switching from thread #%d to thread #%d, %s]\n"),
+		       pid_to_thread_id (previous_inferior_ptid),
+		       pid_to_thread_id (inferior_ptid),
 		       target_pid_to_str (inferior_ptid));
       annotate_thread_changed ();
       previous_inferior_ptid = inferior_ptid;
Index: linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-nat.c,v
retrieving revision 1.128
diff -u -p -r1.128 linux-nat.c
--- linux-nat.c	1 Apr 2009 19:07:50 -0000	1.128
+++ linux-nat.c	8 May 2009 04:53:40 -0000
@@ -1159,7 +1159,8 @@ exit_lwp (struct lwp_info *lp)
   if (th)
     {
       if (print_thread_events)
-	printf_unfiltered (_("[%s exited]\n"), target_pid_to_str (lp->ptid));
+	printf_unfiltered (_("[Thread #%d, %s exited]\n"),
+			   th->num, target_pid_to_str (lp->ptid));
 
       delete_thread (lp->ptid);
     }
Index: procfs.c
===================================================================
RCS file: /cvs/src/src/gdb/procfs.c,v
retrieving revision 1.106
diff -u -p -r1.106 procfs.c
--- procfs.c	6 May 2009 17:18:52 -0000	1.106
+++ procfs.c	8 May 2009 04:53:40 -0000
@@ -4046,7 +4046,8 @@ wait_again:
 		if (syscall_is_lwp_exit (pi, what))
 		  {
 		    if (print_thread_events)
-		      printf_unfiltered (_("[%s exited]\n"),
+		      printf_unfiltered (_("[Thread #%d, %s exited]\n"),
+					 pid_to_thread_id (retval),
 					 target_pid_to_str (retval));
 		    delete_thread (retval);
 		    status->kind = TARGET_WAITKIND_SPURIOUS;
@@ -4182,7 +4183,8 @@ wait_again:
 		else if (syscall_is_lwp_exit (pi, what))
 		  {
 		    if (print_thread_events)
-		      printf_unfiltered (_("[%s exited]\n"),
+		      printf_unfiltered (_("[Thread #%d, %s exited]\n"),
+					 pid_to_thread_id (retval),
 					 target_pid_to_str (retval));
 		    delete_thread (retval);
 		    status->kind = TARGET_WAITKIND_SPURIOUS;
Index: thread.c
===================================================================
RCS file: /cvs/src/src/gdb/thread.c,v
retrieving revision 1.110
diff -u -p -r1.110 thread.c
--- thread.c	5 May 2009 13:24:49 -0000	1.110
+++ thread.c	8 May 2009 04:53:40 -0000
@@ -211,7 +211,8 @@ add_thread_with_info (ptid_t ptid, struc
   result->private = private;
 
   if (print_thread_events)
-    printf_unfiltered (_("[New %s]\n"), target_pid_to_str (ptid));
+    printf_unfiltered (_("[New thread #%d, %s]\n"),
+		       result->num, target_pid_to_str (ptid));
 
   annotate_new_thread ();
   return result;

Re: [RFA] improved thread id reporting

[Doug Evans]

Ping.

Ok to check in?


On Thu, May 7, 2009 at 10:46 PM, Doug Evans <dje@google.com> wrote:
> Blech.  Right list this time.
>
>
> On Sun, Apr 5, 2009 at 8:33 PM, Daniel Jacobowitz <drow@false.org> wrote:
>> On Sun, Apr 05, 2009 at 01:36:06PM -0700, Doug Evans wrote:
>>> Attached is a simplistic patch to help illustrate the challenge.
>>>
>>> Here is an example session that prints from/to and the thread number
>>> in "[New ...", "[Switching ...", etc. messages.
>>> I can think of two issues with the patch:
>>> 1) Printing "[tT]hread" twice in one line is a bit annoying.
>>> 2) Spreading from/to over two lines is a bit annoying.
>>
>> What do you think of this?  On the theory that you can go look up
>> thread #2, either in 'info threads' or in a previous notification:
>>
>> [Switching from thread #2 to thread #3, Thread 0x41001960 (LWP 14407)]
>
> "works for me"
>
>> Or, migrating the "Thread" out:
>>
>> [Switching from thread #2 to thread #3, 0x41001960 (LWP 14407)]
>>
>> But that might be tricky with multi-process, some ptid_t's are not
>> threads.
>
> It's not clear how multi-process is going to work yet (or more likely
> I've forgotten).  I played with attaching and running several
> processes via gdbserver and all processes appear in "info threads".
> [sidebar: I wouldn't mind "info threads" just showing the threads of
> the current process, otherwise it might get confusing.  And given that
> "info inferiors" is used to show all the processes IWBN if "inferior
> N" switched to the specified process; a straightforward and intuitive
> mapping from "info threads" + "thread N".]
>
> How about this?
>

Re: [RFA] improved thread id reporting

[Pedro Alves]

On Tuesday 19 May 2009 00:23:15, Doug Evans wrote:
> Ping.
> 
> Ok to check in?

I haven't convinced myself yet that your normal_stop change always
does what you want.  Won't it print:

[Switching from thread #0 to thread #3, 0x41001960 (LWP 14407)]
                        ^

if the previosly selected thread happens to exit in a series
of misfortunate events?  Say, 

thread 1
break foo thread 2
continue
<thread 3 hits breakpoint at foo, gdb thread hops, having switched inferior_ptid to thread 3>
<thread 1 exits, exit_lwp calls delete_thread>
<thread 2 hits breakpoint at foo, call normal_stop, but previous_inferior_ptid is not in the thread list anymore>

> 
> 
> On Thu, May 7, 2009 at 10:46 PM, Doug Evans <dje@google.com> wrote:
> > Blech.  Right list this time.
> >
> >
> > On Sun, Apr 5, 2009 at 8:33 PM, Daniel Jacobowitz <drow@false.org> wrote:
> >> On Sun, Apr 05, 2009 at 01:36:06PM -0700, Doug Evans wrote:
> >>> Attached is a simplistic patch to help illustrate the challenge.
> >>>
> >>> Here is an example session that prints from/to and the thread number
> >>> in "[New ...", "[Switching ...", etc. messages.
> >>> I can think of two issues with the patch:
> >>> 1) Printing "[tT]hread" twice in one line is a bit annoying.
> >>> 2) Spreading from/to over two lines is a bit annoying.
> >>
> >> What do you think of this?  On the theory that you can go look up
> >> thread #2, either in 'info threads' or in a previous notification:
> >>
> >> [Switching from thread #2 to thread #3, Thread 0x41001960 (LWP 14407)]
> >
> > "works for me"
> >
> >> Or, migrating the "Thread" out:
> >>
> >> [Switching from thread #2 to thread #3, 0x41001960 (LWP 14407)]
> >>
> >> But that might be tricky with multi-process, some ptid_t's are not
> >> threads.
> >
> > It's not clear how multi-process is going to work yet (or more likely
> > I've forgotten).  I played with attaching and running several
> > processes via gdbserver and all processes appear in "info threads".
> > [sidebar: I wouldn't mind "info threads" just showing the threads of
> > the current process, otherwise it might get confusing.  And given that
> > "info inferiors" is used to show all the processes IWBN if "inferior
> > N" switched to the specified process; a straightforward and intuitive
> > mapping from "info threads" + "thread N".]
> >
> > How about this?
> >
> 



-- 
Pedro Alves

Re: [RFA] improved thread id reporting

[Pedro Alves]

On Tuesday 19 May 2009 04:29:38, Pedro Alves wrote:
> On Tuesday 19 May 2009 00:23:15, Doug Evans wrote:
> > Ping.
> > 
> > Ok to check in?
> 
> I haven't convinced myself yet that your normal_stop change always
> does what you want.  Won't it print:
> 
> [Switching from thread #0 to thread #3, 0x41001960 (LWP 14407)]
>                         ^
> 
> if the previosly selected thread happens to exit in a series
> of misfortunate events?  Say, 

Following a fork is also a case where the previously thread
may be long gone from the thread list when you get here.  Note that
I contructed the contrived case below, because delete_thread
refuses to delete inferior_ptid from the thread list.  There are
several options here: make a better effort at storing the previous
thread id; printing something about previous thread having exited;
or (ugly?) documenting this #0 thing.  I do have my reservations
about being always possible to print a sensible previous thread id.

> 
> thread 1
> break foo thread 2
> continue
> <thread 3 hits breakpoint at foo, gdb thread hops, having switched inferior_ptid to thread 3>
> <thread 1 exits, exit_lwp calls delete_thread>
> <thread 2 hits breakpoint at foo, call normal_stop, but previous_inferior_ptid is not in the thread list anymore>
> 
> > 
> > 
> > On Thu, May 7, 2009 at 10:46 PM, Doug Evans <dje@google.com> wrote:
> > > Blech.  Right list this time.
> > >
> > >
> > > On Sun, Apr 5, 2009 at 8:33 PM, Daniel Jacobowitz <drow@false.org> wrote:
> > >> On Sun, Apr 05, 2009 at 01:36:06PM -0700, Doug Evans wrote:
> > >>> Attached is a simplistic patch to help illustrate the challenge.
> > >>>
> > >>> Here is an example session that prints from/to and the thread number
> > >>> in "[New ...", "[Switching ...", etc. messages.
> > >>> I can think of two issues with the patch:
> > >>> 1) Printing "[tT]hread" twice in one line is a bit annoying.
> > >>> 2) Spreading from/to over two lines is a bit annoying.
> > >>
> > >> What do you think of this?  On the theory that you can go look up
> > >> thread #2, either in 'info threads' or in a previous notification:
> > >>
> > >> [Switching from thread #2 to thread #3, Thread 0x41001960 (LWP 14407)]
> > >
> > > "works for me"
> > >
> > >> Or, migrating the "Thread" out:
> > >>
> > >> [Switching from thread #2 to thread #3, 0x41001960 (LWP 14407)]
> > >>
> > >> But that might be tricky with multi-process, some ptid_t's are not
> > >> threads.
> > >
> > > It's not clear how multi-process is going to work yet (or more likely
> > > I've forgotten).  I played with attaching and running several
> > > processes via gdbserver and all processes appear in "info threads".
> > > [sidebar: I wouldn't mind "info threads" just showing the threads of
> > > the current process, otherwise it might get confusing.  And given that
> > > "info inferiors" is used to show all the processes IWBN if "inferior
> > > N" switched to the specified process; a straightforward and intuitive
> > > mapping from "info threads" + "thread N".]
> > >
> > > How about this?
> > >
> > 
> 
> 
> 



-- 
Pedro Alves