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

Daniel Jacobowitz
2006-01-24T21:20:53Z

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