infrun debug output: print enum gdb_signal symbol names instead of POSIX signal names.

The other day while debugging something related to random signals, I
got confused with "set debug infrun 1" output, for it said:

 infrun: TARGET_WAITKIND_STOPPED
 infrun: stop_pc = 0x323d4e8b94
 infrun: random signal 20

On GNU/Linux, 20 is SIGTSTP.  For some reason, it took me a few
minutes to realize that 20 is actually a GDB signal number, not a
target signal number (duh!).  In any case, I propose making GDB's
output clearer here:

One way would be to use gdb_signal_to_name, like already used
elsewhere:

 infrun: TARGET_WAITKIND_STOPPED
 infrun: stop_pc = 0x323d4e8b94
 infrun: random signal SIGCHLD (20)

but I think that might confuse someone too ("20? Why does GDB believe
SIGCHLD is 20?").  So I thought of printing the enum string instead:

 infrun: TARGET_WAITKIND_STOPPED
 infrun: stop_pc = 0x323d4e8b94
 infrun: random signal GDB_SIGNAL_CHLD (20)

Looking at a more complete infrun debug log, we had actually printed
the (POSIX) signal name name a bit before:

 infrun: target_wait (-1, status) =
 infrun:   9300 [Thread 0x7ffff7fcb740 (LWP 9300)],
 infrun:   status->kind = stopped, signal = SIGCHLD
 ...
 infrun: TARGET_WAITKIND_STOPPED
 infrun: stop_pc = 0x323d4e8b94
 infrun: random signal 20

So I'm now thinking that it'd be even better to make infrun output
consistently use the enum symbol string, like so:

 infrun: clear_proceed_status_thread (Thread 0x7ffff7fca700 (LWP 25663))
 infrun: clear_proceed_status_thread (Thread 0x7ffff7fcb740 (LWP 25659))
- infrun: proceed (addr=0xffffffffffffffff, signal=144, step=1)
+ infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT, step=1)
- infrun: resume (step=1, signal=0), trap_expected=0, current thread [Thread 0x7ffff7fcb740 (LWP 25659)] at 0x400700
+ infrun: resume (step=1, signal=GDB_SIGNAL_0), trap_expected=0, current thread [Thread 0x7ffff7fcb740 (LWP 25659)] at 0x400700
 infrun: wait_for_inferior ()
 infrun: target_wait (-1, status) =
 infrun:   25659 [Thread 0x7ffff7fcb740 (LWP 25659)],
- infrun:   status->kind = stopped, signal = SIGCHLD
+ infrun:   status->kind = stopped, signal = GDB_SIGNAL_CHLD
 infrun: infwait_normal_state
 infrun: TARGET_WAITKIND_STOPPED
 infrun: stop_pc = 0x400700
- infrun: random signal 20
+ infrun: random signal (GDB_SIGNAL_CHLD)
 infrun: random signal, keep going
- infrun: resume (step=1, signal=20), trap_expected=0, current thread [Thread 0x7ffff7fcb740 (LWP 25659)] at 0x400700
+ infrun: resume (step=1, signal=GDB_SIGNAL_CHLD), trap_expected=0, current thread [Thread 0x7ffff7fcb740 (LWP 25659)] at 0x400700
 infrun: prepare_to_wait
 infrun: target_wait (-1, status) =
 infrun:   25659 [Thread 0x7ffff7fcb740 (LWP 25659)],
- infrun:   status->kind = stopped, signal = SIGTRAP
+ infrun:   status->kind = stopped, signal = GDB_SIGNAL_TRAP
 infrun: infwait_normal_state
 infrun: TARGET_WAITKIND_STOPPED
 infrun: stop_pc = 0x400704
 infrun: stepi/nexti
 infrun: stop_stepping

GDB's signal numbers are public and hardcoded (see
include/gdb/signals.h), so there's really no need to clutter the
output with numeric values in some places while others not.  Replacing
the magic "144" with GDB_SIGNAL_DEFAULT in "proceed"'s debug output
(see above) I think is quite nice.

I posit that all this makes it clearer to newcomers that GDB has its
own signal numbering (and that there must be some mapping going on).

Tested on x86_64 Fedora 17.

gdb/
2013-10-23  Pedro Alves  <palves@redhat.com>

	* common/gdb_signals.h (gdb_signal_to_symbol_string): Declare.
	* common/signals.c: Include "gdb_assert.h".
	(signals): New field 'symbol'.
	(SET): Use the 'symbol' parameter.
	(gdb_signal_to_symbol_string): New function.
	* infrun.c (handle_inferior_event) <random signal>: In debug
	output, print the random signal enum as string in addition to its
	number.
	* target/waitstatus.c (target_waitstatus_to_string): Print the
	signal's enum value as string instead of the (POSIX) signal name.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1c70fac304fdb82fcf9be11ead58d0d47007a57f..0c61c89642e3c922c330036ca824c7bdc8a176d1 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,16 @@
+2013-10-23  Pedro Alves  <palves@redhat.com>
+
+       * common/gdb_signals.h (gdb_signal_to_symbol_string): Declare.
+       * common/signals.c: Include "gdb_assert.h".
+       (signals): New field 'symbol'.
+       (SET): Use the 'symbol' parameter.
+       (gdb_signal_to_symbol_string): New function.
+       * infrun.c (handle_inferior_event) <random signal>: In debug
+       output, print the random signal enum as string in addition to its
+       number.
+       * target/waitstatus.c (target_waitstatus_to_string): Print the
+       signal's enum value as string instead of the (POSIX) signal name.
+
 2013-10-23  Gary Benson  <gbenson@redhat.com>
 
        PR 16013

diff --git a/gdb/common/gdb_signals.h b/gdb/common/gdb_signals.h
index 76e64f83b9334a92b5b68f93f035a228ed451a8c..e02d1ee7f94d299d664b537a174cd163fc276a0d 100644 (file)
--- a/gdb/common/gdb_signals.h
+++ b/gdb/common/gdb_signals.h
@@ -42,6 +42,10 @@ extern int gdb_signal_to_host_p (enum gdb_signal signo);
 extern enum gdb_signal gdb_signal_from_host (int);
 extern int gdb_signal_to_host (enum gdb_signal);
 
+/* Return the enum symbol name of SIG as a string, to use in debug
+   output.  */
+extern const char *gdb_signal_to_symbol_string (enum gdb_signal sig);
+
 /* Return the string for a signal.  */
 extern const char *gdb_signal_to_string (enum gdb_signal);
 

diff --git a/gdb/common/signals.c b/gdb/common/signals.c
index 8fe4bedcb4d71928ac8468f6b485dcd2c78a60e9..1f3e3a4e0b84d40c07f1519035189269c211b02e 100644 (file)
--- a/gdb/common/signals.c
+++ b/gdb/common/signals.c
@@ -29,6 +29,7 @@
 #endif
 
 #include "gdb_signals.h"
+#include "gdb_assert.h"
 
 struct gdbarch;
 
@@ -50,15 +51,23 @@ struct gdbarch;
    gdb_signal.  */
 
 static const struct {
+  const char *symbol;
   const char *name;
   const char *string;
   } signals [] =
 {
-#define SET(symbol, constant, name, string) { name, string },
+#define SET(symbol, constant, name, string) { #symbol, name, string },
 #include "gdb/signals.def"
 #undef SET
 };
 
+const char *
+gdb_signal_to_symbol_string (enum gdb_signal sig)
+{
+  gdb_assert ((int) sig >= GDB_SIGNAL_FIRST && (int) sig <= GDB_SIGNAL_LAST);
+
+  return signals[sig].symbol;
+}
 
 /* Return the string for a signal.  */
 const char *

diff --git a/gdb/infrun.c b/gdb/infrun.c
index c1cb99fe32ea71b77d2502ccc26d4d41e7795cc7..4f7068c8996b07ee408048e4954f957a26632393 100644 (file)
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1749,9 +1749,10 @@ resume (int step, enum gdb_signal sig)
 
   if (debug_infrun)
     fprintf_unfiltered (gdb_stdlog,
-                        "infrun: resume (step=%d, signal=%d), "
+                       "infrun: resume (step=%d, signal=%s), "
                        "trap_expected=%d, current thread [%s] at %s\n",
-                       step, sig, tp->control.trap_expected,
+                       step, gdb_signal_to_symbol_string (sig),
+                       tp->control.trap_expected,
                        target_pid_to_str (inferior_ptid),
                        paddress (gdbarch, pc));
 
@@ -2200,8 +2201,9 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
 
   if (debug_infrun)
     fprintf_unfiltered (gdb_stdlog,
-                       "infrun: proceed (addr=%s, signal=%d, step=%d)\n",
-                       paddress (gdbarch, addr), siggnal, step);
+                       "infrun: proceed (addr=%s, signal=%s, step=%d)\n",
+                       paddress (gdbarch, addr),
+                       gdb_signal_to_symbol_string (siggnal), step);
 
   if (non_stop)
     /* In non-stop, each thread is handled individually.  The context
@@ -4313,10 +4315,11 @@ process_event_stop_test:
       /* Signal not for debugging purposes.  */
       int printed = 0;
       struct inferior *inf = find_inferior_pid (ptid_get_pid (ecs->ptid));
+      enum gdb_signal stop_signal = ecs->event_thread->suspend.stop_signal;
 
       if (debug_infrun)
-        fprintf_unfiltered (gdb_stdlog, "infrun: random signal %d\n",
-                            ecs->event_thread->suspend.stop_signal);
+        fprintf_unfiltered (gdb_stdlog, "infrun: random signal (%s)\n",
+                            gdb_signal_to_symbol_string (stop_signal));
 
       stopped_by_random_signal = 1;
 

diff --git a/gdb/target/waitstatus.c b/gdb/target/waitstatus.c
index e6b850268b697d3242a4b1e823195d24fa36820c..4e5e8d6a0bafa2980c514bf9c0192e93586a55e7 100644 (file)
--- a/gdb/target/waitstatus.c
+++ b/gdb/target/waitstatus.c
@@ -40,10 +40,12 @@ target_waitstatus_to_string (const struct target_waitstatus *ws)
                         kind_str, ws->value.integer);
     case TARGET_WAITKIND_STOPPED:
       return xstrprintf ("%sstopped, signal = %s",
-                        kind_str, gdb_signal_to_name (ws->value.sig));
+                        kind_str,
+                        gdb_signal_to_symbol_string (ws->value.sig));
     case TARGET_WAITKIND_SIGNALLED:
       return xstrprintf ("%ssignalled, signal = %s",
-                        kind_str, gdb_signal_to_name (ws->value.sig));
+                        kind_str,
+                        gdb_signal_to_symbol_string (ws->value.sig));
     case TARGET_WAITKIND_LOADED:
       return xstrprintf ("%sloaded", kind_str);
     case TARGET_WAITKIND_FORKED: