Re: [RFC] Remove i386 low level debug register function from nm- header file.

Ulrich Weigand
2009-05-12T13:02:12Z

Pierre Muller wrote:

> Following my first RFC
> http://sourceware.org/ml/gdb-patches/2009-03/msg00197.html
>
> and the following answers, I resubmit
> a new version where all i386 low level debug register
> is contained in a single struct defined in the
> new i386-nat.h header (to comply with DOS file name restrictions
> as asked for by Eli).

This looks good to me. However, now that you've moved the
prototypes to i386-nat.h, there is no longer any need for
the config/i386/nm-i386.h file -- please get rid of that file
(by removing the places where it is included into other nm-
files).

Also, it seems to me that now there is no need any more for
the I386_USE_GENERIC_WATCHPOINTS define -- targets that use
watchpoints call the i386_use_watchpoints routines, those
that don't, do not.

I think you should provide everything in i386-nat.c (and
the new i386-nat.h) unconditionally, and eliminate the
various definitions of I386_USE_GENERIC_WATCHPOINTS in the
nm- header files.

Note that in combination with removing nm-i386.h, this
actually makes several nm- files completely empty, so they
should be removed as well.

> 2009-05-10 Pierre Muller <muller.u-strasbg.fr>
>
> Remove all i386 debug register low level macros in config tm files.

Should read "nm" files, not tm.

> +/* Targets should define this to use the generic x86 watchpoint support.
> */
> +#ifdef I386_USE_GENERIC_WATCHPOINTS

As mentioned above, please get rid of this conditional ...

> +
> +/* Add watchpoint methods to the provided target_ops.
> + Targets defining I386_USE_GENERIC_WATCHPOINTS should call
> + this. */

... as well as the second sentence of the comment.

> /* Support for 8-byte wide hw watchpoints. */
> #ifndef TARGET_HAS_DR_LEN_8
> -#define TARGET_HAS_DR_LEN_8 0
> +#define TARGET_HAS_DR_LEN_8 (i386_dr_low.debug_register_length == 8)
> #endif

Nobody should be defining this any more, so please eliminate
the #ifndef ... conditional.

> Index: src/gdb/i386fbsd-nat.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/i386fbsd-nat.c,v
> retrieving revision 1.18
> @@ -126,7 +127,19 @@ _initialize_i386fbsd_nat (void)
>
> /* Add some extra features to the common *BSD/i386 target. */
> t = i386bsd_target ();
> +
> +#ifdef I386_USE_GENERIC_WATCHPOINTS
> +
> i386_use_watchpoints (t);
> +
> + i386_dr_low.set_control = i386bsd_dr_set_control;
> + i386_dr_low.set_addr = i386bsd_dr_set_addr;
> + i386_dr_low.reset_addr = i386bsd_dr_reset_addr;
> + i386_dr_low.get_status = i386bsd_dr_get_status;
> + i386_set_debug_register_length (4);
> +#endif /* I386_USE_GENERIC_WATCHPOINTS */

In this case, because the header file currently conditionally defines
I386_USE_GENERIC_WATCHPOINTS:

#ifdef HAVE_PT_GETDBREGS
#define I386_USE_GENERIC_WATCHPOINTS
#endif

you should move that condition to the i386fbsd-nat.c file:

#ifdef HAVE_PT_GETDBREGS
i386_use_watchpoints (t);

...
#endif

> diff -u -p -r1.189 windows-nat.c
> --- windows-nat.c 17 Apr 2009 15:44:28 -0000 1.189
> +++ windows-nat.c 8 May 2009 14:02:25 -0000
> @@ -2166,8 +2171,22 @@ init_windows_ops (void)
> windows_ops.to_has_execution = 1;
> windows_ops.to_pid_to_exec_file = windows_pid_to_exec_file;
> windows_ops.to_get_ada_task_ptid = windows_get_ada_task_ptid;
> +
> +#ifdef I386_USE_GENERIC_WATCHPOINTS
> +
> i386_use_watchpoints (&windows_ops);
>
> + i386_dr_low.set_control = cygwin_set_dr7;
> + i386_dr_low.set_addr = cygwin_set_dr;
> + i386_dr_low.reset_addr = NULL;
> + i386_dr_low.get_status = cygwin_get_dr6;

Here, it seems every user of windows-nat.c unconditionally
defines I386_USE_GENERIC_WATCHPOINTS, so there's no point in
adding the #ifdef's to this file ...

Bye,
Ulrich

--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com