[PATCH v1.99 0/6] Make register fetch/store implementations use ptid from regcache

[PATCH v1.99 0/6] Make register fetch/store implementations use ptid from regcache

[Simon Marchi]

This is almost a v2 of:

  Pass ptid to target_ops register methods
  https://sourceware.org/ml/gdb-patches/2017-03/msg00100.html

but since the work can be done incrementally (unlike the original version), I'd
like to make sure that the idea is good before proceeding with all the changes.
I've only done changes to a few target files for the moment and will do the
others if these look good..

Simon Marchi (6):
  Introduce regcache_get_ptid
  Add asserts in target_fetch/store_registers
  amd64-linux-nat: Use ptid from regcache instead of inferior_ptid
  aarc64-linux-nat: Use ptid from regcache instead of inferior_ptid
  aix-thread: Use ptid from regcache instead of inferior_ptid
  alpha-bsd-nat: Use ptid from regcache instead of inferior_ptid

 gdb/aarch64-linux-nat.c |  8 ++++----
 gdb/aix-thread.c        | 14 +++++++-------
 gdb/alpha-bsd-nat.c     | 12 ++++++------
 gdb/amd64-linux-nat.c   |  8 ++++----
 gdb/regcache.c          | 10 ++++++++++
 gdb/regcache.h          |  4 ++++
 gdb/target.c            |  4 ++++
 7 files changed, 39 insertions(+), 21 deletions(-)

-- 
2.12.0

[PATCH v1.99 1/6] Introduce regcache_get_ptid

[Simon Marchi]

This patch introduces the regcache_get_ptid function, which can be used
to retrieve the ptid a regcache is connected to.  It is used in
subsequent patches.

gdb/ChangeLog:

	* regcache.h (regcache_get_ptid): New function.
	* regcache.c (regcache_get_ptid): New function.
---
 gdb/regcache.c | 10 ++++++++++
 gdb/regcache.h |  4 ++++
 2 files changed, 14 insertions(+)

diff --git a/gdb/regcache.c b/gdb/regcache.c
index 0728a03b65..58d4f56292 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -215,6 +215,16 @@ struct regcache
   ptid_t ptid;
 };
 
+/* See regcache.h.  */
+
+ptid_t
+regcache_get_ptid (const struct regcache *regcache)
+{
+  gdb_assert (!ptid_equal (regcache->ptid, minus_one_ptid));
+
+  return regcache->ptid;
+}
+
 static struct regcache *
 regcache_xmalloc_1 (struct gdbarch *gdbarch, struct address_space *aspace,
 		    int readonly_p)
diff --git a/gdb/regcache.h b/gdb/regcache.h
index e1495f6142..d0107cd76e 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -39,6 +39,10 @@ struct cleanup *make_cleanup_regcache_xfree (struct regcache *regcache);
 struct regcache *regcache_xmalloc (struct gdbarch *gdbarch,
 				   struct address_space *aspace);
 
+/* Return REGCACHE's ptid.  */
+
+extern ptid_t regcache_get_ptid (const struct regcache *regcache);
+
 /* Return REGCACHE's architecture.  */
 
 extern struct gdbarch *get_regcache_arch (const struct regcache *regcache);
-- 
2.12.0

[PATCH v1.99 6/6] alpha-bsd-nat: Use ptid from regcache instead of inferior_ptid

[Simon Marchi]

gdb/ChangeLog:

	* alpha-bsd-nat.c (alphabsd_fetch_inferior_registers,
	alphabsd_store_inferior_registers): Use regcache->ptid instead
	of inferior_ptid.
---
 gdb/alpha-bsd-nat.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/gdb/alpha-bsd-nat.c b/gdb/alpha-bsd-nat.c
index 5914d89882..102555344d 100644
--- a/gdb/alpha-bsd-nat.c
+++ b/gdb/alpha-bsd-nat.c
@@ -91,7 +91,7 @@ alphabsd_fetch_inferior_registers (struct target_ops *ops,
     {
       struct reg gregs;
 
-      if (ptrace (PT_GETREGS, ptid_get_pid (inferior_ptid),
+      if (ptrace (PT_GETREGS, ptid_get_pid (regcache_get_ptid (regcache)),
 		  (PTRACE_TYPE_ARG3) &gregs, 0) == -1)
 	perror_with_name (_("Couldn't get registers"));
 
@@ -105,7 +105,7 @@ alphabsd_fetch_inferior_registers (struct target_ops *ops,
     {
       struct fpreg fpregs;
 
-      if (ptrace (PT_GETFPREGS, ptid_get_pid (inferior_ptid),
+      if (ptrace (PT_GETFPREGS, ptid_get_pid (regcache_get_ptid (regcache)),
 		  (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
 	perror_with_name (_("Couldn't get floating point status"));
 
@@ -123,13 +123,13 @@ alphabsd_store_inferior_registers (struct target_ops *ops,
   if (regno == -1 || getregs_supplies (regno))
     {
       struct reg gregs;
-      if (ptrace (PT_GETREGS, ptid_get_pid (inferior_ptid),
+      if (ptrace (PT_GETREGS, ptid_get_pid (regcache_get_ptid (regcache)),
                   (PTRACE_TYPE_ARG3) &gregs, 0) == -1)
         perror_with_name (_("Couldn't get registers"));
 
       alphabsd_fill_reg (regcache, (char *) &gregs, regno);
 
-      if (ptrace (PT_SETREGS, ptid_get_pid (inferior_ptid),
+      if (ptrace (PT_SETREGS, ptid_get_pid (regcache_get_ptid (regcache)),
                   (PTRACE_TYPE_ARG3) &gregs, 0) == -1)
         perror_with_name (_("Couldn't write registers"));
 
@@ -142,13 +142,13 @@ alphabsd_store_inferior_registers (struct target_ops *ops,
     {
       struct fpreg fpregs;
 
-      if (ptrace (PT_GETFPREGS, ptid_get_pid (inferior_ptid),
+      if (ptrace (PT_GETFPREGS, ptid_get_pid (regcache_get_ptid (regcache)),
 		  (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
 	perror_with_name (_("Couldn't get floating point status"));
 
       alphabsd_fill_fpreg (regcache, (char *) &fpregs, regno);
 
-      if (ptrace (PT_SETFPREGS, ptid_get_pid (inferior_ptid),
+      if (ptrace (PT_SETFPREGS, ptid_get_pid (regcache_get_ptid (regcache)),
 		  (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
 	perror_with_name (_("Couldn't write floating point status"));
     }
-- 
2.12.0

[PATCH v1.99 3/6] amd64-linux-nat: Use ptid from regcache instead of inferior_ptid

[Simon Marchi]

gdb/ChangeLog:

	* amd64-linux-nat.c (amd64_linux_fetch_inferior_registers,
	amd64_linux_fetch_inferior_registers): Use regcache->ptid
	instead of inferior_ptid.
---
 gdb/amd64-linux-nat.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index 4a429ec2a1..d24c028d21 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -138,9 +138,9 @@ amd64_linux_fetch_inferior_registers (struct target_ops *ops,
   int tid;
 
   /* GNU/Linux LWP ID's are process ID's.  */
-  tid = ptid_get_lwp (inferior_ptid);
+  tid = ptid_get_lwp (regcache_get_ptid (regcache));
   if (tid == 0)
-    tid = ptid_get_pid (inferior_ptid); /* Not a threaded program.  */
+    tid = ptid_get_pid (regcache_get_ptid (regcache)); /* Not a threaded program.  */
 
   if (regnum == -1 || amd64_native_gregset_supplies_p (gdbarch, regnum))
     {
@@ -217,9 +217,9 @@ amd64_linux_store_inferior_registers (struct target_ops *ops,
   int tid;
 
   /* GNU/Linux LWP ID's are process ID's.  */
-  tid = ptid_get_lwp (inferior_ptid);
+  tid = ptid_get_lwp (regcache_get_ptid (regcache));
   if (tid == 0)
-    tid = ptid_get_pid (inferior_ptid); /* Not a threaded program.  */
+    tid = ptid_get_pid (regcache_get_ptid (regcache)); /* Not a threaded program.  */
 
   if (regnum == -1 || amd64_native_gregset_supplies_p (gdbarch, regnum))
     {
-- 
2.12.0

[PATCH v1.99 2/6] Add asserts in target_fetch/store_registers

[Simon Marchi]

We are currently assuming that regcache->ptid is equal to inferior_ptid
when we call target_fetch/store_registers.  These asserts just validate
that assumption.  Also, since the following patches will change target
code to use regcache->ptid instead of inferior_ptid, asserting that they
are the same should ensure that our changes don't have any unintended
consequences.

gdb/ChangeLog:

	* target.c (target_fetch_registers, target_store_registers): Add
	assert.
---
 gdb/target.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gdb/target.c b/gdb/target.c
index 0ff8515d3b..359bf0dec9 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3587,6 +3587,8 @@ debug_print_register (const char * func,
 void
 target_fetch_registers (struct regcache *regcache, int regno)
 {
+  gdb_assert (ptid_equal (regcache_get_ptid (regcache), inferior_ptid));
+
   current_target.to_fetch_registers (&current_target, regcache, regno);
   if (targetdebug)
     debug_print_register ("target_fetch_registers", regcache, regno);
@@ -3598,6 +3600,8 @@ target_store_registers (struct regcache *regcache, int regno)
   if (!may_write_registers)
     error (_("Writing to registers is not allowed (regno %d)"), regno);
 
+  gdb_assert (ptid_equal (regcache_get_ptid (regcache), inferior_ptid));
+
   current_target.to_store_registers (&current_target, regcache, regno);
   if (targetdebug)
     {
-- 
2.12.0

[PATCH v1.99 4/6] aarc64-linux-nat: Use ptid from regcache instead of inferior_ptid

[Simon Marchi]

gdb/ChangeLog:

	* aarch64-linux-nat.c (fetch_gregs_from_thread,
	store_gregs_to_thread, fetch_fpregs_from_thread,
	store_fpregs_to_thread): Use regcache->ptid instead of
	inferior_ptid.
---
 gdb/aarch64-linux-nat.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
index 0d472e2e53..3f5b30eaeb 100644
--- a/gdb/aarch64-linux-nat.c
+++ b/gdb/aarch64-linux-nat.c
@@ -163,7 +163,7 @@ fetch_gregs_from_thread (struct regcache *regcache)
      and arm.  */
   gdb_static_assert (sizeof (regs) >= 18 * 4);
 
-  tid = ptid_get_lwp (inferior_ptid);
+  tid = ptid_get_lwp (regcache_get_ptid (regcache));
 
   iovec.iov_base = &regs;
   if (gdbarch_bfd_arch_info (gdbarch)->bits_per_word == 32)
@@ -200,7 +200,7 @@ store_gregs_to_thread (const struct regcache *regcache)
   /* Make sure REGS can hold all registers contents on both aarch64
      and arm.  */
   gdb_static_assert (sizeof (regs) >= 18 * 4);
-  tid = ptid_get_lwp (inferior_ptid);
+  tid = ptid_get_lwp (regcache_get_ptid (regcache));
 
   iovec.iov_base = &regs;
   if (gdbarch_bfd_arch_info (gdbarch)->bits_per_word == 32)
@@ -244,7 +244,7 @@ fetch_fpregs_from_thread (struct regcache *regcache)
      and arm.  */
   gdb_static_assert (sizeof regs >= VFP_REGS_SIZE);
 
-  tid = ptid_get_lwp (inferior_ptid);
+  tid = ptid_get_lwp (regcache_get_ptid (regcache));
 
   iovec.iov_base = &regs;
 
@@ -291,7 +291,7 @@ store_fpregs_to_thread (const struct regcache *regcache)
   /* Make sure REGS can hold all VFP registers contents on both aarch64
      and arm.  */
   gdb_static_assert (sizeof regs >= VFP_REGS_SIZE);
-  tid = ptid_get_lwp (inferior_ptid);
+  tid = ptid_get_lwp (regcache_get_ptid (regcache));
 
   iovec.iov_base = &regs;
 
-- 
2.12.0

[PATCH v1.99 5/6] aix-thread: Use ptid from regcache instead of inferior_ptid

[Simon Marchi]

gdb/ChangeLog:

	* aix-thread.c (aix_thread_fetch_registers,
	aix_thread_store_registers): Use regcache->ptid instead of
	inferior_ptid.
---
 gdb/aix-thread.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index cf1a462216..3fbe96a736 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -1297,8 +1297,8 @@ fetch_regs_kernel_thread (struct regcache *regcache, int regno,
     }
 }
 
-/* Fetch register REGNO if != -1 or all registers otherwise in the
-   thread/process specified by inferior_ptid.  */
+/* Fetch register REGNO if != -1 or all registers otherwise from the
+   thread/process connected to REGCACHE.  */
 
 static void
 aix_thread_fetch_registers (struct target_ops *ops,
@@ -1308,11 +1308,11 @@ aix_thread_fetch_registers (struct target_ops *ops,
   pthdb_tid_t tid;
   struct target_ops *beneath = find_target_beneath (ops);
 
-  if (!PD_TID (inferior_ptid))
+  if (!PD_TID (regcache_get_ptid (regcache)))
     beneath->to_fetch_registers (beneath, regcache, regno);
   else
     {
-      thread = find_thread_ptid (inferior_ptid);
+      thread = find_thread_ptid (regcache_get_ptid (regcache));
       tid = thread->priv->tid;
 
       if (tid == PTHDB_INVALID_TID)
@@ -1652,7 +1652,7 @@ store_regs_kernel_thread (const struct regcache *regcache, int regno,
 }
 
 /* Store gdb's current view of the register set into the
-   thread/process specified by inferior_ptid.  */
+   thread/process connected to REGCACHE.  */
 
 static void
 aix_thread_store_registers (struct target_ops *ops,
@@ -1662,11 +1662,11 @@ aix_thread_store_registers (struct target_ops *ops,
   pthdb_tid_t tid;
   struct target_ops *beneath = find_target_beneath (ops);
 
-  if (!PD_TID (inferior_ptid))
+  if (!PD_TID (regcache_get_ptid (regcache)))
     beneath->to_store_registers (beneath, regcache, regno);
   else
     {
-      thread = find_thread_ptid (inferior_ptid);
+      thread = find_thread_ptid (regcache_get_ptid (regcache));
       tid = thread->priv->tid;
 
       if (tid == PTHDB_INVALID_TID)
-- 
2.12.0

Re: [PATCH v1.99 0/6] Make register fetch/store implementations use ptid from regcache

[Pedro Alves]

On 03/11/2017 05:03 PM, Simon Marchi wrote:
> This is almost a v2 of:
> 
>   Pass ptid to target_ops register methods
>   https://sourceware.org/ml/gdb-patches/2017-03/msg00100.html
> 
> but since the work can be done incrementally (unlike the original version), I'd
> like to make sure that the idea is good before proceeding with all the changes.
> I've only done changes to a few target files for the moment and will do the
> others if these look good..

LGTM.

[ I'd also be fine with moving the struct regcache definition to the
header in order to add a regcache::ptid() method.  :-) ]

Thanks,
Pedro Alves

Re: [PATCH v1.99 0/6] Make register fetch/store implementations use ptid from regcache

[Simon Marchi]

On 2017-03-13 15:15, Pedro Alves wrote:
> On 03/11/2017 05:03 PM, Simon Marchi wrote:
>> This is almost a v2 of:
>> 
>>   Pass ptid to target_ops register methods
>>   https://sourceware.org/ml/gdb-patches/2017-03/msg00100.html
>> 
>> but since the work can be done incrementally (unlike the original 
>> version), I'd
>> like to make sure that the idea is good before proceeding with all the 
>> changes.
>> I've only done changes to a few target files for the moment and will 
>> do the
>> others if these look good..
> 
> LGTM.

Thanks, pushed.

> [ I'd also be fine with moving the struct regcache definition to the
> header in order to add a regcache::ptid() method.  :-) ]

Well, if we move the struct to the header, we don't need a ptid method, 
since the field is public :).  But yeah, it would be an obvious 
candidate for cxx-ification.