From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9869 invoked by alias); 21 May 2012 18:21:37 -0000 Received: (qmail 9851 invoked by uid 22791); 21 May 2012 18:21:34 -0000 X-SWARE-Spam-Status: No, hits=-2.7 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED X-Spam-Check-By: sourceware.org Received: from shell4.bayarea.net (HELO shell4.bayarea.net) (209.128.82.1) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 21 May 2012 18:21:20 +0000 Received: (qmail 5874 invoked from network); 21 May 2012 11:21:20 -0700 Received: from c-76-102-3-160.hsd1.ca.comcast.net (HELO redwood.eagercon.com) (76.102.3.160) by shell4.bayarea.net with SMTP; 21 May 2012 11:21:19 -0700 Message-ID: <4FBA879F.2020107@eagerm.com> Date: Mon, 21 May 2012 18:21:00 -0000 From: Michael Eager User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120329 Thunderbird/11.0.1 MIME-Version: 1.0 To: "Maciej W. Rozycki" , "gdb-patches@sourceware.org" Subject: Re: MIPS Linux signals References: <4FB850CA.7090701@eagerm.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-05/txt/msg00783.txt.bz2 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 (, 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 (, 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