[PATCH 00/13 v2] Refactor low-level Linux x86 debug register code

[PATCH 00/13 v2] Refactor low-level Linux x86 debug register code

[Gary Benson]

Hi all,

This series refactors the low-level Linux x86 debug register code
in x86-linux-nat.c and gdbserver/linux-x86-low.c into new shared
files nat/x86-linux.[ch] and nat/x86-linux-dregs.[ch].

The first ten patches smooth out various differences between the
two implementations, and the final three patches perform the code
reorganization.

This series replaces the previous version that I retracted after
renaming gdbserver's current_inferior as current_thread.  This
series has been reworked to reflect this and is substantially
cleaner.

Built and regtested on RHEL 6.5 x86_64 (native and gdbserver).

Ok to commit?

Thanks,
Gary

--

[PATCH 04/13 v2] Make linux_stop_lwp be a shared function

[Gary Benson]

Both GDB and gdbserver had linux_stop_lwp functions with identical
declarations.  This commit moves these to nat/linux-nat.h to allow
shared code to use the function.

gdb/ChangeLog:

	* linux-nat.h (linux_stop_lwp): Move declaration to...
	* nat/linux-nat.h (linux_stop_lwp): New declaration.

gdb/gdbserver/ChangeLog:

	* linux-low.h (linux_stop_lwp): Remove declaration.
---
 gdb/ChangeLog             |    5 +++++
 gdb/gdbserver/ChangeLog   |    4 ++++
 gdb/gdbserver/linux-low.h |    2 +-
 gdb/linux-nat.h           |    2 +-
 gdb/nat/linux-nat.h       |    5 +++++
 5 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 11d73f3..697e0fb 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -359,7 +359,7 @@ int linux_attach_lwp (ptid_t ptid);
 char *linux_attach_fail_reason_string (ptid_t ptid, int err);
 
 struct lwp_info *find_lwp_pid (ptid_t ptid);
-void linux_stop_lwp (struct lwp_info *lwp);
+/* For linux_stop_lwp see nat/linux-nat.h.  */
 
 #ifdef HAVE_LINUX_REGSETS
 void initialize_regsets_info (struct regsets_info *regsets_info);
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index 0195c5a..e1edbf8 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -124,7 +124,7 @@ void linux_proc_pending_signals (int pid, sigset_t *pending,
 
 extern int lin_lwp_attach_lwp (ptid_t ptid);
 
-extern void linux_stop_lwp (struct lwp_info *lwp);
+/* For linux_stop_lwp see nat/linux-nat.h.  */
 
 /* Create a prototype generic GNU/Linux target.  The client can
    override it with local methods.  */
diff --git a/gdb/nat/linux-nat.h b/gdb/nat/linux-nat.h
index a2a0a98..da94f2f 100644
--- a/gdb/nat/linux-nat.h
+++ b/gdb/nat/linux-nat.h
@@ -45,4 +45,9 @@ extern struct lwp_info *iterate_over_lwps (ptid_t filter,
 					   iterate_over_lwps_ftype callback,
 					   void *data);
 
+/* Cause LWP to stop.  This function must be provided by the
+   client.  */
+
+extern void linux_stop_lwp (struct lwp_info *lwp);
+
 #endif /* LINUX_NAT_H */
-- 
1.7.1

Re: [PATCH 04/13 v2] Make linux_stop_lwp be a shared function

[Pedro Alves]

On 10/09/2014 10:52 AM, Gary Benson wrote:
> Both GDB and gdbserver had linux_stop_lwp functions with identical
> declarations.  This commit moves these to nat/linux-nat.h to allow
> shared code to use the function.
> 
> gdb/ChangeLog:
> 
> 	* linux-nat.h (linux_stop_lwp): Move declaration to...
> 	* nat/linux-nat.h (linux_stop_lwp): New declaration.
> 
> gdb/gdbserver/ChangeLog:
> 
> 	* linux-low.h (linux_stop_lwp): Remove declaration.

OK.

Thanks,
Pedro Alves

[PATCH 01/13 v2] Introduce current_lwp_ptid

[Gary Benson]

This commit introduces a new function, current_lwp_ptid, that
shared Linux code can use to obtain the ptid of the current
lightweight process.

gdb/ChangeLog:

	* nat/linux-nat.h (current_lwp_ptid): New declaration.
	* linux-nat.c (current_lwp_ptid): New function.
	* x86-linux-nat.c: Include nat/linux-nat.h.
	(x86_linux_dr_get_addr): Use current_lwp_ptid.
	(x86_linux_dr_get_control): Likewise.
	(x86_linux_dr_get_status): Likewise.
	(x86_linux_dr_set_control): Likewise.
	(x86_linux_dr_set_addr): Likewise.

gdb/gdbserver/ChangeLog:

	* linux-low.c (current_lwp_ptid): New function.
	* linux-x86-low.c: Include nat/linux-nat.h.
	(x86_dr_low_get_addr): Use current_lwp_ptid.
	(x86_dr_low_get_control): Likewise.
	(x86_dr_low_get_status): Likewise.
---
 gdb/ChangeLog                 |   11 +++++++++++
 gdb/gdbserver/ChangeLog       |    8 ++++++++
 gdb/gdbserver/linux-low.c     |    8 ++++++++
 gdb/gdbserver/linux-x86-low.c |   13 ++++---------
 gdb/linux-nat.c               |    9 +++++++++
 gdb/nat/linux-nat.h           |    5 +++++
 gdb/x86-linux-nat.c           |   11 ++++++-----
 7 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 8776670..6e1ed8a 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -6035,6 +6035,14 @@ linux_low_read_btrace (struct btrace_target_info *tinfo, struct buffer *buffer,
 }
 #endif /* HAVE_LINUX_BTRACE */
 
+/* See common/common-inferior.h.  */
+
+ptid_t
+current_lwp_ptid (void)
+{
+  return ptid_of (current_thread);
+}
+
 static struct target_ops linux_target_ops = {
   linux_create_inferior,
   linux_attach,
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 838e7c9..d3ca298 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -37,6 +37,7 @@
 #include "tdesc.h"
 #include "tracepoint.h"
 #include "ax.h"
+#include "nat/linux-nat.h"
 
 #ifdef __x86_64__
 /* Defined in auto-generated file amd64-linux.c.  */
@@ -602,11 +603,9 @@ x86_dr_low_set_addr (int regnum, CORE_ADDR addr)
 static CORE_ADDR
 x86_dr_low_get_addr (int regnum)
 {
-  ptid_t ptid = ptid_of (current_thread);
-
   gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);
 
-  return x86_linux_dr_get (ptid, regnum);
+  return x86_linux_dr_get (current_lwp_ptid (), regnum);
 }
 
 /* Update the inferior's DR7 debug control register from STATE.  */
@@ -625,9 +624,7 @@ x86_dr_low_set_control (unsigned long control)
 static unsigned long
 x86_dr_low_get_control (void)
 {
-  ptid_t ptid = ptid_of (current_thread);
-
-  return x86_linux_dr_get (ptid, DR_CONTROL);
+  return x86_linux_dr_get (current_lwp_ptid (), DR_CONTROL);
 }
 
 /* Get the value of the DR6 debug status register from the inferior
@@ -636,9 +633,7 @@ x86_dr_low_get_control (void)
 static unsigned long
 x86_dr_low_get_status (void)
 {
-  ptid_t ptid = ptid_of (current_thread);
-
-  return x86_linux_dr_get (ptid, DR_STATUS);
+  return x86_linux_dr_get (current_lwp_ptid (), DR_STATUS);
 }
 
 /* Low-level function vector.  */
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 396c30c..7d84589 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4769,6 +4769,15 @@ linux_nat_get_siginfo (ptid_t ptid, siginfo_t *siginfo)
   return 1;
 }
 
+/* See nat/linux-nat.h.  */
+
+ptid_t
+current_lwp_ptid (void)
+{
+  gdb_assert (ptid_lwp_p (inferior_ptid));
+  return inferior_ptid;
+}
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 extern initialize_file_ftype _initialize_linux_nat;
 
diff --git a/gdb/nat/linux-nat.h b/gdb/nat/linux-nat.h
index 83a6d91..71d4ee8 100644
--- a/gdb/nat/linux-nat.h
+++ b/gdb/nat/linux-nat.h
@@ -25,4 +25,9 @@
    instead SIGTRAP with bit 7 set.  */
 #define SYSCALL_SIGTRAP (SIGTRAP | 0x80)
 
+/* Return the ptid of the current lightweight process.  This function
+   must be provided by the client. */
+
+extern ptid_t current_lwp_ptid (void);
+
 #endif /* LINUX_NAT_H */
diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
index b2141eb..7a80991 100644
--- a/gdb/x86-linux-nat.c
+++ b/gdb/x86-linux-nat.c
@@ -38,6 +38,7 @@
 #endif
 #include "x86-xstate.h"
 #include "nat/linux-btrace.h"
+#include "nat/linux-nat.h"
 
 /* Per-thread arch-specific data we want to keep.  */
 
@@ -98,7 +99,7 @@ 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 (inferior_ptid, regnum);
+  return x86_linux_dr_get (current_lwp_ptid (), regnum);
 }
 
 /* Return the inferior's DR7 debug control register.  */
@@ -106,7 +107,7 @@ x86_linux_dr_get_addr (int regnum)
 static unsigned long
 x86_linux_dr_get_control (void)
 {
-  return x86_linux_dr_get (inferior_ptid, DR_CONTROL);
+  return x86_linux_dr_get (current_lwp_ptid (), DR_CONTROL);
 }
 
 /* Get DR_STATUS from only the one LWP of INFERIOR_PTID.  */
@@ -114,7 +115,7 @@ x86_linux_dr_get_control (void)
 static unsigned long
 x86_linux_dr_get_status (void)
 {
-  return x86_linux_dr_get (inferior_ptid, DR_STATUS);
+  return x86_linux_dr_get (current_lwp_ptid (), DR_STATUS);
 }
 
 /* Callback for iterate_over_lwps.  Update the debug registers of
@@ -144,7 +145,7 @@ update_debug_registers_callback (struct lwp_info *lwp, void *arg)
 static void
 x86_linux_dr_set_control (unsigned long control)
 {
-  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
+  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (current_lwp_ptid ()));
 
   iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
 }
@@ -155,7 +156,7 @@ x86_linux_dr_set_control (unsigned long control)
 static void
 x86_linux_dr_set_addr (int regnum, CORE_ADDR addr)
 {
-  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
+  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (current_lwp_ptid ()));
 
   gdb_assert (regnum >= 0 && regnum <= DR_LASTADDR - DR_FIRSTADDR);
 
-- 
1.7.1

Re: [PATCH 01/13 v2] Introduce current_lwp_ptid

[Pedro Alves]

On 10/09/2014 10:52 AM, Gary Benson wrote:
> This commit introduces a new function, current_lwp_ptid, that
> shared Linux code can use to obtain the ptid of the current
> lightweight process.

OK.

Thanks,
Pedro Alves

Re: [PATCH 01/13 v2] Introduce current_lwp_ptid

[Doug Evans]

On Tue, Oct 28, 2014 at 5:55 AM, Pedro Alves <palves@redhat.com> wrote:
> On 10/09/2014 10:52 AM, Gary Benson wrote:
>> This commit introduces a new function, current_lwp_ptid, that
>> shared Linux code can use to obtain the ptid of the current
>> lightweight process.
>
> OK.

Hi.  The name bothers me enough to speak up.

I'm ok with lwp being a member of ptid, but we're essentially
replacing "thread" with "lwp".
Is there a particular reason current_lwp_ptid is chosen over
current_thread_ptid?

Re: [PATCH 01/13 v2] Introduce current_lwp_ptid

[Doug Evans]

On Tue, Oct 28, 2014 at 9:44 AM, Doug Evans <xdje42@gmail.com> wrote:
> On Tue, Oct 28, 2014 at 5:55 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 10/09/2014 10:52 AM, Gary Benson wrote:
>>> This commit introduces a new function, current_lwp_ptid, that
>>> shared Linux code can use to obtain the ptid of the current
>>> lightweight process.
>>
>> OK.
>
> Hi.  The name bothers me enough to speak up.
>
> I'm ok with lwp being a member of ptid, but we're essentially
> replacing "thread" with "lwp".
> Is there a particular reason current_lwp_ptid is chosen over
> current_thread_ptid?

Hi.
In an attempt to answer my own question, I can imagine that "lwp" is
the linux backend's term for "thread".  OK.

[This is probably written down somewhere, but a comment to that effect
in the definition of current_lwp_ptid would be helpful.]

Re: [PATCH 01/13 v2] Introduce current_lwp_ptid

[Doug Evans]

On Tue, Oct 28, 2014 at 10:12 AM, Doug Evans <xdje42@gmail.com> wrote:
> On Tue, Oct 28, 2014 at 9:44 AM, Doug Evans <xdje42@gmail.com> wrote:
>> On Tue, Oct 28, 2014 at 5:55 AM, Pedro Alves <palves@redhat.com> wrote:
>>> On 10/09/2014 10:52 AM, Gary Benson wrote:
>>>> This commit introduces a new function, current_lwp_ptid, that
>>>> shared Linux code can use to obtain the ptid of the current
>>>> lightweight process.
>>>
>>> OK.
>>
>> Hi.  The name bothers me enough to speak up.
>>
>> I'm ok with lwp being a member of ptid, but we're essentially
>> replacing "thread" with "lwp".
>> Is there a particular reason current_lwp_ptid is chosen over
>> current_thread_ptid?
>
> Hi.
> In an attempt to answer my own question, I can imagine that "lwp" is
> the linux backend's term for "thread".  OK.
>
> [This is probably written down somewhere, but a comment to that effect
> in the definition of current_lwp_ptid would be helpful.]

Well, OK, to be more precise s/in the definition/in the declaration/.

Re: [PATCH 01/13 v2] Introduce current_lwp_ptid

[Pedro Alves]

On 10/28/2014 04:44 PM, Doug Evans wrote:

> Is there a particular reason current_lwp_ptid is chosen over
> current_thread_ptid?

For-specific Linux native code, it doesn't really matter that much
to call something "thread" or "lwp" nowadays, given with NPTL, we
assume a 1:1 model.  But this is native Linux code working at the
lwp level.  The code around this will end up calling iterate_over_lwps.
And then x86_linux_dr_get thinks in terms of lwps too.  Likewise a
all the x86 Linux debug regs related code touched or added by the
rest of the series.  Using "lwp" here is more consistent.

Thanks,
Pedro Alves

Re: [PATCH 01/13 v2] Introduce current_lwp_ptid

[Doug Evans]

On Tue, Oct 28, 2014 at 10:04 AM, Pedro Alves <palves@redhat.com> wrote:
> On 10/28/2014 04:44 PM, Doug Evans wrote:
>
>> Is there a particular reason current_lwp_ptid is chosen over
>> current_thread_ptid?
>
> For-specific Linux native code, it doesn't really matter that much
> to call something "thread" or "lwp" nowadays, given with NPTL, we
> assume a 1:1 model.  But this is native Linux code working at the
> lwp level.  The code around this will end up calling iterate_over_lwps.
> And then x86_linux_dr_get thinks in terms of lwps too.  Likewise a
> all the x86 Linux debug regs related code touched or added by the
> rest of the series.  Using "lwp" here is more consistent.

Ergo my followup request:

Can a comment please be added to the declaration of current_lwp_ptid
explaining this?
[I don't have a strong preference for documenting this naming choice
with this particular function, but it's as good a place as any for
me.]

Re: [PATCH 01/13 v2] Introduce current_lwp_ptid

[Doug Evans]

Gary Benson writes:
 > This commit introduces a new function, current_lwp_ptid, that
 > shared Linux code can use to obtain the ptid of the current
 > lightweight process.
 > 
 > gdb/ChangeLog:
 > 
 > 	* nat/linux-nat.h (current_lwp_ptid): New declaration.
 > 	* linux-nat.c (current_lwp_ptid): New function.
 > 	* x86-linux-nat.c: Include nat/linux-nat.h.
 > 	(x86_linux_dr_get_addr): Use current_lwp_ptid.
 > 	(x86_linux_dr_get_control): Likewise.
 > 	(x86_linux_dr_get_status): Likewise.
 > 	(x86_linux_dr_set_control): Likewise.
 > 	(x86_linux_dr_set_addr): Likewise.
 > 
 > gdb/gdbserver/ChangeLog:
 > 
 > 	* linux-low.c (current_lwp_ptid): New function.
 > 	* linux-x86-low.c: Include nat/linux-nat.h.
 > 	(x86_dr_low_get_addr): Use current_lwp_ptid.
 > 	(x86_dr_low_get_control): Likewise.
 > 	(x86_dr_low_get_status): Likewise.
 > ---
 >  gdb/ChangeLog                 |   11 +++++++++++
 >  gdb/gdbserver/ChangeLog       |    8 ++++++++
 >  gdb/gdbserver/linux-low.c     |    8 ++++++++
 >  gdb/gdbserver/linux-x86-low.c |   13 ++++---------
 >  gdb/linux-nat.c               |    9 +++++++++
 >  gdb/nat/linux-nat.h           |    5 +++++
 >  gdb/x86-linux-nat.c           |   11 ++++++-----
 >  7 files changed, 51 insertions(+), 14 deletions(-)
 > 
 > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
 > index 8776670..6e1ed8a 100644
 > --- a/gdb/gdbserver/linux-low.c
 > +++ b/gdb/gdbserver/linux-low.c
 > @@ -6035,6 +6035,14 @@ linux_low_read_btrace (struct btrace_target_info *tinfo, struct buffer *buffer,
 >  }
 >  #endif /* HAVE_LINUX_BTRACE */
 >  
 > +/* See common/common-inferior.h.  */
 > +
 > +ptid_t
 > +current_lwp_ptid (void)
 > +{
 > +  return ptid_of (current_thread);
 > +}
 > +

Hi.

Nit I just noticed.  s,common/common-inferior.h,nat/linux-nat.h,

[PATCH 02/13 v2] Add x86_debug_reg_state to gdbserver

[Gary Benson]

This commit introduces a new function, x86_debug_reg_state, that
shared x86 code can use to access the local mirror of a process's
debug registers.  This function already existed in GDB and was
in use by GDB's x86_linux_prepare_to_resume.  An equivalent was
written for gdbserver and gdbserver's x86_linux_prepare_to_resume
was modified to use it.

gdb/ChangeLog:

	* x86-nat.h (x86_debug_reg_state): Move declaration to...
	* nat/x86-dregs.h (x86_debug_reg_state): New declaration.

gdb/gdbserver/ChangeLog:

	* linux-x86-low.c (x86_debug_reg_state): New function.
	(x86_linux_prepare_to_resume): Use the above.
---
 gdb/ChangeLog                 |    5 +++++
 gdb/gdbserver/ChangeLog       |    5 +++++
 gdb/gdbserver/linux-x86-low.c |   16 ++++++++++++----
 gdb/nat/x86-dregs.h           |    5 +++++
 gdb/x86-nat.h                 |    5 -----
 5 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index d3ca298..e8c6a9c 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -763,6 +763,16 @@ x86_linux_new_thread (void)
   return info;
 }
 
+/* See nat/x86-dregs.h.  */
+
+struct x86_debug_reg_state *
+x86_debug_reg_state (pid_t pid)
+{
+  struct process_info *proc = find_process_pid (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.  */
 
@@ -774,11 +784,9 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
 
   if (lwp->arch_private->debug_registers_changed)
     {
-      int i;
-      int pid = ptid_get_pid (ptid);
-      struct process_info *proc = find_process_pid (pid);
       struct x86_debug_reg_state *state
-	= &proc->private->arch_private->debug_reg_state;
+	= x86_debug_reg_state (ptid_get_pid (ptid));
+      int i;
 
       x86_linux_dr_set (ptid, DR_CONTROL, 0);
 
diff --git a/gdb/nat/x86-dregs.h b/gdb/nat/x86-dregs.h
index aebcbce..104d033 100644
--- a/gdb/nat/x86-dregs.h
+++ b/gdb/nat/x86-dregs.h
@@ -93,6 +93,11 @@ struct x86_debug_reg_state
 #define ALL_DEBUG_ADDRESS_REGISTERS(i) \
   for (i = DR_FIRSTADDR; i <= DR_LASTADDR; i++)
 
+/* Return a pointer to the local mirror of the debug registers of
+   process PID.  This function must be provided by the client
+   if required.  */
+extern struct x86_debug_reg_state *x86_debug_reg_state (pid_t pid);
+
 /* 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.  */
diff --git a/gdb/x86-nat.h b/gdb/x86-nat.h
index df5e9f2..7769572 100644
--- a/gdb/x86-nat.h
+++ b/gdb/x86-nat.h
@@ -44,11 +44,6 @@ extern void x86_set_debug_register_length (int len);
 
 extern void x86_cleanup_dregs (void);
 
-/* Return a pointer to the local mirror of the debug registers of
-   process PID.  */
-
-extern struct x86_debug_reg_state *x86_debug_reg_state (pid_t pid);
-
 /* Called whenever GDB is no longer debugging process PID.  It deletes
    data structures that keep track of debug register state.  */
 
-- 
1.7.1

Re: [PATCH 02/13 v2] Add x86_debug_reg_state to gdbserver

[Pedro Alves]

On 10/09/2014 10:52 AM, Gary Benson wrote:
> This commit introduces a new function, x86_debug_reg_state, that
> shared x86 code can use to access the local mirror of a process's
> debug registers.  This function already existed in GDB and was
> in use by GDB's x86_linux_prepare_to_resume.  An equivalent was
> written for gdbserver and gdbserver's x86_linux_prepare_to_resume
> was modified to use it.
> 
> gdb/ChangeLog:
> 
> 	* x86-nat.h (x86_debug_reg_state): Move declaration to...
> 	* nat/x86-dregs.h (x86_debug_reg_state): New declaration.
> 
> gdb/gdbserver/ChangeLog:
> 
> 	* linux-x86-low.c (x86_debug_reg_state): New function.
> 	(x86_linux_prepare_to_resume): Use the above.

OK.

Thanks,
Pedro Alves

[PATCH 12/13 v2] Move low-level Linux x86 debug register code to a shared file

[Gary Benson]

This commit moves the now-identical low-level Linux x86 debug register
code from gdb/x86-linux-nat.c and gdb/gdbserver/linux-x86-low.c into a
new shared file gdb/nat/x86-linux-dregs.c.

gdb/ChangeLog:

	* nat/x86-linux-dregs.h: New file.
	* nat/x86-linux-dregs.c: Likewise.
	* Makefile.in (HFILES_NO_SRCDIR): Add nat/x86-linux-dregs.h.
	(x86-linux-dregs.o): New rule.
	* config/i386/linux.mh (NATDEPFILES): Add x86-linux-dregs.o.
	* config/i386/linux64.mh (NATDEPFILES): Likewise.
	* x86-linux-nat.c: Include nat/x86-linux-dregs.h.
	(x86_linux_dr_get): Moved to nat/x86-linux-dregs.c.
	(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_update_debug_registers): Likewise.

gdb/gdbserver/ChangeLog:

	* Makefile.in (x86-linux-dregs.o): New rule.
	* configure.srv: Add x86-linux-dregs.o to relevant targets.
	* linux-x86-low.c: Include nat/x86-linux-dregs.h.
	(x86_linux_dr_get): Removed.
	(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_update_debug_registers): Likewise.
---
 gdb/ChangeLog                 |   19 +++++
 gdb/Makefile.in               |    6 +-
 gdb/config/i386/linux.mh      |    2 +-
 gdb/config/i386/linux64.mh    |    2 +-
 gdb/gdbserver/ChangeLog       |   15 ++++
 gdb/gdbserver/Makefile.in     |    3 +
 gdb/gdbserver/configure.srv   |    2 +
 gdb/gdbserver/linux-x86-low.c |  157 +-------------------------------------
 gdb/nat/x86-linux-dregs.c     |  173 +++++++++++++++++++++++++++++++++++++++++
 gdb/nat/x86-linux-dregs.h     |   53 +++++++++++++
 gdb/x86-linux-nat.c           |  158 +-------------------------------------
 11 files changed, 274 insertions(+), 316 deletions(-)
 create mode 100644 gdb/nat/x86-linux-dregs.c
 create mode 100644 gdb/nat/x86-linux-dregs.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 3379347..b985906 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -940,7 +940,7 @@ common/print-utils.h common/rsp-low.h nat/x86-dregs.h x86-linux-nat.h \
 i386-linux-nat.h common/common-defs.h common/errors.h common/common-types.h \
 common/common-debug.h common/cleanups.h common/gdb_setjmp.h \
 common/common-exceptions.h target/target.h common/symbol.h \
-common/common-regcache.h fbsd-tdep.h nat/x86-linux.h
+common/common-regcache.h fbsd-tdep.h nat/x86-linux.h nat/x86-linux-dregs.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
@@ -2222,6 +2222,10 @@ x86-linux.o: ${srcdir}/nat/x86-linux.c
 	$(COMPILE) $(srcdir)/nat/x86-linux.c
 	$(POSTCOMPILE)
 
+x86-linux-dregs.o: ${srcdir}/nat/x86-linux-dregs.c
+	$(COMPILE) $(srcdir)/nat/x86-linux-dregs.c
+	$(POSTCOMPILE)
+
 #
 # gdb/tui/ dependencies
 #
diff --git a/gdb/config/i386/linux.mh b/gdb/config/i386/linux.mh
index f409bfe..804bb0b 100644
--- a/gdb/config/i386/linux.mh
+++ b/gdb/config/i386/linux.mh
@@ -5,7 +5,7 @@ NATDEPFILES= inf-ptrace.o fork-child.o \
 	x86-nat.o x86-dregs.o i386-linux-nat.o x86-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 x86-linux.o
+	linux-btrace.o linux-waitpid.o x86-linux.o x86-linux-dregs.o
 NAT_CDEPS = $(srcdir)/proc-service.list
 
 # The dynamically loaded libthread_db needs access to symbols in the
diff --git a/gdb/config/i386/linux64.mh b/gdb/config/i386/linux64.mh
index 1d40167..384e393 100644
--- a/gdb/config/i386/linux64.mh
+++ b/gdb/config/i386/linux64.mh
@@ -5,7 +5,7 @@ NATDEPFILES= inf-ptrace.o fork-child.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 \
-	linux-waitpid.o x86-linux.o
+	linux-waitpid.o x86-linux.o x86-linux-dregs.o
 NAT_FILE= config/nm-linux.h
 NAT_CDEPS = $(srcdir)/proc-service.list
 
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index a58fd68..b7ee1dd 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -584,6 +584,9 @@ mips-linux-watch.o: ../nat/mips-linux-watch.c
 x86-linux.o: ../nat/x86-linux.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+x86-linux-dregs.o: ../nat/x86-linux-dregs.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 
 aarch64.c : $(srcdir)/../regformats/aarch64.dat $(regdat_sh)
 	$(SHELL) $(regdat_sh) $(srcdir)/../regformats/aarch64.dat aarch64.c
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index 2c261dc..ebbd63d 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -110,6 +110,7 @@ case "${target}" in
 			fi
 			srv_tgtobj="$srv_linux_obj linux-x86-low.o x86-low.o x86-dregs.o i387-fp.o"
 			srv_tgtobj="${srv_tgtobj} linux-btrace.o x86-linux.o"
+			srv_tgtobj="${srv_tgtobj} x86-linux-dregs.o"
 			srv_linux_usrregs=yes
 			srv_linux_regsets=yes
 			srv_linux_thread_db=yes
@@ -317,6 +318,7 @@ case "${target}" in
   x86_64-*-linux*)	srv_regobj="$srv_amd64_linux_regobj $srv_i386_linux_regobj"
 			srv_tgtobj="$srv_linux_obj linux-x86-low.o x86-low.o x86-dregs.o i387-fp.o"
 			srv_tgtobj="${srv_tgtobj} linux-btrace.o x86-linux.o"
+			srv_tgtobj="${srv_tgtobj} x86-linux-dregs.o"
 			srv_xmlfiles="$srv_i386_linux_xmlfiles $srv_amd64_linux_xmlfiles"
 			srv_linux_usrregs=yes # This is for i386 progs.
 			srv_linux_regsets=yes
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index bb8e1a7..b1d2d27 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -39,6 +39,7 @@
 #include "ax.h"
 #include "nat/linux-nat.h"
 #include "nat/x86-linux.h"
+#include "nat/x86-linux-dregs.h"
 
 #ifdef __x86_64__
 /* Defined in auto-generated file amd64-linux.c.  */
@@ -521,115 +522,6 @@ x86_breakpoint_at (CORE_ADDR pc)
   return 0;
 }
 \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)
-{
-  int tid;
-  unsigned long value;
-
-  gdb_assert (ptid_lwp_p (ptid));
-  tid = ptid_get_lwp (ptid);
-
-  errno = 0;
-  value = ptrace (PTRACE_PEEKUSER, tid,
-		  offsetof (struct user, u_debugreg[regnum]), 0);
-  if (errno != 0)
-    perror_with_name (_("Couldn't read debug register"));
-
-  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)
-{
-  int tid;
-
-  gdb_assert (ptid_lwp_p (ptid));
-  tid = ptid_get_lwp (ptid);
-
-  errno = 0;
-  ptrace (PTRACE_POKEUSER, tid,
-	  offsetof (struct user, u_debugreg[regnum]), value);
-  if (errno != 0)
-    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)
-{
-  lwp_set_debug_registers_changed (lwp, 1);
-
-  if (!lwp_is_stopped (lwp))
-    linux_stop_lwp (lwp);
-
-  /* Continue the iteration.  */
-  return 0;
-}
-
-/* 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)
-{
-  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (current_lwp_ptid ()));
-
-  gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);
-
-  iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
-}
-
-/* Return the address stored in the current inferior's debug register
-   REGNUM.  */
-
-static CORE_ADDR
-x86_linux_dr_get_addr (int regnum)
-{
-  gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);
-
-  return x86_linux_dr_get (current_lwp_ptid (), regnum);
-}
-
-/* Store CONTROL in the debug control registers of all LWPs of the
-   current inferior.  */
-
-static void
-x86_linux_dr_set_control (unsigned long control)
-{
-  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 value stored in the current inferior's debug control
-   register.  */
-
-static unsigned long
-x86_linux_dr_get_control (void)
-{
-  return x86_linux_dr_get (current_lwp_ptid (), DR_CONTROL);
-}
-
-/* Return the value stored in the current inferior's debug status
-   register.  */
-
-static unsigned long
-x86_linux_dr_get_status (void)
-{
-  return x86_linux_dr_get (current_lwp_ptid (), DR_STATUS);
-}
-
 /* Low-level function vector.  */
 struct x86_dr_low_type x86_dr_low =
   {
@@ -763,53 +655,6 @@ x86_debug_reg_state (pid_t pid)
   return &proc->private->arch_private->debug_reg_state;
 }
 
-/* Update the thread's debug registers if the values in our local
-   mirror have been changed.  */
-
-static void
-x86_linux_update_debug_registers (struct lwp_info *lwp)
-{
-  ptid_t ptid = ptid_of_lwp (lwp);
-  int clear_status = 0;
-
-  gdb_assert (lwp_is_stopped (lwp));
-
-  if (lwp_debug_registers_changed (lwp))
-    {
-      struct x86_debug_reg_state *state
-	= 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)
-	if (state->dr_ref_count[i] > 0)
-	  {
-	    x86_linux_dr_set (ptid, i, state->dr_mirror[i]);
-
-	    /* If we're setting a watchpoint, any change the inferior
-	       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);
-
-      lwp_set_debug_registers_changed (lwp, 0);
-    }
-
-  if (clear_status || lwp_is_stopped_by_watchpoint (lwp))
-    x86_linux_dr_set (ptid, DR_STATUS, 0);
-}
-
 /* Called prior to resuming a thread.  */
 
 static void
diff --git a/gdb/nat/x86-linux-dregs.c b/gdb/nat/x86-linux-dregs.c
new file mode 100644
index 0000000..3e3d876
--- /dev/null
+++ b/gdb/nat/x86-linux-dregs.c
@@ -0,0 +1,173 @@
+/* Low-level debug register code for x86 (i386 and x86-64).
+
+   Copyright (C) 1999-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/>.  */
+
+#include "common-defs.h"
+#include <sys/ptrace.h>
+#include <sys/user.h>
+#include "nat/x86-linux.h"
+#include "nat/x86-dregs.h"
+#include "nat/x86-linux-dregs.h"
+
+/* Get debug register REGNUM value from the LWP specified by PTID.  */
+
+static unsigned long
+x86_linux_dr_get (ptid_t ptid, int regnum)
+{
+  int tid;
+  unsigned long value;
+
+  gdb_assert (ptid_lwp_p (ptid));
+  tid = ptid_get_lwp (ptid);
+
+  errno = 0;
+  value = ptrace (PTRACE_PEEKUSER, tid,
+		  offsetof (struct user, u_debugreg[regnum]), 0);
+  if (errno != 0)
+    perror_with_name (_("Couldn't read debug register"));
+
+  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)
+{
+  int tid;
+
+  gdb_assert (ptid_lwp_p (ptid));
+  tid = ptid_get_lwp (ptid);
+
+  errno = 0;
+  ptrace (PTRACE_POKEUSER, tid,
+	  offsetof (struct user, u_debugreg[regnum]), value);
+  if (errno != 0)
+    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)
+{
+  lwp_set_debug_registers_changed (lwp, 1);
+
+  if (!lwp_is_stopped (lwp))
+    linux_stop_lwp (lwp);
+
+  /* Continue the iteration.  */
+  return 0;
+}
+
+/* See nat/x86-linux-dregs.h.  */
+
+CORE_ADDR
+x86_linux_dr_get_addr (int regnum)
+{
+  gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);
+
+  return x86_linux_dr_get (current_lwp_ptid (), regnum);
+}
+
+/* See nat/x86-linux-dregs.h.  */
+
+void
+x86_linux_dr_set_addr (int regnum, CORE_ADDR addr)
+{
+  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (current_lwp_ptid ()));
+
+  gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);
+
+  iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+}
+
+/* See nat/x86-linux-dregs.h.  */
+
+unsigned long
+x86_linux_dr_get_control (void)
+{
+  return x86_linux_dr_get (current_lwp_ptid (), DR_CONTROL);
+}
+
+/* See nat/x86-linux-dregs.h.  */
+
+void
+x86_linux_dr_set_control (unsigned long control)
+{
+  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (current_lwp_ptid ()));
+
+  iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+}
+
+/* See nat/x86-linux-dregs.h.  */
+
+unsigned long
+x86_linux_dr_get_status (void)
+{
+  return x86_linux_dr_get (current_lwp_ptid (), DR_STATUS);
+}
+
+/* See nat/x86-linux-dregs.h.  */
+
+void
+x86_linux_update_debug_registers (struct lwp_info *lwp)
+{
+  ptid_t ptid = ptid_of_lwp (lwp);
+  int clear_status = 0;
+
+  gdb_assert (lwp_is_stopped (lwp));
+
+  if (lwp_debug_registers_changed (lwp))
+    {
+      struct x86_debug_reg_state *state
+	= 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)
+	if (state->dr_ref_count[i] > 0)
+	  {
+	    x86_linux_dr_set (ptid, i, state->dr_mirror[i]);
+
+	    /* If we're setting a watchpoint, any change the inferior
+	       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);
+
+      lwp_set_debug_registers_changed (lwp, 0);
+    }
+
+  if (clear_status || lwp_is_stopped_by_watchpoint (lwp))
+    x86_linux_dr_set (ptid, DR_STATUS, 0);
+}
diff --git a/gdb/nat/x86-linux-dregs.h b/gdb/nat/x86-linux-dregs.h
new file mode 100644
index 0000000..7d526b7
--- /dev/null
+++ b/gdb/nat/x86-linux-dregs.h
@@ -0,0 +1,53 @@
+/* Low-level debug register code for x86 (i386 and x86-64).
+
+   Copyright (C) 1999-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/>.  */
+
+#ifndef X86_LINUX_DREGS_H
+#define X86_LINUX_DREGS_H
+
+/* Return the address stored in the current inferior's debug register
+   REGNUM.  */
+
+extern CORE_ADDR x86_linux_dr_get_addr (int regnum);
+
+/* Store ADDR in debug register REGNUM of all LWPs of the current
+   inferior.  */
+
+extern void x86_linux_dr_set_addr (int regnum, CORE_ADDR addr);
+
+/* Return the value stored in the current inferior's debug control
+   register.  */
+
+extern unsigned long x86_linux_dr_get_control (void);
+
+/* Store CONTROL in the debug control registers of all LWPs of the
+   current inferior.  */
+
+extern void x86_linux_dr_set_control (unsigned long control);
+
+/* Return the value stored in the current inferior's debug status
+   register.  */
+
+extern unsigned long x86_linux_dr_get_status (void);
+
+/* Update the thread's debug registers if the values in our local
+   mirror have been changed.  */
+
+extern void x86_linux_update_debug_registers (struct lwp_info *lwp);
+
+#endif /* X86_LINUX_DREGS_H */
diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
index 55c3bd3..97ce443 100644
--- a/gdb/x86-linux-nat.c
+++ b/gdb/x86-linux-nat.c
@@ -40,6 +40,7 @@
 #include "nat/linux-btrace.h"
 #include "nat/linux-nat.h"
 #include "nat/x86-linux.h"
+#include "nat/x86-linux-dregs.h"
 
 /* Per-thread arch-specific data we want to keep.  */
 
@@ -51,163 +52,6 @@ struct arch_lwp_info
 
 /* Does the current host support PTRACE_GETREGSET?  */
 int have_ptrace_getregset = -1;
-\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)
-{
-  int tid;
-  unsigned long value;
-
-  gdb_assert (ptid_lwp_p (ptid));
-  tid = ptid_get_lwp (ptid);
-
-  errno = 0;
-  value = ptrace (PTRACE_PEEKUSER, tid,
-		  offsetof (struct user, u_debugreg[regnum]), 0);
-  if (errno != 0)
-    perror_with_name (_("Couldn't read debug register"));
-
-  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)
-{
-  int tid;
-
-  gdb_assert (ptid_lwp_p (ptid));
-  tid = ptid_get_lwp (ptid);
-
-  errno = 0;
-  ptrace (PTRACE_POKEUSER, tid,
-	  offsetof (struct user, u_debugreg[regnum]), value);
-  if (errno != 0)
-    perror_with_name (_("Couldn't write debug register"));
-}
-
-/* Return the address stored in the current inferior's debug register
-   REGNUM.  */
-
-static CORE_ADDR
-x86_linux_dr_get_addr (int regnum)
-{
-  gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);
-
-  return x86_linux_dr_get (current_lwp_ptid (), regnum);
-}
-
-/* Return the value stored in the current inferior's debug control
-   register.  */
-
-static unsigned long
-x86_linux_dr_get_control (void)
-{
-  return x86_linux_dr_get (current_lwp_ptid (), DR_CONTROL);
-}
-
-/* Return the value stored in the current inferior's debug status
-   register.  */
-
-static unsigned long
-x86_linux_dr_get_status (void)
-{
-  return x86_linux_dr_get (current_lwp_ptid (), DR_STATUS);
-}
-
-/* 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)
-{
-  lwp_set_debug_registers_changed (lwp, 1);
-
-  if (!lwp_is_stopped (lwp))
-    linux_stop_lwp (lwp);
-
-  /* Continue the iteration.  */
-  return 0;
-}
-
-/* Store CONTROL in the debug control registers of all LWPs of the
-   current inferior.  */
-
-static void
-x86_linux_dr_set_control (unsigned long control)
-{
-  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (current_lwp_ptid ()));
-
-  iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
-}
-
-/* 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)
-{
-  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (current_lwp_ptid ()));
-
-  gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);
-
-  iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
-}
-
-/* Update the thread's debug registers if the values in our local
-   mirror have been changed.  */
-
-static void
-x86_linux_update_debug_registers (struct lwp_info *lwp)
-{
-  ptid_t ptid = ptid_of_lwp (lwp);
-  int clear_status = 0;
-
-  gdb_assert (lwp_is_stopped (lwp));
-
-  if (lwp_debug_registers_changed (lwp))
-    {
-      struct x86_debug_reg_state *state
-	= 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)
-	if (state->dr_ref_count[i] > 0)
-	  {
-	    x86_linux_dr_set (ptid, i, state->dr_mirror[i]);
-
-	    /* If we're setting a watchpoint, any change the inferior
-	       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);
-
-      lwp_set_debug_registers_changed (lwp, 0);
-    }
-
-  if (clear_status || lwp_is_stopped_by_watchpoint (lwp))
-    x86_linux_dr_set (ptid, DR_STATUS, 0);
-}
 
 /* Called prior to resuming a thread.  */
 
-- 
1.7.1

Re: [PATCH 12/13 v2] Move low-level Linux x86 debug register code to a shared file

[Pedro Alves]

On 10/09/2014 10:52 AM, Gary Benson wrote:
> diff --git a/gdb/nat/x86-linux-dregs.c b/gdb/nat/x86-linux-dregs.c
> new file mode 100644
> index 0000000..3e3d876
> --- /dev/null
> +++ b/gdb/nat/x86-linux-dregs.c
> @@ -0,0 +1,173 @@
> +/* Low-level debug register code for x86 (i386 and x86-64).

I think this comment should mention "Linux" too.

> +++ b/gdb/nat/x86-linux-dregs.h
> @@ -0,0 +1,53 @@
> +/* Low-level debug register code for x86 (i386 and x86-64).

Likewise.

Otherwise OK.

Thanks,
Pedro Alves

[PATCH 07/13 v2] Make lwp_info.arch_private handling shared

[Gary Benson]

This commit moves the code to handle lwp_info.arch_private for
Linux x86 into a new shared file, nat/x86-linux.c.

gdb/ChangeLog:

	* nat/x86-linux.h: New file.
	* nat/x86-linux.c: Likewise.
	* Makefile.in (HFILES_NO_SRCDIR): Add nat/x86-linux.h.
	(x86-linux.o): New rule.
	* config/i386/linux.mh (NATDEPFILES): Add x86-linux.o.
	* config/i386/linux64.mh (NATDEPFILES): Likewise.
	* nat/linux-nat.h (struct arch_lwp_info): New forward declaration.
	(lwp_set_arch_private_info): New declaration.
	(lwp_arch_private_info): Likewise.
	* linux-nat.c (lwp_set_arch_private_info): New function.
	(lwp_arch_private_info): Likewise.
	* x86-linux-nat.c: Include nat/x86-linux.h.
	(arch_lwp_info): Removed structure.
	(update_debug_registers_callback):
	Use lwp_set_debug_registers_changed.
	(x86_linux_prepare_to_resume): Use lwp_debug_registers_changed
	and lwp_set_debug_registers_changed.
	(x86_linux_new_thread): Use lwp_set_debug_registers_changed.

gdb/gdbserver/ChangeLog:

	* Makefile.in (x86-linux.o): New rule.
	* configure.srv: Add x86-linux.o to relevant targets.
	* linux-low.c (lwp_set_arch_private_info): New function.
	(lwp_arch_private_info): Likewise.
	* linux-x86-low.c: Include nat/x86-linux.h.
	(arch_lwp_info): Removed structure.
	(update_debug_registers_callback):
	Use lwp_set_debug_registers_changed.
	(x86_linux_prepare_to_resume): Use lwp_debug_registers_changed
	and lwp_set_debug_registers_changed.
	(x86_linux_new_thread): Use lwp_set_debug_registers_changed.
---
 gdb/ChangeLog                 |   21 +++++++++++++++
 gdb/Makefile.in               |    6 +++-
 gdb/config/i386/linux.mh      |    2 +-
 gdb/config/i386/linux64.mh    |    2 +-
 gdb/gdbserver/ChangeLog       |   14 ++++++++++
 gdb/gdbserver/Makefile.in     |    3 ++
 gdb/gdbserver/configure.srv   |    4 +-
 gdb/gdbserver/linux-low.c     |   17 ++++++++++++
 gdb/gdbserver/linux-x86-low.c |   21 +++-----------
 gdb/linux-nat.c               |   17 ++++++++++++
 gdb/nat/linux-nat.h           |   12 ++++++++
 gdb/nat/x86-linux.c           |   57 +++++++++++++++++++++++++++++++++++++++++
 gdb/nat/x86-linux.h           |   38 +++++++++++++++++++++++++++
 gdb/x86-linux-nat.c           |   22 +++------------
 14 files changed, 198 insertions(+), 38 deletions(-)
 create mode 100644 gdb/nat/x86-linux.c
 create mode 100644 gdb/nat/x86-linux.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 1ffa62a..3379347 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -940,7 +940,7 @@ common/print-utils.h common/rsp-low.h nat/x86-dregs.h x86-linux-nat.h \
 i386-linux-nat.h common/common-defs.h common/errors.h common/common-types.h \
 common/common-debug.h common/cleanups.h common/gdb_setjmp.h \
 common/common-exceptions.h target/target.h common/symbol.h \
-common/common-regcache.h fbsd-tdep.h
+common/common-regcache.h fbsd-tdep.h nat/x86-linux.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
@@ -2218,6 +2218,10 @@ mips-linux-watch.o: ${srcdir}/nat/mips-linux-watch.c
 	$(COMPILE) $(srcdir)/nat/mips-linux-watch.c
 	$(POSTCOMPILE)
 
+x86-linux.o: ${srcdir}/nat/x86-linux.c
+	$(COMPILE) $(srcdir)/nat/x86-linux.c
+	$(POSTCOMPILE)
+
 #
 # gdb/tui/ dependencies
 #
diff --git a/gdb/config/i386/linux.mh b/gdb/config/i386/linux.mh
index a4e4842..f409bfe 100644
--- a/gdb/config/i386/linux.mh
+++ b/gdb/config/i386/linux.mh
@@ -5,7 +5,7 @@ NATDEPFILES= inf-ptrace.o fork-child.o \
 	x86-nat.o x86-dregs.o i386-linux-nat.o x86-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
+	linux-btrace.o linux-waitpid.o x86-linux.o
 NAT_CDEPS = $(srcdir)/proc-service.list
 
 # The dynamically loaded libthread_db needs access to symbols in the
diff --git a/gdb/config/i386/linux64.mh b/gdb/config/i386/linux64.mh
index d557202..1d40167 100644
--- a/gdb/config/i386/linux64.mh
+++ b/gdb/config/i386/linux64.mh
@@ -5,7 +5,7 @@ NATDEPFILES= inf-ptrace.o fork-child.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 \
-	linux-waitpid.o
+	linux-waitpid.o x86-linux.o
 NAT_FILE= config/nm-linux.h
 NAT_CDEPS = $(srcdir)/proc-service.list
 
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 8b0318a..a58fd68 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -581,6 +581,9 @@ linux-waitpid.o: ../nat/linux-waitpid.c
 mips-linux-watch.o: ../nat/mips-linux-watch.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+x86-linux.o: ../nat/x86-linux.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 
 aarch64.c : $(srcdir)/../regformats/aarch64.dat $(regdat_sh)
 	$(SHELL) $(regdat_sh) $(srcdir)/../regformats/aarch64.dat aarch64.c
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index 679fc9f..2c261dc 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -109,7 +109,7 @@ case "${target}" in
 			    srv_xmlfiles="${srv_xmlfiles} $srv_amd64_linux_xmlfiles"
 			fi
 			srv_tgtobj="$srv_linux_obj linux-x86-low.o x86-low.o x86-dregs.o i387-fp.o"
-			srv_tgtobj="${srv_tgtobj} linux-btrace.o"
+			srv_tgtobj="${srv_tgtobj} linux-btrace.o x86-linux.o"
 			srv_linux_usrregs=yes
 			srv_linux_regsets=yes
 			srv_linux_thread_db=yes
@@ -316,7 +316,7 @@ case "${target}" in
 			;;
   x86_64-*-linux*)	srv_regobj="$srv_amd64_linux_regobj $srv_i386_linux_regobj"
 			srv_tgtobj="$srv_linux_obj linux-x86-low.o x86-low.o x86-dregs.o i387-fp.o"
-			srv_tgtobj="${srv_tgtobj} linux-btrace.o"
+			srv_tgtobj="${srv_tgtobj} linux-btrace.o x86-linux.o"
 			srv_xmlfiles="$srv_i386_linux_xmlfiles $srv_amd64_linux_xmlfiles"
 			srv_linux_usrregs=yes # This is for i386 progs.
 			srv_linux_regsets=yes
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 15a5c56..31c7878 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -146,6 +146,23 @@ ptid_of_lwp (struct lwp_info *lwp)
 
 /* See nat/linux-nat.h.  */
 
+void
+lwp_set_arch_private_info (struct lwp_info *lwp,
+			   struct arch_lwp_info *info)
+{
+  lwp->arch_private = info;
+}
+
+/* See nat/linux-nat.h.  */
+
+struct arch_lwp_info *
+lwp_arch_private_info (struct lwp_info *lwp)
+{
+  return lwp->arch_private;
+}
+
+/* See nat/linux-nat.h.  */
+
 int
 lwp_is_stopped (struct lwp_info *lwp)
 {
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index f04e1f1..445a07e 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -38,6 +38,7 @@
 #include "tracepoint.h"
 #include "ax.h"
 #include "nat/linux-nat.h"
+#include "nat/x86-linux.h"
 
 #ifdef __x86_64__
 /* Defined in auto-generated file amd64-linux.c.  */
@@ -152,14 +153,6 @@ struct arch_process_info
   struct x86_debug_reg_state debug_reg_state;
 };
 
-/* Per-thread arch-specific data we want to keep.  */
-
-struct arch_lwp_info
-{
-  /* Non-zero if our copy differs from what's recorded in the thread.  */
-  int debug_registers_changed;
-};
-
 #ifdef __x86_64__
 
 /* Mapping between the general-purpose registers in `struct user'
@@ -566,7 +559,7 @@ 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->arch_private->debug_registers_changed = 1;
+  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.  */
@@ -747,11 +740,7 @@ x86_linux_new_process (void)
 static void
 x86_linux_new_thread (struct lwp_info *lwp)
 {
-  struct arch_lwp_info *info = XCNEW (struct arch_lwp_info);
-
-  info->debug_registers_changed = 1;
-
-  lwp->arch_private = info;
+  lwp_set_debug_registers_changed (lwp, 1);
 }
 
 /* See nat/x86-dregs.h.  */
@@ -773,7 +762,7 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
   ptid_t ptid = ptid_of_lwp (lwp);
   int clear_status = 0;
 
-  if (lwp->arch_private->debug_registers_changed)
+  if (lwp_debug_registers_changed (lwp))
     {
       struct x86_debug_reg_state *state
 	= x86_debug_reg_state (ptid_get_pid (ptid));
@@ -796,7 +785,7 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
       if (state->dr_control_mirror != 0)
 	x86_linux_dr_set (ptid, DR_CONTROL, state->dr_control_mirror);
 
-      lwp->arch_private->debug_registers_changed = 0;
+      lwp_set_debug_registers_changed (lwp, 0);
     }
 
   if (clear_status || lwp_is_stopped_by_watchpoint (lwp))
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 4a32bb9..16151b5 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -290,6 +290,23 @@ ptid_of_lwp (struct lwp_info *lwp)
 
 /* See nat/linux-nat.h.  */
 
+void
+lwp_set_arch_private_info (struct lwp_info *lwp,
+			   struct arch_lwp_info *info)
+{
+  lwp->arch_private = info;
+}
+
+/* See nat/linux-nat.h.  */
+
+struct arch_lwp_info *
+lwp_arch_private_info (struct lwp_info *lwp)
+{
+  return lwp->arch_private;
+}
+
+/* See nat/linux-nat.h.  */
+
 int
 lwp_is_stopped (struct lwp_info *lwp)
 {
diff --git a/gdb/nat/linux-nat.h b/gdb/nat/linux-nat.h
index 502c2cb..5d3ef90 100644
--- a/gdb/nat/linux-nat.h
+++ b/gdb/nat/linux-nat.h
@@ -21,6 +21,7 @@
 #define LINUX_NAT_H
 
 struct lwp_info;
+struct arch_lwp_info;
 
 /* Unlike other extended result codes, WSTOPSIG (status) on
    PTRACE_O_TRACESYSGOOD syscall events doesn't return SIGTRAP, but
@@ -49,6 +50,17 @@ extern struct lwp_info *iterate_over_lwps (ptid_t filter,
 
 extern ptid_t ptid_of_lwp (struct lwp_info *lwp);
 
+/* Set the architecture-specific data of LWP.  This function must be
+   provided by the client. */
+
+extern void lwp_set_arch_private_info (struct lwp_info *lwp,
+				       struct arch_lwp_info *info);
+
+/* Return the architecture-specific data of LWP.  This function must
+   be provided by the client. */
+
+extern struct arch_lwp_info *lwp_arch_private_info (struct lwp_info *lwp);
+
 /* Return nonzero if LWP is stopped, zero otherwise.  This function
    must be provided by the client.  */
 
diff --git a/gdb/nat/x86-linux.c b/gdb/nat/x86-linux.c
new file mode 100644
index 0000000..efeeab9
--- /dev/null
+++ b/gdb/nat/x86-linux.c
@@ -0,0 +1,57 @@
+/* Native-dependent code for GNU/Linux x86 (i386 and x86-64).
+
+   Copyright (C) 1999-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/>.  */
+
+#include "common-defs.h"
+#include "x86-linux.h"
+
+/* Per-thread arch-specific data we want to keep.  */
+
+struct arch_lwp_info
+{
+  /* Non-zero if our copy differs from what's recorded in the
+     thread.  */
+  int debug_registers_changed;
+};
+
+/* See nat/x86-linux.h.  */
+
+void
+lwp_set_debug_registers_changed (struct lwp_info *lwp, int value)
+{
+  if (lwp_arch_private_info (lwp) == NULL)
+    lwp_set_arch_private_info (lwp, XCNEW (struct arch_lwp_info));
+
+  lwp_arch_private_info (lwp)->debug_registers_changed = value;
+}
+
+/* See nat/x86-linux.h.  */
+
+int
+lwp_debug_registers_changed (struct lwp_info *lwp)
+{
+  struct arch_lwp_info *info = lwp_arch_private_info (lwp);
+
+  /* NULL means either that this is the main thread still going
+     through the shell, or that no watchpoint has been set yet.
+     The debug registers are unchanged in either case.  */
+  if (info == NULL)
+    return 0;
+
+  return info->debug_registers_changed;
+}
diff --git a/gdb/nat/x86-linux.h b/gdb/nat/x86-linux.h
new file mode 100644
index 0000000..6409898
--- /dev/null
+++ b/gdb/nat/x86-linux.h
@@ -0,0 +1,38 @@
+/* Native-dependent code for GNU/Linux x86 (i386 and x86-64).
+
+   Copyright (C) 1999-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/>.  */
+
+#ifndef X86_LINUX_H
+#define X86_LINUX_H
+
+#include "nat/linux-nat.h"
+
+/* Set whether our local mirror of LWP's debug registers has been
+   changed since the values were last written to the thread.  Nonzero
+   indicates that a change has been made, zero indicates no change.  */
+
+extern void lwp_set_debug_registers_changed (struct lwp_info *lwp,
+					     int value);
+
+/* Return nonzero if our local mirror of LWP's debug registers has
+   been changed since the values were last written to the thread,
+   zero otherwise.  */
+
+extern int lwp_debug_registers_changed (struct lwp_info *lwp);
+
+#endif /* X86_LINUX_H */
diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
index c25240d..c4387eb 100644
--- a/gdb/x86-linux-nat.c
+++ b/gdb/x86-linux-nat.c
@@ -39,6 +39,7 @@
 #include "x86-xstate.h"
 #include "nat/linux-btrace.h"
 #include "nat/linux-nat.h"
+#include "nat/x86-linux.h"
 
 /* Per-thread arch-specific data we want to keep.  */
 
@@ -124,12 +125,9 @@ x86_linux_dr_get_status (void)
 static int
 update_debug_registers_callback (struct lwp_info *lwp, void *arg)
 {
-  if (lwp->arch_private == NULL)
-    lwp->arch_private = XCNEW (struct arch_lwp_info);
-
   /* The actual update is done later just before resuming the lwp, we
      just mark that the registers need updating.  */
-  lwp->arch_private->debug_registers_changed = 1;
+  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.  */
@@ -172,13 +170,7 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
   ptid_t ptid = ptid_of_lwp (lwp);
   int clear_status = 0;
 
-  /* NULL means this is the main thread still going through the shell,
-     or, no watchpoint has been set yet.  In that case, there's
-     nothing to do.  */
-  if (lwp->arch_private == NULL)
-    return;
-
-  if (lwp->arch_private->debug_registers_changed)
+  if (lwp_debug_registers_changed (lwp))
     {
       struct x86_debug_reg_state *state
 	= x86_debug_reg_state (ptid_get_pid (ptid));
@@ -213,7 +205,7 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
       if (state->dr_control_mirror != 0)
 	x86_linux_dr_set (ptid, DR_CONTROL, state->dr_control_mirror);
 
-      lwp->arch_private->debug_registers_changed = 0;
+      lwp_set_debug_registers_changed (lwp, 0);
     }
 
   if (clear_status || lwp_is_stopped_by_watchpoint (lwp))
@@ -223,11 +215,7 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
 static void
 x86_linux_new_thread (struct lwp_info *lp)
 {
-  struct arch_lwp_info *info = XCNEW (struct arch_lwp_info);
-
-  info->debug_registers_changed = 1;
-
-  lp->arch_private = info;
+  lwp_set_debug_registers_changed (lp, 1);
 }
 \f
 
-- 
1.7.1

Re: [PATCH 07/13 v2] Make lwp_info.arch_private handling shared

[Pedro Alves]

On 10/09/2014 10:52 AM, Gary Benson wrote:
> This commit moves the code to handle lwp_info.arch_private for
> Linux x86 into a new shared file, nat/x86-linux.c.
> 
> gdb/ChangeLog:
> 
> 	* nat/x86-linux.h: New file.
> 	* nat/x86-linux.c: Likewise.
> 	* Makefile.in (HFILES_NO_SRCDIR): Add nat/x86-linux.h.
> 	(x86-linux.o): New rule.
> 	* config/i386/linux.mh (NATDEPFILES): Add x86-linux.o.
> 	* config/i386/linux64.mh (NATDEPFILES): Likewise.
> 	* nat/linux-nat.h (struct arch_lwp_info): New forward declaration.
> 	(lwp_set_arch_private_info): New declaration.
> 	(lwp_arch_private_info): Likewise.
> 	* linux-nat.c (lwp_set_arch_private_info): New function.
> 	(lwp_arch_private_info): Likewise.
> 	* x86-linux-nat.c: Include nat/x86-linux.h.
> 	(arch_lwp_info): Removed structure.
> 	(update_debug_registers_callback):
> 	Use lwp_set_debug_registers_changed.
> 	(x86_linux_prepare_to_resume): Use lwp_debug_registers_changed
> 	and lwp_set_debug_registers_changed.
> 	(x86_linux_new_thread): Use lwp_set_debug_registers_changed.
> 
> gdb/gdbserver/ChangeLog:
> 
> 	* Makefile.in (x86-linux.o): New rule.
> 	* configure.srv: Add x86-linux.o to relevant targets.
> 	* linux-low.c (lwp_set_arch_private_info): New function.
> 	(lwp_arch_private_info): Likewise.
> 	* linux-x86-low.c: Include nat/x86-linux.h.
> 	(arch_lwp_info): Removed structure.
> 	(update_debug_registers_callback):
> 	Use lwp_set_debug_registers_changed.
> 	(x86_linux_prepare_to_resume): Use lwp_debug_registers_changed
> 	and lwp_set_debug_registers_changed.
> 	(x86_linux_new_thread): Use lwp_set_debug_registers_changed.

OK.

Thanks,
Pedro Alves

[PATCH 05/13 v2] Introduce basic LWP accessors

[Gary Benson]

This commit introduces three accessors that shared Linux code can
use to access fields of struct lwp_info.  The GDB and gdbserver
Linux x86 code is modified to use them.

gdb/ChangeLog:

	* nat/linux-nat.h (ptid_of_lwp): New declaration.
	(lwp_is_stopped): Likewise.
	(lwp_is_stopped_by_watchpoint): Likewise.
	* linux-nat.c (ptid_of_lwp): New function.
	(lwp_is_stopped): Likewise.
	(lwp_is_stopped_by_watchpoint): Likewise.
	* x86-linux-nat.c (update_debug_registers_callback):
	Use lwp_is_stopped.
	(x86_linux_prepare_to_resume): Use ptid_of_lwp and
	lwp_is_stopped_by_watchpoint.

gdb/gdbserver/ChangeLog:

	* linux-low.c (ptid_of_lwp): New function.
	(lwp_is_stopped): Likewise.
	(lwp_is_stopped_by_watchpoint): Likewise.
	* linux-x86-low.c (update_debug_registers_callback):
	Use lwp_is_stopped.
	(x86_linux_prepare_to_resume): Use ptid_of_lwp and
	lwp_is_stopped_by_watchpoint.
---
 gdb/ChangeLog                 |   13 +++++++++++++
 gdb/gdbserver/ChangeLog       |   10 ++++++++++
 gdb/gdbserver/linux-low.c     |   26 ++++++++++++++++++++++++++
 gdb/gdbserver/linux-x86-low.c |    6 +++---
 gdb/linux-nat.c               |   27 +++++++++++++++++++++++++++
 gdb/nat/linux-nat.h           |   14 ++++++++++++++
 gdb/x86-linux-nat.c           |   15 ++++++++-------
 7 files changed, 101 insertions(+), 10 deletions(-)

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index c980d10..4992945 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -134,6 +134,32 @@ typedef struct
 } Elf64_auxv_t;
 #endif
 
+/* LWP accessors.  */
+
+/* See nat/linux-nat.h.  */
+
+ptid_t
+ptid_of_lwp (struct lwp_info *lwp)
+{
+  return ptid_of (get_lwp_thread (lwp));
+}
+
+/* See nat/linux-nat.h.  */
+
+int
+lwp_is_stopped (struct lwp_info *lwp)
+{
+  return lwp->stopped;
+}
+
+/* See nat/linux-nat.h.  */
+
+int
+lwp_is_stopped_by_watchpoint (struct lwp_info *lwp)
+{
+  return lwp->stopped_by_watchpoint;
+}
+
 /* A list of all unknown processes which receive stop signals.  Some
    other process will presumably claim each of these as forked
    children momentarily.  */
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index a861872..64fab4c 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -570,7 +570,7 @@ update_debug_registers_callback (struct lwp_info *lwp, void *arg)
 
   /* If the lwp isn't stopped, force it to momentarily pause, so
      we can update its debug registers.  */
-  if (!lwp->stopped)
+  if (!lwp_is_stopped (lwp))
     linux_stop_lwp (lwp);
 
   return 0;
@@ -770,7 +770,7 @@ x86_debug_reg_state (pid_t pid)
 static void
 x86_linux_prepare_to_resume (struct lwp_info *lwp)
 {
-  ptid_t ptid = ptid_of (get_lwp_thread (lwp));
+  ptid_t ptid = ptid_of_lwp (lwp);
   int clear_status = 0;
 
   if (lwp->arch_private->debug_registers_changed)
@@ -799,7 +799,7 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
       lwp->arch_private->debug_registers_changed = 0;
     }
 
-  if (clear_status || lwp->stopped_by_watchpoint)
+  if (clear_status || lwp_is_stopped_by_watchpoint (lwp))
     x86_linux_dr_set (ptid, DR_STATUS, 0);
 }
 \f
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 1353d43..4a32bb9 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -278,6 +278,33 @@ static void delete_lwp (ptid_t ptid);
 static struct lwp_info *find_lwp_pid (ptid_t ptid);
 
 \f
+/* LWP accessors.  */
+
+/* See nat/linux-nat.h.  */
+
+ptid_t
+ptid_of_lwp (struct lwp_info *lwp)
+{
+  return lwp->ptid;
+}
+
+/* See nat/linux-nat.h.  */
+
+int
+lwp_is_stopped (struct lwp_info *lwp)
+{
+  return lwp->stopped;
+}
+
+/* See nat/linux-nat.h.  */
+
+int
+lwp_is_stopped_by_watchpoint (struct lwp_info *lwp)
+{
+  return lwp->stopped_by_watchpoint;
+}
+
+\f
 /* Trivial list manipulation functions to keep track of a list of
    new stopped processes.  */
 static void
diff --git a/gdb/nat/linux-nat.h b/gdb/nat/linux-nat.h
index da94f2f..502c2cb 100644
--- a/gdb/nat/linux-nat.h
+++ b/gdb/nat/linux-nat.h
@@ -45,6 +45,20 @@ extern struct lwp_info *iterate_over_lwps (ptid_t filter,
 					   iterate_over_lwps_ftype callback,
 					   void *data);
 
+/* Return the ptid of LWP.  */
+
+extern ptid_t ptid_of_lwp (struct lwp_info *lwp);
+
+/* Return nonzero if LWP is stopped, zero otherwise.  This function
+   must be provided by the client.  */
+
+extern int lwp_is_stopped (struct lwp_info *lwp);
+
+/* Return nonzero if LWP is stopped with a data watchpoint trap,
+   zero otherwise.  This function must be provided by the client.  */
+
+extern int lwp_is_stopped_by_watchpoint (struct lwp_info *lwp);
+
 /* Cause LWP to stop.  This function must be provided by the
    client.  */
 
diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
index 7a80991..c25240d 100644
--- a/gdb/x86-linux-nat.c
+++ b/gdb/x86-linux-nat.c
@@ -133,7 +133,7 @@ update_debug_registers_callback (struct lwp_info *lwp, void *arg)
 
   /* If the lwp isn't stopped, force it to momentarily pause, so we
      can update its debug registers.  */
-  if (!lwp->stopped)
+  if (!lwp_is_stopped (lwp))
     linux_stop_lwp (lwp);
 
   /* Continue the iteration.  */
@@ -169,6 +169,7 @@ x86_linux_dr_set_addr (int regnum, CORE_ADDR addr)
 static void
 x86_linux_prepare_to_resume (struct lwp_info *lwp)
 {
+  ptid_t ptid = ptid_of_lwp (lwp);
   int clear_status = 0;
 
   /* NULL means this is the main thread still going through the shell,
@@ -180,7 +181,7 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
   if (lwp->arch_private->debug_registers_changed)
     {
       struct x86_debug_reg_state *state
-	= x86_debug_reg_state (ptid_get_pid (lwp->ptid));
+	= x86_debug_reg_state (ptid_get_pid (ptid));
       int i;
 
       /* On Linux kernel before 2.6.33 commit
@@ -193,12 +194,12 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
       /* 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.  */
-      x86_linux_dr_set (lwp->ptid, DR_CONTROL, 0);
+      x86_linux_dr_set (ptid, DR_CONTROL, 0);
 
       ALL_DEBUG_ADDRESS_REGISTERS (i)
 	if (state->dr_ref_count[i] > 0)
 	  {
-	    x86_linux_dr_set (lwp->ptid, i, state->dr_mirror[i]);
+	    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
@@ -210,13 +211,13 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
       /* If DR_CONTROL is supposed to be zero, we've already set it
 	 above.  */
       if (state->dr_control_mirror != 0)
-	x86_linux_dr_set (lwp->ptid, DR_CONTROL, state->dr_control_mirror);
+	x86_linux_dr_set (ptid, DR_CONTROL, state->dr_control_mirror);
 
       lwp->arch_private->debug_registers_changed = 0;
     }
 
-  if (clear_status || lwp->stopped_by_watchpoint)
-    x86_linux_dr_set (lwp->ptid, DR_STATUS, 0);
+  if (clear_status || lwp_is_stopped_by_watchpoint (lwp))
+    x86_linux_dr_set (ptid, DR_STATUS, 0);
 }
 
 static void
-- 
1.7.1

Re: [PATCH 05/13 v2] Introduce basic LWP accessors

[Pedro Alves]

On 10/09/2014 10:52 AM, Gary Benson wrote:
> This commit introduces three accessors that shared Linux code can
> use to access fields of struct lwp_info.  The GDB and gdbserver
> Linux x86 code is modified to use them.

OK.

Thanks,
Pedro Alves

[PATCH 03/13 v2] Add iterate_over_lwps to gdbserver

[Gary Benson]

This commit introduces a new function, iterate_over_lwps, that
shared Linux code can use to call a function for each LWP that
matches certain criteria.  This function already existed in GDB
and was in use by GDB's various low-level Linux x86 debug register
setters.  An equivalent was written for gdbserver and gdbserver's
low-level Linux x86 debug register setters were modified to use
it.

gdb/ChangeLog:

	* linux-nat.h: Include nat/linux-nat.h.
	(iterate_over_lwps): Move declaration to nat/linux-nat.h.
	* nat/linux-nat.h (struct lwp_info): New forward declaration.
	(iterate_over_lwps_ftype): New typedef.
	(iterate_over_lwps): New declaration.
	* linux-nat.h (iterate_over_lwps): Update comment.  Use
	iterate_over_lwps_ftype.  Update callback return value check.

gdb/gdbserver/ChangeLog:

	* linux-low.h: Include nat/linux-nat.h.
	* linux-low.c (iterate_over_lwps_args): New structure.
	(iterate_over_lwps_filter): New function.
	(iterate_over_lwps): Likewise.
	* linux-x86-low.c (update_debug_registers_callback):
	Updated signature to what iterate_over_lwps expects.
	Remove PID check that iterate_over_lwps now performs.
	(x86_dr_low_set_addr): Use iterate_over_lwps.
	(x86_dr_low_set_control): Likewise.
---
 gdb/ChangeLog                 |   10 +++++++
 gdb/gdbserver/ChangeLog       |   12 +++++++++
 gdb/gdbserver/linux-low.c     |   54 +++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/linux-low.h     |    1 +
 gdb/gdbserver/linux-x86-low.c |   33 +++++++++----------------
 gdb/linux-nat.c               |    9 ++----
 gdb/linux-nat.h               |    8 +-----
 gdb/nat/linux-nat.h           |   15 +++++++++++
 8 files changed, 108 insertions(+), 34 deletions(-)

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 6e1ed8a..c980d10 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -1304,6 +1304,60 @@ num_lwps (int pid)
   return count;
 }
 
+/* The arguments passed to iterate_over_lwps.  */
+
+struct iterate_over_lwps_args
+{
+  /* The FILTER argument passed to iterate_over_lwps.  */
+  ptid_t filter;
+
+  /* The CALLBACK argument passed to iterate_over_lwps.  */
+  iterate_over_lwps_ftype *callback;
+
+  /* The DATA argument passed to iterate_over_lwps.  */
+  void *data;
+};
+
+/* Callback for find_inferior used by iterate_over_lwps to filter
+   calls to the callback supplied to that function.  Returning a
+   nonzero value causes find_inferiors to stop iterating and return
+   the current inferior_list_entry.  Returning zero indicates that
+   find_inferiors should continue iterating.  */
+
+static int
+iterate_over_lwps_filter (struct inferior_list_entry *entry, void *args_p)
+{
+  struct iterate_over_lwps_args *args
+    = (struct iterate_over_lwps_args *) args_p;
+
+  if (ptid_match (entry->id, args->filter))
+    {
+      struct thread_info *thr = (struct thread_info *) entry;
+      struct lwp_info *lwp = get_thread_lwp (thr);
+
+      return (*args->callback) (lwp, args->data);
+    }
+
+  return 0;
+}
+
+/* See nat/linux-nat.h.  */
+
+struct lwp_info *
+iterate_over_lwps (ptid_t filter,
+		   iterate_over_lwps_ftype callback,
+		   void *data)
+{
+  struct iterate_over_lwps_args args = {filter, callback, data};
+  struct inferior_list_entry *entry;
+
+  entry = find_inferior (&all_threads, iterate_over_lwps_filter, &args);
+  if (entry == NULL)
+    return NULL;
+
+  return get_thread_lwp ((struct thread_info *) entry);
+}
+
 /* Detect zombie thread group leaders, and "exit" them.  We can't reap
    their exits until all other threads in the group have exited.  */
 
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 4820929..11d73f3 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -16,6 +16,7 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#include "nat/linux-nat.h"
 #include "nat/gdb_thread_db.h"
 #include <signal.h>
 
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index e8c6a9c..a861872 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -562,25 +562,16 @@ x86_linux_dr_set (ptid_t ptid, int regnum, unsigned long value)
 }
 
 static int
-update_debug_registers_callback (struct inferior_list_entry *entry,
-				 void *pid_p)
+update_debug_registers_callback (struct lwp_info *lwp, void *arg)
 {
-  struct thread_info *thr = (struct thread_info *) entry;
-  struct lwp_info *lwp = get_thread_lwp (thr);
-  int pid = *(int *) pid_p;
+  /* The actual update is done later just before resuming the lwp,
+     we just mark that the registers need updating.  */
+  lwp->arch_private->debug_registers_changed = 1;
 
-  /* Only update the threads of this process.  */
-  if (pid_of (thr) == pid)
-    {
-      /* The actual update is done later just before resuming the lwp,
-	 we just mark that the registers need updating.  */
-      lwp->arch_private->debug_registers_changed = 1;
-
-      /* If the lwp isn't stopped, force it to momentarily pause, so
-	 we can update its debug registers.  */
-      if (!lwp->stopped)
-	linux_stop_lwp (lwp);
-    }
+  /* If the lwp isn't stopped, force it to momentarily pause, so
+     we can update its debug registers.  */
+  if (!lwp->stopped)
+    linux_stop_lwp (lwp);
 
   return 0;
 }
@@ -591,11 +582,11 @@ static void
 x86_dr_low_set_addr (int regnum, CORE_ADDR addr)
 {
   /* Only update the threads of this process.  */
-  int pid = pid_of (current_thread);
+  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (current_lwp_ptid ()));
 
   gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);
 
-  find_inferior (&all_threads, update_debug_registers_callback, &pid);
+  iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
 }
 
 /* Return the inferior's debug register REGNUM.  */
@@ -614,9 +605,9 @@ static void
 x86_dr_low_set_control (unsigned long control)
 {
   /* Only update the threads of this process.  */
-  int pid = pid_of (current_thread);
+  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (current_lwp_ptid ()));
 
-  find_inferior (&all_threads, update_debug_registers_callback, &pid);
+  iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
 }
 
 /* Return the inferior's DR7 debug control register.  */
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 7d84589..1353d43 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -828,14 +828,11 @@ find_lwp_pid (ptid_t ptid)
   return NULL;
 }
 
-/* Call CALLBACK with its second argument set to DATA for every LWP in
-   the list.  If CALLBACK returns 1 for a particular LWP, return a
-   pointer to the structure describing that LWP immediately.
-   Otherwise return NULL.  */
+/* See nat/linux-nat.h.  */
 
 struct lwp_info *
 iterate_over_lwps (ptid_t filter,
-		   int (*callback) (struct lwp_info *, void *),
+		   iterate_over_lwps_ftype callback,
 		   void *data)
 {
   struct lwp_info *lp, *lpnext;
@@ -846,7 +843,7 @@ iterate_over_lwps (ptid_t filter,
 
       if (ptid_match (lp->ptid, filter))
 	{
-	  if ((*callback) (lp, data))
+	  if ((*callback) (lp, data) != 0)
 	    return lp;
 	}
     }
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index 0aa8377..0195c5a 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -17,8 +17,8 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#include "nat/linux-nat.h"
 #include "target.h"
-
 #include <signal.h>
 
 struct arch_lwp_info;
@@ -126,12 +126,6 @@ extern int lin_lwp_attach_lwp (ptid_t ptid);
 
 extern void linux_stop_lwp (struct lwp_info *lwp);
 
-/* Iterator function for lin-lwp's lwp list.  */
-struct lwp_info *iterate_over_lwps (ptid_t filter,
-				    int (*callback) (struct lwp_info *,
-						     void *), 
-				    void *data);
-
 /* Create a prototype generic GNU/Linux target.  The client can
    override it with local methods.  */
 struct target_ops * linux_target (void);
diff --git a/gdb/nat/linux-nat.h b/gdb/nat/linux-nat.h
index 71d4ee8..a2a0a98 100644
--- a/gdb/nat/linux-nat.h
+++ b/gdb/nat/linux-nat.h
@@ -20,6 +20,8 @@
 #ifndef LINUX_NAT_H
 #define LINUX_NAT_H
 
+struct lwp_info;
+
 /* Unlike other extended result codes, WSTOPSIG (status) on
    PTRACE_O_TRACESYSGOOD syscall events doesn't return SIGTRAP, but
    instead SIGTRAP with bit 7 set.  */
@@ -30,4 +32,17 @@
 
 extern ptid_t current_lwp_ptid (void);
 
+/* Function type for the CALLBACK argument of iterate_over_lwps.  */
+typedef int (iterate_over_lwps_ftype) (struct lwp_info *lwp, void *arg);
+
+/* Iterate over all LWPs.  Calls CALLBACK with its second argument set
+   to DATA for every LWP in the list.  If CALLBACK returns nonzero for
+   a particular LWP, return a pointer to the structure describing that
+   LWP immediately.  Otherwise return NULL.  This function must be
+   provided by the client.  */
+
+extern struct lwp_info *iterate_over_lwps (ptid_t filter,
+					   iterate_over_lwps_ftype callback,
+					   void *data);
+
 #endif /* LINUX_NAT_H */
-- 
1.7.1

Re: [PATCH 03/13 v2] Add iterate_over_lwps to gdbserver

[Pedro Alves]

On 10/09/2014 10:52 AM, Gary Benson wrote:
> This commit introduces a new function, iterate_over_lwps, that
> shared Linux code can use to call a function for each LWP that
> matches certain criteria.  This function already existed in GDB
> and was in use by GDB's various low-level Linux x86 debug register
> setters.  An equivalent was written for gdbserver and gdbserver's
> low-level Linux x86 debug register setters were modified to use
> it.

OK.

Thanks,
Pedro Alves

[PATCH 08/13 v2] Rename gdbserver's low-level Linux x86 debug register accessors

[Gary Benson]

This commit renames gdbserver's low-level Linux x86 debug register
accessors to the same names used by GDB.

gdb/gdbserver/ChangeLog:

	* linux-x86-low.c (x86_dr_low_set_addr): Renamed as...
	(x86_linux_dr_set_addr): New function.
	(x86_dr_low_get_addr): Renamed as...
	(x86_linux_dr_get_addr): New function.
	(x86_dr_low_set_control): Renamed as...
	(x86_linux_dr_set_control): New function.
	(x86_dr_low_get_control): Renamed as...
	(x86_linux_dr_get_control): New function.
	(x86_dr_low_get_status): Renamed as...
	(x86_linux_dr_get_status): New function.
	(x86_dr_low): Updated with new function names.
---
 gdb/gdbserver/ChangeLog       |   14 ++++++++++++++
 gdb/gdbserver/linux-x86-low.c |   20 ++++++++++----------
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 445a07e..e5257a3 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -572,7 +572,7 @@ update_debug_registers_callback (struct lwp_info *lwp, void *arg)
 /* Update the inferior's debug register REGNUM from STATE.  */
 
 static void
-x86_dr_low_set_addr (int regnum, CORE_ADDR addr)
+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 ()));
@@ -585,7 +585,7 @@ x86_dr_low_set_addr (int regnum, CORE_ADDR addr)
 /* Return the inferior's debug register REGNUM.  */
 
 static CORE_ADDR
-x86_dr_low_get_addr (int regnum)
+x86_linux_dr_get_addr (int regnum)
 {
   gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);
 
@@ -595,7 +595,7 @@ x86_dr_low_get_addr (int regnum)
 /* Update the inferior's DR7 debug control register from STATE.  */
 
 static void
-x86_dr_low_set_control (unsigned long control)
+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 ()));
@@ -606,7 +606,7 @@ x86_dr_low_set_control (unsigned long control)
 /* Return the inferior's DR7 debug control register.  */
 
 static unsigned long
-x86_dr_low_get_control (void)
+x86_linux_dr_get_control (void)
 {
   return x86_linux_dr_get (current_lwp_ptid (), DR_CONTROL);
 }
@@ -615,7 +615,7 @@ x86_dr_low_get_control (void)
    and record it in STATE.  */
 
 static unsigned long
-x86_dr_low_get_status (void)
+x86_linux_dr_get_status (void)
 {
   return x86_linux_dr_get (current_lwp_ptid (), DR_STATUS);
 }
@@ -623,11 +623,11 @@ x86_dr_low_get_status (void)
 /* Low-level function vector.  */
 struct x86_dr_low_type x86_dr_low =
   {
-    x86_dr_low_set_control,
-    x86_dr_low_set_addr,
-    x86_dr_low_get_addr,
-    x86_dr_low_get_status,
-    x86_dr_low_get_control,
+    x86_linux_dr_set_control,
+    x86_linux_dr_set_addr,
+    x86_linux_dr_get_addr,
+    x86_linux_dr_get_status,
+    x86_linux_dr_get_control,
     sizeof (void *),
   };
 \f
-- 
1.7.1

Re: [PATCH 08/13 v2] Rename gdbserver's low-level Linux x86 debug register accessors

[Pedro Alves]

On 10/09/2014 10:52 AM, Gary Benson wrote:
> This commit renames gdbserver's low-level Linux x86 debug register
> accessors to the same names used by GDB.
> 
> gdb/gdbserver/ChangeLog:
> 
> 	* linux-x86-low.c (x86_dr_low_set_addr): Renamed as...
> 	(x86_linux_dr_set_addr): New function.
> 	(x86_dr_low_get_addr): Renamed as...
> 	(x86_linux_dr_get_addr): New function.
> 	(x86_dr_low_set_control): Renamed as...
> 	(x86_linux_dr_set_control): New function.
> 	(x86_dr_low_get_control): Renamed as...
> 	(x86_linux_dr_get_control): New function.
> 	(x86_dr_low_get_status): Renamed as...
> 	(x86_linux_dr_get_status): New function.
> 	(x86_dr_low): Updated with new function names.

This "New function" confuses me, as these are not new functions.
Would you mind changing this to (I think) the usual way to write
a rename:

 	* linux-x86-low.c (x86_dr_low_set_addr): Rename to ...
 	(x86_linux_dr_set_addr): ... this.

Also please s/renamed/rename/, s/updated/update/, etc., in this
and other patches.

Otherwise OK.

Thanks,
Pedro Alves

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

[Gary Benson]

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

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

[Pedro Alves]

On 10/09/2014 10:52 AM, Gary Benson wrote:
> 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.

OK.

Thanks,
Pedro Alves

[PATCH 11/13 v2] Introduce x86_linux_update_debug_registers

[Gary Benson]

This commit moves the entire body of both GDB's and gdbserver's
x86_linux_prepare_to_resume functions into new functions,
x86_linux_update_debug_registers.  This reorganisation allows
all Linux x86 low-level debug register code to be placed in one
shared file, separate from general Linux x86 shared code.

gdb/ChangeLog:

	* x86-linux-nat.c (x86_linux_update_debug_registers):
	New function.
	(x86_linux_prepare_to_resume): Call the above.

gdb/gdbserver/ChangeLog:

	* linux-x86-low.c (x86_linux_update_debug_registers):
	New function.
	(x86_linux_prepare_to_resume): Call the above.
---
 gdb/ChangeLog                 |    6 ++++++
 gdb/gdbserver/ChangeLog       |    6 ++++++
 gdb/gdbserver/linux-x86-low.c |   16 +++++++++++++---
 gdb/x86-linux-nat.c           |   16 +++++++++++++---
 4 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index f4d6437..bb8e1a7 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -763,15 +763,17 @@ x86_debug_reg_state (pid_t pid)
   return &proc->private->arch_private->debug_reg_state;
 }
 
-/* Called prior to resuming a thread.  Updates the thread's debug
-   registers if the values in our local mirror have been changed.  */
+/* Update 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)
+x86_linux_update_debug_registers (struct lwp_info *lwp)
 {
   ptid_t ptid = ptid_of_lwp (lwp);
   int clear_status = 0;
 
+  gdb_assert (lwp_is_stopped (lwp));
+
   if (lwp_debug_registers_changed (lwp))
     {
       struct x86_debug_reg_state *state
@@ -807,6 +809,14 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
   if (clear_status || lwp_is_stopped_by_watchpoint (lwp))
     x86_linux_dr_set (ptid, DR_STATUS, 0);
 }
+
+/* Called prior to resuming a thread.  */
+
+static void
+x86_linux_prepare_to_resume (struct lwp_info *lwp)
+{
+  x86_linux_update_debug_registers (lwp);
+}
 \f
 /* When GDBSERVER is built as a 64-bit application on linux, the
    PTRACE_GETSIGINFO data is always presented in 64-bit layout.  Since
diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
index d806474..55c3bd3 100644
--- a/gdb/x86-linux-nat.c
+++ b/gdb/x86-linux-nat.c
@@ -162,15 +162,17 @@ x86_linux_dr_set_addr (int regnum, CORE_ADDR addr)
   iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
 }
 
-/* Called prior to resuming a thread.  Updates the thread's debug
-   registers if the values in our local mirror have been changed.  */
+/* Update 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)
+x86_linux_update_debug_registers (struct lwp_info *lwp)
 {
   ptid_t ptid = ptid_of_lwp (lwp);
   int clear_status = 0;
 
+  gdb_assert (lwp_is_stopped (lwp));
+
   if (lwp_debug_registers_changed (lwp))
     {
       struct x86_debug_reg_state *state
@@ -207,6 +209,14 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
     x86_linux_dr_set (ptid, DR_STATUS, 0);
 }
 
+/* Called prior to resuming a thread.  */
+
+static void
+x86_linux_prepare_to_resume (struct lwp_info *lwp)
+{
+  x86_linux_update_debug_registers (lwp);
+}
+
 /* Called when a new thread is detected.  */
 
 static void
-- 
1.7.1

Re: [PATCH 11/13 v2] Introduce x86_linux_update_debug_registers

[Pedro Alves]

On 10/09/2014 10:52 AM, Gary Benson wrote:
> This commit moves the entire body of both GDB's and gdbserver's
> x86_linux_prepare_to_resume functions into new functions,
> x86_linux_update_debug_registers.  This reorganisation allows
> all Linux x86 low-level debug register code to be placed in one
> shared file, separate from general Linux x86 shared code.
> 
> gdb/ChangeLog:
> 
> 	* x86-linux-nat.c (x86_linux_update_debug_registers):
> 	New function.
> 	(x86_linux_prepare_to_resume): Call the above.

I'd prefer making it a little clearer that this is code being
factored out, instead of being really new code, like:

 	* x86-linux-nat.c (x86_linux_update_debug_registers):
 	New function, factored out from ...
 	(x86_linux_prepare_to_resume): ... this.

> gdb/gdbserver/ChangeLog:
> 
> 	* linux-x86-low.c (x86_linux_update_debug_registers):
> 	New function.
> 	(x86_linux_prepare_to_resume): Call the above.

OK.

Thanks,
Pedro Alves

[PATCH 06/13 v2] Change signature of linux_target_ops.new_thread

[Gary Benson]

This commit changes the signature of linux_target_ops.new_thread in
gdbserver to match that used in GDB's equivalent.

gdb/gdbserver/ChangeLog:

	* linux-low.h (linux_target_ops) <new_thread>: Changed signature.
	* linux-arm-low.c (arm_new_thread): Likewise.
	* linux-aarch64-low.c (aarch64_linux_new_thread): Likewise.
	* linux-mips-low.c (mips_linux_new_thread): Likewise.
	* linux-x86-low.c (x86_linux_new_thread): Likewise.
	* linux-low.c (add_lwp): Update the_low_target.new_thread call.
---
 gdb/gdbserver/ChangeLog           |    9 +++++++++
 gdb/gdbserver/linux-aarch64-low.c |    6 +++---
 gdb/gdbserver/linux-arm-low.c     |    6 +++---
 gdb/gdbserver/linux-low.c         |    2 +-
 gdb/gdbserver/linux-low.h         |    2 +-
 gdb/gdbserver/linux-mips-low.c    |    6 +++---
 gdb/gdbserver/linux-x86-low.c     |    6 +++---
 7 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
index 654b319..82f3ec2 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -1115,8 +1115,8 @@ aarch64_linux_new_process (void)
 
 /* Called when a new thread is detected.  */
 
-static struct arch_lwp_info *
-aarch64_linux_new_thread (void)
+static void
+aarch64_linux_new_thread (struct lwp_info *lwp)
 {
   struct arch_lwp_info *info = xcalloc (1, sizeof (*info));
 
@@ -1126,7 +1126,7 @@ aarch64_linux_new_thread (void)
   DR_MARK_ALL_CHANGED (info->dr_changed_bp, aarch64_num_bp_regs);
   DR_MARK_ALL_CHANGED (info->dr_changed_wp, aarch64_num_wp_regs);
 
-  return info;
+  lwp->arch_private = info;
 }
 
 /* Called when resuming a thread.
diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index 8b72523..d33c622 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -703,8 +703,8 @@ arm_new_process (void)
 }
 
 /* Called when a new thread is detected.  */
-static struct arch_lwp_info *
-arm_new_thread (void)
+static void
+arm_new_thread (struct lwp_info *lwp)
 {
   struct arch_lwp_info *info = xcalloc (1, sizeof (*info));
   int i;
@@ -714,7 +714,7 @@ arm_new_thread (void)
   for (i = 0; i < MAX_WPTS; i++)
     info->wpts_changed[i] = 1;
 
-  return info;
+  lwp->arch_private = info;
 }
 
 /* Called when resuming a thread.
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 4992945..15a5c56 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -556,7 +556,7 @@ add_lwp (ptid_t ptid)
   memset (lwp, 0, sizeof (*lwp));
 
   if (the_low_target.new_thread != NULL)
-    lwp->arch_private = the_low_target.new_thread ();
+    the_low_target.new_thread (lwp);
 
   lwp->thread = add_thread (ptid, lwp);
 
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 697e0fb..5ef0807 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -184,7 +184,7 @@ struct linux_target_ops
   /* Hook to call when a new thread is detected.
      If extra per-thread architecture-specific data is needed,
      allocate it here.  */
-  struct arch_lwp_info * (*new_thread) (void);
+  void (*new_thread) (struct lwp_info *);
 
   /* Hook to call prior to resuming a thread.  */
   void (*prepare_to_resume) (struct lwp_info *);
diff --git a/gdb/gdbserver/linux-mips-low.c b/gdb/gdbserver/linux-mips-low.c
index 0fc8cb4..4e09ce4 100644
--- a/gdb/gdbserver/linux-mips-low.c
+++ b/gdb/gdbserver/linux-mips-low.c
@@ -334,14 +334,14 @@ mips_linux_new_process (void)
    Mark the watch registers as changed, so the threads' copies will
    be updated.  */
 
-static struct arch_lwp_info *
-mips_linux_new_thread (void)
+static void
+mips_linux_new_thread (struct lwp_info *lwp)
 {
   struct arch_lwp_info *info = xcalloc (1, sizeof (*info));
 
   info->watch_registers_changed = 1;
 
-  return info;
+  lwp->arch_private = info;
 }
 
 /* This is the implementation of linux_target_ops method
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 64fab4c..f04e1f1 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -744,14 +744,14 @@ x86_linux_new_process (void)
 
 /* Called when a new thread is detected.  */
 
-static struct arch_lwp_info *
-x86_linux_new_thread (void)
+static void
+x86_linux_new_thread (struct lwp_info *lwp)
 {
   struct arch_lwp_info *info = XCNEW (struct arch_lwp_info);
 
   info->debug_registers_changed = 1;
 
-  return info;
+  lwp->arch_private = info;
 }
 
 /* See nat/x86-dregs.h.  */
-- 
1.7.1

Re: [PATCH 06/13 v2] Change signature of linux_target_ops.new_thread

[Pedro Alves]

On 10/09/2014 10:52 AM, Gary Benson wrote:
> This commit changes the signature of linux_target_ops.new_thread in
> gdbserver to match that used in GDB's equivalent.
> 
> gdb/gdbserver/ChangeLog:
> 
> 	* linux-low.h (linux_target_ops) <new_thread>: Changed signature.
> 	* linux-arm-low.c (arm_new_thread): Likewise.
> 	* linux-aarch64-low.c (aarch64_linux_new_thread): Likewise.
> 	* linux-mips-low.c (mips_linux_new_thread): Likewise.
> 	* linux-x86-low.c (x86_linux_new_thread): Likewise.
> 	* linux-low.c (add_lwp): Update the_low_target.new_thread call.

OK

Thanks,
Pedro Alves

[PATCH 13/13 v2] Move duplicated Linux x86 code to nat/x86-linux.c

[Gary Benson]

This commit moves two identical functions from gdb/x86-linux-nat.c and
gdb/gdbserver/linux-x86-low.c into the shared file gdb/nat/x86-linux.c.

gdb/ChangeLog:

	* nat/x86-linux.h (x86_linux_new_thread): New declaration.
	(x86_linux_prepare_to_resume): Likewise.
	* x86-linux-nat.c (x86_linux_new_thread):
	Moved to nat/x86-linux.c.
	(x86_linux_prepare_to_resume): Likewise.
	* nat/x86-linux.c (x86_linux_new_thread): New function.
	(x86_linux_prepare_to_resume): Likewise.

gdb/gdbserver/ChangeLog:

	* linux-x86-low.c (x86_linux_new_thread): Removed.
	(x86_linux_prepare_to_resume): Likewise.
---
 gdb/ChangeLog                 |   10 ++++++++++
 gdb/gdbserver/ChangeLog       |    5 +++++
 gdb/gdbserver/linux-x86-low.c |   16 ----------------
 gdb/nat/x86-linux.c           |   17 +++++++++++++++++
 gdb/nat/x86-linux.h           |    8 ++++++++
 gdb/x86-linux-nat.c           |   16 ----------------
 6 files changed, 40 insertions(+), 32 deletions(-)

diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index b1d2d27..40e96e2 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -637,14 +637,6 @@ x86_linux_new_process (void)
   return info;
 }
 
-/* Called when a new thread is detected.  */
-
-static void
-x86_linux_new_thread (struct lwp_info *lwp)
-{
-  lwp_set_debug_registers_changed (lwp, 1);
-}
-
 /* See nat/x86-dregs.h.  */
 
 struct x86_debug_reg_state *
@@ -654,14 +646,6 @@ x86_debug_reg_state (pid_t pid)
 
   return &proc->private->arch_private->debug_reg_state;
 }
-
-/* Called prior to resuming a thread.  */
-
-static void
-x86_linux_prepare_to_resume (struct lwp_info *lwp)
-{
-  x86_linux_update_debug_registers (lwp);
-}
 \f
 /* When GDBSERVER is built as a 64-bit application on linux, the
    PTRACE_GETSIGINFO data is always presented in 64-bit layout.  Since
diff --git a/gdb/nat/x86-linux.c b/gdb/nat/x86-linux.c
index efeeab9..7dd3f9b 100644
--- a/gdb/nat/x86-linux.c
+++ b/gdb/nat/x86-linux.c
@@ -19,6 +19,7 @@
 
 #include "common-defs.h"
 #include "x86-linux.h"
+#include "x86-linux-dregs.h"
 
 /* Per-thread arch-specific data we want to keep.  */
 
@@ -55,3 +56,19 @@ lwp_debug_registers_changed (struct lwp_info *lwp)
 
   return info->debug_registers_changed;
 }
+
+/* See nat/x86-linux.h.  */
+
+void
+x86_linux_new_thread (struct lwp_info *lwp)
+{
+  lwp_set_debug_registers_changed (lwp, 1);
+}
+
+/* See nat/x86-linux.h.  */
+
+void
+x86_linux_prepare_to_resume (struct lwp_info *lwp)
+{
+  x86_linux_update_debug_registers (lwp);
+}
diff --git a/gdb/nat/x86-linux.h b/gdb/nat/x86-linux.h
index 6409898..76271b5 100644
--- a/gdb/nat/x86-linux.h
+++ b/gdb/nat/x86-linux.h
@@ -35,4 +35,12 @@ extern void lwp_set_debug_registers_changed (struct lwp_info *lwp,
 
 extern int lwp_debug_registers_changed (struct lwp_info *lwp);
 
+/* Function to call when a new thread is detected.  */
+
+extern void x86_linux_new_thread (struct lwp_info *lwp);
+
+/* Function to call prior to resuming a thread.  */
+
+extern void x86_linux_prepare_to_resume (struct lwp_info *lwp);
+
 #endif /* X86_LINUX_H */
diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
index 97ce443..61710ce 100644
--- a/gdb/x86-linux-nat.c
+++ b/gdb/x86-linux-nat.c
@@ -52,22 +52,6 @@ struct arch_lwp_info
 
 /* Does the current host support PTRACE_GETREGSET?  */
 int have_ptrace_getregset = -1;
-
-/* Called prior to resuming a thread.  */
-
-static void
-x86_linux_prepare_to_resume (struct lwp_info *lwp)
-{
-  x86_linux_update_debug_registers (lwp);
-}
-
-/* Called when a new thread is detected.  */
-
-static void
-x86_linux_new_thread (struct lwp_info *lwp)
-{
-  lwp_set_debug_registers_changed (lwp, 1);
-}
 \f
 
 /* linux_nat_new_fork hook.   */
-- 
1.7.1

Re: [PATCH 13/13 v2] Move duplicated Linux x86 code to nat/x86-linux.c

[Pedro Alves]

On 10/09/2014 10:52 AM, Gary Benson wrote:
> This commit moves two identical functions from gdb/x86-linux-nat.c and
> gdb/gdbserver/linux-x86-low.c into the shared file gdb/nat/x86-linux.c.
> 
> gdb/ChangeLog:
> 
> 	* nat/x86-linux.h (x86_linux_new_thread): New declaration.
> 	(x86_linux_prepare_to_resume): Likewise.
> 	* x86-linux-nat.c (x86_linux_new_thread):
> 	Moved to nat/x86-linux.c.
> 	(x86_linux_prepare_to_resume): Likewise.
> 	* nat/x86-linux.c (x86_linux_new_thread): New function.
> 	(x86_linux_prepare_to_resume): Likewise.
> 
> gdb/gdbserver/ChangeLog:
> 
> 	* linux-x86-low.c (x86_linux_new_thread): Removed.
> 	(x86_linux_prepare_to_resume): Likewise.

OK.

Thanks,
Pedro Alves

[PATCH 09/13 v2] Linux x86 low-level debug register code synchronization

[Gary Benson]

This commit makes several small changes to the low-level debug
register code for Linux x86, making the code in the GDB and
gdbserver implementations identical.

gdb/ChangeLog:

	* x86-linux-nat.c (x86_linux_dr_set_addr): Updated assertion.
	(x86_linux_new_thread): Renamed argument.

gdb/gdbserver/ChangeLog:

	* linux-x86-low.c (x86_linux_dr_get): Add assertion.
	Use perror_with_name.
	(x86_linux_dr_set): Likewise.
---
 gdb/ChangeLog                 |    5 +++++
 gdb/gdbserver/ChangeLog       |    6 ++++++
 gdb/gdbserver/linux-x86-low.c |    6 ++++--
 gdb/x86-linux-nat.c           |    6 +++---
 4 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index e5257a3..3064791 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -529,13 +529,14 @@ x86_linux_dr_get (ptid_t ptid, int regnum)
   int tid;
   unsigned long value;
 
+  gdb_assert (ptid_lwp_p (ptid));
   tid = ptid_get_lwp (ptid);
 
   errno = 0;
   value = ptrace (PTRACE_PEEKUSER, tid,
 		  offsetof (struct user, u_debugreg[regnum]), 0);
   if (errno != 0)
-    error ("Couldn't read debug register");
+    perror_with_name (_("Couldn't read debug register"));
 
   return value;
 }
@@ -545,13 +546,14 @@ x86_linux_dr_set (ptid_t ptid, int regnum, unsigned long value)
 {
   int tid;
 
+  gdb_assert (ptid_lwp_p (ptid));
   tid = ptid_get_lwp (ptid);
 
   errno = 0;
   ptrace (PTRACE_POKEUSER, tid,
 	  offsetof (struct user, u_debugreg[regnum]), value);
   if (errno != 0)
-    error ("Couldn't write debug register");
+    perror_with_name (_("Couldn't write debug register"));
 }
 
 static int
diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
index c4387eb..c5c8576 100644
--- a/gdb/x86-linux-nat.c
+++ b/gdb/x86-linux-nat.c
@@ -156,7 +156,7 @@ x86_linux_dr_set_addr (int regnum, CORE_ADDR addr)
 {
   ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (current_lwp_ptid ()));
 
-  gdb_assert (regnum >= 0 && regnum <= DR_LASTADDR - DR_FIRSTADDR);
+  gdb_assert (DR_FIRSTADDR <= regnum && regnum <= DR_LASTADDR);
 
   iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
 }
@@ -213,9 +213,9 @@ x86_linux_prepare_to_resume (struct lwp_info *lwp)
 }
 
 static void
-x86_linux_new_thread (struct lwp_info *lp)
+x86_linux_new_thread (struct lwp_info *lwp)
 {
-  lwp_set_debug_registers_changed (lp, 1);
+  lwp_set_debug_registers_changed (lwp, 1);
 }
 \f
 
-- 
1.7.1

Re: [PATCH 09/13 v2] Linux x86 low-level debug register code synchronization

[Pedro Alves]

On 10/09/2014 10:52 AM, Gary Benson wrote:
> This commit makes several small changes to the low-level debug
> register code for Linux x86, making the code in the GDB and
> gdbserver implementations identical.
> 
> gdb/ChangeLog:
> 
> 	* x86-linux-nat.c (x86_linux_dr_set_addr): Updated assertion.
> 	(x86_linux_new_thread): Renamed argument.

"Update"m "Rename".

> 
> gdb/gdbserver/ChangeLog:
> 
> 	* linux-x86-low.c (x86_linux_dr_get): Add assertion.
> 	Use perror_with_name.

 	* linux-x86-low.c (x86_linux_dr_get): Add assertion.
 	Use perror_with_name.  Pass string through gettext.

> 	(x86_linux_dr_set): Likewise.

OK.

Thanks,
Pedro Alves

[pushed] Refactor low-level Linux x86 debug register code

[Gary Benson]

Hi all,

Gary Benson wrote:
> This series refactors the low-level Linux x86 debug register code
> in x86-linux-nat.c and gdbserver/linux-x86-low.c into new shared
> files nat/x86-linux.[ch] and nat/x86-linux-dregs.[ch].
> 
> The first ten patches smooth out various differences between the
> two implementations, and the final three patches perform the code
> reorganization.
> 
> This series replaces the previous version that I retracted after
> renaming gdbserver's current_inferior as current_thread.  This
> series has been reworked to reflect this and is substantially
> cleaner.

I just pushed this series that Pedro approved back in October, with
all of Pedro's and Doug's suggestions and a couple of tweaks to get
it to apply to the current tree.

Cheers,
Gary

-- 
http://gbenson.net/