This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] PowerPC: use libgcc _Unwind functions to get backtrace


On 24-07-2013 15:14, Ryan S. Arnold wrote:
> On powerpc64 the ucontext contains the mcontext_t, which holds the
> register states.
>
> You'll notice that the last entry in that structure in the glibc
> header is the vmx_reserve field: (look at
> glibc/sysdeps/unix/sysv/linux/powerpc/sys/ucontext.h):
>
> long            vmx_reserve[NVRREG+NVRREG+1];
>
> This means there is no Vector Scalar Register reserve area accounted
> for in the glibc mcontext structure.  The [get|set] context structures
> have been deprecated in POSIX so Uli Drepper wouldn't allow us to
> extend the structure definitions for VSRs (due to complications with
> versioning structures in APIs).
>
> So in your code, using offsetof (struct signal_frame_64, tramp) will
> presumably compute the size of the ucontext structure, as informed by
> the glibc ucontext.h header, when determining the offset of 'tramp'
> from the frame pointer.
>
> Newer kernels  unconditionally extend the vmx_reserve field for the
> vector scalar registers (look at
> linux/arch/powerpc/include/uapi/asm/sigcontext.h):
>
> " * Part of the VSX data is stored here also by extending vmx_restore
>  * by an additional 32 double words.  Architecturally the layout of
>  * the VSR registers and how they overlap on top of the legacy FPR and
>  * VR registers is shown below:"
> ...
>
> long            vmx_reserve[ELF_NVRREG+ELF_NVRREG+32+1];
>
> Doesn't this mean that when running this on POWER7 or POWER8 that
> offset you've computed for 'tramp' will actually come up short and
> will point into the Vector Scalar Register section of vmx_reserve as
> allocated by the kernel and not at 'tramp' as you've intended?
>
> So on a system configured with VSX support you'll need to account for
> this extended vmx_reserve.
>
> I'm not sure of the best way to do this.  You could wrap it in a
> kernel guard and assume that vmx_reserve has been extended based on
> when this was added to the kernel (version).  If not 'assuming' a
> kernel version you could check the dl_hwcap for PPC_FEATURE_VSX.  I
> wonder how much this will slow down the backtrace?
>
> Additionally, if you look in the Linux kernel's
> linux/arch/powerpc/kernel/signal_64.c at the definition of struct
> rt_sigframe:
>
> struct rt_sigframe {
>         /* sys_rt_sigreturn requires the ucontext be the first field */
>         struct ucontext uc;
> #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>         struct ucontext uc_transact;
> #endif
>         unsigned long _unused[2];
>         unsigned int tramp[TRAMP_SIZE];
> ....
>
> The addition of the uc_transact structure on POWER8 system will
> additionally throw off your 'tramp' offset computation unless you
> account for it in your computation on POWER8 by checking hwcap2 for
> PPC_FEATURE2_HTM and compensating.
>
> Let me know if I'm completely wrong about all of this.
>
> Ryan S. Arnold
>
Hi Ryan, thanks for the notes. You are correct and I realized it does needed to support
the non vDSO test (Linux 2.6.19 already provides the signal trampolines for PPC32
and PPC64). I also corrected the comments you pointed out.

Following Joseph comments, I added another test (tst-backtrace6) and changed tst-backtrace5
so the sa_flags can be changed if a define. The current behavior of tst-backtrace5 is unchanged.
I also checked on x86_64 and x86 and tst-backtrace6 produces the same output as tst-backtrace5,
it only changes for PPC32 that uses different signal trampolines depending of sa_flags.

And since 2.18 is close, I'd like to push it to 2.19.

What do you think now Ryan?

---

2013-07-23  Adhemerval Zanella  <azanella@linux.vnet.ibm.com>

	* sysdeps/powerpc/powerpc32/backtrace.c (__backtrace): Handle signal
	trampoline stack frame information.
	* sysdeps/powerpc/powerpc64/backtrace.c (__backtrace): Likewise.
	* sysdeps/unix/sysv/linux/powerpc/bits/libc-vdso.h
	(__vdso_sigtramp_rt64): New variable: PPC64 signal trampoline. 
	(__vdso_sigtramp32): New variable: PPC32 signal trampoline.
	(__vdso_sigtramp_rt32): New variable: PPC32 signal trampoline.
	* sysdeps/unix/sysv/linux/powerpc/init-first.c 
	(_libc_vdso_platform_setup): Initialize the signal trampolines.
	* debug/tst-backtrace5.c (fn): Add an option set modify sigaction
	sa_flags value.
	* debug/tst-backtrace6.c: New file: check backtrace for signal frames,
	interrupting a syscall and set with option SA_SIGINFO.

--

diff --git a/debug/Makefile b/debug/Makefile
index 779741f..13ee5c8 100644
--- a/debug/Makefile
+++ b/debug/Makefile
@@ -130,16 +130,18 @@ CFLAGS-tst-backtrace2.c += -funwind-tables
 CFLAGS-tst-backtrace3.c += -funwind-tables
 CFLAGS-tst-backtrace4.c += -funwind-tables
 CFLAGS-tst-backtrace5.c += -funwind-tables
+CFLAGS-tst-backtrace6.c += -funwind-tables
 LDFLAGS-tst-backtrace2 = -rdynamic
 LDFLAGS-tst-backtrace3 = -rdynamic
 LDFLAGS-tst-backtrace4 = -rdynamic
 LDFLAGS-tst-backtrace5 = -rdynamic
+LDFLAGS-tst-backtrace6 = -rdynamic
 
 tests = backtrace-tst tst-longjmp_chk tst-chk1 tst-chk2 tst-chk3 \
 	tst-lfschk1 tst-lfschk2 tst-lfschk3 test-strcpy_chk test-stpcpy_chk \
 	tst-chk4 tst-chk5 tst-chk6 tst-lfschk4 tst-lfschk5 tst-lfschk6 \
 	tst-longjmp_chk2 tst-backtrace2 tst-backtrace3 tst-backtrace4 \
-	tst-backtrace5
+	tst-backtrace5 tst-backtrace6
 
 tests-ifunc := $(stpcpy_chk strcpy_chk:%=test-%-ifunc)
 tests += $(tests-ifunc)
diff --git a/debug/tst-backtrace5.c b/debug/tst-backtrace5.c
index ca47437..51180c1 100644
--- a/debug/tst-backtrace5.c
+++ b/debug/tst-backtrace5.c
@@ -28,6 +28,10 @@
 
 #include "tst-backtrace.h"
 
+#ifndef SIGACTION_FLAGS
+# define SIGACTION_FLAGS 0
+#endif
+
 static int do_test (void);
 #define TEST_FUNCTION do_test ()
 #include "../test-skeleton.c"
@@ -91,7 +95,7 @@ handle_signal (int signum)
 }
 
 NO_INLINE int
-fn (int c)
+fn (int c, int flags)
 {
   pid_t parent_pid, child_pid;
   int pipefd[2];
@@ -100,12 +104,13 @@ fn (int c)
 
   if (c > 0)
     {
-      fn (c - 1);
+      fn (c - 1, flags);
       return x;
     }
 
   memset (&act, 0, sizeof (act));
   act.sa_handler = handle_signal;
+  act.sa_flags = flags;
   sigemptyset (&act.sa_mask);
   sigaction (SIGUSR1, &act, NULL);
   parent_pid = getpid ();
@@ -131,6 +136,6 @@ fn (int c)
 NO_INLINE static int
 do_test (void)
 {
-  fn (2);
+  fn (2, SIGACTION_FLAGS);
   return ret;
 }
diff --git a/debug/tst-backtrace6.c b/debug/tst-backtrace6.c
new file mode 100644
index 0000000..cd8dbcd
--- /dev/null
+++ b/debug/tst-backtrace6.c
@@ -0,0 +1,21 @@
+/* Test backtrace and backtrace_symbols for signal frames, where a
+   system call was interrupted by a signal.
+   Copyright (C) 2013 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#define SIGACTION_FLAGS SA_SIGINFO
+#include <debug/tst-backtrace5.c>
diff --git a/sysdeps/powerpc/powerpc32/backtrace.c b/sysdeps/powerpc/powerpc32/backtrace.c
index b4b11dd..8d413e6 100644
--- a/sysdeps/powerpc/powerpc32/backtrace.c
+++ b/sysdeps/powerpc/powerpc32/backtrace.c
@@ -18,6 +18,9 @@
 
 #include <execinfo.h>
 #include <stddef.h>
+#include <string.h>
+#include <signal.h>
+#include <bits/libc-vdso.h>
 
 /* This is the stack layout we see with every stack frame.
    Note that every routine is required by the ABI to lay out the stack
@@ -35,6 +38,46 @@ struct layout
   void *return_address;
 };
 
+#define SIGNAL_FRAMESIZE 64
+
+/* Since the signal handler is just like any other function it needs to
+   save/restore its LR and it will save it into callers stack frame.
+   Since a signal handler doesn't have a caller, the kernel creates a
+   dummy frame to make it look like it has a caller.  */
+struct signal_frame_32 {
+  char               dummy[SIGNAL_FRAMESIZE];
+  struct sigcontext  sctx;
+  mcontext_t         mctx;
+  /* We don't care about the rest, since IP value is at 'mctx' field.  */
+};
+
+static inline int
+is_sigtramp_address (unsigned int nip)
+{
+#ifdef SHARED
+  if (nip == (unsigned int)__vdso_sigtramp32)
+    return 1;
+#endif
+  return 0;
+}
+
+struct rt_signal_frame_32 {
+  char               dummy[SIGNAL_FRAMESIZE + 16];
+  siginfo_t          info;
+  struct ucontext    uc;
+  /* We don't care about the rest, since IP value is at 'uc' field.  */
+};
+
+static inline int
+is_sigtramp_address_rt (unsigned int nip)
+{
+#ifdef SHARED
+  if (nip == (unsigned int)__vdso_sigtramp_rt32)
+    return 1;
+#endif
+  return 0;
+}
+
 int
 __backtrace (void **array, int size)
 {
@@ -50,7 +93,28 @@ __backtrace (void **array, int size)
   for (				count = 0;
        current != NULL && 	count < size;
        current = current->next, count++)
-    array[count] = current->return_address;
+    {
+      gregset_t *gregset = NULL;
+
+      array[count] = current->return_address;
+
+      /* Check if the symbol is the signal trampoline and get the interrupted
+       * symbol address from the trampoline saved area.  */
+      if (is_sigtramp_address ((unsigned int)current->return_address))
+	{
+	  struct signal_frame_32 *sigframe =
+	    (struct signal_frame_32*) current;
+          gregset = &sigframe->mctx.gregs;
+        }
+      else if (is_sigtramp_address_rt ((unsigned int)current->return_address))
+	{
+	  struct rt_signal_frame_32 *sigframe =
+            (struct rt_signal_frame_32*) current;
+          gregset = &sigframe->uc.uc_mcontext.uc_regs->gregs;
+        }
+      if (gregset)
+	array[++count] = (void*)((*gregset)[PT_NIP]);
+    }
 
   /* It's possible the second-last stack frame can't return
      (that is, it's __libc_start_main), in which case
diff --git a/sysdeps/powerpc/powerpc64/backtrace.c b/sysdeps/powerpc/powerpc64/backtrace.c
index 2d3e051..9b9a9f1 100644
--- a/sysdeps/powerpc/powerpc64/backtrace.c
+++ b/sysdeps/powerpc/powerpc64/backtrace.c
@@ -18,6 +18,9 @@
 
 #include <execinfo.h>
 #include <stddef.h>
+#include <string.h>
+#include <signal.h>
+#include <bits/libc-vdso.h>
 
 /* This is the stack layout we see with every stack frame.
    Note that every routine is required by the ABI to lay out the stack
@@ -38,6 +41,27 @@ struct layout
   void *return_address;
 };
 
+/* Since the signal handler is just like any other function it needs to
+   save/restore its LR and it will save it into callers stack frame.
+   Since a signal handler doesn't have a caller, the kernel creates a
+   dummy frame to make it look like it has a caller.  */
+struct signal_frame_64 {
+#define SIGNAL_FRAMESIZE 128
+  char            dummy[SIGNAL_FRAMESIZE];
+  struct ucontext uc;
+  /* We don't care about the rest, since the IP value is at 'uc' field.  */
+};
+
+static inline int
+is_sigtramp_address (unsigned long nip)
+{
+#ifdef SHARED
+  if (nip == (unsigned long)__vdso_sigtramp_rt64)
+    return 1;
+#endif
+  return 0;
+}
+
 int
 __backtrace (void **array, int size)
 {
@@ -53,7 +77,17 @@ __backtrace (void **array, int size)
   for (				count = 0;
        current != NULL && 	count < size;
        current = current->next, count++)
-    array[count] = current->return_address;
+    {
+      array[count] = current->return_address;
+
+      /* Check if the symbol is the signal trampoline and get the interrupted
+       * symbol address from the trampoline saved area.  */
+      if (is_sigtramp_address ((unsigned long)current->return_address))
+        {
+	  struct signal_frame_64 *sigframe = (struct signal_frame_64*) current;
+          array[++count] = (void*)sigframe->uc.uc_mcontext.gp_regs[PT_NIP];
+	}
+    }
 
   /* It's possible the second-last stack frame can't return
      (that is, it's __libc_start_main), in which case
diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/libc-vdso.h b/sysdeps/unix/sysv/linux/powerpc/bits/libc-vdso.h
index 8b195db..ba54de4 100644
--- a/sysdeps/unix/sysv/linux/powerpc/bits/libc-vdso.h
+++ b/sysdeps/unix/sysv/linux/powerpc/bits/libc-vdso.h
@@ -34,6 +34,13 @@ extern void *__vdso_getcpu;
 
 extern void *__vdso_time;
 
+#if defined(__PPC64__) || defined(__powerpc64__)
+extern void *__vdso_sigtramp_rt64;
+#else
+extern void *__vdso_sigtramp32;
+extern void *__vdso_sigtramp_rt32;
+#endif
+
 /* This macro is needed for PPC64 to return a skeleton OPD entry of a vDSO
    symbol.  This works because _dl_vdso_vsym always return the function
    address, and no vDSO symbols use the TOC or chain pointers from the OPD
diff --git a/sysdeps/unix/sysv/linux/powerpc/init-first.c b/sysdeps/unix/sysv/linux/powerpc/init-first.c
index f6f05f0..061715f 100644
--- a/sysdeps/unix/sysv/linux/powerpc/init-first.c
+++ b/sysdeps/unix/sysv/linux/powerpc/init-first.c
@@ -29,6 +29,12 @@ void *__vdso_clock_getres;
 void *__vdso_get_tbfreq;
 void *__vdso_getcpu;
 void *__vdso_time;
+#if defined(__PPC64__) || defined(__powerpc64__)
+void *__vdso_sigtramp_rt64;
+#else
+void *__vdso_sigtramp32;
+void *__vdso_sigtramp_rt32;
+#endif
 
 static inline void
 _libc_vdso_platform_setup (void)
@@ -46,6 +52,16 @@ _libc_vdso_platform_setup (void)
   __vdso_getcpu = _dl_vdso_vsym ("__kernel_getcpu", &linux2615);
 
   __vdso_time = _dl_vdso_vsym ("__kernel_time", &linux2615);
+
+  /* PPC64 uses only one signal trampoline symbol, while PPC32 will use
+     two depending if SA_SIGINFO is used (__kernel_sigtramp_rt32) or not
+     (__kernel_sigtramp32).  */
+#if defined(__PPC64__) || defined(__powerpc64__)
+  __vdso_sigtramp_rt64 = _dl_vdso_vsym ("__kernel_sigtramp_rt64", &linux2615);
+#else
+  __vdso_sigtramp32 = _dl_vdso_vsym ("__kernel_sigtramp32", &linux2615);
+  __vdso_sigtramp_rt32 = _dl_vdso_vsym ("__kernel_sigtramp_rt32", &linux2615);
+#endif
 }
 
 # define VDSO_SETUP _libc_vdso_platform_setup


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]