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

[Daniel Jacobowitz <drow@false.org>]

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.

> 	(ppc_linux_check_watch_resources): New function to check whether
> 	the target has available hardware watchpoint resource.
> 	(ppc_linux_region_ok_for_hw_watchpoint): New function to check if
> 	the region is ok for hardware watchpoint.
> 	(ppc_linux_insert_watchpoint): New function to insert a hardware
> 	watchpoint.
> 	(ppc_linux_remove_watchpoint): New function to remove a hardware
> 	watchpoint.
> 	(ppc_linux_stopped_data_address): New function to get the stopped
> 	data address of the watchpoint.
> 	(ppc_linux_stopped_by_watchpoint): New function to see if the 
> 	inferior is stopped by watchpoint.

"New function" is sufficient; if you feel that the function needs an
explanation, it should go in the code, not in the changelog.

> 	target.h (struct target_ops): Add a new target vector 

Missed a "* " there.

> +  /* P630 has nonsteppable watchpoint.  So we are assuming that all PowerPC
> +     targets have nonsteppable watchpoint.  */
> +  set_gdbarch_have_nonsteppable_watchpoint (gdbarch, 1);

Please skip this comment.

> @@ -1575,6 +1582,12 @@ find_default_create_inferior (char *exec
>  }
>  
>  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