All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Srinivasa Ds <srinivasa@in.ibm.com>, Roland McGrath <roland@redhat.com>
Cc: linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
	paulus@samba.org, roland@redhat.com, linuxppc-dev@ozlabs.org,
	linux-parisc@vger.kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [RFC][PATCH] Demultiplexing SIGTRAP signal
Date: Mon, 22 Sep 2008 12:42:02 +0200	[thread overview]
Message-ID: <20080922104202.GE30137@elte.hu> (raw)
In-Reply-To: <200809221602.32616.srinivasa@in.ibm.com>


* Srinivasa Ds <srinivasa@in.ibm.com> wrote:

> Currently a SIGTRAP signal can denote any one of below reasons.
> 	- Breakpoint hit
> 	- H/W debug register hit
> 	- Single step
> 	- SIGTRAP signal sent through kill() or rasie()
> 
> Architectures like powerpc/parisc provides infrastructure to 
> demultiplex SIGTRAP signal by passing down the information for 
> receiving SIGTRAP through si_code of siginfot_t structure. Here is an 
> attempt is generalise this infrastructure by extending it to x86 and 
> x86_64 archs.

no fundamental objections - assuming existing x86 apps have not grown an 
ABI dependency on the existing send_sigtrap() semantics. (Debuggers and 
JITs would be a candidate for such dependencies.)

a small implementational detail, this bit:

> @@ -935,8 +936,22 @@ void __kprobes do_debug(struct pt_regs *
>  			goto clear_TF_reenable;
>  	}
>  
> -	/* Ok, finally something we can handle */
> -	send_sigtrap(tsk, regs, error_code);
> +	tsk->thread.trap_no = 1;
> +	tsk->thread.error_code = error_code;
> +
> +	memset(&info, 0, sizeof(info));
> +	info.si_signo = SIGTRAP;
> +	if (condition & DR_STEP)
> +		info.si_code = TRAP_TRACE;
> +	else if (condition & (DR_TRAP0|DR_TRAP1|DR_TRAP2|DR_TRAP3))
> +		info.si_code = TRAP_HWBKPT;
> +	else
> +		info.si_code = TRAP_BRKPT;
> +	/* User-mode ip? */
> +	info.si_addr = user_mode_vm(regs) ? (void __user *) regs->ip : NULL;
> +
> +	/* Send us the fake SIGTRAP */
> +	force_sig_info(SIGTRAP, &info, tsk);

should be pushed into [a sufficiently extended] send_sigtrap() instead.

and this bit:

> -	info.si_code = TRAP_BRKPT;
> +	if (condition & DR_STEP)
> +		info.si_code = TRAP_TRACE;
> +	else if (condition & (DR_TRAP0|DR_TRAP1|DR_TRAP2|DR_TRAP3))
> +		info.si_code = TRAP_HWBKPT;
> +	else
> +		info.si_code = TRAP_BRKPT;

should be separated into a helper function as well i guess.

Roland, any objections to the core idea (or to the implementation)?

	Ingo

WARNING: multiple messages have this Message-ID (diff)
From: Ingo Molnar <mingo@elte.hu>
To: Srinivasa Ds <srinivasa@in.ibm.com>, Roland McGrath <roland@redhat.com>
Cc: linux-parisc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linuxppc-dev@ozlabs.org, paulus@samba.org,
	"H. Peter Anvin" <hpa@zytor.com>,
	akpm@linux-foundation.org, Thomas Gleixner <tglx@linutronix.de>,
	roland@redhat.com
Subject: Re: [RFC][PATCH] Demultiplexing SIGTRAP signal
Date: Mon, 22 Sep 2008 12:42:02 +0200	[thread overview]
Message-ID: <20080922104202.GE30137@elte.hu> (raw)
In-Reply-To: <200809221602.32616.srinivasa@in.ibm.com>


* Srinivasa Ds <srinivasa@in.ibm.com> wrote:

> Currently a SIGTRAP signal can denote any one of below reasons.
> 	- Breakpoint hit
> 	- H/W debug register hit
> 	- Single step
> 	- SIGTRAP signal sent through kill() or rasie()
> 
> Architectures like powerpc/parisc provides infrastructure to 
> demultiplex SIGTRAP signal by passing down the information for 
> receiving SIGTRAP through si_code of siginfot_t structure. Here is an 
> attempt is generalise this infrastructure by extending it to x86 and 
> x86_64 archs.

no fundamental objections - assuming existing x86 apps have not grown an 
ABI dependency on the existing send_sigtrap() semantics. (Debuggers and 
JITs would be a candidate for such dependencies.)

a small implementational detail, this bit:

> @@ -935,8 +936,22 @@ void __kprobes do_debug(struct pt_regs *
>  			goto clear_TF_reenable;
>  	}
>  
> -	/* Ok, finally something we can handle */
> -	send_sigtrap(tsk, regs, error_code);
> +	tsk->thread.trap_no = 1;
> +	tsk->thread.error_code = error_code;
> +
> +	memset(&info, 0, sizeof(info));
> +	info.si_signo = SIGTRAP;
> +	if (condition & DR_STEP)
> +		info.si_code = TRAP_TRACE;
> +	else if (condition & (DR_TRAP0|DR_TRAP1|DR_TRAP2|DR_TRAP3))
> +		info.si_code = TRAP_HWBKPT;
> +	else
> +		info.si_code = TRAP_BRKPT;
> +	/* User-mode ip? */
> +	info.si_addr = user_mode_vm(regs) ? (void __user *) regs->ip : NULL;
> +
> +	/* Send us the fake SIGTRAP */
> +	force_sig_info(SIGTRAP, &info, tsk);

should be pushed into [a sufficiently extended] send_sigtrap() instead.

and this bit:

> -	info.si_code = TRAP_BRKPT;
> +	if (condition & DR_STEP)
> +		info.si_code = TRAP_TRACE;
> +	else if (condition & (DR_TRAP0|DR_TRAP1|DR_TRAP2|DR_TRAP3))
> +		info.si_code = TRAP_HWBKPT;
> +	else
> +		info.si_code = TRAP_BRKPT;

should be separated into a helper function as well i guess.

Roland, any objections to the core idea (or to the implementation)?

	Ingo

  reply	other threads:[~2008-09-22 10:42 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-22 10:32 [RFC][PATCH] Demultiplexing SIGTRAP signal Srinivasa Ds
2008-09-22 10:42 ` Ingo Molnar [this message]
2008-09-22 10:42   ` Ingo Molnar
2008-09-22 13:11   ` Srinivasa Ds
2008-09-22 13:11     ` Srinivasa Ds
2008-09-22 14:54     ` Ingo Molnar
2008-09-22 14:54       ` Ingo Molnar
2008-09-23  9:53       ` Srinivasa Ds
2008-09-23  9:53         ` Srinivasa Ds
2008-09-23 11:28         ` Ingo Molnar
2008-09-23 11:28           ` Ingo Molnar
2008-09-23 11:30           ` Ingo Molnar
2008-09-23 11:30             ` Ingo Molnar
2008-09-23 14:25             ` [RFC][PATCH] Demultiplexing SIGTRAP signal -v2 Srinivasa Ds
2008-09-23 14:25               ` Srinivasa Ds
2008-09-23 14:31               ` Ingo Molnar
2008-09-23 14:31                 ` Ingo Molnar
2008-09-26  9:06                 ` Roland McGrath
2008-09-26  9:06                   ` Roland McGrath
2008-09-26  9:06                   ` Roland McGrath
2008-09-29 13:34                   ` Srinivasa DS
2008-09-29 13:34                     ` Srinivasa DS
2008-09-23 15:53               ` Gabriel Paubert
2008-09-23 15:53                 ` Gabriel Paubert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080922104202.GE30137@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@linux-foundation.org \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-parisc@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=paulus@samba.org \
    --cc=roland@redhat.com \
    --cc=srinivasa@in.ibm.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.