[PATCH] backends: Use PTRACE_GETREGSET for ppc_set_initial_registers_tid

[PATCH] backends: Use PTRACE_GETREGSET for ppc_set_initial_registers_tid

[Mark Wielaard]

The code in ppc_initreg.c used PTRACE_PEEKUSER to fetch all registers
one by one. Which is slightly inefficient. It did this because it wanted
things to work on linux 2.6.18 which didn't support PTRACE_GETREGSET.

PTRACE_GETREGSET was only officially since 2.6.34 (but backported
to some earlier versions). It seems ok to require a linux kernel that
supports PTRACE_GETREGSET now. This is much more efficient since it
takes just one ptrace call instead of 44 calls to fetch each register
individually.

For some really old versions we need to include <linux/ptrace.h> to
get PTRACE_GETREGSET defined. And on ppc64 there is no 32bit version
of struct pt_regs available, so we define that ourselves and check
how much data is returned to know whether this is a full pt_regs or
one for a 32bit process. An alternative would be to use the raw
iov_base bytes with 64bit or 32bit offset constants to get at the
registers instead of using a struct with names.

The code works for inspecting a 32bit process from a 64bit build,
but not the other way around (the previous code also didn't). This
could work if we also defined and used a 64bit pt_regs struct on
ppc32. But it seems a use case that is not really used (it was hard
enough finding ppc32 setups to test this on).

Tested against ppc and ppc64 on linux 2.6.32 and glibc 2.12 and
ppc and ppc64 on linux 3.10.0 with glibc 2.17.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 backends/ChangeLog     |  5 ++++
 backends/ppc_initreg.c | 66 ++++++++++++++++++++++++++----------------
 2 files changed, 46 insertions(+), 25 deletions(-)

https://code.wildebeest.org/git/user/mjw/elfutils/commit/?h=ppc-ptrace-getregset

diff --git a/backends/ChangeLog b/backends/ChangeLog
index b48af4e1..51959259 100644
--- a/backends/ChangeLog
+++ b/backends/ChangeLog
@@ -1,3 +1,8 @@
+2022-02-16  Mark Wielaard  <mark@klomp.org>
+
+	* ppc_initreg.c (ppc_set_initial_registers_tid): Define struct
+	pt_regs32. Use PTRACE_GETREGSET.
+
 2021-09-29  William Cohen  <wcohen@redhat.com>
 
 	* riscv_init.c (riscv_return_value_location_lp64f): New function
diff --git a/backends/ppc_initreg.c b/backends/ppc_initreg.c
index e5cca7e1..8ed72fb4 100644
--- a/backends/ppc_initreg.c
+++ b/backends/ppc_initreg.c
@@ -1,5 +1,6 @@
 /* Fetch live process registers from TID.
    Copyright (C) 2013 Red Hat, Inc.
+   Copyright (C) 2022 Mark J. Wielaard <mark@klomp.org>
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -34,7 +35,11 @@
 #if defined(__powerpc__) && defined(__linux__)
 # include <sys/ptrace.h>
 # include <asm/ptrace.h>
+# ifndef PTRACE_GETREGSET
+#  include <linux/ptrace.h>
+# endif
 # include <sys/user.h>
+# include <sys/uio.h>
 #endif
 
 #include "system.h"
@@ -75,39 +80,50 @@ ppc_set_initial_registers_tid (pid_t tid __attribute__ ((unused)),
 #if !defined(__powerpc__) || !defined(__linux__)
   return false;
 #else /* __powerpc__ */
-  union
-    {
-      struct pt_regs r;
-      long l[sizeof (struct pt_regs) / sizeof (long)];
-    }
-  user_regs;
-  eu_static_assert (sizeof (struct pt_regs) % sizeof (long) == 0);
-  /* PTRACE_GETREGS is EIO on kernel-2.6.18-308.el5.ppc64.  */
-  errno = 0;
-  for (unsigned regno = 0; regno < sizeof (user_regs) / sizeof (long);
-       regno++)
-    {
-      user_regs.l[regno] = ptrace (PTRACE_PEEKUSER, tid,
-				   (void *) (uintptr_t) (regno
-							 * sizeof (long)),
-				   NULL);
-      if (errno != 0)
-	return false;
-    }
-#define GPRS (sizeof (user_regs.r.gpr) / sizeof (*user_regs.r.gpr))
+
+/* pt_regs for 32bit processes. Same as 64bit pt_regs but all registers
+   are 32bit instead of 64bit long.  */
+#define GPRS 32
+  struct pt_regs32
+  {
+    uint32_t gpr[GPRS];
+    uint32_t nip;
+    uint32_t msr;
+    uint32_t orig_gpr3;
+    uint32_t ctr;
+    uint32_t link;
+    uint32_t xer;
+    uint32_t ccr;
+    uint32_t mq;
+    uint32_t trap;
+    uint32_t dar;
+    uint32_t dsisr;
+    uint32_t result;
+  };
+
+  struct pt_regs regs;
+  struct pt_regs32 *regs32 = (struct pt_regs32 *) &regs;
+  struct iovec iovec;
+  iovec.iov_base = &regs;
+  iovec.iov_len = sizeof (regs);
+  if (ptrace (PTRACE_GETREGSET, tid, NT_PRSTATUS, &iovec) != 0)
+    return false;
+
+  /* Did we get the full pt_regs or less (the 32bit pt_regs)?  */
+  bool get32 = iovec.iov_len < sizeof (struct pt_regs);
   Dwarf_Word dwarf_regs[GPRS];
   for (unsigned gpr = 0; gpr < GPRS; gpr++)
-    dwarf_regs[gpr] = user_regs.r.gpr[gpr];
+    dwarf_regs[gpr] = get32 ? regs32->gpr[gpr] : regs.gpr[gpr];
   if (! setfunc (0, GPRS, dwarf_regs, arg))
     return false;
-  dwarf_regs[0] = user_regs.r.link;
   // LR uses both 65 and 108 numbers, there is no consistency for it.
-  if (! setfunc (65, 1, dwarf_regs, arg))
+  Dwarf_Word link = get32 ? regs32->link : regs.link;
+  if (! setfunc (65, 1, &link, arg))
     return false;
   /* Registers like msr, ctr, xer, dar, dsisr etc. are probably irrelevant
      for CFI.  */
-  dwarf_regs[0] = user_regs.r.nip;
-  return setfunc (-1, 1, dwarf_regs, arg);
+  Dwarf_Word pc = get32 ? (Dwarf_Word) regs32->nip : regs.nip;
+  return setfunc (-1, 1, &pc, arg);
 #endif /* __powerpc__ */
 }
 
-- 
2.18.4

Re: [PATCH] backends: Use PTRACE_GETREGSET for ppc_set_initial_registers_tid

[Mark Wielaard]

Hi,

On Wed, 2022-02-16 at 15:07 +0100, Mark Wielaard wrote:
> The code in ppc_initreg.c used PTRACE_PEEKUSER to fetch all registers
> one by one. Which is slightly inefficient. It did this because it
> wanted
> things to work on linux 2.6.18 which didn't support PTRACE_GETREGSET.
> 
> PTRACE_GETREGSET was only officially since 2.6.34 (but backported
> to some earlier versions). It seems ok to require a linux kernel that
> supports PTRACE_GETREGSET now. This is much more efficient since it
> takes just one ptrace call instead of 44 calls to fetch each register
> individually.
> 
> For some really old versions we need to include <linux/ptrace.h> to
> get PTRACE_GETREGSET defined. And on ppc64 there is no 32bit version
> of struct pt_regs available, so we define that ourselves and check
> how much data is returned to know whether this is a full pt_regs or
> one for a 32bit process. An alternative would be to use the raw
> iov_base bytes with 64bit or 32bit offset constants to get at the
> registers instead of using a struct with names.
> 
> The code works for inspecting a 32bit process from a 64bit build,
> but not the other way around (the previous code also didn't). This
> could work if we also defined and used a 64bit pt_regs struct on
> ppc32. But it seems a use case that is not really used (it was hard
> enough finding ppc32 setups to test this on).
> 
> Tested against ppc and ppc64 on linux 2.6.32 and glibc 2.12 and
> ppc and ppc64 on linux 3.10.0 with glibc 2.17.

Pushed.

Cheers,

Mark