Patchwork [RFC] Demultiplexing SIGTRAP signal -v2

login
register
mail settings
Submitter Ingo Molnar
Date Sept. 23, 2008, 2:31 p.m.
Message ID <20080923143120.GD14041@elte.hu>
Download mbox | patch
Permalink /patch/1109/
State Accepted, archived
Commit e8d3f455de4f42d4bab2f6f1aeb2cf3bd18eb508
Headers show

Comments

Ingo Molnar - Sept. 23, 2008, 2:31 p.m.
* Srinivasa Ds <srinivasa@in.ibm.com> wrote:

> > -tip testing found the following build error with the attached 
> > config:
> 
> Ingo, Reproduced build break issue with your config on tip tree. It 
> was a costly overlook to miss one header file. I included it in this 
> patch and tested it out.

thanks - applied the delta fix below to tip/core/signal.

	Ingo

--------------->
From e8d3f455de4f42d4bab2f6f1aeb2cf3bd18eb508 Mon Sep 17 00:00:00 2001
From: Srinivasa Ds <srinivasa@in.ibm.com>
Date: Tue, 23 Sep 2008 15:23:52 +0530
Subject: [PATCH] signals: demultiplexing SIGTRAP signal, fix

fix build breakage, missing header file.

Signed-off-by: Srinivasa DS <srinivasa@in.ibm.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 include/asm-x86/traps.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Roland McGrath - Sept. 26, 2008, 9:06 a.m.
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
Srinivasa Ds - Sept. 29, 2008, 1:34 p.m.
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

Patch

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);