linux: Use user_regs instead of pt_regs for 32-bit ARM in ProcessInfo [crashpad/crashpad : master]

Mark Mentovai (Gerrit)

unread,

Mar 15, 2017, 10:06:20 AM3/15/17

to Joshua Peraza, Commit Bot, crashp...@chromium.org

Mark Mentovai posted comments on this change.

Patch set 2:

I couldn’t build crashpad_util_test on a Raspberry Pi because its libc, glibc, deals with these types differently than Android’s libc, Bionic.

To view, visit change 455558. To unsubscribe, visit settings.

Joshua Peraza (Gerrit)

unread,

Mar 15, 2017, 12:05:23 PM3/15/17

to Mark Mentovai, Commit Bot, crashp...@chromium.org

Joshua Peraza posted comments on this change.

View Change

Patch set 7:Code-Review +1

To view, visit change 455558. To unsubscribe, visit settings.

Mark Mentovai (Gerrit)

unread,

Mar 15, 2017, 1:11:51 PM3/15/17

to Joshua Peraza, Commit Bot, crashp...@chromium.org

Mark Mentovai merged this change.

View Change

Approvals:
  Joshua Peraza: Looks good to me

linux: Use user_regs instead of pt_regs for 32-bit ARM in ProcessInfo

Not all libc implementations reliably expose pt_regs from
<sys/ptrace.h>. glibc-2.25/sysdeps/generic/sys/ptrace.h, for example,
does not #include <asm/ptrace.h> (which defines the structure) or
anything else that would #include that file such as <linux/ptrace.h>. On
the other hand, Android 7.1.1 bionic/libc/include/sys/ptrace.h does
#include <linux/ptrace.h>.

It is not viable to #include <asm/ptrace.h> or <linux/ptrace.h>
directly: it would be natural to #include them, sorted, before
<sys/ptrace.h> but this causes problems for glibc’s <sys/ptrace.h>.
Constants like PTRACE_GETREGS and PTRACE_TRACEME are simple macros in
<asm/ptrace.h> and <linux/ptrace.h>, respectively, but are defined in
enums in glibc’s <sys/ptrace.h>, and this doesn’t mix well. It is
possible to #include <asm/ptrace.h> (but not <linux/ptrace.h>) after
<sys/ptrace.h>, but because this involves same-value macro redefinitions
and because it reaches into internal headers, it’s not preferred.

The alternative approach taken here is to use the user_regs structure
from <sys/user.h>, which is reliably defined by both Bionic and glibc,
and has the same layout as the kernel’s pt_regs structure. (All that
matters in this code is the size of the structure.) See Android 7.1.1
bionic/libc/include/sys/user.h,
glibc-2.25/sysdeps/unix/sysv/linux/arm/sys/user.h, and
linux-4.9.15/arch/arm/include/asm/ptrace.h for the various equivalent
definitions.

Take the same approach for 64-bit ARM: use user_regs_struct from
<sys/user.h> in preference to hoping for a C library’s <sys/ptrace.h> to
somehow provide the kernel’s user_pt_regs.

This mirrors the approach already being used for x86 and x86_64, which
use the C library’s <sys/user.h> user_regs_struct.

Bug: crashpad:30
Test: crashpad_util_test ProcessInfo.*
Change-Id: I3067e32c7fa4d6c8f4f2d5b63df141a0f490cd13
Reviewed-on: https://chromium-review.googlesource.com/455558
Reviewed-by: Joshua Peraza <jpe...@chromium.org>
---
M util/posix/process_info_linux.cc
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/util/posix/process_info_linux.cc b/util/posix/process_info_linux.cc
index 9147e71..82f5888 100644
--- a/util/posix/process_info_linux.cc
+++ b/util/posix/process_info_linux.cc
@@ -342,12 +342,10 @@
       // more/larger registers than this process. If the kernel fills less space
       // than sizeof(regs) then the target process uses smaller/fewer registers.
       struct {
-#if defined(ARCH_CPU_X86_FAMILY)
+#if defined(ARCH_CPU_X86_FAMILY) || defined(ARCH_CPU_ARM64)
         using PrStatusType = user_regs_struct;
 #elif defined(ARCH_CPU_ARMEL)
-        using PrStatusType = pt_regs;
-#elif defined(ARCH_CPU_ARM64)
-        using PrStatusType = user_pt_regs;
+        using PrStatusType = user_regs;
 #endif
         PrStatusType regs;
         char extra;

To view, visit change 455558. To unsubscribe, visit settings.

Reply all

Reply to author

Forward