Re: [RFA] procfs.c: TARGET_CAN_USE_HARDWARE_WATCHPOINT via target vector

[Michael Snyder <msnyder@redhat.com>]

Kevin Buettner wrote:
> 
> On Aug 12,  5:21pm, Michael Snyder wrote:
> 
> > Kevin Buettner wrote:
> > >
> > > On Irix, it's possible to debug processes using the o32, n32, and n64
> > > ABIs from a single gdb.  Unfortunately, due to some limitations with
> > > the watchpoint support in procfs.c, it is not possible to use hardware
> > > watchpoints in all cases.  (See patch comments for details.)
> > >
> > > My first cut at a patch simply changed the macro in
> > > config/mips/nm-irix5.h.  However, since this same solution will be
> > > required for each platform requiring procfs.c, I decided to use the
> > > newly added hardware breakpoint facilities in the target vector.
> > >
> > > Okay to commit?
> >
> > Sure -- but don't you think it should default to zero?
> > Or is being able to do it the norm?  (I don't remember).
> 
> Here's the part of the patch to look at:
> 
> > > +/* Return non-zero if we can set a hardware watchpoint of type TYPE.  TYPE
> > > +   is one of bp_hardware_watchpoint, bp_read_watchpoint, bp_write_watchpoint,
> > > +   or bp_hardware_watchpoint.  CNT is the number of watchpoints used so
> > > +   far.
> > > +
> > > +   Note:  procfs_can_use_hw_breakpoint() is not yet used by all
> > > +   procfs.c targets due to the fact that some of them still define
> > > +   TARGET_CAN_USE_HARDWARE_WATCHPOINT.  */
> > > +
> > > +static int
> > > +procfs_can_use_hw_breakpoint (int type, int cnt, int othertype)
> > > +{
> > > +#ifndef TARGET_HAS_HARDWARE_WATCHPOINTS
> > > +  return 0;
> > > +#else
> > > +  /* Due to the way that proc_set_watchpoint() is implemented, host
> > > +     and target pointers must be of the same size.  If they are not,
> > > +     we can't use hardware watchpoints.  This limitation is due to the
> > > +     fact that proc_set_watchpoint() calls address_to_host_pointer();
> > > +     a close inspection of address_to_host_pointer will reveal that
> > > +     an internal error will be generated when the host and target
> > > +     pointer sizes are different.  */
> > > +  if (sizeof (void *) != TYPE_LENGTH (builtin_type_void_data_ptr))
> > > +    return 0;
> > > +
> > > +  /* Other tests here???  */
> > > +
> > > +  return 1;
> > > +#endif
> > >  }
> 
> So...
> 
> It returns zero for targets without hardware watchpoint support.
> 
> It returns 1 for targets with hardware watchpoint support,...
> 
> UNLESS the host and target pointer sizes don't match, in which case it
> returns 0.

Oh yeah.  Sorry, I missed that.

> I think the above is what procfs.c targets do for
> TARGET_CAN_USE_HARDWARE_WATCHPOINT with the exception of the "UNLESS"
> clause which is new.
> 
> BTW, it seems to me that we should have some other tests (for type,
> size, number of existing hardware watchpoints, etc) in the "Other
> tests here???" section, but I don't really know what these tests
> should be yet.  Also, none of the other procfs.c targets currently
> seem to care about this, so the functionality will be the same as
> what we currently have (right or wrong).

Yeah, if you take a survey, I think you'll find that virtually no
existing targets test for things like whether we've used up our
available watchpoint registers.  They mostly either always say
true, or always say false.