Time to expand "Program received signal" ?

[Pedro Alves <palves at redhat dot com>]

Part of the confusion on <http://sourceware.org/ml/gdb/2012-11/msg00005.html>
can be attributed to the fact that:

  Program received signal SIGUSR1, User defined signal 1.

  Program received signal SIGUSR1, User defined signal 1.

  Program received signal SIGUSR1, User defined signal 1.

  Program received signal SIGUSR1, User defined signal 1.
  33        tgkill (getpid (), gettid (), SIGUSR1);       /* step-2 */

doesn't give any clue that these were different threads getting the
SIGUSR1 signal.

I had chickened out before in changing this output, but today
I've decided to try raising the issue.  :-)

So today, for signals, we have:

  Program received signal SIGUSR1, User defined signal 1.

and for "interrupt" (GDB_SIGNAL_0), we get:

  [Thread 0x7ffff7fd0700 (LWP 15209)] #2 stopped.


A patch like the below would result in:

  Thread 2 [Thread 0x7ffff7fcf700 (LWP 12023) "sigstep-threads"] received signal SIGUSR1, User defined signal 1.

  Thread 1 [Thread 0x7ffff7fd0740 (LWP 12019) "sigstep-threads"] received signal SIGUSR1, User defined signal 1.

And (for "interrupt"):

  Thread 2 [Thread 0x7ffff7fd0700 (LWP 12077) "threads"] stopped.


"Thread" appears twice on this target, but on others it wouldn't.
The second "Thread " is responsability of target_pid_to_str
(the whole "Thread 0x7ffff7fcf700 (LWP 12023)" in fact), while
the first is hard coded.  E.g., with some targets, we might see:

  Thread 1 [process PID] received signal SIGUSR1, User defined signal 1.
or:
  Thread 1 [<main task>] received signal SIGUSR1, User defined signal 1.

and if there's target thread extra info, we could see:

  Thread 1 [Thread 0x7ffff7fd0740 (LWP 12019) "sigstep-threads" (Exiting)] received signal SIGUSR1, User defined signal 1.

Roughly, in printf-style, that's:

  printf (Thread %d [%s "%s" (%s)],
           gdb-thread-number,
           target_pid_to_str(),
           target_extra_thread_info(),
           target_thread_name())

where the stuff in []'s is exactly the same format as we have in "info threads", e.g.:

...
  2    Thread 0x7ffff7fd0700 (LWP 15209) "threads" 0x0000003de92ba6cd in nanosleep () from /lib64/libc.so.6
...


An option to avoid the duplicate "Thread" would be to stick with the 
current "stopped" output.  To recap, for "interrupt" (GDB_SIGNAL_0), we get:

  [Thread 0x7ffff7fd0700 (LWP 15209)] #2 stopped.

The difference is that "Thread N" is not a prefix, but instead we
get #N after the target_pid_to_str, etc.

Which would make for:

  [Thread 0x7ffff7fcf700 (LWP 12023) "sigstep-threads"] #2 received signal SIGUSR1, User defined signal 1.
  [Thread 0x7ffff7fd0740 (LWP 12019) "sigstep-threads"] #1 received signal SIGUSR1, User defined signal 1.

Not sure whether people find the #2/#1 confusing/not obvious.

Also not sure people find that including thread number, target string, thread
name and target extra thread info, too much and too long.  OTOH, I like having
all the info available (best while looking at logs, where it is too late to
do "info threads").  I could think of a lot of alternative variations, with
varying amounts of info, and with varying placements of the info on the line...
If being verbose with all the thread info is good, then it might be a good idea
to expand the "Switching to ..." output to match (and others that might make
sense).

I count 65 instances of "Program received" in the test suite, and surprisingly,
only one relevant in the manual in an example for $_siginfo.

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 4efc2af..6c8164e 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -5877,32 +5877,57 @@ print_signal_received_reason (enum gdb_signal siggnal)
 
   annotate_signal ();
 
-  if (siggnal == GDB_SIGNAL_0 && !ui_out_is_mi_like_p (uiout))
+  if (!ui_out_is_mi_like_p (uiout))
     {
       struct thread_info *t = inferior_thread ();
+      char *extra_info, *name, *target_id;
+      struct cleanup *str_cleanup;
+      char *contents;
+
+      ui_out_text (uiout, "\nThread ");
+      ui_out_field_fmt (uiout, "thread-id", "%d [", t->num);
+
+      target_id = target_pid_to_str (t->ptid);
+      extra_info = target_extra_thread_info (t);
+      name = t->name ? t->name : target_thread_name (t);
+
+      if (extra_info != NULL && name != NULL)
+	contents = xstrprintf ("%s \"%s\" (%s)", target_id,
+			       name, extra_info);
+      else if (extra_info != NULL)
+	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, "thread-name", contents);
+      do_cleanups (str_cleanup);
+
+      if (siggnal == GDB_SIGNAL_0)
+	{
+	  ui_out_text (uiout, "] stopped.\n");
+	  return;
+	}
 
-      ui_out_text (uiout, "\n[");
-      ui_out_field_string (uiout, "thread-name",
-			   target_pid_to_str (t->ptid));
-      ui_out_field_fmt (uiout, "thread-id", "] #%d", t->num);
-      ui_out_text (uiout, " stopped");
+      ui_out_text (uiout, "] received signal ");
     }
   else
-    {
-      ui_out_text (uiout, "\nProgram received signal ");
-      annotate_signal_name ();
-      if (ui_out_is_mi_like_p (uiout))
-	ui_out_field_string
+    ui_out_text (uiout, "Program received signal ");
+
+  annotate_signal_name ();
+  if (ui_out_is_mi_like_p (uiout))
+    ui_out_field_string
 	  (uiout, "reason", async_reason_lookup (EXEC_ASYNC_SIGNAL_RECEIVED));
-      ui_out_field_string (uiout, "signal-name",
+  ui_out_field_string (uiout, "signal-name",
 			   gdb_signal_to_name (siggnal));
-      annotate_signal_name_end ();
-      ui_out_text (uiout, ", ");
-      annotate_signal_string ();
-      ui_out_field_string (uiout, "signal-meaning",
-			   gdb_signal_to_string (siggnal));
-      annotate_signal_string_end ();
-    }
+  annotate_signal_name_end ();
+  ui_out_text (uiout, ", ");
+  annotate_signal_string ();
+  ui_out_field_string (uiout, "signal-meaning",
+		       gdb_signal_to_string (siggnal));
+  annotate_signal_string_end ();
   ui_out_text (uiout, ".\n");
 }
 
diff --git a/gdb/testsuite/gdb.threads/sigstep-threads.exp b/gdb/testsuite/gdb.threads/sigstep-threads.exp
index 484ca37..4397dc2 100644
--- a/gdb/testsuite/gdb.threads/sigstep-threads.exp
+++ b/gdb/testsuite/gdb.threads/sigstep-threads.exp
@@ -45,7 +45,7 @@ for {set i 0} {$i < 100} {incr i} {
     # Presume this step failed - as in the case of a timeout.
     set failed 1
     gdb_test_multiple "step" $test {
-	-re "\r\nProgram received signal SIGUSR1, User defined signal 1.\r\n" {
+	-re "received signal SIGUSR1, User defined signal 1.\r\n" {
 	    exp_continue -continue_timer
 	}
 	-re "step-(\[012\]).*\r\n$gdb_prompt $" {