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

Wu Zhou
2006-02-08T05:35:14Z

Hi Daniel,

Sorry for the delayed reply. I am just back from a vacation.

On Wed, 1 Feb 2006, Daniel Jacobowitz wrote:

> On Wed, Jan 25, 2006 at 04:34:14PM +0800, Wu Zhou wrote:
> > 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?
>
> I'm not sure what you mean. After these patches, the only reference to
> TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT is in gdbint.texinfo (which I'd
> appreciate if you fixed, in a separate patch - thanks in advance).
> What are the two ways now?

What I mean is the second patch below, which you had said ok. :-)
That is the only way I can thought of at that time. Now that you had said
ok, I don't need to find a second way. :-)

BTW. I will fix the reference to TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT
in gdbint.texinfo in a separate patch after commiting this.

>
> On Wed, Jan 25, 2006 at 11:18:49AM +0800, Wu Zhou wrote:
> > OK to commit?
> >
> > 2006-01-22 Ben Elliston <bje@au1.ibm.com>
> > Wu Zhou <woodzltc@cn.ibm.com>
> >
> > * ppc-linux-nat.c (PTRACE_GET_DEBUGREG, PTRACE_SET_DEBUGREG,
> > PTRACE_GETSIGINFO): Define.
> > (last_stopped_data_address): New.
> > (ppc_linux_check_watch_resources): New function.
> > (ppc_linux_region_ok_for_hw_watchpoint): New function.
> > (ppc_linux_insert_watchpoint): New function.
> > (ppc_linux_remove_watchpoint): New function.
> > (ppc_linux_stopped_data_address): New function.
> > (ppc_linux_stopped_by_watchpoint): New function.
> > (_initialize_ppc_linux_nat): Set the above hardware watchpoint
> > related target vectors.
> > * rs6000-tdep.c (rs6000_gdbarch_init): Set PPC architectures
> > to have nonsteppable watchpoint.
> > * target.c (default_region_ok_for_hw_watchpoint,
> > debug_to_region_ok_for_hw_watchpoint): New prototypes.
> > (update_current_target): Inherit to_region_ok_for_hw_watchpoint
> > and set default to_region_ok_for_hw_watchpoint.
> > (default_region_ok_for_hw_watchpoint): New function.
> > (debug_to_region_ok_for_hw_watchpoint): New function.
> > (setup_target_debug): Set to_region_ok_for_hw_watchpoint of
> > debug_target.
> > * target.h (struct target_ops): Add a new target vector
> > to_region_ok_for_hw_watchpoint.
> > (TARGET_REGION_OK_FOR_HW_WATCHPOINT): Define this if it is not
> > defined anyplace else.
>
> On Wed, Jan 25, 2006 at 04:34:14PM +0800, Wu Zhou wrote:
> > 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.
>
> These patches are both OK. You might want to combine them - it's a
> much smaller diff :-) But it doesn't matter since you've already got
> them separated out.

Thanks for reviewing that. I would like to commit them one by one, thus I
don't need to think about how to re-describe them. :-)

I also think that it is clearer to differentiate the purpose of these two
patches. Combining them together seems a little confusing to me.

Best Regards
- Wu Zhou