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

Wu Zhou
2006-01-24T04:33:12Z

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