Re: [RFC] Set I386_USE_GENERIC_WATCHPOINTS in configure

[Pedro Alves <pedro@codesourcery.com>]

On Friday 13 March 2009 00:27:47, Pierre Muller wrote:
>   To get fully functional multi-arch,
> we should try to remove all nm files.

We need to make everything that is target dependent selectable
at runtime.  Things that affect native debugging only can stay here
without any prejudice.  E.g., not having watchpoints in the target
vector was something that was problematic for multi-target support,
hence the slow moving towards making I386_WATCHPOINTS_IN_TARGET_VECTOR
the default.  A define that only affects the native target
is not problematic, since there's only ever one native
target configured in.

>   
>   I tried to write a patch that completely removes
> the need of 
> I386_USE_GENERIC_WATCHPOINTS in config/i386/nm-*.h
> headers.
>   Instead 
> I386_USE_GENERIC_WATCHPOINTS and TARGET_HAS_DR_LEN_8
> are set in configure.

Why do we need I386_USE_GENERIC_WATCHPOINTS at all?

The right way to do this is to only add i386-nat.o to
NATDEPFILES in configurations that want to use it.  For fbsd,
you can guard the call to i386_use_watchpoints with
HAVE_PT_GETDBREGS, and completely get rid of
I386_USE_GENERIC_WATCHPOINTS.

I suggest we first eliminate I386_WATCHPOINTS_IN_TARGET_VECTOR
as a separate step, independently of the I386_USE_GENERIC_WATCHPOINTS
changes.

> 
>   This allows to almost completely empty
> the config/i386/nm*.h files.
> 
>   I used four function variables to replace
> the I386_DR_LOW_* macros, and set those variables
> close to where i386_use_watchpoints function
> is called.
>   I am not sure that this is the best solution,
> but I was afraid of the idea of implementing the 
> same function in different sources, with a possible
> conflict if two of these native files get compiled into
> the same executable. 

Not likely to happen anytime soon, if ever (cough).  GDB can
only be configured for one native target.

> 
> I am not really confident in the configure.ac part,
> as this is the first time I try to modify this
> file and I have no knowledge of autoconf :(
> 
> All remarks are welcome.
> 

I guess a new structure holding function pointers
would be cleaner.  Say,

struct i386_watchpoint_ops
{
   void (*dr_low_set_control) (unsigned);
   void (*dr_low_set_addr) (int, CORE_ADDR);
   void (*dr_low_reset_addr) (int);
   unsigned (*dr_low_get_status) (void);
   int dr_size;
};

This removes the need for any configure.ac change, I believe.


One thing that could be investigated is if exposing the
i386 debug registers through the register cache (exposed
as process wide registers, not per-thread registers) could
be a good interface for remote watchpoint support, thus making
remote targets benefit from the reference counting done on the
GDB side instead of implementing Z watchpoint packets.  Not
sure yet how feasible or efficient that is.

-- 
Pedro Alves