2011-07-22 Pedro Alves <pedro@codesourcery.com>

	gdb/testsuite/
	* gdb.arch/i386-dr3-watch.exp: Test that the i386 watchpoints
	backend doesn't leave used debug registers behind.

	gdb/gdbserver/
	* i386-low.c (i386_insert_aligned_watchpoint): Don't pass the info
	to the inferior here.
	(i386_remove_aligned_watchpoint): Ditto.
	(i386_handle_nonaligned_watchpoint): Return immediate on fail to
	fit part of the watchpoint in the debug registers.
	(i386_update_inferior_debug_regs): New.
	(i386_low_insert_watchpoint): Work on a local mirror of the debug
	registers, and only update the inferior on success.
	(i386_low_remove_watchpoint): Ditto.

	gdb/
	* i386-nat.c (I386_DR_VACANT, 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): Add state parameter and
	adjust.
	(dr_mirror, dr_status_mirror, dr_control_mirror): Delete.
	(struct i386_debug_reg_state): New.
	(i386_init_dregs): New.
	(dr_mirror): New.
	(i386_cleanup_dregs): Use i386_init_dregs.
	(i386_show_dr): Add state parameter and adjust.
	(i386_insert_aligned_watchpoint): Ditto.  Don't pass the info to
	the inferior here.
	(i386_remove_aligned_watchpoint): Likewise.
	(i386_handle_nonaligned_watchpoint): Add state parameter and adjust.
	(i386_update_inferior_debug_regs): New.
	(i386_insert_watchpoint): Work on a local mirror of the debug
	registers, and only update the inferior on success.
	(i386_remove_watchpoint): Ditto.
	(i386_region_ok_for_watchpoint): Adjust.
	(i386_stopped_data_address): Adjust.
	(i386_insert_hw_breakpoint): Adjust.
	(i386_remove_hw_breakpoint): Adjust.

6 files changed, 313 insertions, 123 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 37214d3..e8b5d05 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,28 @@
+2011-07-22  Pedro Alves  <pedro@codesourcery.com>
+
+	* i386-nat.c (I386_DR_VACANT, 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): Add state parameter and
+	adjust.
+	(dr_mirror, dr_status_mirror, dr_control_mirror): Delete.
+	(struct i386_debug_reg_state): New.
+	(i386_init_dregs): New.
+	(dr_mirror): New.
+	(i386_cleanup_dregs): Use i386_init_dregs.
+	(i386_show_dr): Add state parameter and adjust.
+	(i386_insert_aligned_watchpoint): Ditto.  Don't pass the info to
+	the inferior here.
+	(i386_remove_aligned_watchpoint): Likewise.
+	(i386_handle_nonaligned_watchpoint): Add state parameter and adjust.
+	(i386_update_inferior_debug_regs): New.
+	(i386_insert_watchpoint): Work on a local mirror of the debug
+	registers, and only update the inferior on success.
+	(i386_remove_watchpoint): Ditto.
+	(i386_region_ok_for_watchpoint): Adjust.
+	(i386_stopped_data_address): Adjust.
+	(i386_insert_hw_breakpoint): Adjust.
+	(i386_remove_hw_breakpoint): Adjust.
+
 2011-07-22  Tom Tromey  <tromey@redhat.com>
 
 	* amd64-tdep.c (amd64_pseudo_register_read_value): Rename
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index d0a914f..338f2c6 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,15 @@
+2011-07-22  Pedro Alves  <pedro@codesourcery.com>
+
+	* i386-low.c (i386_insert_aligned_watchpoint): Don't pass the info
+	to the inferior here.
+	(i386_remove_aligned_watchpoint): Ditto.
+	(i386_handle_nonaligned_watchpoint): Return immediate on fail to
+	fit part of the watchpoint in the debug registers.
+	(i386_update_inferior_debug_regs): New.
+	(i386_low_insert_watchpoint): Work on a local mirror of the debug
+	registers, and only update the inferior on success.
+	(i386_low_remove_watchpoint): Ditto.
+
 2011-07-22  Kwok Cheung Yeung  <kcy@codesourcery.com>
 
 	* linux-low.c (compare_ints, unique, list_threads, show_process,
diff --git a/gdb/gdbserver/i386-low.c b/gdb/gdbserver/i386-low.c
index 1baa23d..a959179 100644
--- a/gdb/gdbserver/i386-low.c
+++ b/gdb/gdbserver/i386-low.c
@@ -307,10 +307,6 @@ i386_insert_aligned_watchpoint (struct i386_debug_reg_state *state,
   state->dr_control_mirror |= DR_LOCAL_SLOWDOWN;
   state->dr_control_mirror &= I386_DR_CONTROL_MASK;
 
-  /* Finally, actually pass the info to the inferior.  */
-  i386_dr_low_set_addr (state, i);
-  i386_dr_low_set_control (state);
-
   return 0;
 }
 
@@ -337,9 +333,6 @@ i386_remove_aligned_watchpoint (struct i386_debug_reg_state *state,
 	      /* Reset our mirror.  */
 	      state->dr_mirror[i] = 0;
 	      I386_DR_DISABLE (state, i);
-	      /* Reset it in the inferior.  */
-	      i386_dr_low_set_control (state);
-	      i386_dr_low_set_addr (state, i);
 	    }
 	  retval = 0;
 	}
@@ -360,7 +353,7 @@ i386_handle_nonaligned_watchpoint (struct i386_debug_reg_state *state,
 				   i386_wp_op_t what, CORE_ADDR addr, int len,
 				   enum target_hw_bp_type type)
 {
-  int retval = 0, status = 0;
+  int retval = 0;
   int max_wp_len = TARGET_HAS_DR_LEN_8 ? 8 : 4;
 
   static const int size_try_array[8][8] =
@@ -398,25 +391,16 @@ i386_handle_nonaligned_watchpoint (struct i386_debug_reg_state *state,
 	  unsigned len_rw = i386_length_and_rw_bits (size, type);
 
 	  if (what == WP_INSERT)
-	    status = i386_insert_aligned_watchpoint (state, addr, len_rw);
+	    retval = i386_insert_aligned_watchpoint (state, addr, len_rw);
 	  else if (what == WP_REMOVE)
-	    status = i386_remove_aligned_watchpoint (state, addr, len_rw);
+	    retval = i386_remove_aligned_watchpoint (state, addr, len_rw);
 	  else
 	    fatal ("\
 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)
-	    retval = status;
+	  if (retval)
+	    break;
 	}
 
       addr += size;
@@ -448,6 +432,29 @@ Z_packet_to_hw_type (char type)
     }
 }
 
+/* Update the inferior debug registers state, in INF_STATE, with the
+   new debug registers state, in NEW_STATE.  */
+
+static void
+i386_update_inferior_debug_regs (struct i386_debug_reg_state *inf_state,
+				 struct i386_debug_reg_state *new_state)
+{
+  int i;
+
+  ALL_DEBUG_REGISTERS (i)
+    {
+      if (I386_DR_VACANT (new_state, i) != I386_DR_VACANT (inf_state, i))
+	i386_dr_low_set_addr (new_state, i);
+      else
+	gdb_assert (new_state->dr_mirror[i] == inf_state->dr_mirror[i]);
+    }
+
+  if (new_state->dr_control_mirror != inf_state->dr_control_mirror)
+    i386_dr_low_set_control (new_state);
+
+  *inf_state = *new_state;
+}
+
 /* 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_FROM_PACKET.  Return 0 on success, -1 on failure.  */
@@ -458,6 +465,9 @@ i386_low_insert_watchpoint (struct i386_debug_reg_state *state,
 {
   int retval;
   enum target_hw_bp_type type = Z_packet_to_hw_type (type_from_packet);
+  /* Work on a local copy of the debug registers, and on success,
+     commit the change back to the inferior.  */
+  struct i386_debug_reg_state local_state = *state;
 
   if (type == hw_read)
     return 1; /* unsupported */
@@ -466,16 +476,19 @@ i386_low_insert_watchpoint (struct i386_debug_reg_state *state,
        && !(TARGET_HAS_DR_LEN_8 && len == 8))
       || addr % len != 0)
     {
-      retval = i386_handle_nonaligned_watchpoint (state, WP_INSERT,
+      retval = i386_handle_nonaligned_watchpoint (&local_state, WP_INSERT,
 						  addr, len, type);
     }
   else
     {
       unsigned len_rw = i386_length_and_rw_bits (len, type);
 
-      retval = i386_insert_aligned_watchpoint (state, addr, len_rw);
+      retval = i386_insert_aligned_watchpoint (&local_state, addr, len_rw);
     }
 
+  if (retval == 0)
+    i386_update_inferior_debug_regs (state, &local_state);
+
   if (debug_hw_points)
     i386_show_dr (state, "insert_watchpoint", addr, len, type);
 
@@ -492,21 +505,27 @@ i386_low_remove_watchpoint (struct i386_debug_reg_state *state,
 {
   int retval;
   enum target_hw_bp_type type = Z_packet_to_hw_type (type_from_packet);
+  /* Work on a local copy of the debug registers, and on success,
+     commit the change back to the inferior.  */
+  struct i386_debug_reg_state local_state = *state;
 
   if (((len != 1 && len != 2 && len != 4)
        && !(TARGET_HAS_DR_LEN_8 && len == 8))
       || addr % len != 0)
     {
-      retval = i386_handle_nonaligned_watchpoint (state, WP_REMOVE,
+      retval = i386_handle_nonaligned_watchpoint (&local_state, WP_REMOVE,
 						  addr, len, type);
     }
   else
     {
       unsigned len_rw = i386_length_and_rw_bits (len, type);
 
-      retval = i386_remove_aligned_watchpoint (state, addr, len_rw);
+      retval = i386_remove_aligned_watchpoint (&local_state, addr, len_rw);
     }
 
+  if (retval == 0)
+    i386_update_inferior_debug_regs (state, &local_state);
+
   if (debug_hw_points)
     i386_show_dr (state, "remove_watchpoint", addr, len, type);
 
diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c
index 7e6814e..4c6c4e9 100644
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -111,45 +111,88 @@ struct i386_dr_low_type i386_dr_low;
 
 /* 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)
+#define I386_DR_VACANT(state, i)					\
+  (((state)->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)))
+#define I386_DR_LOCAL_ENABLE(state, i) \
+  do { \
+    (state)->dr_control_mirror |= \
+      (1 << (DR_LOCAL_ENABLE_SHIFT + DR_ENABLE_SIZE * (i))); \
+  } while (0)
 
 /* 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)))
+#define I386_DR_GLOBAL_ENABLE(state, i) \
+  do { \
+    (state)->dr_control_mirror |= \
+      (1 << (DR_GLOBAL_ENABLE_SHIFT + DR_ENABLE_SIZE * (i))); \
+  } while (0)
 
 /* Disable the break/watchpoint in the I'th debug register.  */
-#define I386_DR_DISABLE(i) \
-  dr_control_mirror &= ~(3 << (DR_ENABLE_SIZE * (i)))
+#define I386_DR_DISABLE(state, i) \
+  do { \
+    (state)->dr_control_mirror &= \
+      ~(3 << (DR_ENABLE_SIZE * (i))); \
+  } while (0)
 
 /* Set in DR7 the RW and LEN fields for the I'th debug register.  */
-#define I386_DR_SET_RW_LEN(i,rwlen) \
+#define I386_DR_SET_RW_LEN(state, 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))); \
+    (state)->dr_control_mirror &= \
+      ~(0x0f << (DR_CONTROL_SHIFT + DR_CONTROL_SIZE * (i))); \
+    (state)->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)
+#define I386_DR_GET_RW_LEN(dr7, i) \
+  (((dr7) \
+    >> (DR_CONTROL_SHIFT + DR_CONTROL_SIZE * (i))) & 0x0f)
 
 /* Mask that this I'th watchpoint has triggered.  */
 #define I386_DR_WATCH_MASK(i)	(1 << (i))
 
 /* Did the watchpoint whose address is in the I'th register break?  */
-#define I386_DR_WATCH_HIT(i)	(dr_status_mirror & I386_DR_WATCH_MASK (i))
+#define I386_DR_WATCH_HIT(dr6, i) ((dr6) & (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 long dr_status_mirror, dr_control_mirror;
+
+/* Global state needed to track h/w watchpoints.  */
+
+struct i386_debug_reg_state
+{
+  /* Mirror the inferior's DRi registers.  We keep the status and
+     control registers separated because they don't hold addresses.
+     Note that since we can change these mirrors while threads are
+     running, we never trust them to explain a cause of a trap.
+     For that, we need to peek directly in the inferior registers.  */
+  CORE_ADDR dr_mirror[DR_NADDR];
+  unsigned dr_status_mirror, dr_control_mirror;
+
+  /* Reference counts for each debug register.  */
+  int dr_ref_count[DR_NADDR];
+};
+
+/* Clear the reference counts and forget everything we knew about the
+   debug registers.  */
+
+static void
+i386_init_dregs (struct i386_debug_reg_state *state)
+{
+  int i;
+
+  ALL_DEBUG_REGISTERS (i)
+    {
+      state->dr_mirror[i] = 0;
+      state->dr_ref_count[i] = 0;
+    }
+  state->dr_control_mirror = 0;
+  state->dr_status_mirror  = 0;
+}
+
+static struct i386_debug_reg_state dr_mirror;
 
 /* Reference counts for each debug register.  */
 static int dr_ref_count[DR_NADDR];
@@ -172,7 +215,8 @@ static unsigned i386_length_and_rw_bits (int len, enum target_hw_bp_type type);
    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,
+static int i386_insert_aligned_watchpoint (struct i386_debug_reg_state *state,
+					   CORE_ADDR addr,
 					   unsigned len_rw_bits);
 
 /* Remove a watchpoint at address ADDR, which is assumed to be aligned
@@ -180,7 +224,8 @@ static int i386_insert_aligned_watchpoint (CORE_ADDR addr,
    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,
+static int i386_remove_aligned_watchpoint (struct i386_debug_reg_state *state,
+					   CORE_ADDR addr,
 					   unsigned len_rw_bits);
 
 /* Insert or remove a (possibly non-aligned) watchpoint, or count the
@@ -189,7 +234,8 @@ static int i386_remove_aligned_watchpoint (CORE_ADDR addr,
    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,
+static int i386_handle_nonaligned_watchpoint (struct i386_debug_reg_state *state,
+					      i386_wp_op_t what,
 					      CORE_ADDR addr, int len,
 					      enum target_hw_bp_type type);
 
@@ -201,15 +247,7 @@ static int i386_handle_nonaligned_watchpoint (i386_wp_op_t what,
 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;
+  i386_init_dregs (&dr_mirror);
 }
 
 /* Print the values of the mirrored debug registers.  This is called
@@ -217,7 +255,8 @@ i386_cleanup_dregs (void)
    show-debug-regs" at GDB's prompt.  */
 
 static void
-i386_show_dr (const char *func, CORE_ADDR addr,
+i386_show_dr (struct i386_debug_reg_state *state,
+	      const char *func, CORE_ADDR addr,
 	      int len, enum target_hw_bp_type type)
 {
   int addr_size = gdbarch_addr_bit (target_gdbarch) / 8;
@@ -239,13 +278,16 @@ i386_show_dr (const char *func, CORE_ADDR addr,
 				: "??unknown??"))));
   puts_unfiltered (":\n");
   printf_unfiltered ("\tCONTROL (DR7): %s          STATUS (DR6): %s\n",
-		     phex (dr_control_mirror, 8), phex (dr_status_mirror, 8));
+		     phex (state->dr_control_mirror, 8),
+		     phex (state->dr_status_mirror, 8));
   ALL_DEBUG_REGISTERS(i)
     {
       printf_unfiltered ("\
 \tDR%d: addr=0x%s, ref.count=%d  DR%d: addr=0x%s, ref.count=%d\n",
-		 i, phex (dr_mirror[i], addr_size), dr_ref_count[i],
-		 i+1, phex (dr_mirror[i+1], addr_size), dr_ref_count[i+1]);
+			 i, phex (state->dr_mirror[i], addr_size),
+			 state->dr_ref_count[i],
+			 i + 1, phex (state->dr_mirror[i + 1], addr_size),
+			 state->dr_ref_count[i+1]);
       i++;
     }
 }
@@ -311,7 +353,8 @@ Invalid hardware breakpoint length %d in i386_length_and_rw_bits.\n"), len);
    success, -1 on failure.  */
 
 static int
-i386_insert_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits)
+i386_insert_aligned_watchpoint (struct i386_debug_reg_state *state,
+				CORE_ADDR addr, unsigned len_rw_bits)
 {
   int i;
 
@@ -323,11 +366,11 @@ i386_insert_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits)
      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)
+      if (!I386_DR_VACANT (state, i)
+	  && state->dr_mirror[i] == addr
+	  && I386_DR_GET_RW_LEN (state->dr_control_mirror, i) == len_rw_bits)
 	{
-	  dr_ref_count[i]++;
+	  state->dr_ref_count[i]++;
 	  return 0;
 	}
     }
@@ -335,7 +378,7 @@ i386_insert_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits)
   /* Next, look for a vacant debug register.  */
   ALL_DEBUG_REGISTERS(i)
     {
-      if (I386_DR_VACANT (i))
+      if (I386_DR_VACANT (state, i))
 	break;
     }
 
@@ -346,9 +389,9 @@ i386_insert_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits)
   /* 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);
+  state->dr_mirror[i] = addr;
+  state->dr_ref_count[i] = 1;
+  I386_DR_SET_RW_LEN (state, i, len_rw_bits);
   /* Note: we only enable the watchpoint locally, i.e. in the current
      task.  Currently, no i386 target allows or supports global
      watchpoints; however, if any target would want that in the
@@ -356,17 +399,9 @@ i386_insert_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits)
      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_addr (i, addr);
-  i386_dr_low.set_control (dr_control_mirror);
-
-  /* Only a sanity check for leftover bits (set possibly only by inferior).  */
-  if (i386_dr_low.unset_status)
-    i386_dr_low.unset_status (I386_DR_WATCH_MASK (i));
+  I386_DR_LOCAL_ENABLE (state, i);
+  state->dr_control_mirror |= DR_LOCAL_SLOWDOWN;
+  state->dr_control_mirror &= I386_DR_CONTROL_MASK;
 
   return 0;
 }
@@ -378,25 +413,22 @@ i386_insert_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits)
    success, -1 on failure.  */
 
 static int
-i386_remove_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits)
+i386_remove_aligned_watchpoint (struct i386_debug_reg_state *state,
+				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 (!I386_DR_VACANT (state, i)
+	  && state->dr_mirror[i] == addr
+	  && I386_DR_GET_RW_LEN (state->dr_control_mirror, i) == len_rw_bits)
 	{
-	  if (--dr_ref_count[i] == 0) /* no longer in use?  */
+	  if (--state->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.set_control (dr_control_mirror);
-	      if (i386_dr_low.reset_addr)
-		i386_dr_low.reset_addr (i);
+	      state->dr_mirror[i] = 0;
+	      I386_DR_DISABLE (state, i);
 	    }
 	  retval = 0;
 	}
@@ -413,10 +445,11 @@ i386_remove_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits)
    valid value, bombs through internal_error.  */
 
 static int
-i386_handle_nonaligned_watchpoint (i386_wp_op_t what, CORE_ADDR addr, int len,
+i386_handle_nonaligned_watchpoint (struct i386_debug_reg_state *state,
+				   i386_wp_op_t what, CORE_ADDR addr, int len,
 				   enum target_hw_bp_type type)
 {
-  int retval = 0, status = 0;
+  int retval = 0;
   int max_wp_len = TARGET_HAS_DR_LEN_8 ? 8 : 4;
 
   static int size_try_array[8][8] =
@@ -454,24 +487,15 @@ i386_handle_nonaligned_watchpoint (i386_wp_op_t what, CORE_ADDR addr, int len,
 	  unsigned len_rw = i386_length_and_rw_bits (size, type);
 
 	  if (what == WP_INSERT)
-	    status = i386_insert_aligned_watchpoint (addr, len_rw);
+	    retval = i386_insert_aligned_watchpoint (state, addr, len_rw);
 	  else if (what == WP_REMOVE)
-	    status = i386_remove_aligned_watchpoint (addr, len_rw);
+	    retval = i386_remove_aligned_watchpoint (state, 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)
-	    retval = status;
+	  if (retval)
+	    break;
 	}
 
       addr += size;
@@ -481,6 +505,43 @@ Invalid value %d of operation in i386_handle_nonaligned_watchpoint.\n"),
   return retval;
 }
 
+/* Update the inferior's debug registers with the new debug registers
+   state, in NEW_STATE, and then update our local mirror to match.  */
+
+static void
+i386_update_inferior_debug_regs (struct i386_debug_reg_state *new_state)
+{
+  int i;
+
+  ALL_DEBUG_REGISTERS (i)
+    {
+      if (I386_DR_VACANT (new_state, i) != I386_DR_VACANT (&dr_mirror, i))
+	{
+	  if (!I386_DR_VACANT (new_state, i))
+	    {
+	      i386_dr_low.set_addr (i, new_state->dr_mirror[i]);
+
+	      /* Only a sanity check for leftover bits (set possibly only
+		 by inferior).  */
+	      if (i386_dr_low.unset_status)
+		i386_dr_low.unset_status (I386_DR_WATCH_MASK (i));
+	    }
+	  else
+	    {
+	      if (i386_dr_low.reset_addr)
+		i386_dr_low.reset_addr (i);
+	    }
+	}
+      else
+	gdb_assert (new_state->dr_mirror[i] == dr_mirror.dr_mirror[i]);
+    }
+
+  if (new_state->dr_control_mirror != dr_mirror.dr_control_mirror)
+    i386_dr_low.set_control (new_state->dr_control_mirror);
+
+  dr_mirror = *new_state;
+}
+
 /* 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.  */
@@ -490,22 +551,30 @@ i386_insert_watchpoint (CORE_ADDR addr, int len, int type,
 			struct expression *cond)
 {
   int retval;
+  /* Work on a local copy of the debug registers, and on success,
+     commit the change back to the inferior.  */
+  struct i386_debug_reg_state local_state = dr_mirror;
 
   if (type == hw_read)
     return 1; /* unsupported */
 
   if (((len != 1 && len !=2 && len !=4) && !(TARGET_HAS_DR_LEN_8 && len == 8))
       || addr % len != 0)
-    retval = i386_handle_nonaligned_watchpoint (WP_INSERT, addr, len, type);
+    retval = i386_handle_nonaligned_watchpoint (&local_state,
+						WP_INSERT, addr, len, type);
   else
     {
       unsigned len_rw = i386_length_and_rw_bits (len, type);
 
-      retval = i386_insert_aligned_watchpoint (addr, len_rw);
+      retval = i386_insert_aligned_watchpoint (&local_state,
+					       addr, len_rw);
     }
 
+  if (retval == 0)
+    i386_update_inferior_debug_regs (&local_state);
+
   if (maint_show_dr)
-    i386_show_dr ("insert_watchpoint", addr, len, type);
+    i386_show_dr (&dr_mirror, "insert_watchpoint", addr, len, type);
 
   return retval;
 }
@@ -518,19 +587,27 @@ i386_remove_watchpoint (CORE_ADDR addr, int len, int type,
 			struct expression *cond)
 {
   int retval;
+  /* Work on a local copy of the debug registers, and on success,
+     commit the change back to the inferior.  */
+  struct i386_debug_reg_state local_state = dr_mirror;
 
   if (((len != 1 && len !=2 && len !=4) && !(TARGET_HAS_DR_LEN_8 && len == 8))
       || addr % len != 0)
-    retval = i386_handle_nonaligned_watchpoint (WP_REMOVE, addr, len, type);
+    retval = i386_handle_nonaligned_watchpoint (&local_state,
+						WP_REMOVE, addr, len, type);
   else
     {
       unsigned len_rw = i386_length_and_rw_bits (len, type);
 
-      retval = i386_remove_aligned_watchpoint (addr, len_rw);
+      retval = i386_remove_aligned_watchpoint (&local_state,
+					       addr, len_rw);
     }
 
+  if (retval == 0)
+    i386_update_inferior_debug_regs (&local_state);
+
   if (maint_show_dr)
-    i386_show_dr ("remove_watchpoint", addr, len, type);
+    i386_show_dr (&dr_mirror, "remove_watchpoint", addr, len, type);
 
   return retval;
 }
@@ -545,7 +622,8 @@ i386_region_ok_for_watchpoint (CORE_ADDR addr, int len)
 
   /* Compute how many aligned watchpoints we would need to cover this
      region.  */
-  nregs = i386_handle_nonaligned_watchpoint (WP_COUNT, addr, len, hw_write);
+  nregs = i386_handle_nonaligned_watchpoint (&dr_mirror,
+					     WP_COUNT, addr, len, hw_write);
   return nregs <= DR_NADDR ? 1 : 0;
 }
 
@@ -559,30 +637,35 @@ i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
   CORE_ADDR addr = 0;
   int i;
   int rc = 0;
+  unsigned status;
+  unsigned control;
+  struct i386_debug_reg_state *state = &dr_mirror;
 
-  dr_status_mirror = i386_dr_low.get_status ();
+  dr_mirror.dr_status_mirror = i386_dr_low.get_status ();
+  status = dr_mirror.dr_status_mirror;
+  control = dr_mirror.dr_control_mirror;
 
   ALL_DEBUG_REGISTERS(i)
     {
-      if (I386_DR_WATCH_HIT (i)
+      if (I386_DR_WATCH_HIT (status, 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
+	  && I386_DR_GET_RW_LEN (control, i) != 0
 	  /* This third condition makes sure DRi is not vacant, this
 	     avoids false positives in windows-nat.c.  */
-	  && !I386_DR_VACANT (i))
+	  && !I386_DR_VACANT (state, i))
 	{
-	  addr = dr_mirror[i];
+	  addr = state->dr_mirror[i];
 	  rc = 1;
 	  if (maint_show_dr)
-	    i386_show_dr ("watchpoint_hit", addr, -1, hw_write);
+	    i386_show_dr (&dr_mirror, "watchpoint_hit", addr, -1, hw_write);
 	}
     }
   if (maint_show_dr && addr == 0)
-    i386_show_dr ("stopped_data_addr", 0, 0, hw_write);
+    i386_show_dr (&dr_mirror, "stopped_data_addr", 0, 0, hw_write);
 
   if (rc)
     *addr_p = addr;
@@ -604,10 +687,11 @@ i386_insert_hw_breakpoint (struct gdbarch *gdbarch,
 {
   unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
   CORE_ADDR addr = bp_tgt->placed_address;
-  int retval = i386_insert_aligned_watchpoint (addr, len_rw) ? EBUSY : 0;
+  int retval = i386_insert_aligned_watchpoint (&dr_mirror,
+					       addr, len_rw) ? EBUSY : 0;
 
   if (maint_show_dr)
-    i386_show_dr ("insert_hwbp", addr, 1, hw_execute);
+    i386_show_dr (&dr_mirror, "insert_hwbp", addr, 1, hw_execute);
 
   return retval;
 }
@@ -621,10 +705,11 @@ i386_remove_hw_breakpoint (struct gdbarch *gdbarch,
 {
   unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
   CORE_ADDR addr = bp_tgt->placed_address;
-  int retval = i386_remove_aligned_watchpoint (addr, len_rw);
+  int retval = i386_remove_aligned_watchpoint (&dr_mirror,
+					       addr, len_rw);
 
   if (maint_show_dr)
-    i386_show_dr ("remove_hwbp", addr, 1, hw_execute);
+    i386_show_dr (&dr_mirror, "remove_hwbp", addr, 1, hw_execute);
 
   return retval;
 }
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index a907aa1..9a90952 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2011-07-22  Pedro Alves  <pedro@codesourcery.com>
+
+	* gdb.arch/i386-dr3-watch.exp: Test that the i386 watchpoints
+	backend doesn't leave used debug registers behind.
+
 2011-07-22  Tom Tromey  <tromey@redhat.com>
 
 	* gdb.dwarf2/typeddwarf.c: XFAIL 'z' on x86-64.
diff --git a/gdb/testsuite/gdb.arch/i386-dr3-watch.exp b/gdb/testsuite/gdb.arch/i386-dr3-watch.exp
index f560b01..35d7587 100644
--- a/gdb/testsuite/gdb.arch/i386-dr3-watch.exp
+++ b/gdb/testsuite/gdb.arch/i386-dr3-watch.exp
@@ -38,6 +38,8 @@ if ![runto_main] then {
 
 gdb_test_no_output "set breakpoint always-inserted on"
 
+# Test that we handle watchpoints in all of DR0-DR3.
+
 gdb_test "watch i1" "Hardware watchpoint .*: i1"
 gdb_test "watch i2" "Hardware watchpoint .*: i2"
 gdb_test "watch i3" "Hardware watchpoint .*: i3"
@@ -47,3 +49,45 @@ gdb_test "c" "Hardware watchpoint.*: i1.*" "continue to i1 watchpoint"
 gdb_test "c" "Hardware watchpoint.*: i2.*" "continue to i2 watchpoint"
 gdb_test "c" "Hardware watchpoint.*: i3.*" "continue to i3 watchpoint"
 gdb_test "c" "Hardware watchpoint.*: i4.*" "continue to i4 watchpoint"
+
+delete_breakpoints
+
+# Regression test for a bug where the i386 watchpoints support backend
+# would leave some debug registers occupied even if not enough debug
+# registers were available to cover a single watchpoint location.
+
+gdb_test "watch i1" \
+    "Hardware watchpoint .*: i1" \
+    "set watchpoint occuping one debug register"
+
+# gap1 too long to fit the 3 left over debug registers (but would fit
+# 4 if all were available).
+set test "watchpoint on gap1 does not fit debug registers"
+gdb_test_multiple "watch gap1" "$test" {
+    -re "Hardware watchpoint .*: gap1.*Warning:.*Could not insert hardware watchpoint.*You may have requested too many.*" {
+	pass $test
+    }
+    -re "Hardware watchpoint .*: gap1\r\n$gdb_prompt $" {
+	pass "$test (target emulates hardware watchpoints)"
+	return
+    }
+    -re "Watchpoint .*: gap1\r\n$gdb_prompt $" {
+	pass "$test (gdb figured out itself the watchpoint does not fit)"
+	return
+    }
+}
+
+# Start over.
+gdb_test "delete" \
+    "" \
+    "delete all watchpoints" \
+    "Delete all breakpoints.*$" \
+    "y"
+
+# If debug registers were left occupied by mistake, we'll fail to set
+# some of these watchpoints.  Each watchpoint should fit in one of the
+# 4 debug registers available.
+gdb_test "watch i1" "Hardware watchpoint .*: i1" "watch i1 still fits"
+gdb_test "watch i2" "Hardware watchpoint .*: i2" "watch i2 still fits"
+gdb_test "watch i3" "Hardware watchpoint .*: i3" "watch i3 still fits"
+gdb_test "watch i4" "Hardware watchpoint .*: i4" "watch i4 still fits"