2011-07-22 Pedro Alves <pedro@codesourcery.com>
authorPedro Alves <palves@redhat.com>
Fri, 22 Jul 2011 16:58:32 +0000 (16:58 +0000)
committerPedro Alves <palves@redhat.com>
Fri, 22 Jul 2011 16:58:32 +0000 (16:58 +0000)
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.

gdb/ChangeLog
gdb/gdbserver/ChangeLog
gdb/gdbserver/i386-low.c
gdb/i386-nat.c
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.arch/i386-dr3-watch.exp

index 37214d35d3001185dcc52719bdb35ee9196077a4..e8b5d057c360c5e52fe008a8fa9ddcf2d85e83b8 100644 (file)
@@ -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
index d0a914faacafb364be4233b8294a9750f8683154..338f2c62bc2c21081b0779dd40ec2391b1e96d0f 100644 (file)
@@ -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,
index 1baa23dd9645e4b0760301472e278b526fd9bdc8..a9591794680d63bda2c426f1d31d2769fdfa0190 100644 (file)
@@ -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);
 
index 7e6814ebad7fe2a6f583a6393e7c5b68b796c948..4c6c4e959f80817bef227bb8de428000b6d0169d 100644 (file)
@@ -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;
 }
index a907aa160c4f9744f1be240d573a961f1cc71f50..9a909522148a18ddb9eb16566334e0ee79a4a4e4 100644 (file)
@@ -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.
index f560b013fe6bed2b1d487e7a6f0b2f01011f51b0..35d758706d3710d62bbb406ce5b1c7e01b38d980 100644 (file)
@@ -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"