Re: [RFC] GDB patches for hw watchpoints - revised

[Wu Zhou <woodzltc at cn dot ibm dot com>]

On Fri, 9 Dec 2005, Daniel Jacobowitz wrote:

> On Sat, Dec 10, 2005 at 12:46:36PM +0800, Wu Zhou wrote:
> > On Fri, 9 Dec 2005, Daniel Jacobowitz wrote:
> > 
> > > On Fri, Dec 09, 2005 at 10:25:33AM +0800, Wu Zhou wrote:
> > > > 
> > > > BTW. It seems that to_stopped_data_address is only used in rwatch and 
> > > > awatch, which is not that frequently used as watch itself.  So may we 
> > > > postpone its implementation till some later time?
> > > 
> > > No, let's get it right the first time.  rwatch is extremely valuable,
> > > and it sounds like you're having to play with ABI changes to get it to
> > > work.
> > 
> > OK. I will try to make it right the first time. :-)
> > 
> > And can you elaborate on the statement that I am having to play with ABI 
> > changes to get it to work?  It seems that you must find something 
> > noticeable, right? If so, please point out. Thanks.
> 
> I was just talking about Anton's kernel patch.  If you're still
> changing the kernel to make it work, it's not quite done yet.

I am now trying three different method to get the stopped data address. 
But aach one seems to have its shortcoming , so I had to list them here to 
solicit comments and suggestions.  Thanks in advance.

1. The first one don't need any more change to kernel 2.6.14.3, I use 
GET_DEBUG_REG to get the content of DABR and assume it is the same as the 
stopped_data_address.  But the problem is that the content of DABR is not 
all the time the same as the data breakpoint.  What DABR monitor is an 
8-bytes region. The last three bits are used for setting read/write/translating
flag.

The code is something like this:
 
 tid = TIDGET (ptid);
  if (tid == 0)
    tid = PIDGET (ptid);

  ptrace (PTRACE_GET_DEBUGREG, tid, (PTRACE_TYPE_ARG3) 0, addr_p);
  *addr_p = *addr_p & ~7;

2. The second one need Anton's patch, which changed three lines in 
arch/ppc64/mm/fault.c:

Index: linux-2.6/arch/powerpc/mm/fault.c
===================================================================
--- linux-2.6.orig/arch/powerpc/mm/fault.c		 2005-11-16 
03:21:49.000000000 +1100
+++ linux-2.6/arch/powerpc/mm/fault.c		 2005-12-08 
16:34:21.000000000 +1100
@@ -81,7 +81,8 @@
 }
 
 #if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
-static void do_dabr(struct pt_regs *regs, unsigned long error_code)
+static void do_dabr(struct pt_regs *regs, unsigned long address,
+		 		     unsigned long error_code)
 {
 		 siginfo_t info;
 
@@ -99,7 +100,7 @@
 		 info.si_signo = SIGTRAP;
 		 info.si_errno = 0;
 		 info.si_code = TRAP_HWBKPT;
-		 info.si_addr = (void __user *)regs->nip;
+		 info.si_addr = (void __user *)address;
 		 force_sig_info(SIGTRAP, &info, current);
 }
 #endif /* !(CONFIG_4xx || CONFIG_BOOKE)*/
@@ -159,7 +160,7 @@
 #if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
   		 if (error_code & DSISR_DABRMATCH) {
 		 		 /* DABR match */
-		 		 do_dabr(regs, error_code);
+		 		 do_dabr(regs, address, error_code);
 		 		 return 0;
 		 }
 #endif /* !(CONFIG_4xx || CONFIG_BOOKE)*/

With this patch, I can use PTRACE_GETSIGINFO to get the stopped data 
address (it is in siginfo.si_addr).  But one problem is that 
to_stopped_by_watchpoint will call PTRACE_GETSIGINFO first to determine if 
the stop is caused by watchpoint.  And another problem is that gdb need to 
single step the process to execute current instruction when a watchpoint 
is hit.  This will again drop into bpstat_stop_status, which will call 
stopped_by_watchpoint and thus call PTRACE_GETSIGINFO again.  

I take a look at IA64's code, it set the dd bit of IA64_PSR_REGNUM, which
will disable the watchpoint for the next instruction.  But it seems that 
ppc don't have such a way.  Do we have any workaround for this?  

3. The third one is a little tricky.  Now that ppc has at most 1 DABR. So 
I can set the stopped_data_address to the data address when we set the 
watchpoint (in ppc_linux_insert_watchpoint).  Everytime 
target_stopped_data_address is called, the breakpoint is either read or 
access, so it is already clear that it is stopped by watchpoint.  Then 
this trick seems to make sense, right?


I had tested the above three methods.  The first one works ok when the 
data breakpoint is aligned by 8 bytes.  The third one works ok for both 
aligned and non-aligned data breakpoint.  For the second one, I don't know 
how to work around the extra PTRACE_GETSIGINFO call caused by the single 
step yet.  But if I reserver the stopped_data_address when we first 
hit watchpoint, and store it back when I call ppc_linux_stopped_data_address. 
I can make rwatch and awatch to work as expected.

Any comments on the above three methods.  Thanks a lot in advance.

Regards
- Wu Zhou