[01/13,v2] Introduce current_lwp_ptid

Message ID 1412848358-9958-2-git-send-email-gbenson@redhat.com
State Committed
Headers

Commit Message

Gary Benson Oct. 9, 2014, 9:52 a.m. UTC
  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(-)
  

Comments

Pedro Alves Oct. 28, 2014, 12:55 p.m. UTC | #1
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
  
Doug Evans Oct. 28, 2014, 4:44 p.m. UTC | #2
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?
  
Pedro Alves Oct. 28, 2014, 5:04 p.m. UTC | #3
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
  
Doug Evans Oct. 28, 2014, 5:12 p.m. UTC | #4
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.]
  
Doug Evans Oct. 28, 2014, 5:13 p.m. UTC | #5
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/.
  
Doug Evans Oct. 28, 2014, 7:43 p.m. UTC | #6
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.]
  
Doug Evans Oct. 31, 2014, 6:57 p.m. UTC | #7
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

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