[Patch 3/6] Modify ptrace code to use Hardware Breakpoint interfaces

[David Gibson]

On Mon, Jul 27, 2009 at 05:48:27AM +0530, K.Prasad wrote:
> Modify the ptrace code to use the hardware breakpoint interfaces for user-space.
> 
> Signed-off-by: K.Prasad <prasad at linux.vnet.ibm.com>
> ---
>  arch/powerpc/kernel/ptrace.c |   43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/ptrace.c
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/ptrace.c
> +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/ptrace.c
> @@ -37,6 +37,7 @@
>  #include <asm/page.h>
>  #include <asm/pgtable.h>
>  #include <asm/system.h>
> +#include <asm/hw_breakpoint.h>
>  
>  /*
>   * does not yet catch signals sent when the child dies.
> @@ -757,11 +758,24 @@ void user_disable_single_step(struct tas
>  
>  void ptrace_triggered(struct hw_breakpoint *bp, struct pt_regs *regs)
>  {
> +	/*
> +	 * Unregister the breakpoint request here since ptrace has defined a
> +	 * one-shot behaviour for breakpoint exceptions in PPC64.
> +	 * The SIGTRAP signal is generated automatically for us in do_dabr().
> +	 * We don't have to do anything here
> +	 */
> +	unregister_user_hw_breakpoint(current, bp);
> +	kfree(bp);

This unregisters the breakpoint, but doesn't actually abort the
current breakpoint handling sequence it's invoked from.  So, if your
breakpoint handler was invoked at all, which as previously mentioned,
I don't think it is, wouldn't this result in *two* SIGTRAPs from a
ptrace breakpoint: one issued before the trapping instruction from
do_dabr() and another afterwards from your step-over code.

>  }
>  
>  int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
>  			       unsigned long data)
>  {
> +#ifdef CONFIG_PPC64
> +	struct thread_struct *thread = &(task->thread);
> +	struct hw_breakpoint *bp;
> +	int ret;
> +#endif
>  	/* For ppc64 we support one DABR and no IABR's at the moment (ppc64).
>  	 *  For embedded processors we support one DAC and no IAC's at the
>  	 *  moment.
> @@ -791,6 +805,35 @@ int ptrace_set_debugreg(struct task_stru
>  	if (data && !(data & DABR_TRANSLATION))
>  		return -EIO;
>  
> +#ifdef CONFIG_PPC64
> +	bp = thread->hbp[0];
> +	if (data == 0) {
> +		if (bp) {
> +			unregister_user_hw_breakpoint(task, bp);
> +			kfree(bp);
> +		}
> +		return 0;
> +	}
> +
> +	if (bp) {
> +		bp->info.type = data & HW_BREAKPOINT_RW;
> +		task->thread.dabr = bp->info.address = data;
> +		return modify_user_hw_breakpoint(task, bp);
> +	}
> +	bp = kzalloc(sizeof(struct hw_breakpoint), GFP_KERNEL);
> +	if (!bp)
> +		return -ENOMEM;
> +
> +	/* Store the type of breakpoint */
> +	bp->info.type = data & HW_BREAKPOINT_RW;
> +	bp->triggered = ptrace_triggered;
> +	task->thread.dabr = bp->info.address = data;
> +
> +	ret = register_user_hw_breakpoint(task, bp);
> +	if (ret)
> +		return ret;
> +#endif /* CONFIG_PPC64 */
> +
>  	/* Move contents to the DABR register */
>  	task->thread.dabr = data;
>  
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson