Re: [RFA] Unified watchpoints for x86 platforms

Re: [RFA] Unified watchpoints for x86 platforms

[Eli Zaretskii]

This patch supersedes the patch I submitted for approval in this
message:

  http://sources.redhat.com/ml/gdb/2001-03/msg00011.html

(as you see, I posted that to the wrong list ;-).

In response to concerns sent to me by Peter Schauer, I introduced a
new macro, I386_USE_GENERIC_WATCHPOINTS, which targets should define
to get the new watchpoint code in i386-tdep.c compiled.  Thus, targets
which don't define that macro, will continue to use their current
watchpoint support.  This avoids breaking any such targets, in case
the code I wrote conflicts with what they have now.

In other words, targets which want to use the code I wrote need to say
"I do" explicitly ;-)

2001-03-06  Eli Zaretskii  <eliz@is.elta.co.il>

	Unified support for hardware breakpoints and watchpoints on
	x86 targets:

	* config/i386/tm-i386.h (i386_cleanup_dregs, i386_insert_watchpoint)
	(i386_remove_watchpoint, i386_region_ok_for_watchpoint)
	(i386_stopped_by_hwbp, i386_stopped_data_address)
	(i386_insert_hw_breakpoint, i386_remove_hw_breakpoint): Declare
	prototypes.
	(TARGET_CAN_USE_HARDWARE_WATCHPOINT) 
	(TARGET_REGION_OK_FOR_HW_WATCHPOINT, HAVE_CONTINUABLE_WATCHPOINT) 
	(STOPPED_BY_WATCHPOINT, target_stopped_data_address) 
	(target_insert_watchpoint, target_remove_watchpoint) 
	(target_insert_hw_breakpoint, target_remove_hw_breakpoint): Define
	to call the appropriate i386_* functions.

	* i386-tdep.c (I386_DR_CONTROL_MASK, I386_DR_LOCAL_ENABLE) 
	(I386_DR_GLOBAL_ENABLE, I386_DR_DISABLE, I386_DR_SET_RW_LEN) 
	(I386_DR_GET_RW_LEN, I386_DR_WATCH_HIT): New macros.
	(dr_mirror, dr_status_mirror, dr_control_mirror, dr_ref_count)
	(maint_show_dr): New variables.
	(i386_cleanup_dregs, i386_show_dr, i386_length_and_rw_bits) 
	(i386_insert_aligned_watchpoint, i386_remove_aligned_watchpoint) 
	(i386_handle_nonaligned_watchpoint, i386_insert_watchpoint) 
	(i386_remove_watchpoint, i386_region_ok_for_watchpoint) 
	(i386_stopped_data_address, i386_stopped_by_hwbp) 
	(i386_insert_hw_breakpoint, i386_remove_hw_breakpoint): New
	functions.
	(_initialize_i386_tdep) [I386_USE_GENERIC_WATCHPOINTS]: Add new
	maint command `show-debug-regs', sets maint_show_dr to non-zero
	value and activates debugging print-outs in functions which
	insert, remove, and test watchpoints and hardware breakpoints.


--- gdb/config/i386/tm-i386.h~0	Thu Jan  4 17:46:20 2001
+++ gdb/config/i386/tm-i386.h	Tue Mar  6 20:01:38 2001
@@ -1,5 +1,5 @@
 /* Macro definitions for GDB on an Intel i[345]86.
-   Copyright (C) 1995, 1996, 2000 Free Software Foundation, Inc.
+   Copyright (C) 1995, 1996, 2000, 2001 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -418,4 +418,102 @@
 
 #define SP_ARG0 (1 * 4)
 
+\f
+
+/* Hardware-assisted breakpoints and watchpoints.  */
+
+#ifdef TARGET_HAS_HARDWARE_WATCHPOINTS
+
+/* Targets should define this to use the generic x86 watchpoint support.  */
+#ifdef I386_USE_GENERIC_WATCHPOINTS
+
+/* Clear the reference counts and forget everything we knew about DRi.  */
+extern void i386_cleanup_dregs (void);
+
+/* Insert a watchpoint to watch a memory region which starts at
+   address ADDR and whose length is LEN bytes.  Watch memory accesses
+   of the type TYPE.  Return 0 on success, -1 on failure.  */
+extern int  i386_insert_watchpoint (CORE_ADDR addr, int len, int type);
+
+/* Remove a watchpoint that watched the memory region which starts at
+   address ADDR, whose length is LEN bytes, and for accesses of the
+   type TYPE.  Return 0 on success, -1 on failure.  */
+extern int  i386_remove_watchpoint (CORE_ADDR addr, int len, int type);
+
+/* Return non-zero if we can watch a memory region that starts at
+   address ADDR and whose length is LEN bytes.  */
+extern int  i386_region_ok_for_watchpoint (CORE_ADDR addr, int len);
+
+/* Return non-zero if the inferior has some break/watchpoint that
+   triggered.  */
+extern int  i386_stopped_by_hwbp (void);
+
+/* If the inferior has some break/watchpoint that triggered, return
+   the address associated with that break/watchpoint.  Otherwise,
+   return zero.  */
+extern CORE_ADDR i386_stopped_data_address (void);
+
+/* Insert a hardware-assisted breakpoint at address ADDR.  SHADOW is
+   unused.  Return 0 on success, EBUSY on failure.  */
+extern int  i386_insert_hw_breakpoint (CORE_ADDR addr, void *shadow);
+
+/* Remove a hardware-assisted breakpoint at address ADDR.  SHADOW is
+   unused.  Return 0 on success, -1 on failure.  */
+extern int  i386_remove_hw_breakpoint (CORE_ADDR addr, void *shadow);
+
+/* Returns the number of hardware watchpoints of type TYPE that we can
+   set.  Value is positive if we can set CNT watchpoints, zero if
+   setting watchpoints of type TYPE is not supported, and negative if
+   CNT is more than the maximum number of watchpoints of type TYPE
+   that we can support.  TYPE is one of bp_hardware_watchpoint,
+   bp_read_watchpoint, bp_write_watchpoint, or bp_hardware_breakpoint.
+   CNT is the number of such watchpoints used so far (including this
+   one).  OTHERTYPE is non-zero if other types of watchpoints are
+   currently enabled.
+
+   We always return 1 here because we don't have enough information
+   about possible overlap of addresses that they want to watch.  As
+   an extreme example, consider the case where all the watchpoints
+   watch the same address and the same region length: then we can
+   handle a virtually unlimited number of watchpoints, due to debug
+   register sharing implemented via reference counts in i386-tdep.c.  */
+
+#define TARGET_CAN_USE_HARDWARE_WATCHPOINT(type, cnt, ot) 1
+
+/* Returns non-zero if we can use hardware watchpoints to watch a region
+   whose address is ADDR and whose length is LEN.  */
+
+#define TARGET_REGION_OK_FOR_HW_WATCHPOINT(addr,len) \
+	i386_region_ok_for_watchpoint(addr,len)
+
+/* 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
+
+#define STOPPED_BY_WATCHPOINT(W)       (i386_stopped_data_address () != 0)
+
+#define target_stopped_data_address()  i386_stopped_data_address ()
+
+/* Use these macros for watchpoint insertion/removal.  */
+
+#define target_insert_watchpoint(addr, len, type)  \
+  i386_insert_watchpoint (addr, len, type)
+
+#define target_remove_watchpoint(addr, len, type)  \
+  i386_remove_watchpoint (addr, len, type)
+
+#define target_insert_hw_breakpoint(addr, shadow)  \
+  i386_insert_hw_breakpoint(addr, shadow)
+
+#define target_remove_hw_breakpoint(addr, shadow)  \
+  i386_remove_hw_breakpoint(addr, shadow)
+
+#define DECR_PC_AFTER_HW_BREAK 0
+
+#endif /* I386_USE_GENERIC_WATCHPOINTS */
+
+#endif /* TARGET_HAS_HARDWARE_WATCHPOINTS */
+
 #endif /* ifndef TM_I386_H */


--- gdb/i386-tdep.c~0	Thu Dec 21 22:52:58 2000
+++ gdb/i386-tdep.c	Tue Mar  6 20:03:30 2001
@@ -1,5 +1,5 @@
 /* Intel 386 target-dependent stuff.
-   Copyright (C) 1988, 1989, 1991, 1994, 1995, 1996, 1998
+   Copyright (C) 1988, 1989, 1991, 1994, 1995, 1996, 1998, 2001
    Free Software Foundation, Inc.
 
    This file is part of GDB.
@@ -858,6 +858,663 @@
   memcpy (to, from, FPU_REG_RAW_SIZE);
 }
 
+\f
+/* Support for hardware watchpoints and breakpoints using the x86
+   debug registers.
+
+   This provides several functions for inserting and removing
+   hardware-assisted breakpoints and watchpoints, testing if
+   one or more of the watchpoints triggerd and at what address,
+   checking whether a given region can be watched, etc.  See
+   the section "Exported API functions" below.
+
+   A target which wants to use these functions should define
+   several macros, such as `target_insert_watchpoint' and
+   `target_stopped_data_address', listed in target.h, to call
+   the appropriate functions below.  It should also define
+   I386_USE_GENERIC_WATCHPOINTS in its tm.h file.
+
+   In addition, each target should provide several low-level
+   macros that will be called to insert watchpoints and hardware
+   breakpoints into the inferior, remove them, and check their
+   status.  These macros are:
+
+      I386_DR_LOW_SET_CONTROL  -- set the debug control (DR7)
+				  register to a given value
+
+      I386_DR_LOW_SET_ADDR     -- put an address into one debug
+				  register
+
+      I386_DR_LOW_RESET_ADDR   -- reset the address stored in
+				  one debug register
+
+      I386_DR_LOW_GET_STATUS   -- return the value of the debug
+				  status (DR6) register.
+
+   The functions below implement debug registers sharing by
+   reference counts, and allow to watch regions up to 16 bytes
+   long.  */
+
+#ifdef TARGET_HAS_HARDWARE_WATCHPOINTS
+
+#ifdef I386_USE_GENERIC_WATCHPOINTS
+
+#ifdef HAVE_PTRACE_H
+#include <ptrace.h>
+#else
+#ifdef HAVE_SYS_PTRACE_H
+#include <sys/ptrace.h>
+#endif
+#endif
+
+#ifdef HAVE_SYS_USER
+#include <sys/user.h>
+#endif
+
+/* FIXME: The following should be just "#include <sys/debugreg.h>",
+   but the the Linux 2.1.x kernel and glibc 2.0.x are not in sync;
+   including <sys/debugreg.h> will result in an error.  With luck,
+   these losers will get their act together and we can trash this hack
+   in the near future.
+
+   --jsm 1998-10-21; modified by eliz 2001-02-24.  */
+
+#ifdef HAVE_ASM_DEBUGREG_H
+#include <asm/debugreg.h>
+#else  /* !HAVE_ASM_DEBUGREG_H */
+
+#ifdef HAVE_SYS_DEBUGREG_H
+#include <sys/debugreg.h>
+#else  /* !HAVE_SYS_DEBUGREG_H */
+
+/* Provide definitions for platforms which don't have debugreg.h, such
+   as DJGPP.  As a side effect, explain what each macro means or does.  */
+
+/* Debug registers' indices.  */
+#define DR_FIRSTADDR		0  /* index of first debug address register */
+#define DR_LASTADDR		3  /* index of last debug address register */
+#define DR_STATUS		6  /* index of debug status register (DR6) */
+#define DR_CONTROL		7  /* index of debug control register (DR7) */
+
+/* DR7 Debug Control register fields.  */
+
+/* How many bits to skip in DR7 to get to R/W and LEN fields.  */
+#define DR_CONTROL_SHIFT	16
+/* How many bits in DR7 per R/W and LEN field for each watchpoint.  */
+#define DR_CONTROL_SIZE		4
+
+/* Watchpoint/breakpoint read/write fields in DR7.  */
+#define DR_RW_EXECUTE		(0x0) /* break on instruction execution */
+#define DR_RW_WRITE		(0x1) /* break on data writes */
+#define DR_RW_READ		(0x3) /* break on data reads or writes */
+
+/* Watchpoint/breakpoint length fields in DR7.  The 2-bit left shift
+   is so we could OR this with the read/write field defined above.  */
+#define DR_LEN_1		(0x0 << 2) /* 1-byte region watch or breakpt */
+#define DR_LEN_2		(0x1 << 2) /* 2-byte region watch */
+#define DR_LEN_4		(0x3 << 2) /* 4-byte region watch */
+
+/* Local and Global Enable flags in DR7.
+
+   When the Local Enable flag is set, the breakpoint/watchpoint is
+   enabled only for the current task; the processor automatically
+   clears this flag on every task switch.  When the Global Enable
+   flag is set, the breakpoint/watchpoint is enabled for all tasks;
+   the processor never clears this flag.
+
+   Currently, all watchpoint are locally enabled.  If you need to
+   enable them globally, read the comment which pertains to this in
+   i386_insert_aligned_watchpoint below.  */
+#define DR_LOCAL_ENABLE_SHIFT	0   /* extra shift to the local enable bit */
+#define DR_GLOBAL_ENABLE_SHIFT	1   /* extra shift to the global enable bit */
+#define DR_ENABLE_SIZE		2   /* 2 enable bits per debug register */
+
+/* Local and global exact breakpoint enable flags (a.k.a. slowdown
+   flags).  These are only required on i386, to allow detection of the
+   exact instruction which caused a watchpoint to break; i486 and
+   later processors do that automatically.  We set these flags for
+   back compatibility.  */
+#define DR_LOCAL_SLOWDOWN	(0x100)
+#define DR_GLOBAL_SLOWDOWN	(0x200)
+
+/* Fields reserved by Intel.  This includes the GD (General Detect
+   Enable) flag, which causes a debug exception to be generated when a
+   MOV instruction accesses one of the debug registers.
+
+   FIXME: My Intel manual says we should use 0xF800, not 0xFC00.  */
+#define DR_CONTROL_RESERVED	(0xFC00)
+
+#endif /* !HAVE_SYS_DEBUGREG_H */
+
+#endif /* !HAVE_ASM_DEBUGREG_H */
+
+/* This is here for completeness.  No platform supports this
+   functionality yet (as of Feb-2001).  Note that the DE flag in the
+   CR4 register needs to be set to support this.  */
+#ifndef DR_RW_IORW
+#define DR_RW_IORW		(0x2) /* break on I/O reads or writes */
+#endif
+
+/* Auxiliary helper macros.  */
+
+/* A value that masks all fields in DR7 that are reserved by Intel.  */
+#define I386_DR_CONTROL_MASK		(~DR_CONTROL_RESERVED)
+
+/* The I'th debug register is vacant if its Local and Global Enable
+   bits are reset in the Debug Control register.  */
+#define I386_DR_VACANT(i) \
+  ((dr_control_mirror & (3 << (DR_ENABLE_SIZE * (i)))) == 0)
+
+/* Locally enable the break/watchpoint in the I'th debug register.  */
+#define I386_DR_LOCAL_ENABLE(i) \
+  dr_control_mirror |= (1 << (DR_LOCAL_ENABLE_SHIFT + DR_ENABLE_SIZE * (i)))
+
+/* Globally enable the break/watchpoint in the I'th debug register.  */
+#define I386_DR_GLOBAL_ENABLE(i) \
+  dr_control_mirror |= (1 << (DR_GLOBAL_ENABLE_SHIFT + DR_ENABLE_SIZE * (i)))
+
+/* Disable the break/watchpoint in the I'th debug register.  */
+#define I386_DR_DISABLE(i) \
+  dr_control_mirror &= ~(3 << (DR_ENABLE_SIZE * (i)))
+
+/* Set in DR7 the RW and LEN fields for the I'th debug register.  */
+#define I386_DR_SET_RW_LEN(i,rwlen) \
+  do { \
+    dr_control_mirror &= ~(0x0f << (DR_CONTROL_SHIFT+DR_CONTROL_SIZE*(i)));   \
+    dr_control_mirror |= ((rwlen) << (DR_CONTROL_SHIFT+DR_CONTROL_SIZE*(i))); \
+  } while (0)
+
+/* Get from DR7 the RW and LEN fields for the I'th debug register.  */
+#define I386_DR_GET_RW_LEN(i) \
+  ((dr_control_mirror >> (DR_CONTROL_SHIFT + DR_CONTROL_SIZE * (i))) & 0x0f)
+
+/* Did the watchpoint whose address is in the I'th register break?  */
+#define I386_DR_WATCH_HIT(i)	(dr_status_mirror & (1 << (i)))
+
+/* A macro to loop over all debug registers.  */
+#define ALL_DEBUG_REGISTERS(i)	for (i = 0; i <= DR_LASTADDR-DR_FIRSTADDR; i++)
+
+/* This is in i386v-nat.c, so let's have it here, just in case.  */
+#if !defined (offsetof)
+#define offsetof(TYPE, MEMBER) ((unsigned long) &((TYPE *)0)->MEMBER)
+#endif
+
+/* Mirror the inferior's DRi registers.  We keep the status and
+   control registers separated because they don't hold addresses.  */
+static CORE_ADDR dr_mirror[DR_LASTADDR - DR_FIRSTADDR + 1];
+static unsigned	 dr_status_mirror, dr_control_mirror;
+
+/* Reference counts for each debug register.  */
+static int	 dr_ref_count[DR_LASTADDR - DR_FIRSTADDR + 1];
+
+/* Whether or not to print the mirrored debug registers.  */
+static int	 maint_show_dr;
+
+/* Types of operations supported by i386_handle_nonaligned_watchpoint.  */
+typedef enum { WP_INSERT, WP_REMOVE, WP_COUNT } i386_wp_op_t;
+
+/* Exported API functions.  */
+
+/* Clear the reference counts and forget everything we knew about DRi.  */
+void i386_cleanup_dregs (void);
+
+/* Insert a watchpoint to watch a memory region which starts at
+   address ADDR and whose length is LEN bytes.  Watch memory accesses
+   of the type TYPE.  Return 0 on success, -1 on failure.  */
+int  i386_insert_watchpoint (CORE_ADDR addr, int len, int type);
+
+/* Remove a watchpoint that watched the memory region which starts at
+   address ADDR, whose length is LEN bytes, and for accesses of the
+   type TYPE.  Return 0 on success, -1 on failure.  */
+int  i386_remove_watchpoint (CORE_ADDR addr, int len, int type);
+
+/* Return non-zero if we can watch a memory region that starts at
+   address ADDR and whose length is LEN bytes.  */
+int  i386_region_ok_for_watchpoint (CORE_ADDR addr, int len);
+
+/* Return non-zero if the inferior has some break/watchpoint that
+   triggered.  */
+int  i386_stopped_by_hwbp (void);
+
+/* If the inferior has some break/watchpoint that triggered, return
+   the address associated with that break/watchpoint.  Otherwise,
+   return zero.  */
+CORE_ADDR i386_stopped_data_address (void);
+
+/* Insert a hardware-assisted breakpoint at address ADDR.  SHADOW is
+   unused.  Return 0 on success, EBUSY on failure.  */
+int  i386_insert_hw_breakpoint (CORE_ADDR addr, void *shadow);
+
+/* Remove a hardware-assisted breakpoint at address ADDR.  SHADOW is
+   unused.  Return 0 on success, -1 on failure.  */
+int  i386_remove_hw_breakpoint (CORE_ADDR addr, void *shadow);
+
+/* Internal functions.  */
+
+/* Return the value of a 4-bit field for DR7 suitable for watching a
+   region of LEN bytes for accesses of type TYPE.  LEN is assumed
+   to have the value of 1, 2, or 4.  */
+static unsigned i386_length_and_rw_bits (int len, enum target_hw_bp_type type);
+
+/* Insert a watchpoint at address ADDR, which is assumed to be aligned
+   according to the length of the region to watch.  LEN_RW_BITS is the
+   value of the bit-field from DR7 which describes the length and
+   access type of the region to be watched by this watchpoint.  Return
+   0 on success, -1 on failure.  */
+static int i386_insert_aligned_watchpoint (CORE_ADDR addr,
+					   unsigned len_rw_bits);
+
+/* Remove a watchpoint at address ADDR, which is assumed to be aligned
+   according to the length of the region to watch.  LEN_RW_BITS is the
+   value of the bits from DR7 which describes the length and access
+   type of the region watched by this watchpoint.  Return 0 on
+   success, -1 on failure.  */
+static int i386_remove_aligned_watchpoint (CORE_ADDR addr,
+					   unsigned len_rw_bits);
+
+/* Insert or remove a (possibly non-aligned) watchpoint, or count the
+   number of debug registers required to watch a region at address
+   ADDR whose length is LEN for accesses of type TYPE.  Return 0 on
+   successful insertion or removal, a positive number when queried
+   about the number of registers, or -1 on failure.  If WHAT is not
+   a valid value, returns EINVAL.  */
+static int i386_handle_nonaligned_watchpoint (i386_wp_op_t what,
+					      CORE_ADDR addr, int len,
+					      enum target_hw_bp_type type);
+
+/* Implementation.  */
+
+/* Clear the reference counts and forget everything we knew about
+   the debug registers.  */
+void
+i386_cleanup_dregs (void)
+{
+  int i;
+
+  ALL_DEBUG_REGISTERS(i)
+    {
+      dr_mirror[i] = 0;
+      dr_ref_count[i] = 0;
+    }
+  dr_control_mirror = 0;
+  dr_status_mirror  = 0;
+}
+
+/* Print the values of the mirrored debug registers.
+   This is called when maint_show_dr is non-zero.  To set that
+   up, type "maint show-debug-regs" at GDB's prompt.  */
+static void
+i386_show_dr (const char *func, CORE_ADDR addr,
+	      int len, enum target_hw_bp_type type)
+{
+  int i;
+
+  puts_unfiltered (func);
+  if (addr || len)
+    printf_unfiltered (" (addr=%lx, len=%d, type=%s)",
+		       /* This code is for ia32, so casting CORE_ADDR
+			  to unsigned long should be okay.  */
+		       (unsigned long)addr, len,
+		       type == hw_write ? "data-write"
+		       : (type == hw_read ? "data-read"
+			  : (type == hw_access ? "data-read/write"
+			     : (type == hw_execute ? "instruction-execute"
+				/* FIXME: if/when I/O read/write
+				   watchpoints are supported, add them
+				   here.  */
+				: "??unknown??"))));
+  puts_unfiltered (":\n");
+  printf_unfiltered ("CONTROL (DR7): %08x          STATUS (DR6): %08x\n",
+		     dr_control_mirror, dr_status_mirror);
+  ALL_DEBUG_REGISTERS(i)
+    {
+      printf_unfiltered ("DR%d: addr=%08lx, ref.count=%d  DR%d: addr=%08lx, ref.count=%d\n",
+			 i, dr_mirror[i], dr_ref_count[i],
+			 i+1, dr_mirror[i+1], dr_ref_count[i+1]);
+      i++;
+    }
+}
+
+/* Return the value of a 4-bit field for DR7 suitable for watching a
+   region of LEN bytes for accesses of type TYPE.  LEN is assumed
+   to have the value of 1, 2, or 4.  */
+static unsigned
+i386_length_and_rw_bits (int len, enum target_hw_bp_type type)
+{
+  unsigned rw;
+
+  switch (type)
+    {
+      case hw_execute:
+	rw = DR_RW_EXECUTE;
+	break;
+      case hw_write:
+	rw = DR_RW_WRITE;
+	break;
+      case hw_read:	 /* x86 doesn't support data-read watchpoints */
+      case hw_access:
+	rw = DR_RW_READ;
+	break;
+#if 0
+      case hw_io_access: /* not yet supported */
+	rw = DR_RW_IORW;
+	break;
+#endif
+      default:
+	internal_error (__FILE__, __LINE__, "\
+Invalid hw breakpoint type %d in i386_length_and_rw_bits.\n", (int)type);
+    }
+
+  switch (len)
+    {
+      case 4:
+	return (DR_LEN_4 | rw);
+      case 2:
+	return (DR_LEN_2 | rw);
+      case 1:
+	return (DR_LEN_1 | rw);
+      default:
+	internal_error (__FILE__, __LINE__, "\
+Invalid hw breakpoint length %d in i386_length_and_rw_bits.\n", len);
+    }
+}
+
+/* Insert a watchpoint at address ADDR, which is assumed to be aligned
+   according to the length of the region to watch.  LEN_RW_BITS is the
+   value of the bits from DR7 which describes the length and access
+   type of the region to be watched by this watchpoint.  Return 0 on
+   success, -1 on failure.  */
+static int
+i386_insert_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits)
+{
+  int i;
+
+  /* First, look for an occupied debug register with the same address
+     and the same RW and LEN definitions.  If we find one, we can
+     reuse it for this watchpoint as well (and save a register).  */
+  ALL_DEBUG_REGISTERS(i)
+    {
+      if (!I386_DR_VACANT (i)
+	  && dr_mirror[i] == addr
+	  && I386_DR_GET_RW_LEN (i) == len_rw_bits)
+	{
+	  dr_ref_count[i]++;
+	  return 0;
+	}
+    }
+
+  /* Next, look for a vacant debug register.  */
+  ALL_DEBUG_REGISTERS(i)
+    {
+      if (I386_DR_VACANT (i))
+	break;
+    }
+
+  /* No more debug registers!  */
+  if (i > DR_LASTADDR - DR_FIRSTADDR)
+    return -1;
+
+  /* Now set up the register I to watch our region.  */
+
+  /* Record the info in our local mirrored array.  */
+  dr_mirror[i] = addr;
+  dr_ref_count[i] = 1;
+  I386_DR_SET_RW_LEN (i, len_rw_bits);
+  /* Note: we only enable the watchpoint locally, i.e. in the current
+     task.  Currently, no x86 target allows or supports global
+     watchpoints; however, if any target would want that in the
+     future, GDB should probably provide a command to control whether
+     to enable watchpoints globally or locally, and the code below
+     should use global or local enable and slow-down flags as
+     appropriate.  */
+  I386_DR_LOCAL_ENABLE (i);
+  dr_control_mirror |= DR_LOCAL_SLOWDOWN;
+  dr_control_mirror &= I386_DR_CONTROL_MASK;
+
+  /* Finally, actually pass the info to the inferior.  */
+  I386_DR_LOW_SET_CONTROL (dr_control_mirror);
+  I386_DR_LOW_SET_ADDR (i + DR_FIRSTADDR, addr);
+
+  return 0;
+}
+
+/* Remove a watchpoint at address ADDR, which is assumed to be aligned
+   according to the length of the region to watch.  LEN_RW_BITS is the
+   value of the bits from DR7 which describes the length and access
+   type of the region watched by this watchpoint.  Return 0 on
+   success, -1 on failure.  */
+static int
+i386_remove_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits)
+{
+  int i, retval = -1;
+
+  ALL_DEBUG_REGISTERS(i)
+    {
+      if (!I386_DR_VACANT (i)
+	  && dr_mirror[i] == addr
+	  && I386_DR_GET_RW_LEN (i) == len_rw_bits)
+	{
+	  if (--dr_ref_count[i] == 0) /* no longer in use? */
+	    {
+	      /* Reset our mirror.  */
+	      dr_mirror[i] = 0;
+	      I386_DR_DISABLE (i);
+	      /* Reset it in the inferior.  */
+	      I386_DR_LOW_RESET_ADDR (i + DR_FIRSTADDR);
+	      I386_DR_LOW_SET_CONTROL (dr_control_mirror);
+	    }
+	  retval = 0;
+	}
+    }
+
+  return retval;
+}
+
+/* Insert or remove a (possibly non-aligned) watchpoint, or count the
+   number of debug registers required to watch a region at address
+   ADDR whose length is LEN for accesses of type TYPE.  Return 0 on
+   successful insertion or removal, a positive number when queried
+   about the number of registers, or -1 on failure.  If WHAT is not
+   a valid value, returns EINVAL.  */
+static int
+i386_handle_nonaligned_watchpoint (i386_wp_op_t what, CORE_ADDR addr, int len,
+				   enum target_hw_bp_type type)
+{
+  int align;
+  int size;
+  int rv = 0, status = 0;
+
+  static int size_try_array[4][4] =
+  {
+    { 1, 1, 1, 1 },		/* trying size one */
+    { 2, 1, 2, 1 },		/* trying size two */
+    { 2, 1, 2, 1 },		/* trying size three */
+    { 4, 1, 2, 1 }		/* trying size four */
+  };
+
+  while (len > 0)
+    {
+      align = addr % 4;
+      /* Four is the maximum length an x86 debug register can watch.  */
+      size = size_try_array[len > 4 ? 3 : len - 1][align];
+      if (what == WP_COUNT)
+	/* size_try_array[] is defined so that each iteration through
+	   the loop is guaranteed to produce an address and a size
+	   that can be watched with a single debug register.  Thus,
+	   for counting the registers required to watch a region, we
+	   simply need to increment the count on each iteration.  */
+	rv++;
+      else
+	{
+	  unsigned len_rw = i386_length_and_rw_bits (size, type);
+
+	  if (what == WP_INSERT)
+	    status = i386_insert_aligned_watchpoint (addr, len_rw);
+	  else if (what == WP_REMOVE)
+	    status = i386_remove_aligned_watchpoint (addr, len_rw);
+	  else
+	    status = EINVAL;
+	  /* We keep the loop going even after a failure, because some
+	     of the other aligned watchpoints might still succeed
+	     (e.g. if they watch addresses that are already watched,
+	     in which case we just increment the reference counts of
+	     occupied debug registers).  If we break out of the loop
+	     too early, we could cause those addresses watched by
+	     other watchpoints to be disabled when breakpoint.c reacts
+	     to our failure to insert this watchpoint and tries to
+	     remove it.  */
+	  if (status)
+	    rv = status;
+	}
+      addr += size;
+      len -= size;
+    }
+  return rv;
+}
+
+/* Insert a watchpoint to watch a memory region which starts at
+   address ADDR and whose length is LEN bytes.  Watch memory accesses
+   of the type TYPE.  Return 0 on success, -1 on failure.  */
+int
+i386_insert_watchpoint (CORE_ADDR addr, int len, int type)
+{
+  int retval;
+
+  if (len == 3 || len > 4 || addr % len != 0)
+    retval = i386_handle_nonaligned_watchpoint (WP_INSERT, addr, len, type);
+  else
+    {
+      unsigned len_rw = i386_length_and_rw_bits (len, type);
+
+      retval = i386_insert_aligned_watchpoint (addr, len_rw);
+    }
+
+  if (maint_show_dr)
+    i386_show_dr ("insert_watchpoint", addr, len, type);
+
+  return retval;
+}
+
+/* Remove a watchpoint that watched the memory region which starts at
+   address ADDR, whose length is LEN bytes, and for accesses of the
+   type TYPE.  Return 0 on success, -1 on failure.  */
+int
+i386_remove_watchpoint (CORE_ADDR addr, int len, int type)
+{
+  int retval;
+
+  if (len == 3 || len > 4 || addr % len != 0)
+    retval = i386_handle_nonaligned_watchpoint (WP_REMOVE, addr, len, type);
+  else
+    {
+      unsigned len_rw = i386_length_and_rw_bits (len, type);
+
+      retval = i386_remove_aligned_watchpoint (addr, len_rw);
+    }
+
+  if (maint_show_dr)
+    i386_show_dr ("remove_watchpoint", addr, len, type);
+
+  return retval;
+}
+
+/* Return non-zero if we can watch a memory region that starts at
+   address ADDR and whose length is LEN bytes.  */
+int
+i386_region_ok_for_watchpoint (CORE_ADDR addr, int len)
+{
+  /* Compute how many aligned watchpoints we would need to cover this
+     region.  */
+  int nregs = i386_handle_nonaligned_watchpoint (WP_COUNT, addr, len,
+						 hw_write);
+
+  return nregs <= 4 ? 1 : 0;
+}
+
+/* If the inferior has some watchpoint that triggered, return the
+   address associated with that watchpoint.  Otherwise, return
+   zero.  */
+CORE_ADDR
+i386_stopped_data_address (void)
+{
+  int i;
+  CORE_ADDR ret = 0;
+
+  dr_status_mirror = I386_DR_LOW_GET_STATUS ();
+
+  ALL_DEBUG_REGISTERS(i)
+    {
+      if (I386_DR_WATCH_HIT (i)
+	  /* This second condition makes sure DRi is set up for a data
+	     watchpoint, not a hardware breakpoint.  The reason is
+	     that GDB doesn't call the target_stopped_data_address
+	     method except for data watchpoints.  In other words, I'm
+	     being paranoiac.  */
+	  && I386_DR_GET_RW_LEN (i) != 0)
+	{
+	  ret = dr_mirror[i];
+	  if (maint_show_dr)
+	    i386_show_dr ("watchpoint_hit", ret, -1, hw_write);
+	}
+    }
+  if (maint_show_dr && ret == 0)
+    i386_show_dr ("stopped_data_addr", 0, 0, hw_write);
+
+  return ret;
+}
+
+/* Return non-zero if the inferior has some break/watchpoint that
+   triggered.  */
+int
+i386_stopped_by_hwbp (void)
+{
+  int i;
+
+  dr_status_mirror = I386_DR_LOW_GET_STATUS ();
+  if (maint_show_dr)
+    i386_show_dr ("stopped_by_hwbp", 0, 0, hw_execute);
+
+  ALL_DEBUG_REGISTERS(i)
+    {
+      if (I386_DR_WATCH_HIT (i))
+	return 1;
+    }
+
+  return 0;
+}
+
+/* Insert a hardware-assisted breakpoint at address ADDR.  SHADOW is
+   unused.  Return 0 on success, EBUSY on failure.  */
+int
+i386_insert_hw_breakpoint (CORE_ADDR addr, void *shadow)
+{
+  unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
+  int retval = i386_insert_aligned_watchpoint (addr, len_rw) ? EBUSY : 0;
+
+  if (maint_show_dr)
+    i386_show_dr ("insert_hwbp", addr, 1, hw_execute);
+
+  return retval;
+}
+
+/* Remove a hardware-assisted breakpoint at address ADDR.  SHADOW is
+   unused.  Return 0 on success, -1 on failure.  */
+int
+i386_remove_hw_breakpoint (CORE_ADDR addr, void *shadow)
+{
+  unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
+  int retval = i386_remove_aligned_watchpoint (addr, len_rw);
+
+  if (maint_show_dr)
+    i386_show_dr ("remove_hwbp", addr, 1, hw_execute);
+
+  return retval;
+}
+
+#endif /* I386_USE_GENERIC_WATCHPOINTS */
+
+#endif /* TARGET_HAS_HARDWARE_WATCHPOINTS */
+
 \f     
 #ifdef I386V4_SIGTRAMP_SAVED_PC
 /* Get saved user PC for sigtramp from the pushed ucontext on the stack
@@ -1014,4 +1671,17 @@
      in the disassembly_flavor variable */
 
   set_disassembly_flavor ();
+
+#ifdef I386_USE_GENERIC_WATCHPOINTS
+  /* A maintenance command to enable printing the internal DRi mirror
+     variables.  */
+  add_set_cmd ("show-debug-regs", class_maintenance,
+	       var_boolean, (char *) &maint_show_dr,
+	       "\
+Set whether to show variables that mirror the x86 debug registers.\n\
+Use \"on\" to enable, \"off\" to disable.\n\
+If enabled, the debug registers values are shown when GDB inserts\n\
+or removes a hardware breakpoint or watchpoint, and when the inferior\n\
+triggers a breakpoint or watchpoint.", &maintenancelist);
+#endif
 }

Re: [RFA] Unified watchpoints for x86 platforms

[Mark Kettenis]

Eli Zaretskii <eliz@is.elta.co.il> writes:

Hi Eli,

Sorry for not responding sooner.  I've been suffering from a bad flu
:-(.  But I do have some comcerns though.

> In response to concerns sent to me by Peter Schauer, I introduced a
> new macro, I386_USE_GENERIC_WATCHPOINTS, which targets should define
> to get the new watchpoint code in i386-tdep.c compiled.  Thus, targets
> which don't define that macro, will continue to use their current
> watchpoint support.  This avoids breaking any such targets, in case
> the code I wrote conflicts with what they have now.

That addresses one of my concerns :-).  However, why are you
conditionalizing the code both on I386_USE_GENERIC_WATCHPOINTS and
TARGET_HAS_HARDWARE_WATCHPOINTS?

> In other words, targets which want to use the code I wrote need to say
> "I do" explicitly ;-)

That's good!

I'm still not convinced that this stuff belongs in i386-tdep.c.  While
the code defenitely is an improvement over what's currently in GDB, it
has some limitations.  The fact that you rely on the I386_DR_LOW_*
macro's means that adding these functions to i386-tdep.c is moving us
farther away from multi-arching the i386 stuff.  To those that
suggested putting this code in i386-tdep.c: please take another look
at the support for hardware watchpoints in GDB.  The only real support
for hardware watchpoints is for native targets.

Another limitation is the use of global varaibles to keep reference
counts and debug register mirrors.  This means that the current
implementation can only work for a single target with a single thread
of control.  This is another reason why I'd rather not put this stuff
in i386-tdep.c.

Rather than asking Eli to address these issues, I again suggest
putting them in i386-nat.c.  If Fernando (or somebody else) really
wants this code in i386-tdep.c, he'll have to fix GDB to properly
support hardware watchpoints in a more native-independent fashion first.

Anyway, I also have some comments on the the code itself:

> +#ifdef HAVE_PTRACE_H
> +#include <ptrace.h>
> +#else
> +#ifdef HAVE_SYS_PTRACE_H
> +#include <sys/ptrace.h>
> +#endif
> +#endif
> +
> +#ifdef HAVE_SYS_USER
> +#include <sys/user.h>
> +#endif
> +
> +/* FIXME: The following should be just "#include <sys/debugreg.h>",
> +   but the the Linux 2.1.x kernel and glibc 2.0.x are not in sync;
> +   including <sys/debugreg.h> will result in an error.  With luck,
> +   these losers will get their act together and we can trash this hack
> +   in the near future.
> +
> +   --jsm 1998-10-21; modified by eliz 2001-02-24.  */
> +
> +#ifdef HAVE_ASM_DEBUGREG_H
> +#include <asm/debugreg.h>
> +#else  /* !HAVE_ASM_DEBUGREG_H */
> +
> +#ifdef HAVE_SYS_DEBUGREG_H
> +#include <sys/debugreg.h>
> +#else  /* !HAVE_SYS_DEBUGREG_H */

Please get rid of this crap.  You're completely free to define these
things yourself.  There is absolutely no need to rely on constants
defined in those possibly broken headers.  Simply assume there are
four address registers and the bits are laid out as in the Intel
documentation.

> +/* This is in i386v-nat.c, so let's have it here, just in case.  */
> +#if !defined (offsetof)
> +#define offsetof(TYPE, MEMBER) ((unsigned long) &((TYPE *)0)->MEMBER)
> +#endif

Remove this crap.  You don't use offsetof() in your code.

> +/* Exported API functions.  */
> +
> +/* Clear the reference counts and forget everything we knew about DRi.  */
> +void i386_cleanup_dregs (void);
> +
> +/* Insert a watchpoint to watch a memory region which starts at
> +   address ADDR and whose length is LEN bytes.  Watch memory accesses
> +   of the type TYPE.  Return 0 on success, -1 on failure.  */
> +int  i386_insert_watchpoint (CORE_ADDR addr, int len, int type);
> +
> +/* Remove a watchpoint that watched the memory region which starts at
> +   address ADDR, whose length is LEN bytes, and for accesses of the
> +   type TYPE.  Return 0 on success, -1 on failure.  */
> +int  i386_remove_watchpoint (CORE_ADDR addr, int len, int type);
> +
> +/* Return non-zero if we can watch a memory region that starts at
> +   address ADDR and whose length is LEN bytes.  */
> +int  i386_region_ok_for_watchpoint (CORE_ADDR addr, int len);
> +
> +/* Return non-zero if the inferior has some break/watchpoint that
> +   triggered.  */
> +int  i386_stopped_by_hwbp (void);
> +
> +/* If the inferior has some break/watchpoint that triggered, return
> +   the address associated with that break/watchpoint.  Otherwise,
> +   return zero.  */
> +CORE_ADDR i386_stopped_data_address (void);
> +
> +/* Insert a hardware-assisted breakpoint at address ADDR.  SHADOW is
> +   unused.  Return 0 on success, EBUSY on failure.  */
> +int  i386_insert_hw_breakpoint (CORE_ADDR addr, void *shadow);
> +
> +/* Remove a hardware-assisted breakpoint at address ADDR.  SHADOW is
> +   unused.  Return 0 on success, -1 on failure.  */
> +int  i386_remove_hw_breakpoint (CORE_ADDR addr, void *shadow);
> +

If these are exported they should live in a public header
(i386-nat.h?), and you shouldn't provide them here.

> +/* Insert or remove a (possibly non-aligned) watchpoint, or count the
> +   number of debug registers required to watch a region at address
> +   ADDR whose length is LEN for accesses of type TYPE.  Return 0 on
> +   successful insertion or removal, a positive number when queried
> +   about the number of registers, or -1 on failure.  If WHAT is not
> +   a valid value, returns EINVAL.  */
> +static int
> +i386_handle_nonaligned_watchpoint (i386_wp_op_t what, CORE_ADDR addr, int len,
> +				   enum target_hw_bp_type type)

The use of EINVAL seems odd to me since it is used nowhere.  If WHAT
is not a valid value, this should be flagged as an internal error I
think (using gdb_assert() is probably appropriate here.

> +/* Insert a hardware-assisted breakpoint at address ADDR.  SHADOW is
> +   unused.  Return 0 on success, EBUSY on failure.  */
> +int
> +i386_insert_hw_breakpoint (CORE_ADDR addr, void *shadow)
> +{
> +  unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
> +  int retval = i386_insert_aligned_watchpoint (addr, len_rw) ? EBUSY : 0;
> +
> +  if (maint_show_dr)
> +    i386_show_dr ("insert_hwbp", addr, 1, hw_execute);
> +
> +  return retval;
> +}

What's this crap about EBUSY?

Mark

Re: [RFA] Unified watchpoints for x86 platforms

[Eli Zaretskii]

On 7 Mar 2001, Mark Kettenis wrote:

> Sorry for not responding sooner.  I've been suffering from a bad flu
> :-(.  But I do have some comcerns though.

Thanks for the feedback.

> That addresses one of my concerns :-).  However, why are you
> conditionalizing the code both on I386_USE_GENERIC_WATCHPOINTS and
> TARGET_HAS_HARDWARE_WATCHPOINTS?

TARGET_HAS_HARDWARE_WATCHPOINTS is widely used elsewehere in GDB.  I 
didn't want to replace it with I386_USE_GENERIC_WATCHPOINTS that I 
invented.  On the other hand, I didn't want to make it possible for
some port to define the latter without defining the former, because I 
think the rest of GDB code will become confused, or the port will not 
work as intended.  So I opted for a compile-time detection of such 
problems.

> I'm still not convinced that this stuff belongs in i386-tdep.c.  While
> the code defenitely is an improvement over what's currently in GDB, it
> has some limitations.  The fact that you rely on the I386_DR_LOW_*
> macro's means that adding these functions to i386-tdep.c is moving us
> farther away from multi-arching the i386 stuff.

I admit I don't know enough about multi-arch to be sure I don't say 
something stupid.  However, at least in principle, I386_DR_LOW_* macros 
can be themselves multi-arch'ed, can't they?

> Another limitation is the use of global varaibles to keep reference
> counts and debug register mirrors.  This means that the current
> implementation can only work for a single target with a single thread
> of control.

The non-multi-arch nature of the code was a specific limitation of the 
design I presented.  Someone, perhaps even you, told me that 
multi-arching this stuff would be SEP (Someone Else's Problem).  I don't 
know enough to do that myself, and even if I did, I don't have any 
practical way of testing it.

As for multithreading, the discussion we held back in November concluded 
that only the status register should be thread specific; the other debug 
registers are global.  The code I wrote supports that (or at least I 
think it does; if you see some fragment that doesn't please identify it).

IMHO, since currently only native debugging uses watchpoints, it 
shouldn't matter one way or the other.  In the long run, when the code is 
multi-arched, Someone(tm) will have to figure out how to do that so that
non-native targets would be able to benefit from this code.  If and when 
they do, they will want to move the code back to i386-tdep.c.  So why not 
leave it there in the first place?

As written, the code is definitely good for any x86 target, including 
non-native targets, as long as that target doesn't try to use multiple 
architectures.  I don't think it's right to prevent such targets from 
using the code just because it is not general enough at this time.

> > +#ifdef HAVE_PTRACE_H
> > +#include <ptrace.h>
> > +#else
> > +#ifdef HAVE_SYS_PTRACE_H
> > +#include <sys/ptrace.h>
> > +#endif
> > +#endif
> > +
> > +#ifdef HAVE_SYS_USER
> > +#include <sys/user.h>
> > +#endif
> > +
> > +/* FIXME: The following should be just "#include <sys/debugreg.h>",
> > +   but the the Linux 2.1.x kernel and glibc 2.0.x are not in sync;
> > +   including <sys/debugreg.h> will result in an error.  With luck,
> > +   these losers will get their act together and we can trash this hack
> > +   in the near future.
> > +
> > +   --jsm 1998-10-21; modified by eliz 2001-02-24.  */
> > +
> > +#ifdef HAVE_ASM_DEBUGREG_H
> > +#include <asm/debugreg.h>
> > +#else  /* !HAVE_ASM_DEBUGREG_H */
> > +
> > +#ifdef HAVE_SYS_DEBUGREG_H
> > +#include <sys/debugreg.h>
> > +#else  /* !HAVE_SYS_DEBUGREG_H */
> 
> Please get rid of this crap.  You're completely free to define these
> things yourself.  There is absolutely no need to rely on constants
> defined in those possibly broken headers.  Simply assume there are
> four address registers and the bits are laid out as in the Intel
> documentation.

I will remove debugreg.h if no one objects.  As for ptrace.h, is it wise 
to remove that as well?  I'd imagine that just about every target will 
want to include it anyway.

> > +/* This is in i386v-nat.c, so let's have it here, just in case.  */
> > +#if !defined (offsetof)
> > +#define offsetof(TYPE, MEMBER) ((unsigned long) &((TYPE *)0)->MEMBER)
> > +#endif
> 
> Remove this crap.  You don't use offsetof() in your code.

I left it there because at least one implementation of the I386_DR_LOW_* 
macros will want it.

> > +/* Remove a hardware-assisted breakpoint at address ADDR.  SHADOW is
> > +   unused.  Return 0 on success, -1 on failure.  */
> > +int  i386_remove_hw_breakpoint (CORE_ADDR addr, void *shadow);
> > +
> 
> If these are exported they should live in a public header
> (i386-nat.h?), and you shouldn't provide them here.

Why is it bad to have them here?  If nothing else, they document the code 
in one place.

As for the header, the same prototypes are in config/i386/tm-i386.h.  Is 
that okay?

> > +/* Insert a hardware-assisted breakpoint at address ADDR.  SHADOW is
> > +   unused.  Return 0 on success, EBUSY on failure.  */
> > +int
> > +i386_insert_hw_breakpoint (CORE_ADDR addr, void *shadow)
> > +{
> > +  unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
> > +  int retval = i386_insert_aligned_watchpoint (addr, len_rw) ? EBUSY : 0;
> > +
> > +  if (maint_show_dr)
> > +    i386_show_dr ("insert_hwbp", addr, 1, hw_execute);
> > +
> > +  return retval;
> > +}
> 
> What's this crap about EBUSY?

It was in the go32-nat.c code which served as a prototype.  IIRC, EBUSY 
is used in an error message printed by GDB when a breakpoint cannot be 
inserted.  Perhaps I'm mistaken.

Re: [RFA] Unified watchpoints for x86 platforms

[Mark Kettenis]

   Date: Wed, 7 Mar 2001 19:21:23 +0200 (IST)
   From: Eli Zaretskii <eliz@is.elta.co.il>

   > I'm still not convinced that this stuff belongs in i386-tdep.c.  While
   > the code defenitely is an improvement over what's currently in GDB, it
   > has some limitations.  The fact that you rely on the I386_DR_LOW_*
   > macro's means that adding these functions to i386-tdep.c is moving us
   > farther away from multi-arching the i386 stuff.

   I admit I don't know enough about multi-arch to be sure I don't say 
   something stupid.  However, at least in principle, I386_DR_LOW_* macros 
   can be themselves multi-arch'ed, can't they?

Yep.  But instead of multi-arching all those functions we should
probably use an approach similar to what has been done for shared
library support (`struct target_so_ops').

   > Another limitation is the use of global varaibles to keep reference
   > counts and debug register mirrors.  This means that the current
   > implementation can only work for a single target with a single thread
   > of control.

   The non-multi-arch nature of the code was a specific limitation of the 
   design I presented.  Someone, perhaps even you, told me that 
   multi-arching this stuff would be SEP (Someone Else's Problem).  I don't 
   know enough to do that myself, and even if I did, I don't have any 
   practical way of testing it.

Considering it as SEP.  But as long as it hasn't been address the
could shouldn't be put in i386-tdep.c.

   As for multithreading, the discussion we held back in November concluded 
   that only the status register should be thread specific; the other debug 
   registers are global.  The code I wrote supports that (or at least I 
   think it does; if you see some fragment that doesn't please identify it).

   IMHO, since currently only native debugging uses watchpoints, it 
   shouldn't matter one way or the other.  In the long run, when the code is 
   multi-arched, Someone(tm) will have to figure out how to do that so that
   non-native targets would be able to benefit from this code.  If and when 
   they do, they will want to move the code back to i386-tdep.c.  So why not 
   leave it there in the first place?

Simply because I'd like i386-tdep.c to contain only code that's
multi-arch ready.  GDB already contains too many hacks that never got
cleaned up.  You're code is very useful for native targets, but simply
isn't multi-arch ready.

   As written, the code is definitely good for any x86 target, including 
   non-native targets, as long as that target doesn't try to use multiple 
   architectures.  I don't think it's right to prevent such targets from 
   using the code just because it is not general enough at this time.

If someone really wants to use the code in non-native targets he/she
should address the multi-arch problems.

   I will remove debugreg.h if no one objects.  As for ptrace.h, is it wise 
   to remove that as well?  I'd imagine that just about every target will 
   want to include it anyway.

Yes, but the actual implementation of the I386_DR_LOW_* will live in
an entirely different file.  This stuff is totally unrelevant for the
implementation of the generic code.

   > > +/* This is in i386v-nat.c, so let's have it here, just in case.  */
   > > +#if !defined (offsetof)
   > > +#define offsetof(TYPE, MEMBER) ((unsigned long) &((TYPE *)0)->MEMBER)
   > > +#endif
   > 
   > Remove this crap.  You don't use offsetof() in your code.

   I left it there because at least one implementation of the I386_DR_LOW_* 
   macros will want it.

See my previous comment.

   > > +/* Remove a hardware-assisted breakpoint at address ADDR.  SHADOW is
   > > +   unused.  Return 0 on success, -1 on failure.  */
   > > +int  i386_remove_hw_breakpoint (CORE_ADDR addr, void *shadow);
   > > +
   > 
   > If these are exported they should live in a public header
   > (i386-nat.h?), and you shouldn't provide them here.

   Why is it bad to have them here?  If nothing else, they document the code 
   in one place.

   As for the header, the same prototypes are in config/i386/tm-i386.h.  Is 
   that okay?

It's much easier to maintain one single copy of these declarations
than having them scattered all around the place.

   It was in the go32-nat.c code which served as a prototype.  IIRC, EBUSY 
   is used in an error message printed by GDB when a breakpoint cannot be 
   inserted.  Perhaps I'm mistaken.

I think you are.

Mark

Re: [RFA] Unified watchpoints for x86 platforms

[Eli Zaretskii]

On Fri, 9 Mar 2001, Mark Kettenis wrote:

>    IMHO, since currently only native debugging uses watchpoints, it 
>    shouldn't matter one way or the other.  In the long run, when the code is 
>    multi-arched, Someone(tm) will have to figure out how to do that so that
>    non-native targets would be able to benefit from this code.  If and when 
>    they do, they will want to move the code back to i386-tdep.c.  So why not 
>    leave it there in the first place?
> 
> Simply because I'd like i386-tdep.c to contain only code that's
> multi-arch ready.

Why is this goal so important that it justifies preventing
non-multiarched targets from using watchpoints, and creating a
separate file on top of that?

> If someone really wants to use the code in non-native targets he/she
> should address the multi-arch problems.

IMHO, this means we are being too harsh to target maintainers.

Anyway, since you insist on moving the code to a separate file, I'll
do that.  I just wish I understood the motivation for that better than
I do now.

>    I will remove debugreg.h if no one objects.  As for ptrace.h, is it wise 
>    to remove that as well?  I'd imagine that just about every target will 
>    want to include it anyway.
> 
> Yes, but the actual implementation of the I386_DR_LOW_* will live in
> an entirely different file.

That's one possibility.  What I had in mind was something different;
for example:

  #define I386_DR_LOW_SET_ADDR(dr,addr) \
     ptrace (6, inferior_pid, offsetof (struct user, u_debugreg[dr]),(addr))

  #define I386_DR_LOW_SET_CONTROL(val) \
     ptrace (6, inferior_pid, offsetof (struct user, u_debugreg[DR_CONTROL]),(val))

I thought that when the macros are defined like this, ptrace.h would
be most useful.

While writing the code, I tried to make it very easy for the targets
to start using it.  As written, all they need to do is define a small
number of macros in tm-*.h header and say "./configure; make".

>    It was in the go32-nat.c code which served as a prototype.  IIRC, EBUSY 
>    is used in an error message printed by GDB when a breakpoint cannot be 
>    inserted.  Perhaps I'm mistaken.
> 
> I think you are.

Here's the relevant snippet from breakpoint.c:insert_breakpoints (with 
some of the code removed for brevity):

	if (b->type == bp_hardware_breakpoint)
	  val = target_insert_hw_breakpoint (b->address, b->shadow_contents);
	else
	  {
	   ...
	      val = target_insert_breakpoint (b->address, b->shadow_contents);
	  }
	if (val)
	  {
	    /* Can't set the breakpoint.  */
#if defined (DISABLE_UNSETTABLE_BREAK)
	   ...
#endif
	      {
		target_terminal_ours_for_output ();
		warning ("Cannot insert breakpoint %d:", b->number);
#ifdef ONE_PROCESS_WRITETEXT
		warning ("The same program may be running in another process.");
#endif
		memory_error (val, b->address);	   /* which bombs us out */
	      }
	  }

As you see, it calls memory_error with the value returned by
target_insert_hw_breakpoint.  memory_error then interprets this arg
as a value of errno and prints the text returned by safe_strerror for
it.

So the question is: what do we want GDB to print when it fails to
insert a breakpoint, hardware or otherwise?

Re: [RFA] Unified watchpoints for x86 platforms

[Mark Kettenis]

   Date: Sun, 11 Mar 2001 13:16:19 +0200 (IST)
   From: Eli Zaretskii <eliz@is.elta.co.il>

   On Fri, 9 Mar 2001, Mark Kettenis wrote:

   > Simply because I'd like i386-tdep.c to contain only code that's
   > multi-arch ready.

   Why is this goal so important that it justifies preventing
   non-multiarched targets from using watchpoints, and creating a
   separate file on top of that?

Long term maintainability of the code.

   > If someone really wants to use the code in non-native targets he/she
   > should address the multi-arch problems.

   IMHO, this means we are being too harsh to target maintainers.

   Anyway, since you insist on moving the code to a separate file, I'll
   do that.  I just wish I understood the motivation for that better than
   I do now.

   >    I will remove debugreg.h if no one objects.  As for ptrace.h, is it wise 
   >    to remove that as well?  I'd imagine that just about every target will 
   >    want to include it anyway.
   > 
   > Yes, but the actual implementation of the I386_DR_LOW_* will live in
   > an entirely different file.

   That's one possibility.  What I had in mind was something different;
   for example:

     #define I386_DR_LOW_SET_ADDR(dr,addr) \
	ptrace (6, inferior_pid, offsetof (struct user, u_debugreg[dr]),(addr))

     #define I386_DR_LOW_SET_CONTROL(val) \
	ptrace (6, inferior_pid, offsetof (struct user, u_debugreg[DR_CONTROL]),(val))

We certainly don't want to encourage that kind of macro's.  Again this
style of coding has caused many problems in the past.  We've been
converting these kind of macro's into proper functions all over the
place over the last couple of years.

   I thought that when the macros are defined like this, ptrace.h would
   be most useful.

   While writing the code, I tried to make it very easy for the targets
   to start using it.  As written, all they need to do is define a small
   number of macros in tm-*.h header and say "./configure; make".

I don't think that demanding people to provide proper functions is
making the implementation that much harder.

   >    It was in the go32-nat.c code which served as a prototype.  IIRC, EBUSY 
   >    is used in an error message printed by GDB when a breakpoint cannot be 
   >    inserted.  Perhaps I'm mistaken.
   > 
   > I think you are.

   Here's the relevant snippet from breakpoint.c:insert_breakpoints (with 
   some of the code removed for brevity):

	   if (b->type == bp_hardware_breakpoint)
	     val = target_insert_hw_breakpoint (b->address, b->shadow_contents);
	   else
	     {
	      ...
		 val = target_insert_breakpoint (b->address, b->shadow_contents);
	     }
	   if (val)
	     {
	       /* Can't set the breakpoint.  */
   #if defined (DISABLE_UNSETTABLE_BREAK)
	      ...
   #endif
		 {
		   target_terminal_ours_for_output ();
		   warning ("Cannot insert breakpoint %d:", b->number);
   #ifdef ONE_PROCESS_WRITETEXT
		   warning ("The same program may be running in another process.");
   #endif
		   memory_error (val, b->address);	   /* which bombs us out */
		 }
	     }

   As you see, it calls memory_error with the value returned by
   target_insert_hw_breakpoint.  memory_error then interprets this arg
   as a value of errno and prints the text returned by safe_strerror for
   it.

Ah, I didn't really look at the hardware breakpoint stuff, only at the
watchpoints.  memory_error isn't really appropriate here of course.

   So the question is: what do we want GDB to print when it fails to
   insert a breakpoint, hardware or otherwise?

I'm not sure.  We should really fix the stuff in breakpoint.c to not
assume that all breakpoints are implemented by doing some sort of
memory access.  Meanwhile, returning EBUSY for hardware watchpoints is
probably fine.  But I think that the case where you propose to return
EINVAL is really a GDB internal error.

Mark

Re: [RFA] Unified watchpoints for x86 platforms

[Eli Zaretskii]

Okay, I think the new patch below addresses all the issues raised in
this thread.  It supercedes the previous patches I posted.

I see that awatch causes GDB to call internal_error, but this was so
in the version before applying this patch, so I don't think it's a
regression caused by these changes.  In any case, I will debug these
problems.

I'm sending the patch for the docs in a separate message.

2001-03-17  Eli Zaretskii  <eliz@is.elta.co.il>

	Unified support for hardware breakpoints and watchpoints on
	x86 targets:

	* config/i386/nm-i386v.h (i386_cleanup_dregs)
	(i386_insert_watchpoint, i386_remove_watchpoint)
	(i386_region_ok_for_watchpoint, i386_stopped_by_hwbp)
	(i386_stopped_data_address, i386_insert_hw_breakpoint)
	(i386_remove_hw_breakpoint): Declare prototypes.
	[I386_USE_GENERIC_WATCHPOINTS] (TARGET_CAN_USE_HARDWARE_WATCHPOINT):
	Define if not already defined.
	(TARGET_REGION_OK_FOR_HW_WATCHPOINT, HAVE_CONTINUABLE_WATCHPOINT)
	(STOPPED_BY_WATCHPOINT, target_stopped_data_address)
	(target_insert_watchpoint, target_remove_watchpoint)
	(target_insert_hw_breakpoint, target_remove_hw_breakpoint): Define
	to call the appropriate i386_* functions.

	* i386-nat.c: New file.

	* i386-nat.c (I386_DR_CONTROL_MASK, I386_DR_LOCAL_ENABLE)
	(I386_DR_GLOBAL_ENABLE, I386_DR_DISABLE, I386_DR_SET_RW_LEN)
	(I386_DR_GET_RW_LEN, I386_DR_WATCH_HIT): New macros.
	(dr_mirror, dr_status_mirror, dr_control_mirror, dr_ref_count)
	(maint_show_dr): New variables.
	(i386_cleanup_dregs, i386_show_dr, i386_length_and_rw_bits)
	(i386_insert_aligned_watchpoint, i386_remove_aligned_watchpoint)
	(i386_handle_nonaligned_watchpoint, i386_insert_watchpoint)
	(i386_remove_watchpoint, i386_region_ok_for_watchpoint)
	(i386_stopped_data_address, i386_stopped_by_hwbp)
	(i386_insert_hw_breakpoint, i386_remove_hw_breakpoint): New
	functions.
	(_initialize_i386_nat): New function.
	[I386_USE_GENERIC_WATCHPOINTS]: Add new	maint command
	`show-debug-regs', sets maint_show_dr to non-zero value and
	activates debugging print-outs in functions which insert, remove,
	and test watchpoints and hardware breakpoints.

	* Makefile.in (i386-nat.o): New target.
	(ALLDEPFILES): Add i386-nat.o.


--- gdb/config/i386/nm-i386v.h~1	Sun May 28 04:12:34 2000
+++ gdb/config/i386/nm-i386v.h	Sat Mar 17 13:27:44 2001
@@ -1,5 +1,5 @@
 /* Native support for i386.
-   Copyright 1986, 1987, 1989, 1992 Free Software Foundation, Inc.
+   Copyright 1986, 1987, 1989, 1992, 2001 Free Software Foundation, Inc.
    Changes for 80386 by Pace Willisson (pace@prep.ai.mit.edu), July 1988.
 
    This file is part of GDB.
@@ -34,3 +34,100 @@
	(addr) = i386_register_u_addr ((blockend),(regno));
 
 extern int i386_register_u_addr (int, int);
+
+/* Hardware-assisted breakpoints and watchpoints.  */
+
+/* Targets should define this to use the generic x86 watchpoint support.  */
+#ifdef I386_USE_GENERIC_WATCHPOINTS
+
+
+#ifndef TARGET_HAS_HARDWARE_WATCHPOINTS
+#define TARGET_HAS_HARDWARE_WATCHPOINTS
+#endif
+
+/* Clear the reference counts and forget everything we knew about DRi.  */
+extern void i386_cleanup_dregs (void);
+
+/* Insert a watchpoint to watch a memory region which starts at
+   address ADDR and whose length is LEN bytes.  Watch memory accesses
+   of the type TYPE.  Return 0 on success, -1 on failure.  */
+extern int  i386_insert_watchpoint (CORE_ADDR addr, int len, int type);
+
+/* Remove a watchpoint that watched the memory region which starts at
+   address ADDR, whose length is LEN bytes, and for accesses of the
+   type TYPE.  Return 0 on success, -1 on failure.  */
+extern int  i386_remove_watchpoint (CORE_ADDR addr, int len, int type);
+
+/* Return non-zero if we can watch a memory region that starts at
+   address ADDR and whose length is LEN bytes.  */
+extern int  i386_region_ok_for_watchpoint (CORE_ADDR addr, int len);
+
+/* Return non-zero if the inferior has some break/watchpoint that
+   triggered.  */
+extern int  i386_stopped_by_hwbp (void);
+
+/* If the inferior has some break/watchpoint that triggered, return
+   the address associated with that break/watchpoint.  Otherwise,
+   return zero.  */
+extern CORE_ADDR i386_stopped_data_address (void);
+
+/* Insert a hardware-assisted breakpoint at address ADDR.  SHADOW is
+   unused.  Return 0 on success, EBUSY on failure.  */
+extern int  i386_insert_hw_breakpoint (CORE_ADDR addr, void *shadow);
+
+/* Remove a hardware-assisted breakpoint at address ADDR.  SHADOW is
+   unused.  Return 0 on success, -1 on failure.  */
+extern int  i386_remove_hw_breakpoint (CORE_ADDR addr, void *shadow);
+
+/* Returns the number of hardware watchpoints of type TYPE that we can
+   set.  Value is positive if we can set CNT watchpoints, zero if
+   setting watchpoints of type TYPE is not supported, and negative if
+   CNT is more than the maximum number of watchpoints of type TYPE
+   that we can support.  TYPE is one of bp_hardware_watchpoint,
+   bp_read_watchpoint, bp_write_watchpoint, or bp_hardware_breakpoint.
+   CNT is the number of such watchpoints used so far (including this
+   one).  OTHERTYPE is non-zero if other types of watchpoints are
+   currently enabled.
+
+   We always return 1 here because we don't have enough information
+   about possible overlap of addresses that they want to watch.  As
+   an extreme example, consider the case where all the watchpoints
+   watch the same address and the same region length: then we can
+   handle a virtually unlimited number of watchpoints, due to debug
+   register sharing implemented via reference counts in i386-tdep.c.  */
+
+#define TARGET_CAN_USE_HARDWARE_WATCHPOINT(type, cnt, ot) 1
+
+/* Returns non-zero if we can use hardware watchpoints to watch a region
+   whose address is ADDR and whose length is LEN.  */
+
+#define TARGET_REGION_OK_FOR_HW_WATCHPOINT(addr,len) \
+	i386_region_ok_for_watchpoint(addr,len)
+
+/* 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
+
+#define STOPPED_BY_WATCHPOINT(W)       (i386_stopped_data_address () != 0)
+
+#define target_stopped_data_address()  i386_stopped_data_address ()
+
+/* Use these macros for watchpoint insertion/removal.  */
+
+#define target_insert_watchpoint(addr, len, type)  \
+  i386_insert_watchpoint (addr, len, type)
+
+#define target_remove_watchpoint(addr, len, type)  \
+  i386_remove_watchpoint (addr, len, type)
+
+#define target_insert_hw_breakpoint(addr, shadow)  \
+  i386_insert_hw_breakpoint(addr, shadow)
+
+#define target_remove_hw_breakpoint(addr, shadow)  \
+  i386_remove_hw_breakpoint(addr, shadow)
+
+#define DECR_PC_AFTER_HW_BREAK 0
+
+#endif /* I386_USE_GENERIC_WATCHPOINTS */


--- /dev/null	Sat Mar 17 13:31:57 2001
+++ gdb/i386-nat.c	Sat Mar 17 12:10:54 2001
@@ -0,0 +1,618 @@
+/* Intel x86 (a.k.a. ia32) native-dependent code.
+   Copyright (C) 2001 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330,
+   Boston, MA 02111-1307, USA.  */
+
+#include "defs.h"
+#include "breakpoint.h"
+#include "command.h"
+#include "gdbcmd.h"
+
+/* Support for hardware watchpoints and breakpoints using the x86
+   debug registers.
+
+   This provides several functions for inserting and removing
+   hardware-assisted breakpoints and watchpoints, testing if
+   one or more of the watchpoints triggerd and at what address,
+   checking whether a given region can be watched, etc.
+
+   A target which wants to use these functions should define
+   several macros, such as `target_insert_watchpoint' and
+   `target_stopped_data_address', listed in target.h, to call
+   the appropriate functions below.  It should also define
+   I386_USE_GENERIC_WATCHPOINTS in its tm.h file.
+
+   In addition, each target should provide several low-level
+   macros that will be called to insert watchpoints and hardware
+   breakpoints into the inferior, remove them, and check their
+   status.  These macros are:
+
+      I386_DR_LOW_SET_CONTROL  -- set the debug control (DR7)
+				  register to a given value
+
+      I386_DR_LOW_SET_ADDR     -- put an address into one debug
+				  register
+
+      I386_DR_LOW_RESET_ADDR   -- reset the address stored in
+				  one debug register
+
+      I386_DR_LOW_GET_STATUS   -- return the value of the debug
+				  status (DR6) register.
+
+   The functions below implement debug registers sharing by
+   reference counts, and allow to watch regions up to 16 bytes
+   long.  */
+
+#ifdef I386_USE_GENERIC_WATCHPOINTS
+
+/* Debug registers' indices.  */
+#define DR_NADDR		4  /* the number of debug address registers */
+#define DR_STATUS		6  /* index of debug status register (DR6) */
+#define DR_CONTROL		7  /* index of debug control register (DR7) */
+
+/* DR7 Debug Control register fields.  */
+
+/* How many bits to skip in DR7 to get to R/W and LEN fields.  */
+#define DR_CONTROL_SHIFT	16
+/* How many bits in DR7 per R/W and LEN field for each watchpoint.  */
+#define DR_CONTROL_SIZE		4
+
+/* Watchpoint/breakpoint read/write fields in DR7.  */
+#define DR_RW_EXECUTE		(0x0) /* break on instruction execution */
+#define DR_RW_WRITE		(0x1) /* break on data writes */
+#define DR_RW_READ		(0x3) /* break on data reads or writes */
+
+/* This is here for completeness.  No platform supports this
+   functionality yet (as of Mar-2001).  Note that the DE flag in the
+   CR4 register needs to be set to support this.  */
+#ifndef DR_RW_IORW
+#define DR_RW_IORW		(0x2) /* break on I/O reads or writes */
+#endif
+
+/* Watchpoint/breakpoint length fields in DR7.  The 2-bit left shift
+   is so we could OR this with the read/write field defined above.  */
+#define DR_LEN_1		(0x0 << 2) /* 1-byte region watch or breakpt */
+#define DR_LEN_2		(0x1 << 2) /* 2-byte region watch */
+#define DR_LEN_4		(0x3 << 2) /* 4-byte region watch */
+
+/* Local and Global Enable flags in DR7.
+
+   When the Local Enable flag is set, the breakpoint/watchpoint is
+   enabled only for the current task; the processor automatically
+   clears this flag on every task switch.  When the Global Enable
+   flag is set, the breakpoint/watchpoint is enabled for all tasks;
+   the processor never clears this flag.
+
+   Currently, all watchpoint are locally enabled.  If you need to
+   enable them globally, read the comment which pertains to this in
+   i386_insert_aligned_watchpoint below.  */
+#define DR_LOCAL_ENABLE_SHIFT	0   /* extra shift to the local enable bit */
+#define DR_GLOBAL_ENABLE_SHIFT	1   /* extra shift to the global enable bit */
+#define DR_ENABLE_SIZE		2   /* 2 enable bits per debug register */
+
+/* Local and global exact breakpoint enable flags (a.k.a. slowdown
+   flags).  These are only required on i386, to allow detection of the
+   exact instruction which caused a watchpoint to break; i486 and
+   later processors do that automatically.  We set these flags for
+   back compatibility.  */
+#define DR_LOCAL_SLOWDOWN	(0x100)
+#define DR_GLOBAL_SLOWDOWN	(0x200)
+
+/* Fields reserved by Intel.  This includes the GD (General Detect
+   Enable) flag, which causes a debug exception to be generated when a
+   MOV instruction accesses one of the debug registers.
+
+   FIXME: My Intel manual says we should use 0xF800, not 0xFC00.  */
+#define DR_CONTROL_RESERVED	(0xFC00)
+
+/* Auxiliary helper macros.  */
+
+/* A value that masks all fields in DR7 that are reserved by Intel.  */
+#define I386_DR_CONTROL_MASK		(~DR_CONTROL_RESERVED)
+
+/* The I'th debug register is vacant if its Local and Global Enable
+   bits are reset in the Debug Control register.  */
+#define I386_DR_VACANT(i) \
+  ((dr_control_mirror & (3 << (DR_ENABLE_SIZE * (i)))) == 0)
+
+/* Locally enable the break/watchpoint in the I'th debug register.  */
+#define I386_DR_LOCAL_ENABLE(i) \
+  dr_control_mirror |= (1 << (DR_LOCAL_ENABLE_SHIFT + DR_ENABLE_SIZE * (i)))
+
+/* Globally enable the break/watchpoint in the I'th debug register.  */
+#define I386_DR_GLOBAL_ENABLE(i) \
+  dr_control_mirror |= (1 << (DR_GLOBAL_ENABLE_SHIFT + DR_ENABLE_SIZE * (i)))
+
+/* Disable the break/watchpoint in the I'th debug register.  */
+#define I386_DR_DISABLE(i) \
+  dr_control_mirror &= ~(3 << (DR_ENABLE_SIZE * (i)))
+
+/* Set in DR7 the RW and LEN fields for the I'th debug register.  */
+#define I386_DR_SET_RW_LEN(i,rwlen) \
+  do { \
+    dr_control_mirror &= ~(0x0f << (DR_CONTROL_SHIFT+DR_CONTROL_SIZE*(i)));   \
+    dr_control_mirror |= ((rwlen) << (DR_CONTROL_SHIFT+DR_CONTROL_SIZE*(i))); \
+  } while (0)
+
+/* Get from DR7 the RW and LEN fields for the I'th debug register.  */
+#define I386_DR_GET_RW_LEN(i) \
+  ((dr_control_mirror >> (DR_CONTROL_SHIFT + DR_CONTROL_SIZE * (i))) & 0x0f)
+
+/* Did the watchpoint whose address is in the I'th register break?  */
+#define I386_DR_WATCH_HIT(i)	(dr_status_mirror & (1 << (i)))
+
+/* A macro to loop over all debug registers.  */
+#define ALL_DEBUG_REGISTERS(i)	for (i = 0; i < DR_NADDR; i++)
+
+/* Mirror the inferior's DRi registers.  We keep the status and
+   control registers separated because they don't hold addresses.  */
+static CORE_ADDR dr_mirror[DR_NADDR];
+static unsigned	 dr_status_mirror, dr_control_mirror;
+
+/* Reference counts for each debug register.  */
+static int	 dr_ref_count[DR_NADDR];
+
+/* Whether or not to print the mirrored debug registers.  */
+static int	 maint_show_dr;
+
+/* Types of operations supported by i386_handle_nonaligned_watchpoint.  */
+typedef enum { WP_INSERT, WP_REMOVE, WP_COUNT } i386_wp_op_t;
+
+/* Internal functions.  */
+
+/* Return the value of a 4-bit field for DR7 suitable for watching a
+   region of LEN bytes for accesses of type TYPE.  LEN is assumed
+   to have the value of 1, 2, or 4.  */
+static unsigned i386_length_and_rw_bits (int len, enum target_hw_bp_type type);
+
+/* Insert a watchpoint at address ADDR, which is assumed to be aligned
+   according to the length of the region to watch.  LEN_RW_BITS is the
+   value of the bit-field from DR7 which describes the length and
+   access type of the region to be watched by this watchpoint.  Return
+   0 on success, -1 on failure.  */
+static int i386_insert_aligned_watchpoint (CORE_ADDR addr,
+					   unsigned len_rw_bits);
+
+/* Remove a watchpoint at address ADDR, which is assumed to be aligned
+   according to the length of the region to watch.  LEN_RW_BITS is the
+   value of the bits from DR7 which describes the length and access
+   type of the region watched by this watchpoint.  Return 0 on
+   success, -1 on failure.  */
+static int i386_remove_aligned_watchpoint (CORE_ADDR addr,
+					   unsigned len_rw_bits);
+
+/* Insert or remove a (possibly non-aligned) watchpoint, or count the
+   number of debug registers required to watch a region at address
+   ADDR whose length is LEN for accesses of type TYPE.  Return 0 on
+   successful insertion or removal, a positive number when queried
+   about the number of registers, or -1 on failure.  If WHAT is not
+   a valid value, bombs through internal_error.  */
+static int i386_handle_nonaligned_watchpoint (i386_wp_op_t what,
+					      CORE_ADDR addr, int len,
+					      enum target_hw_bp_type type);
+
+/* Implementation.  */
+
+/* Clear the reference counts and forget everything we knew about
+   the debug registers.  */
+void
+i386_cleanup_dregs (void)
+{
+  int i;
+
+  ALL_DEBUG_REGISTERS(i)
+    {
+      dr_mirror[i] = 0;
+      dr_ref_count[i] = 0;
+    }
+  dr_control_mirror = 0;
+  dr_status_mirror  = 0;
+}
+
+/* Print the values of the mirrored debug registers.
+   This is called when maint_show_dr is non-zero.  To set that
+   up, type "maint show-debug-regs" at GDB's prompt.  */
+static void
+i386_show_dr (const char *func, CORE_ADDR addr,
+	      int len, enum target_hw_bp_type type)
+{
+  int i;
+
+  puts_unfiltered (func);
+  if (addr || len)
+    printf_unfiltered (" (addr=%lx, len=%d, type=%s)",
+		       /* This code is for ia32, so casting CORE_ADDR
+			  to unsigned long should be okay.  */
+		       (unsigned long)addr, len,
+		       type == hw_write ? "data-write"
+		       : (type == hw_read ? "data-read"
+			  : (type == hw_access ? "data-read/write"
+			     : (type == hw_execute ? "instruction-execute"
+				/* FIXME: if/when I/O read/write
+				   watchpoints are supported, add them
+				   here.  */
+				: "??unknown??"))));
+  puts_unfiltered (":\n");
+  printf_unfiltered ("\tCONTROL (DR7): %08x          STATUS (DR6): %08x\n",
+		     dr_control_mirror, dr_status_mirror);
+  ALL_DEBUG_REGISTERS(i)
+    {
+      printf_unfiltered ("\tDR%d: addr=%08lx, ref.count=%d  DR%d: addr=%08lx, ref.count=%d\n",
+			 i, dr_mirror[i], dr_ref_count[i],
+			 i+1, dr_mirror[i+1], dr_ref_count[i+1]);
+      i++;
+    }
+}
+
+/* Return the value of a 4-bit field for DR7 suitable for watching a
+   region of LEN bytes for accesses of type TYPE.  LEN is assumed
+   to have the value of 1, 2, or 4.  */
+static unsigned
+i386_length_and_rw_bits (int len, enum target_hw_bp_type type)
+{
+  unsigned rw;
+
+  switch (type)
+    {
+      case hw_execute:
+	rw = DR_RW_EXECUTE;
+	break;
+      case hw_write:
+	rw = DR_RW_WRITE;
+	break;
+      case hw_read:	 /* x86 doesn't support data-read watchpoints */
+      case hw_access:
+	rw = DR_RW_READ;
+	break;
+#if 0
+      case hw_io_access: /* not yet supported */
+	rw = DR_RW_IORW;
+	break;
+#endif
+      default:
+	internal_error (__FILE__, __LINE__, "\
+Invalid hw breakpoint type %d in i386_length_and_rw_bits.\n", (int)type);
+    }
+
+  switch (len)
+    {
+      case 4:
+	return (DR_LEN_4 | rw);
+      case 2:
+	return (DR_LEN_2 | rw);
+      case 1:
+	return (DR_LEN_1 | rw);
+      default:
+	internal_error (__FILE__, __LINE__, "\
+Invalid hw breakpoint length %d in i386_length_and_rw_bits.\n", len);
+    }
+}
+
+/* Insert a watchpoint at address ADDR, which is assumed to be aligned
+   according to the length of the region to watch.  LEN_RW_BITS is the
+   value of the bits from DR7 which describes the length and access
+   type of the region to be watched by this watchpoint.  Return 0 on
+   success, -1 on failure.  */
+static int
+i386_insert_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits)
+{
+  int i;
+
+  /* First, look for an occupied debug register with the same address
+     and the same RW and LEN definitions.  If we find one, we can
+     reuse it for this watchpoint as well (and save a register).  */
+  ALL_DEBUG_REGISTERS(i)
+    {
+      if (!I386_DR_VACANT (i)
+	  && dr_mirror[i] == addr
+	  && I386_DR_GET_RW_LEN (i) == len_rw_bits)
+	{
+	  dr_ref_count[i]++;
+	  return 0;
+	}
+    }
+
+  /* Next, look for a vacant debug register.  */
+  ALL_DEBUG_REGISTERS(i)
+    {
+      if (I386_DR_VACANT (i))
+	break;
+    }
+
+  /* No more debug registers!  */
+  if (i >= DR_NADDR)
+    return -1;
+
+  /* Now set up the register I to watch our region.  */
+
+  /* Record the info in our local mirrored array.  */
+  dr_mirror[i] = addr;
+  dr_ref_count[i] = 1;
+  I386_DR_SET_RW_LEN (i, len_rw_bits);
+  /* Note: we only enable the watchpoint locally, i.e. in the current
+     task.  Currently, no x86 target allows or supports global
+     watchpoints; however, if any target would want that in the
+     future, GDB should probably provide a command to control whether
+     to enable watchpoints globally or locally, and the code below
+     should use global or local enable and slow-down flags as
+     appropriate.  */
+  I386_DR_LOCAL_ENABLE (i);
+  dr_control_mirror |= DR_LOCAL_SLOWDOWN;
+  dr_control_mirror &= I386_DR_CONTROL_MASK;
+
+  /* Finally, actually pass the info to the inferior.  */
+  I386_DR_LOW_SET_CONTROL (dr_control_mirror);
+  I386_DR_LOW_SET_ADDR (i, addr);
+
+  return 0;
+}
+
+/* Remove a watchpoint at address ADDR, which is assumed to be aligned
+   according to the length of the region to watch.  LEN_RW_BITS is the
+   value of the bits from DR7 which describes the length and access
+   type of the region watched by this watchpoint.  Return 0 on
+   success, -1 on failure.  */
+static int
+i386_remove_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits)
+{
+  int i, retval = -1;
+
+  ALL_DEBUG_REGISTERS(i)
+    {
+      if (!I386_DR_VACANT (i)
+	  && dr_mirror[i] == addr
+	  && I386_DR_GET_RW_LEN (i) == len_rw_bits)
+	{
+	  if (--dr_ref_count[i] == 0) /* no longer in use? */
+	    {
+	      /* Reset our mirror.  */
+	      dr_mirror[i] = 0;
+	      I386_DR_DISABLE (i);
+	      /* Reset it in the inferior.  */
+	      I386_DR_LOW_RESET_ADDR (i);
+	      I386_DR_LOW_SET_CONTROL (dr_control_mirror);
+	    }
+	  retval = 0;
+	}
+    }
+
+  return retval;
+}
+
+/* Insert or remove a (possibly non-aligned) watchpoint, or count the
+   number of debug registers required to watch a region at address
+   ADDR whose length is LEN for accesses of type TYPE.  Return 0 on
+   successful insertion or removal, a positive number when queried
+   about the number of registers, or -1 on failure.  If WHAT is not
+   a valid value, bombs through internal_error.  */
+static int
+i386_handle_nonaligned_watchpoint (i386_wp_op_t what, CORE_ADDR addr, int len,
+				   enum target_hw_bp_type type)
+{
+  int align;
+  int size;
+  int rv = 0, status = 0;
+
+  static int size_try_array[4][4] =
+  {
+    { 1, 1, 1, 1 },		/* trying size one */
+    { 2, 1, 2, 1 },		/* trying size two */
+    { 2, 1, 2, 1 },		/* trying size three */
+    { 4, 1, 2, 1 }		/* trying size four */
+  };
+
+  while (len > 0)
+    {
+      align = addr % 4;
+      /* Four is the maximum length an x86 debug register can watch.  */
+      size = size_try_array[len > 4 ? 3 : len - 1][align];
+      if (what == WP_COUNT)
+	/* size_try_array[] is defined so that each iteration through
+	   the loop is guaranteed to produce an address and a size
+	   that can be watched with a single debug register.  Thus,
+	   for counting the registers required to watch a region, we
+	   simply need to increment the count on each iteration.  */
+	rv++;
+      else
+	{
+	  unsigned len_rw = i386_length_and_rw_bits (size, type);
+
+	  if (what == WP_INSERT)
+	    status = i386_insert_aligned_watchpoint (addr, len_rw);
+	  else if (what == WP_REMOVE)
+	    status = i386_remove_aligned_watchpoint (addr, len_rw);
+	  else
+	    internal_error (__FILE__, __LINE__, "\
+Invalid value %d of operation in i386_handle_nonaligned_watchpoint.\n",
+			    (int)what);
+	  /* We keep the loop going even after a failure, because some
+	     of the other aligned watchpoints might still succeed
+	     (e.g. if they watch addresses that are already watched,
+	     in which case we just increment the reference counts of
+	     occupied debug registers).  If we break out of the loop
+	     too early, we could cause those addresses watched by
+	     other watchpoints to be disabled when breakpoint.c reacts
+	     to our failure to insert this watchpoint and tries to
+	     remove it.  */
+	  if (status)
+	    rv = status;
+	}
+      addr += size;
+      len -= size;
+    }
+  return rv;
+}
+
+/* Insert a watchpoint to watch a memory region which starts at
+   address ADDR and whose length is LEN bytes.  Watch memory accesses
+   of the type TYPE.  Return 0 on success, -1 on failure.  */
+int
+i386_insert_watchpoint (CORE_ADDR addr, int len, int type)
+{
+  int retval;
+
+  if (len == 3 || len > 4 || addr % len != 0)
+    retval = i386_handle_nonaligned_watchpoint (WP_INSERT, addr, len, type);
+  else
+    {
+      unsigned len_rw = i386_length_and_rw_bits (len, type);
+
+      retval = i386_insert_aligned_watchpoint (addr, len_rw);
+    }
+
+  if (maint_show_dr)
+    i386_show_dr ("insert_watchpoint", addr, len, type);
+
+  return retval;
+}
+
+/* Remove a watchpoint that watched the memory region which starts at
+   address ADDR, whose length is LEN bytes, and for accesses of the
+   type TYPE.  Return 0 on success, -1 on failure.  */
+int
+i386_remove_watchpoint (CORE_ADDR addr, int len, int type)
+{
+  int retval;
+
+  if (len == 3 || len > 4 || addr % len != 0)
+    retval = i386_handle_nonaligned_watchpoint (WP_REMOVE, addr, len, type);
+  else
+    {
+      unsigned len_rw = i386_length_and_rw_bits (len, type);
+
+      retval = i386_remove_aligned_watchpoint (addr, len_rw);
+    }
+
+  if (maint_show_dr)
+    i386_show_dr ("remove_watchpoint", addr, len, type);
+
+  return retval;
+}
+
+/* Return non-zero if we can watch a memory region that starts at
+   address ADDR and whose length is LEN bytes.  */
+int
+i386_region_ok_for_watchpoint (CORE_ADDR addr, int len)
+{
+  /* Compute how many aligned watchpoints we would need to cover this
+     region.  */
+  int nregs = i386_handle_nonaligned_watchpoint (WP_COUNT, addr, len,
+						 hw_write);
+
+  return nregs <= DR_NADDR ? 1 : 0;
+}
+
+/* If the inferior has some watchpoint that triggered, return the
+   address associated with that watchpoint.  Otherwise, return
+   zero.  */
+CORE_ADDR
+i386_stopped_data_address (void)
+{
+  int i;
+  CORE_ADDR ret = 0;
+
+  dr_status_mirror = I386_DR_LOW_GET_STATUS ();
+
+  ALL_DEBUG_REGISTERS(i)
+    {
+      if (I386_DR_WATCH_HIT (i)
+	  /* This second condition makes sure DRi is set up for a data
+	     watchpoint, not a hardware breakpoint.  The reason is
+	     that GDB doesn't call the target_stopped_data_address
+	     method except for data watchpoints.  In other words, I'm
+	     being paranoiac.  */
+	  && I386_DR_GET_RW_LEN (i) != 0)
+	{
+	  ret = dr_mirror[i];
+	  if (maint_show_dr)
+	    i386_show_dr ("watchpoint_hit", ret, -1, hw_write);
+	}
+    }
+  if (maint_show_dr && ret == 0)
+    i386_show_dr ("stopped_data_addr", 0, 0, hw_write);
+
+  return ret;
+}
+
+/* Return non-zero if the inferior has some break/watchpoint that
+   triggered.  */
+int
+i386_stopped_by_hwbp (void)
+{
+  int i;
+
+  dr_status_mirror = I386_DR_LOW_GET_STATUS ();
+  if (maint_show_dr)
+    i386_show_dr ("stopped_by_hwbp", 0, 0, hw_execute);
+
+  ALL_DEBUG_REGISTERS(i)
+    {
+      if (I386_DR_WATCH_HIT (i))
+	return 1;
+    }
+
+  return 0;
+}
+
+/* Insert a hardware-assisted breakpoint at address ADDR.  SHADOW is
+   unused.  Return 0 on success, EBUSY on failure.  */
+int
+i386_insert_hw_breakpoint (CORE_ADDR addr, void *shadow)
+{
+  unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
+  int retval = i386_insert_aligned_watchpoint (addr, len_rw) ? EBUSY : 0;
+
+  if (maint_show_dr)
+    i386_show_dr ("insert_hwbp", addr, 1, hw_execute);
+
+  return retval;
+}
+
+/* Remove a hardware-assisted breakpoint at address ADDR.  SHADOW is
+   unused.  Return 0 on success, -1 on failure.  */
+int
+i386_remove_hw_breakpoint (CORE_ADDR addr, void *shadow)
+{
+  unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
+  int retval = i386_remove_aligned_watchpoint (addr, len_rw);
+
+  if (maint_show_dr)
+    i386_show_dr ("remove_hwbp", addr, 1, hw_execute);
+
+  return retval;
+}
+
+#endif /* I386_USE_GENERIC_WATCHPOINTS */
+
+
+void
+_initialize_i386_nat (void)
+{
+#ifdef I386_USE_GENERIC_WATCHPOINTS
+  /* A maintenance command to enable printing the internal DRi mirror
+     variables.  */
+  add_set_cmd ("show-debug-regs", class_maintenance,
+	       var_boolean, (char *) &maint_show_dr,
+	       "\
+Set whether to show variables that mirror the x86 debug registers.\n\
+Use \"on\" to enable, \"off\" to disable.\n\
+If enabled, the debug registers values are shown when GDB inserts\n\
+or removes a hardware breakpoint or watchpoint, and when the inferior\n\
+triggers a breakpoint or watchpoint.", &maintenancelist);
+#endif
+}


--- gdb/Makefile.i~1	Wed Feb  7 22:40:08 2001
+++ gdb/Makefile.in	Wed Mar 14 19:00:32 2001
@@ -1141,7 +1141,7 @@
	i386-tdep.c i386b-nat.c i386mach-nat.c i386v-nat.c i386-linux-nat.c \
	i386aix-nat.c i386m3-nat.c i386v4-nat.c i386ly-tdep.c \
	i387-tdep.c \
-	i386-linux-tdep.c \
+	i386-linux-tdep.c i386-nat.c \
	i960-tdep.c \
	ia64-linux-nat.c ia64-linux-tdep.c ia64-tdep.c \
	infptrace.c inftarg.c irix4-nat.c irix5-nat.c isi-xdep.c \
@@ -1477,6 +1477,8 @@
	$(inferior_h) $(gdbcore_h) target.h $(floatformat_h)	\
	$(symtab_h) $(gdbcmd_h) $(command_h) $(arch_utils_h)
 
+i386-nat.o: i386-nat.c $(defs_h) $(breakpoint_h) $(command_h) $(gdbcmd_h)
+
 i386aix-nat.o: i386aix-nat.c $(defs_h) $(frame_h) $(inferior_h) \
	language.h $(gdbcore_h) $(floatformat_h) target.h
 

Re: [RFA] Unified watchpoints for x86 platforms

[Mark Kettenis]

Oh, by the way, I don't think it's necessary to list changes for newly
added files to the ChangeLog.  Just something like:

   * i386-nat.c: New file.

should be enough.

Mark

Re: [RFA] Unified watchpoints for x86 platforms

[Eli Zaretskii]

On Sat, 17 Mar 2001, Mark Kettenis wrote:

> Oh, by the way, I don't think it's necessary to list changes for newly
> added files to the ChangeLog.  Just something like:
> 
>    * i386-nat.c: New file.
> 
> should be enough.

I know.  But does it hurt to have the new functions listed as well?

I was always annoyed by the fact that the first functions on each file 
need to be deduced by elimination, instead of just reading the logs.  
For example, imagine that you want to know when was a certain function 
introduced.

Re: [RFA] Unified watchpoints for x86 platforms

[Mark Kettenis]

   Date: Sat, 17 Mar 2001 12:18:02 -0500 (EST)
   From: Eli Zaretskii <eliz@delorie.com>

   Okay, I think the new patch below addresses all the issues raised in
   this thread.  It supercedes the previous patches I posted.

Thanks for addressing my objections.  However...

... config/i386/nm-i386v.h is really for System V-ish systems only,
and not a generic i386 file (the comment on the first line of that
file is a misleading).  So the stuff you're adding there doesn't
really belong there.  Therefore I think it's best to create a new
config/i386/nm-i386.h file and put your stuff there instead.

With these changes you can consider your patch approved.  You'll want
to update your doc patch accordingly.

Mark

PS I'll take care of the necessary changes to make Linux/x86 and
FreeBSD/i386 use the new code.

Re: [RFA] Unified watchpoints for x86 platforms

[Eli Zaretskii]

On Sat, 17 Mar 2001, Mark Kettenis wrote:

> ... config/i386/nm-i386v.h is really for System V-ish systems only,
> and not a generic i386 file (the comment on the first line of that
> file is a misleading).

Not only is the comment misleading, but it looks like all the x86
targets include nm-i386v.h, so it is definitely used like a generic
i386 file!

> Therefore I think it's best to create a new config/i386/nm-i386.h
> file and put your stuff there instead.

Should I make nm-i386v.h include nm-i386.h, then?

Re: [RFA] Make access watchpoints work again

[Eli Zaretskii]

Earlier in the thread "Unified watchpoints for x86 platforms" I
mentioned that awatch causes GDB to call internal_error.  The patch
below fixes the problem which was causing this.  It looks like two
lines mysteriously changed places (and were later reindented).

I wish all bugs were as easy to find and fix as this one ;-)

Okay to commit?

(Btw, why isn't the case, where an access watchpoint triggers, but the
value didn't change, being annotated with a call to
annotate_watchpoint?)

2001-03-18  Eli Zaretskii  <eliz@is.elta.co.il>

	* breakpoint.c (print_it_typical) <bp_access_watchpoint> [UI_OUT]:
	Correct the order of calls to ui_out_field_string and
	ui_out_list_begin when bs->old_val is NULL.

--- gdb/breakpoint.c~0	Wed Jan 31 03:24:00 2001
+++ gdb/breakpoint.c	Sun Mar 18 20:34:06 2001
@@ -2093,8 +2093,8 @@ print_it_typical (bpstat bs)
	{
	  mention (bs->breakpoint_at);
	  if (interpreter_p && strcmp (interpreter_p, "mi") == 0)
-	    ui_out_list_begin (uiout, "value");
-	  ui_out_field_string (uiout, "reason", "access-watchpoint-trigger");
+	    ui_out_field_string (uiout, "reason", "access-watchpoint-trigger");
+	  ui_out_list_begin (uiout, "value");
	  ui_out_text (uiout, "\nValue = ");
	}
       value_print (bs->breakpoint_at->val, stb->stream, 0,Val_pretty_default);

Re: [RFA] Make access watchpoints work again

[Andrew Cagney]

> (Btw, why isn't the case, where an access watchpoint triggers, but the
> value didn't change, being annotated with a call to
> annotate_watchpoint?)

If it wasn't being done in the pre- ui-out code, I'd assume it's because
no one noticed it missing :-) If it was being done then I'd assume a bug
in the conversion to ui-out :-(

This probably illustrates a reason for trying to move GDB to ui-out /
libgdb - get a single unified mechanism for representing this
information.

	Andrew

PS: Assume the ui-out change is an obvious fix.

	Andrew

Re: [RFA] Make access watchpoints work again

[Eli Zaretskii]

On Mon, 19 Mar 2001, Andrew Cagney wrote:

> > (Btw, why isn't the case, where an access watchpoint triggers, but the
> > value didn't change, being annotated with a call to
> > annotate_watchpoint?)
> 
> If it wasn't being done in the pre- ui-out code, I'd assume it's because
> no one noticed it missing :-) If it was being done then I'd assume a bug
> in the conversion to ui-out :-(

No, the conversion to ui-out seems to properly follow what's been
there before: any watchpoint which triggers is not annotated if the
watched expression's value is unchanged.

The only way I can understand this is that the users of the annotation
interface did something which doesn't make sense when the value hasn't
changed.  I don't have experience with any application that uses
annotations, so I cannot figure out whether this hypothesis is true or
not.  Anybody?

FWIW, annotate.texi doesn't say anything about read watchpoints being
unannotated.

If no one can find the reason, how about adding the missing annotation
and waiting for someone to scream?

> PS: Assume the ui-out change is an obvious fix.

Commited.

Re: [RFA] Make access watchpoints work again

[Andrew Cagney]

> If no one can find the reason, how about adding the missing annotation
> and waiting for someone to scream?

I'd go the other way - do nothing and just wait for someone to scream.

	Andrew

Re: [RFA] Unified watchpoints for x86 platforms

[Eli Zaretskii]

This is the patch for the docs which adds a section about watchpoints
to gdbint.texinfo.  Please see if the description is clear and useful.

2001-03-17  Eli Zaretskii  <eliz@is.elta.co.il>

	* gdbint.texinfo (Algorithms): New section "Watchpoints" and new
	subsection "x86 Watchpoints".
	(Target Architecture Definition): Document
	I386_USE_GENERIC_WATCHPOINTS and TARGET_HAS_HARDWARE_WATCHPOINTS.
	(Native Debugging): Document I386_USE_GENERIC_WATCHPOINTS.


--- gdb/doc/gdbint.t~1	Sat Feb 10 22:35:34 2001
+++ gdb/doc/gdbint.texinfo	Sat Mar 17 16:20:18 2001
@@ -1,4 +1,4 @@
-\input texinfo
+\input texinfo   @c -*- texinfo -*-
 @setfilename gdbint.info
 @include gdb-cfg.texi
 @ifinfo
@@ -291,7 +291,11 @@
 
 Since they depend on hardware resources, hardware breakpoints may be
 limited in number; when the user asks for more, @value{GDBN} will
-start trying to set software breakpoints.
+start trying to set software breakpoints.  (On some architectures,
+notably the 32-bit x86 platforms, @value{GDBN} cannot alsways know
+whether there's enough hardware resources to insert all the hardware
+breakpoints and watchpoints.  On those platforms, @value{GDBN} prints
+an error message only when the program being debugged is continued.)
 
 @cindex software breakpoints
 Software breakpoints require @value{GDBN} to do somewhat more work.
@@ -350,6 +354,306 @@
 @file{tm-@var{target}.h} file.  Look in @file{tm-sun4os4.h} and
 @file{sparc-tdep.c} for examples of how to do this.
 
+@section Watchpoints
+@cindex watchpoints
+
+Watchpoints are a special kind of breakpoints (@pxref{Algorithms,
+breakpoints}) which break when data is accessed rather than when some
+instruction is executed.  When you have data which changes without
+your knowing what code does that, watchpoints are the silver bullet to
+hunt down and kill such bugs.
+
+@cindex hardware watchpoints
+@cindex software watchpoints
+Watchpoints can be either hardware-assisted or not; the latter type is
+known as ``software watchpoints.''  @value{GDBN} always uses
+hardware-assisted watchpoints if they are available, and falls back on
+software watchpoints otherwise.  Typical situations where @value{GDBN}
+will use software watchpoints are:
+
+@itemize @bullet
+@item
+The watched memory region is too large for the underlying hardware
+watchpoint support.  For example, each x86 debug register can watch up
+to 4 bytes of memory, so trying to watch data structures whose size is
+more than 16 bytes will cause @value{GDBN} to use software
+watchpoints.
+
+@item
+Too many different watchpoints requested.  (On some architectures,
+this situation is impossible to detect until the debugged program is
+resumed.)  Note that x86 debug registers are used both for hardware
+breakpoints and for watchpoints, so setting too many hardware
+breakpoints might cause watchpoint insertion to fail.
+
+@item
+No hardware-assisted watchpoints provided by the target
+implementation.
+@end itemize
+
+Software watchpoints are very slow, since @value{GDBN} needs to
+single-step the program being debugged and test the value of the
+watched expression(s) after each instruction.  The rest of this
+section is mostly irrelevant for software watchpoints.
+
+@value{GDBN} uses several macros and primitives to support hardware
+watchpoints:
+
+@table @code
+@findex TARGET_HAS_HARDWARE_WATCHPOINTS
+@item TARGET_HAS_HARDWARE_WATCHPOINTS
+If defined, the target supports hardware watchpoints.
+
+@findex TARGET_CAN_USE_HARDWARE_WATCHPOINT
+@item TARGET_CAN_USE_HARDWARE_WATCHPOINT (@var{type}, @var{count}, @var{other})
+Return the number of hardware watchpoints of type @var{type} that are
+possible to be set.  The value is positive if @var{count} watchpoints
+of this type can be set, zero if setting watchpoints of this type is
+not supported, and negative if @var{count} is more than the maximum
+number of watchpoints of type @var{type} that can be set.  @var{other}
+is non-zero if other types of watchpoints are currently enabled (there
+are architectures which cannot set watchpoints of different types).
+
+@findex TARGET_REGION_OK_FOR_HW_WATCHPOINT
+@item TARGET_REGION_OK_FOR_HW_WATCHPOINT (@var{addr}, @var{len})
+Return non-zero if hardware watchpoints can be used to watch a region
+whose address is @var{addr} and whose length in bytes is @var{len}.
+
+@findex TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT
+@item TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT (@var{size})
+Return non-zero if hardware watchpoints can be used to watch a region
+whose size is @var{size}.  @value{GDBN} only uses this macro as a
+fall-back, in case @code{TARGET_REGION_OK_FOR_HW_WATCHPOINT} is not
+defined.
+
+@findex TARGET_DISABLE_HW_WATCHPOINTS
+@item TARGET_DISABLE_HW_WATCHPOINTS (@var{pid})
+Disables watchpoints in the process identified by @var{pid}.  This is
+used, e.g., on HP-UX which provides operations to disable and enable
+the page-level memory protection that implements hardware watchpoints
+on that platform.
+
+@findex TARGET_ENABLE_HW_WATCHPOINTS
+@item TARGET_ENABLE_HW_WATCHPOINTS (@var{pid})
+Enables watchpoints in the process identified by @var{pid}.  This is
+used, e.g., on HP-UX which provides operations to disable and enable
+the page-level memory protection that implements hardware watchpoints
+on that platform.
+
+@findex TARGET_RANGE_PROFITABLE_FOR_HW_WATCHPOINT
+@item TARGET_RANGE_PROFITABLE_FOR_HW_WATCHPOINT (@var{pid},@var{start},@var{len})
+Some addresses may not be profitable to use hardware to watch, or may
+be difficult to understand when the addressed object is out of scope,
+and hence should not be watched with hardware watchpoints.  On some
+targets, this may have severe performance penalties, such that we
+might as well use regular watchpoints, and save (possibly precious)
+hardware watchpoints for other locations.
+
+@findex target_insert_watchpoint
+@findex target_remove_watchpoint
+@item target_insert_watchpoint (@var{addr}, @var{len}, @var{type})
+@itemx target_remove_watchpoint (@var{addr}, @var{len}, @var{type})
+Insert or remove a hardware watchpoint starting at @var{addr}, for
+@var{len} bytes.  @var{type} is the watchpoint type, one of the
+possible values of the enumerated data type @code{target_hw_bp_type},
+defined by @file{breakpoint.h} as follows:
+
+@example
+ enum target_hw_bp_type
+   @{
+     hw_write   = 0, /* Common (write) HW watchpoint */
+     hw_read    = 1, /* Read    HW watchpoint */
+     hw_access  = 2, /* Access (read or write) HW watchpoint */
+     hw_execute = 3  /* Execute HW breakpoint */
+   @};
+@end example
+
+@noindent
+These two macros should return 0 for success, non-zero for failure.
+
+@cindex insert or remove hardware breakpoint
+@findex target_remove_hw_breakpoint
+@findex target_insert_hw_breakpoint
+@item target_remove_hw_breakpoint (@var{addr}, @var{shadow})
+@itemx target_insert_hw_breakpoint (@var{addr}, @var{shadow})
+Insert or remove a hardware-assisted breakpoint at address @var{addr}.
+Returns zero for success, non-zero for failure.  @var{shadow} is the
+real contents of the byte where the breakpoint has been inserted; it
+is generally not valid when hardware breakpoints are used, but since
+no other code touches these values, the implementations of the above
+two macros can use them for their internal purposes.
+
+@findex target_stopped_data_address
+@item target_stopped_data_address ()
+If the inferior has some watchpoint that triggered, return the address
+associated with that watchpoint.  Otherwise, return zero.
+
+@findex DECR_PC_AFTER_HW_BREAK
+@item DECR_PC_AFTER_HW_BREAK
+If defined, @value{GDBN} decrements the program counter by the value
+of @code{DECR_PC_AFTER_HW_BREAK} after a hardware break-point.  This
+overrides the value of @code{DECR_PC_AFTER_BREAK} when hardware
+breakpoints are used.
+
+@findex HAVE_STEPPABLE_WATCHPOINT
+@item HAVE_STEPPABLE_WATCHPOINT
+If defined to a non-zero value, it is not necessary to disable a
+watchpoint to step over it.
+
+@findex HAVE_NONSTEPPABLE_WATCHPOINT
+@item HAVE_NONSTEPPABLE_WATCHPOINT
+If defined to a non-zero value, @value{GDBN} should disable a
+watchpoint to step the inferior over it.
+
+@findex HAVE_CONTINUABLE_WATCHPOINT
+@item HAVE_CONTINUABLE_WATCHPOINT
+If defined to a non-zero value, it is possible to continue the
+inferior after a watchpoint has been hit.
+
+@findex CANNOT_STEP_HW_WATCHPOINTS
+@item CANNOT_STEP_HW_WATCHPOINTS
+If this is defined to a non-zero value, @value{GDBN} will remove all
+watchpoints before stepping the inferior.
+
+@findex STOPPED_BY_WATCHPOINT
+@item STOPPED_BY_WATCHPOINT (@var{wait_status})
+Return non-zero if stopped by a watchpoint.  @var{wait_status} is of
+the type @code{struct target_waitstatus}, defined by @file{target.h}.
+@end table
+
+@subsection x86 Watchpoints
+@cindex x86 debug registers
+@cindex watchpoints, on x86
+
+The 32-bit Intel x86 (a.k.a.@: ia32) processors feature special debug
+registers designed to facilitate debugging.  @value{GDBN} provides a
+generic library of functions that x86-based ports can use to implement
+support for watchpoints and hardware-assisted breakpoints.  This
+subsection documents the x86 watchpoint facilities in @value{GDBN}.
+
+To use the generic x86 watchpoint support, a port should do the
+following:
+
+@itemize @bullet
+@findex I386_USE_GENERIC_WATCHPOINTS
+@item
+Define the macro @code{I386_USE_GENERIC_WATCHPOINTS} somewhere in the
+target-dependent headers.
+
+@item
+Include the @file{config/i386/nm-i386v.h} header file @emph{after}
+defining @code{I386_USE_GENERIC_WATCHPOINTS}.
+
+@item
+Add @file{i386-nat.o} to the value of the Make variable
+@code{NATDEPFILES} (@pxref{Native Debugging, NATDEPFILES}) or
+@code{TDEPFILES} (@pxref{Target Architecture Definition, TDEPFILES}).
+
+@item
+Provide implementations for the @code{I386_DR_LOW_*} macros described
+below.  Typically, each macro should call a target-specific function
+which does the real work.
+@end itemize
+
+The x86 watchpoint support works by maintaining mirror images of the
+debug registers.  Values are copied between the mirror images and the
+real debug registers via a set of macros which each target needs to
+provide:
+
+@table @code
+@findex I386_DR_LOW_SET_CONTROL
+@item I386_DR_LOW_SET_CONTROL (@var{val})
+Set the Debug Control (DR7) register to the value @var{val}.
+
+@findex I386_DR_LOW_SET_ADDR
+@item I386_DR_LOW_SET_ADDR (@var{idx}, @var{addr})
+Put the address @var{addr} into the debug register number @var{idx}.
+
+@findex I386_DR_LOW_RESET_ADDR
+@item I386_DR_LOW_RESET_ADDR (@var{idx})
+Reset (i.e. zero out) the address stored in the debug register number
+@var{idx}.
+
+@findex I386_DR_LOW_GET_STATUS
+@item I386_DR_LOW_GET_STATUS
+Return the value of the Debug Status (DR6) register.
+@end table
+
+For each one of the 4 debug registers (whose indices are from 0 to 3)
+that store addresses, a reference count is maintained by @value{GDBN},
+to allow sharing of debug registers by several watchpoints.  This
+allows the users to define several watchpoints which watch the same
+expression, but with different conditions and/or commands, without
+wasting debug registers which are in short supply.  @value{GDBN}
+maintains the reference counts internally, targets don't have to do
+anything to use this feature.
+
+The generic x86 watchpoint support provides the following API for the
+@value{GDBN}'s application code:
+
+@table @code
+@findex i386_region_ok_for_watchpoint
+@item i386_region_ok_for_watchpoint (@var{addr}, @var{len})
+The macro @code{TARGET_REGION_OK_FOR_HW_WATCHPOINT} is set to call
+this function.
+
+@findex i386_stopped_data_address
+@item i386_stopped_data_address (void)
+The macro @code{STOPPED_BY_WATCHPOINT} is set to call this function.
+The argument passed to @code{STOPPED_BY_WATCHPOINT} is ignored.
+
+@findex i386_stopped_data_address
+@item i386_stopped_data_address (void)
+The macro target_stopped_data_address is set to call this function.
+
+@findex i386_insert_watchpoint
+@findex i386_remove_watchpoint
+@item i386_insert_watchpoint (@var{addr}, @var{len}, @var{type})
+@itemx i386_remove_watchpoint (@var{addr}, @var{len}, @var{type})
+Insert or remove a watchpoint.  The macros
+@code{target_insert_watchpoint} and @code{target_remove_watchpoint}
+are set to call these functions.
+
+@findex i386_insert_hw_breakpoint
+@findex i386_remove_hw_breakpoint
+@item i386_insert_hw_breakpoint (@var{addr}, @var{shadow}
+@itemx i386_remove_hw_breakpoint (@var{addr}, @var{shadow})
+These functions insert and remove hardware-assisted breakpoints.  The
+macros @code{target_insert_hw_breakpoint} and
+@code{target_remove_hw_breakpoint} are set to call these functions.
+
+@findex i386_stopped_by_hwbp
+@item i386_stopped_by_hwbp (void)
+This function returns non-zero if the inferior has some watchpoint or
+hardware breakpoint that triggered.
+
+@findex i386_cleanup_dregs
+@item i386_cleanup_dregs (void)
+This function clears all the reference counts, addresses, and control
+bits in the mirror images of the debug registers.
+@end table
+
+@noindent
+@strong{Notes:}
+@enumerate 1
+@item
+x86 processors support setting watchpoints on I/O reads or writes.
+However, since no target supports this (as of March 2001), and since
+@code{enum target_hw_bp_type} doesn't even have an enumeration for I/O
+watchpoints, this feature is not yet available to @value{GDBN} running
+on x86.
+
+@item
+x86 processors can enable watchpoints locally, for the current task
+only, or globally, for all the tasks.  For each debug register,
+there's a bit in the DR7 control register that determines whether the
+associated address is watched locally or globally.  The current
+implementation of x86 watchpoint support in @value{GDBN} always sets
+watchpoints to be locally enabled, since global watchpoints might
+interfere with the underlying OS and are probably unavailable in many
+platforms.
+@end enumerate
+
 @node User Interface
 
 @chapter User Interface
@@ -374,7 +678,7 @@
 To add commands in general, use @code{add_cmd}.  @code{add_com} adds to
 the main command list, and should be used for those commands.  The usual
 place to add commands is in the @code{_initialize_@var{xyz}} routines at
-the ends of most source files. 
+the ends of most source files.
 
 @cindex deprecating commands
 @findex deprecate_cmd
@@ -1102,10 +1406,10 @@
 @code{BIG_ENDIAN} or @code{LITTLE_ENDIAN}.
 
 @item INT_MAX
-@item INT_MIN
-@item LONG_MAX
-@item UINT_MAX
-@item ULONG_MAX
+@itemx INT_MIN
+@itemx LONG_MAX
+@itemx UINT_MAX
+@itemx ULONG_MAX
 Values for host-side constants.
 
 @item ISATTY
@@ -1222,7 +1526,7 @@
 Define if this is not in a system header file (typically, @file{unistd.h}).
 
 @item SEEK_CUR
-@item SEEK_SET
+@itemx SEEK_SET
 Define these to appropriate value for the system @code{lseek}, if not already
 defined.
 
@@ -1990,6 +2294,10 @@
 feature-specific macros.  It was introduced in a haste and we are
 repenting at leisure.
 
+@item I386_USE_GENERIC_WATCHPOINTS
+An x86-based target can define this to use the generic x86 watchpoint
+support; see @ref{Algorithms, I386_USE_GENERIC_WATCHPOINTS}.
+
 @item SYMBOLS_CAN_START_WITH_DOLLAR
 @findex SYMBOLS_CAN_START_WITH_DOLLAR
 Some systems have routines whose names start with @samp{$}.  Giving this
@@ -2466,6 +2774,11 @@
 frame pointers are not used, a default definition simply returns 
 @code{FP_REGNUM}, with an offset of zero.
 
+@item TARGET_HAS_HARDWARE_WATCHPOINTS
+If non-zero, the target has support for hardware-assisted
+watchpoints.  @xref{Algorithms, watchpoints}, for more details and
+other related macros.
+
 @item USE_STRUCT_CONVENTION (@var{gcc_p}, @var{type})
 @findex USE_STRUCT_CONVENTION
 If defined, this must be an expression that is nonzero if a value of the
@@ -2795,6 +3108,10 @@
 @code{CORE_ADDR *} as argument, and stores the target PC value through this
 pointer.  It examines the current state of the machine as needed.
 
+@item I386_USE_GENERIC_WATCHPOINTS
+An x86-based machine can define this to use the generic x86 watchpoint
+support; see @ref{Algorithms, I386_USE_GENERIC_WATCHPOINTS}.
+
 @item KERNEL_U_ADDR
 @findex KERNEL_U_ADDR
 Define this to the address of the @code{u} structure (the ``user