Re: [PATCH] aarch64: use <sys/user.h> defined register structures

[Mark Wielaard <mjw at redhat dot com>]

On Thu, 2014-06-12 at 14:37 +0200, Mark Wielaard wrote:
> On Tue, 2014-06-10 at 09:37 -0400, Kyle McMartin wrote:
> > On Tue, Jun 10, 2014 at 11:51:48AM +0200, Mark Wielaard wrote:
> > > On Mon, 2014-06-09 at 21:06 +0200, Kyle McMartin wrote:
> > > > glibc now supplies these (compatible) structs instead of including the
> > > > kernel's <asm/ptrace.h> header, so let's use them. Annoyingly this will
> > > > cause new elfutils to FTBFS on old glibc, and vice versa, but that seems
> > > > unavoidable in the growth of a new port, and the workaround of checking
> > > > for header defines and defining one to the other seems unpleasant as
> > > > well. Therefore, bite the bullet, and let packaging systems alter their
> > > > build requires accordingly.
> > > 
> > > That is indeed annoying, but using the glibc defined structs seems to be
> > > the right thing to do. Do you know which glibc version introduced them?
> > > 
> > Hrm, looks like it's rawhide churn which has caused this... I guess
> > it'll be in glibc 2.20, but isn't in a released version. I can sit on
> > this and resend this patch when it is, if you'd like?
> 
> I think that would be better. I don't think we'll do the next elfutils
> release before glibc 2.20 is released. But if we do, it would be
> somewhat inconvenient if we relied on an unreleased glibc version.

Meanwhile it became a bit inconvenient because some other distros also
switched to a glibc 2.20-prerelease setup, but stable aarch64 distros
are still on older glibc. I think we just have to add the configure
check. Which I added to your patch.  I tested it against old and new
glibc setups on aarch64. What do you think? Does that look OK?

Thanks,

Mark

>From 5df2dc63e96808afb1602d4338e30dbca560a656 Mon Sep 17 00:00:00 2001
From: Kyle McMartin <kyle@redhat.com>
Date: Mon, 9 Jun 2014 21:06:26 +0200
Subject: [PATCH] aarch64: use <sys/user.h> defined register structures

glibc now supplies these (compatible) structs instead of including the
kernel's <asm/ptrace.h> header, so let's use them. Annoyingly this will
cause new elfutils to FTBFS on old glibc, and vice versa. So include a
new configure check for the new struct names and use the old ones if
they are not avilable.

Signed-off-by: Kyle McMartin <kyle@redhat.com>
Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 ChangeLog                  |    4 ++++
 backends/ChangeLog         |    9 +++++++++
 backends/aarch64_initreg.c |   11 ++++++++---
 backends/arm_initreg.c     |    6 +++++-
 configure.ac               |   13 +++++++++++++
 5 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index b05f5bc..b470b3d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2014-07-18  Mark Wielaard  <mjw@redhat.com>
+
+	* configure.ac (AC_CHECK_TYPE): Test for struct user_regs_struct.
+
 2014-05-26  Mark Wielaard  <mjw@redhat.com>
 
 	* NEWS: New section 0.160. Add unstrip --force.
diff --git a/backends/ChangeLog b/backends/ChangeLog
index d29a80f..a335b20 100644
--- a/backends/ChangeLog
+++ b/backends/ChangeLog
@@ -1,3 +1,12 @@
+2014-07-18  Kyle McMartin  <kyle@redhat.com>
+	    Mark Wielaard  <mjw@redhat.com>
+
+	* aarch64_initreg.c: Check HAVE_SYS_USER_REGS.
+	(aarch64_set_initial_registers_tid): Use user_regs_struct and
+	user_fpsimd_struct.
+	* arm_initreg.c: Check HAVE_SYS_USER_REGS.
+	(arm_set_initial_registers_tid): Use user_regs_struct in compat mode.
+
 2014-07-04  Menanteau Guy  <menantea@linux.vnet.ibm.com>
 	    Mark Wielaard  <mjw@redhat.com>
 
diff --git a/backends/aarch64_initreg.c b/backends/aarch64_initreg.c
index 2492d56..9706205 100644
--- a/backends/aarch64_initreg.c
+++ b/backends/aarch64_initreg.c
@@ -1,5 +1,5 @@
 /* Fetch live process registers from TID.
-   Copyright (C) 2013 Red Hat, Inc.
+   Copyright (C) 2013, 2014 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -36,6 +36,11 @@
 # include <linux/uio.h>
 # include <sys/user.h>
 # include <sys/ptrace.h>
+/* Deal with old glibc defining user_pt_regs instead of user_regs_struct.  */
+# ifndef HAVE_SYS_USER_REGS
+#  define user_regs_struct user_pt_regs
+#  define user_fpsimd_struct user_fpsimd_state
+# endif
 #endif
 
 #define BACKEND aarch64_
@@ -51,7 +56,7 @@ aarch64_set_initial_registers_tid (pid_t tid __attribute__ ((unused)),
 #else /* __aarch64__ */
 
   /* General registers.  */
-  struct user_pt_regs gregs;
+  struct user_regs_struct gregs;
   struct iovec iovec;
   iovec.iov_base = &gregs;
   iovec.iov_len = sizeof (gregs);
@@ -69,7 +74,7 @@ aarch64_set_initial_registers_tid (pid_t tid __attribute__ ((unused)),
   /* ELR cannot be found.  */
 
   /* FP registers (only 64bits are used).  */
-  struct user_fpsimd_state fregs;
+  struct user_fpsimd_struct fregs;
   iovec.iov_base = &fregs;
   iovec.iov_len = sizeof (fregs);
   if (ptrace (PTRACE_GETREGSET, tid, NT_FPREGSET, &iovec) != 0)
diff --git a/backends/arm_initreg.c b/backends/arm_initreg.c
index 5837383..a0a9be9 100644
--- a/backends/arm_initreg.c
+++ b/backends/arm_initreg.c
@@ -40,6 +40,10 @@
 # include <linux/uio.h>
 # include <sys/user.h>
 # include <sys/ptrace.h>
+/* Deal with old glibc defining user_pt_regs instead of user_regs_struct.  */
+# ifndef HAVE_SYS_USER_REGS
+#  define user_regs_struct user_pt_regs
+# endif
 #endif
 
 #define BACKEND arm_
@@ -67,7 +71,7 @@ arm_set_initial_registers_tid (pid_t tid __attribute__ ((unused)),
 #elif defined __aarch64__
   /* Compat mode: arm compatible code running on aarch64 */
   int i;
-  struct user_pt_regs gregs;
+  struct user_regs_struct gregs;
   struct iovec iovec;
   iovec.iov_base = &gregs;
   iovec.iov_len = sizeof (gregs);
diff --git a/configure.ac b/configure.ac
index 1d79597..f9c3c30 100644
--- a/configure.ac
+++ b/configure.ac
@@ -301,6 +301,19 @@ eu_version=$(( (eu_version + 999) / 1000 ))
 
 AC_CHECK_SIZEOF(long)
 
+# On aarch64 before glibc 2.20 we would get the kernel user_pt_regs instead
+# of the user_regs_struct from sys/user.h. They are structurally the same
+# but we get either one or the other.
+AC_CHECK_TYPE([struct user_regs_struct],
+              [sys_user_has_user_regs=yes], [sys_user_has_user_regs=no],
+              [[#include <sys/ptrace.h>]
+               [#include <sys/time.h>]
+               [#include <sys/user.h>]])
+if test "$sys_user_has_user_regs" = "yes"; then
+  AC_DEFINE(HAVE_SYS_USER_REGS, 1,
+            [Define to 1 if <sys/user.h> defines struct user_regs_struct])
+fi
+
 # On a 64-bit host where can can use $CC -m32, we'll run two sets of tests.
 # Likewise in a 32-bit build on a host where $CC -m64 works.
 utrace_BIARCH
-- 
1.7.1