Re: MIPS Linux signals

Michael Eager
2012-05-21T18:21:37Z

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