[PATCH 10/13 v2] Linux x86 low-level debug register comment synchronization

[Gary Benson <gbenson@redhat.com>]

This commit updates comments in the low-level debug register code for
Linux x86, making GDB's and gdbserver's implementations identical.

gdb/ChangeLog:

	* x86-linux-nat.c (x86_linux_dr_get): Updated comments.
	(x86_linux_dr_set): Likewise.
	(x86_linux_dr_get_addr): Likewise.
	(x86_linux_dr_get_control): Likewise.
	(x86_linux_dr_get_status): Likewise.
	(update_debug_registers_callback): Likewise.
	(x86_linux_dr_set_control): Likewise.
	(x86_linux_dr_set_addr): Likewise.
	(x86_linux_prepare_to_resume): Likewise.
	(x86_linux_new_thread): Likewise.

gdb/gdbserver/ChangeLog:

	* linux-x86-low.c (x86_linux_dr_get): Updated comments.
	(x86_linux_dr_set): Likewise.
	(update_debug_registers_callback): Likewise.
	(x86_linux_dr_set_addr): Likewise.
	(x86_linux_dr_get_addr): Likewise.
	(x86_linux_dr_set_control): Likewise.
	(x86_linux_dr_get_control): Likewise.
	(x86_linux_dr_get_status): Likewise.
	(x86_linux_prepare_to_resume): Likewise.
---
 gdb/ChangeLog                 |   13 +++++++++
 gdb/gdbserver/ChangeLog       |   12 ++++++++
 gdb/gdbserver/linux-x86-low.c |   48 +++++++++++++++++++++------------
 gdb/x86-linux-nat.c           |   59 +++++++++++++++++++---------------------
 4 files changed, 84 insertions(+), 48 deletions(-)

diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 3064791..f4d6437 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -523,6 +523,8 @@ x86_breakpoint_at (CORE_ADDR pc)
 \f
 /* Support for debug registers.  */
 
+/* Get debug register REGNUM value from the LWP specified by PTID.  */
+
 static unsigned long
 x86_linux_dr_get (ptid_t ptid, int regnum)
 {
@@ -541,6 +543,8 @@ x86_linux_dr_get (ptid_t ptid, int regnum)
   return value;
 }
 
+/* Set debug register REGNUM to VALUE in the LWP specified by PTID.  */
+
 static void
 x86_linux_dr_set (ptid_t ptid, int regnum, unsigned long value)
 {
@@ -556,27 +560,29 @@ x86_linux_dr_set (ptid_t ptid, int regnum, unsigned long value)
     perror_with_name (_("Couldn't write debug register"));
 }
 
+/* Callback for iterate_over_lwps.  Mark that our local mirror of
+   LWP's debug registers has been changed, and cause LWP to stop if
+   it isn't already.  Values are written from our local mirror to
+   the actual debug registers immediately prior to LWP resuming.  */
+
 static int
 update_debug_registers_callback (struct lwp_info *lwp, void *arg)
 {
-  /* The actual update is done later just before resuming the lwp,
-     we just mark that the registers need updating.  */
   lwp_set_debug_registers_changed (lwp, 1);
 
-  /* If the lwp isn't stopped, force it to momentarily pause, so
-     we can update its debug registers.  */
   if (!lwp_is_stopped (lwp))
     linux_stop_lwp (lwp);
 
+  /* Continue the iteration.  */
   return 0;
 }
 
-/* Update the inferior's debug register REGNUM from STATE.  */
+/* Store ADDR in debug register REGNUM of all LWPs of the current
+   inferior.  */
 
 static void
 x86_linux_dr_set_addr (int regnum, CORE_ADDR addr)
 {
-  /* Only update the threads of this process.  */
   ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (current_lwp_ptid ()));
 
   gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);
@@ -584,7 +590,8 @@ x86_linux_dr_set_addr (int regnum, CORE_ADDR addr)
   iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
 }
 
-/* Return the inferior's debug register REGNUM.  */
+/* Return the address stored in the current inferior's debug register
+   REGNUM.  */
 
 static CORE_ADDR
 x86_linux_dr_get_addr (int regnum)
@@ -594,18 +601,19 @@ x86_linux_dr_get_addr (int regnum)
   return x86_linux_dr_get (current_lwp_ptid (), regnum);
 }
 
-/* Update the inferior's DR7 debug control register from STATE.  */
+/* Store CONTROL in the debug control registers of all LWPs of the
+   current inferior.  */
 
 static void
 x86_linux_dr_set_control (unsigned long control)
 {
-  /* Only update the threads of this process.  */
   ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (current_lwp_ptid ()));
 
   iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
 }
 
-/* Return the inferior's DR7 debug control register.  */
+/* Return the value stored in the current inferior's debug control
+   register.  */
 
 static unsigned long
 x86_linux_dr_get_control (void)
@@ -613,8 +621,8 @@ x86_linux_dr_get_control (void)
   return x86_linux_dr_get (current_lwp_ptid (), DR_CONTROL);
 }
 
-/* Get the value of the DR6 debug status register from the inferior
-   and record it in STATE.  */
+/* Return the value stored in the current inferior's debug status
+   register.  */
 
 static unsigned long
 x86_linux_dr_get_status (void)
@@ -755,8 +763,8 @@ x86_debug_reg_state (pid_t pid)
   return &proc->private->arch_private->debug_reg_state;
 }
 
-/* Called when resuming a thread.
-   If the debug regs have changed, update the thread's copies.  */
+/* Called prior to resuming a thread.  Updates the thread's debug
+   registers if the values in our local mirror have been changed.  */
 
 static void
 x86_linux_prepare_to_resume (struct lwp_info *lwp)
@@ -770,6 +778,12 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
 	= x86_debug_reg_state (ptid_get_pid (ptid));
       int i;
 
+      /* Prior to Linux kernel 2.6.33 commit
+	 72f674d203cd230426437cdcf7dd6f681dad8b0d, setting DR0-3 to
+	 a value that did not match what was enabled in DR_CONTROL
+	 resulted in EINVAL.  To avoid this we zero DR_CONTROL before
+	 writing address registers, only writing DR_CONTROL's actual
+	 value once all the addresses are in place.  */
       x86_linux_dr_set (ptid, DR_CONTROL, 0);
 
       ALL_DEBUG_ADDRESS_REGISTERS (i)
@@ -778,12 +792,12 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
 	    x86_linux_dr_set (ptid, i, state->dr_mirror[i]);
 
 	    /* If we're setting a watchpoint, any change the inferior
-	       had done itself to the debug registers needs to be
-	       discarded, otherwise, x86_dr_stopped_data_address can
-	       get confused.  */
+	       has made to its debug registers needs to be discarded
+	       to avoid x86_stopped_data_address getting confused.  */
 	    clear_status = 1;
 	  }
 
+      /* If DR_CONTROL is supposed to be zero then it's already set.  */
       if (state->dr_control_mirror != 0)
 	x86_linux_dr_set (ptid, DR_CONTROL, state->dr_control_mirror);
 
diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
index c5c8576..d806474 100644
--- a/gdb/x86-linux-nat.c
+++ b/gdb/x86-linux-nat.c
@@ -55,7 +55,7 @@ int have_ptrace_getregset = -1;
 
 /* Support for debug registers.  */
 
-/* Get debug register REGNUM value from only the one LWP of PTID.  */
+/* Get debug register REGNUM value from the LWP specified by PTID.  */
 
 static unsigned long
 x86_linux_dr_get (ptid_t ptid, int regnum)
@@ -75,7 +75,7 @@ x86_linux_dr_get (ptid_t ptid, int regnum)
   return value;
 }
 
-/* Set debug register REGNUM to VALUE in only the one LWP of PTID.  */
+/* Set debug register REGNUM to VALUE in the LWP specified by PTID.  */
 
 static void
 x86_linux_dr_set (ptid_t ptid, int regnum, unsigned long value)
@@ -92,18 +92,19 @@ x86_linux_dr_set (ptid_t ptid, int regnum, unsigned long value)
     perror_with_name (_("Couldn't write debug register"));
 }
 
-/* Return the inferior's debug register REGNUM.  */
+/* Return the address stored in the current inferior's debug register
+   REGNUM.  */
 
 static CORE_ADDR
 x86_linux_dr_get_addr (int regnum)
 {
-  /* DR6 and DR7 are retrieved with some other way.  */
   gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);
 
   return x86_linux_dr_get (current_lwp_ptid (), regnum);
 }
 
-/* Return the inferior's DR7 debug control register.  */
+/* Return the value stored in the current inferior's debug control
+   register.  */
 
 static unsigned long
 x86_linux_dr_get_control (void)
@@ -111,7 +112,8 @@ x86_linux_dr_get_control (void)
   return x86_linux_dr_get (current_lwp_ptid (), DR_CONTROL);
 }
 
-/* Get DR_STATUS from only the one LWP of INFERIOR_PTID.  */
+/* Return the value stored in the current inferior's debug status
+   register.  */
 
 static unsigned long
 x86_linux_dr_get_status (void)
@@ -119,18 +121,16 @@ x86_linux_dr_get_status (void)
   return x86_linux_dr_get (current_lwp_ptid (), DR_STATUS);
 }
 
-/* Callback for iterate_over_lwps.  Update the debug registers of
-   LWP.  */
+/* Callback for iterate_over_lwps.  Mark that our local mirror of
+   LWP's debug registers has been changed, and cause LWP to stop if
+   it isn't already.  Values are written from our local mirror to
+   the actual debug registers immediately prior to LWP resuming.  */
 
 static int
 update_debug_registers_callback (struct lwp_info *lwp, void *arg)
 {
-  /* The actual update is done later just before resuming the lwp, we
-     just mark that the registers need updating.  */
   lwp_set_debug_registers_changed (lwp, 1);
 
-  /* If the lwp isn't stopped, force it to momentarily pause, so we
-     can update its debug registers.  */
   if (!lwp_is_stopped (lwp))
     linux_stop_lwp (lwp);
 
@@ -138,7 +138,8 @@ update_debug_registers_callback (struct lwp_info *lwp, void *arg)
   return 0;
 }
 
-/* Set DR_CONTROL to CONTROL in all LWPs of the current inferior.  */
+/* Store CONTROL in the debug control registers of all LWPs of the
+   current inferior.  */
 
 static void
 x86_linux_dr_set_control (unsigned long control)
@@ -148,7 +149,7 @@ x86_linux_dr_set_control (unsigned long control)
   iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
 }
 
-/* Set address REGNUM (zero based) to ADDR in all LWPs of the current
+/* Store ADDR in debug register REGNUM of all LWPs of the current
    inferior.  */
 
 static void
@@ -161,8 +162,8 @@ x86_linux_dr_set_addr (int regnum, CORE_ADDR addr)
   iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
 }
 
-/* Called when resuming a thread.
-   If the debug regs have changed, update the thread's copies.  */
+/* Called prior to resuming a thread.  Updates the thread's debug
+   registers if the values in our local mirror have been changed.  */
 
 static void
 x86_linux_prepare_to_resume (struct lwp_info *lwp)
@@ -176,16 +177,12 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
 	= x86_debug_reg_state (ptid_get_pid (ptid));
       int i;
 
-      /* On Linux kernel before 2.6.33 commit
-	 72f674d203cd230426437cdcf7dd6f681dad8b0d
-	 if you enable a breakpoint by the DR_CONTROL bits you need to have
-	 already written the corresponding DR_FIRSTADDR...DR_LASTADDR registers.
-
-	 Ensure DR_CONTROL gets written as the very last register here.  */
-
-      /* Clear DR_CONTROL first.  In some cases, setting DR0-3 to a
-	 value that doesn't match what is enabled in DR_CONTROL
-	 results in EINVAL.  */
+      /* Prior to Linux kernel 2.6.33 commit
+	 72f674d203cd230426437cdcf7dd6f681dad8b0d, setting DR0-3 to
+	 a value that did not match what was enabled in DR_CONTROL
+	 resulted in EINVAL.  To avoid this we zero DR_CONTROL before
+	 writing address registers, only writing DR_CONTROL's actual
+	 value once all the addresses are in place.  */
       x86_linux_dr_set (ptid, DR_CONTROL, 0);
 
       ALL_DEBUG_ADDRESS_REGISTERS (i)
@@ -194,14 +191,12 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
 	    x86_linux_dr_set (ptid, i, state->dr_mirror[i]);
 
 	    /* If we're setting a watchpoint, any change the inferior
-	       had done itself to the debug registers needs to be
-	       discarded, otherwise, x86_stopped_data_address can get
-	       confused.  */
+	       has made to its debug registers needs to be discarded
+	       to avoid x86_stopped_data_address getting confused.  */
 	    clear_status = 1;
 	  }
 
-      /* If DR_CONTROL is supposed to be zero, we've already set it
-	 above.  */
+      /* If DR_CONTROL is supposed to be zero then it's already set.  */
       if (state->dr_control_mirror != 0)
 	x86_linux_dr_set (ptid, DR_CONTROL, state->dr_control_mirror);
 
@@ -212,6 +207,8 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
     x86_linux_dr_set (ptid, DR_STATUS, 0);
 }
 
+/* Called when a new thread is detected.  */
+
 static void
 x86_linux_new_thread (struct lwp_info *lwp)
 {
-- 
1.7.1