Replace to_region_size_ok_for_hw_watchpoint references with to_region_ok_for_hw_watchpoint ones

Wu Zhou
2006-01-25T08:34:39Z

Hi Daniel,

This is the patch we talked about to replace
to_region_size_ok_for_hw_watchpoint references with
references to to_region_ok_for_hw_watchpoint.

I am also thinking of replace the macro
TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT (SIZE) with
TARGET_REGION_OK_FOR_HW_WATCHPOINT(ADDR, SIZE). Thus the code will seem
to be more clean: we will only have one macro to see if the target region
is ok for watchpoint monitoring. Following this way, function
default_region_ok_for_hw_watchpoint will also return back to its original
implementation. What is your thought on this?

Here goes the patch. Please review. Thanks.

P.S: I had tested this on ppc64 and x86. Didn't see any regression.

2006-01-25 Wu Zhou <woodzltc@cn.ibm.com>

* breakpoint.c (TARGET_REGION_OK_FOR_HW_WATCHPOINT): Delete.
* config/i386/nm-i386sol2.h (TARGET_REGION_OK_FOR_HW_WATCHPOINT): New.
(TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT): Delete.
* config/mips/nm-irix5.h (TARGET_REGION_OK_FOR_HW_WATCHPOINT): New.
(TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT): Delete.
* config/sparc/nm-sol2.h (TARGET_REGION_OK_FOR_HW_WATCHPOINT): New.
(TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT): Delete.
* inf-ttrace.c (inf_ttrace_region_ok_for_hw_watchpoint): New.
(inf_ttrace_region_size_ok_for_hw_watchpoint): Delete.
(inf_ttrace_target): Delete to_region_size_ok_for_hw_watchpoint and
add to_region_ok_for_hw_watchpoint.
* s390-nat.c (s390_region_size_ok_for_hw_watchpoint): Delete.
(s390_region_ok_for_hw_watchpoint): New.
(_initialize_s390_nat): Delete to_region_size_ok_for_hw_watchpoint
and add to_region_ok_for_hw_watchpoint.
* target.c (default_region_size_ok_for_hw_watchpoint,
debug_to_region_size_ok_for_hw_watchpoint): Delete prototype.
(update_current_target): Delete to_region_size_ok_for_hw_watchpoint
inheritance and default_region_size_ok_for_hw_watchpoint.
(default_region_ok_for_hw_watchpoint): If len is less than or equal
the length of void pointer, return ok.
(default_region_size_ok_for_hw_watchpoint): Delete.
(debug_to_region_size_ok_for_hw_watchpoint): Delete.
(setup_target_debug): Delete to_region_size_ok_for_hw_watchpoint.
* target.h (struct target_ops): Delete
to_region_size_ok_for_hw_watchpoint.
(TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT): Delete.

diff -rcp src/gdb/breakpoint.c src.new/gdb/breakpoint.c
*** src/gdb/breakpoint.c 2005-05-28 23:13:17.000000000 -0400
--- src.new/gdb/breakpoint.c 2006-01-24 22:38:40.000000000 -0500
*************** watch_command_1 (char *arg, int accessfl
*** 5791,5801 ****
in hardware. If the watchpoint can not be handled
in hardware return zero. */

- #if !defined(TARGET_REGION_OK_FOR_HW_WATCHPOINT)
- #define TARGET_REGION_OK_FOR_HW_WATCHPOINT(ADDR,LEN) \
- (TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT(LEN))
- #endif
-
static int
can_use_hardware_watchpoint (struct value *v)
{
--- 5791,5796 ----
diff -rcp src/gdb/config/i386/nm-i386sol2.h src.new/gdb/config/i386/nm-i386sol2.h
*** src/gdb/config/i386/nm-i386sol2.h 2004-11-30 10:05:19.000000000 -0500
--- src.new/gdb/config/i386/nm-i386sol2.h 2006-01-24 22:43:40.000000000 -0500
***************
*** 34,40 ****
can support "thousands" of hardware watchpoints, but gives no
method for finding out how many. So just tell GDB 'yes'. */
#define TARGET_CAN_USE_HARDWARE_WATCHPOINT(TYPE, CNT, OT) 1
! #define TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT(SIZE) 1

/* When a hardware watchpoint fires off the PC will be left at the
instruction following the one which caused the watchpoint.
--- 34,40 ----
can support "thousands" of hardware watchpoints, but gives no
method for finding out how many. So just tell GDB 'yes'. */
#define TARGET_CAN_USE_HARDWARE_WATCHPOINT(TYPE, CNT, OT) 1
! #define TARGET_REGION_OK_FOR_HW_WATCHPOINT(ADDR, SIZE) 1

/* When a hardware watchpoint fires off the PC will be left at the
instruction following the one which caused the watchpoint.
diff -rcp src/gdb/config/mips/nm-irix5.h src.new/gdb/config/mips/nm-irix5.h
*** src/gdb/config/mips/nm-irix5.h 2004-11-30 10:05:20.000000000 -0500
--- src.new/gdb/config/mips/nm-irix5.h 2006-01-24 22:44:33.000000000 -0500
*************** extern int procfs_stopped_by_watchpoint
*** 51,57 ****
procfs_set_watchpoint (inferior_ptid, ADDR, 0, 0, 0)
extern int procfs_set_watchpoint (ptid_t, CORE_ADDR, int, int, int);

! #define TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT(SIZE) 1

/* Override register locations in upage for SGI machines */
#define REGISTER_U_ADDR(addr, blockend, regno) \
--- 51,57 ----
procfs_set_watchpoint (inferior_ptid, ADDR, 0, 0, 0)
extern int procfs_set_watchpoint (ptid_t, CORE_ADDR, int, int, int);

! #define TARGET_REGION_OK_FOR_HW_WATCHPOINT(ADDR, SIZE) 1

/* Override register locations in upage for SGI machines */
#define REGISTER_U_ADDR(addr, blockend, regno) \
diff -rcp src/gdb/config/sparc/nm-sol2.h src.new/gdb/config/sparc/nm-sol2.h
*** src/gdb/config/sparc/nm-sol2.h 2004-01-03 05:08:45.000000000 -0500
--- src.new/gdb/config/sparc/nm-sol2.h 2006-01-24 22:42:56.000000000 -0500
***************
*** 40,46 ****
method for finding out how many; It doesn't say anything about the
allowed size for the watched area either. So we just tell GDB
'yes'. */
! #define TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT(SIZE) 1

/* When a hardware watchpoint fires off the PC will be left at the
instruction following the one which caused the watchpoint. It will
--- 40,46 ----
method for finding out how many; It doesn't say anything about the
allowed size for the watched area either. So we just tell GDB
'yes'. */
! #define TARGET_REGION_OK_FOR_HW_WATCHPOINT(ADDR, SIZE) 1

/* When a hardware watchpoint fires off the PC will be left at the
instruction following the one which caused the watchpoint. It will
diff -rcp src/gdb/inf-ttrace.c src.new/gdb/inf-ttrace.c
*** src/gdb/inf-ttrace.c 2005-10-29 17:22:39.000000000 -0400
--- src.new/gdb/inf-ttrace.c 2006-01-24 22:40:56.000000000 -0500
*************** inf_ttrace_can_use_hw_breakpoint (int ty
*** 360,366 ****
}

static int
! inf_ttrace_region_size_ok_for_hw_watchpoint (int len)
{
return 1;
}
--- 360,366 ----
}

static int
! inf_ttrace_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
{
return 1;
}
*************** inf_ttrace_target (void)
*** 1132,1139 ****
t->to_insert_watchpoint = inf_ttrace_insert_watchpoint;
t->to_remove_watchpoint = inf_ttrace_remove_watchpoint;
t->to_stopped_by_watchpoint = inf_ttrace_stopped_by_watchpoint;
! t->to_region_size_ok_for_hw_watchpoint =
! inf_ttrace_region_size_ok_for_hw_watchpoint;
t->to_kill = inf_ttrace_kill;
t->to_create_inferior = inf_ttrace_create_inferior;
t->to_follow_fork = inf_ttrace_follow_fork;
--- 1132,1139 ----
t->to_insert_watchpoint = inf_ttrace_insert_watchpoint;
t->to_remove_watchpoint = inf_ttrace_remove_watchpoint;
t->to_stopped_by_watchpoint = inf_ttrace_stopped_by_watchpoint;
! t->to_region_ok_for_hw_watchpoint =
! inf_ttrace_region_ok_for_hw_watchpoint;
t->to_kill = inf_ttrace_kill;
t->to_create_inferior = inf_ttrace_create_inferior;
t->to_follow_fork = inf_ttrace_follow_fork;
diff -rcp src/gdb/s390-nat.c src.new/gdb/s390-nat.c
*** src/gdb/s390-nat.c 2005-09-11 17:54:58.000000000 -0400
--- src.new/gdb/s390-nat.c 2006-01-24 22:51:26.000000000 -0500
*************** s390_can_use_hw_breakpoint (int type, in
*** 358,364 ****
}

static int
! s390_region_size_ok_for_hw_watchpoint (int cnt)
{
return 1;
}
--- 358,364 ----
}

static int
! s390_region_ok_for_hw_watchpoint (CORE_ADDR addr, int cnt)
{
return 1;
}
*************** _initialize_s390_nat (void)
*** 380,386 ****

/* 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_have_continuable_watchpoint = 1;
t->to_stopped_by_watchpoint = s390_stopped_by_watchpoint;
t->to_insert_watchpoint = s390_insert_watchpoint;
--- 380,386 ----

/* Add our watchpoint methods. */
t->to_can_use_hw_breakpoint = s390_can_use_hw_breakpoint;
! 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;
t->to_insert_watchpoint = s390_insert_watchpoint;
diff -rcp src/gdb/target.c src.new/gdb/target.c
*** src/gdb/target.c 2006-01-25 10:16:16.000000000 -0500
--- src.new/gdb/target.c 2006-01-24 22:34:45.000000000 -0500
*************** static void default_terminal_info (char
*** 49,56 ****

static int default_region_ok_for_hw_watchpoint (CORE_ADDR, int);

- static int default_region_size_ok_for_hw_watchpoint (int);
-
static int nosymbol (char *, CORE_ADDR *);

static void tcomplain (void);
--- 49,54 ----
*************** static int debug_to_stopped_data_address
*** 132,139 ****

static int debug_to_region_ok_for_hw_watchpoint (CORE_ADDR, int);

- static int debug_to_region_size_ok_for_hw_watchpoint (int);
-
static void debug_to_terminal_init (void);

static void debug_to_terminal_inferior (void);
--- 130,135 ----
*************** update_current_target (void)
*** 410,416 ****
INHERIT (to_stopped_by_watchpoint, t);
INHERIT (to_have_continuable_watchpoint, t);
INHERIT (to_region_ok_for_hw_watchpoint, t);
- INHERIT (to_region_size_ok_for_hw_watchpoint, t);
INHERIT (to_terminal_init, t);
INHERIT (to_terminal_inferior, t);
INHERIT (to_terminal_ours_for_output, t);
--- 406,411 ----
*************** update_current_target (void)
*** 538,545 ****
return_zero);
de_fault (to_region_ok_for_hw_watchpoint,
default_region_ok_for_hw_watchpoint);
- de_fault (to_region_size_ok_for_hw_watchpoint,
- default_region_size_ok_for_hw_watchpoint);
de_fault (to_terminal_init,
(void (*) (void))
target_ignore);
--- 533,538 ----
*************** find_default_create_inferior (char *exec
*** 1584,1596 ****
static int
default_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
{
! return TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT (len);
! }
!
! static int
! default_region_size_ok_for_hw_watchpoint (int byte_count)
! {
! return (byte_count <= TYPE_LENGTH (builtin_type_void_data_ptr));
}

static int
--- 1577,1583 ----
static int
default_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
{
! return (len <= TYPE_LENGTH (builtin_type_void_data_ptr));
}

static int
*************** debug_to_region_ok_for_hw_watchpoint (CO
*** 2143,2162 ****
}

static int
- debug_to_region_size_ok_for_hw_watchpoint (int byte_count)
- {
- CORE_ADDR retval;
-
- retval = debug_target.to_region_size_ok_for_hw_watchpoint (byte_count);
-
- fprintf_unfiltered (gdb_stdlog,
- "TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT (%ld) = 0x%lx\n",
- (unsigned long) byte_count,
- (unsigned long) retval);
- return retval;
- }
-
- static int
debug_to_stopped_by_watchpoint (void)
{
int retval;
--- 2130,2135 ----
*************** setup_target_debug (void)
*** 2562,2568 ****
current_target.to_stopped_by_watchpoint = debug_to_stopped_by_watchpoint;
current_target.to_stopped_data_address = debug_to_stopped_data_address;
current_target.to_region_ok_for_hw_watchpoint = debug_to_region_ok_for_hw_watchpoint;
- current_target.to_region_size_ok_for_hw_watchpoint = debug_to_region_size_ok_for_hw_watchpoint;
current_target.to_terminal_init = debug_to_terminal_init;
current_target.to_terminal_inferior = debug_to_terminal_inferior;
current_target.to_terminal_ours_for_output = debug_to_terminal_ours_for_output;
--- 2535,2540 ----
diff -rcp src/gdb/target.h src.new/gdb/target.h
*** src/gdb/target.h 2006-01-24 16:59:38.000000000 -0500
--- src.new/gdb/target.h 2006-01-24 22:30:50.000000000 -0500
*************** struct target_ops
*** 346,352 ****
int to_have_continuable_watchpoint;
int (*to_stopped_data_address) (struct target_ops *, CORE_ADDR *);
int (*to_region_ok_for_hw_watchpoint) (CORE_ADDR, int);
- int (*to_region_size_ok_for_hw_watchpoint) (int);
void (*to_terminal_init) (void);
void (*to_terminal_inferior) (void);
void (*to_terminal_ours_for_output) (void);
--- 346,351 ----
*************** extern void (*deprecated_target_new_objf
*** 1036,1046 ****
(*current_target.to_region_ok_for_hw_watchpoint) (addr, len)
#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)
- #endif
-

/* Set/clear a hardware watchpoint starting at ADDR, for LEN bytes. TYPE is 0
for write, 1 for read, and 2 for read/write accesses. Returns 0 for
--- 1035,1040 ----

Regards
- Wu Zhou

On Tue, 24 Jan 2006, Daniel Jacobowitz wrote:

> On Tue, Jan 24, 2006 at 06:59:53PM +0800, Wu Zhou wrote:
> > Hi Daniel,
> >
> > Because there are quite a few places in different arch/target of GDB which
> > use region_size_ok_for_hw_watchpoint, so I am prefering to get this done
> > in two steps: first add to_region_ok_for_hw_watchpoint into struct
> > target_ops, then replace these to_region_size_ok_for_hw_watchpoint
> > reference with to_region_ok_for_hw_watchpoint ones. IMHO, it is easier to
> > not confuse the original intention of this patch with this replacement.
> > What is your thought on this?
>
> Sure. Couple small things but we're almost done.
>
> > static int
> > +default_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
> > +{
> > + return (len <= TYPE_LENGTH (builtin_type_void_data_ptr));
> > +}
>
> TARGET_REGION_OK_FOR_HW_WATCHPOINT will now always be defined, because
> of the #ifdef in target.h. Therefore this won't trigger (from
> breakpoint.c):
>
> #if !defined(TARGET_REGION_OK_FOR_HW_WATCHPOINT)
> #define TARGET_REGION_OK_FOR_HW_WATCHPOINT(ADDR,LEN) \
> (TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT(LEN))
> #endif
>
> So you need to call TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT (len) here,
> in case the target has overridden that.
>
>
> --
> Daniel Jacobowitz
> CodeSourcery
>
>