public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 01/15] Whitespace changes
  2014-06-17 14:13 [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Gary Benson
  2014-06-17 14:13 ` [PATCH 03/15] Remove unused macro Gary Benson
  2014-06-17 14:13 ` [PATCH 04/15] Comment changes Gary Benson
@ 2014-06-17 14:13 ` Gary Benson
  2014-06-17 15:23   ` Pedro Alves
  2014-06-17 14:40 ` [PATCH 14/15] Create nat/i386-dregs.c Gary Benson
                   ` (12 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Gary Benson @ 2014-06-17 14:13 UTC (permalink / raw)
  To: gdb-patches

This commit fixes various whitespace differences between i386-nat.c
and i386-low.c.

gdb/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-nat.c: Whitespace changes.

gdb/gdbserver/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-low.c: Whitespace changes.
---
 gdb/ChangeLog            |    4 ++++
 gdb/gdbserver/ChangeLog  |    4 ++++
 gdb/gdbserver/i386-low.c |   16 ++++++++++------
 gdb/i386-nat.c           |   46 +++++++++++++++++++++++++++-------------------
 4 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/gdb/gdbserver/i386-low.c b/gdb/gdbserver/i386-low.c
index de2d11a..304d6f3 100644
--- a/gdb/gdbserver/i386-low.c
+++ b/gdb/gdbserver/i386-low.c
@@ -115,7 +115,7 @@
   } while (0)
 
 /* Set in DR7 the RW and LEN fields for the I'th debug register.  */
-#define I386_DR_SET_RW_LEN(state, i,rwlen) \
+#define I386_DR_SET_RW_LEN(state, i, rwlen) \
   do { \
     (state)->dr_control_mirror &= \
       ~(0x0f << (DR_CONTROL_SHIFT + DR_CONTROL_SIZE * (i))); \
@@ -136,7 +136,7 @@
 
 /* Types of operations supported by i386_handle_nonaligned_watchpoint.  */
 typedef enum { WP_INSERT, WP_REMOVE, WP_COUNT } i386_wp_op_t;
-\f
+
 /* Implementation.  */
 
 /* Clear the reference counts and forget everything we knew about the
@@ -446,14 +446,16 @@ 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 (&local_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 (&local_state, addr, len_rw);
+      retval = i386_insert_aligned_watchpoint (&local_state,
+					       addr, len_rw);
     }
 
   if (retval == 0)
@@ -483,14 +485,16 @@ i386_low_remove_watchpoint (struct i386_debug_reg_state *state,
        && !(TARGET_HAS_DR_LEN_8 && len == 8))
       || addr % len != 0)
     {
-      retval = i386_handle_nonaligned_watchpoint (&local_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 (&local_state, addr, len_rw);
+      retval = i386_remove_aligned_watchpoint (&local_state,
+					       addr, len_rw);
     }
 
   if (retval == 0)
diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c
index 2f80a6e..a56a116 100644
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -39,7 +39,6 @@
 
 struct i386_dr_low_type i386_dr_low;
 
-
 /* Support for 8-byte wide hw watchpoints.  */
 #define TARGET_HAS_DR_LEN_8 (i386_dr_low.debug_register_length == 8)
 
@@ -90,7 +89,7 @@ struct i386_dr_low_type i386_dr_low;
    later processors do that automatically.  We set these flags for
    backwards compatibility.  */
 #define DR_LOCAL_SLOWDOWN	(0x100)
-#define DR_GLOBAL_SLOWDOWN     	(0x200)
+#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
@@ -106,7 +105,7 @@ 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(state, i)					\
+#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.  */
@@ -322,7 +321,7 @@ i386_show_dr (struct i386_debug_reg_state *state,
     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,
+		       (unsigned long) addr, len,
 		       type == hw_write ? "data-write"
 		       : (type == hw_read ? "data-read"
 			  : (type == hw_access ? "data-read/write"
@@ -335,14 +334,14 @@ i386_show_dr (struct i386_debug_reg_state *state,
   printf_unfiltered ("\tCONTROL (DR7): %s          STATUS (DR6): %s\n",
 		     phex (state->dr_control_mirror, 8),
 		     phex (state->dr_status_mirror, 8));
-  ALL_DEBUG_REGISTERS(i)
+  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 (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]);
+			 state->dr_ref_count[i + 1]);
       i++;
     }
 }
@@ -419,7 +418,7 @@ i386_insert_aligned_watchpoint (struct i386_debug_reg_state *state,
   /* 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)
+  ALL_DEBUG_REGISTERS (i)
     {
       if (!I386_DR_VACANT (state, i)
 	  && state->dr_mirror[i] == addr
@@ -431,7 +430,7 @@ i386_insert_aligned_watchpoint (struct i386_debug_reg_state *state,
     }
 
   /* Next, look for a vacant debug register.  */
-  ALL_DEBUG_REGISTERS(i)
+  ALL_DEBUG_REGISTERS (i)
     {
       if (I386_DR_VACANT (state, i))
 	break;
@@ -473,7 +472,7 @@ i386_remove_aligned_watchpoint (struct i386_debug_reg_state *state,
 {
   int i, retval = -1;
 
-  ALL_DEBUG_REGISTERS(i)
+  ALL_DEBUG_REGISTERS (i)
     {
       if (!I386_DR_VACANT (state, i)
 	  && state->dr_mirror[i] == addr
@@ -548,7 +547,7 @@ i386_handle_nonaligned_watchpoint (struct i386_debug_reg_state *state,
 	  else
 	    internal_error (__FILE__, __LINE__, _("\
 Invalid value %d of operation in i386_handle_nonaligned_watchpoint.\n"),
-			    (int)what);
+			    (int) what);
 	  if (retval)
 	    break;
 	}
@@ -603,10 +602,14 @@ i386_insert_watchpoint (struct target_ops *self,
   if (type == hw_read)
     return 1; /* unsupported */
 
-  if (((len != 1 && len !=2 && len !=4) && !(TARGET_HAS_DR_LEN_8 && len == 8))
+  if (((len != 1 && len != 2 && len != 4)
+       && !(TARGET_HAS_DR_LEN_8 && len == 8))
       || addr % len != 0)
-    retval = i386_handle_nonaligned_watchpoint (&local_state,
-						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);
@@ -639,10 +642,14 @@ i386_remove_watchpoint (struct target_ops *self,
      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))
+  if (((len != 1 && len != 2 && len != 4)
+       && !(TARGET_HAS_DR_LEN_8 && len == 8))
       || addr % len != 0)
-    retval = i386_handle_nonaligned_watchpoint (&local_state,
-						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);
@@ -673,8 +680,8 @@ i386_region_ok_for_watchpoint (struct target_ops *self,
 
   /* Compute how many aligned watchpoints we would need to cover this
      region.  */
-  nregs = i386_handle_nonaligned_watchpoint (state,
-					     WP_COUNT, addr, len, hw_write);
+  nregs = i386_handle_nonaligned_watchpoint (state, WP_COUNT,
+					     addr, len, hw_write);
   return nregs <= DR_NADDR ? 1 : 0;
 }
 
@@ -726,7 +733,7 @@ i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
      registers.  */
   status = i386_dr_low.get_status ();
 
-  ALL_DEBUG_REGISTERS(i)
+  ALL_DEBUG_REGISTERS (i)
     {
       if (!I386_DR_WATCH_HIT (status, i))
 	continue;
@@ -750,6 +757,7 @@ i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
 	    i386_show_dr (state, "watchpoint_hit", addr, -1, hw_write);
 	}
     }
+
   if (maint_show_dr && addr == 0)
     i386_show_dr (state, "stopped_data_addr", 0, 0, hw_write);
 
-- 
1.7.1

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH 03/15] Remove unused macro
  2014-06-17 14:13 [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Gary Benson
@ 2014-06-17 14:13 ` Gary Benson
  2014-06-17 15:35   ` Pedro Alves
  2014-06-17 14:13 ` [PATCH 04/15] Comment changes Gary Benson
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Gary Benson @ 2014-06-17 14:13 UTC (permalink / raw)
  To: gdb-patches

This commit removes an unused macro from i386-nat.c.

gdb/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-nat.c (I386_DR_WATCH_MASK): Remove macro.
---
 gdb/ChangeLog  |    4 ++++
 gdb/i386-nat.c |    3 ---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c
index 7c9e7ca..f9fb52f 100644
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -143,9 +143,6 @@ struct i386_dr_low_type i386_dr_low;
   (((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(dr6, i) ((dr6) & (1 << (i)))
 
-- 
1.7.1

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch]
@ 2014-06-17 14:13 Gary Benson
  2014-06-17 14:13 ` [PATCH 03/15] Remove unused macro Gary Benson
                   ` (15 more replies)
  0 siblings, 16 replies; 45+ messages in thread
From: Gary Benson @ 2014-06-17 14:13 UTC (permalink / raw)
  To: gdb-patches

Hi all,

This series refactors the shared code in i386-{nat,low}.[ch] into
a pair of new files, nat/i386-dregs.[ch].  The patches are in two
groups.  Group 1 removes the changes between the various files so
that the code to be shared is identical:

   1/15 - Whitespace changes
   2/15 - Remove unnecessary prototypes
   3/15 - Remove unused macro
   4/15 - Comment changes
   5/15 - Add a const
   6/15 - Merge printing code
   7/15 - Merge error handling
   8/15 - Rename maint_show_dr to debug_hw_points
   9/15 - Merge i386_update_inferior_debug_regs
  10/15 - Abstract i386_dr_low access

Group 2 moves all that identical code into the new files:

  11/15 - Rename functions and make nonstatic as necessary
  12/15 - Create nat/i386-dregs.h
  13/15 - Move macros from i386-{nat,low}.c to i386-{nat,low}.h
  14/15 - Create nat/i386-dregs.c
  15/15 - Buildsystem changes

Is this ok to commit?

Thanks,
Gary

--
http://gbenson.net/

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH 04/15] Comment changes
  2014-06-17 14:13 [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Gary Benson
  2014-06-17 14:13 ` [PATCH 03/15] Remove unused macro Gary Benson
@ 2014-06-17 14:13 ` Gary Benson
  2014-06-17 15:45   ` Pedro Alves
  2014-06-17 14:13 ` [PATCH 01/15] Whitespace changes Gary Benson
                   ` (13 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Gary Benson @ 2014-06-17 14:13 UTC (permalink / raw)
  To: gdb-patches

This commit fixes various whitespace differences between
i386-nat.[ch] and i386-low.[ch].

gdb/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-nat.c: Comment changes.

gdb/gdbserver/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-low.h: Comment changes.
	* i386-low.c: Likewise.
---
 gdb/ChangeLog            |    4 +++
 gdb/gdbserver/ChangeLog  |    5 ++++
 gdb/gdbserver/i386-low.c |   24 +++++++++++++++------
 gdb/gdbserver/i386-low.h |    6 ++--
 gdb/i386-nat.c           |   51 ++++++++++++++++++++++++---------------------
 5 files changed, 56 insertions(+), 34 deletions(-)

diff --git a/gdb/gdbserver/i386-low.c b/gdb/gdbserver/i386-low.c
index 304d6f3..d122ff7 100644
--- a/gdb/gdbserver/i386-low.c
+++ b/gdb/gdbserver/i386-low.c
@@ -21,6 +21,17 @@
 #include "target.h"
 #include "i386-low.h"
 
+/* Support for hardware watchpoints and breakpoints using the i386
+   debug registers.
+
+   This provides several functions for inserting and removing
+   hardware-assisted breakpoints and watchpoints, testing if one or
+   more of the watchpoints triggered and at what address, checking
+   whether a given region can be watched, etc.
+
+   The functions below implement debug registers sharing by reference
+   counts, and allow to watch regions up to 16 bytes long.  */
+
 /* Support for 8-byte wide hw watchpoints.  */
 #ifndef TARGET_HAS_DR_LEN_8
 /* NOTE: sizeof (long) == 4 on win64.  */
@@ -156,8 +167,7 @@ i386_low_init_dregs (struct i386_debug_reg_state *state)
   state->dr_status_mirror  = 0;
 }
 
-/* Print the values of the mirrored debug registers.  This is enabled via
-   the "set debug-hw-points 1" monitor command.  */
+/* Print the values of the mirrored debug registers.  */
 
 static void
 i386_show_dr (struct i386_debug_reg_state *state,
@@ -427,7 +437,7 @@ i386_update_inferior_debug_regs (struct i386_debug_reg_state *inf_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.  */
+   of the type TYPE.  Return 0 on success, -1 on failure.  */
 
 int
 i386_low_insert_watchpoint (struct i386_debug_reg_state *state,
@@ -523,8 +533,8 @@ i386_low_region_ok_for_watchpoint (struct i386_debug_reg_state *state,
 }
 
 /* If the inferior has some break/watchpoint that triggered, set the
-   address associated with that break/watchpoint and return true.
-   Otherwise, return false.  */
+   address associated with that break/watchpoint and return non-zero.
+   Otherwise, return zero.  */
 
 int
 i386_low_stopped_data_address (struct i386_debug_reg_state *state,
@@ -603,8 +613,8 @@ i386_low_stopped_data_address (struct i386_debug_reg_state *state,
   return rc;
 }
 
-/* Return true if the inferior has some watchpoint that triggered.
-   Otherwise return false.  */
+/* Return non-zero if the inferior has some watchpoint that triggered.
+   Otherwise return zero.  */
 
 int
 i386_low_stopped_by_watchpoint (struct i386_debug_reg_state *state)
diff --git a/gdb/gdbserver/i386-low.h b/gdb/gdbserver/i386-low.h
index 8d8e73f..681ade0 100644
--- a/gdb/gdbserver/i386-low.h
+++ b/gdb/gdbserver/i386-low.h
@@ -32,9 +32,9 @@
 /* Debug registers' indices.  */
 #define DR_FIRSTADDR 0
 #define DR_LASTADDR  3
-#define DR_NADDR     4 /* The number of debug address registers.  */
-#define DR_STATUS    6
-#define DR_CONTROL   7
+#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).  */
 
 /* Global state needed to track h/w watchpoints.  */
 
diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c
index f9fb52f..a7742f2 100644
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -37,6 +37,7 @@
    The functions below implement debug registers sharing by reference
    counts, and allow to watch regions up to 16 bytes long.  */
 
+/* Low-level function vector.  */
 struct i386_dr_low_type i386_dr_low;
 
 /* Support for 8-byte wide hw watchpoints.  */
@@ -265,9 +266,7 @@ i386_cleanup_dregs (void)
   i386_forget_process (ptid_get_pid (inferior_ptid));
 }
 
-/* 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.  */
+/* Print the values of the mirrored debug registers.  */
 
 static void
 i386_show_dr (struct i386_debug_reg_state *state,
@@ -439,7 +438,7 @@ i386_remove_aligned_watchpoint (struct i386_debug_reg_state *state,
 	  && state->dr_mirror[i] == addr
 	  && I386_DR_GET_RW_LEN (state->dr_control_mirror, i) == len_rw_bits)
 	{
-	  if (--state->dr_ref_count[i] == 0) /* no longer in use?  */
+	  if (--state->dr_ref_count[i] == 0) /* No longer in use?  */
 	    {
 	      /* Reset our mirror.  */
 	      state->dr_mirror[i] = 0;
@@ -646,8 +645,8 @@ i386_region_ok_for_watchpoint (struct target_ops *self,
   return nregs <= DR_NADDR ? 1 : 0;
 }
 
-/* If the inferior has some watchpoint that triggered, set the
-   address associated with that watchpoint and return non-zero.
+/* If the inferior has some break/watchpoint that triggered, set the
+   address associated with that break/watchpoint and return non-zero.
    Otherwise, return zero.  */
 
 static int
@@ -668,24 +667,25 @@ i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
   unsigned control = 0;
 
   /* In non-stop/async, threads can be running while we change the
-     STATE (and friends).  Say, we set a watchpoint, and let threads
-     resume.  Now, say you delete the watchpoint, or add/remove
-     watchpoints such that STATE changes while threads are running.
-     On targets that support non-stop, inserting/deleting watchpoints
-     updates the STATE only.  It does not update the real thread's
-     debug registers; that's only done prior to resume.  Instead, if
-     threads are running when the mirror changes, a temporary and
-     transparent stop on all threads is forced so they can get their
-     copy of the debug registers updated on re-resume.  Now, say,
-     a thread hit a watchpoint before having been updated with the new
-     STATE contents, and we haven't yet handled the corresponding
-     SIGTRAP.  If we trusted STATE below, we'd mistake the real
-     trapped address (from the last time we had updated debug
-     registers in the thread) with whatever was currently in STATE.
-     So to fix this, STATE always represents intention, what we _want_
-     threads to have in debug registers.  To get at the address and
-     cause of the trap, we need to read the state the thread still has
-     in its debug registers.
+     global dr_mirror (and friends).  Say, we set a watchpoint, and
+     let threads resume.  Now, say you delete the watchpoint, or
+     add/remove watchpoints such that dr_mirror changes while threads
+     are running.  On targets that support non-stop,
+     inserting/deleting watchpoints updates the global dr_mirror only.
+     It does not update the real thread's debug registers; that's only
+     done prior to resume.  Instead, if threads are running when the
+     mirror changes, a temporary and transparent stop on all threads
+     is forced so they can get their copy of the debug registers
+     updated on re-resume.  Now, say, a thread hit a watchpoint before
+     having been updated with the new dr_mirror contents, and we
+     haven't yet handled the corresponding SIGTRAP.  If we trusted
+     dr_mirror below, we'd mistake the real trapped address (from the
+     last time we had updated debug registers in the thread) with
+     whatever was currently in dr_mirror.  So to fix this, dr_mirror
+     always represents intention, what we _want_ threads to have in
+     debug registers.  To get at the address and cause of the trap, we
+     need to read the state the thread still has in its debug
+     registers.
 
      In sum, always get the current debug register values the current
      thread has, instead of trusting the global mirror.  If the thread
@@ -727,6 +727,9 @@ i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
   return rc;
 }
 
+/* Return non-zero if the inferior has some watchpoint that triggered.
+   Otherwise return zero.  */
+
 static int
 i386_stopped_by_watchpoint (struct target_ops *ops)
 {
-- 
1.7.1

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH 14/15] Create nat/i386-dregs.c
  2014-06-17 14:13 [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Gary Benson
                   ` (2 preceding siblings ...)
  2014-06-17 14:13 ` [PATCH 01/15] Whitespace changes Gary Benson
@ 2014-06-17 14:40 ` Gary Benson
  2014-06-17 16:56   ` Pedro Alves
  2014-06-17 14:40 ` [PATCH 06/15] Merge printing code Gary Benson
                   ` (11 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Gary Benson @ 2014-06-17 14:40 UTC (permalink / raw)
  To: gdb-patches

This commit moves code to be shared from i386-{nat,low}.[ch]
into a new file, nat/i386-dregs.c.

gdb/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* nat/i386-dregs.c: New file.
	* i386-nat.h (debug_hw_points): New declaration.
	* i386-nat.c (breakpoint.h): Remove include.
	(command.h): Likewise.
	(target.h): Likewise.
	(gdb_assert.h): Likewise.
	(debug_hw_points): Made nonstatic.
	(debug_printf): Now in i386-dregs.c.
	(TARGET_HAS_DR_LEN_8): Likewise.
	(DR_CONTROL_SHIFT): Likewise.
	(DR_CONTROL_SIZE): Likewise.
	(DR_RW_EXECUTE): Likewise.
	(DR_RW_WRITE): Likewise.
	(DR_RW_READ): Likewise.
	(DR_RW_IORW): Likewise.
	(DR_LEN_1): Likewise.
	(DR_LEN_2): Likewise.
	(DR_LEN_4): Likewise.
	(DR_LEN_8): Likewise.
	(DR_LOCAL_ENABLE_SHIFT): Likewise.
	(DR_GLOBAL_ENABLE_SHIFT): Likewise.
	(DR_ENABLE_SIZE): Likewise.
	(DR_LOCAL_SLOWDOWN): Likewise.
	(DR_GLOBAL_SLOWDOWN): Likewise.
	(DR_CONTROL_RESERVED): Likewise.
	(I386_DR_CONTROL_MASK): Likewise.
	(I386_DR_VACANT): Likewise.
	(I386_DR_LOCAL_ENABLE): Likewise.
	(I386_DR_GLOBAL_ENABLE): Likewise.
	(I386_DR_DISABLE): Likewise.
	(I386_DR_SET_RW_LEN): Likewise.
	(I386_DR_GET_RW_LEN): Likewise.
	(I386_DR_WATCH_HIT): Likewise.
	(i386_wp_op_t): Likewise.
	(i386_dr_show): Likewise.
	(i386_dr_length_and_rw_bits): Likewise.
	(i386_dr_insert_aligned_watchpoint): Likewise.
	(i386_dr_remove_aligned_watchpoint): Likewise.
	(i386_handle_nonaligned_watchpoint): Likewise.
	(i386_insert_watchpoint): Use i386_dr_insert_watchpoint.
	(i386_remove_watchpoint): Use i386_dr_remove_watchpoint.
	(i386_region_ok_for_watchpoint):
	Use i386_dr_region_ok_for_watchpoint.
	(i386_stopped_data_address): Use i386_dr_stopped_data_address.
	(i386_stopped_by_watchpoint): Use i386_dr_stopped_by_watchpoint.

gdb/gdbserver/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-low.c (target.h): Remove include.
	(TARGET_HAS_DR_LEN_8): Now in i386-dregs.c.
	(DR_CONTROL_SHIFT): Likewise.
	(DR_CONTROL_SIZE): Likewise.
	(DR_RW_EXECUTE): Likewise.
	(DR_RW_WRITE): Likewise.
	(DR_RW_READ): Likewise.
	(DR_RW_IORW): Likewise.
	(DR_LEN_1): Likewise.
	(DR_LEN_2): Likewise.
	(DR_LEN_4): Likewise.
	(DR_LEN_8): Likewise.
	(DR_LOCAL_ENABLE_SHIFT): Likewise.
	(DR_GLOBAL_ENABLE_SHIFT): Likewise.
	(DR_ENABLE_SIZE): Likewise.
	(DR_LOCAL_SLOWDOWN): Likewise.
	(DR_GLOBAL_SLOWDOWN): Likewise.
	(DR_CONTROL_RESERVED): Likewise.
	(I386_DR_CONTROL_MASK): Likewise.
	(I386_DR_VACANT): Likewise.
	(I386_DR_LOCAL_ENABLE): Likewise.
	(I386_DR_GLOBAL_ENABLE): Likewise.
	(I386_DR_DISABLE): Likewise.
	(I386_DR_SET_RW_LEN): Likewise.
	(I386_DR_GET_RW_LEN): Likewise.
	(I386_DR_WATCH_HIT): Likewise.
	(i386_wp_op_t): Likewise.
	(i386_dr_show): Likewise.
	(i386_dr_length_and_rw_bits): Likewise.
	(i386_dr_insert_aligned_watchpoint): Likewise.
	(i386_dr_remove_aligned_watchpoint): Likewise.
	(i386_handle_nonaligned_watchpoint): Likewise.
	(i386_dr_insert_watchpoint): Likewise.
	(i386_dr_remove_watchpoint): Likewise.
	(i386_dr_region_ok_for_watchpoint): Likewise.
	(i386_dr_stopped_data_address): Likewise.
	(i386_dr_stopped_by_watchpoint): Likewise.
---
 gdb/ChangeLog            |   48 ++++
 gdb/gdbserver/ChangeLog  |   40 +++
 gdb/gdbserver/i386-low.c |  587 -------------------------------------------
 gdb/i386-nat.c           |  540 +---------------------------------------
 gdb/i386-nat.h           |    3 +
 gdb/nat/i386-dregs.c     |  617 ++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 717 insertions(+), 1118 deletions(-)
 create mode 100644 gdb/nat/i386-dregs.c

diff --git a/gdb/gdbserver/i386-low.c b/gdb/gdbserver/i386-low.c
index 5333819..845c3b0 100644
--- a/gdb/gdbserver/i386-low.c
+++ b/gdb/gdbserver/i386-low.c
@@ -18,132 +18,8 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "server.h"
-#include "target.h"
 #include "i386-low.h"
 
-/* Support for hardware watchpoints and breakpoints using the i386
-   debug registers.
-
-   This provides several functions for inserting and removing
-   hardware-assisted breakpoints and watchpoints, testing if one or
-   more of the watchpoints triggered and at what address, checking
-   whether a given region can be watched, etc.
-
-   The functions below implement debug registers sharing by reference
-   counts, and allow to watch regions up to 16 bytes long.  */
-
-/* Support for 8-byte wide hw watchpoints.  */
-#define TARGET_HAS_DR_LEN_8 (i386_get_debug_register_length () == 8)
-
-/* 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 March 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 breakpoint.  */
-#define DR_LEN_2	(0x1 << 2) /* 2-byte region watch.  */
-#define DR_LEN_4	(0x3 << 2) /* 4-byte region watch.  */
-#define DR_LEN_8	(0x2 << 2) /* 8-byte region watch (AMD64).  */
-
-/* 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_dr_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 /* Two 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
-   backwards 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(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(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(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(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(state, i, rwlen) \
-  do { \
-    (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(dr7, i) \
-  (((dr7) \
-    >> (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(dr6, i) ((dr6) & (1 << (i)))
-
-/* Types of operations supported by i386_handle_nonaligned_watchpoint.  */
-typedef enum { WP_INSERT, WP_REMOVE, WP_COUNT } i386_wp_op_t;
-
-/* Implementation.  */
-
 /* Clear the reference counts and forget everything we knew about the
    debug registers.  */
 
@@ -160,466 +36,3 @@ i386_low_init_dregs (struct i386_debug_reg_state *state)
   state->dr_control_mirror = 0;
   state->dr_status_mirror  = 0;
 }
-
-/* Print the values of the mirrored debug registers.  */
-
-void
-i386_dr_show (struct i386_debug_reg_state *state,
-	      const char *func, CORE_ADDR addr,
-	      int len, enum target_hw_bp_type type)
-{
-  int i;
-
-  debug_printf ("%s", func);
-  if (addr || len)
-    debug_printf (" (addr=%s, len=%d, type=%s)",
-		  phex (addr, 8), 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??"))));
-  debug_printf (":\n");
-  debug_printf ("\tCONTROL (DR7): %s          STATUS (DR6): %s\n",
-		phex (state->dr_control_mirror, 8),
-		phex (state->dr_status_mirror, 8));
-  ALL_DEBUG_REGISTERS (i)
-    {
-      debug_printf ("\
-\tDR%d: addr=0x%s, ref.count=%d  DR%d: addr=0x%s, ref.count=%d\n",
-		    i, phex (state->dr_mirror[i],
-			     i386_get_debug_register_length ()),
-		    state->dr_ref_count[i],
-		    i + 1, phex (state->dr_mirror[i + 1],
-				 i386_get_debug_register_length ()),
-		    state->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.  */
-
-unsigned
-i386_dr_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:
-	internal_error (__FILE__, __LINE__,
-			_("The i386 doesn't support "
-			  "data-read watchpoints.\n"));
-      case hw_access:
-	rw = DR_RW_READ;
-	break;
-#if 0
-	/* Not yet supported.  */
-      case hw_io_access:
-	rw = DR_RW_IORW;
-	break;
-#endif
-      default:
-	internal_error (__FILE__, __LINE__, _("\
-Invalid hardware breakpoint type %d in i386_dr_length_and_rw_bits.\n"),
-			(int) type);
-    }
-
-  switch (len)
-    {
-      case 1:
-	return (DR_LEN_1 | rw);
-      case 2:
-	return (DR_LEN_2 | rw);
-      case 4:
-	return (DR_LEN_4 | rw);
-      case 8:
-        if (TARGET_HAS_DR_LEN_8)
- 	  return (DR_LEN_8 | rw);
-	/* ELSE FALL THROUGH */
-      default:
-	internal_error (__FILE__, __LINE__, _("\
-Invalid hardware breakpoint length %d in i386_dr_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.  */
-
-int
-i386_dr_insert_aligned_watchpoint (struct i386_debug_reg_state *state,
-				   CORE_ADDR addr, unsigned len_rw_bits)
-{
-  int i;
-
-  if (!i386_dr_low_can_set_addr () || !i386_dr_low_can_set_control ())
-    return -1;
-
-  /* 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 (state, i)
-	  && state->dr_mirror[i] == addr
-	  && I386_DR_GET_RW_LEN (state->dr_control_mirror, i) == len_rw_bits)
-	{
-	  state->dr_ref_count[i]++;
-	  return 0;
-	}
-    }
-
-  /* Next, look for a vacant debug register.  */
-  ALL_DEBUG_REGISTERS (i)
-    {
-      if (I386_DR_VACANT (state, 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.  */
-  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
-     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 (state, i);
-  state->dr_control_mirror |= DR_LOCAL_SLOWDOWN;
-  state->dr_control_mirror &= I386_DR_CONTROL_MASK;
-
-  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.  */
-
-int
-i386_dr_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 (state, i)
-	  && state->dr_mirror[i] == addr
-	  && I386_DR_GET_RW_LEN (state->dr_control_mirror, i) == len_rw_bits)
-	{
-	  if (--state->dr_ref_count[i] == 0) /* No longer in use?  */
-	    {
-	      /* Reset our mirror.  */
-	      state->dr_mirror[i] = 0;
-	      I386_DR_DISABLE (state, i);
-	    }
-	  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 (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;
-  int max_wp_len = TARGET_HAS_DR_LEN_8 ? 8 : 4;
-
-  static const int size_try_array[8][8] =
-  {
-    {1, 1, 1, 1, 1, 1, 1, 1},	/* Trying size one.  */
-    {2, 1, 2, 1, 2, 1, 2, 1},	/* Trying size two.  */
-    {2, 1, 2, 1, 2, 1, 2, 1},	/* Trying size three.  */
-    {4, 1, 2, 1, 4, 1, 2, 1},	/* Trying size four.  */
-    {4, 1, 2, 1, 4, 1, 2, 1},	/* Trying size five.  */
-    {4, 1, 2, 1, 4, 1, 2, 1},	/* Trying size six.  */
-    {4, 1, 2, 1, 4, 1, 2, 1},	/* Trying size seven.  */
-    {8, 1, 2, 1, 4, 1, 2, 1},	/* Trying size eight.  */
-  };
-
-  while (len > 0)
-    {
-      int align = addr % max_wp_len;
-      /* Four (eight on AMD64) is the maximum length a debug register
-	 can watch.  */
-      int try = (len > max_wp_len ? (max_wp_len - 1) : len - 1);
-      int size = size_try_array[try][align];
-
-      if (what == WP_COUNT)
-	{
-	  /* size_try_array[] is defined such 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.  */
-	  retval++;
-	}
-      else
-	{
-	  unsigned len_rw = i386_dr_length_and_rw_bits (size, type);
-
-	  if (what == WP_INSERT)
-	    retval = i386_dr_insert_aligned_watchpoint (state, addr, len_rw);
-	  else if (what == WP_REMOVE)
-	    retval = i386_dr_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);
-	  if (retval)
-	    break;
-	}
-
-      addr += size;
-      len -= size;
-    }
-
-  return retval;
-}
-
-/* Update the inferior debug registers state, in STATE, with the
-   new debug registers state, in NEW_STATE.  */
-
-void
-i386_dr_update_inferior_debug_regs (struct i386_debug_reg_state *state,
-				    struct i386_debug_reg_state *new_state)
-{
-  int i;
-
-  ALL_DEBUG_REGISTERS (i)
-    {
-      if (I386_DR_VACANT (new_state, i) != I386_DR_VACANT (state, i))
-	i386_dr_low_set_addr (new_state, i);
-      else
-	gdb_assert (new_state->dr_mirror[i] == state->dr_mirror[i]);
-    }
-
-  if (new_state->dr_control_mirror != state->dr_control_mirror)
-    i386_dr_low_set_control (new_state);
-
-  *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.  Return 0 on success, -1 on failure.  */
-
-int
-i386_dr_insert_watchpoint (struct i386_debug_reg_state *state,
-			   enum target_hw_bp_type type,
-			   CORE_ADDR addr, int len)
-{
-  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 = *state;
-
-  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 (&local_state,
-						  WP_INSERT,
-						  addr, len, type);
-    }
-  else
-    {
-      unsigned len_rw = i386_dr_length_and_rw_bits (len, type);
-
-      retval = i386_dr_insert_aligned_watchpoint (&local_state,
-					       addr, len_rw);
-    }
-
-  if (retval == 0)
-    i386_dr_update_inferior_debug_regs (state, &local_state);
-
-  if (debug_hw_points)
-    i386_dr_show (state, "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_dr_remove_watchpoint (struct i386_debug_reg_state *state,
-			   enum target_hw_bp_type type,
-			   CORE_ADDR addr, int len)
-{
-  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 = *state;
-
-  if (((len != 1 && len != 2 && len != 4)
-       && !(TARGET_HAS_DR_LEN_8 && len == 8))
-      || addr % len != 0)
-    {
-      retval = i386_handle_nonaligned_watchpoint (&local_state,
-						  WP_REMOVE,
-						  addr, len, type);
-    }
-  else
-    {
-      unsigned len_rw = i386_dr_length_and_rw_bits (len, type);
-
-      retval = i386_dr_remove_aligned_watchpoint (&local_state,
-					       addr, len_rw);
-    }
-
-  if (retval == 0)
-    i386_dr_update_inferior_debug_regs (state, &local_state);
-
-  if (debug_hw_points)
-    i386_dr_show (state, "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_dr_region_ok_for_watchpoint (struct i386_debug_reg_state *state,
-				  CORE_ADDR addr, int len)
-{
-  int nregs;
-
-  /* Compute how many aligned watchpoints we would need to cover this
-     region.  */
-  nregs = i386_handle_nonaligned_watchpoint (state, WP_COUNT,
-					     addr, len, hw_write);
-  return nregs <= DR_NADDR ? 1 : 0;
-}
-
-/* If the inferior has some break/watchpoint that triggered, set the
-   address associated with that break/watchpoint and return non-zero.
-   Otherwise, return zero.  */
-
-int
-i386_dr_stopped_data_address (struct i386_debug_reg_state *state,
-			      CORE_ADDR *addr_p)
-{
-  CORE_ADDR addr = 0;
-  int i;
-  int rc = 0;
-  /* The current thread's DR_STATUS.  We always need to read this to
-     check whether some watchpoint caused the trap.  */
-  unsigned status;
-  /* We need DR_CONTROL as well, but only iff DR_STATUS indicates a
-     data breakpoint trap.  Only fetch it when necessary, to avoid an
-     unnecessary extra syscall when no watchpoint triggered.  */
-  int control_p = 0;
-  unsigned control = 0;
-
-  /* In non-stop/async, threads can be running while we change the
-     global dr_mirror (and friends).  Say, we set a watchpoint, and
-     let threads resume.  Now, say you delete the watchpoint, or
-     add/remove watchpoints such that dr_mirror changes while threads
-     are running.  On targets that support non-stop,
-     inserting/deleting watchpoints updates the global dr_mirror only.
-     It does not update the real thread's debug registers; that's only
-     done prior to resume.  Instead, if threads are running when the
-     mirror changes, a temporary and transparent stop on all threads
-     is forced so they can get their copy of the debug registers
-     updated on re-resume.  Now, say, a thread hit a watchpoint before
-     having been updated with the new dr_mirror contents, and we
-     haven't yet handled the corresponding SIGTRAP.  If we trusted
-     dr_mirror below, we'd mistake the real trapped address (from the
-     last time we had updated debug registers in the thread) with
-     whatever was currently in dr_mirror.  So to fix this, dr_mirror
-     always represents intention, what we _want_ threads to have in
-     debug registers.  To get at the address and cause of the trap, we
-     need to read the state the thread still has in its debug
-     registers.
-
-     In sum, always get the current debug register values the current
-     thread has, instead of trusting the global mirror.  If the thread
-     was running when we last changed watchpoints, the mirror no
-     longer represents what was set in this thread's debug
-     registers.  */
-  status = i386_dr_low_get_status ();
-
-  ALL_DEBUG_REGISTERS (i)
-    {
-      if (!I386_DR_WATCH_HIT (status, i))
-	continue;
-
-      if (!control_p)
-	{
-	  control = i386_dr_low_get_control ();
-	  control_p = 1;
-	}
-
-      /* 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.  */
-      if (I386_DR_GET_RW_LEN (control, i) != 0)
-	{
-	  addr = i386_dr_low_get_addr (i);
-	  rc = 1;
-	  if (debug_hw_points)
-	    i386_dr_show (state, "watchpoint_hit", addr, -1, hw_write);
-	}
-    }
-
-  if (debug_hw_points && addr == 0)
-    i386_dr_show (state, "stopped_data_addr", 0, 0, hw_write);
-
-  if (rc)
-    *addr_p = addr;
-  return rc;
-}
-
-/* Return non-zero if the inferior has some watchpoint that triggered.
-   Otherwise return zero.  */
-
-int
-i386_dr_stopped_by_watchpoint (struct i386_debug_reg_state *state)
-{
-  CORE_ADDR addr = 0;
-  return i386_dr_stopped_data_address (state, &addr);
-}
diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c
index 1f7b70c..54128ed 100644
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -19,11 +19,7 @@
 
 #include "defs.h"
 #include "i386-nat.h"
-#include "breakpoint.h"
-#include "command.h"
 #include "gdbcmd.h"
-#include "target.h"
-#include "gdb_assert.h"
 #include "inferior.h"
 
 /* Support for hardware watchpoints and breakpoints using the i386
@@ -38,122 +34,11 @@
    counts, and allow to watch regions up to 16 bytes long.  */
 
 /* Whether or not to print the mirrored debug registers.  */
-static int debug_hw_points;
-
-/* Function used for printing mirrored debug registers.  */
-#define debug_printf(fmt, args...) \
-  fprintf_unfiltered (gdb_stdlog, fmt, ##args);
+int debug_hw_points;
 
 /* Low-level function vector.  */
 struct i386_dr_low_type i386_dr_low;
 
-/* Support for 8-byte wide hw watchpoints.  */
-#define TARGET_HAS_DR_LEN_8 (i386_get_debug_register_length () == 8)
-
-/* 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 March 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 breakpoint.  */
-#define DR_LEN_2	(0x1 << 2) /* 2-byte region watch.  */
-#define DR_LEN_4	(0x3 << 2) /* 4-byte region watch.  */
-#define DR_LEN_8	(0x2 << 2) /* 8-byte region watch (AMD64).  */
-
-/* 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_dr_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 /* Two 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
-   backwards 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(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(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(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(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(state, i, rwlen) \
-  do { \
-    (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(dr7, i) \
-  (((dr7) \
-    >> (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(dr6, i) ((dr6) & (1 << (i)))
-
 /* Per-process data.  We don't bind this to a per-inferior registry
    because of targets like x86 GNU/Linux that need to keep track of
    processes that aren't bound to any inferior (e.g., fork children,
@@ -252,11 +137,6 @@ i386_forget_process (pid_t pid)
     }
 }
 
-/* Types of operations supported by i386_handle_nonaligned_watchpoint.  */
-typedef enum { WP_INSERT, WP_REMOVE, WP_COUNT } i386_wp_op_t;
-
-/* Implementation.  */
-
 /* Clear the reference counts and forget everything we knew about the
    debug registers.  */
 
@@ -267,281 +147,6 @@ i386_cleanup_dregs (void)
   i386_forget_process (ptid_get_pid (inferior_ptid));
 }
 
-/* Print the values of the mirrored debug registers.  */
-
-void
-i386_dr_show (struct i386_debug_reg_state *state,
-	      const char *func, CORE_ADDR addr,
-	      int len, enum target_hw_bp_type type)
-{
-  int i;
-
-  debug_printf ("%s", func);
-  if (addr || len)
-    debug_printf (" (addr=%s, len=%d, type=%s)",
-		  phex (addr, 8), 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??"))));
-  debug_printf (":\n");
-  debug_printf ("\tCONTROL (DR7): %s          STATUS (DR6): %s\n",
-		phex (state->dr_control_mirror, 8),
-		phex (state->dr_status_mirror, 8));
-  ALL_DEBUG_REGISTERS (i)
-    {
-      debug_printf ("\
-\tDR%d: addr=0x%s, ref.count=%d  DR%d: addr=0x%s, ref.count=%d\n",
-		    i, phex (state->dr_mirror[i],
-			     i386_get_debug_register_length ()),
-		    state->dr_ref_count[i],
-		    i + 1, phex (state->dr_mirror[i + 1],
-				 i386_get_debug_register_length ()),
-		    state->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.  */
-
-unsigned
-i386_dr_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:
-	internal_error (__FILE__, __LINE__,
-			_("The i386 doesn't support "
-			  "data-read watchpoints.\n"));
-      case hw_access:
-	rw = DR_RW_READ;
-	break;
-#if 0
-	/* Not yet supported.  */
-      case hw_io_access:
-	rw = DR_RW_IORW;
-	break;
-#endif
-      default:
-	internal_error (__FILE__, __LINE__, _("\
-Invalid hardware breakpoint type %d in i386_dr_length_and_rw_bits.\n"),
-			(int) type);
-    }
-
-  switch (len)
-    {
-      case 1:
-	return (DR_LEN_1 | rw);
-      case 2:
-	return (DR_LEN_2 | rw);
-      case 4:
-	return (DR_LEN_4 | rw);
-      case 8:
-        if (TARGET_HAS_DR_LEN_8)
- 	  return (DR_LEN_8 | rw);
-	/* ELSE FALL THROUGH */
-      default:
-	internal_error (__FILE__, __LINE__, _("\
-Invalid hardware breakpoint length %d in i386_dr_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.  */
-
-int
-i386_dr_insert_aligned_watchpoint (struct i386_debug_reg_state *state,
-				   CORE_ADDR addr, unsigned len_rw_bits)
-{
-  int i;
-
-  if (!i386_dr_low_can_set_addr () || !i386_dr_low_can_set_control ())
-    return -1;
-
-  /* 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 (state, i)
-	  && state->dr_mirror[i] == addr
-	  && I386_DR_GET_RW_LEN (state->dr_control_mirror, i) == len_rw_bits)
-	{
-	  state->dr_ref_count[i]++;
-	  return 0;
-	}
-    }
-
-  /* Next, look for a vacant debug register.  */
-  ALL_DEBUG_REGISTERS (i)
-    {
-      if (I386_DR_VACANT (state, 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.  */
-  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
-     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 (state, i);
-  state->dr_control_mirror |= DR_LOCAL_SLOWDOWN;
-  state->dr_control_mirror &= I386_DR_CONTROL_MASK;
-
-  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.  */
-
-int
-i386_dr_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 (state, i)
-	  && state->dr_mirror[i] == addr
-	  && I386_DR_GET_RW_LEN (state->dr_control_mirror, i) == len_rw_bits)
-	{
-	  if (--state->dr_ref_count[i] == 0) /* No longer in use?  */
-	    {
-	      /* Reset our mirror.  */
-	      state->dr_mirror[i] = 0;
-	      I386_DR_DISABLE (state, i);
-	    }
-	  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 (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;
-  int max_wp_len = TARGET_HAS_DR_LEN_8 ? 8 : 4;
-
-  static const int size_try_array[8][8] =
-  {
-    {1, 1, 1, 1, 1, 1, 1, 1},	/* Trying size one.  */
-    {2, 1, 2, 1, 2, 1, 2, 1},	/* Trying size two.  */
-    {2, 1, 2, 1, 2, 1, 2, 1},	/* Trying size three.  */
-    {4, 1, 2, 1, 4, 1, 2, 1},	/* Trying size four.  */
-    {4, 1, 2, 1, 4, 1, 2, 1},	/* Trying size five.  */
-    {4, 1, 2, 1, 4, 1, 2, 1},	/* Trying size six.  */
-    {4, 1, 2, 1, 4, 1, 2, 1},	/* Trying size seven.  */
-    {8, 1, 2, 1, 4, 1, 2, 1},	/* Trying size eight.  */
-  };
-
-  while (len > 0)
-    {
-      int align = addr % max_wp_len;
-      /* Four (eight on AMD64) is the maximum length a debug register
-	 can watch.  */
-      int try = (len > max_wp_len ? (max_wp_len - 1) : len - 1);
-      int size = size_try_array[try][align];
-
-      if (what == WP_COUNT)
-	{
-	  /* size_try_array[] is defined such 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.  */
-	  retval++;
-	}
-      else
-	{
-	  unsigned len_rw = i386_dr_length_and_rw_bits (size, type);
-
-	  if (what == WP_INSERT)
-	    retval = i386_dr_insert_aligned_watchpoint (state, addr, len_rw);
-	  else if (what == WP_REMOVE)
-	    retval = i386_dr_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);
-	  if (retval)
-	    break;
-	}
-
-      addr += size;
-      len -= size;
-    }
-
-  return retval;
-}
-
-/* Update the inferior debug registers state, in STATE, with the
-   new debug registers state, in NEW_STATE.  */
-
-void
-i386_dr_update_inferior_debug_regs (struct i386_debug_reg_state *state,
-				    struct i386_debug_reg_state *new_state)
-{
-  int i;
-
-  ALL_DEBUG_REGISTERS (i)
-    {
-      if (I386_DR_VACANT (new_state, i) != I386_DR_VACANT (state, i))
-	i386_dr_low_set_addr (new_state, i);
-      else
-	gdb_assert (new_state->dr_mirror[i] == state->dr_mirror[i]);
-    }
-
-  if (new_state->dr_control_mirror != state->dr_control_mirror)
-    i386_dr_low_set_control (new_state);
-
-  *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.  Return 0 on success, -1 on failure.  */
@@ -553,37 +158,8 @@ i386_insert_watchpoint (struct target_ops *self,
 {
   struct i386_debug_reg_state *state
     = i386_debug_reg_state (ptid_get_pid (inferior_ptid));
-  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 = *state;
-
-  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 (&local_state,
-						  WP_INSERT,
-						  addr, len, type);
-    }
-  else
-    {
-      unsigned len_rw = i386_dr_length_and_rw_bits (len, type);
-
-      retval = i386_dr_insert_aligned_watchpoint (&local_state,
-					       addr, len_rw);
-    }
-
-  if (retval == 0)
-    i386_dr_update_inferior_debug_regs (state, &local_state);
-
-  if (debug_hw_points)
-    i386_dr_show (state, "insert_watchpoint", addr, len, type);
 
-  return retval;
+  return i386_dr_insert_watchpoint (state, type, addr, len);
 }
 
 /* Remove a watchpoint that watched the memory region which starts at
@@ -596,34 +172,8 @@ i386_remove_watchpoint (struct target_ops *self,
 {
   struct i386_debug_reg_state *state
     = i386_debug_reg_state (ptid_get_pid (inferior_ptid));
-  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 = *state;
-
-  if (((len != 1 && len != 2 && len != 4)
-       && !(TARGET_HAS_DR_LEN_8 && len == 8))
-      || addr % len != 0)
-    {
-      retval = i386_handle_nonaligned_watchpoint (&local_state,
-						  WP_REMOVE,
-						  addr, len, type);
-    }
-  else
-    {
-      unsigned len_rw = i386_dr_length_and_rw_bits (len, type);
-
-      retval = i386_dr_remove_aligned_watchpoint (&local_state,
-					       addr, len_rw);
-    }
-
-  if (retval == 0)
-    i386_dr_update_inferior_debug_regs (state, &local_state);
-
-  if (debug_hw_points)
-    i386_dr_show (state, "remove_watchpoint", addr, len, type);
 
-  return retval;
+  return i386_dr_remove_watchpoint (state, type, addr, len);
 }
 
 /* Return non-zero if we can watch a memory region that starts at
@@ -635,13 +185,8 @@ i386_region_ok_for_watchpoint (struct target_ops *self,
 {
   struct i386_debug_reg_state *state
     = i386_debug_reg_state (ptid_get_pid (inferior_ptid));
-  int nregs;
 
-  /* Compute how many aligned watchpoints we would need to cover this
-     region.  */
-  nregs = i386_handle_nonaligned_watchpoint (state, WP_COUNT,
-					     addr, len, hw_write);
-  return nregs <= DR_NADDR ? 1 : 0;
+  return i386_dr_region_ok_for_watchpoint (state, addr, len);
 }
 
 /* If the inferior has some break/watchpoint that triggered, set the
@@ -653,77 +198,8 @@ i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
 {
   struct i386_debug_reg_state *state
     = i386_debug_reg_state (ptid_get_pid (inferior_ptid));
-  CORE_ADDR addr = 0;
-  int i;
-  int rc = 0;
-  /* The current thread's DR_STATUS.  We always need to read this to
-     check whether some watchpoint caused the trap.  */
-  unsigned status;
-  /* We need DR_CONTROL as well, but only iff DR_STATUS indicates a
-     data breakpoint trap.  Only fetch it when necessary, to avoid an
-     unnecessary extra syscall when no watchpoint triggered.  */
-  int control_p = 0;
-  unsigned control = 0;
-
-  /* In non-stop/async, threads can be running while we change the
-     global dr_mirror (and friends).  Say, we set a watchpoint, and
-     let threads resume.  Now, say you delete the watchpoint, or
-     add/remove watchpoints such that dr_mirror changes while threads
-     are running.  On targets that support non-stop,
-     inserting/deleting watchpoints updates the global dr_mirror only.
-     It does not update the real thread's debug registers; that's only
-     done prior to resume.  Instead, if threads are running when the
-     mirror changes, a temporary and transparent stop on all threads
-     is forced so they can get their copy of the debug registers
-     updated on re-resume.  Now, say, a thread hit a watchpoint before
-     having been updated with the new dr_mirror contents, and we
-     haven't yet handled the corresponding SIGTRAP.  If we trusted
-     dr_mirror below, we'd mistake the real trapped address (from the
-     last time we had updated debug registers in the thread) with
-     whatever was currently in dr_mirror.  So to fix this, dr_mirror
-     always represents intention, what we _want_ threads to have in
-     debug registers.  To get at the address and cause of the trap, we
-     need to read the state the thread still has in its debug
-     registers.
-
-     In sum, always get the current debug register values the current
-     thread has, instead of trusting the global mirror.  If the thread
-     was running when we last changed watchpoints, the mirror no
-     longer represents what was set in this thread's debug
-     registers.  */
-  status = i386_dr_low_get_status ();
-
-  ALL_DEBUG_REGISTERS (i)
-    {
-      if (!I386_DR_WATCH_HIT (status, i))
-	continue;
-
-      if (!control_p)
-	{
-	  control = i386_dr_low_get_control ();
-	  control_p = 1;
-	}
-
-      /* 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.  */
-      if (I386_DR_GET_RW_LEN (control, i) != 0)
-	{
-	  addr = i386_dr_low_get_addr (i);
-	  rc = 1;
-	  if (debug_hw_points)
-	    i386_dr_show (state, "watchpoint_hit", addr, -1, hw_write);
-	}
-    }
-
-  if (debug_hw_points && addr == 0)
-    i386_dr_show (state, "stopped_data_addr", 0, 0, hw_write);
 
-  if (rc)
-    *addr_p = addr;
-  return rc;
+  return i386_dr_stopped_data_address (state, addr_p);
 }
 
 /* Return non-zero if the inferior has some watchpoint that triggered.
@@ -732,8 +208,10 @@ i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
 static int
 i386_stopped_by_watchpoint (struct target_ops *ops)
 {
-  CORE_ADDR addr = 0;
-  return i386_stopped_data_address (ops, &addr);
+  struct i386_debug_reg_state *state
+    = i386_debug_reg_state (ptid_get_pid (inferior_ptid));
+
+  return i386_dr_stopped_by_watchpoint (state);
 }
 
 /* Insert a hardware-assisted breakpoint at BP_TGT->placed_address.
diff --git a/gdb/i386-nat.h b/gdb/i386-nat.h
index f20b013..4a213b3 100644
--- a/gdb/i386-nat.h
+++ b/gdb/i386-nat.h
@@ -27,6 +27,9 @@
 
 /* Hardware-assisted breakpoints and watchpoints.  */
 
+/* Whether or not to print the mirrored debug registers.  */
+extern int debug_hw_points;
+
 /* Add watchpoint methods to the provided target_ops.  
    Targets using i386 family debug registers for watchpoints should call
    this.  */
diff --git a/gdb/nat/i386-dregs.c b/gdb/nat/i386-dregs.c
new file mode 100644
index 0000000..517bdd4
--- /dev/null
+++ b/gdb/nat/i386-dregs.c
@@ -0,0 +1,617 @@
+/* Debug register code for the i386.
+
+   Copyright (C) 2001-2014 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 3 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, see <http://www.gnu.org/licenses/>.  */
+
+#ifdef GDBSERVER
+#include "server.h"
+#include "i386-low.h"
+#else
+#include "defs.h"
+#include "i386-nat.h"
+#include "inferior.h"
+#endif
+
+/* Support for hardware watchpoints and breakpoints using the i386
+   debug registers.
+
+   This provides several functions for inserting and removing
+   hardware-assisted breakpoints and watchpoints, testing if one or
+   more of the watchpoints triggered and at what address, checking
+   whether a given region can be watched, etc.
+
+   The functions below implement debug registers sharing by reference
+   counts, and allow to watch regions up to 16 bytes long.  */
+
+/* Support for 8-byte wide hw watchpoints.  */
+#define TARGET_HAS_DR_LEN_8 (i386_get_debug_register_length () == 8)
+
+/* 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 March 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 breakpoint.  */
+#define DR_LEN_2	(0x1 << 2) /* 2-byte region watch.  */
+#define DR_LEN_4	(0x3 << 2) /* 4-byte region watch.  */
+#define DR_LEN_8	(0x2 << 2) /* 8-byte region watch (AMD64).  */
+
+/* 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_dr_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 /* Two 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
+   backwards 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(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(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(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(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(state, i, rwlen) \
+  do { \
+    (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(dr7, i) \
+  (((dr7) \
+    >> (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(dr6, i) ((dr6) & (1 << (i)))
+
+/* Types of operations supported by i386_handle_nonaligned_watchpoint.  */
+typedef enum { WP_INSERT, WP_REMOVE, WP_COUNT } i386_wp_op_t;
+
+/* Print debugging messages.  */
+#ifndef GDBSERVER
+#define debug_printf(fmt, args...) \
+  fprintf_unfiltered (gdb_stdlog, fmt, ##args);
+#endif
+
+/* Print the values of the mirrored debug registers.  */
+
+void
+i386_dr_show (struct i386_debug_reg_state *state,
+	      const char *func, CORE_ADDR addr,
+	      int len, enum target_hw_bp_type type)
+{
+  int i;
+
+  debug_printf ("%s", func);
+  if (addr || len)
+    debug_printf (" (addr=%s, len=%d, type=%s)",
+		  phex (addr, 8), 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??"))));
+  debug_printf (":\n");
+  debug_printf ("\tCONTROL (DR7): %s          STATUS (DR6): %s\n",
+		phex (state->dr_control_mirror, 8),
+		phex (state->dr_status_mirror, 8));
+  ALL_DEBUG_REGISTERS (i)
+    {
+      debug_printf ("\
+\tDR%d: addr=0x%s, ref.count=%d  DR%d: addr=0x%s, ref.count=%d\n",
+		    i, phex (state->dr_mirror[i],
+			     i386_get_debug_register_length ()),
+		    state->dr_ref_count[i],
+		    i + 1, phex (state->dr_mirror[i + 1],
+				 i386_get_debug_register_length ()),
+		    state->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.  */
+
+unsigned
+i386_dr_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:
+	internal_error (__FILE__, __LINE__,
+			_("The i386 doesn't support "
+			  "data-read watchpoints.\n"));
+      case hw_access:
+	rw = DR_RW_READ;
+	break;
+#if 0
+	/* Not yet supported.  */
+      case hw_io_access:
+	rw = DR_RW_IORW;
+	break;
+#endif
+      default:
+	internal_error (__FILE__, __LINE__, _("\
+Invalid hardware breakpoint type %d in i386_dr_length_and_rw_bits.\n"),
+			(int) type);
+    }
+
+  switch (len)
+    {
+      case 1:
+	return (DR_LEN_1 | rw);
+      case 2:
+	return (DR_LEN_2 | rw);
+      case 4:
+	return (DR_LEN_4 | rw);
+      case 8:
+        if (TARGET_HAS_DR_LEN_8)
+ 	  return (DR_LEN_8 | rw);
+	/* ELSE FALL THROUGH */
+      default:
+	internal_error (__FILE__, __LINE__, _("\
+Invalid hardware breakpoint length %d in i386_dr_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.  */
+
+int
+i386_dr_insert_aligned_watchpoint (struct i386_debug_reg_state *state,
+				   CORE_ADDR addr, unsigned len_rw_bits)
+{
+  int i;
+
+  if (!i386_dr_low_can_set_addr () || !i386_dr_low_can_set_control ())
+    return -1;
+
+  /* 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 (state, i)
+	  && state->dr_mirror[i] == addr
+	  && I386_DR_GET_RW_LEN (state->dr_control_mirror, i) == len_rw_bits)
+	{
+	  state->dr_ref_count[i]++;
+	  return 0;
+	}
+    }
+
+  /* Next, look for a vacant debug register.  */
+  ALL_DEBUG_REGISTERS (i)
+    {
+      if (I386_DR_VACANT (state, 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.  */
+  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
+     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 (state, i);
+  state->dr_control_mirror |= DR_LOCAL_SLOWDOWN;
+  state->dr_control_mirror &= I386_DR_CONTROL_MASK;
+
+  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.  */
+
+int
+i386_dr_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 (state, i)
+	  && state->dr_mirror[i] == addr
+	  && I386_DR_GET_RW_LEN (state->dr_control_mirror, i) == len_rw_bits)
+	{
+	  if (--state->dr_ref_count[i] == 0) /* No longer in use?  */
+	    {
+	      /* Reset our mirror.  */
+	      state->dr_mirror[i] = 0;
+	      I386_DR_DISABLE (state, i);
+	    }
+	  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 (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;
+  int max_wp_len = TARGET_HAS_DR_LEN_8 ? 8 : 4;
+
+  static const int size_try_array[8][8] =
+  {
+    {1, 1, 1, 1, 1, 1, 1, 1},	/* Trying size one.  */
+    {2, 1, 2, 1, 2, 1, 2, 1},	/* Trying size two.  */
+    {2, 1, 2, 1, 2, 1, 2, 1},	/* Trying size three.  */
+    {4, 1, 2, 1, 4, 1, 2, 1},	/* Trying size four.  */
+    {4, 1, 2, 1, 4, 1, 2, 1},	/* Trying size five.  */
+    {4, 1, 2, 1, 4, 1, 2, 1},	/* Trying size six.  */
+    {4, 1, 2, 1, 4, 1, 2, 1},	/* Trying size seven.  */
+    {8, 1, 2, 1, 4, 1, 2, 1},	/* Trying size eight.  */
+  };
+
+  while (len > 0)
+    {
+      int align = addr % max_wp_len;
+      /* Four (eight on AMD64) is the maximum length a debug register
+	 can watch.  */
+      int try = (len > max_wp_len ? (max_wp_len - 1) : len - 1);
+      int size = size_try_array[try][align];
+
+      if (what == WP_COUNT)
+	{
+	  /* size_try_array[] is defined such 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.  */
+	  retval++;
+	}
+      else
+	{
+	  unsigned len_rw = i386_dr_length_and_rw_bits (size, type);
+
+	  if (what == WP_INSERT)
+	    retval = i386_dr_insert_aligned_watchpoint (state, addr, len_rw);
+	  else if (what == WP_REMOVE)
+	    retval = i386_dr_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);
+	  if (retval)
+	    break;
+	}
+
+      addr += size;
+      len -= size;
+    }
+
+  return retval;
+}
+
+/* Update the inferior debug registers state, in STATE, with the
+   new debug registers state, in NEW_STATE.  */
+
+void
+i386_dr_update_inferior_debug_regs (struct i386_debug_reg_state *state,
+				    struct i386_debug_reg_state *new_state)
+{
+  int i;
+
+  ALL_DEBUG_REGISTERS (i)
+    {
+      if (I386_DR_VACANT (new_state, i) != I386_DR_VACANT (state, i))
+	i386_dr_low_set_addr (new_state, i);
+      else
+	gdb_assert (new_state->dr_mirror[i] == state->dr_mirror[i]);
+    }
+
+  if (new_state->dr_control_mirror != state->dr_control_mirror)
+    i386_dr_low_set_control (new_state);
+
+  *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.  Return 0 on success, -1 on failure.  */
+
+int
+i386_dr_insert_watchpoint (struct i386_debug_reg_state *state,
+			   enum target_hw_bp_type type,
+			   CORE_ADDR addr, int len)
+{
+  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 = *state;
+
+  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 (&local_state,
+						  WP_INSERT,
+						  addr, len, type);
+    }
+  else
+    {
+      unsigned len_rw = i386_dr_length_and_rw_bits (len, type);
+
+      retval = i386_dr_insert_aligned_watchpoint (&local_state,
+					       addr, len_rw);
+    }
+
+  if (retval == 0)
+    i386_dr_update_inferior_debug_regs (state, &local_state);
+
+  if (debug_hw_points)
+    i386_dr_show (state, "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_dr_remove_watchpoint (struct i386_debug_reg_state *state,
+			   enum target_hw_bp_type type,
+			   CORE_ADDR addr, int len)
+{
+  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 = *state;
+
+  if (((len != 1 && len != 2 && len != 4)
+       && !(TARGET_HAS_DR_LEN_8 && len == 8))
+      || addr % len != 0)
+    {
+      retval = i386_handle_nonaligned_watchpoint (&local_state,
+						  WP_REMOVE,
+						  addr, len, type);
+    }
+  else
+    {
+      unsigned len_rw = i386_dr_length_and_rw_bits (len, type);
+
+      retval = i386_dr_remove_aligned_watchpoint (&local_state,
+					       addr, len_rw);
+    }
+
+  if (retval == 0)
+    i386_dr_update_inferior_debug_regs (state, &local_state);
+
+  if (debug_hw_points)
+    i386_dr_show (state, "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_dr_region_ok_for_watchpoint (struct i386_debug_reg_state *state,
+				  CORE_ADDR addr, int len)
+{
+  int nregs;
+
+  /* Compute how many aligned watchpoints we would need to cover this
+     region.  */
+  nregs = i386_handle_nonaligned_watchpoint (state, WP_COUNT,
+					     addr, len, hw_write);
+  return nregs <= DR_NADDR ? 1 : 0;
+}
+
+/* If the inferior has some break/watchpoint that triggered, set the
+   address associated with that break/watchpoint and return non-zero.
+   Otherwise, return zero.  */
+
+int
+i386_dr_stopped_data_address (struct i386_debug_reg_state *state,
+			      CORE_ADDR *addr_p)
+{
+  CORE_ADDR addr = 0;
+  int i;
+  int rc = 0;
+  /* The current thread's DR_STATUS.  We always need to read this to
+     check whether some watchpoint caused the trap.  */
+  unsigned status;
+  /* We need DR_CONTROL as well, but only iff DR_STATUS indicates a
+     data breakpoint trap.  Only fetch it when necessary, to avoid an
+     unnecessary extra syscall when no watchpoint triggered.  */
+  int control_p = 0;
+  unsigned control = 0;
+
+  /* In non-stop/async, threads can be running while we change the
+     global dr_mirror (and friends).  Say, we set a watchpoint, and
+     let threads resume.  Now, say you delete the watchpoint, or
+     add/remove watchpoints such that dr_mirror changes while threads
+     are running.  On targets that support non-stop,
+     inserting/deleting watchpoints updates the global dr_mirror only.
+     It does not update the real thread's debug registers; that's only
+     done prior to resume.  Instead, if threads are running when the
+     mirror changes, a temporary and transparent stop on all threads
+     is forced so they can get their copy of the debug registers
+     updated on re-resume.  Now, say, a thread hit a watchpoint before
+     having been updated with the new dr_mirror contents, and we
+     haven't yet handled the corresponding SIGTRAP.  If we trusted
+     dr_mirror below, we'd mistake the real trapped address (from the
+     last time we had updated debug registers in the thread) with
+     whatever was currently in dr_mirror.  So to fix this, dr_mirror
+     always represents intention, what we _want_ threads to have in
+     debug registers.  To get at the address and cause of the trap, we
+     need to read the state the thread still has in its debug
+     registers.
+
+     In sum, always get the current debug register values the current
+     thread has, instead of trusting the global mirror.  If the thread
+     was running when we last changed watchpoints, the mirror no
+     longer represents what was set in this thread's debug
+     registers.  */
+  status = i386_dr_low_get_status ();
+
+  ALL_DEBUG_REGISTERS (i)
+    {
+      if (!I386_DR_WATCH_HIT (status, i))
+	continue;
+
+      if (!control_p)
+	{
+	  control = i386_dr_low_get_control ();
+	  control_p = 1;
+	}
+
+      /* 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.  */
+      if (I386_DR_GET_RW_LEN (control, i) != 0)
+	{
+	  addr = i386_dr_low_get_addr (i);
+	  rc = 1;
+	  if (debug_hw_points)
+	    i386_dr_show (state, "watchpoint_hit", addr, -1, hw_write);
+	}
+    }
+
+  if (debug_hw_points && addr == 0)
+    i386_dr_show (state, "stopped_data_addr", 0, 0, hw_write);
+
+  if (rc)
+    *addr_p = addr;
+  return rc;
+}
+
+/* Return non-zero if the inferior has some watchpoint that triggered.
+   Otherwise return zero.  */
+
+int
+i386_dr_stopped_by_watchpoint (struct i386_debug_reg_state *state)
+{
+  CORE_ADDR addr = 0;
+  return i386_dr_stopped_data_address (state, &addr);
+}
-- 
1.7.1

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH 06/15] Merge printing code
  2014-06-17 14:13 [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Gary Benson
                   ` (3 preceding siblings ...)
  2014-06-17 14:40 ` [PATCH 14/15] Create nat/i386-dregs.c Gary Benson
@ 2014-06-17 14:40 ` Gary Benson
  2014-06-17 16:01   ` Pedro Alves
  2014-06-17 14:44 ` [PATCH 09/15] Merge i386_update_inferior_debug_regs Gary Benson
                   ` (10 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Gary Benson @ 2014-06-17 14:40 UTC (permalink / raw)
  To: gdb-patches

This commit synchronizes the debug printing code in i386-nat.c and
i386-low.c.

gdb/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-nat.c (debug_printf): New macro.
	(i386_get_debug_register_length): Likewise.
	(TARGET_HAS_DR_LEN_8): Use above macro.
	(i386_show_dr): Use debug_printf instead of puts_unfiltered
	and printf_unfiltered.  Use phex to format values.

gdb/gdbserver/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-low.c (i386_get_debug_register_length): New macro.
	(TARGET_HAS_DR_LEN_8): Remove conditional.  Use above macro.
	(i386_show_dr): Use debug_printf instead of fprintf.  Use
	phex to format values.
---
 gdb/ChangeLog            |    8 ++++++
 gdb/gdbserver/ChangeLog  |    7 ++++++
 gdb/gdbserver/i386-low.c |   50 ++++++++++++++++++++++-------------------
 gdb/i386-nat.c           |   55 ++++++++++++++++++++++++++--------------------
 4 files changed, 73 insertions(+), 47 deletions(-)

diff --git a/gdb/gdbserver/i386-low.c b/gdb/gdbserver/i386-low.c
index d122ff7..b154fcd 100644
--- a/gdb/gdbserver/i386-low.c
+++ b/gdb/gdbserver/i386-low.c
@@ -32,11 +32,12 @@
    The functions below implement debug registers sharing by reference
    counts, and allow to watch regions up to 16 bytes long.  */
 
-/* Support for 8-byte wide hw watchpoints.  */
-#ifndef TARGET_HAS_DR_LEN_8
+/* Debug register size, in bytes.  */
 /* NOTE: sizeof (long) == 4 on win64.  */
-#define TARGET_HAS_DR_LEN_8 (sizeof (void *) == 8)
-#endif
+#define i386_get_debug_register_length() (sizeof (void *))
+
+/* Support for 8-byte wide hw watchpoints.  */
+#define TARGET_HAS_DR_LEN_8 (i386_get_debug_register_length () == 8)
 
 /* DR7 Debug Control register fields.  */
 
@@ -176,29 +177,32 @@ i386_show_dr (struct i386_debug_reg_state *state,
 {
   int i;
 
-  fprintf (stderr, "%s", func);
+  debug_printf ("%s", func);
   if (addr || len)
-    fprintf (stderr, " (addr=%lx, len=%d, type=%s)",
-	     (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??"))));
-  fprintf (stderr, ":\n");
-  fprintf (stderr, "\tCONTROL (DR7): %08x          STATUS (DR6): %08x\n",
-	   state->dr_control_mirror, state->dr_status_mirror);
+    debug_printf (" (addr=%s, len=%d, type=%s)",
+		  phex (addr, 8), 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??"))));
+  debug_printf (":\n");
+  debug_printf ("\tCONTROL (DR7): %s          STATUS (DR6): %s\n",
+		phex (state->dr_control_mirror, 8),
+		phex (state->dr_status_mirror, 8));
   ALL_DEBUG_REGISTERS (i)
     {
-      fprintf (stderr, "\
+      debug_printf ("\
 \tDR%d: addr=0x%s, ref.count=%d  DR%d: addr=0x%s, ref.count=%d\n",
-	      i, paddress (state->dr_mirror[i]),
-	      state->dr_ref_count[i],
-	      i + 1, paddress (state->dr_mirror[i + 1]),
-	      state->dr_ref_count[i + 1]);
+		    i, phex (state->dr_mirror[i],
+			     i386_get_debug_register_length ()),
+		    state->dr_ref_count[i],
+		    i + 1, phex (state->dr_mirror[i + 1],
+				 i386_get_debug_register_length ()),
+		    state->dr_ref_count[i + 1]);
       i++;
     }
 }
diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c
index 23efb33..91d868d 100644
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -37,11 +37,19 @@
    The functions below implement debug registers sharing by reference
    counts, and allow to watch regions up to 16 bytes long.  */
 
+/* Function used for printing mirrored debug registers.  */
+#define debug_printf(fmt, args...) \
+  fprintf_unfiltered (gdb_stdlog, fmt, ##args);
+
 /* Low-level function vector.  */
 struct i386_dr_low_type i386_dr_low;
 
+/* Debug register size, in bytes.  */
+#define i386_get_debug_register_length() \
+  (i386_dr_low.debug_register_length)
+
 /* Support for 8-byte wide hw watchpoints.  */
-#define TARGET_HAS_DR_LEN_8 (i386_dr_low.debug_register_length == 8)
+#define TARGET_HAS_DR_LEN_8 (i386_get_debug_register_length () == 8)
 
 /* DR7 Debug Control register fields.  */
 
@@ -273,35 +281,34 @@ 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;
   int i;
 
-  puts_unfiltered (func);
+  debug_printf ("%s", 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): %s          STATUS (DR6): %s\n",
-		     phex (state->dr_control_mirror, 8),
-		     phex (state->dr_status_mirror, 8));
+    debug_printf (" (addr=%s, len=%d, type=%s)",
+		  phex (addr, 8), 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??"))));
+  debug_printf (":\n");
+  debug_printf ("\tCONTROL (DR7): %s          STATUS (DR6): %s\n",
+		phex (state->dr_control_mirror, 8),
+		phex (state->dr_status_mirror, 8));
   ALL_DEBUG_REGISTERS (i)
     {
-      printf_unfiltered ("\
+      debug_printf ("\
 \tDR%d: addr=0x%s, ref.count=%d  DR%d: addr=0x%s, ref.count=%d\n",
-			 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, phex (state->dr_mirror[i],
+			     i386_get_debug_register_length ()),
+		    state->dr_ref_count[i],
+		    i + 1, phex (state->dr_mirror[i + 1],
+				 i386_get_debug_register_length ()),
+		    state->dr_ref_count[i + 1]);
       i++;
     }
 }
-- 
1.7.1

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH 09/15] Merge i386_update_inferior_debug_regs
  2014-06-17 14:13 [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Gary Benson
                   ` (4 preceding siblings ...)
  2014-06-17 14:40 ` [PATCH 06/15] Merge printing code Gary Benson
@ 2014-06-17 14:44 ` Gary Benson
  2014-06-17 16:06   ` Pedro Alves
  2014-06-17 14:44 ` [PATCH 12/15] Create nat/i386-dregs.h Gary Benson
                   ` (9 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Gary Benson @ 2014-06-17 14:44 UTC (permalink / raw)
  To: gdb-patches

This commit synchronizes the i386_update_inferior_debug_regs functions
in i386-nat.c and i386-low.c.

gdb/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-nat.c (i386_update_inferior_debug_regs) <state>:
	New parameter.  All uses updated.

gdb/gdbserver/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-low.c (i386_update_inferior_debug_regs) <inf_state>:
	Renamed to state.
---
 gdb/ChangeLog            |    5 +++++
 gdb/gdbserver/ChangeLog  |   10 ++++++++++
 gdb/gdbserver/i386-low.c |   12 ++++++------
 gdb/i386-nat.c           |   17 ++++++++---------
 4 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/gdb/gdbserver/i386-low.c b/gdb/gdbserver/i386-low.c
index 54dd4b8..8c50838 100644
--- a/gdb/gdbserver/i386-low.c
+++ b/gdb/gdbserver/i386-low.c
@@ -417,27 +417,27 @@ Invalid value %d of operation in i386_handle_nonaligned_watchpoint.\n"),
   return retval;
 }
 
-/* Update the inferior debug registers state, in INF_STATE, with the
+/* Update the inferior debug registers state, in STATE, with the
    new debug registers state, in NEW_STATE.  */
 
 static void
-i386_update_inferior_debug_regs (struct i386_debug_reg_state *inf_state,
+i386_update_inferior_debug_regs (struct i386_debug_reg_state *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))
+      if (I386_DR_VACANT (new_state, i) != I386_DR_VACANT (state, i))
 	i386_dr_low_set_addr (new_state, i);
       else
-	gdb_assert (new_state->dr_mirror[i] == inf_state->dr_mirror[i]);
+	gdb_assert (new_state->dr_mirror[i] == state->dr_mirror[i]);
     }
 
-  if (new_state->dr_control_mirror != inf_state->dr_control_mirror)
+  if (new_state->dr_control_mirror != state->dr_control_mirror)
     i386_dr_low_set_control (new_state);
 
-  *inf_state = *new_state;
+  *state = *new_state;
 }
 
 /* Insert a watchpoint to watch a memory region which starts at
diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c
index 6520549..a64cdab 100644
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -526,14 +526,13 @@ 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.  */
+/* Update the inferior debug registers state, in STATE, with the
+   new debug registers state, in NEW_STATE.  */
 
 static void
-i386_update_inferior_debug_regs (struct i386_debug_reg_state *new_state)
+i386_update_inferior_debug_regs (struct i386_debug_reg_state *state,
+				 struct i386_debug_reg_state *new_state)
 {
-  struct i386_debug_reg_state *state
-    = i386_debug_reg_state (ptid_get_pid (inferior_ptid));
   int i;
 
   ALL_DEBUG_REGISTERS (i)
@@ -586,7 +585,7 @@ i386_insert_watchpoint (struct target_ops *self,
     }
 
   if (retval == 0)
-    i386_update_inferior_debug_regs (&local_state);
+    i386_update_inferior_debug_regs (state, &local_state);
 
   if (debug_hw_points)
     i386_show_dr (state, "insert_watchpoint", addr, len, type);
@@ -626,7 +625,7 @@ i386_remove_watchpoint (struct target_ops *self,
     }
 
   if (retval == 0)
-    i386_update_inferior_debug_regs (&local_state);
+    i386_update_inferior_debug_regs (state, &local_state);
 
   if (debug_hw_points)
     i386_show_dr (state, "remove_watchpoint", addr, len, type);
@@ -761,7 +760,7 @@ i386_insert_hw_breakpoint (struct target_ops *self, struct gdbarch *gdbarch,
 					       addr, len_rw) ? EBUSY : 0;
 
   if (retval == 0)
-    i386_update_inferior_debug_regs (&local_state);
+    i386_update_inferior_debug_regs (state, &local_state);
 
   if (debug_hw_points)
     i386_show_dr (state, "insert_hwbp", addr, 1, hw_execute);
@@ -787,7 +786,7 @@ i386_remove_hw_breakpoint (struct target_ops *self, struct gdbarch *gdbarch,
 					       addr, len_rw);
 
   if (retval == 0)
-    i386_update_inferior_debug_regs (&local_state);
+    i386_update_inferior_debug_regs (state, &local_state);
 
   if (debug_hw_points)
     i386_show_dr (state, "remove_hwbp", addr, 1, hw_execute);
-- 
1.7.1

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH 12/15] Create nat/i386-dregs.h
  2014-06-17 14:13 [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Gary Benson
                   ` (5 preceding siblings ...)
  2014-06-17 14:44 ` [PATCH 09/15] Merge i386_update_inferior_debug_regs Gary Benson
@ 2014-06-17 14:44 ` Gary Benson
  2014-06-17 16:48   ` Pedro Alves
  2014-06-17 14:48 ` [PATCH 11/15] Rename functions and make nonstatic as necessary Gary Benson
                   ` (8 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Gary Benson @ 2014-06-17 14:44 UTC (permalink / raw)
  To: gdb-patches

This commit moves code to be shared from i386-{nat,low}.[ch]
into a new file, nat/i386-dregs.h.

gdb/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* nat/i386-dregs.h: New file.
	* i386-nat.h (i386-dregs.h): New include.
	(DR_FIRSTADDR): Now in i386-dregs.h.
	(DR_LASTADDR): Likewise.
	(DR_NADDR): Likewise.
	(DR_STATUS): Likewise.
	(DR_CONTROL): Likewise.
	(i386_debug_reg_state): Likewise.
	* i386-nat.c (ALL_DEBUG_REGISTERS): Likewise.

gdb/gdbserver/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-low.h (i386-dregs.h): New include.
	(DR_FIRSTADDR): Now in i386-dregs.h.
	(DR_LASTADDR): Likewise.
	(DR_NADDR): Likewise.
	(DR_STATUS): Likewise.
	(DR_CONTROL): Likewise.
	(i386_debug_reg_state): Likewise.
	(i386_dr_insert_watchpoint): Likewise.
	(i386_dr_remove_watchpoint): Likewise.
	(i386_dr_region_ok_for_watchpoint): Likewise.
	(i386_dr_stopped_data_address): Likewise.
	(i386_dr_stopped_by_watchpoint): Likewise.
	* i386-low.c (ALL_DEBUG_REGISTERS): Likewise.
---
 gdb/ChangeLog            |   12 ++++
 gdb/gdbserver/ChangeLog  |   16 +++++
 gdb/gdbserver/i386-low.c |    3 -
 gdb/gdbserver/i386-low.h |   64 +---------------------
 gdb/i386-nat.c           |    3 -
 gdb/i386-nat.h           |   32 +----------
 gdb/nat/i386-dregs.h     |  138 ++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 170 insertions(+), 98 deletions(-)
 create mode 100644 gdb/nat/i386-dregs.h

diff --git a/gdb/gdbserver/i386-low.c b/gdb/gdbserver/i386-low.c
index 1c9d7aa..bd25694 100644
--- a/gdb/gdbserver/i386-low.c
+++ b/gdb/gdbserver/i386-low.c
@@ -146,9 +146,6 @@
 /* Did the watchpoint whose address is in the I'th register break?  */
 #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++)
-
 /* Types of operations supported by i386_handle_nonaligned_watchpoint.  */
 typedef enum { WP_INSERT, WP_REMOVE, WP_COUNT } i386_wp_op_t;
 
diff --git a/gdb/gdbserver/i386-low.h b/gdb/gdbserver/i386-low.h
index 3d3feb3..71f7c32 100644
--- a/gdb/gdbserver/i386-low.h
+++ b/gdb/gdbserver/i386-low.h
@@ -17,74 +17,12 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-/* Support for hardware watchpoints and breakpoints using the i386
-   debug registers.
-
-   This provides several functions for inserting and removing
-   hardware-assisted breakpoints and watchpoints, testing if one or
-   more of the watchpoints triggered and at what address, checking
-   whether a given region can be watched, etc.
-
-   The functions below implement debug registers sharing by reference
-   counts, and allow to watch regions up to 16 bytes long
-   (32 bytes on 64 bit hosts).  */
-
-/* Debug registers' indices.  */
-#define DR_FIRSTADDR 0
-#define DR_LASTADDR  3
-#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).  */
-
-/* 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];
-};
+
+#include "nat/i386-dregs.h"
 
 /* Initialize STATE.  */
 extern void i386_low_init_dregs (struct i386_debug_reg_state *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.  */
-extern int i386_dr_insert_watchpoint (struct i386_debug_reg_state *state,
-				      enum target_hw_bp_type type,
-				      CORE_ADDR addr,
-				      int len);
-
-/* 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_dr_remove_watchpoint (struct i386_debug_reg_state *state,
-				      enum target_hw_bp_type type,
-				      CORE_ADDR addr,
-				      int len);
-
-/* Return non-zero if we can watch a memory region that starts at
-   address ADDR and whose length is LEN bytes.  */
-extern int i386_dr_region_ok_for_watchpoint (struct i386_debug_reg_state *state,
-					     CORE_ADDR addr, int len);
-
-/* If the inferior has some break/watchpoint that triggered, set the
-   address associated with that break/watchpoint and return true.
-   Otherwise, return false.  */
-extern int i386_dr_stopped_data_address (struct i386_debug_reg_state *state,
-					 CORE_ADDR *addr_p);
-
-/* Return true if the inferior has some watchpoint that triggered.
-   Otherwise return false.  */
-extern int i386_dr_stopped_by_watchpoint (struct i386_debug_reg_state *state);
 \f
 /* Each target needs to provide several low-level functions
    that will be called to insert watchpoints and hardware breakpoints
diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c
index 86e655f..1dfa9ee 100644
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -171,9 +171,6 @@ struct i386_dr_low_type i386_dr_low;
 /* Did the watchpoint whose address is in the I'th register break?  */
 #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++)
-
 /* Per-process data.  We don't bind this to a per-inferior registry
    because of targets like x86 GNU/Linux that need to keep track of
    processes that aren't bound to any inferior (e.g., fork children,
diff --git a/gdb/i386-nat.h b/gdb/i386-nat.h
index ae2f1a7..1f4130c 100644
--- a/gdb/i386-nat.h
+++ b/gdb/i386-nat.h
@@ -23,6 +23,8 @@
 #ifndef I386_NAT_H
 #define I386_NAT_H 1
 
+#include "nat/i386-dregs.h"
+
 /* Hardware-assisted breakpoints and watchpoints.  */
 
 /* Add watchpoint methods to the provided target_ops.  
@@ -34,12 +36,7 @@ extern void i386_use_watchpoints (struct target_ops *);
 /* Support for hardware watchpoints and breakpoints using the i386
    debug registers.
 
-   This provides several functions for inserting and removing
-   hardware-assisted breakpoints and watchpoints, testing if one or
-   more of the watchpoints triggered and at what address, checking
-   whether a given region can be watched, etc.
-
-   In addition, each target should provide several low-level functions
+   Each target should provide several low-level functions
    regrouped into i386_dr_low_type struct below.  These functions
    that will be called to insert watchpoints and hardware breakpoints
    into the inferior, remove them, and check their status.  These
@@ -76,29 +73,6 @@ struct i386_dr_low_type
 
 extern struct i386_dr_low_type i386_dr_low;
 
-/* Debug registers' indices.  */
-#define DR_FIRSTADDR 0
-#define DR_LASTADDR  3
-#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).  */
-
-/* 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];
-};
-
 /* Use this function to set i386_dr_low debug_register_length field
    rather than setting it directly to check that the length is only
    set once.  It also enables the 'maint set/show show-debug-regs' 
diff --git a/gdb/nat/i386-dregs.h b/gdb/nat/i386-dregs.h
new file mode 100644
index 0000000..c17c6ae
--- /dev/null
+++ b/gdb/nat/i386-dregs.h
@@ -0,0 +1,138 @@
+/* Debug register code for the i386.
+
+   Copyright (C) 2009-2014 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 3 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, see <http://www.gnu.org/licenses/>.  */
+
+/* Support for hardware watchpoints and breakpoints using the i386
+   debug registers.
+
+   This provides several functions for inserting and removing
+   hardware-assisted breakpoints and watchpoints, testing if one or
+   more of the watchpoints triggered and at what address, checking
+   whether a given region can be watched, etc.
+
+   The functions below implement debug registers sharing by reference
+   counts, and allow to watch regions up to 16 bytes long
+   (32 bytes on 64 bit hosts).  */
+
+#ifndef I386_DREGS_H
+#define I386_DREGS_H 1
+
+/* Forward declaration.  */
+enum target_hw_bp_type;
+
+/* Debug registers' indices.  */
+#define DR_FIRSTADDR 0
+#define DR_LASTADDR  3
+#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).  */
+
+/* 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];
+};
+
+/* A macro to loop over all debug registers.  */
+#define ALL_DEBUG_REGISTERS(i)	for (i = 0; i < DR_NADDR; i++)
+\f
+/* High-level functions.  */
+
+/* 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_dr_insert_watchpoint (struct i386_debug_reg_state *state,
+				      enum target_hw_bp_type type,
+				      CORE_ADDR addr,
+				      int len);
+
+/* 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_dr_remove_watchpoint (struct i386_debug_reg_state *state,
+				      enum target_hw_bp_type type,
+				      CORE_ADDR addr,
+				      int len);
+
+/* Return non-zero if we can watch a memory region that starts at
+   address ADDR and whose length is LEN bytes.  */
+extern int i386_dr_region_ok_for_watchpoint (struct i386_debug_reg_state *state,
+					     CORE_ADDR addr, int len);
+
+/* If the inferior has some break/watchpoint that triggered, set the
+   address associated with that break/watchpoint and return true.
+   Otherwise, return false.  */
+extern int i386_dr_stopped_data_address (struct i386_debug_reg_state *state,
+					 CORE_ADDR *addr_p);
+
+/* Return true if the inferior has some watchpoint that triggered.
+   Otherwise return false.  */
+extern int i386_dr_stopped_by_watchpoint (struct i386_debug_reg_state *state);
+\f
+/* Low-level functions.  */
+
+/* Print the values of the mirrored debug registers.  */
+
+extern void i386_dr_show (struct i386_debug_reg_state *state,
+			  const char *func, CORE_ADDR addr,
+			  int len, enum target_hw_bp_type type);
+
+/* 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.  */
+
+extern unsigned i386_dr_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 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.  */
+
+extern int i386_dr_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
+   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.  */
+
+extern int i386_dr_remove_aligned_watchpoint (struct i386_debug_reg_state *state,
+					      CORE_ADDR addr,
+					      unsigned len_rw_bits);
+
+/* Update the inferior debug registers state, in STATE, with the
+   new debug registers state, in NEW_STATE.  */
+
+extern void i386_dr_update_inferior_debug_regs (struct i386_debug_reg_state *state,
+						struct i386_debug_reg_state *new_state);
+
+#endif /* I386_DREGS_H */
-- 
1.7.1

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH 07/15] Merge error handling
  2014-06-17 14:13 [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Gary Benson
                   ` (7 preceding siblings ...)
  2014-06-17 14:48 ` [PATCH 11/15] Rename functions and make nonstatic as necessary Gary Benson
@ 2014-06-17 14:48 ` Gary Benson
  2014-06-17 16:02   ` Pedro Alves
  2014-06-17 14:53 ` [PATCH 10/15] Abstract i386_dr_low access Gary Benson
                   ` (6 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Gary Benson @ 2014-06-17 14:48 UTC (permalink / raw)
  To: gdb-patches

This commit makes all error handling in i386-low.c use internal_error
rather than fatal and error.

gdb/gdbserver/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-low.c (i386_length_and_rw_bits): Use internal_error
	instead of fatal and error.
	(i386_handle_nonaligned_watchpoint): Likewise.
---
 gdb/gdbserver/ChangeLog  |    6 ++++++
 gdb/gdbserver/i386-low.c |   19 ++++++++++---------
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/gdb/gdbserver/i386-low.c b/gdb/gdbserver/i386-low.c
index b154fcd..54dd4b8 100644
--- a/gdb/gdbserver/i386-low.c
+++ b/gdb/gdbserver/i386-low.c
@@ -225,7 +225,9 @@ i386_length_and_rw_bits (int len, enum target_hw_bp_type type)
 	rw = DR_RW_WRITE;
 	break;
       case hw_read:
-	fatal ("The i386 doesn't support data-read watchpoints.\n");
+	internal_error (__FILE__, __LINE__,
+			_("The i386 doesn't support "
+			  "data-read watchpoints.\n"));
       case hw_access:
 	rw = DR_RW_READ;
 	break;
@@ -236,8 +238,8 @@ i386_length_and_rw_bits (int len, enum target_hw_bp_type type)
 	break;
 #endif
       default:
-	error ("\
-Invalid hardware breakpoint type %d in i386_length_and_rw_bits.\n",
+	internal_error (__FILE__, __LINE__, _("\
+Invalid hardware breakpoint type %d in i386_length_and_rw_bits.\n"),
 			(int) type);
     }
 
@@ -254,8 +256,8 @@ Invalid hardware breakpoint type %d in i386_length_and_rw_bits.\n",
  	  return (DR_LEN_8 | rw);
 	/* ELSE FALL THROUGH */
       default:
-	error ("\
-Invalid hardware breakpoint length %d in i386_length_and_rw_bits.\n", len);
+	internal_error (__FILE__, __LINE__, _("\
+Invalid hardware breakpoint length %d in i386_length_and_rw_bits.\n"), len);
     }
 }
 
@@ -401,10 +403,9 @@ i386_handle_nonaligned_watchpoint (struct i386_debug_reg_state *state,
 	  else if (what == WP_REMOVE)
 	    retval = i386_remove_aligned_watchpoint (state, addr, len_rw);
 	  else
-	    fatal ("\
-Invalid value %d of operation in i386_handle_nonaligned_watchpoint.\n",
-		   (int) what);
-
+	    internal_error (__FILE__, __LINE__, _("\
+Invalid value %d of operation in i386_handle_nonaligned_watchpoint.\n"),
+			    (int) what);
 	  if (retval)
 	    break;
 	}
-- 
1.7.1

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH 11/15] Rename functions and make nonstatic as necessary
  2014-06-17 14:13 [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Gary Benson
                   ` (6 preceding siblings ...)
  2014-06-17 14:44 ` [PATCH 12/15] Create nat/i386-dregs.h Gary Benson
@ 2014-06-17 14:48 ` Gary Benson
  2014-06-17 16:13   ` Pedro Alves
  2014-06-18 10:25   ` Siva Chandra
  2014-06-17 14:48 ` [PATCH 07/15] Merge error handling Gary Benson
                   ` (7 subsequent siblings)
  15 siblings, 2 replies; 45+ messages in thread
From: Gary Benson @ 2014-06-17 14:48 UTC (permalink / raw)
  To: gdb-patches

This commit renames the functions that are to be shared.
Functions to be shared that were static are made nonstatic.

gdb/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-nat.c (i386_show_dr): Renamed to
	i386_dr_show and made nonstatic.  All uses updated.
	(i386_length_and_rw_bits): Renamed to
	i386_dr_length_and_rw_bits and made nonstatic.
	All uses updated.
	(i386_insert_aligned_watchpoint): Renamed to
	i386_dr_insert_aligned_watchpoint and made nonstatic.
	All uses updated.
	(i386_remove_aligned_watchpoint): Renamed to
	i386_dr_remove_aligned_watchpoint and made nonstatic.
	All uses updated.
	(i386_update_inferior_debug_regs): Renamed to
	i386_dr_update_inferior_debug_regs and made nonstatic.
	All uses updated.

gdb/gdbserver/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-low.h (i386_low_insert_watchpoint): Renamed to
	i386_dr_insert_watchpoint.
	(i386_low_remove_watchpoint): Renamed to
	i386_dr_remove_watchpoint.
	(i386_low_region_ok_for_watchpoint): Renamed to
	i386_dr_region_ok_for_watchpoint.
	(i386_low_stopped_data_address): Renamed to
	i386_dr_stopped_data_address.
	(i386_low_stopped_by_watchpoint): Renamed to
	i386_dr_stopped_by_watchpoint.
	* i386-low.c (i386_show_dr): Renamed to
	i386_dr_show and made nonstatic.  All uses updated.
	(i386_length_and_rw_bits): Renamed to
	i386_dr_length_and_rw_bits and made nonstatic.
	All uses updated.
	(i386_insert_aligned_watchpoint): Renamed to
	i386_dr_insert_aligned_watchpoint and made nonstatic.
	All uses updated.
	(i386_remove_aligned_watchpoint): Renamed to
	i386_dr_remove_aligned_watchpoint and made nonstatic.
	All uses updated.
	(i386_update_inferior_debug_regs): Renamed to
	i386_dr_update_inferior_debug_regs and made nonstatic.
	All uses updated.
	(i386_low_insert_watchpoint): Renamed to
	i386_dr_insert_watchpoint.  All uses updated.
	(i386_low_remove_watchpoint): Renamed to
	i386_dr_remove_watchpoint.  All uses updated.
	(i386_low_region_ok_for_watchpoint): Renamed to
	i386_dr_region_ok_for_watchpoint.  All uses updated.
	(i386_low_stopped_data_address): Renamed to
	i386_dr_stopped_data_address.  All uses updated.
	(i386_low_stopped_by_watchpoint): Renamed to
	i386_dr_stopped_by_watchpoint.  All uses updated.
---
 gdb/ChangeLog                  |   17 ++++++++
 gdb/gdbserver/ChangeLog        |   37 ++++++++++++++++++
 gdb/gdbserver/i386-low.c       |   82 ++++++++++++++++++++--------------------
 gdb/gdbserver/i386-low.h       |   26 ++++++------
 gdb/gdbserver/linux-x86-low.c  |   12 +++---
 gdb/gdbserver/win32-i386-low.c |   12 +++---
 gdb/i386-nat.c                 |   79 +++++++++++++++++++-------------------
 7 files changed, 160 insertions(+), 105 deletions(-)

diff --git a/gdb/gdbserver/i386-low.c b/gdb/gdbserver/i386-low.c
index 8d6f26c..1c9d7aa 100644
--- a/gdb/gdbserver/i386-low.c
+++ b/gdb/gdbserver/i386-low.c
@@ -78,7 +78,7 @@
 
    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.  */
+   i386_dr_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 /* Two enable bits per debug register.  */
@@ -173,8 +173,8 @@ i386_low_init_dregs (struct i386_debug_reg_state *state)
 
 /* Print the values of the mirrored debug registers.  */
 
-static void
-i386_show_dr (struct i386_debug_reg_state *state,
+void
+i386_dr_show (struct i386_debug_reg_state *state,
 	      const char *func, CORE_ADDR addr,
 	      int len, enum target_hw_bp_type type)
 {
@@ -214,8 +214,8 @@ i386_show_dr (struct i386_debug_reg_state *state,
    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
+i386_dr_length_and_rw_bits (int len, enum target_hw_bp_type type)
 {
   unsigned rw;
 
@@ -242,7 +242,7 @@ i386_length_and_rw_bits (int len, enum target_hw_bp_type type)
 #endif
       default:
 	internal_error (__FILE__, __LINE__, _("\
-Invalid hardware breakpoint type %d in i386_length_and_rw_bits.\n"),
+Invalid hardware breakpoint type %d in i386_dr_length_and_rw_bits.\n"),
 			(int) type);
     }
 
@@ -260,7 +260,7 @@ Invalid hardware breakpoint type %d in i386_length_and_rw_bits.\n"),
 	/* ELSE FALL THROUGH */
       default:
 	internal_error (__FILE__, __LINE__, _("\
-Invalid hardware breakpoint length %d in i386_length_and_rw_bits.\n"), len);
+Invalid hardware breakpoint length %d in i386_dr_length_and_rw_bits.\n"), len);
     }
 }
 
@@ -270,9 +270,9 @@ Invalid hardware breakpoint length %d in i386_length_and_rw_bits.\n"), len);
    type of the region to be watched by this watchpoint.  Return 0 on
    success, -1 on failure.  */
 
-static int
-i386_insert_aligned_watchpoint (struct i386_debug_reg_state *state,
-				CORE_ADDR addr, unsigned len_rw_bits)
+int
+i386_dr_insert_aligned_watchpoint (struct i386_debug_reg_state *state,
+				   CORE_ADDR addr, unsigned len_rw_bits)
 {
   int i;
 
@@ -330,9 +330,9 @@ i386_insert_aligned_watchpoint (struct i386_debug_reg_state *state,
    type of the region watched by this watchpoint.  Return 0 on
    success, -1 on failure.  */
 
-static int
-i386_remove_aligned_watchpoint (struct i386_debug_reg_state *state,
-				CORE_ADDR addr, unsigned len_rw_bits)
+int
+i386_dr_remove_aligned_watchpoint (struct i386_debug_reg_state *state,
+				   CORE_ADDR addr, unsigned len_rw_bits)
 {
   int i, retval = -1;
 
@@ -402,12 +402,12 @@ i386_handle_nonaligned_watchpoint (struct i386_debug_reg_state *state,
 	}
       else
 	{
-	  unsigned len_rw = i386_length_and_rw_bits (size, type);
+	  unsigned len_rw = i386_dr_length_and_rw_bits (size, type);
 
 	  if (what == WP_INSERT)
-	    retval = i386_insert_aligned_watchpoint (state, addr, len_rw);
+	    retval = i386_dr_insert_aligned_watchpoint (state, addr, len_rw);
 	  else if (what == WP_REMOVE)
-	    retval = i386_remove_aligned_watchpoint (state, addr, len_rw);
+	    retval = i386_dr_remove_aligned_watchpoint (state, addr, len_rw);
 	  else
 	    internal_error (__FILE__, __LINE__, _("\
 Invalid value %d of operation in i386_handle_nonaligned_watchpoint.\n"),
@@ -426,9 +426,9 @@ Invalid value %d of operation in i386_handle_nonaligned_watchpoint.\n"),
 /* Update the inferior debug registers state, in STATE, with the
    new debug registers state, in NEW_STATE.  */
 
-static void
-i386_update_inferior_debug_regs (struct i386_debug_reg_state *state,
-				 struct i386_debug_reg_state *new_state)
+void
+i386_dr_update_inferior_debug_regs (struct i386_debug_reg_state *state,
+				    struct i386_debug_reg_state *new_state)
 {
   int i;
 
@@ -451,9 +451,9 @@ i386_update_inferior_debug_regs (struct i386_debug_reg_state *state,
    of the type TYPE.  Return 0 on success, -1 on failure.  */
 
 int
-i386_low_insert_watchpoint (struct i386_debug_reg_state *state,
-			    enum target_hw_bp_type type,
-			    CORE_ADDR addr, int len)
+i386_dr_insert_watchpoint (struct i386_debug_reg_state *state,
+			   enum target_hw_bp_type type,
+			   CORE_ADDR addr, int len)
 {
   int retval;
   /* Work on a local copy of the debug registers, and on success,
@@ -473,17 +473,17 @@ i386_low_insert_watchpoint (struct i386_debug_reg_state *state,
     }
   else
     {
-      unsigned len_rw = i386_length_and_rw_bits (len, type);
+      unsigned len_rw = i386_dr_length_and_rw_bits (len, type);
 
-      retval = i386_insert_aligned_watchpoint (&local_state,
+      retval = i386_dr_insert_aligned_watchpoint (&local_state,
 					       addr, len_rw);
     }
 
   if (retval == 0)
-    i386_update_inferior_debug_regs (state, &local_state);
+    i386_dr_update_inferior_debug_regs (state, &local_state);
 
   if (debug_hw_points)
-    i386_show_dr (state, "insert_watchpoint", addr, len, type);
+    i386_dr_show (state, "insert_watchpoint", addr, len, type);
 
   return retval;
 }
@@ -493,9 +493,9 @@ i386_low_insert_watchpoint (struct i386_debug_reg_state *state,
    type TYPE.  Return 0 on success, -1 on failure.  */
 
 int
-i386_low_remove_watchpoint (struct i386_debug_reg_state *state,
-			    enum target_hw_bp_type type,
-			    CORE_ADDR addr, int len)
+i386_dr_remove_watchpoint (struct i386_debug_reg_state *state,
+			   enum target_hw_bp_type type,
+			   CORE_ADDR addr, int len)
 {
   int retval;
   /* Work on a local copy of the debug registers, and on success,
@@ -512,17 +512,17 @@ i386_low_remove_watchpoint (struct i386_debug_reg_state *state,
     }
   else
     {
-      unsigned len_rw = i386_length_and_rw_bits (len, type);
+      unsigned len_rw = i386_dr_length_and_rw_bits (len, type);
 
-      retval = i386_remove_aligned_watchpoint (&local_state,
+      retval = i386_dr_remove_aligned_watchpoint (&local_state,
 					       addr, len_rw);
     }
 
   if (retval == 0)
-    i386_update_inferior_debug_regs (state, &local_state);
+    i386_dr_update_inferior_debug_regs (state, &local_state);
 
   if (debug_hw_points)
-    i386_show_dr (state, "remove_watchpoint", addr, len, type);
+    i386_dr_show (state, "remove_watchpoint", addr, len, type);
 
   return retval;
 }
@@ -531,8 +531,8 @@ i386_low_remove_watchpoint (struct i386_debug_reg_state *state,
    address ADDR and whose length is LEN bytes.  */
 
 int
-i386_low_region_ok_for_watchpoint (struct i386_debug_reg_state *state,
-				   CORE_ADDR addr, int len)
+i386_dr_region_ok_for_watchpoint (struct i386_debug_reg_state *state,
+				  CORE_ADDR addr, int len)
 {
   int nregs;
 
@@ -548,8 +548,8 @@ i386_low_region_ok_for_watchpoint (struct i386_debug_reg_state *state,
    Otherwise, return zero.  */
 
 int
-i386_low_stopped_data_address (struct i386_debug_reg_state *state,
-			       CORE_ADDR *addr_p)
+i386_dr_stopped_data_address (struct i386_debug_reg_state *state,
+			      CORE_ADDR *addr_p)
 {
   CORE_ADDR addr = 0;
   int i;
@@ -612,12 +612,12 @@ i386_low_stopped_data_address (struct i386_debug_reg_state *state,
 	  addr = i386_dr_low_get_addr (i);
 	  rc = 1;
 	  if (debug_hw_points)
-	    i386_show_dr (state, "watchpoint_hit", addr, -1, hw_write);
+	    i386_dr_show (state, "watchpoint_hit", addr, -1, hw_write);
 	}
     }
 
   if (debug_hw_points && addr == 0)
-    i386_show_dr (state, "stopped_data_addr", 0, 0, hw_write);
+    i386_dr_show (state, "stopped_data_addr", 0, 0, hw_write);
 
   if (rc)
     *addr_p = addr;
@@ -628,8 +628,8 @@ i386_low_stopped_data_address (struct i386_debug_reg_state *state,
    Otherwise return zero.  */
 
 int
-i386_low_stopped_by_watchpoint (struct i386_debug_reg_state *state)
+i386_dr_stopped_by_watchpoint (struct i386_debug_reg_state *state)
 {
   CORE_ADDR addr = 0;
-  return i386_low_stopped_data_address (state, &addr);
+  return i386_dr_stopped_data_address (state, &addr);
 }
diff --git a/gdb/gdbserver/i386-low.h b/gdb/gdbserver/i386-low.h
index 681ade0..3d3feb3 100644
--- a/gdb/gdbserver/i386-low.h
+++ b/gdb/gdbserver/i386-low.h
@@ -58,33 +58,33 @@ extern void i386_low_init_dregs (struct i386_debug_reg_state *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.  */
-extern int i386_low_insert_watchpoint (struct i386_debug_reg_state *state,
-				       enum target_hw_bp_type type,
-				       CORE_ADDR addr,
-				       int len);
+extern int i386_dr_insert_watchpoint (struct i386_debug_reg_state *state,
+				      enum target_hw_bp_type type,
+				      CORE_ADDR addr,
+				      int len);
 
 /* 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_low_remove_watchpoint (struct i386_debug_reg_state *state,
-				       enum target_hw_bp_type type,
-				       CORE_ADDR addr,
-				       int len);
+extern int i386_dr_remove_watchpoint (struct i386_debug_reg_state *state,
+				      enum target_hw_bp_type type,
+				      CORE_ADDR addr,
+				      int len);
 
 /* Return non-zero if we can watch a memory region that starts at
    address ADDR and whose length is LEN bytes.  */
-extern int i386_low_region_ok_for_watchpoint (struct i386_debug_reg_state *state,
-					      CORE_ADDR addr, int len);
+extern int i386_dr_region_ok_for_watchpoint (struct i386_debug_reg_state *state,
+					     CORE_ADDR addr, int len);
 
 /* If the inferior has some break/watchpoint that triggered, set the
    address associated with that break/watchpoint and return true.
    Otherwise, return false.  */
-extern int i386_low_stopped_data_address (struct i386_debug_reg_state *state,
-					  CORE_ADDR *addr_p);
+extern int i386_dr_stopped_data_address (struct i386_debug_reg_state *state,
+					 CORE_ADDR *addr_p);
 
 /* Return true if the inferior has some watchpoint that triggered.
    Otherwise return false.  */
-extern int i386_low_stopped_by_watchpoint (struct i386_debug_reg_state *state);
+extern int i386_dr_stopped_by_watchpoint (struct i386_debug_reg_state *state);
 \f
 /* Each target needs to provide several low-level functions
    that will be called to insert watchpoints and hardware breakpoints
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index e478394..b8a0006 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -681,7 +681,7 @@ x86_insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
 	struct i386_debug_reg_state *state
 	  = &proc->private->arch_private->debug_reg_state;
 
-	return i386_low_insert_watchpoint (state, hw_type, addr, size);
+	return i386_dr_insert_watchpoint (state, hw_type, addr, size);
       }
 
     default:
@@ -710,7 +710,7 @@ x86_remove_point (enum raw_bkpt_type type, CORE_ADDR addr,
 	struct i386_debug_reg_state *state
 	  = &proc->private->arch_private->debug_reg_state;
 
-	return i386_low_remove_watchpoint (state, hw_type, addr, size);
+	return i386_dr_remove_watchpoint (state, hw_type, addr, size);
       }
     default:
       /* Unsupported.  */
@@ -722,7 +722,7 @@ static int
 x86_stopped_by_watchpoint (void)
 {
   struct process_info *proc = current_process ();
-  return i386_low_stopped_by_watchpoint (&proc->private->arch_private->debug_reg_state);
+  return i386_dr_stopped_by_watchpoint (&proc->private->arch_private->debug_reg_state);
 }
 
 static CORE_ADDR
@@ -730,8 +730,8 @@ x86_stopped_data_address (void)
 {
   struct process_info *proc = current_process ();
   CORE_ADDR addr;
-  if (i386_low_stopped_data_address (&proc->private->arch_private->debug_reg_state,
-				     &addr))
+  if (i386_dr_stopped_data_address (&proc->private->arch_private->debug_reg_state,
+				    &addr))
     return addr;
   return 0;
 }
@@ -784,7 +784,7 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
 
 	    /* If we're setting a watchpoint, any change the inferior
 	       had done itself to the debug registers needs to be
-	       discarded, otherwise, i386_low_stopped_data_address can
+	       discarded, otherwise, i386_dr_stopped_data_address can
 	       get confused.  */
 	    clear_status = 1;
 	  }
diff --git a/gdb/gdbserver/win32-i386-low.c b/gdb/gdbserver/win32-i386-low.c
index 08242aa..09160dd 100644
--- a/gdb/gdbserver/win32-i386-low.c
+++ b/gdb/gdbserver/win32-i386-low.c
@@ -122,8 +122,8 @@ i386_insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
 	enum target_hw_bp_type hw_type
 	  = raw_bkpt_type_to_target_hw_bp_type (type);
 
-	return i386_low_insert_watchpoint (&debug_reg_state,
-					   hw_type, addr, size);
+	return i386_dr_insert_watchpoint (&debug_reg_state,
+					  hw_type, addr, size);
       }
     default:
       /* Unsupported.  */
@@ -143,8 +143,8 @@ i386_remove_point (enum raw_bkpt_type type, CORE_ADDR addr,
 	enum target_hw_bp_type hw_type
 	  = raw_bkpt_type_to_target_hw_bp_type (type);
 
-	return i386_low_remove_watchpoint (&debug_reg_state,
-					   hw_type, addr, size);
+	return i386_dr_remove_watchpoint (&debug_reg_state,
+					  hw_type, addr, size);
       }
     default:
       /* Unsupported.  */
@@ -155,14 +155,14 @@ i386_remove_point (enum raw_bkpt_type type, CORE_ADDR addr,
 static int
 i386_stopped_by_watchpoint (void)
 {
-  return i386_low_stopped_by_watchpoint (&debug_reg_state);
+  return i386_dr_stopped_by_watchpoint (&debug_reg_state);
 }
 
 static CORE_ADDR
 i386_stopped_data_address (void)
 {
   CORE_ADDR addr;
-  if (i386_low_stopped_data_address (&debug_reg_state, &addr))
+  if (i386_dr_stopped_data_address (&debug_reg_state, &addr))
     return addr;
   return 0;
 }
diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c
index 4282535..86e655f 100644
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -103,7 +103,7 @@ struct i386_dr_low_type i386_dr_low;
 
    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.  */
+   i386_dr_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 /* Two enable bits per debug register.  */
@@ -289,8 +289,8 @@ i386_cleanup_dregs (void)
 
 /* Print the values of the mirrored debug registers.  */
 
-static void
-i386_show_dr (struct i386_debug_reg_state *state,
+void
+i386_dr_show (struct i386_debug_reg_state *state,
 	      const char *func, CORE_ADDR addr,
 	      int len, enum target_hw_bp_type type)
 {
@@ -330,8 +330,8 @@ i386_show_dr (struct i386_debug_reg_state *state,
    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
+i386_dr_length_and_rw_bits (int len, enum target_hw_bp_type type)
 {
   unsigned rw;
 
@@ -358,7 +358,7 @@ i386_length_and_rw_bits (int len, enum target_hw_bp_type type)
 #endif
       default:
 	internal_error (__FILE__, __LINE__, _("\
-Invalid hardware breakpoint type %d in i386_length_and_rw_bits.\n"),
+Invalid hardware breakpoint type %d in i386_dr_length_and_rw_bits.\n"),
 			(int) type);
     }
 
@@ -376,7 +376,7 @@ Invalid hardware breakpoint type %d in i386_length_and_rw_bits.\n"),
 	/* ELSE FALL THROUGH */
       default:
 	internal_error (__FILE__, __LINE__, _("\
-Invalid hardware breakpoint length %d in i386_length_and_rw_bits.\n"), len);
+Invalid hardware breakpoint length %d in i386_dr_length_and_rw_bits.\n"), len);
     }
 }
 
@@ -386,9 +386,9 @@ Invalid hardware breakpoint length %d in i386_length_and_rw_bits.\n"), len);
    type of the region to be watched by this watchpoint.  Return 0 on
    success, -1 on failure.  */
 
-static int
-i386_insert_aligned_watchpoint (struct i386_debug_reg_state *state,
-				CORE_ADDR addr, unsigned len_rw_bits)
+int
+i386_dr_insert_aligned_watchpoint (struct i386_debug_reg_state *state,
+				   CORE_ADDR addr, unsigned len_rw_bits)
 {
   int i;
 
@@ -446,9 +446,9 @@ i386_insert_aligned_watchpoint (struct i386_debug_reg_state *state,
    type of the region watched by this watchpoint.  Return 0 on
    success, -1 on failure.  */
 
-static int
-i386_remove_aligned_watchpoint (struct i386_debug_reg_state *state,
-				CORE_ADDR addr, unsigned len_rw_bits)
+int
+i386_dr_remove_aligned_watchpoint (struct i386_debug_reg_state *state,
+				   CORE_ADDR addr, unsigned len_rw_bits)
 {
   int i, retval = -1;
 
@@ -518,12 +518,12 @@ i386_handle_nonaligned_watchpoint (struct i386_debug_reg_state *state,
 	}
       else
 	{
-	  unsigned len_rw = i386_length_and_rw_bits (size, type);
+	  unsigned len_rw = i386_dr_length_and_rw_bits (size, type);
 
 	  if (what == WP_INSERT)
-	    retval = i386_insert_aligned_watchpoint (state, addr, len_rw);
+	    retval = i386_dr_insert_aligned_watchpoint (state, addr, len_rw);
 	  else if (what == WP_REMOVE)
-	    retval = i386_remove_aligned_watchpoint (state, addr, len_rw);
+	    retval = i386_dr_remove_aligned_watchpoint (state, addr, len_rw);
 	  else
 	    internal_error (__FILE__, __LINE__, _("\
 Invalid value %d of operation in i386_handle_nonaligned_watchpoint.\n"),
@@ -542,9 +542,9 @@ Invalid value %d of operation in i386_handle_nonaligned_watchpoint.\n"),
 /* Update the inferior debug registers state, in STATE, with the
    new debug registers state, in NEW_STATE.  */
 
-static void
-i386_update_inferior_debug_regs (struct i386_debug_reg_state *state,
-				 struct i386_debug_reg_state *new_state)
+void
+i386_dr_update_inferior_debug_regs (struct i386_debug_reg_state *state,
+				    struct i386_debug_reg_state *new_state)
 {
   int i;
 
@@ -591,17 +591,17 @@ i386_insert_watchpoint (struct target_ops *self,
     }
   else
     {
-      unsigned len_rw = i386_length_and_rw_bits (len, type);
+      unsigned len_rw = i386_dr_length_and_rw_bits (len, type);
 
-      retval = i386_insert_aligned_watchpoint (&local_state,
+      retval = i386_dr_insert_aligned_watchpoint (&local_state,
 					       addr, len_rw);
     }
 
   if (retval == 0)
-    i386_update_inferior_debug_regs (state, &local_state);
+    i386_dr_update_inferior_debug_regs (state, &local_state);
 
   if (debug_hw_points)
-    i386_show_dr (state, "insert_watchpoint", addr, len, type);
+    i386_dr_show (state, "insert_watchpoint", addr, len, type);
 
   return retval;
 }
@@ -631,17 +631,17 @@ i386_remove_watchpoint (struct target_ops *self,
     }
   else
     {
-      unsigned len_rw = i386_length_and_rw_bits (len, type);
+      unsigned len_rw = i386_dr_length_and_rw_bits (len, type);
 
-      retval = i386_remove_aligned_watchpoint (&local_state,
+      retval = i386_dr_remove_aligned_watchpoint (&local_state,
 					       addr, len_rw);
     }
 
   if (retval == 0)
-    i386_update_inferior_debug_regs (state, &local_state);
+    i386_dr_update_inferior_debug_regs (state, &local_state);
 
   if (debug_hw_points)
-    i386_show_dr (state, "remove_watchpoint", addr, len, type);
+    i386_dr_show (state, "remove_watchpoint", addr, len, type);
 
   return retval;
 }
@@ -734,12 +734,12 @@ i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
 	  addr = i386_dr_low_get_addr (i);
 	  rc = 1;
 	  if (debug_hw_points)
-	    i386_show_dr (state, "watchpoint_hit", addr, -1, hw_write);
+	    i386_dr_show (state, "watchpoint_hit", addr, -1, hw_write);
 	}
     }
 
   if (debug_hw_points && addr == 0)
-    i386_show_dr (state, "stopped_data_addr", 0, 0, hw_write);
+    i386_dr_show (state, "stopped_data_addr", 0, 0, hw_write);
 
   if (rc)
     *addr_p = addr;
@@ -764,19 +764,20 @@ i386_insert_hw_breakpoint (struct target_ops *self, struct gdbarch *gdbarch,
 {
   struct i386_debug_reg_state *state
     = i386_debug_reg_state (ptid_get_pid (inferior_ptid));
-  unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
+  unsigned len_rw = i386_dr_length_and_rw_bits (1, hw_execute);
   CORE_ADDR addr = bp_tgt->placed_address;
   /* 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;
-  int retval = i386_insert_aligned_watchpoint (&local_state,
-					       addr, len_rw) ? EBUSY : 0;
+  int retval = i386_dr_insert_aligned_watchpoint (&local_state,
+						  addr,
+						  len_rw) ? EBUSY : 0;
 
   if (retval == 0)
-    i386_update_inferior_debug_regs (state, &local_state);
+    i386_dr_update_inferior_debug_regs (state, &local_state);
 
   if (debug_hw_points)
-    i386_show_dr (state, "insert_hwbp", addr, 1, hw_execute);
+    i386_dr_show (state, "insert_hwbp", addr, 1, hw_execute);
 
   return retval;
 }
@@ -790,19 +791,19 @@ i386_remove_hw_breakpoint (struct target_ops *self, struct gdbarch *gdbarch,
 {
   struct i386_debug_reg_state *state
     = i386_debug_reg_state (ptid_get_pid (inferior_ptid));
-  unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
+  unsigned len_rw = i386_dr_length_and_rw_bits (1, hw_execute);
   CORE_ADDR addr = bp_tgt->placed_address;
   /* 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;
-  int retval = i386_remove_aligned_watchpoint (&local_state,
-					       addr, len_rw);
+  int retval = i386_dr_remove_aligned_watchpoint (&local_state,
+						  addr, len_rw);
 
   if (retval == 0)
-    i386_update_inferior_debug_regs (state, &local_state);
+    i386_dr_update_inferior_debug_regs (state, &local_state);
 
   if (debug_hw_points)
-    i386_show_dr (state, "remove_hwbp", addr, 1, hw_execute);
+    i386_dr_show (state, "remove_hwbp", addr, 1, hw_execute);
 
   return retval;
 }
-- 
1.7.1

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH 10/15] Abstract i386_dr_low access
  2014-06-17 14:13 [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Gary Benson
                   ` (8 preceding siblings ...)
  2014-06-17 14:48 ` [PATCH 07/15] Merge error handling Gary Benson
@ 2014-06-17 14:53 ` Gary Benson
  2014-06-17 16:10   ` Pedro Alves
  2014-06-17 14:53 ` [PATCH 02/15] Remove unnecessary prototypes Gary Benson
                   ` (5 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Gary Benson @ 2014-06-17 14:53 UTC (permalink / raw)
  To: gdb-patches

This commit adds macros to abstract access to the i386_dr_low
function vector used by i386-nat.c.  The macros are named so
as to match the names of the functions that do the same work
in gdbserver.

gdb/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-nat.c (i386_dr_low_can_set_addr): New macro.
	(i386_dr_low_can_set_control): Likewise.
	(i386_dr_low_set_addr): Likewise.
	(i386_dr_low_set_control): Likewise.
	(i386_dr_low_get_addr): Likewise.
	(i386_dr_low_get_status): Likewise.
	(i386_dr_low_get_control): Likewise.
	(i386_insert_aligned_watchpoint): Use new macros.
	(i386_update_inferior_debug_regs): Likewise.
	(i386_stopped_data_address): Likewise.

gdb/gdbserver/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-low.c (i386_dr_low_can_set_addr): New macro.
	(i386_dr_low_can_set_control): Likewise.
	(i386_insert_aligned_watchpoint): New check.
---
 gdb/ChangeLog            |   13 +++++++++++++
 gdb/gdbserver/ChangeLog  |    6 ++++++
 gdb/gdbserver/i386-low.c |    6 ++++++
 gdb/i386-nat.c           |   25 +++++++++++++++++++------
 4 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/gdb/gdbserver/i386-low.c b/gdb/gdbserver/i386-low.c
index 8c50838..8d6f26c 100644
--- a/gdb/gdbserver/i386-low.c
+++ b/gdb/gdbserver/i386-low.c
@@ -32,6 +32,9 @@
    The functions below implement debug registers sharing by reference
    counts, and allow to watch regions up to 16 bytes long.  */
 
+#define i386_dr_low_can_set_addr() 1
+#define i386_dr_low_can_set_control() 1
+
 /* Debug register size, in bytes.  */
 /* NOTE: sizeof (long) == 4 on win64.  */
 #define i386_get_debug_register_length() (sizeof (void *))
@@ -273,6 +276,9 @@ i386_insert_aligned_watchpoint (struct i386_debug_reg_state *state,
 {
   int i;
 
+  if (!i386_dr_low_can_set_addr () || !i386_dr_low_can_set_control ())
+    return -1;
+
   /* 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).  */
diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c
index a64cdab..4282535 100644
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -47,6 +47,19 @@ static int debug_hw_points;
 /* Low-level function vector.  */
 struct i386_dr_low_type i386_dr_low;
 
+#define i386_dr_low_can_set_addr() (i386_dr_low.set_addr != NULL)
+#define i386_dr_low_can_set_control() (i386_dr_low.set_control != NULL)
+
+#define i386_dr_low_set_addr(new_state, i) \
+  (i386_dr_low.set_addr ((i), (new_state)->dr_mirror[(i)]))
+
+#define i386_dr_low_set_control(new_state) \
+  (i386_dr_low.set_control ((new_state)->dr_control_mirror))
+
+#define i386_dr_low_get_addr(i) (i386_dr_low.get_addr ((i)))
+#define i386_dr_low_get_status() (i386_dr_low.get_status ())
+#define i386_dr_low_get_control() (i386_dr_low.get_control ())
+
 /* Debug register size, in bytes.  */
 #define i386_get_debug_register_length() \
   (i386_dr_low.debug_register_length)
@@ -379,7 +392,7 @@ i386_insert_aligned_watchpoint (struct i386_debug_reg_state *state,
 {
   int i;
 
-  if (!i386_dr_low.set_addr || !i386_dr_low.set_control)
+  if (!i386_dr_low_can_set_addr () || !i386_dr_low_can_set_control ())
     return -1;
 
   /* First, look for an occupied debug register with the same address
@@ -538,13 +551,13 @@ i386_update_inferior_debug_regs (struct i386_debug_reg_state *state,
   ALL_DEBUG_REGISTERS (i)
     {
       if (I386_DR_VACANT (new_state, i) != I386_DR_VACANT (state, i))
-	i386_dr_low.set_addr (i, new_state->dr_mirror[i]);
+	i386_dr_low_set_addr (new_state, i);
       else
 	gdb_assert (new_state->dr_mirror[i] == state->dr_mirror[i]);
     }
 
   if (new_state->dr_control_mirror != state->dr_control_mirror)
-    i386_dr_low.set_control (new_state->dr_control_mirror);
+    i386_dr_low_set_control (new_state);
 
   *state = *new_state;
 }
@@ -698,7 +711,7 @@ i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
      was running when we last changed watchpoints, the mirror no
      longer represents what was set in this thread's debug
      registers.  */
-  status = i386_dr_low.get_status ();
+  status = i386_dr_low_get_status ();
 
   ALL_DEBUG_REGISTERS (i)
     {
@@ -707,7 +720,7 @@ i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
 
       if (!control_p)
 	{
-	  control = i386_dr_low.get_control ();
+	  control = i386_dr_low_get_control ();
 	  control_p = 1;
 	}
 
@@ -718,7 +731,7 @@ i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
 	 paranoiac.  */
       if (I386_DR_GET_RW_LEN (control, i) != 0)
 	{
-	  addr = i386_dr_low.get_addr (i);
+	  addr = i386_dr_low_get_addr (i);
 	  rc = 1;
 	  if (debug_hw_points)
 	    i386_show_dr (state, "watchpoint_hit", addr, -1, hw_write);
-- 
1.7.1

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH 02/15] Remove unnecessary prototypes
  2014-06-17 14:13 [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Gary Benson
                   ` (9 preceding siblings ...)
  2014-06-17 14:53 ` [PATCH 10/15] Abstract i386_dr_low access Gary Benson
@ 2014-06-17 14:53 ` Gary Benson
  2014-06-17 15:26   ` Pedro Alves
  2014-06-17 14:53 ` [PATCH 08/15] Rename maint_show_dr to debug_hw_points Gary Benson
                   ` (4 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Gary Benson @ 2014-06-17 14:53 UTC (permalink / raw)
  To: gdb-patches

This commit removes four unnecessary prototypes from i386-nat.c.

gdb/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-nat.c (i386_length_and_rw_bits): Remove prototype.
	(i386_insert_aligned_watchpoint): Likewise.
	(i386_remove_aligned_watchpoint): Likewise.
	(i386_handle_nonaligned_watchpoint): Likewise.
---
 gdb/ChangeLog  |    7 +++++++
 gdb/i386-nat.c |   36 ------------------------------------
 2 files changed, 7 insertions(+), 36 deletions(-)

diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c
index a56a116..7c9e7ca 100644
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -256,42 +256,6 @@ 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 (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
-   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 (struct i386_debug_reg_state *state,
-					   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 (struct i386_debug_reg_state *state,
-					      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
-- 
1.7.1

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH 05/15] Add a const
  2014-06-17 14:13 [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Gary Benson
                   ` (11 preceding siblings ...)
  2014-06-17 14:53 ` [PATCH 08/15] Rename maint_show_dr to debug_hw_points Gary Benson
@ 2014-06-17 14:53 ` Gary Benson
  2014-06-17 15:46   ` Pedro Alves
  2014-06-17 14:58 ` [PATCH 15/15] Buildsystem changes Gary Benson
                   ` (2 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Gary Benson @ 2014-06-17 14:53 UTC (permalink / raw)
  To: gdb-patches

This commit adds a const that was in i386-low.c but not in i386-nat.c.

gdb/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-nat.c (i386_handle_nonaligned_watchpoint) <size_try_array>:
	Make const.
---
 gdb/ChangeLog  |    5 +++++
 gdb/i386-nat.c |    2 +-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c
index a7742f2..23efb33 100644
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -466,7 +466,7 @@ i386_handle_nonaligned_watchpoint (struct i386_debug_reg_state *state,
   int retval = 0;
   int max_wp_len = TARGET_HAS_DR_LEN_8 ? 8 : 4;
 
-  static int size_try_array[8][8] =
+  static const int size_try_array[8][8] =
   {
     {1, 1, 1, 1, 1, 1, 1, 1},	/* Trying size one.  */
     {2, 1, 2, 1, 2, 1, 2, 1},	/* Trying size two.  */
-- 
1.7.1

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH 08/15] Rename maint_show_dr to debug_hw_points
  2014-06-17 14:13 [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Gary Benson
                   ` (10 preceding siblings ...)
  2014-06-17 14:53 ` [PATCH 02/15] Remove unnecessary prototypes Gary Benson
@ 2014-06-17 14:53 ` Gary Benson
  2014-06-17 16:03   ` Pedro Alves
  2014-06-17 14:53 ` [PATCH 05/15] Add a const Gary Benson
                   ` (3 subsequent siblings)
  15 siblings, 1 reply; 45+ messages in thread
From: Gary Benson @ 2014-06-17 14:53 UTC (permalink / raw)
  To: gdb-patches

This commit renames maint_show_dr to debug_hw_points in i386-nat.c.

gdb/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-nat.c (maint_show_dr): Renamed to debug_hw_points.
	All uses updated.
---
 gdb/ChangeLog  |    5 +++++
 gdb/i386-nat.c |   20 ++++++++++----------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c
index 91d868d..6520549 100644
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -37,6 +37,9 @@
    The functions below implement debug registers sharing by reference
    counts, and allow to watch regions up to 16 bytes long.  */
 
+/* Whether or not to print the mirrored debug registers.  */
+static int debug_hw_points;
+
 /* Function used for printing mirrored debug registers.  */
 #define debug_printf(fmt, args...) \
   fprintf_unfiltered (gdb_stdlog, fmt, ##args);
@@ -256,9 +259,6 @@ i386_forget_process (pid_t pid)
     }
 }
 
-/* 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;
 
@@ -588,7 +588,7 @@ i386_insert_watchpoint (struct target_ops *self,
   if (retval == 0)
     i386_update_inferior_debug_regs (&local_state);
 
-  if (maint_show_dr)
+  if (debug_hw_points)
     i386_show_dr (state, "insert_watchpoint", addr, len, type);
 
   return retval;
@@ -628,7 +628,7 @@ i386_remove_watchpoint (struct target_ops *self,
   if (retval == 0)
     i386_update_inferior_debug_regs (&local_state);
 
-  if (maint_show_dr)
+  if (debug_hw_points)
     i386_show_dr (state, "remove_watchpoint", addr, len, type);
 
   return retval;
@@ -721,12 +721,12 @@ i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
 	{
 	  addr = i386_dr_low.get_addr (i);
 	  rc = 1;
-	  if (maint_show_dr)
+	  if (debug_hw_points)
 	    i386_show_dr (state, "watchpoint_hit", addr, -1, hw_write);
 	}
     }
 
-  if (maint_show_dr && addr == 0)
+  if (debug_hw_points && addr == 0)
     i386_show_dr (state, "stopped_data_addr", 0, 0, hw_write);
 
   if (rc)
@@ -763,7 +763,7 @@ i386_insert_hw_breakpoint (struct target_ops *self, struct gdbarch *gdbarch,
   if (retval == 0)
     i386_update_inferior_debug_regs (&local_state);
 
-  if (maint_show_dr)
+  if (debug_hw_points)
     i386_show_dr (state, "insert_hwbp", addr, 1, hw_execute);
 
   return retval;
@@ -789,7 +789,7 @@ i386_remove_hw_breakpoint (struct target_ops *self, struct gdbarch *gdbarch,
   if (retval == 0)
     i386_update_inferior_debug_regs (&local_state);
 
-  if (maint_show_dr)
+  if (debug_hw_points)
     i386_show_dr (state, "remove_hwbp", addr, 1, hw_execute);
 
   return retval;
@@ -825,7 +825,7 @@ add_show_debug_regs_command (void)
   /* A maintenance command to enable printing the internal DRi mirror
      variables.  */
   add_setshow_boolean_cmd ("show-debug-regs", class_maintenance,
-			   &maint_show_dr, _("\
+			   &debug_hw_points, _("\
 Set whether to show variables that mirror the x86 debug registers."), _("\
 Show whether to show variables that mirror the x86 debug registers."), _("\
 Use \"on\" to enable, \"off\" to disable.\n\
-- 
1.7.1

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH 15/15] Buildsystem changes
  2014-06-17 14:13 [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Gary Benson
                   ` (12 preceding siblings ...)
  2014-06-17 14:53 ` [PATCH 05/15] Add a const Gary Benson
@ 2014-06-17 14:58 ` Gary Benson
  2014-06-17 16:41   ` Pedro Alves
  2014-06-17 14:58 ` [PATCH 13/15] Move macros from i386-{nat,low}.c to i386-{nat,low}.h Gary Benson
  2014-06-17 17:00 ` [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Pedro Alves
  15 siblings, 1 reply; 45+ messages in thread
From: Gary Benson @ 2014-06-17 14:58 UTC (permalink / raw)
  To: gdb-patches

This commit adds the buildsystem changes required for
nat/i386-dregs.[ch].

gdb/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* Makefile.in (HFILES_NO_SRCDIR): Add nat/i386-dregs.h.
	(i386-dregs.o): New rule.
	* config/i386/cygwin.mh (NATDEPFILES): Add i386-dregs.o.
	* config/i386/cygwin64.mh (NATDEPFILES): Likewise.
	* config/i386/darwin.mh (NATDEPFILES): Likewise.
	* config/i386/fbsd.mh (NATDEPFILES): Likewise.
	* config/i386/fbsd64.mh (NATDEPFILES): Likewise.
	* config/i386/go32.mh (NATDEPFILES): Likewise.
	* config/i386/linux.mh (NATDEPFILES): Likewise.
	* config/i386/linux64.mh (NATDEPFILES): Likewise.
	* config/i386/mingw.mh (NATDEPFILES): Likewise.
	* config/i386/mingw64.mh (NATDEPFILES): Likewise.

gdb/gdbserver/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* Makefile.in (i386-dregs.o): New rule.
	* configure.srv: Add i386-dregs.o to all targets using i386-low.o.
---
 gdb/ChangeLog               |   15 +++++++++++++++
 gdb/Makefile.in             |    6 +++++-
 gdb/config/i386/cygwin.mh   |    4 ++--
 gdb/config/i386/cygwin64.mh |    2 +-
 gdb/config/i386/darwin.mh   |    2 +-
 gdb/config/i386/fbsd.mh     |    2 +-
 gdb/config/i386/fbsd64.mh   |    2 +-
 gdb/config/i386/go32.mh     |    2 +-
 gdb/config/i386/linux.mh    |    2 +-
 gdb/config/i386/linux64.mh  |    2 +-
 gdb/config/i386/mingw.mh    |    2 +-
 gdb/config/i386/mingw64.mh  |    2 +-
 gdb/gdbserver/ChangeLog     |    5 +++++
 gdb/gdbserver/Makefile.in   |    4 ++++
 gdb/gdbserver/configure.srv |   14 +++++++-------
 15 files changed, 47 insertions(+), 19 deletions(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index deb64a1..f4dd406 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -934,7 +934,7 @@ common/linux-osdata.h gdb-dlfcn.h auto-load.h probe.h stap-probe.h \
 gdb_bfd.h sparc-ravenscar-thread.h ppc-ravenscar-thread.h common/linux-btrace.h \
 ctf.h common/i386-cpuid.h common/i386-gcc-cpuid.h target/resume.h \
 target/wait.h target/waitstatus.h nat/linux-nat.h nat/linux-waitpid.h \
-common/print-utils.h common/rsp-low.h
+common/print-utils.h common/rsp-low.h nat/i386-dregs.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
@@ -2178,6 +2178,10 @@ waitstatus.o: ${srcdir}/target/waitstatus.c
 # Need to explicitly specify the compile rule as make will do nothing
 # or try to compile the object file into the sub-directory.
 
+i386-dregs.o: ${srcdir}/nat/i386-dregs.c
+	$(COMPILE) $(srcdir)/nat/i386-dregs.c
+	$(POSTCOMPILE)
+
 linux-waitpid.o: ${srcdir}/nat/linux-waitpid.c
 	$(COMPILE) $(srcdir)/nat/linux-waitpid.c
 	$(POSTCOMPILE)
diff --git a/gdb/config/i386/cygwin.mh b/gdb/config/i386/cygwin.mh
index b704158..8ab439b 100644
--- a/gdb/config/i386/cygwin.mh
+++ b/gdb/config/i386/cygwin.mh
@@ -1,3 +1,3 @@
-MH_CFLAGS=
-NATDEPFILES= i386-nat.o windows-nat.o i386-windows-nat.o
+'MH_CFLAGS=
+NATDEPFILES= i386-nat.o i386-dregs.o windows-nat.o i386-windows-nat.o
 XM_CLIBS=
diff --git a/gdb/config/i386/cygwin64.mh b/gdb/config/i386/cygwin64.mh
index 1c21fc6..5ce3095 100644
--- a/gdb/config/i386/cygwin64.mh
+++ b/gdb/config/i386/cygwin64.mh
@@ -17,4 +17,4 @@
 #  You should have received a copy of the GNU General Public License
 #  along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-NATDEPFILES= i386-nat.o windows-nat.o amd64-windows-nat.o
+NATDEPFILES= i386-nat.o i386-dregs.o windows-nat.o amd64-windows-nat.o
diff --git a/gdb/config/i386/darwin.mh b/gdb/config/i386/darwin.mh
index 190d56c..bd4d57e 100644
--- a/gdb/config/i386/darwin.mh
+++ b/gdb/config/i386/darwin.mh
@@ -1,4 +1,4 @@
 # Host: IA86 running Darwin
 
 NATDEPFILES = fork-child.o darwin-nat.o \
-     i386-darwin-nat.o i386-nat.o amd64-nat.o darwin-nat-info.o
+     i386-darwin-nat.o i386-nat.o i386-dregs.o amd64-nat.o darwin-nat-info.o
diff --git a/gdb/config/i386/fbsd.mh b/gdb/config/i386/fbsd.mh
index 2c9cd9a..7aa6a69 100644
--- a/gdb/config/i386/fbsd.mh
+++ b/gdb/config/i386/fbsd.mh
@@ -1,6 +1,6 @@
 # Host: FreeBSD/i386
 NATDEPFILES= fork-child.o inf-ptrace.o \
-	fbsd-nat.o i386-nat.o i386bsd-nat.o i386fbsd-nat.o \
+	fbsd-nat.o i386-nat.o i386-dregs.o i386bsd-nat.o i386fbsd-nat.o \
 	bsd-kvm.o
 NAT_FILE= nm-fbsd.h
 HAVE_NATIVE_GCORE_HOST = 1
diff --git a/gdb/config/i386/fbsd64.mh b/gdb/config/i386/fbsd64.mh
index c719a17..c37f460 100644
--- a/gdb/config/i386/fbsd64.mh
+++ b/gdb/config/i386/fbsd64.mh
@@ -1,7 +1,7 @@
 # Host: FreeBSD/amd64
 NATDEPFILES= fork-child.o inf-ptrace.o \
 	fbsd-nat.o amd64-nat.o amd64bsd-nat.o amd64fbsd-nat.o \
-	bsd-kvm.o i386-nat.o
+	bsd-kvm.o i386-nat.o i386-dregs.o
 HAVE_NATIVE_GCORE_HOST = 1
 
 LOADLIBES= -lkvm
diff --git a/gdb/config/i386/go32.mh b/gdb/config/i386/go32.mh
index 9cebf01..1b8ce86 100644
--- a/gdb/config/i386/go32.mh
+++ b/gdb/config/i386/go32.mh
@@ -3,7 +3,7 @@
 # We include several header files from config/djgpp
 MH_CFLAGS= -I$(srcdir)/config/djgpp
 
-NATDEPFILES= go32-nat.o i386-nat.o
+NATDEPFILES= go32-nat.o i386-nat.o i386-dregs.o
 
 HOST_IPC=
 CC= gcc
diff --git a/gdb/config/i386/linux.mh b/gdb/config/i386/linux.mh
index 10a2584..be18dcf 100644
--- a/gdb/config/i386/linux.mh
+++ b/gdb/config/i386/linux.mh
@@ -2,7 +2,7 @@
 
 NAT_FILE= config/nm-linux.h
 NATDEPFILES= inf-ptrace.o fork-child.o \
-	i386-nat.o i386-linux-nat.o \
+	i386-nat.o i386-dregs.o i386-linux-nat.o \
 	proc-service.o linux-thread-db.o \
 	linux-nat.o linux-osdata.o linux-fork.o linux-procfs.o linux-ptrace.o \
 	linux-btrace.o linux-waitpid.o
diff --git a/gdb/config/i386/linux64.mh b/gdb/config/i386/linux64.mh
index 686c363..3126b75 100644
--- a/gdb/config/i386/linux64.mh
+++ b/gdb/config/i386/linux64.mh
@@ -1,6 +1,6 @@
 # Host: GNU/Linux x86-64
 NATDEPFILES= inf-ptrace.o fork-child.o \
-	i386-nat.o amd64-nat.o amd64-linux-nat.o \
+	i386-nat.o i386-dregs.o amd64-nat.o amd64-linux-nat.o \
 	linux-nat.o linux-osdata.o \
 	proc-service.o linux-thread-db.o linux-fork.o \
 	linux-procfs.o linux-ptrace.o linux-btrace.o \
diff --git a/gdb/config/i386/mingw.mh b/gdb/config/i386/mingw.mh
index b704158..07f06f2 100644
--- a/gdb/config/i386/mingw.mh
+++ b/gdb/config/i386/mingw.mh
@@ -1,3 +1,3 @@
 MH_CFLAGS=
-NATDEPFILES= i386-nat.o windows-nat.o i386-windows-nat.o
+NATDEPFILES= i386-nat.o i386-dregs.o windows-nat.o i386-windows-nat.o
 XM_CLIBS=
diff --git a/gdb/config/i386/mingw64.mh b/gdb/config/i386/mingw64.mh
index ba6fd9f..740f508 100644
--- a/gdb/config/i386/mingw64.mh
+++ b/gdb/config/i386/mingw64.mh
@@ -1 +1 @@
-NATDEPFILES= i386-nat.o windows-nat.o amd64-windows-nat.o
+NATDEPFILES= i386-nat.o i386-dregs.o windows-nat.o amd64-windows-nat.o
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index cbf36ab..76d8965 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -551,6 +551,10 @@ mips-linux-watch.o: ../common/mips-linux-watch.c
 
 # Native object files rules from ../nat
 
+i386-dregs.o: ../nat/i386-dregs.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
+
 linux-waitpid.o: ../nat/linux-waitpid.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index 097c7b4..8ff9c28 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -99,7 +99,7 @@ case "${target}" in
 			srv_linux_thread_db=yes
 			;;
   i[34567]86-*-cygwin*)	srv_regobj="$srv_i386_regobj"
-			srv_tgtobj="i386-low.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="i386-low.o i386-dregs.o win32-low.o win32-i386-low.o"
 			srv_xmlfiles="$srv_i386_xmlfiles"
 			;;
   i[34567]86-*-linux*)	srv_regobj="$srv_i386_linux_regobj"
@@ -108,7 +108,7 @@ case "${target}" in
 			    srv_regobj="$srv_regobj $srv_amd64_linux_regobj"
 			    srv_xmlfiles="${srv_xmlfiles} $srv_amd64_linux_xmlfiles"
 			fi
-			srv_tgtobj="$srv_linux_obj linux-x86-low.o i386-low.o i387-fp.o"
+			srv_tgtobj="$srv_linux_obj linux-x86-low.o i386-low.o i386-dregs.o i387-fp.o"
 			srv_tgtobj="${srv_tgtobj} linux-btrace.o"
 			srv_linux_usrregs=yes
 			srv_linux_regsets=yes
@@ -125,7 +125,7 @@ case "${target}" in
 			;;
   i[34567]86-*-mingw32ce*)
 			srv_regobj="$srv_i386_regobj"
-			srv_tgtobj="i386-low.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="i386-low.o i386-dregs.o win32-low.o win32-i386-low.o"
 			srv_tgtobj="${srv_tgtobj} wincecompat.o"
 			srv_xmlfiles="$srv_i386_xmlfiles"
 			# hostio_last_error implementation is in win32-low.c
@@ -134,7 +134,7 @@ case "${target}" in
 			srv_mingwce=yes
 			;;
   i[34567]86-*-mingw*)	srv_regobj="$srv_i386_regobj"
-			srv_tgtobj="i386-low.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="i386-low.o i386-dregs.o win32-low.o win32-i386-low.o"
 			srv_xmlfiles="$srv_i386_xmlfiles"
 			srv_mingw=yes
 			;;
@@ -315,7 +315,7 @@ case "${target}" in
 			srv_linux_thread_db=yes
 			;;
   x86_64-*-linux*)	srv_regobj="$srv_amd64_linux_regobj $srv_i386_linux_regobj"
-			srv_tgtobj="$srv_linux_obj linux-x86-low.o i386-low.o i387-fp.o"
+			srv_tgtobj="$srv_linux_obj linux-x86-low.o i386-low.o i386-dregs.o i387-fp.o"
 			srv_tgtobj="${srv_tgtobj} linux-btrace.o"
 			srv_xmlfiles="$srv_i386_linux_xmlfiles $srv_amd64_linux_xmlfiles"
 			srv_linux_usrregs=yes # This is for i386 progs.
@@ -325,12 +325,12 @@ case "${target}" in
 			ipa_obj="${ipa_amd64_linux_regobj} linux-amd64-ipa.o"
 			;;
   x86_64-*-mingw*)	srv_regobj="$srv_amd64_regobj"
-			srv_tgtobj="i386-low.o i387-fp.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="i386-low.o i386-dregs.o i387-fp.o win32-low.o win32-i386-low.o"
 			srv_xmlfiles="$srv_i386_xmlfiles $srv_amd64_xmlfiles"
 			srv_mingw=yes
 			;;
   x86_64-*-cygwin*)	srv_regobj="$srv_amd64_regobj"
-			srv_tgtobj="i386-low.o i387-fp.o win32-low.o win32-i386-low.o"
+			srv_tgtobj="i386-low.o i386-dregs.o i387-fp.o win32-low.o win32-i386-low.o"
 			srv_xmlfiles="$srv_i386_xmlfiles"
 			;;
 
-- 
1.7.1

^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH 13/15] Move macros from i386-{nat,low}.c to i386-{nat,low}.h
  2014-06-17 14:13 [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Gary Benson
                   ` (13 preceding siblings ...)
  2014-06-17 14:58 ` [PATCH 15/15] Buildsystem changes Gary Benson
@ 2014-06-17 14:58 ` Gary Benson
  2014-06-17 16:54   ` Pedro Alves
  2014-06-17 17:00 ` [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Pedro Alves
  15 siblings, 1 reply; 45+ messages in thread
From: Gary Benson @ 2014-06-17 14:58 UTC (permalink / raw)
  To: gdb-patches

This commit moves macros required by the soon-to-be-created
nat/i386-dregs.c into i386-{nat,low}.h.

gdb/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-nat.h (i386_dr_low_can_set_addr): New macro.
	(i386_dr_low_set_addr): Likewise.
	(i386_dr_low_get_addr): Likewise.
	(i386_dr_low_can_set_control): Likewise.
	(i386_dr_low_set_control): Likewise.
	(i386_dr_low_get_control): Likewise.
	(i386_dr_low_get_status): Likewise.
	(i386_get_debug_register_length): Likewise.
	* i386-nat.c (i386_dr_low_can_set_addr): Now in i386-nat.h.
	(i386_dr_low_set_addr): Likewise.
	(i386_dr_low_get_addr): Likewise.
	(i386_dr_low_can_set_control): Likewise.
	(i386_dr_low_set_control): Likewise.
	(i386_dr_low_get_control): Likewise.
	(i386_dr_low_get_status): Likewise.
	(i386_get_debug_register_length): Likewise.

gdb/gdbserver/
2014-06-17  Gary Benson  <gbenson@redhat.com>

	* i386-low.h (i386_dr_low_can_set_addr): New macro.
	(i386_dr_low_can_set_control): Likewise.
	(i386_get_debug_register_length): Likewise.
	* i386-low.c (i386_dr_low_can_set_addr): Now in i386-low.h.
	(i386_dr_low_can_set_control): Likewise.
	(i386_get_debug_register_length): Likewise.
---
 gdb/ChangeLog            |   19 +++++++++++++++++++
 gdb/gdbserver/ChangeLog  |    9 +++++++++
 gdb/gdbserver/i386-low.c |    7 -------
 gdb/gdbserver/i386-low.h |   10 ++++++++++
 gdb/i386-nat.c           |   17 -----------------
 gdb/i386-nat.h           |   27 +++++++++++++++++++++++++++
 6 files changed, 65 insertions(+), 24 deletions(-)

diff --git a/gdb/gdbserver/i386-low.c b/gdb/gdbserver/i386-low.c
index bd25694..5333819 100644
--- a/gdb/gdbserver/i386-low.c
+++ b/gdb/gdbserver/i386-low.c
@@ -32,13 +32,6 @@
    The functions below implement debug registers sharing by reference
    counts, and allow to watch regions up to 16 bytes long.  */
 
-#define i386_dr_low_can_set_addr() 1
-#define i386_dr_low_can_set_control() 1
-
-/* Debug register size, in bytes.  */
-/* NOTE: sizeof (long) == 4 on win64.  */
-#define i386_get_debug_register_length() (sizeof (void *))
-
 /* Support for 8-byte wide hw watchpoints.  */
 #define TARGET_HAS_DR_LEN_8 (i386_get_debug_register_length () == 8)
 
diff --git a/gdb/gdbserver/i386-low.h b/gdb/gdbserver/i386-low.h
index 71f7c32..bad03b9 100644
--- a/gdb/gdbserver/i386-low.h
+++ b/gdb/gdbserver/i386-low.h
@@ -38,6 +38,9 @@ extern void i386_low_init_dregs (struct i386_debug_reg_state *state);
 				  status (DR6) register.
 */
 
+/* Can we update the inferior's debug registers?  */
+#define i386_dr_low_can_set_addr() 1
+
 /* Update the inferior's debug register REGNUM from STATE.  */
 extern void i386_dr_low_set_addr (const struct i386_debug_reg_state *state,
 				  int regnum);
@@ -45,6 +48,9 @@ extern void i386_dr_low_set_addr (const struct i386_debug_reg_state *state,
 /* Return the inferior's debug register REGNUM.  */
 extern CORE_ADDR i386_dr_low_get_addr (int regnum);
 
+/* Can we update the inferior's DR7 control register?  */
+#define i386_dr_low_can_set_control() 1
+
 /* Update the inferior's DR7 debug control register from STATE.  */
 extern void i386_dr_low_set_control (const struct i386_debug_reg_state *state);
 
@@ -53,3 +59,7 @@ extern unsigned i386_dr_low_get_control (void);
 
 /* Return the value of the inferior's DR6 debug status register.  */
 extern unsigned i386_dr_low_get_status (void);
+
+/* Return the debug register size, in bytes.  */
+/* Note that sizeof (long) == 4 on win64.  */
+#define i386_get_debug_register_length() (sizeof (void *))
diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c
index 1dfa9ee..1f7b70c 100644
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -47,23 +47,6 @@ static int debug_hw_points;
 /* Low-level function vector.  */
 struct i386_dr_low_type i386_dr_low;
 
-#define i386_dr_low_can_set_addr() (i386_dr_low.set_addr != NULL)
-#define i386_dr_low_can_set_control() (i386_dr_low.set_control != NULL)
-
-#define i386_dr_low_set_addr(new_state, i) \
-  (i386_dr_low.set_addr ((i), (new_state)->dr_mirror[(i)]))
-
-#define i386_dr_low_set_control(new_state) \
-  (i386_dr_low.set_control ((new_state)->dr_control_mirror))
-
-#define i386_dr_low_get_addr(i) (i386_dr_low.get_addr ((i)))
-#define i386_dr_low_get_status() (i386_dr_low.get_status ())
-#define i386_dr_low_get_control() (i386_dr_low.get_control ())
-
-/* Debug register size, in bytes.  */
-#define i386_get_debug_register_length() \
-  (i386_dr_low.debug_register_length)
-
 /* Support for 8-byte wide hw watchpoints.  */
 #define TARGET_HAS_DR_LEN_8 (i386_get_debug_register_length () == 8)
 
diff --git a/gdb/i386-nat.h b/gdb/i386-nat.h
index 1f4130c..f20b013 100644
--- a/gdb/i386-nat.h
+++ b/gdb/i386-nat.h
@@ -73,6 +73,33 @@ struct i386_dr_low_type
 
 extern struct i386_dr_low_type i386_dr_low;
 
+/* Can we update the inferior's debug registers?  */
+#define i386_dr_low_can_set_addr() (i386_dr_low.set_addr != NULL)
+
+/* Update the inferior's debug register REGNUM from STATE.  */
+#define i386_dr_low_set_addr(new_state, i) \
+  (i386_dr_low.set_addr ((i), (new_state)->dr_mirror[(i)]))
+
+/* Return the inferior's debug register REGNUM.  */
+#define i386_dr_low_get_addr(i) (i386_dr_low.get_addr ((i)))
+
+/* Can we update the inferior's DR7 control register?  */
+#define i386_dr_low_can_set_control() (i386_dr_low.set_control != NULL)
+
+/* Update the inferior's DR7 debug control register from STATE.  */
+#define i386_dr_low_set_control(new_state) \
+  (i386_dr_low.set_control ((new_state)->dr_control_mirror))
+
+/* Return the value of the inferior's DR7 debug control register.  */
+#define i386_dr_low_get_control() (i386_dr_low.get_control ())
+
+/* Return the value of the inferior's DR6 debug status register.  */
+#define i386_dr_low_get_status() (i386_dr_low.get_status ())
+
+/* Return the debug register size, in bytes.  */
+#define i386_get_debug_register_length() \
+  (i386_dr_low.debug_register_length)
+
 /* Use this function to set i386_dr_low debug_register_length field
    rather than setting it directly to check that the length is only
    set once.  It also enables the 'maint set/show show-debug-regs' 
-- 
1.7.1

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 01/15] Whitespace changes
  2014-06-17 14:13 ` [PATCH 01/15] Whitespace changes Gary Benson
@ 2014-06-17 15:23   ` Pedro Alves
  0 siblings, 0 replies; 45+ messages in thread
From: Pedro Alves @ 2014-06-17 15:23 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 06/17/2014 03:12 PM, Gary Benson wrote:
> This commit fixes various whitespace differences between i386-nat.c
> and i386-low.c.
> 
> gdb/
> 2014-06-17  Gary Benson  <gbenson@redhat.com>
> 
> 	* i386-nat.c: Whitespace changes.
> 
> gdb/gdbserver/
> 2014-06-17  Gary Benson  <gbenson@redhat.com>
> 
> 	* i386-low.c: Whitespace changes.

OK, please push.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 02/15] Remove unnecessary prototypes
  2014-06-17 14:53 ` [PATCH 02/15] Remove unnecessary prototypes Gary Benson
@ 2014-06-17 15:26   ` Pedro Alves
  0 siblings, 0 replies; 45+ messages in thread
From: Pedro Alves @ 2014-06-17 15:26 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 06/17/2014 03:12 PM, Gary Benson wrote:
> This commit removes four unnecessary prototypes from i386-nat.c.
> 
> gdb/
> 2014-06-17  Gary Benson  <gbenson@redhat.com>
> 
> 	* i386-nat.c (i386_length_and_rw_bits): Remove prototype.
> 	(i386_insert_aligned_watchpoint): Likewise.
> 	(i386_remove_aligned_watchpoint): Likewise.
> 	(i386_handle_nonaligned_watchpoint): Likewise.

OK, please push.

(I'd have been good to mention that all the removed comments
are duplicated in the function definitions.)

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 03/15] Remove unused macro
  2014-06-17 14:13 ` [PATCH 03/15] Remove unused macro Gary Benson
@ 2014-06-17 15:35   ` Pedro Alves
  0 siblings, 0 replies; 45+ messages in thread
From: Pedro Alves @ 2014-06-17 15:35 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 06/17/2014 03:12 PM, Gary Benson wrote:
> This commit removes an unused macro from i386-nat.c.
> 
> gdb/
> 2014-06-17  Gary Benson  <gbenson@redhat.com>
> 
> 	* i386-nat.c (I386_DR_WATCH_MASK): Remove macro.

I looked at the history.  Jan added it in a79d3c27d, and I
removed all uses in 1ced966e3.  It could still be used in I386_DR_WATCH_HIT,
like it was before 1ced966e3 -- not sure why I reverted that bit -- but
that wouldn't increase readability, IMO.

Thus, OK.  Please push.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 04/15] Comment changes
  2014-06-17 14:13 ` [PATCH 04/15] Comment changes Gary Benson
@ 2014-06-17 15:45   ` Pedro Alves
  2014-06-17 16:10     ` Gary Benson
  0 siblings, 1 reply; 45+ messages in thread
From: Pedro Alves @ 2014-06-17 15:45 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 06/17/2014 03:12 PM, Gary Benson wrote:
> This commit fixes various whitespace differences between

s/whitespace/comment/

>  
>    /* In non-stop/async, threads can be running while we change the
> -     STATE (and friends).  Say, we set a watchpoint, and let threads

> +     global dr_mirror (and friends).  Say, we set a watchpoint, and
> +     let threads resume.  Now, say you delete the watchpoint, or

Wrong direction, here, I'm afraid.  There's no _global_ dr_mirror anymore.
There used to be one, before multi-process.

Otherwise OK.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 05/15] Add a const
  2014-06-17 14:53 ` [PATCH 05/15] Add a const Gary Benson
@ 2014-06-17 15:46   ` Pedro Alves
  0 siblings, 0 replies; 45+ messages in thread
From: Pedro Alves @ 2014-06-17 15:46 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 06/17/2014 03:12 PM, Gary Benson wrote:
> This commit adds a const that was in i386-low.c but not in i386-nat.c.
> 
> gdb/
> 2014-06-17  Gary Benson  <gbenson@redhat.com>
> 
> 	* i386-nat.c (i386_handle_nonaligned_watchpoint) <size_try_array>:
> 	Make const.

OK.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 06/15] Merge printing code
  2014-06-17 14:40 ` [PATCH 06/15] Merge printing code Gary Benson
@ 2014-06-17 16:01   ` Pedro Alves
  0 siblings, 0 replies; 45+ messages in thread
From: Pedro Alves @ 2014-06-17 16:01 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 06/17/2014 03:12 PM, Gary Benson wrote:
> This commit synchronizes the debug printing code in i386-nat.c and
> i386-low.c.

OK, thanks.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 07/15] Merge error handling
  2014-06-17 14:48 ` [PATCH 07/15] Merge error handling Gary Benson
@ 2014-06-17 16:02   ` Pedro Alves
  0 siblings, 0 replies; 45+ messages in thread
From: Pedro Alves @ 2014-06-17 16:02 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 06/17/2014 03:12 PM, Gary Benson wrote:
> This commit makes all error handling in i386-low.c use internal_error
> rather than fatal and error.
> 
> gdb/gdbserver/
> 2014-06-17  Gary Benson  <gbenson@redhat.com>
> 
> 	* i386-low.c (i386_length_and_rw_bits): Use internal_error
> 	instead of fatal and error.
> 	(i386_handle_nonaligned_watchpoint): Likewise.

OK.

Thanks,
-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 08/15] Rename maint_show_dr to debug_hw_points
  2014-06-17 14:53 ` [PATCH 08/15] Rename maint_show_dr to debug_hw_points Gary Benson
@ 2014-06-17 16:03   ` Pedro Alves
  0 siblings, 0 replies; 45+ messages in thread
From: Pedro Alves @ 2014-06-17 16:03 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 06/17/2014 03:12 PM, Gary Benson wrote:
> This commit renames maint_show_dr to debug_hw_points in i386-nat.c.
> 
> gdb/
> 2014-06-17  Gary Benson  <gbenson@redhat.com>
> 
> 	* i386-nat.c (maint_show_dr): Renamed to debug_hw_points.
> 	All uses updated.

OK.

Thanks,
-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 09/15] Merge i386_update_inferior_debug_regs
  2014-06-17 14:44 ` [PATCH 09/15] Merge i386_update_inferior_debug_regs Gary Benson
@ 2014-06-17 16:06   ` Pedro Alves
  0 siblings, 0 replies; 45+ messages in thread
From: Pedro Alves @ 2014-06-17 16:06 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 06/17/2014 03:12 PM, Gary Benson wrote:
> This commit synchronizes the i386_update_inferior_debug_regs functions
> in i386-nat.c and i386-low.c.
> 
> gdb/
> 2014-06-17  Gary Benson  <gbenson@redhat.com>
> 
> 	* i386-nat.c (i386_update_inferior_debug_regs) <state>:
> 	New parameter.  All uses updated.
> 
> gdb/gdbserver/
> 2014-06-17  Gary Benson  <gbenson@redhat.com>
> 
> 	* i386-low.c (i386_update_inferior_debug_regs) <inf_state>:
> 	Renamed to state.

OK.

Thanks,
-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 10/15] Abstract i386_dr_low access
  2014-06-17 14:53 ` [PATCH 10/15] Abstract i386_dr_low access Gary Benson
@ 2014-06-17 16:10   ` Pedro Alves
  0 siblings, 0 replies; 45+ messages in thread
From: Pedro Alves @ 2014-06-17 16:10 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 06/17/2014 03:12 PM, Gary Benson wrote:
> This commit adds macros to abstract access to the i386_dr_low
> function vector used by i386-nat.c.  The macros are named so
> as to match the names of the functions that do the same work
> in gdbserver.
> 
> gdb/
> 2014-06-17  Gary Benson  <gbenson@redhat.com>
> 
> 	* i386-nat.c (i386_dr_low_can_set_addr): New macro.
> 	(i386_dr_low_can_set_control): Likewise.
> 	(i386_dr_low_set_addr): Likewise.
> 	(i386_dr_low_set_control): Likewise.
> 	(i386_dr_low_get_addr): Likewise.
> 	(i386_dr_low_get_status): Likewise.
> 	(i386_dr_low_get_control): Likewise.
> 	(i386_insert_aligned_watchpoint): Use new macros.
> 	(i386_update_inferior_debug_regs): Likewise.
> 	(i386_stopped_data_address): Likewise.
> 
> gdb/gdbserver/
> 2014-06-17  Gary Benson  <gbenson@redhat.com>
> 
> 	* i386-low.c (i386_dr_low_can_set_addr): New macro.
> 	(i386_dr_low_can_set_control): Likewise.
> 	(i386_insert_aligned_watchpoint): New check.

OK.

Thanks,
-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 04/15] Comment changes
  2014-06-17 15:45   ` Pedro Alves
@ 2014-06-17 16:10     ` Gary Benson
  2014-06-17 17:11       ` Pedro Alves
  0 siblings, 1 reply; 45+ messages in thread
From: Gary Benson @ 2014-06-17 16:10 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro Alves wrote:
> On 06/17/2014 03:12 PM, Gary Benson wrote:
> > This commit fixes various whitespace differences between
> 
> s/whitespace/comment/

Oops :)

> >    /* In non-stop/async, threads can be running while we change the
> > -     STATE (and friends).  Say, we set a watchpoint, and let threads
> 
> > +     global dr_mirror (and friends).  Say, we set a watchpoint, and
> > +     let threads resume.  Now, say you delete the watchpoint, or
> 
> Wrong direction, here, I'm afraid.  There's no _global_ dr_mirror
> anymore.  There used to be one, before multi-process.

Is s/global dr_mirror/dr_mirror/ ok, or something else?

Cheers,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 11/15] Rename functions and make nonstatic as necessary
  2014-06-17 14:48 ` [PATCH 11/15] Rename functions and make nonstatic as necessary Gary Benson
@ 2014-06-17 16:13   ` Pedro Alves
  2014-06-18 10:25   ` Siva Chandra
  1 sibling, 0 replies; 45+ messages in thread
From: Pedro Alves @ 2014-06-17 16:13 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 06/17/2014 03:12 PM, Gary Benson wrote:
> This commit renames the functions that are to be shared.
> Functions to be shared that were static are made nonstatic.
> 
> gdb/
> 2014-06-17  Gary Benson  <gbenson@redhat.com>
> 
> 	* i386-nat.c (i386_show_dr): Renamed to
> 	i386_dr_show and made nonstatic.  All uses updated.
> 	(i386_length_and_rw_bits): Renamed to
> 	i386_dr_length_and_rw_bits and made nonstatic.
> 	All uses updated.
> 	(i386_insert_aligned_watchpoint): Renamed to
> 	i386_dr_insert_aligned_watchpoint and made nonstatic.
> 	All uses updated.
> 	(i386_remove_aligned_watchpoint): Renamed to
> 	i386_dr_remove_aligned_watchpoint and made nonstatic.
> 	All uses updated.
> 	(i386_update_inferior_debug_regs): Renamed to
> 	i386_dr_update_inferior_debug_regs and made nonstatic.
> 	All uses updated.
> 
> gdb/gdbserver/
> 2014-06-17  Gary Benson  <gbenson@redhat.com>
> 
> 	* i386-low.h (i386_low_insert_watchpoint): Renamed to
> 	i386_dr_insert_watchpoint.
> 	(i386_low_remove_watchpoint): Renamed to
> 	i386_dr_remove_watchpoint.
> 	(i386_low_region_ok_for_watchpoint): Renamed to
> 	i386_dr_region_ok_for_watchpoint.
> 	(i386_low_stopped_data_address): Renamed to
> 	i386_dr_stopped_data_address.
> 	(i386_low_stopped_by_watchpoint): Renamed to
> 	i386_dr_stopped_by_watchpoint.
> 	* i386-low.c (i386_show_dr): Renamed to
> 	i386_dr_show and made nonstatic.  All uses updated.
> 	(i386_length_and_rw_bits): Renamed to
> 	i386_dr_length_and_rw_bits and made nonstatic.
> 	All uses updated.
> 	(i386_insert_aligned_watchpoint): Renamed to
> 	i386_dr_insert_aligned_watchpoint and made nonstatic.
> 	All uses updated.
> 	(i386_remove_aligned_watchpoint): Renamed to
> 	i386_dr_remove_aligned_watchpoint and made nonstatic.
> 	All uses updated.
> 	(i386_update_inferior_debug_regs): Renamed to
> 	i386_dr_update_inferior_debug_regs and made nonstatic.
> 	All uses updated.
> 	(i386_low_insert_watchpoint): Renamed to
> 	i386_dr_insert_watchpoint.  All uses updated.
> 	(i386_low_remove_watchpoint): Renamed to
> 	i386_dr_remove_watchpoint.  All uses updated.
> 	(i386_low_region_ok_for_watchpoint): Renamed to
> 	i386_dr_region_ok_for_watchpoint.  All uses updated.
> 	(i386_low_stopped_data_address): Renamed to
> 	i386_dr_stopped_data_address.  All uses updated.
> 	(i386_low_stopped_by_watchpoint): Renamed to
> 	i386_dr_stopped_by_watchpoint.  All uses updated.


Most excellent.  OK.

Thanks,
-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 15/15] Buildsystem changes
  2014-06-17 14:58 ` [PATCH 15/15] Buildsystem changes Gary Benson
@ 2014-06-17 16:41   ` Pedro Alves
  0 siblings, 0 replies; 45+ messages in thread
From: Pedro Alves @ 2014-06-17 16:41 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 06/17/2014 03:12 PM, Gary Benson wrote:
> This commit adds the buildsystem changes required for
> nat/i386-dregs.[ch].

The contents of the patch are all OK, but please merge it with the previous
patches, so as not to the break the build at any incremental step.

That, is ...

> 
> gdb/
> 2014-06-17  Gary Benson  <gbenson@redhat.com>
> 
> 	* Makefile.in (HFILES_NO_SRCDIR): Add nat/i386-dregs.h.

... move this hunk to the patch that actually creates
nat/i386-dregs.h.

And the rest to the patch that creates nat/i386-dregs.c.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 12/15] Create nat/i386-dregs.h
  2014-06-17 14:44 ` [PATCH 12/15] Create nat/i386-dregs.h Gary Benson
@ 2014-06-17 16:48   ` Pedro Alves
  2014-06-18 11:14     ` Gary Benson
  0 siblings, 1 reply; 45+ messages in thread
From: Pedro Alves @ 2014-06-17 16:48 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 06/17/2014 03:12 PM, Gary Benson wrote:
> This commit moves code to be shared from i386-{nat,low}.[ch]
> into a new file, nat/i386-dregs.h.
> 
> gdb/
> 2014-06-17  Gary Benson  <gbenson@redhat.com>
> 
> 	* nat/i386-dregs.h: New file.
> 	* i386-nat.h (i386-dregs.h): New include.
> 	(DR_FIRSTADDR): Now in i386-dregs.h.
> 	(DR_LASTADDR): Likewise.
> 	(DR_NADDR): Likewise.
> 	(DR_STATUS): Likewise.
> 	(DR_CONTROL): Likewise.
> 	(i386_debug_reg_state): Likewise.
> 	* i386-nat.c (ALL_DEBUG_REGISTERS): Likewise.
> 
> gdb/gdbserver/
> 2014-06-17  Gary Benson  <gbenson@redhat.com>
> 
> 	* i386-low.h (i386-dregs.h): New include.
> 	(DR_FIRSTADDR): Now in i386-dregs.h.
> 	(DR_LASTADDR): Likewise.
> 	(DR_NADDR): Likewise.
> 	(DR_STATUS): Likewise.
> 	(DR_CONTROL): Likewise.
> 	(i386_debug_reg_state): Likewise.
> 	(i386_dr_insert_watchpoint): Likewise.
> 	(i386_dr_remove_watchpoint): Likewise.
> 	(i386_dr_region_ok_for_watchpoint): Likewise.
> 	(i386_dr_stopped_data_address): Likewise.
> 	(i386_dr_stopped_by_watchpoint): Likewise.
> 	* i386-low.c (ALL_DEBUG_REGISTERS): Likewise.

This is OK.

Thanks,
-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 13/15] Move macros from i386-{nat,low}.c to i386-{nat,low}.h
  2014-06-17 14:58 ` [PATCH 13/15] Move macros from i386-{nat,low}.c to i386-{nat,low}.h Gary Benson
@ 2014-06-17 16:54   ` Pedro Alves
  2014-06-18 11:33     ` Gary Benson
  0 siblings, 1 reply; 45+ messages in thread
From: Pedro Alves @ 2014-06-17 16:54 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 06/17/2014 03:12 PM, Gary Benson wrote:
> This commit moves macros required by the soon-to-be-created
> nat/i386-dregs.c into i386-{nat,low}.h.

To get rid of the remaining '#ifdef GDBSERVER's in the
new i386-dregs.c, we could make gdbserver use the i386_dr_low vector
too.  Did you look into that?

Meanwhile, this is already much better than what we have today.
Thus, this is OK.

Thanks,
-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 14/15] Create nat/i386-dregs.c
  2014-06-17 14:40 ` [PATCH 14/15] Create nat/i386-dregs.c Gary Benson
@ 2014-06-17 16:56   ` Pedro Alves
  2014-06-18 13:43     ` Gary Benson
  0 siblings, 1 reply; 45+ messages in thread
From: Pedro Alves @ 2014-06-17 16:56 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 06/17/2014 03:12 PM, Gary Benson wrote:
> @@ -596,34 +172,8 @@ i386_remove_watchpoint (struct target_ops *self,
>  {
>    struct i386_debug_reg_state *state
>      = i386_debug_reg_state (ptid_get_pid (inferior_ptid));
> -  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 = *state;
> -
> -  if (((len != 1 && len != 2 && len != 4)
> -       && !(TARGET_HAS_DR_LEN_8 && len == 8))
> -      || addr % len != 0)
> -    {
> -      retval = i386_handle_nonaligned_watchpoint (&local_state,
> -						  WP_REMOVE,
> -						  addr, len, type);
> -    }
> -  else
> -    {
> -      unsigned len_rw = i386_dr_length_and_rw_bits (len, type);
> -
> -      retval = i386_dr_remove_aligned_watchpoint (&local_state,
> -					       addr, len_rw);
> -    }
> -
> -  if (retval == 0)
> -    i386_dr_update_inferior_debug_regs (state, &local_state);
> -
> -  if (debug_hw_points)
> -    i386_dr_show (state, "remove_watchpoint", addr, len, type);
>  
> -  return retval;
> +  return i386_dr_remove_watchpoint (state, type, addr, len);

Hmm, there's still one remaining difference in gdb vs gdbserver,
after this, in hw breakpoints.

i386_insert_hw_breakpoint and i386_remove_hw_breakpoint on
the GDB side still have the "work on a local copy"
bits, and the i386_dr_update_inferior_debug_regs and
i386_dr_show calls:

/* Insert a hardware-assisted breakpoint at BP_TGT->placed_address.
   Return 0 on success, EBUSY on failure.  */
static int
i386_insert_hw_breakpoint (struct target_ops *self, struct gdbarch *gdbarch,
			   struct bp_target_info *bp_tgt)
{
  struct i386_debug_reg_state *state
    = i386_debug_reg_state (ptid_get_pid (inferior_ptid));
  unsigned len_rw = i386_dr_length_and_rw_bits (1, hw_execute);
  CORE_ADDR addr = bp_tgt->placed_address;
  /* 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;
  int retval = i386_dr_insert_aligned_watchpoint (&local_state,
						  addr,
						  len_rw) ? EBUSY : 0;

  if (retval == 0)
    i386_dr_update_inferior_debug_regs (state, &local_state);

  if (debug_hw_points)
    i386_dr_show (state, "insert_hwbp", addr, 1, hw_execute);

  return retval;
}

While on the gdbserver side, we just call i386_dr_insert_watchpoint:

static int
x86_insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
		  int size, struct raw_breakpoint *bp)
{
  switch (type)
    {
    case raw_bkpt_type_hw:
...
      {
...
	return i386_dr_insert_watchpoint (state, hw_type, addr, size);

Any reason GDB can't do the same?

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch]
  2014-06-17 14:13 [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Gary Benson
                   ` (14 preceding siblings ...)
  2014-06-17 14:58 ` [PATCH 13/15] Move macros from i386-{nat,low}.c to i386-{nat,low}.h Gary Benson
@ 2014-06-17 17:00 ` Pedro Alves
  2014-06-18 10:49   ` Gary Benson
  15 siblings, 1 reply; 45+ messages in thread
From: Pedro Alves @ 2014-06-17 17:00 UTC (permalink / raw)
  To: Gary Benson, gdb-patches

On 06/17/2014 03:12 PM, Gary Benson wrote:
> Hi all,
> 
> This series refactors the shared code in i386-{nat,low}.[ch] into
> a pair of new files, nat/i386-dregs.[ch].  The patches are in two
> groups.  Group 1 removes the changes between the various files so
> that the code to be shared is identical:
> 
>    1/15 - Whitespace changes
>    2/15 - Remove unnecessary prototypes
>    3/15 - Remove unused macro
>    4/15 - Comment changes
>    5/15 - Add a const
>    6/15 - Merge printing code
>    7/15 - Merge error handling
>    8/15 - Rename maint_show_dr to debug_hw_points
>    9/15 - Merge i386_update_inferior_debug_regs
>   10/15 - Abstract i386_dr_low access
> 
> Group 2 moves all that identical code into the new files:
> 
>   11/15 - Rename functions and make nonstatic as necessary
>   12/15 - Create nat/i386-dregs.h
>   13/15 - Move macros from i386-{nat,low}.c to i386-{nat,low}.h
>   14/15 - Create nat/i386-dregs.c
>   15/15 - Buildsystem changes
> 
> Is this ok to commit?

Excellent work.  Almost all of it is OK to commit immediately.
Feel free to push those in, and send the parts that need updating
as a smaller v2 (I think patches 12-15) (that is, no need to
keep carrying and reposting all the preparatory patches that
are already good).

Thanks!

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 04/15] Comment changes
  2014-06-17 16:10     ` Gary Benson
@ 2014-06-17 17:11       ` Pedro Alves
  0 siblings, 0 replies; 45+ messages in thread
From: Pedro Alves @ 2014-06-17 17:11 UTC (permalink / raw)
  To: Gary Benson; +Cc: gdb-patches

On 06/17/2014 05:09 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 06/17/2014 03:12 PM, Gary Benson wrote:
>>> This commit fixes various whitespace differences between
>>
>> s/whitespace/comment/
> 
> Oops :)
> 
>>>    /* In non-stop/async, threads can be running while we change the
>>> -     STATE (and friends).  Say, we set a watchpoint, and let threads
>>
>>> +     global dr_mirror (and friends).  Say, we set a watchpoint, and
>>> +     let threads resume.  Now, say you delete the watchpoint, or
>>
>> Wrong direction, here, I'm afraid.  There's no _global_ dr_mirror
>> anymore.  There used to be one, before multi-process.
> 
> Is s/global dr_mirror/dr_mirror/ ok, or something else?

Hmm, you know what, neither version is strictly correct.

"the STATE (and friends)" was clearly a blind search&replace.

So, to makes things easy, and avoid a potentially make-work
rebase, just leave it as you had it, and we can clean it
up afterwards.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 11/15] Rename functions and make nonstatic as necessary
  2014-06-17 14:48 ` [PATCH 11/15] Rename functions and make nonstatic as necessary Gary Benson
  2014-06-17 16:13   ` Pedro Alves
@ 2014-06-18 10:25   ` Siva Chandra
  2014-06-18 13:30     ` Gary Benson
  1 sibling, 1 reply; 45+ messages in thread
From: Siva Chandra @ 2014-06-18 10:25 UTC (permalink / raw)
  To: Gary Benson; +Cc: gdb-patches

On Tue, Jun 17, 2014 at 7:12 AM, Gary Benson <gbenson@redhat.com> wrote:
> This commit renames the functions that are to be shared.
> Functions to be shared that were static are made nonstatic.

I thinking making the functions nonstatic in this part was not
correct. The build currently fails when built with
"-Werror=missing-prototypes" as 14/15 has not yet been committed.

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch]
  2014-06-17 17:00 ` [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Pedro Alves
@ 2014-06-18 10:49   ` Gary Benson
  0 siblings, 0 replies; 45+ messages in thread
From: Gary Benson @ 2014-06-18 10:49 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro Alves wrote:
> On 06/17/2014 03:12 PM, Gary Benson wrote:
> > This series refactors the shared code in i386-{nat,low}.[ch] into
> > a pair of new files, nat/i386-dregs.[ch].  The patches are in two
> > groups.  Group 1 removes the changes between the various files so
> > that the code to be shared is identical:
> > 
> >    1/15 - Whitespace changes
> >    2/15 - Remove unnecessary prototypes
> >    3/15 - Remove unused macro
> >    4/15 - Comment changes
> >    5/15 - Add a const
> >    6/15 - Merge printing code
> >    7/15 - Merge error handling
> >    8/15 - Rename maint_show_dr to debug_hw_points
> >    9/15 - Merge i386_update_inferior_debug_regs
> >   10/15 - Abstract i386_dr_low access
> > 
> > Group 2 moves all that identical code into the new files:
> > 
> >   11/15 - Rename functions and make nonstatic as necessary
> >   12/15 - Create nat/i386-dregs.h
> >   13/15 - Move macros from i386-{nat,low}.c to i386-{nat,low}.h
> >   14/15 - Create nat/i386-dregs.c
> >   15/15 - Buildsystem changes
> > 
> > Is this ok to commit?
> 
> Excellent work.  Almost all of it is OK to commit immediately.
> Feel free to push those in, and send the parts that need updating
> as a smaller v2 (I think patches 12-15) (that is, no need to keep
> carrying and reposting all the preparatory patches that are already
> good).

Great, I've pushed patches 1-11 and will start fixing up the rest.

Thanks for the speedy review!

Cheers,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 12/15] Create nat/i386-dregs.h
  2014-06-17 16:48   ` Pedro Alves
@ 2014-06-18 11:14     ` Gary Benson
  0 siblings, 0 replies; 45+ messages in thread
From: Gary Benson @ 2014-06-18 11:14 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro Alves wrote:
> On 06/17/2014 03:12 PM, Gary Benson wrote:
> > This commit moves code to be shared from i386-{nat,low}.[ch]
> > into a new file, nat/i386-dregs.h.
[snip]
> 
> This is OK.

I have pushed this, with the relevant buildsystem update from patch 15
added.  The actual patch I pushed is inlined below.

Thanks,
Gary

--
gdb/
2014-06-18  Gary Benson  <gbenson@redhat.com>

	* nat/i386-dregs.h: New file.
	* Makefile.in (HFILES_NO_SRCDIR): Add the above.
	* i386-nat.h (i386-dregs.h): New include.
	(DR_FIRSTADDR): Now in i386-dregs.h.
	(DR_LASTADDR): Likewise.
	(DR_NADDR): Likewise.
	(DR_STATUS): Likewise.
	(DR_CONTROL): Likewise.
	(i386_debug_reg_state): Likewise.
	* i386-nat.c (ALL_DEBUG_REGISTERS): Likewise.

gdbserver/
2014-06-18  Gary Benson  <gbenson@redhat.com>

	* i386-low.h (i386-dregs.h): New include.
	(DR_FIRSTADDR): Now in i386-dregs.h.
	(DR_LASTADDR): Likewise.
	(DR_NADDR): Likewise.
	(DR_STATUS): Likewise.
	(DR_CONTROL): Likewise.
	(i386_debug_reg_state): Likewise.
	(i386_dr_insert_watchpoint): Likewise.
	(i386_dr_remove_watchpoint): Likewise.
	(i386_dr_region_ok_for_watchpoint): Likewise.
	(i386_dr_stopped_data_address): Likewise.
	(i386_dr_stopped_by_watchpoint): Likewise.
	* i386-low.c (ALL_DEBUG_REGISTERS): Likewise.

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index deb64a1..d17774a 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -934,7 +934,7 @@ common/linux-osdata.h gdb-dlfcn.h auto-load.h probe.h stap-probe.h \
 gdb_bfd.h sparc-ravenscar-thread.h ppc-ravenscar-thread.h common/linux-btrace.h \
 ctf.h common/i386-cpuid.h common/i386-gcc-cpuid.h target/resume.h \
 target/wait.h target/waitstatus.h nat/linux-nat.h nat/linux-waitpid.h \
-common/print-utils.h common/rsp-low.h
+common/print-utils.h common/rsp-low.h nat/i386-dregs.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
diff --git a/gdb/gdbserver/i386-low.c b/gdb/gdbserver/i386-low.c
index 1c9d7aa..bd25694 100644
--- a/gdb/gdbserver/i386-low.c
+++ b/gdb/gdbserver/i386-low.c
@@ -146,9 +146,6 @@
 /* Did the watchpoint whose address is in the I'th register break?  */
 #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++)
-
 /* Types of operations supported by i386_handle_nonaligned_watchpoint.  */
 typedef enum { WP_INSERT, WP_REMOVE, WP_COUNT } i386_wp_op_t;
 
diff --git a/gdb/gdbserver/i386-low.h b/gdb/gdbserver/i386-low.h
index 3d3feb3..71f7c32 100644
--- a/gdb/gdbserver/i386-low.h
+++ b/gdb/gdbserver/i386-low.h
@@ -17,74 +17,12 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-/* Support for hardware watchpoints and breakpoints using the i386
-   debug registers.
-
-   This provides several functions for inserting and removing
-   hardware-assisted breakpoints and watchpoints, testing if one or
-   more of the watchpoints triggered and at what address, checking
-   whether a given region can be watched, etc.
-
-   The functions below implement debug registers sharing by reference
-   counts, and allow to watch regions up to 16 bytes long
-   (32 bytes on 64 bit hosts).  */
-
-/* Debug registers' indices.  */
-#define DR_FIRSTADDR 0
-#define DR_LASTADDR  3
-#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).  */
-
-/* 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];
-};
+
+#include "nat/i386-dregs.h"
 
 /* Initialize STATE.  */
 extern void i386_low_init_dregs (struct i386_debug_reg_state *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.  */
-extern int i386_dr_insert_watchpoint (struct i386_debug_reg_state *state,
-				      enum target_hw_bp_type type,
-				      CORE_ADDR addr,
-				      int len);
-
-/* 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_dr_remove_watchpoint (struct i386_debug_reg_state *state,
-				      enum target_hw_bp_type type,
-				      CORE_ADDR addr,
-				      int len);
-
-/* Return non-zero if we can watch a memory region that starts at
-   address ADDR and whose length is LEN bytes.  */
-extern int i386_dr_region_ok_for_watchpoint (struct i386_debug_reg_state *state,
-					     CORE_ADDR addr, int len);
-
-/* If the inferior has some break/watchpoint that triggered, set the
-   address associated with that break/watchpoint and return true.
-   Otherwise, return false.  */
-extern int i386_dr_stopped_data_address (struct i386_debug_reg_state *state,
-					 CORE_ADDR *addr_p);
-
-/* Return true if the inferior has some watchpoint that triggered.
-   Otherwise return false.  */
-extern int i386_dr_stopped_by_watchpoint (struct i386_debug_reg_state *state);
 \f
 /* Each target needs to provide several low-level functions
    that will be called to insert watchpoints and hardware breakpoints
diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c
index 86e655f..1dfa9ee 100644
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -171,9 +171,6 @@ struct i386_dr_low_type i386_dr_low;
 /* Did the watchpoint whose address is in the I'th register break?  */
 #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++)
-
 /* Per-process data.  We don't bind this to a per-inferior registry
    because of targets like x86 GNU/Linux that need to keep track of
    processes that aren't bound to any inferior (e.g., fork children,
diff --git a/gdb/i386-nat.h b/gdb/i386-nat.h
index ae2f1a7..1f4130c 100644
--- a/gdb/i386-nat.h
+++ b/gdb/i386-nat.h
@@ -23,6 +23,8 @@
 #ifndef I386_NAT_H
 #define I386_NAT_H 1
 
+#include "nat/i386-dregs.h"
+
 /* Hardware-assisted breakpoints and watchpoints.  */
 
 /* Add watchpoint methods to the provided target_ops.  
@@ -34,12 +36,7 @@ extern void i386_use_watchpoints (struct target_ops *);
 /* Support for hardware watchpoints and breakpoints using the i386
    debug registers.
 
-   This provides several functions for inserting and removing
-   hardware-assisted breakpoints and watchpoints, testing if one or
-   more of the watchpoints triggered and at what address, checking
-   whether a given region can be watched, etc.
-
-   In addition, each target should provide several low-level functions
+   Each target should provide several low-level functions
    regrouped into i386_dr_low_type struct below.  These functions
    that will be called to insert watchpoints and hardware breakpoints
    into the inferior, remove them, and check their status.  These
@@ -76,29 +73,6 @@ struct i386_dr_low_type
 
 extern struct i386_dr_low_type i386_dr_low;
 
-/* Debug registers' indices.  */
-#define DR_FIRSTADDR 0
-#define DR_LASTADDR  3
-#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).  */
-
-/* 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];
-};
-
 /* Use this function to set i386_dr_low debug_register_length field
    rather than setting it directly to check that the length is only
    set once.  It also enables the 'maint set/show show-debug-regs' 
diff --git a/gdb/nat/i386-dregs.h b/gdb/nat/i386-dregs.h
new file mode 100644
index 0000000..c17c6ae
--- /dev/null
+++ b/gdb/nat/i386-dregs.h
@@ -0,0 +1,138 @@
+/* Debug register code for the i386.
+
+   Copyright (C) 2009-2014 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 3 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, see <http://www.gnu.org/licenses/>.  */
+
+/* Support for hardware watchpoints and breakpoints using the i386
+   debug registers.
+
+   This provides several functions for inserting and removing
+   hardware-assisted breakpoints and watchpoints, testing if one or
+   more of the watchpoints triggered and at what address, checking
+   whether a given region can be watched, etc.
+
+   The functions below implement debug registers sharing by reference
+   counts, and allow to watch regions up to 16 bytes long
+   (32 bytes on 64 bit hosts).  */
+
+#ifndef I386_DREGS_H
+#define I386_DREGS_H 1
+
+/* Forward declaration.  */
+enum target_hw_bp_type;
+
+/* Debug registers' indices.  */
+#define DR_FIRSTADDR 0
+#define DR_LASTADDR  3
+#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).  */
+
+/* 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];
+};
+
+/* A macro to loop over all debug registers.  */
+#define ALL_DEBUG_REGISTERS(i)	for (i = 0; i < DR_NADDR; i++)
+\f
+/* High-level functions.  */
+
+/* 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_dr_insert_watchpoint (struct i386_debug_reg_state *state,
+				      enum target_hw_bp_type type,
+				      CORE_ADDR addr,
+				      int len);
+
+/* 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_dr_remove_watchpoint (struct i386_debug_reg_state *state,
+				      enum target_hw_bp_type type,
+				      CORE_ADDR addr,
+				      int len);
+
+/* Return non-zero if we can watch a memory region that starts at
+   address ADDR and whose length is LEN bytes.  */
+extern int i386_dr_region_ok_for_watchpoint (struct i386_debug_reg_state *state,
+					     CORE_ADDR addr, int len);
+
+/* If the inferior has some break/watchpoint that triggered, set the
+   address associated with that break/watchpoint and return true.
+   Otherwise, return false.  */
+extern int i386_dr_stopped_data_address (struct i386_debug_reg_state *state,
+					 CORE_ADDR *addr_p);
+
+/* Return true if the inferior has some watchpoint that triggered.
+   Otherwise return false.  */
+extern int i386_dr_stopped_by_watchpoint (struct i386_debug_reg_state *state);
+\f
+/* Low-level functions.  */
+
+/* Print the values of the mirrored debug registers.  */
+
+extern void i386_dr_show (struct i386_debug_reg_state *state,
+			  const char *func, CORE_ADDR addr,
+			  int len, enum target_hw_bp_type type);
+
+/* 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.  */
+
+extern unsigned i386_dr_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 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.  */
+
+extern int i386_dr_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
+   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.  */
+
+extern int i386_dr_remove_aligned_watchpoint (struct i386_debug_reg_state *state,
+					      CORE_ADDR addr,
+					      unsigned len_rw_bits);
+
+/* Update the inferior debug registers state, in STATE, with the
+   new debug registers state, in NEW_STATE.  */
+
+extern void i386_dr_update_inferior_debug_regs (struct i386_debug_reg_state *state,
+						struct i386_debug_reg_state *new_state);
+
+#endif /* I386_DREGS_H */

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 13/15] Move macros from i386-{nat,low}.c to i386-{nat,low}.h
  2014-06-17 16:54   ` Pedro Alves
@ 2014-06-18 11:33     ` Gary Benson
  2014-06-18 11:50       ` Pedro Alves
  0 siblings, 1 reply; 45+ messages in thread
From: Gary Benson @ 2014-06-18 11:33 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro Alves wrote:
> On 06/17/2014 03:12 PM, Gary Benson wrote:
> > This commit moves macros required by the soon-to-be-created
> > nat/i386-dregs.c into i386-{nat,low}.h.
> 
> To get rid of the remaining '#ifdef GDBSERVER's in the new
> i386-dregs.c, we could make gdbserver use the i386_dr_low
> vector too.  Did you look into that?

I'm not 100% sure I know what you mean.  The only '#ifdef GDBSERVER's
in i386-dregs.c are the one surrounding the #include files and the
one surrounding debug_printf.

It would be possible to make gdbserver use the i386_dr_low vector and
remove the macros from i386-{nat,low}.h, but you'd have to change the
function signatures in either gdb or gdbserver as the i386_dr_low
"set" functions take an argument (what to set it to) but the functions
in gdbserver don't.  I think in gdbserver they all get set at once,
but I have to confess I don't fully understand it yet.

I'll think about this some more.

> Meanwhile, this is already much better than what we have today.
> Thus, this is OK.

Ok, I pushed it.

Thanks,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 13/15] Move macros from i386-{nat,low}.c to i386-{nat,low}.h
  2014-06-18 11:33     ` Gary Benson
@ 2014-06-18 11:50       ` Pedro Alves
  0 siblings, 0 replies; 45+ messages in thread
From: Pedro Alves @ 2014-06-18 11:50 UTC (permalink / raw)
  To: Gary Benson; +Cc: gdb-patches

On 06/18/2014 12:33 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 06/17/2014 03:12 PM, Gary Benson wrote:
>>> This commit moves macros required by the soon-to-be-created
>>> nat/i386-dregs.c into i386-{nat,low}.h.
>>
>> To get rid of the remaining '#ifdef GDBSERVER's in the new
>> i386-dregs.c, we could make gdbserver use the i386_dr_low
>> vector too.  Did you look into that?
> 
> I'm not 100% sure I know what you mean.  The only '#ifdef GDBSERVER's
> in i386-dregs.c are the one surrounding the #include files and the
> one surrounding debug_printf.

Yes, that one.  The ideal end state builds the exact same code in
gdb and gdbserver.

> 
> It would be possible to make gdbserver use the i386_dr_low vector and
> remove the macros from i386-{nat,low}.h, but you'd have to change the
> function signatures in either gdb or gdbserver as the i386_dr_low
> "set" functions take an argument (what to set it to) but the functions
> in gdbserver don't.  I think in gdbserver they all get set at once,
> but I have to confess I don't fully understand it yet.

Normalizing this is fine, and actually desirable.  Reducing divergence
is what we're really after.

> I'll think about this some more.

Thanks.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 11/15] Rename functions and make nonstatic as necessary
  2014-06-18 10:25   ` Siva Chandra
@ 2014-06-18 13:30     ` Gary Benson
  2014-06-18 13:55       ` Pedro Alves
  0 siblings, 1 reply; 45+ messages in thread
From: Gary Benson @ 2014-06-18 13:30 UTC (permalink / raw)
  To: Siva Chandra, Pedro Alves; +Cc: gdb-patches

Siva Chandra wrote:
> On Tue, Jun 17, 2014 at 7:12 AM, Gary Benson <gbenson@redhat.com> wrote:
> > This commit renames the functions that are to be shared.
> > Functions to be shared that were static are made nonstatic.
> 
> I thinking making the functions nonstatic in this part was not
> correct. The build currently fails when built with
> "-Werror=missing-prototypes" as 14/15 has not yet been committed.

My apologies Siva.

Pedro, should I commit the remaining piece now, and work on the 
i386_insert_hw_breakpoint as a separate patch?

Thanks,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 14/15] Create nat/i386-dregs.c
  2014-06-17 16:56   ` Pedro Alves
@ 2014-06-18 13:43     ` Gary Benson
  0 siblings, 0 replies; 45+ messages in thread
From: Gary Benson @ 2014-06-18 13:43 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro Alves wrote:
> Hmm, there's still one remaining difference in gdb vs gdbserver,
> after this, in hw breakpoints.
> 
> i386_insert_hw_breakpoint and i386_remove_hw_breakpoint on
> the GDB side still have the "work on a local copy"
> bits, and the i386_dr_update_inferior_debug_regs and
> i386_dr_show calls:
[snip]
> 
> While on the gdbserver side, we just call i386_dr_insert_watchpoint:
[snip]

> Any reason GDB can't do the same?

Very good spot!

I'll work on this.  It also allows a bunch of those static calls to be
reverted back to nonstatic.  I like that (they made the exported
interface of i386-dregs ugly!)

I will work on a series to do this.

Thanks,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 11/15] Rename functions and make nonstatic as necessary
  2014-06-18 13:30     ` Gary Benson
@ 2014-06-18 13:55       ` Pedro Alves
  2014-06-18 14:03         ` Siva Chandra
  2014-06-18 14:09         ` Gary Benson
  0 siblings, 2 replies; 45+ messages in thread
From: Pedro Alves @ 2014-06-18 13:55 UTC (permalink / raw)
  To: Gary Benson, Siva Chandra, Pedro Alves; +Cc: gdb-patches

On 06/18/2014 02:28 PM, Gary Benson wrote:
> Siva Chandra wrote:
>> On Tue, Jun 17, 2014 at 7:12 AM, Gary Benson <gbenson@redhat.com> wrote:
>>> This commit renames the functions that are to be shared.
>>> Functions to be shared that were static are made nonstatic.
>>
>> I thinking making the functions nonstatic in this part was not
>> correct. The build currently fails when built with
>> "-Werror=missing-prototypes" as 14/15 has not yet been committed.
> 
> My apologies Siva.
> 
> Pedro, should I commit the remaining piece now, and work on the 
> i386_insert_hw_breakpoint as a separate patch?

As you've committed the header patch already, I think the issue
is actually already fixed?

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 11/15] Rename functions and make nonstatic as necessary
  2014-06-18 13:55       ` Pedro Alves
@ 2014-06-18 14:03         ` Siva Chandra
  2014-06-18 14:09         ` Gary Benson
  1 sibling, 0 replies; 45+ messages in thread
From: Siva Chandra @ 2014-06-18 14:03 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Gary Benson, gdb-patches

On Wed, Jun 18, 2014 at 6:55 AM, Pedro Alves <palves@redhat.com> wrote:
> On 06/18/2014 02:28 PM, Gary Benson wrote:
>> Siva Chandra wrote:
>>> On Tue, Jun 17, 2014 at 7:12 AM, Gary Benson <gbenson@redhat.com> wrote:
>>>> This commit renames the functions that are to be shared.
>>>> Functions to be shared that were static are made nonstatic.
>>>
>>> I thinking making the functions nonstatic in this part was not
>>> correct. The build currently fails when built with
>>> "-Werror=missing-prototypes" as 14/15 has not yet been committed.
>>
>> My apologies Siva.
>>
>> Pedro, should I commit the remaining piece now, and work on the
>> i386_insert_hw_breakpoint as a separate patch?
>
> As you've committed the header patch already, I think the issue
> is actually already fixed?

Yes, I just checked; the issue is already fixed.

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 11/15] Rename functions and make nonstatic as necessary
  2014-06-18 13:55       ` Pedro Alves
  2014-06-18 14:03         ` Siva Chandra
@ 2014-06-18 14:09         ` Gary Benson
  2014-06-18 14:18           ` Pedro Alves
  1 sibling, 1 reply; 45+ messages in thread
From: Gary Benson @ 2014-06-18 14:09 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Siva Chandra, gdb-patches

Pedro Alves wrote:
> On 06/18/2014 02:28 PM, Gary Benson wrote:
> > Siva Chandra wrote:
> > > On Tue, Jun 17, 2014 at 7:12 AM, Gary Benson <gbenson@redhat.com> wrote:
> > > > This commit renames the functions that are to be shared.
> > > > Functions to be shared that were static are made nonstatic.
> > >

> > > I thinking making the functions nonstatic in this part was
> > > not correct. The build currently fails when built with
> > > "-Werror=missing-prototypes" as 14/15 has not yet been
> > > committed.
> > 
> > My apologies Siva.
> > 
> > Pedro, should I commit the remaining piece now, and work on the
> > i386_insert_hw_breakpoint as a separate patch?
> 
> As you've committed the header patch already, I think the issue
> is actually already fixed?

Ah, ok, I misunderstood.  I can stop rushing!

I actually wish I hadn't committed the header patch now, the
i386_{insert,remove}_hw_breakpoint refactoring you spotted
means those functions didn't need exposing at all.

I'm working on a patch to revert part of that change: remove the
prototypes, make them static again, *and* rename the now static
functions them back to their original names.  The last point isn't
strictly necessary, but I don't like that the refactoring would
have renamed some functions it didn't need to.  I want to put the
old names back so that people familiar with that code don't see
more disruption than strictly necessary.

I was going to mail it alone (I thought Siva was stuck) but since
he's not I'll mail it as part of a three-patch series to complete
the refactoring.

Thanks,
Gary

-- 
http://gbenson.net/

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH 11/15] Rename functions and make nonstatic as necessary
  2014-06-18 14:09         ` Gary Benson
@ 2014-06-18 14:18           ` Pedro Alves
  0 siblings, 0 replies; 45+ messages in thread
From: Pedro Alves @ 2014-06-18 14:18 UTC (permalink / raw)
  To: Gary Benson; +Cc: Siva Chandra, gdb-patches

On 06/18/2014 03:09 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 06/18/2014 02:28 PM, Gary Benson wrote:
>>> Siva Chandra wrote:
>>>> On Tue, Jun 17, 2014 at 7:12 AM, Gary Benson <gbenson@redhat.com> wrote:
>>>>> This commit renames the functions that are to be shared.
>>>>> Functions to be shared that were static are made nonstatic.
>>>>
> 
>>>> I thinking making the functions nonstatic in this part was
>>>> not correct. The build currently fails when built with
>>>> "-Werror=missing-prototypes" as 14/15 has not yet been
>>>> committed.
>>>
>>> My apologies Siva.
>>>
>>> Pedro, should I commit the remaining piece now, and work on the
>>> i386_insert_hw_breakpoint as a separate patch?
>>
>> As you've committed the header patch already, I think the issue
>> is actually already fixed?
> 
> Ah, ok, I misunderstood.  I can stop rushing!
> 
> I actually wish I hadn't committed the header patch now, the
> i386_{insert,remove}_hw_breakpoint refactoring you spotted
> means those functions didn't need exposing at all.
> 
> I'm working on a patch to revert part of that change: remove the
> prototypes, make them static again, *and* rename the now static
> functions them back to their original names.  The last point isn't
> strictly necessary, but I don't like that the refactoring would
> have renamed some functions it didn't need to.  I want to put the
> old names back so that people familiar with that code don't see
> more disruption than strictly necessary.
> 
> I was going to mail it alone (I thought Siva was stuck) but since
> he's not I'll mail it as part of a three-patch series to complete
> the refactoring.

Sounds excellent.

Thanks,
-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 45+ messages in thread

end of thread, other threads:[~2014-06-18 14:18 UTC | newest]

Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-17 14:13 [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Gary Benson
2014-06-17 14:13 ` [PATCH 03/15] Remove unused macro Gary Benson
2014-06-17 15:35   ` Pedro Alves
2014-06-17 14:13 ` [PATCH 04/15] Comment changes Gary Benson
2014-06-17 15:45   ` Pedro Alves
2014-06-17 16:10     ` Gary Benson
2014-06-17 17:11       ` Pedro Alves
2014-06-17 14:13 ` [PATCH 01/15] Whitespace changes Gary Benson
2014-06-17 15:23   ` Pedro Alves
2014-06-17 14:40 ` [PATCH 14/15] Create nat/i386-dregs.c Gary Benson
2014-06-17 16:56   ` Pedro Alves
2014-06-18 13:43     ` Gary Benson
2014-06-17 14:40 ` [PATCH 06/15] Merge printing code Gary Benson
2014-06-17 16:01   ` Pedro Alves
2014-06-17 14:44 ` [PATCH 09/15] Merge i386_update_inferior_debug_regs Gary Benson
2014-06-17 16:06   ` Pedro Alves
2014-06-17 14:44 ` [PATCH 12/15] Create nat/i386-dregs.h Gary Benson
2014-06-17 16:48   ` Pedro Alves
2014-06-18 11:14     ` Gary Benson
2014-06-17 14:48 ` [PATCH 11/15] Rename functions and make nonstatic as necessary Gary Benson
2014-06-17 16:13   ` Pedro Alves
2014-06-18 10:25   ` Siva Chandra
2014-06-18 13:30     ` Gary Benson
2014-06-18 13:55       ` Pedro Alves
2014-06-18 14:03         ` Siva Chandra
2014-06-18 14:09         ` Gary Benson
2014-06-18 14:18           ` Pedro Alves
2014-06-17 14:48 ` [PATCH 07/15] Merge error handling Gary Benson
2014-06-17 16:02   ` Pedro Alves
2014-06-17 14:53 ` [PATCH 10/15] Abstract i386_dr_low access Gary Benson
2014-06-17 16:10   ` Pedro Alves
2014-06-17 14:53 ` [PATCH 02/15] Remove unnecessary prototypes Gary Benson
2014-06-17 15:26   ` Pedro Alves
2014-06-17 14:53 ` [PATCH 08/15] Rename maint_show_dr to debug_hw_points Gary Benson
2014-06-17 16:03   ` Pedro Alves
2014-06-17 14:53 ` [PATCH 05/15] Add a const Gary Benson
2014-06-17 15:46   ` Pedro Alves
2014-06-17 14:58 ` [PATCH 15/15] Buildsystem changes Gary Benson
2014-06-17 16:41   ` Pedro Alves
2014-06-17 14:58 ` [PATCH 13/15] Move macros from i386-{nat,low}.c to i386-{nat,low}.h Gary Benson
2014-06-17 16:54   ` Pedro Alves
2014-06-18 11:33     ` Gary Benson
2014-06-18 11:50       ` Pedro Alves
2014-06-17 17:00 ` [PATCH 00/15] Refactor shared code in i386-{nat,low}.[ch] Pedro Alves
2014-06-18 10:49   ` Gary Benson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).