* [PATCH] MIPS Linux signals @ 2012-06-01 20:21 Michael Eager 2012-06-01 20:53 ` Maciej W. Rozycki 0 siblings, 1 reply; 10+ messages in thread From: Michael Eager @ 2012-06-01 20:21 UTC (permalink / raw) To: gdb-patches; +Cc: Maciej W. Rozycki, Pedro Alves [-- Attachment #1: Type: text/plain, Size: 340 bytes --] This patch adds a target function to translate MIPS Linux signals to GDB internal signal numbers. 2012-06-01 Michael Eager <eager@eagercon.com> * mips-linux-tdep.c (mips_gdb_signal_from_target): New * mips-linux-tdep.h (mips_signals): New -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077 [-- Attachment #2: mips.patch --] [-- Type: text/x-patch, Size: 5519 bytes --] Index: gdb/mips-linux-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/mips-linux-tdep.c,v retrieving revision 1.94 diff -r1.94 mips-linux-tdep.c 42a43 > #include "gdb_signals.h" 1332a1334,1428 > /* Translate signals based on MIPS signal values. > Adapted from gdb/common/signals.c. */ > > static enum gdb_signal > mips_gdb_signal_from_target (struct gdbarch *gdbarch, int signo) > { > switch (signo) > { > case 0: > return GDB_SIGNAL_0; > case MIPS_SIGHUP: > return GDB_SIGNAL_HUP; > case MIPS_SIGINT: > return GDB_SIGNAL_INT; > case MIPS_SIGQUIT: > return GDB_SIGNAL_QUIT; > case MIPS_SIGILL: > return GDB_SIGNAL_ILL; > case MIPS_SIGTRAP: > return GDB_SIGNAL_TRAP; > case MIPS_SIGABRT: > return GDB_SIGNAL_ABRT; > case MIPS_SIGEMT: > return GDB_SIGNAL_EMT; > case MIPS_SIGFPE: > return GDB_SIGNAL_FPE; > case MIPS_SIGKILL: > return GDB_SIGNAL_KILL; > case MIPS_SIGBUS: > return GDB_SIGNAL_BUS; > case MIPS_SIGSEGV: > return GDB_SIGNAL_SEGV; > case MIPS_SIGSYS: > return GDB_SIGNAL_SYS; > case MIPS_SIGPIPE: > return GDB_SIGNAL_PIPE; > case MIPS_SIGALRM: > return GDB_SIGNAL_ALRM; > case MIPS_SIGTERM: > return GDB_SIGNAL_TERM; > case MIPS_SIGUSR1: > return GDB_SIGNAL_USR1; > case MIPS_SIGUSR2: > return GDB_SIGNAL_USR2; > case MIPS_SIGCHLD: > return GDB_SIGNAL_CHLD; > case MIPS_SIGPWR: > return GDB_SIGNAL_PWR; > case MIPS_SIGWINCH: > return GDB_SIGNAL_WINCH; > case MIPS_SIGURG: > return GDB_SIGNAL_URG; > case MIPS_SIGPOLL: > return GDB_SIGNAL_POLL; > case MIPS_SIGSTOP: > return GDB_SIGNAL_STOP; > case MIPS_SIGTSTP: > return GDB_SIGNAL_TSTP; > case MIPS_SIGCONT: > return GDB_SIGNAL_CONT; > case MIPS_SIGTTIN: > return GDB_SIGNAL_TTIN; > case MIPS_SIGTTOU: > return GDB_SIGNAL_TTOU; > case MIPS_SIGVTALRM: > return GDB_SIGNAL_VTALRM; > case MIPS_SIGPROF: > return GDB_SIGNAL_PROF; > case MIPS_SIGXCPU: > return GDB_SIGNAL_XCPU; > case MIPS_SIGXFSZ: > return GDB_SIGNAL_XFSZ; > } > > #if defined (REALTIME_LO) > if (signo >= REALTIME_LO && signo < REALTIME_HI) > { > /* This block of GDB_SIGNAL_REALTIME value is in order. */ > if (33 <= signo && signo <= 63) > return (enum gdb_signal) > (signo - 33 + (int) GDB_SIGNAL_REALTIME_33); > else if (signo == 32) > return GDB_SIGNAL_REALTIME_32; > else if (64 <= signo && signo <= 127) > return (enum gdb_signal) > (signo - 64 + (int) GDB_SIGNAL_REALTIME_64); > else > error ("GDB bug: unrecognized real-time signal"); > } > #endif > > > return GDB_SIGNAL_UNKNOWN; > } > 1416a1513,1515 > set_gdbarch_gdb_signal_from_target (gdbarch, > mips_gdb_signal_from_target); > Index: gdb/mips-linux-tdep.h =================================================================== RCS file: /cvs/src/src/gdb/mips-linux-tdep.h,v retrieving revision 1.13 diff -r1.13 mips-linux-tdep.h 107a108,152 > > /* 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). */ > MIPS_SIGRTMIN = 32, /* Minimum RT signal. */ > MIPS_SIGRTMAX = 128-1 /* Maximum RT signal. */ > }; > > #define REALTIME_LO MIPS_SIGRTMIN > #define REALTIME_HI MIPS_SIGRTMAX ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] MIPS Linux signals 2012-06-01 20:21 [PATCH] MIPS Linux signals Michael Eager @ 2012-06-01 20:53 ` Maciej W. Rozycki 2012-06-01 21:07 ` Michael Eager 2012-06-01 22:52 ` [PATCH] MIPS Linux signals Michael Eager 0 siblings, 2 replies; 10+ messages in thread From: Maciej W. Rozycki @ 2012-06-01 20:53 UTC (permalink / raw) To: Michael Eager; +Cc: gdb-patches, Pedro Alves Hi Michael, > This patch adds a target function to translate MIPS Linux signals > to GDB internal signal numbers. > > 2012-06-01 Michael Eager <eager@eagercon.com> > > * mips-linux-tdep.c (mips_gdb_signal_from_target): New > * mips-linux-tdep.h (mips_signals): New Thanks, please make a unified diff (`diff -up') next time. Further notes below. Index: gdb/mips-linux-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/mips-linux-tdep.c,v retrieving revision 1.94 diff -r1.94 mips-linux-tdep.c 42a43 > #include "gdb_signals.h" 1332a1334,1428 > /* Translate signals based on MIPS signal values. > Adapted from gdb/common/signals.c. */ > > static enum gdb_signal > mips_gdb_signal_from_target (struct gdbarch *gdbarch, int signo) > { > switch (signo) > { > case 0: > return GDB_SIGNAL_0; > case MIPS_SIGHUP: > return GDB_SIGNAL_HUP; > case MIPS_SIGINT: > return GDB_SIGNAL_INT; > case MIPS_SIGQUIT: > return GDB_SIGNAL_QUIT; > case MIPS_SIGILL: > return GDB_SIGNAL_ILL; > case MIPS_SIGTRAP: > return GDB_SIGNAL_TRAP; > case MIPS_SIGABRT: > return GDB_SIGNAL_ABRT; > case MIPS_SIGEMT: > return GDB_SIGNAL_EMT; > case MIPS_SIGFPE: > return GDB_SIGNAL_FPE; > case MIPS_SIGKILL: > return GDB_SIGNAL_KILL; > case MIPS_SIGBUS: > return GDB_SIGNAL_BUS; > case MIPS_SIGSEGV: > return GDB_SIGNAL_SEGV; > case MIPS_SIGSYS: > return GDB_SIGNAL_SYS; > case MIPS_SIGPIPE: > return GDB_SIGNAL_PIPE; > case MIPS_SIGALRM: > return GDB_SIGNAL_ALRM; > case MIPS_SIGTERM: > return GDB_SIGNAL_TERM; > case MIPS_SIGUSR1: > return GDB_SIGNAL_USR1; > case MIPS_SIGUSR2: > return GDB_SIGNAL_USR2; > case MIPS_SIGCHLD: > return GDB_SIGNAL_CHLD; > case MIPS_SIGPWR: > return GDB_SIGNAL_PWR; > case MIPS_SIGWINCH: > return GDB_SIGNAL_WINCH; > case MIPS_SIGURG: > return GDB_SIGNAL_URG; > case MIPS_SIGPOLL: > return GDB_SIGNAL_POLL; > case MIPS_SIGSTOP: > return GDB_SIGNAL_STOP; > case MIPS_SIGTSTP: > return GDB_SIGNAL_TSTP; > case MIPS_SIGCONT: > return GDB_SIGNAL_CONT; > case MIPS_SIGTTIN: > return GDB_SIGNAL_TTIN; > case MIPS_SIGTTOU: > return GDB_SIGNAL_TTOU; > case MIPS_SIGVTALRM: > return GDB_SIGNAL_VTALRM; > case MIPS_SIGPROF: > return GDB_SIGNAL_PROF; > case MIPS_SIGXCPU: > return GDB_SIGNAL_XCPU; > case MIPS_SIGXFSZ: > return GDB_SIGNAL_XFSZ; > } > > #if defined (REALTIME_LO) Is this #ifdef needed? How about you rewrite this block using MIPS-specific definitions (MIPS_SIGRTMIN, etc.)? Please avoid hardcoded magic numbers too -- e.g. what are the magic values 32 and 33 below and how do they correlate to MIPS_SIGRTMIN, etc.? I can't get it from the change below. Should they be made relative to MIPS_SIGRTMIN perhaps? Are the values GDB_SIGNAL_REALTIME_* expand to not in order? Additionally, please watch your code formatting: > if (signo >= REALTIME_LO && signo < REALTIME_HI) > { > /* This block of GDB_SIGNAL_REALTIME value is in order. */ > if (33 <= signo && signo <= 63) > return (enum gdb_signal) > (signo - 33 + (int) GDB_SIGNAL_REALTIME_33); This needs extra brackets and indentation: return ((enum gdb_signal) (signo - 33 + (int) GDB_SIGNAL_REALTIME_33)); > else if (signo == 32) > return GDB_SIGNAL_REALTIME_32; > else if (64 <= signo && signo <= 127) > return (enum gdb_signal) > (signo - 64 + (int) GDB_SIGNAL_REALTIME_64); Likewise. > else > error ("GDB bug: unrecognized real-time signal"); > } > #endif > > > return GDB_SIGNAL_UNKNOWN; > } > 1416a1513,1515 > set_gdbarch_gdb_signal_from_target (gdbarch, > mips_gdb_signal_from_target); > Index: gdb/mips-linux-tdep.h =================================================================== RCS file: /cvs/src/src/gdb/mips-linux-tdep.h,v retrieving revision 1.13 diff -r1.13 mips-linux-tdep.h 107a108,152 > > /* 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). */ Please add a space after the comma here, I suggest replacing the tabs after MIPS_SIGIOT and MIPS_SIGCHLD above for consistency too. > 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). */ > MIPS_SIGRTMIN = 32, /* Minimum RT signal. */ > MIPS_SIGRTMAX = 128-1 /* Maximum RT signal. */ Missing spaces after full stops and around "-" above. > }; > > #define REALTIME_LO MIPS_SIGRTMIN > #define REALTIME_HI MIPS_SIGRTMAX Please resend with these clean-ups, as a unified diff, and I'll see if I need to infer any other changes from the context then. Thanks. Maciej ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] MIPS Linux signals 2012-06-01 20:53 ` Maciej W. Rozycki @ 2012-06-01 21:07 ` Michael Eager 2012-06-04 15:49 ` Pedro Alves 2012-06-01 22:52 ` [PATCH] MIPS Linux signals Michael Eager 1 sibling, 1 reply; 10+ messages in thread From: Michael Eager @ 2012-06-01 21:07 UTC (permalink / raw) To: Maciej W. Rozycki; +Cc: gdb-patches, Pedro Alves On 06/01/2012 01:53 PM, Maciej W. Rozycki wrote: > Hi Michael, > >> This patch adds a target function to translate MIPS Linux signals >> to GDB internal signal numbers. >> >> 2012-06-01 Michael Eager<eager@eagercon.com> >> >> * mips-linux-tdep.c (mips_gdb_signal_from_target): New >> * mips-linux-tdep.h (mips_signals): New > > Thanks, please make a unified diff (`diff -up') next time. Further notes > below. > > Index: gdb/mips-linux-tdep.c > =================================================================== > RCS file: /cvs/src/src/gdb/mips-linux-tdep.c,v > retrieving revision 1.94 > diff -r1.94 mips-linux-tdep.c > 42a43 >> #include "gdb_signals.h" > 1332a1334,1428 >> /* Translate signals based on MIPS signal values. >> Adapted from gdb/common/signals.c. */ >> >> static enum gdb_signal >> mips_gdb_signal_from_target (struct gdbarch *gdbarch, int signo) >> { >> switch (signo) >> { >> case 0: >> return GDB_SIGNAL_0; >> case MIPS_SIGHUP: >> return GDB_SIGNAL_HUP; >> case MIPS_SIGINT: >> return GDB_SIGNAL_INT; >> case MIPS_SIGQUIT: >> return GDB_SIGNAL_QUIT; >> case MIPS_SIGILL: >> return GDB_SIGNAL_ILL; >> case MIPS_SIGTRAP: >> return GDB_SIGNAL_TRAP; >> case MIPS_SIGABRT: >> return GDB_SIGNAL_ABRT; >> case MIPS_SIGEMT: >> return GDB_SIGNAL_EMT; >> case MIPS_SIGFPE: >> return GDB_SIGNAL_FPE; >> case MIPS_SIGKILL: >> return GDB_SIGNAL_KILL; >> case MIPS_SIGBUS: >> return GDB_SIGNAL_BUS; >> case MIPS_SIGSEGV: >> return GDB_SIGNAL_SEGV; >> case MIPS_SIGSYS: >> return GDB_SIGNAL_SYS; >> case MIPS_SIGPIPE: >> return GDB_SIGNAL_PIPE; >> case MIPS_SIGALRM: >> return GDB_SIGNAL_ALRM; >> case MIPS_SIGTERM: >> return GDB_SIGNAL_TERM; >> case MIPS_SIGUSR1: >> return GDB_SIGNAL_USR1; >> case MIPS_SIGUSR2: >> return GDB_SIGNAL_USR2; >> case MIPS_SIGCHLD: >> return GDB_SIGNAL_CHLD; >> case MIPS_SIGPWR: >> return GDB_SIGNAL_PWR; >> case MIPS_SIGWINCH: >> return GDB_SIGNAL_WINCH; >> case MIPS_SIGURG: >> return GDB_SIGNAL_URG; >> case MIPS_SIGPOLL: >> return GDB_SIGNAL_POLL; >> case MIPS_SIGSTOP: >> return GDB_SIGNAL_STOP; >> case MIPS_SIGTSTP: >> return GDB_SIGNAL_TSTP; >> case MIPS_SIGCONT: >> return GDB_SIGNAL_CONT; >> case MIPS_SIGTTIN: >> return GDB_SIGNAL_TTIN; >> case MIPS_SIGTTOU: >> return GDB_SIGNAL_TTOU; >> case MIPS_SIGVTALRM: >> return GDB_SIGNAL_VTALRM; >> case MIPS_SIGPROF: >> return GDB_SIGNAL_PROF; >> case MIPS_SIGXCPU: >> return GDB_SIGNAL_XCPU; >> case MIPS_SIGXFSZ: >> return GDB_SIGNAL_XFSZ; >> } >> >> #if defined (REALTIME_LO) > > Is this #ifdef needed? How about you rewrite this block using > MIPS-specific definitions (MIPS_SIGRTMIN, etc.)? Please avoid hardcoded > magic numbers too -- e.g. what are the magic values 32 and 33 below and > how do they correlate to MIPS_SIGRTMIN, etc.? I can't get it from the > change below. Should they be made relative to MIPS_SIGRTMIN perhaps? > Are the values GDB_SIGNAL_REALTIME_* expand to not in order? This code is copied from signals.c. I don't really want to re-design the logic since I don't have a straight-forward way to test the code. I'll recast this in terms of MIPS_SIGRTMIN, but all of your questions can be addressed to the code in signals.c. I didn't find it clear, either. > > Additionally, please watch your code formatting: I'll resend with formatting changes. > >> if (signo>= REALTIME_LO&& signo< REALTIME_HI) >> { >> /* This block of GDB_SIGNAL_REALTIME value is in order. */ >> if (33<= signo&& signo<= 63) >> return (enum gdb_signal) >> (signo - 33 + (int) GDB_SIGNAL_REALTIME_33); > > This needs extra brackets and indentation: > > return ((enum gdb_signal) > (signo - 33 + (int) GDB_SIGNAL_REALTIME_33)); > >> else if (signo == 32) >> return GDB_SIGNAL_REALTIME_32; >> else if (64<= signo&& signo<= 127) >> return (enum gdb_signal) >> (signo - 64 + (int) GDB_SIGNAL_REALTIME_64); > > Likewise. > >> else >> error ("GDB bug: unrecognized real-time signal"); >> } >> #endif >> >> >> return GDB_SIGNAL_UNKNOWN; >> } >> > 1416a1513,1515 >> set_gdbarch_gdb_signal_from_target (gdbarch, >> mips_gdb_signal_from_target); >> > Index: gdb/mips-linux-tdep.h > =================================================================== > RCS file: /cvs/src/src/gdb/mips-linux-tdep.h,v > retrieving revision 1.13 > diff -r1.13 mips-linux-tdep.h > 107a108,152 >> >> /* 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). */ > > Please add a space after the comma here, I suggest replacing the tabs > after MIPS_SIGIOT and MIPS_SIGCHLD above for consistency too. > >> 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). */ >> MIPS_SIGRTMIN = 32, /* Minimum RT signal. */ >> MIPS_SIGRTMAX = 128-1 /* Maximum RT signal. */ > > Missing spaces after full stops and around "-" above. > >> }; >> >> #define REALTIME_LO MIPS_SIGRTMIN >> #define REALTIME_HI MIPS_SIGRTMAX > > Please resend with these clean-ups, as a unified diff, and I'll see if I > need to infer any other changes from the context then. Thanks. > > Maciej > -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] MIPS Linux signals 2012-06-01 21:07 ` Michael Eager @ 2012-06-04 15:49 ` Pedro Alves 2012-06-04 16:26 ` [PATCH 2/2] Make gdbarch_gdb_signal_from_target a method with predicate Pedro Alves 2012-06-04 16:26 ` [PATCH 1/2] gdbarch_gdb_signal_from_target: Mention host independence Pedro Alves 0 siblings, 2 replies; 10+ messages in thread From: Pedro Alves @ 2012-06-04 15:49 UTC (permalink / raw) To: Michael Eager; +Cc: Maciej W. Rozycki, gdb-patches On 06/01/2012 10:07 PM, Michael Eager wrote: >>> #if defined (REALTIME_LO) >> >> Is this #ifdef needed? How about you rewrite this block using >> MIPS-specific definitions (MIPS_SIGRTMIN, etc.)? Please avoid hardcoded >> magic numbers too -- e.g. what are the magic values 32 and 33 below and >> how do they correlate to MIPS_SIGRTMIN, etc.? I can't get it from the >> change below. Should they be made relative to MIPS_SIGRTMIN perhaps? >> Are the values GDB_SIGNAL_REALTIME_* expand to not in order? > > This code is copied from signals.c. I don't really want to re-design > the logic since I don't have a straight-forward way to test the code. > > I'll recast this in terms of MIPS_SIGRTMIN, but all of your questions > can be addressed to the code in signals.c. I didn't find it clear, either. The whole point of gdb_signal_from_target is to be host independent. common/signals.c is all about host signal mapping. REALTIME_LO is either defined in the native header files in gdb/config/ (the nm-*.h files), or, if not defined there, common/signals.c does: #ifndef REALTIME_LO # if defined(__SIGRTMIN) # define REALTIME_LO __SIGRTMIN # define REALTIME_HI (__SIGRTMAX + 1) # elif defined(SIGRTMIN) # define REALTIME_LO SIGRTMIN # define REALTIME_HI (SIGRTMAX + 1) # endif #endif So that '#if defined (REALTIME_LO)' is really wrong for the gdbarch hook. The only nm header that defines REALTIME_LO presently, making the config/signals.c #ifndef be bypassed, is config/nm-nto.h: /* Setup the valid realtime signal range. */ #define REALTIME_LO 41 #define REALTIME_HI 56 So only when you build GDB hosted for QNX NTO would you see that new REALTIME_LO block in the mips tdep file be compiled... On all other hosts, including native GNU/Linux MIPS, you'd miss the conversions for all the realtime signals. I'm doing adding a few more comments to the gdbarch hook, plus doing a code change (that doesn't affect your patch) that hopefully makes things a bit clearer. -- Pedro Alves ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] Make gdbarch_gdb_signal_from_target a method with predicate. 2012-06-04 15:49 ` Pedro Alves @ 2012-06-04 16:26 ` Pedro Alves 2012-06-04 16:26 ` [PATCH 1/2] gdbarch_gdb_signal_from_target: Mention host independence Pedro Alves 1 sibling, 0 replies; 10+ messages in thread From: Pedro Alves @ 2012-06-04 16:26 UTC (permalink / raw) To: gdb-patches This makes gdb_signal_from_target a method with a predicate, with the intent being to remove the host signal translation fallback from the hook itself, and instead do it at the caller, which actually is already doing the fallback. This reinforces the "must be host independent" comment. Tested on x86_64 Fedora 16, and applied. 2012-06-04 Pedro Alves <palves@redhat.com> gdb/ * arch-utils.c (default_gdb_signal_from_target): Delete. * arch-utils.h (default_gdb_signal_from_target): Delete. * corelow.c (core_open) <signal mapping>: Extended comment. Check gdbarch_gdb_signal_from_target_p. * gdbarch.sh (gdb_signal_from_target): Make it an M method (with predicate). * gdbarch.h: Regenerate. * gdbarch.c: Regenerate. --- gdb/arch-utils.c | 9 --------- gdb/arch-utils.h | 3 --- gdb/corelow.c | 13 +++++++++---- gdb/gdbarch.c | 15 ++++++++++++--- gdb/gdbarch.h | 5 +++-- gdb/gdbarch.sh | 5 ++--- 6 files changed, 26 insertions(+), 24 deletions(-) diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c index 98cf4b8..e683a2d 100644 --- a/gdb/arch-utils.c +++ b/gdb/arch-utils.c @@ -793,15 +793,6 @@ default_gen_return_address (struct gdbarch *gdbarch, error (_("This architecture has no method to collect a return address.")); } -enum gdb_signal -default_gdb_signal_from_target (struct gdbarch *gdbarch, int signo) -{ - /* Lacking a better mapping, assume host signal numbers. If - debugging a cross-core, most likely this translation will be - incorrect. */ - return gdb_signal_from_host (signo); -} - /* */ /* -Wmissing-prototypes */ diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h index ba71ec3..7c398b3 100644 --- a/gdb/arch-utils.h +++ b/gdb/arch-utils.h @@ -172,7 +172,4 @@ extern void default_gen_return_address (struct gdbarch *gdbarch, extern const char *default_auto_charset (void); extern const char *default_auto_wide_charset (void); -extern enum gdb_signal default_gdb_signal_from_target (struct gdbarch *, - int); - #endif diff --git a/gdb/corelow.c b/gdb/corelow.c index 8c3cd5c..dd62560 100644 --- a/gdb/corelow.c +++ b/gdb/corelow.c @@ -441,11 +441,16 @@ core_open (char *filename, int from_tty) if (siggy > 0) { /* If we don't have a CORE_GDBARCH to work with, assume a native - core. */ + core (map gdb_signal from host signals). If we do have + CORE_GDBARCH to work with, but no gdb_signal_from_target + implementation for that gdbarch, as a fallback measure, + assume the host signal mapping. It'll be correct for native + cores, but most likely incorrect for cross-cores. */ enum gdb_signal sig = (core_gdbarch != NULL - ? gdbarch_gdb_signal_from_target (core_gdbarch, - siggy) - : gdb_signal_from_host (siggy)); + && gdbarch_gdb_signal_from_target_p (core_gdbarch) + ? gdbarch_gdb_signal_from_target (core_gdbarch, + siggy) + : gdb_signal_from_host (siggy)); printf_filtered (_("Program terminated with signal %d, %s.\n"), siggy, gdb_signal_to_string (sig)); diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c index 3ebe835..9d7b67a 100644 --- a/gdb/gdbarch.c +++ b/gdb/gdbarch.c @@ -427,7 +427,7 @@ struct gdbarch startup_gdbarch = 0, /* sofun_address_maybe_missing */ 0, /* process_record */ 0, /* process_record_signal */ - default_gdb_signal_from_target, /* gdb_signal_from_target */ + 0, /* gdb_signal_from_target */ 0, /* get_siginfo_type */ 0, /* record_special_symbol */ 0, /* get_syscall_number */ @@ -536,7 +536,6 @@ gdbarch_alloc (const struct gdbarch_info *info, gdbarch->displaced_step_free_closure = NULL; gdbarch->displaced_step_location = NULL; gdbarch->relocate_instruction = NULL; - gdbarch->gdb_signal_from_target = default_gdb_signal_from_target; 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; @@ -727,7 +726,7 @@ verify_gdbarch (struct gdbarch *gdbarch) /* Skip verify of sofun_address_maybe_missing, invalid_p == 0 */ /* Skip verify of process_record, has predicate. */ /* Skip verify of process_record_signal, has predicate. */ - /* Skip verify of gdb_signal_from_target, invalid_p == 0 */ + /* Skip verify of gdb_signal_from_target, has predicate. */ /* Skip verify of get_siginfo_type, has predicate. */ /* Skip verify of record_special_symbol, has predicate. */ /* Skip verify of get_syscall_number, has predicate. */ @@ -996,6 +995,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file) "gdbarch_dump: gcore_bfd_target = %s\n", pstring (gdbarch->gcore_bfd_target)); fprintf_unfiltered (file, + "gdbarch_dump: gdbarch_gdb_signal_from_target_p() = %d\n", + gdbarch_gdb_signal_from_target_p (gdbarch)); + fprintf_unfiltered (file, "gdbarch_dump: gdb_signal_from_target = <%s>\n", host_address_to_string (gdbarch->gdb_signal_from_target)); fprintf_unfiltered (file, @@ -3787,6 +3789,13 @@ set_gdbarch_process_record_signal (struct gdbarch *gdbarch, gdbarch->process_record_signal = process_record_signal; } +int +gdbarch_gdb_signal_from_target_p (struct gdbarch *gdbarch) +{ + gdb_assert (gdbarch != NULL); + return gdbarch->gdb_signal_from_target != NULL; +} + enum gdb_signal gdbarch_gdb_signal_from_target (struct gdbarch *gdbarch, int signo) { diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h index da449c2..e4e7abf 100644 --- a/gdb/gdbarch.h +++ b/gdb/gdbarch.h @@ -945,8 +945,9 @@ extern void set_gdbarch_process_record_signal (struct gdbarch *gdbarch, gdbarch_ header (the nm-*.h files), the host <signal.h> header, or similar headers. This is mainly used when cross-debugging core files --- "Live" targets hide the translation behind the target interface - (target_wait, target_resume, etc.). The default is to do the - translation using host signal numbers. */ + (target_wait, target_resume, etc.). */ + +extern int gdbarch_gdb_signal_from_target_p (struct gdbarch *gdbarch); typedef enum gdb_signal (gdbarch_gdb_signal_from_target_ftype) (struct gdbarch *gdbarch, int signo); extern enum gdb_signal gdbarch_gdb_signal_from_target (struct gdbarch *gdbarch, int signo); diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh index 5cefdab..0f58def 100755 --- a/gdb/gdbarch.sh +++ b/gdb/gdbarch.sh @@ -778,9 +778,8 @@ M:int:process_record_signal:struct regcache *regcache, enum gdb_signal signal:re # header (the nm-*.h files), the host <signal.h> header, or similar # headers. This is mainly used when cross-debugging core files --- # "Live" targets hide the translation behind the target interface -# (target_wait, target_resume, etc.). The default is to do the -# translation using host signal numbers. -m:enum gdb_signal:gdb_signal_from_target:int signo:signo::default_gdb_signal_from_target::0 +# (target_wait, target_resume, etc.). +M:enum gdb_signal:gdb_signal_from_target:int signo:signo # Extra signal info inspection. # ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] gdbarch_gdb_signal_from_target: Mention host independence. 2012-06-04 15:49 ` Pedro Alves 2012-06-04 16:26 ` [PATCH 2/2] Make gdbarch_gdb_signal_from_target a method with predicate Pedro Alves @ 2012-06-04 16:26 ` Pedro Alves 1 sibling, 0 replies; 10+ messages in thread From: Pedro Alves @ 2012-06-04 16:26 UTC (permalink / raw) To: gdb-patches Applied. 2012-06-04 Pedro Alves <palves@redhat.com> * gdbarch.sh (gdb_signal_from_target): Mention that the implementation of the method must be host independent. * gdbarch.h: Regenerate. --- gdb/gdbarch.h | 11 +++++++---- gdb/gdbarch.sh | 11 +++++++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h index 5d73d72..da449c2 100644 --- a/gdb/gdbarch.h +++ b/gdb/gdbarch.h @@ -940,10 +940,13 @@ extern int gdbarch_process_record_signal (struct gdbarch *gdbarch, struct regcac extern void set_gdbarch_process_record_signal (struct gdbarch *gdbarch, gdbarch_process_record_signal_ftype *process_record_signal); /* Signal translation: translate inferior's signal (target's) number - into GDB's representation. This is mainly used when cross-debugging - core files --- "Live" targets hide the translation behind the target - interface (target_wait, target_resume, etc.). The default is to do - the translation using host signal numbers. */ + into GDB's representation. The implementation of this method must + be host independent. IOW, don't rely on symbols of the NAT_FILE + header (the nm-*.h files), the host <signal.h> header, or similar + headers. This is mainly used when cross-debugging core files --- + "Live" targets hide the translation behind the target interface + (target_wait, target_resume, etc.). The default is to do the + translation using host signal numbers. */ typedef enum gdb_signal (gdbarch_gdb_signal_from_target_ftype) (struct gdbarch *gdbarch, int signo); extern enum gdb_signal gdbarch_gdb_signal_from_target (struct gdbarch *gdbarch, int signo); diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh index cc1fe65..5cefdab 100755 --- a/gdb/gdbarch.sh +++ b/gdb/gdbarch.sh @@ -773,10 +773,13 @@ M:int:process_record:struct regcache *regcache, CORE_ADDR addr:regcache, addr M:int:process_record_signal:struct regcache *regcache, enum gdb_signal signal:regcache, signal # Signal translation: translate inferior's signal (target's) number -# into GDB's representation. This is mainly used when cross-debugging -# core files --- "Live" targets hide the translation behind the target -# interface (target_wait, target_resume, etc.). The default is to do -# the translation using host signal numbers. +# into GDB's representation. The implementation of this method must +# be host independent. IOW, don't rely on symbols of the NAT_FILE +# header (the nm-*.h files), the host <signal.h> header, or similar +# headers. This is mainly used when cross-debugging core files --- +# "Live" targets hide the translation behind the target interface +# (target_wait, target_resume, etc.). The default is to do the +# translation using host signal numbers. m:enum gdb_signal:gdb_signal_from_target:int signo:signo::default_gdb_signal_from_target::0 # Extra signal info inspection. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] MIPS Linux signals 2012-06-01 20:53 ` Maciej W. Rozycki 2012-06-01 21:07 ` Michael Eager @ 2012-06-01 22:52 ` Michael Eager 2012-06-06 22:52 ` Maciej W. Rozycki 1 sibling, 1 reply; 10+ messages in thread From: Michael Eager @ 2012-06-01 22:52 UTC (permalink / raw) To: Maciej W. Rozycki; +Cc: gdb-patches, Pedro Alves [-- Attachment #1: Type: text/plain, Size: 877 bytes --] On 06/01/2012 01:53 PM, Maciej W. Rozycki wrote: > Hi Michael, > >> This patch adds a target function to translate MIPS Linux signals >> to GDB internal signal numbers. >> >> 2012-06-01 Michael Eager<eager@eagercon.com> >> >> * mips-linux-tdep.c (mips_gdb_signal_from_target): New >> * mips-linux-tdep.h (mips_signals): New > > Thanks, please make a unified diff (`diff -up') next time. Further notes > below. Revised patch attached. Some of the awkward code in translating REALTIME signals is due to the fact that the GDB_SIGNAL_REALTIME block is not contiguous. I recast the section of code copied from signals.c in terms of MIPS_SIGRTMIN & MIPS_SIGRTMAX, but it isn't clear to me that this is an improvement, since it is now different from the similar code in signals.c. -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077 [-- Attachment #2: mips.patch --] [-- Type: text/x-patch, Size: 6800 bytes --] Index: gdb/ChangeLog =================================================================== RCS file: /cvs/src/src/gdb/ChangeLog,v retrieving revision 1.14309 diff -u -p -r1.14309 ChangeLog --- gdb/ChangeLog 1 Jun 2012 16:37:56 -0000 1.14309 +++ gdb/ChangeLog 1 Jun 2012 22:43:51 -0000 @@ -1,3 +1,8 @@ +2012-06-01 Michael Eager <eager@eagercon.com> + + * mips-linux-tdep.c (mips_gdb_signal_from_target): New + * mips-linux-tdep.h (mips_signals): New + 2012-06-01 Siddhesh Poyarekar <siddhesh@redhat.com> * target.c (target_read_memory): Make LEN argument as size_t. Index: gdb/mips-linux-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/mips-linux-tdep.c,v retrieving revision 1.94 diff -u -p -r1.94 mips-linux-tdep.c --- gdb/mips-linux-tdep.c 22 May 2012 17:12:07 -0000 1.94 +++ gdb/mips-linux-tdep.c 1 Jun 2012 22:43:51 -0000 @@ -40,6 +40,7 @@ #include "glibc-tdep.h" #include "linux-tdep.h" #include "xml-syscall.h" +#include "gdb_signals.h" static struct target_so_ops mips_svr4_so_ops; @@ -1330,6 +1331,98 @@ mips_linux_get_syscall_number (struct gd return ret; } +/* Translate signals based on MIPS signal values. + Adapted from gdb/common/signals.c. */ + +static enum gdb_signal +mips_gdb_signal_from_target (struct gdbarch *gdbarch, int signo) +{ + switch (signo) + { + case 0: + return GDB_SIGNAL_0; + case MIPS_SIGHUP: + return GDB_SIGNAL_HUP; + case MIPS_SIGINT: + return GDB_SIGNAL_INT; + case MIPS_SIGQUIT: + return GDB_SIGNAL_QUIT; + case MIPS_SIGILL: + return GDB_SIGNAL_ILL; + case MIPS_SIGTRAP: + return GDB_SIGNAL_TRAP; + case MIPS_SIGABRT: + return GDB_SIGNAL_ABRT; + case MIPS_SIGEMT: + return GDB_SIGNAL_EMT; + case MIPS_SIGFPE: + return GDB_SIGNAL_FPE; + case MIPS_SIGKILL: + return GDB_SIGNAL_KILL; + case MIPS_SIGBUS: + return GDB_SIGNAL_BUS; + case MIPS_SIGSEGV: + return GDB_SIGNAL_SEGV; + case MIPS_SIGSYS: + return GDB_SIGNAL_SYS; + case MIPS_SIGPIPE: + return GDB_SIGNAL_PIPE; + case MIPS_SIGALRM: + return GDB_SIGNAL_ALRM; + case MIPS_SIGTERM: + return GDB_SIGNAL_TERM; + case MIPS_SIGUSR1: + return GDB_SIGNAL_USR1; + case MIPS_SIGUSR2: + return GDB_SIGNAL_USR2; + case MIPS_SIGCHLD: + return GDB_SIGNAL_CHLD; + case MIPS_SIGPWR: + return GDB_SIGNAL_PWR; + case MIPS_SIGWINCH: + return GDB_SIGNAL_WINCH; + case MIPS_SIGURG: + return GDB_SIGNAL_URG; + case MIPS_SIGPOLL: + return GDB_SIGNAL_POLL; + case MIPS_SIGSTOP: + return GDB_SIGNAL_STOP; + case MIPS_SIGTSTP: + return GDB_SIGNAL_TSTP; + case MIPS_SIGCONT: + return GDB_SIGNAL_CONT; + case MIPS_SIGTTIN: + return GDB_SIGNAL_TTIN; + case MIPS_SIGTTOU: + return GDB_SIGNAL_TTOU; + case MIPS_SIGVTALRM: + return GDB_SIGNAL_VTALRM; + case MIPS_SIGPROF: + return GDB_SIGNAL_PROF; + case MIPS_SIGXCPU: + return GDB_SIGNAL_XCPU; + case MIPS_SIGXFSZ: + return GDB_SIGNAL_XFSZ; + } + + if (signo >= MIPS_SIGRTMIN && signo <= MIPS_SIGRTMAX) + { + /* This block of GDB_SIGNAL_REALTIME value is in order. */ + if (MIPS_SIGRTMIN <= signo && signo <= (MIPS_SIGRTMIN + 32)) + return ((enum gdb_signal) + (signo - MIPS_SIGRTMIN + 1 + (int) GDB_SIGNAL_REALTIME_33)); + else if (signo == MIPS_SIGRTMIN) + return GDB_SIGNAL_REALTIME_32; + else if ((MIPS_SIGRTMIN + 32) <= signo && signo <= MIPS_SIGRTMAX) + return ((enum gdb_signal) + (signo - (MIPS_SIGRTMIN + 32) + (int) GDB_SIGNAL_REALTIME_64)); + else + error ("GDB bug: unrecognized real-time signal"); + } + + return GDB_SIGNAL_UNKNOWN; +} + /* Initialize one of the GNU/Linux OS ABIs. */ static void @@ -1414,6 +1507,9 @@ mips_linux_init_abi (struct gdbarch_info set_gdbarch_regset_from_core_section (gdbarch, mips_linux_regset_from_core_section); + set_gdbarch_gdb_signal_from_target (gdbarch, + mips_gdb_signal_from_target); + tdep->syscall_next_pc = mips_linux_syscall_next_pc; if (tdesc_data) Index: gdb/mips-linux-tdep.h =================================================================== RCS file: /cvs/src/src/gdb/mips-linux-tdep.h,v retrieving revision 1.13 diff -u -p -r1.13 mips-linux-tdep.h --- gdb/mips-linux-tdep.h 1 Mar 2012 22:19:45 -0000 1.13 +++ gdb/mips-linux-tdep.h 1 Jun 2012 22:43:51 -0000 @@ -105,3 +105,45 @@ 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). */ + MIPS_SIGRTMIN = 32, /* Minimum RT signal. */ + MIPS_SIGRTMAX = 128 - 1 /* Maximum RT signal. */ + }; ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] MIPS Linux signals 2012-06-01 22:52 ` [PATCH] MIPS Linux signals Michael Eager @ 2012-06-06 22:52 ` Maciej W. Rozycki 2012-06-06 23:12 ` Michael Eager 2012-06-11 16:09 ` Michael Eager 0 siblings, 2 replies; 10+ messages in thread From: Maciej W. Rozycki @ 2012-06-06 22:52 UTC (permalink / raw) To: Michael Eager; +Cc: gdb-patches, Pedro Alves On Fri, 1 Jun 2012, Michael Eager wrote: > Some of the awkward code in translating REALTIME signals is due to > the fact that the GDB_SIGNAL_REALTIME block is not contiguous. OK, fair enough, it's always good to include any such explanations with patch submissions so as to make the review process easier for the reviewers. It makes sense to note the peculiarity around code concerned too. > I recast the section of code copied from signals.c in terms of > MIPS_SIGRTMIN & MIPS_SIGRTMAX, but it isn't clear to me that this > is an improvement, since it is now different from the similar code > in signals.c. This is target-specific code, its goal is not to be as close as to generic code as possible, but (as with any code, except where critical performance or any other requirements mandate otherwise) straightforward and matching the requirements of the specific target concerned. > Index: gdb/ChangeLog > =================================================================== > RCS file: /cvs/src/src/gdb/ChangeLog,v > retrieving revision 1.14309 > diff -u -p -r1.14309 ChangeLog > --- gdb/ChangeLog 1 Jun 2012 16:37:56 -0000 1.14309 > +++ gdb/ChangeLog 1 Jun 2012 22:43:51 -0000 > @@ -1,3 +1,8 @@ > +2012-06-01 Michael Eager <eager@eagercon.com> > + > + * mips-linux-tdep.c (mips_gdb_signal_from_target): New > + * mips-linux-tdep.h (mips_signals): New > + > 2012-06-01 Siddhesh Poyarekar <siddhesh@redhat.com> > > * target.c (target_read_memory): Make LEN argument as size_t. Please supply ChangeLog entries separately, they're not going to work when included as a diff and will require hand-editing anyway. > Index: gdb/mips-linux-tdep.c > =================================================================== > RCS file: /cvs/src/src/gdb/mips-linux-tdep.c,v > retrieving revision 1.94 > diff -u -p -r1.94 mips-linux-tdep.c > --- gdb/mips-linux-tdep.c 22 May 2012 17:12:07 -0000 1.94 > +++ gdb/mips-linux-tdep.c 1 Jun 2012 22:43:51 -0000 > @@ -1330,6 +1331,98 @@ mips_linux_get_syscall_number (struct gd [...] > + > + if (signo >= MIPS_SIGRTMIN && signo <= MIPS_SIGRTMAX) > + { > + /* This block of GDB_SIGNAL_REALTIME value is in order. */ > + if (MIPS_SIGRTMIN <= signo && signo <= (MIPS_SIGRTMIN + 32)) > + return ((enum gdb_signal) > + (signo - MIPS_SIGRTMIN + 1 + (int) GDB_SIGNAL_REALTIME_33)); > + else if (signo == MIPS_SIGRTMIN) > + return GDB_SIGNAL_REALTIME_32; > + else if ((MIPS_SIGRTMIN + 32) <= signo && signo <= MIPS_SIGRTMAX) > + return ((enum gdb_signal) > + (signo - (MIPS_SIGRTMIN + 32) + (int) GDB_SIGNAL_REALTIME_64)); > + else > + error ("GDB bug: unrecognized real-time signal"); > + } > + > + return GDB_SIGNAL_UNKNOWN; > +} > + > /* Initialize one of the GNU/Linux OS ABIs. */ > > static void I find this unreadable (and buggy too, which is likely a consequence), per observations above please rewrite this as below: if (signo >= MIPS_SIGRTMIN && signo <= MIPS_SIGRTMAX) { /* GDB_SIGNAL_REALTIME values are not contiguous, map parts of the MIPS block to the respective GDB_SIGNAL_REALTIME blocks. */ signo -= MIPS_SIGRTMIN; if (signo == 0) return GDB_SIGNAL_REALTIME_32; else if (signo < 32) return ((enum gdb_signal) (signo - 1 + (int) GDB_SIGNAL_REALTIME_33)); else return ((enum gdb_signal) (signo - 32 + (int) GDB_SIGNAL_REALTIME_64)); } return GDB_SIGNAL_UNKNOWN; } -- the casts are probably redundant, but let them stay. OK with this change, the rest is fine with me. Thanks for tackling this problem. Maciej ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] MIPS Linux signals 2012-06-06 22:52 ` Maciej W. Rozycki @ 2012-06-06 23:12 ` Michael Eager 2012-06-11 16:09 ` Michael Eager 1 sibling, 0 replies; 10+ messages in thread From: Michael Eager @ 2012-06-06 23:12 UTC (permalink / raw) To: Maciej W. Rozycki; +Cc: gdb-patches, Pedro Alves On 06/06/2012 03:52 PM, Maciej W. Rozycki wrote: > On Fri, 1 Jun 2012, Michael Eager wrote: > >> Some of the awkward code in translating REALTIME signals is due to >> the fact that the GDB_SIGNAL_REALTIME block is not contiguous. > > OK, fair enough, it's always good to include any such explanations with > patch submissions so as to make the review process easier for the > reviewers. It makes sense to note the peculiarity around code concerned > too. > >> I recast the section of code copied from signals.c in terms of >> MIPS_SIGRTMIN& MIPS_SIGRTMAX, but it isn't clear to me that this >> is an improvement, since it is now different from the similar code >> in signals.c. > > This is target-specific code, its goal is not to be as close as to > generic code as possible, but (as with any code, except where critical > performance or any other requirements mandate otherwise) straightforward > and matching the requirements of the specific target concerned. > >> Index: gdb/ChangeLog >> =================================================================== >> RCS file: /cvs/src/src/gdb/ChangeLog,v >> retrieving revision 1.14309 >> diff -u -p -r1.14309 ChangeLog >> --- gdb/ChangeLog 1 Jun 2012 16:37:56 -0000 1.14309 >> +++ gdb/ChangeLog 1 Jun 2012 22:43:51 -0000 >> @@ -1,3 +1,8 @@ >> +2012-06-01 Michael Eager<eager@eagercon.com> >> + >> + * mips-linux-tdep.c (mips_gdb_signal_from_target): New >> + * mips-linux-tdep.h (mips_signals): New >> + >> 2012-06-01 Siddhesh Poyarekar<siddhesh@redhat.com> >> >> * target.c (target_read_memory): Make LEN argument as size_t. > > Please supply ChangeLog entries separately, they're not going to work > when included as a diff and will require hand-editing anyway. I forgot to edit the ChangeLog out of the diff. > >> Index: gdb/mips-linux-tdep.c >> =================================================================== >> RCS file: /cvs/src/src/gdb/mips-linux-tdep.c,v >> retrieving revision 1.94 >> diff -u -p -r1.94 mips-linux-tdep.c >> --- gdb/mips-linux-tdep.c 22 May 2012 17:12:07 -0000 1.94 >> +++ gdb/mips-linux-tdep.c 1 Jun 2012 22:43:51 -0000 >> @@ -1330,6 +1331,98 @@ mips_linux_get_syscall_number (struct gd > [...] >> + >> + if (signo>= MIPS_SIGRTMIN&& signo<= MIPS_SIGRTMAX) >> + { >> + /* This block of GDB_SIGNAL_REALTIME value is in order. */ >> + if (MIPS_SIGRTMIN<= signo&& signo<= (MIPS_SIGRTMIN + 32)) >> + return ((enum gdb_signal) >> + (signo - MIPS_SIGRTMIN + 1 + (int) GDB_SIGNAL_REALTIME_33)); >> + else if (signo == MIPS_SIGRTMIN) >> + return GDB_SIGNAL_REALTIME_32; >> + else if ((MIPS_SIGRTMIN + 32)<= signo&& signo<= MIPS_SIGRTMAX) >> + return ((enum gdb_signal) >> + (signo - (MIPS_SIGRTMIN + 32) + (int) GDB_SIGNAL_REALTIME_64)); >> + else >> + error ("GDB bug: unrecognized real-time signal"); >> + } >> + >> + return GDB_SIGNAL_UNKNOWN; >> +} >> + >> /* Initialize one of the GNU/Linux OS ABIs. */ >> >> static void > > I find this unreadable (and buggy too, which is likely a consequence), > per observations above please rewrite this as below: > > if (signo>= MIPS_SIGRTMIN&& signo<= MIPS_SIGRTMAX) > { > /* GDB_SIGNAL_REALTIME values are not contiguous, map parts of > the MIPS block to the respective GDB_SIGNAL_REALTIME blocks. */ > signo -= MIPS_SIGRTMIN; > if (signo == 0) > return GDB_SIGNAL_REALTIME_32; > else if (signo< 32) > return ((enum gdb_signal) (signo - 1 + (int) GDB_SIGNAL_REALTIME_33)); > else > return ((enum gdb_signal) (signo - 32 + (int) GDB_SIGNAL_REALTIME_64)); > } > > return GDB_SIGNAL_UNKNOWN; > } OK. > > -- the casts are probably redundant, but let them stay. > > OK with this change, the rest is fine with me. Thanks for tackling this > problem. Thanks. -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] MIPS Linux signals 2012-06-06 22:52 ` Maciej W. Rozycki 2012-06-06 23:12 ` Michael Eager @ 2012-06-11 16:09 ` Michael Eager 1 sibling, 0 replies; 10+ messages in thread From: Michael Eager @ 2012-06-11 16:09 UTC (permalink / raw) To: Maciej W. Rozycki; +Cc: gdb-patches, Pedro Alves On 06/06/2012 03:52 PM, Maciej W. Rozycki wrote: > On Fri, 1 Jun 2012, Michael Eager wrote: > > I find this unreadable (and buggy too, which is likely a consequence), > per observations above please rewrite this as below: > > if (signo>= MIPS_SIGRTMIN&& signo<= MIPS_SIGRTMAX) > { > /* GDB_SIGNAL_REALTIME values are not contiguous, map parts of > the MIPS block to the respective GDB_SIGNAL_REALTIME blocks. */ > signo -= MIPS_SIGRTMIN; > if (signo == 0) > return GDB_SIGNAL_REALTIME_32; > else if (signo< 32) > return ((enum gdb_signal) (signo - 1 + (int) GDB_SIGNAL_REALTIME_33)); > else > return ((enum gdb_signal) (signo - 32 + (int) GDB_SIGNAL_REALTIME_64)); > } > > return GDB_SIGNAL_UNKNOWN; > } > > -- the casts are probably redundant, but let them stay. > > OK with this change, the rest is fine with me. Thanks for tackling this > problem. Checked in with indicated changes. -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-06-11 16:09 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-06-01 20:21 [PATCH] MIPS Linux signals Michael Eager 2012-06-01 20:53 ` Maciej W. Rozycki 2012-06-01 21:07 ` Michael Eager 2012-06-04 15:49 ` Pedro Alves 2012-06-04 16:26 ` [PATCH 2/2] Make gdbarch_gdb_signal_from_target a method with predicate Pedro Alves 2012-06-04 16:26 ` [PATCH 1/2] gdbarch_gdb_signal_from_target: Mention host independence Pedro Alves 2012-06-01 22:52 ` [PATCH] MIPS Linux signals Michael Eager 2012-06-06 22:52 ` Maciej W. Rozycki 2012-06-06 23:12 ` Michael Eager 2012-06-11 16:09 ` Michael Eager
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox