[PATCH 0/6] Use PTRACE_GETREGSET and PTRACE_SETREGSET in arm-linux-nat.c

[PATCH 0/6] Use PTRACE_GETREGSET and PTRACE_SETREGSET in arm-linux-nat.c

[Yao Qi]

This patch series is to let GDB arm-linux-nat.c use ptrace commands
PTRACE_GETREGSET and PTRACE_SETREGSET if linux kernel supports.

Patch #1 and #2 are factoring out macros and variables into more
common files.  Patch #3 checks whether PTRACE_GETREGSET is supported by
linux kernel.  Patch #4 - #6 use PTRACE_{G,S}ETREGSET ptrace commands
to fetch and store general purpose registers, FP registers and VFP
registers.

The whole test series are tested on x86_64-linux and arm-linux (with
PTRACE_GETREGSET and without PTRACE_GETREGSET kernel support respectively).

After these patches, arm-linux-nat.c is similar to aarch64-linux-nat.c
in the aspect of fetching and storing registers.  It paves the way for
multi-arch support in aarch64 gdb (aarch64 gdb can do native debugging
for arm program), which is the motivation of this patch series.

*** BLURB HERE ***

Yao Qi (6):
  Move PTRACE_GETREGSET and PTRACE_SETREGSET to nat/linux-ptrace.h
  Move have_ptrace_getregset to linux-nat.c
  Check whether kernel supports PTRACE_GETREGSET
  Fetch and store GP registers by PTRACE_{G,S}ETREGSET
  Fetch and store FP registers by PTRACE_{G,S}ETREGSET
  Fetch and store VFP registers by PTRACE_{G,S}ETREGSET

 gdb/amd64-linux-nat.c          |   1 +
 gdb/arm-linux-nat.c            | 211 +++++++++++++++++++++++++++++++++++++----
 gdb/gdbserver/linux-s390-low.c |   8 --
 gdb/gdbserver/linux-x86-low.c  |   9 --
 gdb/i386-linux-nat.c           |   1 +
 gdb/linux-nat.c                |   3 +
 gdb/linux-nat.h                |   3 +
 gdb/nat/linux-ptrace.h         |   8 ++
 gdb/s390-linux-nat.c           |   9 +-
 gdb/x86-linux-nat.c            |   3 +-
 gdb/x86-linux-nat.h            |  10 --
 11 files changed, 212 insertions(+), 54 deletions(-)

-- 
1.9.1

[PATCH 4/6] Fetch and store GP registers by PTRACE_{G,S}ETREGSET

[Yao Qi]

If kernel supports PTRACE_GETREGSET, GDB uses PTRACE_{G,S}ETREGSET
to fetch and store GP registers.

gdb:

2015-05-28  Yao Qi  <yao.qi@linaro.org>

	* arm-linux-nat.c (fetch_register): Use PTRACE_GETREGSET.
	(fetch_regs): Likewise.
	(store_regs): Use PTRACE_SETREGSET.
	(store_register): Likewise.
---
 gdb/arm-linux-nat.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 72 insertions(+), 7 deletions(-)

diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
index 877559e..0a86ed6 100644
--- a/gdb/arm-linux-nat.c
+++ b/gdb/arm-linux-nat.c
@@ -225,7 +225,18 @@ fetch_register (struct regcache *regcache, int regno)
   /* Get the thread id for the ptrace call.  */
   tid = GET_THREAD_ID (inferior_ptid);
   
-  ret = ptrace (PTRACE_GETREGS, tid, 0, &regs);
+  if (have_ptrace_getregset == 1)
+    {
+      struct iovec iov;
+
+      iov.iov_base = &regs;
+      iov.iov_len = sizeof (regs);
+
+      ret = ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS, &iov);
+    }
+  else
+    ret = ptrace (PTRACE_GETREGS, tid, 0, &regs);
+
   if (ret < 0)
     {
       warning (_("Unable to fetch general register."));
@@ -266,8 +277,19 @@ fetch_regs (struct regcache *regcache)
 
   /* Get the thread id for the ptrace call.  */
   tid = GET_THREAD_ID (inferior_ptid);
-  
-  ret = ptrace (PTRACE_GETREGS, tid, 0, &regs);
+
+  if (have_ptrace_getregset == 1)
+    {
+      struct iovec iov;
+
+      iov.iov_base = &regs;
+      iov.iov_len = sizeof (regs);
+
+      ret = ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS, &iov);
+    }
+  else
+    ret = ptrace (PTRACE_GETREGS, tid, 0, &regs);
+
   if (ret < 0)
     {
       warning (_("Unable to fetch general registers."));
@@ -306,7 +328,18 @@ store_register (const struct regcache *regcache, int regno)
   tid = GET_THREAD_ID (inferior_ptid);
   
   /* Get the general registers from the process.  */
-  ret = ptrace (PTRACE_GETREGS, tid, 0, &regs);
+  if (have_ptrace_getregset == 1)
+    {
+      struct iovec iov;
+
+      iov.iov_base = &regs;
+      iov.iov_len = sizeof (regs);
+
+      ret = ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS, &iov);
+    }
+  else
+    ret = ptrace (PTRACE_GETREGS, tid, 0, &regs);
+
   if (ret < 0)
     {
       warning (_("Unable to fetch general registers."));
@@ -322,7 +355,18 @@ store_register (const struct regcache *regcache, int regno)
     regcache_raw_collect (regcache, ARM_PC_REGNUM,
 			 (char *) &regs[ARM_PC_REGNUM]);
 
-  ret = ptrace (PTRACE_SETREGS, tid, 0, &regs);
+  if (have_ptrace_getregset == 1)
+    {
+      struct iovec iov;
+
+      iov.iov_base = &regs;
+      iov.iov_len = sizeof (regs);
+
+      ret = ptrace (PTRACE_SETREGSET, tid, NT_PRSTATUS, &iov);
+    }
+  else
+    ret = ptrace (PTRACE_SETREGS, tid, 0, &regs);
+
   if (ret < 0)
     {
       warning (_("Unable to store general register."));
@@ -340,7 +384,18 @@ store_regs (const struct regcache *regcache)
   tid = GET_THREAD_ID (inferior_ptid);
   
   /* Fetch the general registers.  */
-  ret = ptrace (PTRACE_GETREGS, tid, 0, &regs);
+  if (have_ptrace_getregset == 1)
+    {
+      struct iovec iov;
+
+      iov.iov_base = &regs;
+      iov.iov_len = sizeof (regs);
+
+      ret = ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS, &iov);
+    }
+  else
+    ret = ptrace (PTRACE_GETREGS, tid, 0, &regs);
+
   if (ret < 0)
     {
       warning (_("Unable to fetch general registers."));
@@ -357,7 +412,17 @@ store_regs (const struct regcache *regcache)
     regcache_raw_collect (regcache, ARM_PS_REGNUM,
 			 (char *) &regs[ARM_CPSR_GREGNUM]);
 
-  ret = ptrace (PTRACE_SETREGS, tid, 0, &regs);
+  if (have_ptrace_getregset == 1)
+    {
+      struct iovec iov;
+
+      iov.iov_base = &regs;
+      iov.iov_len = sizeof (regs);
+
+      ret = ptrace (PTRACE_SETREGSET, tid, NT_PRSTATUS, &iov);
+    }
+  else
+    ret = ptrace (PTRACE_SETREGS, tid, 0, &regs);
 
   if (ret < 0)
     {
-- 
1.9.1

Re: [PATCH 4/6] Fetch and store GP registers by PTRACE_{G,S}ETREGSET

[Doug Evans]

On Thu, May 28, 2015 at 7:20 AM, Yao Qi <qiyaoltc@gmail.com> wrote:
> If kernel supports PTRACE_GETREGSET, GDB uses PTRACE_{G,S}ETREGSET
> to fetch and store GP registers.
>
> gdb:
>
> 2015-05-28  Yao Qi  <yao.qi@linaro.org>
>
>         * arm-linux-nat.c (fetch_register): Use PTRACE_GETREGSET.
>         (fetch_regs): Likewise.
>         (store_regs): Use PTRACE_SETREGSET.
>         (store_register): Likewise.
> ---
>  gdb/arm-linux-nat.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 72 insertions(+), 7 deletions(-)
>
> diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
> index 877559e..0a86ed6 100644
> --- a/gdb/arm-linux-nat.c
> +++ b/gdb/arm-linux-nat.c
> @@ -225,7 +225,18 @@ fetch_register (struct regcache *regcache, int regno)
>    /* Get the thread id for the ptrace call.  */
>    tid = GET_THREAD_ID (inferior_ptid);
>
> -  ret = ptrace (PTRACE_GETREGS, tid, 0, &regs);
> +  if (have_ptrace_getregset == 1)

Hi.

The == 1 in this test hinders readability (to me anyway).
[It occurs here and in 5/6, 6/6.]
The name suggests the variable is a boolean, so I'm
left wondering "Can it have values other than 0/1,
and is the else clause correct for those other values?"

Digging deeper the reader would find the variable is tri-state,
but the initial -1 value should never be seen here (at least
that's the intuitive choice).

If one wanted to add an assert that the value is not -1 here
that'd be ok, though one could also argue it's overkill.
I don't have a preference either way.

But I suggest removing the "== 1" in the test.

> +    {
> +      struct iovec iov;
> +
> +      iov.iov_base = &regs;
> +      iov.iov_len = sizeof (regs);
> +
> +      ret = ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS, &iov);
> +    }
> +  else
> +    ret = ptrace (PTRACE_GETREGS, tid, 0, &regs);
> +
>    if (ret < 0)
>      {
>        warning (_("Unable to fetch general register."));
> @@ -266,8 +277,19 @@ fetch_regs (struct regcache *regcache)
>
>    /* Get the thread id for the ptrace call.  */
>    tid = GET_THREAD_ID (inferior_ptid);
> -
> -  ret = ptrace (PTRACE_GETREGS, tid, 0, &regs);
> +
> +  if (have_ptrace_getregset == 1)
> +    {
> +      struct iovec iov;
> +
> +      iov.iov_base = &regs;
> +      iov.iov_len = sizeof (regs);
> +
> +      ret = ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS, &iov);
> +    }
> +  else
> +    ret = ptrace (PTRACE_GETREGS, tid, 0, &regs);
> +
>    if (ret < 0)
>      {
>        warning (_("Unable to fetch general registers."));
> @@ -306,7 +328,18 @@ store_register (const struct regcache *regcache, int regno)
>    tid = GET_THREAD_ID (inferior_ptid);
>
>    /* Get the general registers from the process.  */
> -  ret = ptrace (PTRACE_GETREGS, tid, 0, &regs);
> +  if (have_ptrace_getregset == 1)
> +    {
> +      struct iovec iov;
> +
> +      iov.iov_base = &regs;
> +      iov.iov_len = sizeof (regs);
> +
> +      ret = ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS, &iov);
> +    }
> +  else
> +    ret = ptrace (PTRACE_GETREGS, tid, 0, &regs);
> +
>    if (ret < 0)
>      {
>        warning (_("Unable to fetch general registers."));
> @@ -322,7 +355,18 @@ store_register (const struct regcache *regcache, int regno)
>      regcache_raw_collect (regcache, ARM_PC_REGNUM,
>                          (char *) &regs[ARM_PC_REGNUM]);
>
> -  ret = ptrace (PTRACE_SETREGS, tid, 0, &regs);
> +  if (have_ptrace_getregset == 1)
> +    {
> +      struct iovec iov;
> +
> +      iov.iov_base = &regs;
> +      iov.iov_len = sizeof (regs);
> +
> +      ret = ptrace (PTRACE_SETREGSET, tid, NT_PRSTATUS, &iov);
> +    }
> +  else
> +    ret = ptrace (PTRACE_SETREGS, tid, 0, &regs);
> +
>    if (ret < 0)
>      {
>        warning (_("Unable to store general register."));
> @@ -340,7 +384,18 @@ store_regs (const struct regcache *regcache)
>    tid = GET_THREAD_ID (inferior_ptid);
>
>    /* Fetch the general registers.  */
> -  ret = ptrace (PTRACE_GETREGS, tid, 0, &regs);
> +  if (have_ptrace_getregset == 1)
> +    {
> +      struct iovec iov;
> +
> +      iov.iov_base = &regs;
> +      iov.iov_len = sizeof (regs);
> +
> +      ret = ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS, &iov);
> +    }
> +  else
> +    ret = ptrace (PTRACE_GETREGS, tid, 0, &regs);
> +
>    if (ret < 0)
>      {
>        warning (_("Unable to fetch general registers."));
> @@ -357,7 +412,17 @@ store_regs (const struct regcache *regcache)
>      regcache_raw_collect (regcache, ARM_PS_REGNUM,
>                          (char *) &regs[ARM_CPSR_GREGNUM]);
>
> -  ret = ptrace (PTRACE_SETREGS, tid, 0, &regs);
> +  if (have_ptrace_getregset == 1)
> +    {
> +      struct iovec iov;
> +
> +      iov.iov_base = &regs;
> +      iov.iov_len = sizeof (regs);
> +
> +      ret = ptrace (PTRACE_SETREGSET, tid, NT_PRSTATUS, &iov);
> +    }
> +  else
> +    ret = ptrace (PTRACE_SETREGS, tid, 0, &regs);
>
>    if (ret < 0)
>      {
> --
> 1.9.1
>

Re: [PATCH 4/6] Fetch and store GP registers by PTRACE_{G,S}ETREGSET

[Yao Qi]

Doug Evans <dje@google.com> writes:

> The == 1 in this test hinders readability (to me anyway).
> [It occurs here and in 5/6, 6/6.]
> The name suggests the variable is a boolean, so I'm
> left wondering "Can it have values other than 0/1,
> and is the else clause correct for those other values?"
>
> Digging deeper the reader would find the variable is tri-state,
> but the initial -1 value should never be seen here (at least
> that's the intuitive choice).

Yes, this variable have three states, uninitialised (-1), true (1)
and false (0) and that is reason I check "have_ptrace_getregset == 1"
instead of "have_ptrace_getregset".

>
> If one wanted to add an assert that the value is not -1 here
> that'd be ok, though one could also argue it's overkill.
> I don't have a preference either way.
>
> But I suggest removing the "== 1" in the test.

I am OK to remove "== 1" from patches #4, #5 and #6.

-- 
Yao (齐尧)

[PATCH 1/6] Move PTRACE_GETREGSET and PTRACE_SETREGSET to nat/linux-ptrace.h

[Yao Qi]

Macros PTRACE_GETREGSET and PTRACE_SETREGSET are defined locally in
some places in GDB and GDBserver.  This patch is to move them to
nat/linux-ptrace.h to avoid duplication.

gdb:

2015-05-28  Yao Qi  <yao.qi@linaro.org>

	* amd64-linux-nat.c: Include "nat/linux-ptrace.h".
	* i386-linux-nat.c: Likewise.
	* nat/linux-ptrace.h (PTRACE_GETREGSET, PTRACE_SETREGSET): Define.
	* s390-linux-nat.c: Include "nat/linux-ptrace.h".
	(PTRACE_GETREGSET, PTRACE_SETREGSET): Remove.
	* x86-linux-nat.c: Include "nat/linux-ptrace.h".
	* x86-linux-nat.h (PTRACE_GETREGSET, PTRACE_SETREGSET): Remove.

gdb/gdbserver:

2015-05-28  Yao Qi  <yao.qi@linaro.org>

	* linux-s390-low.c (PTRACE_GETREGSET, PTRACE_SETREGSET): Remove.
	* linux-x86-low.c: Likewise.
---
 gdb/amd64-linux-nat.c          | 1 +
 gdb/gdbserver/linux-s390-low.c | 8 --------
 gdb/gdbserver/linux-x86-low.c  | 9 ---------
 gdb/i386-linux-nat.c           | 1 +
 gdb/nat/linux-ptrace.h         | 8 ++++++++
 gdb/s390-linux-nat.c           | 9 +--------
 gdb/x86-linux-nat.c            | 1 +
 gdb/x86-linux-nat.h            | 8 --------
 8 files changed, 12 insertions(+), 33 deletions(-)

diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index 22a1359..30892ca 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -37,6 +37,7 @@
 #include "x86-xstate.h"
 
 #include "x86-linux-nat.h"
+#include "nat/linux-ptrace.h"
 
 /* Mapping between the general-purpose registers in GNU/Linux x86-64
    `struct user' format and GDB's register cache layout for GNU/Linux
diff --git a/gdb/gdbserver/linux-s390-low.c b/gdb/gdbserver/linux-s390-low.c
index afee1ff..5d4ea1f 100644
--- a/gdb/gdbserver/linux-s390-low.c
+++ b/gdb/gdbserver/linux-s390-low.c
@@ -40,14 +40,6 @@
 #define HWCAP_S390_VX 2048
 #endif
 
-#ifndef PTRACE_GETREGSET
-#define PTRACE_GETREGSET 0x4204
-#endif
-
-#ifndef PTRACE_SETREGSET
-#define PTRACE_SETREGSET 0x4205
-#endif
-
 /* Defined in auto-generated file s390-linux32.c.  */
 void init_registers_s390_linux32 (void);
 extern const struct target_desc *tdesc_s390_linux32;
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 4aef7b7..484eadb 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -120,15 +120,6 @@ static const char *xmltarget_amd64_linux_no_xml = "@<target>\
 #include <sys/ptrace.h>
 #include <sys/uio.h>
 
-#ifndef PTRACE_GETREGSET
-#define PTRACE_GETREGSET	0x4204
-#endif
-
-#ifndef PTRACE_SETREGSET
-#define PTRACE_SETREGSET	0x4205
-#endif
-
-
 #ifndef PTRACE_GET_THREAD_AREA
 #define PTRACE_GET_THREAD_AREA 25
 #endif
diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
index 8cb8c66..5666361 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -34,6 +34,7 @@
 #include "x86-xstate.h"
 
 #include "x86-linux-nat.h"
+#include "nat/linux-ptrace.h"
 
 /* The register sets used in GNU/Linux ELF core-dumps are identical to
    the register sets in `struct user' that is used for a.out
diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
index 1db0cde..be6c395 100644
--- a/gdb/nat/linux-ptrace.h
+++ b/gdb/nat/linux-ptrace.h
@@ -43,6 +43,14 @@ struct buffer;
 # define PTRACE_SETSIGINFO 0x4203
 #endif /* PTRACE_GETSIGINF */
 
+#ifndef PTRACE_GETREGSET
+#define PTRACE_GETREGSET	0x4204
+#endif
+
+#ifndef PTRACE_SETREGSET
+#define PTRACE_SETREGSET	0x4205
+#endif
+
 /* If the system headers did not provide the constants, hard-code the normal
    values.  */
 #ifndef PTRACE_EVENT_FORK
diff --git a/gdb/s390-linux-nat.c b/gdb/s390-linux-nat.c
index 4cd3192..cedc505 100644
--- a/gdb/s390-linux-nat.c
+++ b/gdb/s390-linux-nat.c
@@ -27,6 +27,7 @@
 #include "auxv.h"
 #include "gregset.h"
 #include "regset.h"
+#include "nat/linux-ptrace.h"
 
 #include "s390-linux-tdep.h"
 #include "elf/common.h"
@@ -38,14 +39,6 @@
 #include <sys/ucontext.h>
 #include <elf.h>
 
-#ifndef PTRACE_GETREGSET
-#define PTRACE_GETREGSET 0x4204
-#endif
-
-#ifndef PTRACE_SETREGSET
-#define PTRACE_SETREGSET 0x4205
-#endif
-
 /* Per-thread arch-specific data.  */
 
 struct arch_lwp_info
diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
index f8eb236..0026d26 100644
--- a/gdb/x86-linux-nat.c
+++ b/gdb/x86-linux-nat.c
@@ -41,6 +41,7 @@
 #include "nat/linux-nat.h"
 #include "nat/x86-linux.h"
 #include "nat/x86-linux-dregs.h"
+#include "nat/linux-ptrace.h"
 
 /* Per-thread arch-specific data we want to keep.  */
 
diff --git a/gdb/x86-linux-nat.h b/gdb/x86-linux-nat.h
index 7928aa0..7c4d87c 100644
--- a/gdb/x86-linux-nat.h
+++ b/gdb/x86-linux-nat.h
@@ -20,14 +20,6 @@
 #ifndef X86_LINUX_NAT_H
 #define X86_LINUX_NAT_H 1
 
-#ifndef PTRACE_GETREGSET
-#define PTRACE_GETREGSET	0x4204
-#endif
-
-#ifndef PTRACE_SETREGSET
-#define PTRACE_SETREGSET	0x4205
-#endif
-
 /* Does the current host support PTRACE_GETREGSET?  */
 extern int have_ptrace_getregset;
 \f
-- 
1.9.1

[PATCH 5/6] Fetch and store FP registers by PTRACE_{G,S}ETREGSET

[Yao Qi]

If kernel supports PTRACE_GETREGSET, GDB uses PTRACE_{G,S}ETREGSET
to fetch and store FP registers.

gdb:

2015-05-28  Yao Qi  <yao.qi@linaro.org>

	* arm-linux-nat.c (fetch_fpregister): Use PTRACE_GETREGSET.
	(fetch_fpregs): Likewise.
	* arm-linux-nat.c (store_fpregister): Use PTRACE_SETREGSET.
	(store_fpregs): Likewise.
---
 gdb/arm-linux-nat.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 74 insertions(+), 7 deletions(-)

diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
index 0a86ed6..b74767c 100644
--- a/gdb/arm-linux-nat.c
+++ b/gdb/arm-linux-nat.c
@@ -88,12 +88,23 @@ fetch_fpregister (struct regcache *regcache, int regno)
 {
   int ret, tid;
   gdb_byte fp[ARM_LINUX_SIZEOF_NWFPE];
-  
+
   /* Get the thread id for the ptrace call.  */
   tid = GET_THREAD_ID (inferior_ptid);
 
   /* Read the floating point state.  */
-  ret = ptrace (PT_GETFPREGS, tid, 0, fp);
+  if (have_ptrace_getregset == 1)
+    {
+      struct iovec iov;
+
+      iov.iov_base = &fp;
+      iov.iov_len = ARM_LINUX_SIZEOF_NWFPE;
+
+      ret = ptrace (PTRACE_GETREGSET, tid, NT_FPREGSET, &iov);
+    }
+  else
+    ret = ptrace (PT_GETFPREGS, tid, 0, fp);
+
   if (ret < 0)
     {
       warning (_("Unable to fetch floating point register."));
@@ -123,7 +134,18 @@ fetch_fpregs (struct regcache *regcache)
   tid = GET_THREAD_ID (inferior_ptid);
   
   /* Read the floating point state.  */
-  ret = ptrace (PT_GETFPREGS, tid, 0, fp);
+  if (have_ptrace_getregset == 1)
+    {
+      struct iovec iov;
+
+      iov.iov_base = &fp;
+      iov.iov_len = ARM_LINUX_SIZEOF_NWFPE;
+
+      ret = ptrace (PTRACE_GETREGSET, tid, NT_FPREGSET, &iov);
+    }
+  else
+    ret = ptrace (PT_GETFPREGS, tid, 0, fp);
+
   if (ret < 0)
     {
       warning (_("Unable to fetch the floating point registers."));
@@ -152,7 +174,18 @@ store_fpregister (const struct regcache *regcache, int regno)
   tid = GET_THREAD_ID (inferior_ptid);
   
   /* Read the floating point state.  */
-  ret = ptrace (PT_GETFPREGS, tid, 0, fp);
+  if (have_ptrace_getregset == 1)
+    {
+      struct iovec iov;
+
+      iov.iov_base = &fp;
+      iov.iov_len = ARM_LINUX_SIZEOF_NWFPE;
+
+      ret = ptrace (PTRACE_GETREGSET, tid, NT_FPREGSET, &iov);
+    }
+  else
+    ret = ptrace (PT_GETFPREGS, tid, 0, fp);
+
   if (ret < 0)
     {
       warning (_("Unable to fetch the floating point registers."));
@@ -168,7 +201,18 @@ store_fpregister (const struct regcache *regcache, int regno)
   if (regno >= ARM_F0_REGNUM && regno <= ARM_F7_REGNUM)
     collect_nwfpe_register (regcache, regno, fp);
 
-  ret = ptrace (PTRACE_SETFPREGS, tid, 0, fp);
+  if (have_ptrace_getregset == 1)
+    {
+      struct iovec iov;
+
+      iov.iov_base = &fp;
+      iov.iov_len = ARM_LINUX_SIZEOF_NWFPE;
+
+      ret = ptrace (PTRACE_SETREGSET, tid, NT_FPREGSET, &iov);
+    }
+  else
+    ret = ptrace (PTRACE_SETFPREGS, tid, 0, fp);
+
   if (ret < 0)
     {
       warning (_("Unable to store floating point register."));
@@ -189,7 +233,19 @@ store_fpregs (const struct regcache *regcache)
   tid = GET_THREAD_ID (inferior_ptid);
   
   /* Read the floating point state.  */
-  ret = ptrace (PT_GETFPREGS, tid, 0, fp);
+  if (have_ptrace_getregset == 1)
+    {
+      elf_fpregset_t fpregs;
+      struct iovec iov;
+
+      iov.iov_base = &fpregs;
+      iov.iov_len = sizeof (fpregs);
+
+      ret = ptrace (PTRACE_GETREGSET, tid, NT_FPREGSET, &iov);
+    }
+  else
+    ret = ptrace (PT_GETFPREGS, tid, 0, fp);
+
   if (ret < 0)
     {
       warning (_("Unable to fetch the floating point registers."));
@@ -205,7 +261,18 @@ store_fpregs (const struct regcache *regcache)
     if (REG_VALID == regcache_register_status (regcache, regno))
       collect_nwfpe_register (regcache, regno, fp);
 
-  ret = ptrace (PTRACE_SETFPREGS, tid, 0, fp);
+  if (have_ptrace_getregset == 1)
+    {
+      struct iovec iov;
+
+      iov.iov_base = &fp;
+      iov.iov_len = ARM_LINUX_SIZEOF_NWFPE;
+
+      ret = ptrace (PTRACE_SETREGSET, tid, NT_FPREGSET, &iov);
+    }
+  else
+    ret = ptrace (PTRACE_SETFPREGS, tid, 0, fp);
+
   if (ret < 0)
     {
       warning (_("Unable to store floating point registers."));
-- 
1.9.1

[PATCH 2/6] Move have_ptrace_getregset to linux-nat.c

[Yao Qi]

I'll let arm-linux-nat.c to use PTRACE_GETREGSET if kernel supports,
so this patch is to move have_ptrace_getregset from x86-linux-nat.c
to linux-nat.c.

gdb:

2015-05-28  Yao Qi  <yao.qi@linaro.org>

	* x86-linux-nat.c (have_ptrace_getregset): Move it to ...
	* linux-nat.c: ... here.
	* x86-linux-nat.h (have_ptrace_getregset): Move the declaration
	to ...
	* linux-nat.h: ... here.
---
 gdb/linux-nat.c     | 3 +++
 gdb/linux-nat.h     | 3 +++
 gdb/x86-linux-nat.c | 2 --
 gdb/x86-linux-nat.h | 2 --
 4 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index f95e76c..febee84 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -163,6 +163,9 @@ blocked.  */
 #define O_LARGEFILE 0
 #endif
 
+/* Does the current host support PTRACE_GETREGSET?  */
+int have_ptrace_getregset = -1;
+
 /* The single-threaded native GNU/Linux target_ops.  We save a pointer for
    the use of the multi-threaded target.  */
 static struct target_ops *linux_ops;
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index d935270..3d766b5 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -115,6 +115,9 @@ struct lwp_info
    native target is active.  */
 extern struct lwp_info *lwp_list;
 
+/* Does the current host support PTRACE_GETREGSET?  */
+extern int have_ptrace_getregset;
+
 /* Iterate over each active thread (light-weight process).  */
 #define ALL_LWPS(LP)							\
   for ((LP) = lwp_list;							\
diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
index 0026d26..5d8f7c7 100644
--- a/gdb/x86-linux-nat.c
+++ b/gdb/x86-linux-nat.c
@@ -51,8 +51,6 @@ struct arch_lwp_info
   int debug_registers_changed;
 };
 
-/* Does the current host support PTRACE_GETREGSET?  */
-int have_ptrace_getregset = -1;
 \f
 
 /* linux_nat_new_fork hook.   */
diff --git a/gdb/x86-linux-nat.h b/gdb/x86-linux-nat.h
index 7c4d87c..70c207a 100644
--- a/gdb/x86-linux-nat.h
+++ b/gdb/x86-linux-nat.h
@@ -20,8 +20,6 @@
 #ifndef X86_LINUX_NAT_H
 #define X86_LINUX_NAT_H 1
 
-/* Does the current host support PTRACE_GETREGSET?  */
-extern int have_ptrace_getregset;
 \f
 
 /* Helper for ps_get_thread_area.  Sets BASE_ADDR to a pointer to
-- 
1.9.1

[PATCH 3/6] Check whether kernel supports PTRACE_GETREGSET

[Yao Qi]

gdb:

2015-05-28  Yao Qi  <yao.qi@linaro.org>

	* arm-linux-nat.c (arm_linux_read_description): Check whether
	kernel supports PTRACE_GETREGSET.
---
 gdb/arm-linux-nat.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
index 7352841..877559e 100644
--- a/gdb/arm-linux-nat.c
+++ b/gdb/arm-linux-nat.c
@@ -634,6 +634,22 @@ arm_linux_read_description (struct target_ops *ops)
 {
   CORE_ADDR arm_hwcap = 0;
 
+  if (have_ptrace_getregset == -1)
+    {
+      elf_gregset_t gpregs;
+      struct iovec iov;
+      int tid = GET_THREAD_ID (inferior_ptid);
+
+      iov.iov_base = &gpregs;
+      iov.iov_len = sizeof (gpregs);
+
+      /* Check if PTRACE_GETREGSET works.  */
+      if (ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS, &iov) < 0)
+	have_ptrace_getregset = 0;
+      else
+	have_ptrace_getregset = 1;
+    }
+
   if (target_auxv_search (ops, AT_HWCAP, &arm_hwcap) != 1)
     {
       return ops->beneath->to_read_description (ops->beneath);
-- 
1.9.1

[PATCH 6/6] Fetch and store VFP registers by PTRACE_{G,S}ETREGSET

[Yao Qi]

This patch is to use PTRACE_{G,S}ETREGSET to fetch and store VFP
registers if kernel supports.

gdb:

2015-05-28  Yao Qi  <yao.qi@linaro.org>

	* arm-linux-nat.c (fetch_vfp_regs): Use PTRACE_GETREGSET.
	(store_vfp_regs): Use PTRACE_SETREGSET.
---
 gdb/arm-linux-nat.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
index b74767c..dd24ef6 100644
--- a/gdb/arm-linux-nat.c
+++ b/gdb/arm-linux-nat.c
@@ -591,7 +591,17 @@ fetch_vfp_regs (struct regcache *regcache)
   /* Get the thread id for the ptrace call.  */
   tid = GET_THREAD_ID (inferior_ptid);
 
-  ret = ptrace (PTRACE_GETVFPREGS, tid, 0, regbuf);
+  if (have_ptrace_getregset == 1)
+    {
+      struct iovec iov;
+
+      iov.iov_base = regbuf;
+      iov.iov_len = VFP_REGS_SIZE;
+      ret = ptrace (PTRACE_GETREGSET, tid, NT_ARM_VFP, &iov);
+    }
+  else
+    ret = ptrace (PTRACE_GETVFPREGS, tid, 0, regbuf);
+
   if (ret < 0)
     {
       warning (_("Unable to fetch VFP registers."));
@@ -617,7 +627,17 @@ store_vfp_regs (const struct regcache *regcache)
   /* Get the thread id for the ptrace call.  */
   tid = GET_THREAD_ID (inferior_ptid);
 
-  ret = ptrace (PTRACE_GETVFPREGS, tid, 0, regbuf);
+  if (have_ptrace_getregset == 1)
+    {
+      struct iovec iov;
+
+      iov.iov_base = regbuf;
+      iov.iov_len = VFP_REGS_SIZE;
+      ret = ptrace (PTRACE_GETREGSET, tid, NT_ARM_VFP, &iov);
+    }
+  else
+    ret = ptrace (PTRACE_GETVFPREGS, tid, 0, regbuf);
+
   if (ret < 0)
     {
       warning (_("Unable to fetch VFP registers (for update)."));
@@ -631,7 +651,16 @@ store_vfp_regs (const struct regcache *regcache)
   regcache_raw_collect (regcache, ARM_FPSCR_REGNUM,
 			(char *) regbuf + 32 * 8);
 
-  ret = ptrace (PTRACE_SETVFPREGS, tid, 0, regbuf);
+  if (have_ptrace_getregset == 1)
+    {
+      struct iovec iov;
+
+      iov.iov_base = regbuf;
+      iov.iov_len = VFP_REGS_SIZE;
+      ret = ptrace (PTRACE_SETREGSET, tid, NT_ARM_VFP, &iov);
+    }
+  else
+    ret = ptrace (PTRACE_SETVFPREGS, tid, 0, regbuf);
 
   if (ret < 0)
     {
-- 
1.9.1

Re: [PATCH 0/6] Use PTRACE_GETREGSET and PTRACE_SETREGSET in arm-linux-nat.c

[Gary Benson]

Yao Qi wrote:
> Patch #1 and #2 are factoring out macros and variables into more
> common files.  Patch #3 checks whether PTRACE_GETREGSET is supported
> by linux kernel.

This was on my todo list, so, thanks for doing it! :)

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH 0/6] Use PTRACE_GETREGSET and PTRACE_SETREGSET in arm-linux-nat.c

[Yao Qi]

On 28/05/15 15:20, Yao Qi wrote:
> This patch series is to let GDB arm-linux-nat.c use ptrace commands
> PTRACE_GETREGSET and PTRACE_SETREGSET if linux kernel supports.
>
> Patch #1 and #2 are factoring out macros and variables into more
> common files.  Patch #3 checks whether PTRACE_GETREGSET is supported by
> linux kernel.  Patch #4 - #6 use PTRACE_{G,S}ETREGSET ptrace commands
> to fetch and store general purpose registers, FP registers and VFP
> registers.
>
> The whole test series are tested on x86_64-linux and arm-linux (with
> PTRACE_GETREGSET and without PTRACE_GETREGSET kernel support respectively).
>
> After these patches, arm-linux-nat.c is similar to aarch64-linux-nat.c
> in the aspect of fetching and storing registers.  It paves the way for
> multi-arch support in aarch64 gdb (aarch64 gdb can do native debugging
> for arm program), which is the motivation of this patch series.

I removed " == 1" from the condition, as suggested by Doug, and pushed
these patches in.

-- 
Yao (齐尧)

Re: [PATCH 0/6] Use PTRACE_GETREGSET and PTRACE_SETREGSET in arm-linux-nat.c

[Mark Wielaard]

Hi Yao,

On Mon, 2015-06-01 at 12:17 +0100, Yao Qi wrote:
> On 28/05/15 15:20, Yao Qi wrote:
> > This patch series is to let GDB arm-linux-nat.c use ptrace commands
> > PTRACE_GETREGSET and PTRACE_SETREGSET if linux kernel supports.
> >
> > Patch #1 and #2 are factoring out macros and variables into more
> > common files.  Patch #3 checks whether PTRACE_GETREGSET is supported by
> > linux kernel.  Patch #4 - #6 use PTRACE_{G,S}ETREGSET ptrace commands
> > to fetch and store general purpose registers, FP registers and VFP
> > registers.
> >
> > The whole test series are tested on x86_64-linux and arm-linux (with
> > PTRACE_GETREGSET and without PTRACE_GETREGSET kernel support respectively).
> >
> > After these patches, arm-linux-nat.c is similar to aarch64-linux-nat.c
> > in the aspect of fetching and storing registers.  It paves the way for
> > multi-arch support in aarch64 gdb (aarch64 gdb can do native debugging
> > for arm program), which is the motivation of this patch series.
> 
> I removed " == 1" from the condition, as suggested by Doug, and pushed
> these patches in.

I believe this caused a build regression on i386. After this patch
series went in various buildbot i386 buildslaves builds started failing:
http://gdb-build.sergiodj.net/waterfall

../../binutils-gdb/gdb/i386-linux-nat.c: In function ‘fetch_xstateregs’:
../../binutils-gdb/gdb/i386-linux-nat.c:329:8: error: ‘have_ptrace_getregset’ undeclared (first use in this function)
../../binutils-gdb/gdb/i386-linux-nat.c:329:8: note: each undeclared identifier is reported only once for each function it appears in
../../binutils-gdb/gdb/i386-linux-nat.c: In function ‘store_xstateregs’:
../../binutils-gdb/gdb/i386-linux-nat.c:352:8: error: ‘have_ptrace_getregset’ undeclared (first use in this function)
make[2]: *** [i386-linux-nat.o] Error 1

Could you take a look?

Thanks,

Mark

Re: [PATCH 0/6] Use PTRACE_GETREGSET and PTRACE_SETREGSET in arm-linux-nat.c

[Yao Qi]

Mark Wielaard <mjw@redhat.com> writes:

> I believe this caused a build regression on i386. After this patch
> series went in various buildbot i386 buildslaves builds started failing:
> http://gdb-build.sergiodj.net/waterfall
>
> ../../binutils-gdb/gdb/i386-linux-nat.c: In function ‘fetch_xstateregs’:
> ../../binutils-gdb/gdb/i386-linux-nat.c:329:8: error:
> ‘have_ptrace_getregset’ undeclared (first use in this function)
> ../../binutils-gdb/gdb/i386-linux-nat.c:329:8: note: each undeclared
> identifier is reported only once for each function it appears in
> ../../binutils-gdb/gdb/i386-linux-nat.c: In function ‘store_xstateregs’:
> ../../binutils-gdb/gdb/i386-linux-nat.c:352:8: error:
> ‘have_ptrace_getregset’ undeclared (first use in this function)
> make[2]: *** [i386-linux-nat.o] Error 1
>
> Could you take a look?

Oh, I forget rebuilding GDB for i686 target.  This patch fixes the build failure.

-- 
Yao (齐尧)

From: Yao Qi <yao.qi@linaro.org>
Date: Tue, 2 Jun 2015 10:28:03 +0100
Subject: [PATCH] Include linux-nat.h in i386-linux-nat.c

This commit fixes a build failure on i386-linux.

gdb:

2015-06-02  Yao Qi  <yao.qi@linaro.org>

	* i386-linux-nat.c: Include linux-nat.h.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 5867465..eca000f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2015-06-02  Yao Qi  <yao.qi@linaro.org>
+
+	* i386-linux-nat.c: Include linux-nat.h.
+
 2015-06-01  Andreas Schwab  <schwab@linux-m68k.org>
 	    Jan Kratochvil  <jan.kratochvil@redhat.com>
 
diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
index 5666361..4401a2f 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -33,6 +33,7 @@
 #include "i386-linux-tdep.h"
 #include "x86-xstate.h"
 
+#include "linux-nat.h"
 #include "x86-linux-nat.h"
 #include "nat/linux-ptrace.h"