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.

1 files changed, 44 insertions, 25 deletions

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);