Re: [PATCH]: Make Linux use the new unified x86 watchpoint support

[Michael Snyder <msnyder at cygnus dot com>]

Mark Kettenis wrote:
> 
> FYI, I checked this in.  HJ can finally be happy now (although things
> probably won't work correctly for multithreaded programs).

Mark, this breaks remote i386 targets debugged from linux hosts.
STOPPED_BY_WATCHPOINT is unconditionally defined to a function in
i386-nat.c, but we may not be debugging a native target.

> 
> Mark
> 
> Index: ChangeLog
> from  Mark Kettenis  <kettenis@gnu.org>
> 
>         Make Linux use the new unified support for hardware breakpoints
>         and watchpoints on x86 targets.
>         * i386-linux-nat.c: Doc fixes.  Include "gdb_assert.h".
>         [HAVE_SYS_DEBUGREG_H]: Include <sys/debugreg.h>.
>         (DR_FIRSTADDR, DR_LASTADDR, DR_STATUS, DR_CONTROL): Define to
>         appropriate value if not already defined.
>         (register_u_addr): New function.
>         (kernel_u_size): New function.
>         (i386_linux_dr_get, i386_linux_dr_set): New functions.
>         (i386_linux_dr_set_control, i386_linux_dr_set_addr,
>         i386_linux_reset_addr, i386_linux_dr_get_status): New functions.
>         * config/i386/nm-linux.h: Don't include "nm-i386v.h".
>         (I386_USE_GENERIC_WATCHPOINTS): Define and include "nm-i386.h".
>         (TARGET_HAS_HARDWARE_WATCHPOINTS,
>         TARGET_CAN_USE_HARDWARE_WATCHPOINTS, HAVE_CONTINUABLE_WATCHPOINT,
>         STOPPED_BY_WATCHPOINT, target_insert_watchpoint,
>         target_remove_watchpoint): Remove macros.
>         (i386_stopped_by_watchpoint, i386_insert_watchpoint,
>         i386_remove_watchpoint): Remove prototypes.
>         (register_u_addr): New prototype.
>         (REGISTER_U_ADDR): Define in terms of register_u_addr.
>         (i386_linux_dr_set_control, i386_linux_dr_set_addr,
>         i386_linux_reset_addr, i386_linux_dr_get_status): New prototypes.
>         (I386_DR_LOW_SET_CONTROL, I386_DR_LOW_SET_ADDR,
>         I386_DR_LOW_RESET_ADDR, I386_DR_LOW_GET_STATUS): New macros.
>         * config/i386/linux.mh (NATDEPFILES): Replace i386v-nat.o with
>         i386-nat.o.
> 
> Index: i386-linux-nat.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/i386-linux-nat.c,v
> retrieving revision 1.23
> diff -u -p -r1.23 i386-linux-nat.c
> --- i386-linux-nat.c 2001/03/13 23:31:13 1.23
> +++ i386-linux-nat.c 2001/03/21 21:20:21
> @@ -23,6 +23,7 @@
>  #include "gdbcore.h"
>  #include "regcache.h"
> 
> +#include "gdb_assert.h"
>  #include <sys/ptrace.h>
>  #include <sys/user.h>
>  #include <sys/procfs.h>
> @@ -31,6 +32,26 @@
>  #include <sys/reg.h>
>  #endif
> 
> +#ifdef HAVE_SYS_DEBUGREG_H
> +#include <sys/debugreg.h>
> +#endif
> +
> +#ifndef DR_FIRSTADDR
> +#define DR_FIRSTADDR 0
> +#endif
> +
> +#ifndef DR_LASTADDR
> +#define DR_LASTADDR 3
> +#endif
> +
> +#ifndef DR_STATUS
> +#define DR_STATUS 6
> +#endif
> +
> +#ifndef DR_CONTROL
> +#define DR_CONTROL 7
> +#endif
> +
>  /* Prototypes for supply_gregset etc.  */
>  #include "gregset.h"
> 
> @@ -110,6 +131,26 @@ int have_ptrace_getfpxregs =
>  ;
> 
> 
> +/* Support for the user struct.  */
> +
> +/* Return the address of register REGNUM.  BLOCKEND is the value of
> +   u.u_ar0, which should point to the registers.  */
> +
> +CORE_ADDR
> +register_u_addr (CORE_ADDR blockend, int regnum)
> +{
> +  return (blockend + 4 * regmap[regnum]);
> +}
> +
> +/* Return the size of the user struct.  */
> +
> +int
> +kernel_u_size (void)
> +{
> +  return (sizeof (struct user));
> +}
> +
> +
>  /* Fetching registers directly from the U area, one at a time.  */
> 
>  /* FIXME: kettenis/2000-03-05: This duplicates code from `inptrace.c'.
> @@ -657,6 +698,72 @@ store_inferior_registers (int regno)
> 
>    internal_error (__FILE__, __LINE__,
>                   "Got request to store bad register number %d.", regno);
> +}
> +
> +
> +static long
> +i386_linux_dr_get (int regnum)
> +{
> +  int tid;
> +  long value;
> +
> +  /* FIXME: kettenis/2001-01-29: It's not clear what we should do with
> +     multi-threaded processes here.  For now, pretend there is just
> +     one thread.  */
> +  tid = PIDGET (inferior_pid);
> +
> +  errno = 0;
> +  value = ptrace (PT_READ_U, tid,
> +                 offsetof (struct user, u_debugreg[regnum]), 0);
> +  if (errno != 0)
> +    perror_with_name ("Couldn't read debug register");
> +
> +  return value;
> +}
> +
> +static void
> +i386_linux_dr_set (int regnum, long value)
> +{
> +  int tid;
> +
> +  /* FIXME: kettenis/2001-01-29: It's not clear what we should do with
> +     multi-threaded processes here.  For now, pretend there is just
> +     one thread.  */
> +  tid = PIDGET (inferior_pid);
> +
> +  errno = 0;
> +  ptrace (PT_WRITE_U, tid,
> +         offsetof (struct user, u_debugreg[regnum]), value);
> +  if (errno != 0)
> +    perror_with_name ("Couldn't write debug register");
> +}
> +
> +void
> +i386_linux_dr_set_control (long control)
> +{
> +  i386_linux_dr_set (DR_CONTROL, control);
> +}
> +
> +void
> +i386_linux_dr_set_addr (int regnum, CORE_ADDR addr)
> +{
> +  gdb_assert (regnum >= 0 && regnum <= DR_LASTADDR - DR_FIRSTADDR);
> +
> +  i386_linux_dr_set (DR_FIRSTADDR + regnum, addr);
> +}
> +
> +void
> +i386_linux_dr_reset_addr (int regnum)
> +{
> +  gdb_assert (regnum >= 0 && regnum <= DR_LASTADDR - DR_FIRSTADDR);
> +
> +  i386_linux_dr_set (DR_FIRSTADDR + regnum, 0L);
> +}
> +
> +long
> +i386_linux_dr_get_status (void)
> +{
> +  return i386_linux_dr_get (DR_STATUS);
>  }
> 
> 
> Index: config/i386/nm-linux.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/config/i386/nm-linux.h,v
> retrieving revision 1.8
> diff -u -p -r1.8 nm-linux.h
> --- config/i386/nm-linux.h 2001/03/06 08:21:28 1.8
> +++ config/i386/nm-linux.h 2001/03/21 21:20:21
> @@ -1,6 +1,6 @@
>  /* Native support for Linux/x86.
>     Copyright 1986, 1987, 1989, 1992, 1993, 1994, 1995, 1996, 1997, 1998,
> -   1999, 2000
> +   1999, 2000, 2001
>     Free Software Foundation, Inc.
> 
>     This file is part of GDB.
> @@ -23,52 +23,56 @@
>  #ifndef NM_LINUX_H
>  #define NM_LINUX_H
> 
> -#include "i386/nm-i386v.h"
> +/* GNU/Linux supports the i386 hardware debugging registers.  */
> +#define I386_USE_GENERIC_WATCHPOINTS
> +
> +#include "i386/nm-i386.h"
>  #include "nm-linux.h"
> 
> -/* Return sizeof user struct to callers in less machine dependent routines */
> +/* Return sizeof user struct to callers in less machine dependent
> +   routines.  */
> 
> -#define KERNEL_U_SIZE kernel_u_size()
>  extern int kernel_u_size (void);
> +#define KERNEL_U_SIZE kernel_u_size()
> 
>  #define U_REGS_OFFSET 0
> -
> -/* GNU/Linux supports the 386 hardware debugging registers.  */
> -
> -#define TARGET_HAS_HARDWARE_WATCHPOINTS
> -
> -#define TARGET_CAN_USE_HARDWARE_WATCHPOINT(type, cnt, ot) 1
> 
> -/* After a watchpoint trap, the PC points to the instruction after
> -   the one that caused the trap.  Therefore we don't need to step over it.
> -   But we do need to reset the status register to avoid another trap.  */
> -#define HAVE_CONTINUABLE_WATCHPOINT
> +extern CORE_ADDR register_u_addr (CORE_ADDR blockend, int regnum);
> +#define REGISTER_U_ADDR(addr, blockend, regnum) \
> +  (addr) = register_u_addr (blockend, regnum)
> +
> +/* Provide access to the i386 hardware debugging registers.  */
> +
> +extern void i386_linux_dr_set_control (long control);
> +#define I386_DR_LOW_SET_CONTROL(control) \
> +  i386_linux_dr_set_control (control)
> +
> +extern void i386_linux_dr_set_addr (int regnum, CORE_ADDR addr);
> +#define I386_DR_LOW_SET_ADDR(regnum, addr) \
> +  i386_linux_dr_set_addr (regnum, addr)
> +
> +extern void i386_linux_dr_reset_addr (int regnum);
> +#define I386_DR_LOW_RESET_ADDR(regnum) \
> +  i386_linux_dr_reset_addr (regnum)
> +
> +extern long i386_linux_dr_get_status (void);
> +#define I386_DR_LOW_GET_STATUS() \
> +  i386_linux_dr_get_status ()
> 
> -#define STOPPED_BY_WATCHPOINT(W)  \
> -  i386_stopped_by_watchpoint (inferior_pid)
> +/* We define this if link.h is available, because with ELF we use SVR4
> +   style shared libraries.  */
> 
> -/* Use these macros for watchpoint insertion/removal.  */
> -
> -#define target_insert_watchpoint(addr, len, type)  \
> -  i386_insert_watchpoint (inferior_pid, addr, len, type)
> -
> -#define target_remove_watchpoint(addr, len, type)  \
> -  i386_remove_watchpoint (inferior_pid, addr, len)
> -
> -/* We define this if link.h is available, because with ELF we use SVR4 style
> -   shared libraries. */
> -
>  #ifdef HAVE_LINK_H
>  #define SVR4_SHARED_LIBS
> -#include "solib.h"             /* Support for shared libraries. */
> +#include "solib.h"             /* Support for shared libraries.  */
>  #endif
> 
>  /* Override copies of {fetch,store}_inferior_registers in `infptrace.c'.  */
>  #define FETCH_INFERIOR_REGISTERS
> 
> -/* Nevertheless, define CANNOT_{FETCH,STORE}_REGISTER, because we might fall
> -   back on the code `infptrace.c' (well a copy of that code in
> -   `i386-linux-nat.c' for now) and we can access only the
> +/* Nevertheless, define CANNOT_{FETCH,STORE}_REGISTER, because we
> +   might fall back on the code `infptrace.c' (well a copy of that code
> +   in `i386-linux-nat.c' for now) and we can access only the
>     general-purpose registers in that way.  */
>  extern int cannot_fetch_register (int regno);
>  extern int cannot_store_register (int regno);
> @@ -77,10 +81,6 @@ extern int cannot_store_register (int re
> 
>  /* Override child_resume in `infptrace.c'.  */
>  #define CHILD_RESUME
> -
> -extern CORE_ADDR i386_stopped_by_watchpoint (int);
> -extern int i386_insert_watchpoint (int pid, CORE_ADDR addr, int len, int rw);
> -extern int i386_remove_watchpoint (int pid, CORE_ADDR addr, int len);
> 
>  /* FIXME: kettenis/2000-09-03: This should be moved to ../nm-linux.h
>     once we have converted all Linux targets to use the new threads
> Index: config/i386/linux.mh
> ===================================================================
> RCS file: /cvs/src/src/gdb/config/i386/linux.mh,v
> retrieving revision 1.6
> diff -u -p -r1.6 linux.mh
> --- config/i386/linux.mh 2000/10/30 22:33:32 1.6
> +++ config/i386/linux.mh 2001/03/21 21:20:21
> @@ -5,7 +5,7 @@ XDEPFILES=
> 
>  NAT_FILE= nm-linux.h
>  NATDEPFILES= infptrace.o inftarg.o fork-child.o corelow.o \
> -       core-aout.o i386v-nat.o i386-linux-nat.o i387-nat.o \
> +       core-aout.o i386-nat.o i386-linux-nat.o i387-nat.o \
>         proc-service.o thread-db.o lin-lwp.o
> 
>  LOADLIBES = -ldl -rdynamic