* MIPS Linux signals
@ 2012-05-20 2:03 Michael Eager
2012-05-21 11:04 ` Pedro Alves
` (3 more replies)
0 siblings, 4 replies; 29+ messages in thread
From: Michael Eager @ 2012-05-20 2:03 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1612 bytes --]
MIPS Linux has different values assigned to signals than
X86 or PPC Linux. The result is that a SIGBUS on MIPS
(value 0xA) is reported as SIGUSR1, since that signal has
the that value on X86. This causes confusion (obviously).
Telling gdb to handle SIGUSR1 will cause it to respond to
SIGBUS but ignore SIGUSR1 (which has the value 0x16 on MIPS).
I didn't find a bug report for this in Bugzilla. I
have trouble believing that this problem has gone unnoticed.
The attached patch adds target_signal->internal_signal
translation, and vice versa, for MIPS Linux targets. I
adapted the code from target_symbol_from_host() and
do_target_symbol_to_host()in gdb/common/signals.c. Other
than being poor names, these functions and the wrappers
around them seem unnecessarily complicated. In particular,
I left out EXE_BAD_* translations because they appear to
be darwin-specific and translations for REALTIME_LO/HI
seem to be obsolete. ChangeLog-2006 mentions removing
these defines.
Which leads me to some questions:
1 -- Is there a bug report for this somewhere?
2 -- Is there any reason to implement EXE_BAD_* translations?
3 -- Is the REALTIME_LO/HI translation obsolete cruft?
4 -- Do the multiple layers of wrappers around target_
signal_{to,from}_host in signals.c serve any purpose?
I'm still testing the patch. I have a core file which now
displays the correct signal, but I have to create a test
program where I can test the translation in the other direction.
Any comments on the patch?
--
Michael Eager eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
[-- Attachment #2: mips-signal.patch --]
[-- Type: text/x-patch, Size: 7283 bytes --]
Index: gdb/gdb/mips-linux-tdep.c
===================================================================
--- gdb/.CC/cache/mips-linux-tdep.c@@/main/gdb_7_2_dev/3 2012-05-19 18:25:39.000077000 -0700
+++ gdb/mips-linux-tdep.c 2012-05-19 18:25:27.000033000 -0700
@@ -1130,6 +1130,101 @@ mips_linux_syscall_next_pc (struct frame
return pc + 4;
}
+/* Translate signals based on MIPS signal values.
+ Adapted from gdb/common/signals.c. */
+
+static enum target_signal
+mips_target_signal_from_host (struct gdbarch *gdbarch, int signo)
+{
+ printf ("MJE: mips_target_signal_from_host (<gdbarch>, signo=%d)\n", signo);
+
+ switch (signo) {
+ case 0: return TARGET_SIGNAL_0;
+ case MIPS_SIGHUP: return TARGET_SIGNAL_HUP;
+ case MIPS_SIGINT: return TARGET_SIGNAL_INT;
+ case MIPS_SIGQUIT: return TARGET_SIGNAL_QUIT;
+ case MIPS_SIGILL: return TARGET_SIGNAL_ILL;
+ case MIPS_SIGTRAP: return TARGET_SIGNAL_TRAP;
+ case MIPS_SIGABRT: return TARGET_SIGNAL_ABRT;
+ case MIPS_SIGEMT: return TARGET_SIGNAL_EMT;
+ case MIPS_SIGFPE: return TARGET_SIGNAL_FPE;
+ case MIPS_SIGKILL: return TARGET_SIGNAL_KILL;
+ case MIPS_SIGBUS: return TARGET_SIGNAL_BUS;
+ case MIPS_SIGSEGV: return TARGET_SIGNAL_SEGV;
+ case MIPS_SIGSYS: return TARGET_SIGNAL_SYS;
+ case MIPS_SIGPIPE: return TARGET_SIGNAL_PIPE;
+ case MIPS_SIGALRM: return TARGET_SIGNAL_ALRM;
+ case MIPS_SIGTERM: return TARGET_SIGNAL_TERM;
+ case MIPS_SIGUSR1: return TARGET_SIGNAL_USR1;
+ case MIPS_SIGUSR2: return TARGET_SIGNAL_USR2;
+ case MIPS_SIGCHLD: return TARGET_SIGNAL_CHLD;
+ case MIPS_SIGPWR: return TARGET_SIGNAL_PWR;
+ case MIPS_SIGWINCH: return TARGET_SIGNAL_WINCH;
+ case MIPS_SIGURG: return TARGET_SIGNAL_URG;
+ case MIPS_SIGPOLL: return TARGET_SIGNAL_POLL;
+ case MIPS_SIGSTOP: return TARGET_SIGNAL_STOP;
+ case MIPS_SIGTSTP: return TARGET_SIGNAL_TSTP;
+ case MIPS_SIGCONT: return TARGET_SIGNAL_CONT;
+ case MIPS_SIGTTIN: return TARGET_SIGNAL_TTIN;
+ case MIPS_SIGTTOU: return TARGET_SIGNAL_TTOU;
+ case MIPS_SIGVTALRM: return TARGET_SIGNAL_VTALRM;
+ case MIPS_SIGPROF: return TARGET_SIGNAL_PROF;
+ case MIPS_SIGXCPU: return TARGET_SIGNAL_XCPU;
+ case MIPS_SIGXFSZ: return TARGET_SIGNAL_XFSZ;
+ }
+
+#ifdef _SIG
+#endif
+
+ return TARGET_SIGNAL_UNKNOWN;
+}
+
+static int
+mips_target_signal_to_host (struct gdbarch *gdbarch, enum target_signal signo)
+{
+ printf ("MJE: mips_target_signal_to_host (<gdbarch>, signo=%d)\n", signo);
+
+ switch (signo) {
+ case TARGET_SIGNAL_0: return 0;
+ case TARGET_SIGNAL_HUP: return MIPS_SIGHUP;
+ case TARGET_SIGNAL_INT: return MIPS_SIGINT;
+ case TARGET_SIGNAL_QUIT: return MIPS_SIGQUIT;
+ case TARGET_SIGNAL_ILL: return MIPS_SIGILL;
+ case TARGET_SIGNAL_TRAP: return MIPS_SIGTRAP;
+ case TARGET_SIGNAL_ABRT: return MIPS_SIGABRT;
+ case TARGET_SIGNAL_EMT: return MIPS_SIGEMT;
+ case TARGET_SIGNAL_FPE: return MIPS_SIGFPE;
+ case TARGET_SIGNAL_KILL: return MIPS_SIGKILL;
+ case TARGET_SIGNAL_BUS: return MIPS_SIGBUS;
+ case TARGET_SIGNAL_SEGV: return MIPS_SIGSEGV;
+ case TARGET_SIGNAL_SYS: return MIPS_SIGSYS;
+ case TARGET_SIGNAL_PIPE: return MIPS_SIGPIPE;
+ case TARGET_SIGNAL_ALRM: return MIPS_SIGALRM;
+ case TARGET_SIGNAL_TERM: return MIPS_SIGTERM;
+ case TARGET_SIGNAL_USR1: return MIPS_SIGUSR1;
+ case TARGET_SIGNAL_USR2: return MIPS_SIGUSR2;
+ case TARGET_SIGNAL_CHLD: return MIPS_SIGCHLD;
+ case TARGET_SIGNAL_PWR: return MIPS_SIGPWR;
+ case TARGET_SIGNAL_WINCH: return MIPS_SIGWINCH;
+ case TARGET_SIGNAL_URG: return MIPS_SIGURG;
+ case TARGET_SIGNAL_POLL: return MIPS_SIGPOLL;
+ case TARGET_SIGNAL_STOP: return MIPS_SIGSTOP;
+ case TARGET_SIGNAL_TSTP: return MIPS_SIGTSTP;
+ case TARGET_SIGNAL_CONT: return MIPS_SIGCONT;
+ case TARGET_SIGNAL_TTIN: return MIPS_SIGTTIN;
+ case TARGET_SIGNAL_TTOU: return MIPS_SIGTTOU;
+ case TARGET_SIGNAL_VTALRM: return MIPS_SIGVTALRM;
+ case TARGET_SIGNAL_PROF: return MIPS_SIGPROF;
+ case TARGET_SIGNAL_XCPU: return MIPS_SIGXCPU;
+ case TARGET_SIGNAL_XFSZ: return MIPS_SIGXFSZ;
+
+ default:
+ warning ("Signal %s does not exist on this system.\n",
+ target_signal_to_name (signo));
+ return 0;
+ }
+}
+
/* Initialize one of the GNU/Linux OS ABIs. */
static void
@@ -1203,6 +1298,11 @@ mips_linux_init_abi (struct gdbarch_info
set_gdbarch_core_read_description (gdbarch,
mips_linux_core_read_description);
+ set_gdbarch_target_signal_from_host (gdbarch,
+ mips_target_signal_from_host);
+ set_gdbarch_target_signal_to_host (gdbarch,
+ mips_target_signal_to_host);
+
tdep->syscall_next_pc = mips_linux_syscall_next_pc;
if (tdesc_data)
Index: gdb/gdb/mips-linux-tdep.h
===================================================================
--- gdb/.CC/cache/mips-linux-tdep.h@@/main/gdb_7_2_dev/1 2012-05-19 18:25:40.000041000 -0700
+++ gdb/mips-linux-tdep.h 2012-05-19 10:53:12.000064000 -0700
@@ -100,3 +100,43 @@ enum {
/* Return 1 if MIPS_RESTART_REGNUM is usable. */
int mips_linux_restart_reg_p (struct gdbarch *gdbarch);
+
+/* MIPS Signals -- adapted from linux/arch/mips/include/asm/signal.h. */
+
+enum mips_signals {
+ MIPS_SIGHUP = 1, /* Hangup (POSIX). */
+ MIPS_SIGINT = 2, /* Interrupt (ANSI). */
+ MIPS_SIGQUIT = 3, /* Quit (POSIX). */
+ MIPS_SIGILL = 4, /* Illegal instruction (ANSI). */
+ MIPS_SIGTRAP = 5, /* Trace trap (POSIX). */
+ MIPS_SIGIOT = 6, /* IOT trap (4.2 BSD). */
+ MIPS_SIGABRT = MIPS_SIGIOT, /* Abort (ANSI). */
+ MIPS_SIGEMT = 7,
+ MIPS_SIGFPE = 8, /* Floating-point exception (ANSI). */
+ MIPS_SIGKILL = 9, /* Kill, unblockable (POSIX). */
+ MIPS_SIGBUS = 10, /* BUS error (4.2 BSD). */
+ MIPS_SIGSEGV = 11, /* Segmentation violation (ANSI). */
+ MIPS_SIGSYS = 12,
+ MIPS_SIGPIPE = 13, /* Broken pipe (POSIX). */
+ MIPS_SIGALRM = 14, /* Alarm clock (POSIX). */
+ MIPS_SIGTERM = 15, /* Termination (ANSI). */
+ MIPS_SIGUSR1 = 16, /* User-defined signal 1 (POSIX). */
+ MIPS_SIGUSR2 = 17, /* User-defined signal 2 (POSIX). */
+ MIPS_SIGCHLD = 18, /* Child status has changed (POSIX). */
+ MIPS_SIGCLD = MIPS_SIGCHLD, /* Same as SIGCHLD (System V). */
+ MIPS_SIGPWR = 19, /* Power failure restart (System V). */
+ MIPS_SIGWINCH = 20, /* Window size change (4.3 BSD, Sun). */
+ MIPS_SIGURG = 21, /* Urgent condition on socket (4.2 BSD). */
+ MIPS_SIGIO = 22, /* I/O now possible (4.2 BSD). */
+ MIPS_SIGPOLL = MIPS_SIGIO,/* Pollable event occurred (System V). */
+ MIPS_SIGSTOP = 23, /* Stop, unblockable (POSIX). */
+ MIPS_SIGTSTP = 24, /* Keyboard stop (POSIX). */
+ MIPS_SIGCONT = 25, /* Continue (POSIX). */
+ MIPS_SIGTTIN = 26, /* Background read from tty (POSIX). */
+ MIPS_SIGTTOU = 27, /* Background write to tty (POSIX). */
+ MIPS_SIGVTALRM= 28, /* Virtual alarm clock (4.2 BSD). */
+ MIPS_SIGPROF = 29, /* Profiling alarm clock (4.2 BSD). */
+ MIPS_SIGXCPU = 30, /* CPU limit exceeded (4.2 BSD). */
+ MIPS_SIGXFSZ = 31 /* File size limit exceeded (4.2 BSD). */
+ };
+
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: MIPS Linux signals 2012-05-20 2:03 MIPS Linux signals Michael Eager @ 2012-05-21 11:04 ` Pedro Alves 2012-05-21 14:51 ` Michael Eager 2012-05-21 18:19 ` Maciej W. Rozycki ` (2 subsequent siblings) 3 siblings, 1 reply; 29+ messages in thread From: Pedro Alves @ 2012-05-21 11:04 UTC (permalink / raw) To: Michael Eager; +Cc: gdb-patches On 05/20/2012 03:02 AM, Michael Eager wrote: > Which leads me to some questions: > 2 -- Is there any reason to implement EXE_BAD_* translations? No. > 3 -- Is the REALTIME_LO/HI translation obsolete cruft? ChangeLog-2006 points at: http://sourceware.org/ml/gdb-patches/2006-11/msg00340.html where we see that REALTIME_LO/HI were defined in target headers, while they should really be native/host defines. So it's not that they got deprecated, but a target/host confusion bug was fixed. The macros are still there in common/signals.c. > 4 -- Do the multiple layers of wrappers around target_ > signal_{to,from}_host in signals.c serve any purpose? Can you be more specific? What multiple layers? -- Pedro Alves ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-21 11:04 ` Pedro Alves @ 2012-05-21 14:51 ` Michael Eager 2012-05-21 17:37 ` Pedro Alves 0 siblings, 1 reply; 29+ messages in thread From: Michael Eager @ 2012-05-21 14:51 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches On 05/21/2012 04:03 AM, Pedro Alves wrote: > On 05/20/2012 03:02 AM, Michael Eager wrote: >> 4 -- Do the multiple layers of wrappers around target_ >> signal_{to,from}_host in signals.c serve any purpose? > > Can you be more specific? What multiple layers? target_signal_to_host() is a wrapper around do_target_signal_to_host(). target_signal_to_host_p() is a wrapper around do_target_signal_to_host(). This appears to be only used in gdbserver, where, on MIPS, it will return the wrong result. Incidentally, gdbserver on MIPS calls the X86 target_signal_to_host() to translate signals. This seems confused. If gdb is translating internal signal numbers to the target signal numbers, then this seems to be a second translation. default_target_signal_to_host() is a wrapper around target_signal_to_host(). default_target_signal_from_host() is a wrapper around target_signal_from_host(). -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-21 14:51 ` Michael Eager @ 2012-05-21 17:37 ` Pedro Alves 2012-05-21 18:06 ` Michael Eager 0 siblings, 1 reply; 29+ messages in thread From: Pedro Alves @ 2012-05-21 17:37 UTC (permalink / raw) To: Michael Eager; +Cc: gdb-patches On 05/21/2012 03:50 PM, Michael Eager wrote: > On 05/21/2012 04:03 AM, Pedro Alves wrote: >> On 05/20/2012 03:02 AM, Michael Eager wrote: > >>> 4 -- Do the multiple layers of wrappers around target_ >>> signal_{to,from}_host in signals.c serve any purpose? >> >> Can you be more specific? What multiple layers? > > target_signal_to_host() is a wrapper around do_target_signal_to_host(). > > target_signal_to_host_p() is a wrapper around do_target_signal_to_host(). Yes, do_target_signal_to_host is a helper for both target_signal_to_host and target_signal_to_host_p. > This appears to be only used in gdbserver, where, on MIPS, it will > return the wrong result. Why will it return the wrong result on MIPS/gdbserver? The common/signals.c copy built into gdbserver will naturally be built with a MIPS headers, and should pick up the correct signal defines (SIGINT, etc.). > Incidentally, gdbserver on MIPS calls > the X86 target_signal_to_host() to translate signals. This seems > confused. Now I'm confused. What do you mean that MIPS gdbserver calls the X86 target_signal_to_host ? There's only one target_signal_to_host function AFAICS. > If gdb is translating internal signal numbers to the > target signal numbers, then this seems to be a second translation. gdbserver internally translates Linux signal numbers to the host independent internal signal numbers. We call the latter "target signals", like we call every method that goes through the "target abstraction" (target_ops vector) in gdb target_something. The native signals that are host dependent (host in the same sense as build vs host vs target on autoconf), we call them host signals. On the "wire", RSP packets should contain only the abstracted, internal, target signal numbers. So code on gdbserver that talks to ptrace will need to convert the (native) host signal numbers to target signals when pushing events to gdbserver common code, and ultimately to GDB. AFAIK, there's no such second internal signal numbers to target signal numbers translation in GDB. The remote target already works with GDB's own internal signal numbers. > default_target_signal_to_host() is a wrapper around target_signal_to_host(). > > default_target_signal_from_host() is a wrapper around target_signal_from_host(). Right. gdbarch_target_signal_from_host was invented long after target_signal_from_host already existed to solve the similar case you ran into. That is, of when cross debugging core files. In that case there's no gdbserver involved to do the host -> gdb/internal signal conversion for us, so GDB needs to do it, according to the core's gdbarch. If there's no such gdbarch callback installed, then we fallback to the host's default conversion, which with work when debugging native core files. So you do have the right fix, but the reason why nobody has tripped on this before is that this only affects cross core debugging, not live gdbserver debugging. -- Pedro Alves ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-21 17:37 ` Pedro Alves @ 2012-05-21 18:06 ` Michael Eager 0 siblings, 0 replies; 29+ messages in thread From: Michael Eager @ 2012-05-21 18:06 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches On 05/21/2012 10:36 AM, Pedro Alves wrote: > On 05/21/2012 03:50 PM, Michael Eager wrote: >> This appears to be only used in gdbserver, where, on MIPS, it will >> return the wrong result. > > Why will it return the wrong result on MIPS/gdbserver? The common/signals.c > copy built into gdbserver will naturally be built with a MIPS headers, and > should pick up the correct signal defines (SIGINT, etc.). Yes, you are right. Gdbserver will be built with the correct MIPS signal defs. > So you do have the right fix, but the reason why nobody has tripped on this before is > that this only affects cross core debugging, not live gdbserver debugging. Thanks. -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-20 2:03 MIPS Linux signals Michael Eager 2012-05-21 11:04 ` Pedro Alves @ 2012-05-21 18:19 ` Maciej W. Rozycki [not found] ` <alpine.DEB.1.10.1205211232260.11227@tp.orcam.me.uk> 2012-05-21 21:35 ` Pedro Alves 3 siblings, 0 replies; 29+ messages in thread From: Maciej W. Rozycki @ 2012-05-21 18:19 UTC (permalink / raw) To: Michael Eager; +Cc: gdb-patches Hi Michael, On Sun, 20 May 2012, Michael Eager wrote: > MIPS Linux has different values assigned to signals than > X86 or PPC Linux. The result is that a SIGBUS on MIPS > (value 0xA) is reported as SIGUSR1, since that signal has > the that value on X86. This causes confusion (obviously). > Telling gdb to handle SIGUSR1 will cause it to respond to > SIGBUS but ignore SIGUSR1 (which has the value 0x16 on MIPS). > > I didn't find a bug report for this in Bugzilla. I > have trouble believing that this problem has gone unnoticed. What target are you specifically referring to? Can you describe a scenario where it happens? As far as I know it does not apply for the native target (for obvious reasons) and the remote target uses generic signal numbers (as defined in the Remote Serial Protocol aka RSP) so they are reformed uniformly as well. Did I miss anything? > The attached patch adds target_signal->internal_signal > translation, and vice versa, for MIPS Linux targets. I > adapted the code from target_symbol_from_host() and > do_target_symbol_to_host()in gdb/common/signals.c. Other > than being poor names, these functions and the wrappers > around them seem unnecessarily complicated. In particular, > I left out EXE_BAD_* translations because they appear to > be darwin-specific and translations for REALTIME_LO/HI > seem to be obsolete. ChangeLog-2006 mentions removing > these defines. > > Which leads me to some questions: > 1 -- Is there a bug report for this somewhere? > 2 -- Is there any reason to implement EXE_BAD_* translations? > 3 -- Is the REALTIME_LO/HI translation obsolete cruft? > 4 -- Do the multiple layers of wrappers around target_ > signal_{to,from}_host in signals.c serve any purpose? I'll leave answers to these questions to our general maintainers. > I'm still testing the patch. I have a core file which now > displays the correct signal, but I have to create a test > program where I can test the translation in the other direction. Ah, so that is with the core file target used in a cross-debugger, right? Well, no wonder nobody has noticed, this is a very unusual scenario -- I have used GDB for at least 15 years now and may have had a need to examine a core file with a cross-debugger perhaps once or twice -- and then some common signals looked for like SIGSEGV or SIGILL are identity mapped (and I knew the signal that caused the core file dumped beforehand anyway as it's printed as the program is being killed anyway). From experience what matters the most when examining a core file -- beside the dump of the process's address space of course -- are the contents of registers. The rest -- I would expect hardly anybody to look at. > Any comments on the patch? Overall it looks good to me, but has some coding style problems and some other small issues, listed below. Also by the look of it, such changes will actually be required for all processor/OS targets as obviously when you have e.g. a MIPS/IRIX-host cross-debugger targetting i386/Linux then the signal numbers won't match again, except that in the other direction. That certainly begs for a general solution, perhaps like one used by the RSP where the GDB core uses generic signal numbers the target side translate to/from them as appropriate (obviously the host OS may have no notion of signals at all). That looks like a post-7.5 material to me though -- we've lived with this problem for long enough that I don't think we need to rush and correct it a little more than a week before the release branch. > Index: gdb/gdb/mips-linux-tdep.c > =================================================================== > --- gdb/.CC/cache/mips-linux-tdep.c@@/main/gdb_7_2_dev/3 2012-05-19 18:25:39.000077000 -0700 > +++ gdb/mips-linux-tdep.c 2012-05-19 18:25:27.000033000 -0700 > @@ -1130,6 +1130,101 @@ mips_linux_syscall_next_pc (struct frame > return pc + 4; > } > > +/* Translate signals based on MIPS signal values. > + Adapted from gdb/common/signals.c. */ > + > +static enum target_signal > +mips_target_signal_from_host (struct gdbarch *gdbarch, int signo) > +{ > + printf ("MJE: mips_target_signal_from_host (<gdbarch>, signo=%d)\n", signo); I'm assuming this is a debug message, please drop it. Otherwise we use calls like fprintf_unfiltered (...) to produce output; see other sources for a reference. > + > + switch (signo) { > + case 0: return TARGET_SIGNAL_0; > + case MIPS_SIGHUP: return TARGET_SIGNAL_HUP; > + case MIPS_SIGINT: return TARGET_SIGNAL_INT; > + case MIPS_SIGQUIT: return TARGET_SIGNAL_QUIT; > + case MIPS_SIGILL: return TARGET_SIGNAL_ILL; > + case MIPS_SIGTRAP: return TARGET_SIGNAL_TRAP; > + case MIPS_SIGABRT: return TARGET_SIGNAL_ABRT; > + case MIPS_SIGEMT: return TARGET_SIGNAL_EMT; > + case MIPS_SIGFPE: return TARGET_SIGNAL_FPE; > + case MIPS_SIGKILL: return TARGET_SIGNAL_KILL; > + case MIPS_SIGBUS: return TARGET_SIGNAL_BUS; > + case MIPS_SIGSEGV: return TARGET_SIGNAL_SEGV; > + case MIPS_SIGSYS: return TARGET_SIGNAL_SYS; > + case MIPS_SIGPIPE: return TARGET_SIGNAL_PIPE; > + case MIPS_SIGALRM: return TARGET_SIGNAL_ALRM; > + case MIPS_SIGTERM: return TARGET_SIGNAL_TERM; > + case MIPS_SIGUSR1: return TARGET_SIGNAL_USR1; > + case MIPS_SIGUSR2: return TARGET_SIGNAL_USR2; > + case MIPS_SIGCHLD: return TARGET_SIGNAL_CHLD; > + case MIPS_SIGPWR: return TARGET_SIGNAL_PWR; > + case MIPS_SIGWINCH: return TARGET_SIGNAL_WINCH; > + case MIPS_SIGURG: return TARGET_SIGNAL_URG; > + case MIPS_SIGPOLL: return TARGET_SIGNAL_POLL; > + case MIPS_SIGSTOP: return TARGET_SIGNAL_STOP; > + case MIPS_SIGTSTP: return TARGET_SIGNAL_TSTP; > + case MIPS_SIGCONT: return TARGET_SIGNAL_CONT; > + case MIPS_SIGTTIN: return TARGET_SIGNAL_TTIN; > + case MIPS_SIGTTOU: return TARGET_SIGNAL_TTOU; > + case MIPS_SIGVTALRM: return TARGET_SIGNAL_VTALRM; > + case MIPS_SIGPROF: return TARGET_SIGNAL_PROF; > + case MIPS_SIGXCPU: return TARGET_SIGNAL_XCPU; > + case MIPS_SIGXFSZ: return TARGET_SIGNAL_XFSZ; > + } All the case statements need to be on the next line, correctly indented, e.g.: case MIPS_SIGXFSZ: return TARGET_SIGNAL_XFSZ; > + > +#ifdef _SIG > +#endif Some debug leftover I presume, please drop it. > + > + return TARGET_SIGNAL_UNKNOWN; > +} Indentation: return TARGET_SIGNAL_UNKNOWN; } > + > +static int > +mips_target_signal_to_host (struct gdbarch *gdbarch, enum target_signal signo) > +{ > + printf ("MJE: mips_target_signal_to_host (<gdbarch>, signo=%d)\n", signo); Same as above. > + > + switch (signo) { > + case TARGET_SIGNAL_0: return 0; > + case TARGET_SIGNAL_HUP: return MIPS_SIGHUP; > + case TARGET_SIGNAL_INT: return MIPS_SIGINT; > + case TARGET_SIGNAL_QUIT: return MIPS_SIGQUIT; > + case TARGET_SIGNAL_ILL: return MIPS_SIGILL; > + case TARGET_SIGNAL_TRAP: return MIPS_SIGTRAP; > + case TARGET_SIGNAL_ABRT: return MIPS_SIGABRT; > + case TARGET_SIGNAL_EMT: return MIPS_SIGEMT; > + case TARGET_SIGNAL_FPE: return MIPS_SIGFPE; > + case TARGET_SIGNAL_KILL: return MIPS_SIGKILL; > + case TARGET_SIGNAL_BUS: return MIPS_SIGBUS; > + case TARGET_SIGNAL_SEGV: return MIPS_SIGSEGV; > + case TARGET_SIGNAL_SYS: return MIPS_SIGSYS; > + case TARGET_SIGNAL_PIPE: return MIPS_SIGPIPE; > + case TARGET_SIGNAL_ALRM: return MIPS_SIGALRM; > + case TARGET_SIGNAL_TERM: return MIPS_SIGTERM; > + case TARGET_SIGNAL_USR1: return MIPS_SIGUSR1; > + case TARGET_SIGNAL_USR2: return MIPS_SIGUSR2; > + case TARGET_SIGNAL_CHLD: return MIPS_SIGCHLD; > + case TARGET_SIGNAL_PWR: return MIPS_SIGPWR; > + case TARGET_SIGNAL_WINCH: return MIPS_SIGWINCH; > + case TARGET_SIGNAL_URG: return MIPS_SIGURG; > + case TARGET_SIGNAL_POLL: return MIPS_SIGPOLL; > + case TARGET_SIGNAL_STOP: return MIPS_SIGSTOP; > + case TARGET_SIGNAL_TSTP: return MIPS_SIGTSTP; > + case TARGET_SIGNAL_CONT: return MIPS_SIGCONT; > + case TARGET_SIGNAL_TTIN: return MIPS_SIGTTIN; > + case TARGET_SIGNAL_TTOU: return MIPS_SIGTTOU; > + case TARGET_SIGNAL_VTALRM: return MIPS_SIGVTALRM; > + case TARGET_SIGNAL_PROF: return MIPS_SIGPROF; > + case TARGET_SIGNAL_XCPU: return MIPS_SIGXCPU; > + case TARGET_SIGNAL_XFSZ: return MIPS_SIGXFSZ; Ditto. > + > + default: > + warning ("Signal %s does not exist on this system.\n", > + target_signal_to_name (signo)); > + return 0; > + } > +} > + > /* Initialize one of the GNU/Linux OS ABIs. */ > > static void > @@ -1203,6 +1298,11 @@ mips_linux_init_abi (struct gdbarch_info > set_gdbarch_core_read_description (gdbarch, > mips_linux_core_read_description); > > + set_gdbarch_target_signal_from_host (gdbarch, > + mips_target_signal_from_host); > + set_gdbarch_target_signal_to_host (gdbarch, > + mips_target_signal_to_host); > + Indentation, leading tabs must be used: set_gdbarch_target_signal_from_host (gdbarch, mips_target_signal_from_host); set_gdbarch_target_signal_to_host (gdbarch, mips_target_signal_to_host); > tdep->syscall_next_pc = mips_linux_syscall_next_pc; > > if (tdesc_data) > > Index: gdb/gdb/mips-linux-tdep.h > =================================================================== > --- gdb/.CC/cache/mips-linux-tdep.h@@/main/gdb_7_2_dev/1 2012-05-19 18:25:40.000041000 -0700 > +++ gdb/mips-linux-tdep.h 2012-05-19 10:53:12.000064000 -0700 > @@ -100,3 +100,43 @@ enum { > /* Return 1 if MIPS_RESTART_REGNUM is usable. */ > > int mips_linux_restart_reg_p (struct gdbarch *gdbarch); > + > +/* MIPS Signals -- adapted from linux/arch/mips/include/asm/signal.h. */ > + > +enum mips_signals { > + MIPS_SIGHUP = 1, /* Hangup (POSIX). */ Indentation: enum mips_signals { MIPS_SIGHUP = 1, /* Hangup (POSIX). */ etc. > + MIPS_SIGINT = 2, /* Interrupt (ANSI). */ > + MIPS_SIGQUIT = 3, /* Quit (POSIX). */ > + MIPS_SIGILL = 4, /* Illegal instruction (ANSI). */ > + MIPS_SIGTRAP = 5, /* Trace trap (POSIX). */ > + MIPS_SIGIOT = 6, /* IOT trap (4.2 BSD). */ > + MIPS_SIGABRT = MIPS_SIGIOT, /* Abort (ANSI). */ > + MIPS_SIGEMT = 7, > + MIPS_SIGFPE = 8, /* Floating-point exception (ANSI). */ > + MIPS_SIGKILL = 9, /* Kill, unblockable (POSIX). */ > + MIPS_SIGBUS = 10, /* BUS error (4.2 BSD). */ > + MIPS_SIGSEGV = 11, /* Segmentation violation (ANSI). */ > + MIPS_SIGSYS = 12, > + MIPS_SIGPIPE = 13, /* Broken pipe (POSIX). */ > + MIPS_SIGALRM = 14, /* Alarm clock (POSIX). */ > + MIPS_SIGTERM = 15, /* Termination (ANSI). */ > + MIPS_SIGUSR1 = 16, /* User-defined signal 1 (POSIX). */ > + MIPS_SIGUSR2 = 17, /* User-defined signal 2 (POSIX). */ > + MIPS_SIGCHLD = 18, /* Child status has changed (POSIX). */ > + MIPS_SIGCLD = MIPS_SIGCHLD, /* Same as SIGCHLD (System V). */ > + MIPS_SIGPWR = 19, /* Power failure restart (System V). */ > + MIPS_SIGWINCH = 20, /* Window size change (4.3 BSD, Sun). */ > + MIPS_SIGURG = 21, /* Urgent condition on socket (4.2 BSD). */ > + MIPS_SIGIO = 22, /* I/O now possible (4.2 BSD). */ > + MIPS_SIGPOLL = MIPS_SIGIO,/* Pollable event occurred (System V). */ > + MIPS_SIGSTOP = 23, /* Stop, unblockable (POSIX). */ > + MIPS_SIGTSTP = 24, /* Keyboard stop (POSIX). */ > + MIPS_SIGCONT = 25, /* Continue (POSIX). */ > + MIPS_SIGTTIN = 26, /* Background read from tty (POSIX). */ > + MIPS_SIGTTOU = 27, /* Background write to tty (POSIX). */ > + MIPS_SIGVTALRM= 28, /* Virtual alarm clock (4.2 BSD). */ Use a space between MIPS_SIGVTALRM and = here. > + MIPS_SIGPROF = 29, /* Profiling alarm clock (4.2 BSD). */ > + MIPS_SIGXCPU = 30, /* CPU limit exceeded (4.2 BSD). */ > + MIPS_SIGXFSZ = 31 /* File size limit exceeded (4.2 BSD). */ > + }; > + Otherwise OK, but let's wait for feedback from the others on the general problem I noted above. Maciej ^ permalink raw reply [flat|nested] 29+ messages in thread
[parent not found: <alpine.DEB.1.10.1205211232260.11227@tp.orcam.me.uk>]
* Re: MIPS Linux signals [not found] ` <alpine.DEB.1.10.1205211232260.11227@tp.orcam.me.uk> @ 2012-05-21 18:21 ` Michael Eager 2012-05-21 22:34 ` Maciej W. Rozycki 0 siblings, 1 reply; 29+ messages in thread From: Michael Eager @ 2012-05-21 18:21 UTC (permalink / raw) To: Maciej W. Rozycki, gdb-patches On 05/21/2012 10:53 AM, Maciej W. Rozycki wrote: > Hi Michael, > > On Sun, 20 May 2012, Michael Eager wrote: > >> MIPS Linux has different values assigned to signals than >> X86 or PPC Linux. The result is that a SIGBUS on MIPS >> (value 0xA) is reported as SIGUSR1, since that signal has >> the that value on X86. This causes confusion (obviously). >> Telling gdb to handle SIGUSR1 will cause it to respond to >> SIGBUS but ignore SIGUSR1 (which has the value 0x16 on MIPS). >> >> I didn't find a bug report for this in Bugzilla. I >> have trouble believing that this problem has gone unnoticed. > > What target are you specifically referring to? Can you describe a > scenario where it happens? Debugging a MIPS core file. Pedro suggested that no one would ever see this running gdbserver. > Ah, so that is with the core file target used in a cross-debugger, right? > Well, no wonder nobody has noticed, this is a very unusual scenario -- I > have used GDB for at least 15 years now and may have had a need to examine > a core file with a cross-debugger perhaps once or twice -- and then some > common signals looked for like SIGSEGV or SIGILL are identity mapped (and > I knew the signal that caused the core file dumped beforehand anyway as > it's printed as the program is being killed anyway). Yes, this may be less common than I first thought. SEGVs are much more common than SIGBUS. > Overall it looks good to me, but has some coding style problems and some > other small issues, listed below. > > Also by the look of it, such changes will actually be required for all > processor/OS targets as obviously when you have e.g. a MIPS/IRIX-host > cross-debugger targetting i386/Linux then the signal numbers won't match > again, except that in the other direction. That certainly begs for a > general solution, perhaps like one used by the RSP where the GDB core uses > generic signal numbers the target side translate to/from them as > appropriate (obviously the host OS may have no notion of signals at all). Yes. I'm not sure about creating a generic solution. The same solution can be adapted to Solaris or other OS's which have different signal value assignments, but a solution for a target which doesn't have signals, that would call for a target-based (i.e., gdbserver or core generation) solution. > That looks like a post-7.5 material to me though -- we've lived with this > problem for long enough that I don't think we need to rush and correct it > a little more than a week before the release branch. Right. > >> Index: gdb/gdb/mips-linux-tdep.c >> =================================================================== >> --- gdb/.CC/cache/mips-linux-tdep.c@@/main/gdb_7_2_dev/3 2012-05-19 18:25:39.000077000 -0700 >> +++ gdb/mips-linux-tdep.c 2012-05-19 18:25:27.000033000 -0700 >> @@ -1130,6 +1130,101 @@ mips_linux_syscall_next_pc (struct frame >> return pc + 4; >> } >> >> +/* Translate signals based on MIPS signal values. >> + Adapted from gdb/common/signals.c. */ >> + >> +static enum target_signal >> +mips_target_signal_from_host (struct gdbarch *gdbarch, int signo) >> +{ >> + printf ("MJE: mips_target_signal_from_host (<gdbarch>, signo=%d)\n", signo); > > I'm assuming this is a debug message, please drop it. Otherwise we use > calls like fprintf_unfiltered (...) to produce output; see other sources > for a reference. Yes, it's my internal debugging message. I forgot to remove it. Mostly, I wanted to get an idea of whether I was heading in the wrong direction. I'll clean up the formatting issue you mentioned when I post the finished patch. > >> + >> + switch (signo) { >> + case 0: return TARGET_SIGNAL_0; >> + case MIPS_SIGHUP: return TARGET_SIGNAL_HUP; >> + case MIPS_SIGINT: return TARGET_SIGNAL_INT; >> + case MIPS_SIGQUIT: return TARGET_SIGNAL_QUIT; >> + case MIPS_SIGILL: return TARGET_SIGNAL_ILL; >> + case MIPS_SIGTRAP: return TARGET_SIGNAL_TRAP; >> + case MIPS_SIGABRT: return TARGET_SIGNAL_ABRT; >> + case MIPS_SIGEMT: return TARGET_SIGNAL_EMT; >> + case MIPS_SIGFPE: return TARGET_SIGNAL_FPE; >> + case MIPS_SIGKILL: return TARGET_SIGNAL_KILL; >> + case MIPS_SIGBUS: return TARGET_SIGNAL_BUS; >> + case MIPS_SIGSEGV: return TARGET_SIGNAL_SEGV; >> + case MIPS_SIGSYS: return TARGET_SIGNAL_SYS; >> + case MIPS_SIGPIPE: return TARGET_SIGNAL_PIPE; >> + case MIPS_SIGALRM: return TARGET_SIGNAL_ALRM; >> + case MIPS_SIGTERM: return TARGET_SIGNAL_TERM; >> + case MIPS_SIGUSR1: return TARGET_SIGNAL_USR1; >> + case MIPS_SIGUSR2: return TARGET_SIGNAL_USR2; >> + case MIPS_SIGCHLD: return TARGET_SIGNAL_CHLD; >> + case MIPS_SIGPWR: return TARGET_SIGNAL_PWR; >> + case MIPS_SIGWINCH: return TARGET_SIGNAL_WINCH; >> + case MIPS_SIGURG: return TARGET_SIGNAL_URG; >> + case MIPS_SIGPOLL: return TARGET_SIGNAL_POLL; >> + case MIPS_SIGSTOP: return TARGET_SIGNAL_STOP; >> + case MIPS_SIGTSTP: return TARGET_SIGNAL_TSTP; >> + case MIPS_SIGCONT: return TARGET_SIGNAL_CONT; >> + case MIPS_SIGTTIN: return TARGET_SIGNAL_TTIN; >> + case MIPS_SIGTTOU: return TARGET_SIGNAL_TTOU; >> + case MIPS_SIGVTALRM: return TARGET_SIGNAL_VTALRM; >> + case MIPS_SIGPROF: return TARGET_SIGNAL_PROF; >> + case MIPS_SIGXCPU: return TARGET_SIGNAL_XCPU; >> + case MIPS_SIGXFSZ: return TARGET_SIGNAL_XFSZ; >> + } > > All the case statements need to be on the next line, correctly indented, > e.g.: > > case MIPS_SIGXFSZ: > return TARGET_SIGNAL_XFSZ; > >> + >> +#ifdef _SIG >> +#endif > > Some debug leftover I presume, please drop it. > >> + >> + return TARGET_SIGNAL_UNKNOWN; >> +} > > Indentation: > > return TARGET_SIGNAL_UNKNOWN; > } > >> + >> +static int >> +mips_target_signal_to_host (struct gdbarch *gdbarch, enum target_signal signo) >> +{ >> + printf ("MJE: mips_target_signal_to_host (<gdbarch>, signo=%d)\n", signo); > > Same as above. > >> + >> + switch (signo) { >> + case TARGET_SIGNAL_0: return 0; >> + case TARGET_SIGNAL_HUP: return MIPS_SIGHUP; >> + case TARGET_SIGNAL_INT: return MIPS_SIGINT; >> + case TARGET_SIGNAL_QUIT: return MIPS_SIGQUIT; >> + case TARGET_SIGNAL_ILL: return MIPS_SIGILL; >> + case TARGET_SIGNAL_TRAP: return MIPS_SIGTRAP; >> + case TARGET_SIGNAL_ABRT: return MIPS_SIGABRT; >> + case TARGET_SIGNAL_EMT: return MIPS_SIGEMT; >> + case TARGET_SIGNAL_FPE: return MIPS_SIGFPE; >> + case TARGET_SIGNAL_KILL: return MIPS_SIGKILL; >> + case TARGET_SIGNAL_BUS: return MIPS_SIGBUS; >> + case TARGET_SIGNAL_SEGV: return MIPS_SIGSEGV; >> + case TARGET_SIGNAL_SYS: return MIPS_SIGSYS; >> + case TARGET_SIGNAL_PIPE: return MIPS_SIGPIPE; >> + case TARGET_SIGNAL_ALRM: return MIPS_SIGALRM; >> + case TARGET_SIGNAL_TERM: return MIPS_SIGTERM; >> + case TARGET_SIGNAL_USR1: return MIPS_SIGUSR1; >> + case TARGET_SIGNAL_USR2: return MIPS_SIGUSR2; >> + case TARGET_SIGNAL_CHLD: return MIPS_SIGCHLD; >> + case TARGET_SIGNAL_PWR: return MIPS_SIGPWR; >> + case TARGET_SIGNAL_WINCH: return MIPS_SIGWINCH; >> + case TARGET_SIGNAL_URG: return MIPS_SIGURG; >> + case TARGET_SIGNAL_POLL: return MIPS_SIGPOLL; >> + case TARGET_SIGNAL_STOP: return MIPS_SIGSTOP; >> + case TARGET_SIGNAL_TSTP: return MIPS_SIGTSTP; >> + case TARGET_SIGNAL_CONT: return MIPS_SIGCONT; >> + case TARGET_SIGNAL_TTIN: return MIPS_SIGTTIN; >> + case TARGET_SIGNAL_TTOU: return MIPS_SIGTTOU; >> + case TARGET_SIGNAL_VTALRM: return MIPS_SIGVTALRM; >> + case TARGET_SIGNAL_PROF: return MIPS_SIGPROF; >> + case TARGET_SIGNAL_XCPU: return MIPS_SIGXCPU; >> + case TARGET_SIGNAL_XFSZ: return MIPS_SIGXFSZ; > > Ditto. > >> + >> + default: >> + warning ("Signal %s does not exist on this system.\n", >> + target_signal_to_name (signo)); >> + return 0; >> + } >> +} >> + >> /* Initialize one of the GNU/Linux OS ABIs. */ >> >> static void >> @@ -1203,6 +1298,11 @@ mips_linux_init_abi (struct gdbarch_info >> set_gdbarch_core_read_description (gdbarch, >> mips_linux_core_read_description); >> >> + set_gdbarch_target_signal_from_host (gdbarch, >> + mips_target_signal_from_host); >> + set_gdbarch_target_signal_to_host (gdbarch, >> + mips_target_signal_to_host); >> + > > Indentation, leading tabs must be used: > > set_gdbarch_target_signal_from_host (gdbarch, > mips_target_signal_from_host); > set_gdbarch_target_signal_to_host (gdbarch, > mips_target_signal_to_host); > >> tdep->syscall_next_pc = mips_linux_syscall_next_pc; >> >> if (tdesc_data) >> >> Index: gdb/gdb/mips-linux-tdep.h >> =================================================================== >> --- gdb/.CC/cache/mips-linux-tdep.h@@/main/gdb_7_2_dev/1 2012-05-19 18:25:40.000041000 -0700 >> +++ gdb/mips-linux-tdep.h 2012-05-19 10:53:12.000064000 -0700 >> @@ -100,3 +100,43 @@ enum { >> /* Return 1 if MIPS_RESTART_REGNUM is usable. */ >> >> int mips_linux_restart_reg_p (struct gdbarch *gdbarch); >> + >> +/* MIPS Signals -- adapted from linux/arch/mips/include/asm/signal.h. */ >> + >> +enum mips_signals { >> + MIPS_SIGHUP = 1, /* Hangup (POSIX). */ > > Indentation: > > enum mips_signals > { > MIPS_SIGHUP = 1, /* Hangup (POSIX). */ > > etc. > >> + MIPS_SIGINT = 2, /* Interrupt (ANSI). */ >> + MIPS_SIGQUIT = 3, /* Quit (POSIX). */ >> + MIPS_SIGILL = 4, /* Illegal instruction (ANSI). */ >> + MIPS_SIGTRAP = 5, /* Trace trap (POSIX). */ >> + MIPS_SIGIOT = 6, /* IOT trap (4.2 BSD). */ >> + MIPS_SIGABRT = MIPS_SIGIOT, /* Abort (ANSI). */ >> + MIPS_SIGEMT = 7, >> + MIPS_SIGFPE = 8, /* Floating-point exception (ANSI). */ >> + MIPS_SIGKILL = 9, /* Kill, unblockable (POSIX). */ >> + MIPS_SIGBUS = 10, /* BUS error (4.2 BSD). */ >> + MIPS_SIGSEGV = 11, /* Segmentation violation (ANSI). */ >> + MIPS_SIGSYS = 12, >> + MIPS_SIGPIPE = 13, /* Broken pipe (POSIX). */ >> + MIPS_SIGALRM = 14, /* Alarm clock (POSIX). */ >> + MIPS_SIGTERM = 15, /* Termination (ANSI). */ >> + MIPS_SIGUSR1 = 16, /* User-defined signal 1 (POSIX). */ >> + MIPS_SIGUSR2 = 17, /* User-defined signal 2 (POSIX). */ >> + MIPS_SIGCHLD = 18, /* Child status has changed (POSIX). */ >> + MIPS_SIGCLD = MIPS_SIGCHLD, /* Same as SIGCHLD (System V). */ >> + MIPS_SIGPWR = 19, /* Power failure restart (System V). */ >> + MIPS_SIGWINCH = 20, /* Window size change (4.3 BSD, Sun). */ >> + MIPS_SIGURG = 21, /* Urgent condition on socket (4.2 BSD). */ >> + MIPS_SIGIO = 22, /* I/O now possible (4.2 BSD). */ >> + MIPS_SIGPOLL = MIPS_SIGIO,/* Pollable event occurred (System V). */ >> + MIPS_SIGSTOP = 23, /* Stop, unblockable (POSIX). */ >> + MIPS_SIGTSTP = 24, /* Keyboard stop (POSIX). */ >> + MIPS_SIGCONT = 25, /* Continue (POSIX). */ >> + MIPS_SIGTTIN = 26, /* Background read from tty (POSIX). */ >> + MIPS_SIGTTOU = 27, /* Background write to tty (POSIX). */ >> + MIPS_SIGVTALRM= 28, /* Virtual alarm clock (4.2 BSD). */ > > Use a space between MIPS_SIGVTALRM and = here. > >> + MIPS_SIGPROF = 29, /* Profiling alarm clock (4.2 BSD). */ >> + MIPS_SIGXCPU = 30, /* CPU limit exceeded (4.2 BSD). */ >> + MIPS_SIGXFSZ = 31 /* File size limit exceeded (4.2 BSD). */ >> + }; >> + > > Otherwise OK, but let's wait for feedback from the others on the general > problem I noted above. Yep. -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-21 18:21 ` Michael Eager @ 2012-05-21 22:34 ` Maciej W. Rozycki 2012-05-22 9:38 ` Pedro Alves 0 siblings, 1 reply; 29+ messages in thread From: Maciej W. Rozycki @ 2012-05-21 22:34 UTC (permalink / raw) To: Michael Eager; +Cc: gdb-patches On Mon, 21 May 2012, Michael Eager wrote: > > What target are you specifically referring to? Can you describe a > > scenario where it happens? > > Debugging a MIPS core file. Pedro suggested that no one > would ever see this running gdbserver. Well, these are mutually exclusive -- you can't open a remote target and a core file both at a time, and it's only the signal number that's recorded in the core image as the cause of termination that's causing trouble. I wonder if there's more stuff there actually that needs mapping. > > Ah, so that is with the core file target used in a cross-debugger, right? > > Well, no wonder nobody has noticed, this is a very unusual scenario -- I > > have used GDB for at least 15 years now and may have had a need to examine > > a core file with a cross-debugger perhaps once or twice -- and then some > > common signals looked for like SIGSEGV or SIGILL are identity mapped (and > > I knew the signal that caused the core file dumped beforehand anyway as > > it's printed as the program is being killed anyway). > > Yes, this may be less common than I first thought. SEGVs are much > more common than SIGBUS. Actually you'll only see SIGBUS in two cases under MIPS/Linux: 1. An unaligned memory access -- these trap into the kernel if not made correctly (with the special unaligned access instructions) and are normally emulated by the kernel at a large performance expense for data accesses. You can still get one if you try to execute unaligned instructions (e.g. MIPS16 code on a processor that does not support that mode of execution; MIPS16 code addresses have the lowest bit set), try to access unaligned data outside the user address space (USEG or XUSEG as applicable; the lack of alignment takes precedence over MMU protection) or if you disable the kernel emulation with the obscure syscall that's there for special cases. 2. You actually get a true bus error signalled by system logic in an external bus transaction, e.g. an uncorrected memory ECC error or suchlike. As you can see it's all pretty obscure and uncommon stuff. > > Also by the look of it, such changes will actually be required for all > > processor/OS targets as obviously when you have e.g. a MIPS/IRIX-host > > cross-debugger targetting i386/Linux then the signal numbers won't match > > again, except that in the other direction. That certainly begs for a > > general solution, perhaps like one used by the RSP where the GDB core uses > > generic signal numbers the target side translate to/from them as > > appropriate (obviously the host OS may have no notion of signals at all). > > Yes. > > I'm not sure about creating a generic solution. The same solution can > be adapted to Solaris or other OS's which have different signal value > assignments, but a solution for a target which doesn't have signals, > that would call for a target-based (i.e., gdbserver or core generation) > solution. Targets with no signals are not a problem, they simply won't produce or accept any. It's the hosts that are a problem -- generic code in common/signals.c has stuff like this: #if defined (SIGHUP) case TARGET_SIGNAL_HUP: return SIGHUP; #endif that obviosuly won't work on on a host that doesn't have such a signal defined (I'm not sure if hosts like MinGW or DJGPP pretend that these signals exist there, that would make little sense; there may be others as well). As long as every target that supports signals provides its mapping similar to one you did, then I think we should be fine. And actually, as far as Linux is concerned, I suppose we can actually have a common mapping that will cover most of the processor architectures Linux supports as most if not all of the modern ports have standardised on a common set of magic numbers, as far as I know. So linux-tdep.c would default to that standard mapping and then any oddball port would override that choice with their own. It's only the few older ports that had their reasons, legitimate or not, to stick to other definitions that are different -- for the MIPS port for example there used to be an idea once to run IRIX binaries using a foreign kernel personality and an ABI compatibility layer (similar to iBCS the i386/Linux port used to support) -- reusing the same magic numbers would certainly make the effort easier (many kernel structures exposed via syscalls are like those from IRIX rather than other Linux ports for the very same reason). To the best of my knowledge this idea never fully materialised, but these provisions have stayed for backwards compatibility with the early decisions. Another example is the Alpha/Linux port and its actual possibility to run DEC OSF/1 (or Digital Unix or Tru64 Unix or whatever, hmm, lost track...) binaries; these are not ELF even, but ECOFF. There are probably a few more (<linux/personality.h> suggests SunOS and Solaris, presumably on SPARC and HP-UX presumably on HP-PA), but the rest should be pretty uniform. Maciej ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-21 22:34 ` Maciej W. Rozycki @ 2012-05-22 9:38 ` Pedro Alves 0 siblings, 0 replies; 29+ messages in thread From: Pedro Alves @ 2012-05-22 9:38 UTC (permalink / raw) To: Maciej W. Rozycki; +Cc: Michael Eager, gdb-patches On 05/21/2012 11:33 PM, Maciej W. Rozycki wrote: > And actually, as far as Linux is concerned, I suppose we can actually > have a common mapping that will cover most of the processor architectures > Linux supports as most if not all of the modern ports have standardised on > a common set of magic numbers, as far as I know. Assuming so ... > So linux-tdep.c would > default to that standard mapping and then any oddball port would override > that choice with their own. ... then very much agreed. -- Pedro Alves ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-20 2:03 MIPS Linux signals Michael Eager ` (2 preceding siblings ...) [not found] ` <alpine.DEB.1.10.1205211232260.11227@tp.orcam.me.uk> @ 2012-05-21 21:35 ` Pedro Alves 2012-05-21 21:53 ` Michael Eager 3 siblings, 1 reply; 29+ messages in thread From: Pedro Alves @ 2012-05-21 21:35 UTC (permalink / raw) To: Michael Eager; +Cc: gdb-patches On 05/20/2012 03:02 AM, Michael Eager wrote: > +static int > +mips_target_signal_to_host (struct gdbarch *gdbarch, enum target_signal signo) > +{ > + printf ("MJE: mips_target_signal_to_host (<gdbarch>, signo=%d)\n", signo); > + > + switch (signo) { > + case TARGET_SIGNAL_0: return 0; > + case TARGET_SIGNAL_HUP: return MIPS_SIGHUP; BTW, I wouldn't bother with gdbarch_target_signal_to_host. Nothing ever calls it. -- Pedro Alves ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-21 21:35 ` Pedro Alves @ 2012-05-21 21:53 ` Michael Eager 2012-05-21 22:48 ` Maciej W. Rozycki 0 siblings, 1 reply; 29+ messages in thread From: Michael Eager @ 2012-05-21 21:53 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches On 05/21/2012 02:34 PM, Pedro Alves wrote: > On 05/20/2012 03:02 AM, Michael Eager wrote: > >> +static int >> +mips_target_signal_to_host (struct gdbarch *gdbarch, enum target_signal signo) >> +{ >> + printf ("MJE: mips_target_signal_to_host (<gdbarch>, signo=%d)\n", signo); >> + >> + switch (signo) { >> + case TARGET_SIGNAL_0: return 0; >> + case TARGET_SIGNAL_HUP: return MIPS_SIGHUP; > > > BTW, I wouldn't bother with gdbarch_target_signal_to_host. Nothing ever calls it. I hadn't noticed that. I thought that it was called to translate the signal number when sent to the target. Instead, target_signal_to_host() is called. -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-21 21:53 ` Michael Eager @ 2012-05-21 22:48 ` Maciej W. Rozycki 2012-05-22 0:16 ` Michael Eager ` (2 more replies) 0 siblings, 3 replies; 29+ messages in thread From: Maciej W. Rozycki @ 2012-05-21 22:48 UTC (permalink / raw) To: Michael Eager; +Cc: Pedro Alves, gdb-patches On Mon, 21 May 2012, Michael Eager wrote: > > BTW, I wouldn't bother with gdbarch_target_signal_to_host. Nothing ever > > calls it. > > I hadn't noticed that. I thought that it was called to translate > the signal number when sent to the target. Instead, target_signal_to_host() > is called. Shall we drop the unused gdbarch API so as to avoid further confusion then? Shouldn't target_signal_from_host be renamed to something closer to what it really does, e.g. signal_from_target? It's not that host signals really ever matter unless host == target in which case they're still target signals too (this observation applies to gdbserver as well). Maciej ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-21 22:48 ` Maciej W. Rozycki @ 2012-05-22 0:16 ` Michael Eager 2012-05-22 10:17 ` Pedro Alves 2012-05-22 10:58 ` Pedro Alves 2 siblings, 0 replies; 29+ messages in thread From: Michael Eager @ 2012-05-22 0:16 UTC (permalink / raw) To: Maciej W. Rozycki; +Cc: Pedro Alves, gdb-patches On 05/21/2012 03:48 PM, Maciej W. Rozycki wrote: > On Mon, 21 May 2012, Michael Eager wrote: > >>> BTW, I wouldn't bother with gdbarch_target_signal_to_host. Nothing ever >>> calls it. >> >> I hadn't noticed that. I thought that it was called to translate >> the signal number when sent to the target. Instead, target_signal_to_host() >> is called. > > Shall we drop the unused gdbarch API so as to avoid further confusion > then? Shouldn't target_signal_from_host be renamed to something closer to > what it really does, e.g. signal_from_target? It's not that host signals > really ever matter unless host == target in which case they're still > target signals too (this observation applies to gdbserver as well). This would be OK with me. The gdbarch_target_signal_to_host interface suggests that this should differ for different targets. There is an old bug report about the unclear name for the function. gdbarch_signal_from_target would be a better name. -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-21 22:48 ` Maciej W. Rozycki 2012-05-22 0:16 ` Michael Eager @ 2012-05-22 10:17 ` Pedro Alves 2012-05-22 13:16 ` Maciej W. Rozycki ` (3 more replies) 2012-05-22 10:58 ` Pedro Alves 2 siblings, 4 replies; 29+ messages in thread From: Pedro Alves @ 2012-05-22 10:17 UTC (permalink / raw) To: Maciej W. Rozycki; +Cc: Michael Eager, gdb-patches On 05/21/2012 11:48 PM, Maciej W. Rozycki wrote: > On Mon, 21 May 2012, Michael Eager wrote: > >>> BTW, I wouldn't bother with gdbarch_target_signal_to_host. Nothing ever >>> calls it. >> >> I hadn't noticed that. I thought that it was called to translate >> the signal number when sent to the target. Instead, target_signal_to_host() >> is called. > > Shall we drop the unused gdbarch API so as to avoid further confusion > then? Shouldn't target_signal_from_host be renamed to something closer to what > it really does, e.g. signal_from_target? I agree that the current naming is confusing, but I'll point out why I think target_signal_from_host is actually correct, and then conclude with renaming suggestions to avoid the confusion, and fix the naming of the gdbarch hook, which I think is not correct. We have enum target_signal { ... TARGET_SIGNAL_FOO, TARGET_SIGNAL_BAR, ... }; which represents GDB's internal signal numbers. AFAIK, the "target_" prefix naming in this case is just a natural choice, given that that's how we name everything behind the target_ops abstraction -- target_read_memory, target_resume, etc., etc. (target.h) -- the mechanism that is mostly about abstracting the details of handling the target's debug interface API. So this explains the "target_signal_from_" part of the function's signature. The "from_host" part is really correct: this really is a host function -- think in terms of autoconf's notion of build vs host vs target. It only makes sense to call it in native code (native == host). And in fact, that's what happens. Grepping for calls in GDB we see: ===================== arch-utils.c arch-utils.h This is default_target_signal_from_host . Which is the gdbarch fallback, which only works on native cores. ===================== corelow.c /* NOTE: target_signal_from_host() converts a target signal value into gdb's internal signal value. Unfortunately gdb's internal value is called ``target_signal'' and this function got the name ..._from_host(). */ enum target_signal sig = (core_gdbarch != NULL ? gdbarch_target_signal_from_host (core_gdbarch, siggy) : target_signal_from_host (siggy)); printf_filtered (_("Program terminated with signal %d, %s.\n"), siggy, target_signal_to_string (sig)); This is the only place the gdbarch method is called, which is handling the cross-core scenario. If we don't have a gdbarch hook to do the translation for us, then we fallback to assuming we're debugging a native core, one that was generated on the host (or a similar machine). ===================== darwin-nat.c Native code. ===================== gdbarch.c gdbarch.h gdbarch.sh The gdbarch hook. ===================== gnu-nat.c inf-ttrace.c linux-nat.c linux-thread-db.c nto-procfs.c procfs.c Native code. ===================== remote-mips.c Just a comment, but actually interesting in the MIPS context. /* Return the signal corresponding to SIG, where SIG is the number which the MIPS protocol uses for the signal. */ static enum target_signal mips_signal_from_protocol (int sig) { /* We allow a few more signals than the IDT board actually returns, on the theory that there is at least *some* hope that perhaps the numbering for these signals is widely agreed upon. */ if (sig <= 0 || sig > 31) return TARGET_SIGNAL_UNKNOWN; /* Don't want to use target_signal_from_host because we are converting from MIPS signal numbers, not host ones. Our internal numbers match the MIPS numbers for the signals the board can return, which are: SIGINT, SIGSEGV, SIGBUS, SIGILL, SIGFPE, SIGTRAP. */ return (enum target_signal) sig; } ===================== target.c Actually native code. I think this could move to inf-child.c. /* Helper function for child_wait and the derivatives of child_wait. HOSTSTATUS is the waitstatus from wait() or the equivalent; store our translation of that in OURSTATUS. */ void store_waitstatus (struct target_waitstatus *ourstatus, int hoststatus) { if (WIFEXITED (hoststatus)) { ourstatus->kind = TARGET_WAITKIND_EXITED; ourstatus->value.integer = WEXITSTATUS (hoststatus); } else if (!WIFSTOPPED (hoststatus)) { ourstatus->kind = TARGET_WAITKIND_SIGNALLED; ourstatus->value.sig = target_signal_from_host (WTERMSIG (hoststatus)); } else { ourstatus->kind = TARGET_WAITKIND_STOPPED; ourstatus->value.sig = target_signal_from_host (WSTOPSIG (hoststatus)); } } ===================== windows-nat.c Native code. ===================== gdbserver is always native, so always host. Now, it's the mix between somewhat different etymologies (enum target_signal, vs build/host/target) that leads to confusion. Renaming enum target_signal to enum gdb_signal would fix that side of the problem. = The gdbarch hook = Another source of confusion is that the gdbarch hook is naturally not a host function, it concerns with converting a foreign target's signal numbers (for cross debugging, not host numbers, unless in the special case of host == target) to gdb's internal numbers. So in that sense, gdbarch_target_signal_from_host is the wrong name. It should be called gdbarch_target_signal_from_target (!) with the default, as today, being target_signal_from_host. Obviously, gdbarch_target_signal_from_target is even more confusing. :-) The two "targets" in the name refer to somewhat different ideas. So I think that to sort all of this out, we should: enum target_signal => enum gdb_signal target_signal_from_host => gdb_signal_from_host (or gdb_signal_from_host_signal) target_signal_to_host => gdb_signal_to_host (or gdb_signal_to_host_signal) gdbarch_target_signal_from_host => gdbarch_gdb_signal_from_target (or gdbarch_gdb_signal_from_target_signal) gdbarch_target_signal_to_host => gdbarch_gdb_signal_to_target (or gdbarch_gdb_signal_to_target_signal) -- Pedro Alves ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-22 10:17 ` Pedro Alves @ 2012-05-22 13:16 ` Maciej W. Rozycki 2012-05-22 13:32 ` Pedro Alves 2012-05-22 15:10 ` Move store_waitstatus to inf-child.c (was: Re: MIPS Linux signals) Pedro Alves ` (2 subsequent siblings) 3 siblings, 1 reply; 29+ messages in thread From: Maciej W. Rozycki @ 2012-05-22 13:16 UTC (permalink / raw) To: Pedro Alves; +Cc: Michael Eager, gdb-patches On Tue, 22 May 2012, Pedro Alves wrote: > So I think that to sort all of this out, we should: > > enum target_signal => enum gdb_signal > > target_signal_from_host => gdb_signal_from_host (or gdb_signal_from_host_signal) > target_signal_to_host => gdb_signal_to_host (or gdb_signal_to_host_signal) > > gdbarch_target_signal_from_host => gdbarch_gdb_signal_from_target (or gdbarch_gdb_signal_from_target_signal) > gdbarch_target_signal_to_host => gdbarch_gdb_signal_to_target (or gdbarch_gdb_signal_to_target_signal) How about we call the context of the signal "inferior" here and therefore avoid the confusion which target is the host and which host is the target altogether? I.e.: enum target_signal => enum gdb_signal target_signal_from_host => gdb_signal_from_inferior target_signal_to_host => gdb_signal_to_inferior gdbarch_target_signal_from_host => gdbarch_gdb_signal_from_inferior gdbarch_target_signal_to_host => gdbarch_gdb_signal_to_inferior Maciej ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-22 13:16 ` Maciej W. Rozycki @ 2012-05-22 13:32 ` Pedro Alves 0 siblings, 0 replies; 29+ messages in thread From: Pedro Alves @ 2012-05-22 13:32 UTC (permalink / raw) To: Maciej W. Rozycki; +Cc: Michael Eager, gdb-patches On 05/22/2012 02:16 PM, Maciej W. Rozycki wrote: > On Tue, 22 May 2012, Pedro Alves wrote: > >> > So I think that to sort all of this out, we should: >> > >> > enum target_signal => enum gdb_signal >> > >> > target_signal_from_host => gdb_signal_from_host (or gdb_signal_from_host_signal) >> > target_signal_to_host => gdb_signal_to_host (or gdb_signal_to_host_signal) >> > >> > gdbarch_target_signal_from_host => gdbarch_gdb_signal_from_target (or gdbarch_gdb_signal_from_target_signal) >> > gdbarch_target_signal_to_host => gdbarch_gdb_signal_to_target (or gdbarch_gdb_signal_to_target_signal) > How about we call the context of the signal "inferior" here and therefore > avoid the confusion which target is the host and which host is the target > altogether? I.e.: > > enum target_signal => enum gdb_signal > > target_signal_from_host => gdb_signal_from_inferior > target_signal_to_host => gdb_signal_to_inferior > > gdbarch_target_signal_from_host => gdbarch_gdb_signal_from_inferior > gdbarch_target_signal_to_host => gdbarch_gdb_signal_to_inferior This blurs the point that gdb_signal_from_host/gdb_signal_from_inferior should never be called if not from native/host code, IOW, only when you want a mapping of the signal of the system the code is running on (host) and will thus add to confusion. I prefer my variant for making the distinction clear with the naming. -- Pedro Alves ^ permalink raw reply [flat|nested] 29+ messages in thread
* Move store_waitstatus to inf-child.c (was: Re: MIPS Linux signals) 2012-05-22 10:17 ` Pedro Alves 2012-05-22 13:16 ` Maciej W. Rozycki @ 2012-05-22 15:10 ` Pedro Alves 2012-05-22 15:40 ` MIPS Linux signals Michael Eager 2012-05-22 16:26 ` Pedro Alves 3 siblings, 0 replies; 29+ messages in thread From: Pedro Alves @ 2012-05-22 15:10 UTC (permalink / raw) To: gdb-patches On 05/22/2012 11:17 AM, Pedro Alves wrote: > ===================== > target.c > > Actually native code. I think this could move to inf-child.c. > > /* Helper function for child_wait and the derivatives of child_wait. > HOSTSTATUS is the waitstatus from wait() or the equivalent; store our > translation of that in OURSTATUS. */ > void > store_waitstatus (struct target_waitstatus *ourstatus, int hoststatus) This patch moves it. This is a function only used by (UNIX-ish) native targets, so inf-child.c is a better place. Interestingly, inf-child.o is always linked in through COMMON_OBS, instead of being picked up by .mh fragments... Build tested on X86_64 Fedora 16 as well as cross built for mingw. The amount of grepping I did makes me reasonably confident I haven't broken any target, but you never know... 2012-05-22 Pedro Alves <palves@redhat.com> * target.h (store_waitstatus): Move declaration ... * inf-child.h (store_waitstatus): ... here. * target.c: Move inclusion of gdb_wait.h, and ... (store_waitstatus): ... this ... * inf-child.c: ... here. * linux-nat.c: Include inf-child.h. * rs6000-nat.c: Include inf-child.h. * spu-linux-nat.c: Include inf-child.h. --- gdb/inf-child.c | 24 ++++++++++++++++++++++++ gdb/inf-child.h | 5 +++++ gdb/linux-nat.c | 1 + gdb/rs6000-nat.c | 1 + gdb/spu-linux-nat.c | 1 + gdb/target.c | 24 ------------------------ gdb/target.h | 4 ---- 7 files changed, 32 insertions(+), 28 deletions(-) diff --git a/gdb/inf-child.c b/gdb/inf-child.c index 5531102..08955ea 100644 --- a/gdb/inf-child.c +++ b/gdb/inf-child.c @@ -30,6 +30,7 @@ #include "inf-child.h" #include "gdb/fileio.h" #include "agent.h" +#include "gdb_wait.h" #ifdef HAVE_SYS_PARAM_H #include <sys/param.h> /* for MAXPATHLEN */ @@ -38,6 +39,29 @@ #include <fcntl.h> #include <unistd.h> +/* Helper function for child_wait and the derivatives of child_wait. + HOSTSTATUS is the waitstatus from wait() or the equivalent; store our + translation of that in OURSTATUS. */ +void +store_waitstatus (struct target_waitstatus *ourstatus, int hoststatus) +{ + if (WIFEXITED (hoststatus)) + { + ourstatus->kind = TARGET_WAITKIND_EXITED; + ourstatus->value.integer = WEXITSTATUS (hoststatus); + } + else if (!WIFSTOPPED (hoststatus)) + { + ourstatus->kind = TARGET_WAITKIND_SIGNALLED; + ourstatus->value.sig = target_signal_from_host (WTERMSIG (hoststatus)); + } + else + { + ourstatus->kind = TARGET_WAITKIND_STOPPED; + ourstatus->value.sig = target_signal_from_host (WSTOPSIG (hoststatus)); + } +} + /* Fetch register REGNUM from the inferior. If REGNUM is -1, do this for all registers. */ diff --git a/gdb/inf-child.h b/gdb/inf-child.h index d32c8cb..c394d5a 100644 --- a/gdb/inf-child.h +++ b/gdb/inf-child.h @@ -25,4 +25,9 @@ extern struct target_ops *inf_child_target (void); +/* Functions for helping to write a native target. */ + +/* This is for native targets which use a unix/POSIX-style waitstatus. */ +extern void store_waitstatus (struct target_waitstatus *, int); + #endif diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index ac1a0ea..86b3c7d 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -36,6 +36,7 @@ #include "gdbcmd.h" #include "regcache.h" #include "regset.h" +#include "inf-child.h" #include "inf-ptrace.h" #include "auxv.h" #include <sys/param.h> /* for MAXPATHLEN */ diff --git a/gdb/rs6000-nat.c b/gdb/rs6000-nat.c index 117ca01..b2b3e7e 100644 --- a/gdb/rs6000-nat.c +++ b/gdb/rs6000-nat.c @@ -31,6 +31,7 @@ #include "gdb-stabs.h" #include "regcache.h" #include "arch-utils.h" +#include "inf-child.h" #include "inf-ptrace.h" #include "ppc-tdep.h" #include "rs6000-tdep.h" diff --git a/gdb/spu-linux-nat.c b/gdb/spu-linux-nat.c index 7ddfc7a..235458e 100644 --- a/gdb/spu-linux-nat.c +++ b/gdb/spu-linux-nat.c @@ -23,6 +23,7 @@ #include "gdb_string.h" #include "target.h" #include "inferior.h" +#include "inf-child.h" #include "inf-ptrace.h" #include "regcache.h" #include "symfile.h" diff --git a/gdb/target.c b/gdb/target.c index 5361dbc..6dba936 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -29,7 +29,6 @@ #include "bfd.h" #include "symfile.h" #include "objfiles.h" -#include "gdb_wait.h" #include "dcache.h" #include <signal.h> #include "regcache.h" @@ -3650,29 +3649,6 @@ generic_mourn_inferior (void) deprecated_detach_hook (); } \f -/* Helper function for child_wait and the derivatives of child_wait. - HOSTSTATUS is the waitstatus from wait() or the equivalent; store our - translation of that in OURSTATUS. */ -void -store_waitstatus (struct target_waitstatus *ourstatus, int hoststatus) -{ - if (WIFEXITED (hoststatus)) - { - ourstatus->kind = TARGET_WAITKIND_EXITED; - ourstatus->value.integer = WEXITSTATUS (hoststatus); - } - else if (!WIFSTOPPED (hoststatus)) - { - ourstatus->kind = TARGET_WAITKIND_SIGNALLED; - ourstatus->value.sig = target_signal_from_host (WTERMSIG (hoststatus)); - } - else - { - ourstatus->kind = TARGET_WAITKIND_STOPPED; - ourstatus->value.sig = target_signal_from_host (WSTOPSIG (hoststatus)); - } -} -\f /* Convert a normal process ID to a string. Returns the string in a static buffer. */ diff --git a/gdb/target.h b/gdb/target.h index 50a0ea6..84b462a 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -1857,10 +1857,6 @@ extern int baud_rate; extern int remote_timeout; \f -/* Functions for helping to write a native target. */ - -/* This is for native targets which use a unix/POSIX-style waitstatus. */ -extern void store_waitstatus (struct target_waitstatus *, int); /* Set the show memory breakpoints mode to show, and installs a cleanup to restore it back to the current value. */ ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-22 10:17 ` Pedro Alves 2012-05-22 13:16 ` Maciej W. Rozycki 2012-05-22 15:10 ` Move store_waitstatus to inf-child.c (was: Re: MIPS Linux signals) Pedro Alves @ 2012-05-22 15:40 ` Michael Eager 2012-05-22 16:02 ` Pedro Alves 2012-05-22 16:26 ` Pedro Alves 3 siblings, 1 reply; 29+ messages in thread From: Michael Eager @ 2012-05-22 15:40 UTC (permalink / raw) To: Pedro Alves; +Cc: Maciej W. Rozycki, gdb-patches On 05/22/2012 03:17 AM, Pedro Alves wrote: > I agree that the current naming is confusing, but I'll point out why I think > target_signal_from_host is actually correct, and then conclude with renaming > suggestions to avoid the confusion, and fix the naming of the gdbarch hook, which > I think is not correct. We have > > enum target_signal > { > ... > TARGET_SIGNAL_FOO, > TARGET_SIGNAL_BAR, > ... > }; > > which represents GDB's internal signal numbers.AFAIK, the "target_" prefix > naming in this case is just a natural choice, given that that's how we name > everything behind the target_ops abstraction -- target_read_memory, > target_resume, etc., etc. (target.h) -- the mechanism that is mostly about > abstracting the details of handling the target's debug interface API. As you say, these are GDB's internal values. They are not the values used on (some) targets. They are not the values used by (some) host systems either. There's a difference between target_read_memory() which actually reads a value from the memory on the target, and target_signal which is not the value from the target, but a translated internal value. > So this explains the "target_signal_from_" part of the function's signature. Not for me. :-) > The "from_host" part is really correct: this really is a host function -- think in > terms of autoconf's notion of build vs host vs target. It only makes sense to call > it in native code (native == host). And in fact, that's what happens. GDB tends to muddle host and target. Or perhaps the confusion is between the host environment and GDB's internal data structures. There is no need to translate signal numbers from the host system environment to the target system environment. There is a need to translate from GDB's internal numbering to the target numbering, and vice versa. There is nothing significant about the host's signal numbering that should affect how GDB works with the target, whether the target is a native process, remote system, or core file. When target=host, translating from the target's signal numbering to GDB's internal numbering should be identical to the case when target!=host. target_signal_from_host() makes no sense to me. I don't have a host core file; it's a target core file. > gdbserver is always native, so always host. That's the exception. But that should still use the target to/from internal translation function. > enum target_signal => enum gdb_signal > > target_signal_from_host => gdb_signal_from_host (or gdb_signal_from_host_signal) > target_signal_to_host => gdb_signal_to_host (or gdb_signal_to_host_signal) > > gdbarch_target_signal_from_host => gdbarch_gdb_signal_from_target (or gdbarch_gdb_signal_from_target_signal) > gdbarch_target_signal_to_host => gdbarch_gdb_signal_to_target (or gdbarch_gdb_signal_to_target_signal) OK, but I'd recommend target_signal_from_host => gdb_signal_from_target target_signal_to_host => gdb_signal_to_target This is symmetric with the gdbarch_ functions and clear that the function translates to/from target system values, not the host system. -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-22 15:40 ` MIPS Linux signals Michael Eager @ 2012-05-22 16:02 ` Pedro Alves 2012-05-22 18:14 ` Michael Eager 0 siblings, 1 reply; 29+ messages in thread From: Pedro Alves @ 2012-05-22 16:02 UTC (permalink / raw) To: Michael Eager; +Cc: Maciej W. Rozycki, gdb-patches On 05/22/2012 04:40 PM, Michael Eager wrote: > On 05/22/2012 03:17 AM, Pedro Alves wrote: > >> I agree that the current naming is confusing, but I'll point out why I think >> target_signal_from_host is actually correct, and then conclude with renaming >> suggestions to avoid the confusion, and fix the naming of the gdbarch hook, which >> I think is not correct. We have >> >> enum target_signal >> { >> ... >> TARGET_SIGNAL_FOO, >> TARGET_SIGNAL_BAR, >> ... >> }; >> >> which represents GDB's internal signal numbers.AFAIK, the "target_" prefix >> naming in this case is just a natural choice, given that that's how we name >> everything behind the target_ops abstraction -- target_read_memory, >> target_resume, etc., etc. (target.h) -- the mechanism that is mostly about >> abstracting the details of handling the target's debug interface API. > > As you say, these are GDB's internal values. They are not the values used on > (some) targets. They are not the values used by (some) host systems either. > > There's a difference between target_read_memory() which actually reads a > value from the memory on the target, and target_signal which is not the > value from the target, but a translated internal value. > >> So this explains the "target_signal_from_" part of the function's signature. > > Not for me. :-) Could it be you mean "I agree, but I think the naming of the enum could be clearer/better"? The enum is called "enum target_signal". So the function that converts an enum target_signal from something else is called "target_signal_from_...". That much seems clear enough to me. > >> The "from_host" part is really correct: this really is a host function -- think in >> terms of autoconf's notion of build vs host vs target. It only makes sense to call >> it in native code (native == host). And in fact, that's what happens. > > GDB tends to muddle host and target. Or perhaps the confusion is between the > host environment and GDB's internal data structures. Not sure what you're alluding to. > There is no need to translate signal numbers from the host system environment > to the target system environment. There is a need to translate from GDB's internal > numbering to the target numbering, and vice versa. There is nothing significant > about the host's signal numbering that should affect how GDB works with the target, > whether the target is a native process, remote system, or core file. Correct. Is that in disagreement with anything I said? It's just that GDB's internal signal numbering is called "enum target_signal". > > When target=host, translating from the target's signal numbering to GDB's > internal numbering should be identical to the case when target!=host. > > target_signal_from_host() makes no sense to me. It makes sense in the native/host-only parts of GDB. linux-nat.c and friends, etc. > I don't have a host core file; it's a target core file. Yes, that's why in corelow.c the conversion goes through the gdbarch method. corelow.c is not a native/host-only file (ideally anyway). We have the target_signal_from_host fallback for old ports that haven't yet bothered to convert to modern mechanisms, and can't cross-debug cores. > >> gdbserver is always native, so always host. > > That's the exception. So is all the native-debugger code within GDB; code which we want to actually split out of gdb and share/merge with gdbserver. > But that should still use the target to/from internal > translation function. Correct. > >> enum target_signal => enum gdb_signal >> >> target_signal_from_host => gdb_signal_from_host (or gdb_signal_from_host_signal) >> target_signal_to_host => gdb_signal_to_host (or gdb_signal_to_host_signal) >> >> gdbarch_target_signal_from_host => gdbarch_gdb_signal_from_target (or gdbarch_gdb_signal_from_target_signal) >> gdbarch_target_signal_to_host => gdbarch_gdb_signal_to_target (or gdbarch_gdb_signal_to_target_signal) > > OK, but I'd recommend > target_signal_from_host => gdb_signal_from_target > target_signal_to_host => gdb_signal_to_target > > This is symmetric with the gdbarch_ functions and clear that the function > translates to/from target system values, not the host system. But it's not what the functions do... They really convert from the host system signals, not the target's. I think the symmetry will only lead to people getting confused (which one to call in common/target-independent code?). -- Pedro Alves ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-22 16:02 ` Pedro Alves @ 2012-05-22 18:14 ` Michael Eager 2012-05-22 18:31 ` Pedro Alves 0 siblings, 1 reply; 29+ messages in thread From: Michael Eager @ 2012-05-22 18:14 UTC (permalink / raw) To: Pedro Alves; +Cc: Maciej W. Rozycki, gdb-patches On 05/22/2012 09:01 AM, Pedro Alves wrote: >>> target_signal_from_host => gdb_signal_from_host (or gdb_signal_from_host_signal) >>> target_signal_to_host => gdb_signal_to_host (or gdb_signal_to_host_signal) >>> >>> gdbarch_target_signal_from_host => gdbarch_gdb_signal_from_target (or gdbarch_gdb_signal_from_target_signal) >>> gdbarch_target_signal_to_host => gdbarch_gdb_signal_to_target (or gdbarch_gdb_signal_to_target_signal) >> >> OK, but I'd recommend >> target_signal_from_host => gdb_signal_from_target >> target_signal_to_host => gdb_signal_to_target >> >> This is symmetric with the gdbarch_ functions and clear that the function >> translates to/from target system values, not the host system. > > > But it's not what the functions do... They really convert from the host > system signals, not the target's. I think the symmetry will only lead to > people getting confused (which one to call in common/target-independent code?). If you are running GDB on a Windows host, for example, what host system signals are you translating? -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-22 18:14 ` Michael Eager @ 2012-05-22 18:31 ` Pedro Alves 2012-05-22 19:32 ` Michael Eager 0 siblings, 1 reply; 29+ messages in thread From: Pedro Alves @ 2012-05-22 18:31 UTC (permalink / raw) To: Michael Eager; +Cc: Maciej W. Rozycki, gdb-patches On 05/22/2012 07:14 PM, Michael Eager wrote: > On 05/22/2012 09:01 AM, Pedro Alves wrote: > >>>> target_signal_from_host => gdb_signal_from_host (or gdb_signal_from_host_signal) >>>> target_signal_to_host => gdb_signal_to_host (or gdb_signal_to_host_signal) >>>> >>>> gdbarch_target_signal_from_host => gdbarch_gdb_signal_from_target (or gdbarch_gdb_signal_from_target_signal) >>>> gdbarch_target_signal_to_host => gdbarch_gdb_signal_to_target (or gdbarch_gdb_signal_to_target_signal) >>> >>> OK, but I'd recommend >>> target_signal_from_host => gdb_signal_from_target >>> target_signal_to_host => gdb_signal_to_target >>> >>> This is symmetric with the gdbarch_ functions and clear that the function >>> translates to/from target system values, not the host system. >> >> >> But it's not what the functions do... They really convert from the host >> system signals, not the target's. I think the symmetry will only lead to >> people getting confused (which one to call in common/target-independent code?). > > If you are running GDB on a Windows host, for example, what host > system signals are you translating? There's only be one such call -- the one from within corelow.c, if there's no gdbarch_target_signal_from_host callback installed. (The Windows ports don't call target_signal_from_host anywhere (windows-nat.c and friends). And in that case, if you e.g., load a cygwin core, the target_signal_from_host fallback will try to convert the signal number as if it was a host signal number. If you're running g b on a cygwin host, you'll happen to get the right values. If you're debugging a core (that same core or of some other non-native arch) with a cross debugger, with a mingw hosted gdb, then target_signal_from_host will _still_ translate the signal numbers found in mingw's signal.h header. For reference, those are: #define SIGINT 2 /* Interactive attention */ #define SIGILL 4 /* Illegal instruction */ #define SIGFPE 8 /* Floating point error */ #define SIGSEGV 11 /* Segmentation violation */ #define SIGTERM 15 /* Termination request */ #define SIGBREAK 21 /* Control-break */ #define SIGABRT 22 /* Abnormal termination (abort) */ All other signal numbers you pass to target_signal_from_host will end up as TARGET_SIGNAL_UNKNOWN, due to the bunch of #ifdef SIGFOO bits in common/signals. Obviously, in most cases, this translation will be wrong. But the point to be taken is, target_signal_from_host _always_ translates the signal number passed as argument as if it was a host signal number, no matter what the target really is. -- Pedro Alves ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-22 18:31 ` Pedro Alves @ 2012-05-22 19:32 ` Michael Eager 2012-05-22 22:06 ` Pedro Alves 0 siblings, 1 reply; 29+ messages in thread From: Michael Eager @ 2012-05-22 19:32 UTC (permalink / raw) To: Pedro Alves; +Cc: Maciej W. Rozycki, gdb-patches On 05/22/2012 11:30 AM, Pedro Alves wrote: > On 05/22/2012 07:14 PM, Michael Eager wrote: > >> On 05/22/2012 09:01 AM, Pedro Alves wrote: >> >>>>> target_signal_from_host => gdb_signal_from_host (or gdb_signal_from_host_signal) >>>>> target_signal_to_host => gdb_signal_to_host (or gdb_signal_to_host_signal) >>>>> >>>>> gdbarch_target_signal_from_host => gdbarch_gdb_signal_from_target (or gdbarch_gdb_signal_from_target_signal) >>>>> gdbarch_target_signal_to_host => gdbarch_gdb_signal_to_target (or gdbarch_gdb_signal_to_target_signal) >>>> >>>> OK, but I'd recommend >>>> target_signal_from_host => gdb_signal_from_target >>>> target_signal_to_host => gdb_signal_to_target >>>> >>>> This is symmetric with the gdbarch_ functions and clear that the function >>>> translates to/from target system values, not the host system. >>> >>> > > >>> But it's not what the functions do... They really convert from the host >>> system signals, not the target's. I think the symmetry will only lead to >>> people getting confused (which one to call in common/target-independent code?). >> >> If you are running GDB on a Windows host, for example, what host >> system signals are you translating? > > > There's only be one such call -- the one from within corelow.c, if there's no > gdbarch_target_signal_from_host callback installed. (The Windows ports don't call > target_signal_from_host anywhere (windows-nat.c and friends). And in that case, if > you e.g., load a cygwin core, the target_signal_from_host fallback will try to > convert the signal number as if it was a host signal number. If you're running g > b on a cygwin host, you'll happen to get the right values. If you're debugging > a core (that same core or of some other non-native arch) with a cross debugger, with > a mingw hosted gdb, then target_signal_from_host will _still_ translate the signal > numbers found in mingw's signal.h header. For reference, those are: > > #define SIGINT 2 /* Interactive attention */ > #define SIGILL 4 /* Illegal instruction */ > #define SIGFPE 8 /* Floating point error */ > #define SIGSEGV 11 /* Segmentation violation */ > #define SIGTERM 15 /* Termination request */ > #define SIGBREAK 21 /* Control-break */ > #define SIGABRT 22 /* Abnormal termination (abort) */ > > All other signal numbers you pass to target_signal_from_host will end up > as TARGET_SIGNAL_UNKNOWN, due to the bunch of #ifdef SIGFOO bits in > common/signals. Obviously, in most cases, this translation will > be wrong. But the point to be taken is, target_signal_from_host _always_ > translates the signal number passed as argument as if it was a host > signal number, no matter what the target really is. I think the point is that a target signal number should always be treated as if it were from a target, never that it was from the host. All the #ifdefs checking if the host has this signal or that is confusing. The only thing that seems relevant to me is whether the target has these signals. If it happens that target==host, then these happen to be equivalent. But maintaining a clear target abstraction is a benefit. -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-22 19:32 ` Michael Eager @ 2012-05-22 22:06 ` Pedro Alves 0 siblings, 0 replies; 29+ messages in thread From: Pedro Alves @ 2012-05-22 22:06 UTC (permalink / raw) To: Michael Eager; +Cc: Maciej W. Rozycki, gdb-patches On 05/22/2012 08:31 PM, Michael Eager wrote: >> >> All other signal numbers you pass to target_signal_from_host will end up >> as TARGET_SIGNAL_UNKNOWN, due to the bunch of #ifdef SIGFOO bits in >> common/signals. Obviously, in most cases, this translation will >> be wrong. But the point to be taken is, target_signal_from_host _always_ >> translates the signal number passed as argument as if it was a host >> signal number, no matter what the target really is. > > I think the point is that a target signal number should > always be treated as if it were from a target, never > that it was from the host. The discussion on the table is not about what should or not should be design wise. The discussion is about the confusing target_signal_from_host method name. I've given ample explanation of why that should have "host" in its name. If we should eliminate that function and instead provide gdbarch mappings for all targets or not is a different discussion. The fact remains that target_signal_from_host exists, and is widely used. I'd very much like to retain "host" in its name to remind the reader that while it exists, its implementation really is just a mapping from the host signals. I'm now pretty much done with repeating the arguments. > target signal number should always be treated as if it were from a target, That requires GDB hardcoding signal mappings, and that's what the gdbarch hook is for. And that's why I proposed renaming it: http://sourceware.org/ml/gdb-patches/2012-05/msg00839.html -- Pedro Alves ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-22 10:17 ` Pedro Alves ` (2 preceding siblings ...) 2012-05-22 15:40 ` MIPS Linux signals Michael Eager @ 2012-05-22 16:26 ` Pedro Alves 3 siblings, 0 replies; 29+ messages in thread From: Pedro Alves @ 2012-05-22 16:26 UTC (permalink / raw) To: gdb-patches; +Cc: Maciej W. Rozycki, Michael Eager On 05/22/2012 11:17 AM, Pedro Alves wrote: > > enum target_signal => enum gdb_signal http://sourceware.org/ml/gdb-patches/2012-05/msg00831.html -- Pedro Alves ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-21 22:48 ` Maciej W. Rozycki 2012-05-22 0:16 ` Michael Eager 2012-05-22 10:17 ` Pedro Alves @ 2012-05-22 10:58 ` Pedro Alves 2012-05-22 19:31 ` Aleksandar Ristovski 2 siblings, 1 reply; 29+ messages in thread From: Pedro Alves @ 2012-05-22 10:58 UTC (permalink / raw) To: Maciej W. Rozycki; +Cc: Michael Eager, gdb-patches, Aleksandar Ristovski [Adding Aleksandar] Answering this one. On 05/21/2012 11:48 PM, Maciej W. Rozycki wrote: > Shall we drop the unused gdbarch API so as to avoid further confusion > then? If we don't find a use for it, yes. Aleksandar, we're discussing gdbarch_target_signal_from_host and gdbarch_target_signal_to_host. It turns out that uses to either of those were never added to GDB. gdbarch_target_signal_FROM_host's purpose is clear, and we're about to add a (new) use to fix the same situation you ran into at the time (cross core debugging). I'm wondering if you ever found a use for gdbarch_target_signal_TO_host that we should consider, though. -- Pedro Alves ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-22 10:58 ` Pedro Alves @ 2012-05-22 19:31 ` Aleksandar Ristovski 2012-05-22 21:55 ` Pedro Alves 0 siblings, 1 reply; 29+ messages in thread From: Aleksandar Ristovski @ 2012-05-22 19:31 UTC (permalink / raw) To: Pedro Alves; +Cc: Maciej W. Rozycki, Michael Eager, gdb-patches On 12-05-22 06:57 AM, Pedro Alves wrote: > [Adding Aleksandar] > > Answering this one. > > On 05/21/2012 11:48 PM, Maciej W. Rozycki wrote: > >> Shall we drop the unused gdbarch API so as to avoid further confusion >> then? > > > If we don't find a use for it, yes. > > Aleksandar, we're discussing gdbarch_target_signal_from_host and > gdbarch_target_signal_to_host. It turns out that uses to either of those > were never added to GDB. gdbarch_target_signal_FROM_host's purpose is clear, > and we're about to add a (new) use to fix the same situation you ran into at the > time (cross core debugging). I'm wondering if you ever found a use for > gdbarch_target_signal_TO_host that we should consider, though. > The API was added to introduce consistency between gdb's view of target's numeric signal values and actual numerical signal values of the target. In general case, they should *not* be viewed as the same, but rather as distinct numeric sets which happen to have common names. When cross-examining a core this becomes very obvious, but it is also very obvious when debugging remote target which has different numerical values for signals. I use both from_host and to_host. That being said, I'm not sure why I never submitted actual uses for nto target... I have it in our repository. Looking at the code now, I see why. I use it in our remote target (we have our own) and thus perform translation on-the-fly. Gdb receives correct GDB version as well as target (when gdb sends it). Thanks, Aleksandar ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-22 19:31 ` Aleksandar Ristovski @ 2012-05-22 21:55 ` Pedro Alves 2012-05-22 23:29 ` Aleksandar Ristovski 0 siblings, 1 reply; 29+ messages in thread From: Pedro Alves @ 2012-05-22 21:55 UTC (permalink / raw) To: Aleksandar Ristovski; +Cc: Maciej W. Rozycki, Michael Eager, gdb-patches On 05/22/2012 08:31 PM, Aleksandar Ristovski wrote: > On 12-05-22 06:57 AM, Pedro Alves wrote: >> Aleksandar, we're discussing gdbarch_target_signal_from_host and >> gdbarch_target_signal_to_host. It turns out that uses to either of those >> were never added to GDB. gdbarch_target_signal_FROM_host's purpose is clear, >> and we're about to add a (new) use to fix the same situation you ran into at the >> time (cross core debugging). I'm wondering if you ever found a use for >> gdbarch_target_signal_TO_host that we should consider, though. >> > > The API was added to introduce consistency between gdb's view of target's numeric signal values and actual > numerical signal values of the target. In general case, they should *not* be viewed as the same, but rather > as distinct numeric sets which happen to have common names. When cross-examining a core this becomes very > obvious, but it is also very obvious when debugging remote target which has different numerical values for signals. > > I use both from_host and to_host. I'm confused on the "when debugging remote target which has different numerical values for signals" part, because the target is not supposed to send anything but the generic "enum target_signal" back to GDB core. The core should never need to do such translation with any target other than the core target. > That being said, I'm not sure why I never submitted actual uses for nto target... I have it in our repository. > > > Looking at the code now, I see why. I use it in our remote target (we have our own) and thus perform translation on-the-fly. Gdb receives correct GDB version as well as target (when gdb sends it). So it sounds like there's no real use for the gdbarch method in _common_ code then, right? If that's the case, we should zap it from the FSF tree until we find such a use. -- Pedro Alves ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-22 21:55 ` Pedro Alves @ 2012-05-22 23:29 ` Aleksandar Ristovski 2012-05-23 11:39 ` Pedro Alves 0 siblings, 1 reply; 29+ messages in thread From: Aleksandar Ristovski @ 2012-05-22 23:29 UTC (permalink / raw) To: Pedro Alves; +Cc: Maciej W. Rozycki, Michael Eager, gdb-patches On 12-05-22 05:55 PM, Pedro Alves wrote: > On 05/22/2012 08:31 PM, Aleksandar Ristovski wrote: > >> On 12-05-22 06:57 AM, Pedro Alves wrote: > > >>> Aleksandar, we're discussing gdbarch_target_signal_from_host and >>> gdbarch_target_signal_to_host. It turns out that uses to either of those >>> were never added to GDB. gdbarch_target_signal_FROM_host's purpose is clear, >>> and we're about to add a (new) use to fix the same situation you ran into at the >>> time (cross core debugging). I'm wondering if you ever found a use for >>> gdbarch_target_signal_TO_host that we should consider, though. >>> >> >> The API was added to introduce consistency between gdb's view of target's numeric signal values and actual > >> numerical signal values of the target. In general case, they should *not* be viewed as the same, but rather >> as distinct numeric sets which happen to have common names. When cross-examining a core this becomes very >> obvious, but it is also very obvious when debugging remote target which has different numerical values for signals. > >> >> I use both from_host and to_host. > > > I'm confused on the "when debugging remote target which has different numerical values for signals" > part, because the target is not supposed to send anything but the generic "enum target_signal" back to > GDB core. The core should never need to do such translation with any target other than the > core target. In my case, translation happens in remote-nto.c which is our remote target. (our functional equivalent of gdbserver does not really know about gdb's enum target_signal). > >> That being said, I'm not sure why I never submitted actual uses for nto target... I have it in our repository. >> >> >> Looking at the code now, I see why. I use it in our remote target (we have our own) and thus perform translation on-the-fly. Gdb receives correct GDB version as well as target (when gdb sends it). > > > So it sounds like there's no real use for the gdbarch method in _common_ code then, right? If > that's the case, we should zap it from the FSF tree until we find such a use. > Fine by me. (FWIW, I like renaming enum target_signal to gdb_signal; it will clear up some confusion). ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: MIPS Linux signals 2012-05-22 23:29 ` Aleksandar Ristovski @ 2012-05-23 11:39 ` Pedro Alves 0 siblings, 0 replies; 29+ messages in thread From: Pedro Alves @ 2012-05-23 11:39 UTC (permalink / raw) To: Aleksandar Ristovski; +Cc: Maciej W. Rozycki, Michael Eager, gdb-patches On 05/23/2012 12:28 AM, Aleksandar Ristovski wrote: >> >> So it sounds like there's no real use for the gdbarch method in _common_ code then, right? If >> that's the case, we should zap it from the FSF tree until we find such a use. >> > Fine by me. Thanks. Applied. 2012-05-23 Pedro Alves <palves@redhat.com> * arch-utils.h (default_target_signal_to_host): Delete. * arch-utils.c (default_target_signal_to_host): Delete. * gdbarch.sh (target_signal_to_host): Remove. * gdbarch.h, gdbarch.c: Regenerate. --- gdb/arch-utils.c | 6 ------ gdb/arch-utils.h | 2 -- gdb/gdbarch.c | 24 ------------------------ gdb/gdbarch.h | 7 ------- gdb/gdbarch.sh | 3 --- 5 files changed, 0 insertions(+), 42 deletions(-) diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c index fabb515..c6866a9 100644 --- a/gdb/arch-utils.c +++ b/gdb/arch-utils.c @@ -793,12 +793,6 @@ default_gen_return_address (struct gdbarch *gdbarch, error (_("This architecture has no method to collect a return address.")); } -int -default_target_signal_to_host (struct gdbarch *gdbarch, enum target_signal ts) -{ - return target_signal_to_host (ts); -} - enum target_signal default_target_signal_from_host (struct gdbarch *gdbarch, int signo) { diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h index c2c3398..8f47635 100644 --- a/gdb/arch-utils.h +++ b/gdb/arch-utils.h @@ -174,7 +174,5 @@ extern const char *default_auto_wide_charset (void); extern enum target_signal default_target_signal_from_host (struct gdbarch *, int); -extern int default_target_signal_to_host (struct gdbarch *, - enum target_signal); #endif diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c index af2033b..8d009f3 100644 --- a/gdb/gdbarch.c +++ b/gdb/gdbarch.c @@ -261,7 +261,6 @@ struct gdbarch gdbarch_process_record_ftype *process_record; gdbarch_process_record_signal_ftype *process_record_signal; gdbarch_target_signal_from_host_ftype *target_signal_from_host; - gdbarch_target_signal_to_host_ftype *target_signal_to_host; gdbarch_get_siginfo_type_ftype *get_siginfo_type; gdbarch_record_special_symbol_ftype *record_special_symbol; gdbarch_get_syscall_number_ftype *get_syscall_number; @@ -429,7 +428,6 @@ struct gdbarch startup_gdbarch = 0, /* process_record */ 0, /* process_record_signal */ default_target_signal_from_host, /* target_signal_from_host */ - default_target_signal_to_host, /* target_signal_to_host */ 0, /* get_siginfo_type */ 0, /* record_special_symbol */ 0, /* get_syscall_number */ @@ -539,7 +537,6 @@ gdbarch_alloc (const struct gdbarch_info *info, gdbarch->displaced_step_location = NULL; gdbarch->relocate_instruction = NULL; gdbarch->target_signal_from_host = default_target_signal_from_host; - gdbarch->target_signal_to_host = default_target_signal_to_host; gdbarch->has_shared_address_space = default_has_shared_address_space; gdbarch->fast_tracepoint_valid_at = default_fast_tracepoint_valid_at; gdbarch->auto_charset = default_auto_charset; @@ -731,7 +728,6 @@ verify_gdbarch (struct gdbarch *gdbarch) /* Skip verify of process_record, has predicate. */ /* Skip verify of process_record_signal, has predicate. */ /* Skip verify of target_signal_from_host, invalid_p == 0 */ - /* Skip verify of target_signal_to_host, invalid_p == 0 */ /* Skip verify of get_siginfo_type, has predicate. */ /* Skip verify of record_special_symbol, has predicate. */ /* Skip verify of get_syscall_number, has predicate. */ @@ -1345,9 +1341,6 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file) "gdbarch_dump: target_signal_from_host = <%s>\n", host_address_to_string (gdbarch->target_signal_from_host)); fprintf_unfiltered (file, - "gdbarch_dump: target_signal_to_host = <%s>\n", - host_address_to_string (gdbarch->target_signal_to_host)); - fprintf_unfiltered (file, "gdbarch_dump: gdbarch_unwind_pc_p() = %d\n", gdbarch_unwind_pc_p (gdbarch)); fprintf_unfiltered (file, @@ -3812,23 +3805,6 @@ set_gdbarch_target_signal_from_host (struct gdbarch *gdbarch, } int -gdbarch_target_signal_to_host (struct gdbarch *gdbarch, enum target_signal ts) -{ - gdb_assert (gdbarch != NULL); - gdb_assert (gdbarch->target_signal_to_host != NULL); - if (gdbarch_debug >= 2) - fprintf_unfiltered (gdb_stdlog, "gdbarch_target_signal_to_host called\n"); - return gdbarch->target_signal_to_host (gdbarch, ts); -} - -void -set_gdbarch_target_signal_to_host (struct gdbarch *gdbarch, - gdbarch_target_signal_to_host_ftype target_signal_to_host) -{ - gdbarch->target_signal_to_host = target_signal_to_host; -} - -int gdbarch_get_siginfo_type_p (struct gdbarch *gdbarch) { gdb_assert (gdbarch != NULL); diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h index 5bc4f4d..4388a7d 100644 --- a/gdb/gdbarch.h +++ b/gdb/gdbarch.h @@ -946,13 +946,6 @@ typedef enum target_signal (gdbarch_target_signal_from_host_ftype) (struct gdbar extern enum target_signal gdbarch_target_signal_from_host (struct gdbarch *gdbarch, int signo); extern void set_gdbarch_target_signal_from_host (struct gdbarch *gdbarch, gdbarch_target_signal_from_host_ftype *target_signal_from_host); -/* Signal translation: translate GDB's signal number into inferior's host - signal number. */ - -typedef int (gdbarch_target_signal_to_host_ftype) (struct gdbarch *gdbarch, enum target_signal ts); -extern int gdbarch_target_signal_to_host (struct gdbarch *gdbarch, enum target_signal ts); -extern void set_gdbarch_target_signal_to_host (struct gdbarch *gdbarch, gdbarch_target_signal_to_host_ftype *target_signal_to_host); - /* Extra signal info inspection. Return a type suitable to inspect extra signal information. */ diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh index 3e3b126..9394677 100755 --- a/gdb/gdbarch.sh +++ b/gdb/gdbarch.sh @@ -775,9 +775,6 @@ M:int:process_record_signal:struct regcache *regcache, enum target_signal signal # Signal translation: translate inferior's signal (host's) number into # GDB's representation. m:enum target_signal:target_signal_from_host:int signo:signo::default_target_signal_from_host::0 -# Signal translation: translate GDB's signal number into inferior's host -# signal number. -m:int:target_signal_to_host:enum target_signal ts:ts::default_target_signal_to_host::0 # Extra signal info inspection. # ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2012-05-23 11:39 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-20 2:03 MIPS Linux signals Michael Eager
2012-05-21 11:04 ` Pedro Alves
2012-05-21 14:51 ` Michael Eager
2012-05-21 17:37 ` Pedro Alves
2012-05-21 18:06 ` Michael Eager
2012-05-21 18:19 ` Maciej W. Rozycki
[not found] ` <alpine.DEB.1.10.1205211232260.11227@tp.orcam.me.uk>
2012-05-21 18:21 ` Michael Eager
2012-05-21 22:34 ` Maciej W. Rozycki
2012-05-22 9:38 ` Pedro Alves
2012-05-21 21:35 ` Pedro Alves
2012-05-21 21:53 ` Michael Eager
2012-05-21 22:48 ` Maciej W. Rozycki
2012-05-22 0:16 ` Michael Eager
2012-05-22 10:17 ` Pedro Alves
2012-05-22 13:16 ` Maciej W. Rozycki
2012-05-22 13:32 ` Pedro Alves
2012-05-22 15:10 ` Move store_waitstatus to inf-child.c (was: Re: MIPS Linux signals) Pedro Alves
2012-05-22 15:40 ` MIPS Linux signals Michael Eager
2012-05-22 16:02 ` Pedro Alves
2012-05-22 18:14 ` Michael Eager
2012-05-22 18:31 ` Pedro Alves
2012-05-22 19:32 ` Michael Eager
2012-05-22 22:06 ` Pedro Alves
2012-05-22 16:26 ` Pedro Alves
2012-05-22 10:58 ` Pedro Alves
2012-05-22 19:31 ` Aleksandar Ristovski
2012-05-22 21:55 ` Pedro Alves
2012-05-22 23:29 ` Aleksandar Ristovski
2012-05-23 11:39 ` Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox