From c9737c08e7f804dfa80f71b74cb5678d0b69308e Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Wed, 23 Oct 2013 16:14:53 +0100 Subject: [PATCH] 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 * 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) : 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. --- gdb/ChangeLog | 13 +++++++++++++ gdb/common/gdb_signals.h | 4 ++++ gdb/common/signals.c | 11 ++++++++++- gdb/infrun.c | 15 +++++++++------ gdb/target/waitstatus.c | 6 ++++-- 5 files changed, 40 insertions(+), 9 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 1c70fac304f..0c61c89642e 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,16 @@ +2013-10-23 Pedro Alves + + * 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) : 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 PR 16013 diff --git a/gdb/common/gdb_signals.h b/gdb/common/gdb_signals.h index 76e64f83b93..e02d1ee7f94 100644 --- 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 8fe4bedcb4d..1f3e3a4e0b8 100644 --- 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 c1cb99fe32e..4f7068c8996 100644 --- 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 @@ Cannot fill $_exitsignal with the correct signal number.\n")); /* 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 e6b850268b6..4e5e8d6a0ba 100644 --- 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: