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

[Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>]

On 22-07-2013 10:21, Steven Munroe wrote:
> On Sat, 2013-07-20 at 21:21 -0300, Adhemerval Zanella wrote:
>> This patch changes the way backtraces are obtained on PowerPC by
>> relaying on libgcc _Unwind function instead of just backchain parse.
>> It fixes debug/tst-backtrace5 for PPC32 and PPC64.
> Is this really necessary? And what are the performance implications?
>
> On platforms that switched to using the Unwinder a number of performance
> tools have been rendered useless due to poor performance. I don't want
> that to happen on PowerPC.
>
> What is the issue with tst-backtrace5 and is the test case really valid?
>
The tst-backtrace5 checks if the backtrace function resolves the backtrace in
a signal handler interrupting a blocking syscall. And PowerPC backtrace 
implementation does not handle the signal trampoline stack frame information
about the interrupted symbol, which leads the backchain code to miss such
information in the backtrace.

Below is an alternated implementation that parse the signal trampoline information
saved in stack to get information on such symbol. It should show better performance 
compared to libgcc.so implementation.

No regressions found and it also fixes the tst-backtrace5 tests. Tested on PPC64 and
PPC32.

---

2013-07-22  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.

--

diff --git a/sysdeps/powerpc/powerpc32/backtrace.c b/sysdeps/powerpc/powerpc32/backtrace.c
index b4b11dd..9dd321c 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,43 @@ struct layout
   void *return_address;
 };
 
+#define __SIGNAL_FRAMESIZE32 64
+
+/* signal trampoline saved area.  */
+struct signal_frame_32 {
+  char               dummy[__SIGNAL_FRAMESIZE32];
+  struct sigcontext  sctx;
+  mcontext_t         mctx;
+  /* We don't care about the rest.  */
+}; 
+
+static inline
+int is_sigtramp_address (unsigned int nip, unsigned int fp)
+{
+#ifdef SHARED
+  if (nip == (unsigned long)__vdso_sigtramp32)
+    return 1;
+#endif
+  return 0;
+}
+
+struct rt_signal_frame_32 {
+  char               dummy[__SIGNAL_FRAMESIZE32 + 16];
+  siginfo_t          info;
+  struct ucontext    uc;
+  /* We don't care about the rest.  */
+};
+
+static inline
+int is_sigtramp_address_rt (unsigned int nip, unsigned int fp)
+{
+#ifdef SHARED
+  if (nip == (unsigned long)__vdso_sigtramp_rt32)
+    return 1;
+#endif
+  return 0;
+}
+
 int
 __backtrace (void **array, int size)
 {
@@ -50,7 +90,30 @@ __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,
+                               (unsigned int)current))
+	{
+	  struct signal_frame_32 *sigframe = 
+	    (struct signal_frame_32*) current;
+          gregset = &sigframe->mctx.gregs;
+        }
+      if (is_sigtramp_address ((unsigned int)current->return_address,
+                               (unsigned int)current))
+	{
+	  struct signal_frame_32 *sigframe =
+            (struct signal_frame_32*) current;
+          gregset = &sigframe->mctx.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..1b021fa 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,28 @@ struct layout
   void *return_address;
 };
 
+/* signal trampoline saved area.  */
+struct signal_frame_64 {
+#define __SIGNAL_FRAMESIZE 128
+  char            dummy[__SIGNAL_FRAMESIZE];
+  struct ucontext uc;
+  unsigned long   unused[2];
+  unsigned int    tramp[6];
+  /* We don't care about the rest.  */
+}; 
+
+static inline
+int is_sigtramp_address (unsigned long nip, unsigned long fp)
+{
+  if (nip == fp + offsetof(struct signal_frame_64, tramp))
+    return 1;
+#ifdef SHARED
+  if (nip == (unsigned long)__vdso_sigtramp_rt64)
+    return 1;
+#endif
+  return 0;
+}
+
 int
 __backtrace (void **array, int size)
 {
@@ -53,7 +78,18 @@ __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,
+                               (unsigned long)current))
+        {
+	  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..594eb50 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,13 @@ _libc_vdso_platform_setup (void)
   __vdso_getcpu = _dl_vdso_vsym ("__kernel_getcpu", &linux2615);
 
   __vdso_time = _dl_vdso_vsym ("__kernel_time", &linux2615);
+
+#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