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

[Wu Zhou <woodzltc@cn.ibm.com>]

On Mon, 23 Jan 2006, Daniel Jacobowitz wrote:

> On Tue, Jan 24, 2006 at 11:40:16AM +0800, Wu Zhou wrote:
> > p630 is one kind of POWER4 based pSeriese server. It is currently the only 
> > available ppc machine I can get.  :-)
> > 
> > In fact, I am not sure before if the ppc arch has nonsteppable watchpoints 
> > or not. But testing on my p630 box, it did had nonsteppable ones.  Now 
> > that an architecture either have or doesn't have nonsteppable watchpoints, 
> > can we get from this test a result that ppc architecture has nonsteppable 
> > watchpoints?
> > 
> > If so, maybe I can just remove the stupid conditional statement below.  
> > (my original intention is to verify that v->mach equals bfd_mach_ppc_630 :-)
> 
> Well, it'd be nice to have some architectural reference for this.  But
> it's probably a safe bet to assume that this is generally true for all
> PowerPC targets, so let's just assume it.

OK.  I will assume this then.

> > Function to_region_ok_for_hw_watchpoint is not in the current target 
> > vector (I means struct target_ops).  Maybe we can add it into
> > target_ops? There are a few other archs also use this.  But they had to 
> > include it in nm-xxx-yyy.h.  If not, the only method I can think of is 
> > also include its definition in nm-ppc64-linux.h.  So what about the 
> > following patch section?
> > 
> >       int (*to_region_size_ok_for_hw_watchpoint) (int);
> > +     int (*to_region_ok_for_hw_watchpoint) (CORE_ADDR *, int);
> >       void (*to_terminal_init) (void);
> 
> I would recommend replacing to_region_size_ok_for_hw_watchpoint
> with to_region_ok_for_hw_watchpoint.  You'll have to update the
> callers, including the non-multi-arch ones, to ignore the first
> argument; shouldn't be hard?

Do you means to make the following change in gdb/target.h?

--- gdb/target.h        4 Sep 2005 16:18:20 -0000       1.76
+++ gdb/target.h        24 Jan 2006 04:17:13 -0000
@@ -345,7 +345,7 @@
     int (*to_stopped_by_watchpoint) (void);
     int to_have_continuable_watchpoint;
     int (*to_stopped_data_address) (struct target_ops *, CORE_ADDR *);
-    int (*to_region_size_ok_for_hw_watchpoint) (int);
+    int (*to_region_ok_for_hw_watchpoint) (CORE_ADDR *, int);
     void (*to_terminal_init) (void);
     void (*to_terminal_inferior) (void);
     void (*to_terminal_ours_for_output) (void);
@@ -1030,9 +1030,9 @@
  (*current_target.to_can_use_hw_breakpoint) (TYPE, CNT, OTHERTYPE);
 #endif

-#if !defined(TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT)
-#define TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT(byte_count) \
-    (*current_target.to_region_size_ok_for_hw_watchpoint) (byte_count)
+#if !defined(TARGET_REGION_OK_FOR_HW_WATCHPOINT)
+#define TARGET_REGION_OK_FOR_HW_WATCHPOINT(addr, byte_count) \
+    (*current_target.to_region_ok_for_hw_watchpoint) (addr, byte_count)
 #endif

Then make responsive changes to the code where is referenced?

such as (in config/i386/nm-i386sol2.h):

-#define TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT(SIZE) 1
+#define TARGET_REGION_OK_FOR_HW_WATCHPOINT(ADDR, SIZE) 1

or (in s390-nat.c): 

 static int
-s390_region_size_ok_for_hw_watchpoint (int cnt)
+s390_region_ok_for_hw_watchpoint (CORE_ADDR *, int cnt)
 {
   return 1;
 }
@@ -380,7 +380,7 @@

   /* Add our watchpoint methods.  */
   t->to_can_use_hw_breakpoint = s390_can_use_hw_breakpoint;
-  t->to_region_size_ok_for_hw_watchpoint = s390_region_size_ok_for_hw_watchpoint;
+  t->to_region_ok_for_hw_watchpoint = s390_region_ok_for_hw_watchpoint;
   t->to_have_continuable_watchpoint = 1;
   t->to_stopped_by_watchpoint = s390_stopped_by_watchpoint;

and so on.

P.S: If I can understand and also I understand correctly, it is not hard.
Sometimes I just need  a little more time to understand the precise 
meaning of your words.  Most of the time I attribute it to my english.

;-)

Regards
- Wu Zhou