Message ID | 20080923143120.GD14041@elte.hu (mailing list archive) |
---|---|
State | Accepted, archived |
Commit | e8d3f455de4f42d4bab2f6f1aeb2cf3bd18eb508 |
Headers | show |
I certainly have no objection in principle. I doubt that any x86 userland apps expect certain si_code values for SIGTRAP now, since the existing values are not of any real use. (Signal handlers get the thread.trap_no and thread.error_code values from hardware to guess from, and debuggers via ptrace get the hardware %db6 value to guess from.) I do have a few comments. If you're doing it, I think you should do the do_int3 case too, so every machine-generated SIGTRAP has a meaningful si_code value. The only use of send_sigtrap is for do_debug (and for faking that do_debug happened in the syscall_trace_leave case). You should consolidate all the uses in both 32 and 64 to use send_sigtrap uniformly, change its signature as needed. I'm inclined to consolidate the si_code logic there, and just pass it the hardware bits or let it get them from the thread_struct (trap_nr, error_code, debugreg6). About that si_code logic based on %db6. There are some funny "sticky" properties to how that register gets set in hardware. Even reading the hardware manuals doesn't always make it plain what to expect. I wouldn't want to testify that the patch's logic is correct in distinguishing which event really just happened. (I'm not sure, but I think it may also be possible to have a single do_debug trap for both a single-step trap and a hardware breakpoint trap generated by the same instruction.) I know that Alan Stern figured out a lot of the magic empirically a while back. That deserves a careful double-checking if we are now trying to make si_code tell a clear and reliable story. Thanks, Roland
Roland McGrath wrote: > I certainly have no objection in principle. I doubt that any x86 userland > apps expect certain si_code values for SIGTRAP now, since the existing > values are not of any real use. (Signal handlers get the thread.trap_no and > thread.error_code values from hardware to guess from, and debuggers via > ptrace get the hardware %db6 value to guess from.) I do have a few comments. > > If you're doing it, I think you should do the do_int3 case too, > so every machine-generated SIGTRAP has a meaningful si_code value. Roland Thanks for your comments. > I'm inclined to consolidate the si_code logic there, and just > pass it the hardware bits or let it get them from the thread_struct > (trap_nr, error_code, debugreg6). That sounds like a good idea. Let me go through code and get back to you. Thanks Srinivasa DS
diff --git a/include/asm-x86/traps.h b/include/asm-x86/traps.h index 4b1e904..7a692ba 100644 --- a/include/asm-x86/traps.h +++ b/include/asm-x86/traps.h @@ -1,6 +1,8 @@ #ifndef ASM_X86__TRAPS_H #define ASM_X86__TRAPS_H +#include <asm/debugreg.h> + /* Common in X86_32 and X86_64 */ asmlinkage void divide_error(void); asmlinkage void debug(void);