This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Replace to_region_size_ok_for_hw_watchpoint references with to_region_ok_for_hw_watchpoint ones


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
> 
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]