This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] PowerPC: use libgcc _Unwind functions to get backtrace
- From: Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>
- To: libc-alpha at sourceware dot org
- Date: Tue, 23 Jul 2013 22:42:12 -0300
- Subject: Re: [PATCH] PowerPC: use libgcc _Unwind functions to get backtrace
- References: <51EB297C dot 30104 at linux dot vnet dot ibm dot com> <1374499285 dot 4775 dot 118 dot camel at spokane1 dot rchland dot ibm dot com> <51ED9905 dot 5030808 at linux dot vnet dot ibm dot com> <8761w2tg6f dot fsf at igel dot home> <51EE89D5 dot 5050506 at linux dot vnet dot ibm dot com> <CAAKybw-R43rdifqPa3H+7q7XB47K77ES_hm5bpmY5=5NCa7Kyg at mail dot gmail dot com>
Hi Ryan,
Thanks for the review.
On 23-07-2013 14:05, Ryan S. Arnold wrote:
> On Tue, Jul 23, 2013 at 8:49 AM, Adhemerval Zanella
> <azanella@linux.vnet.ibm.com> wrote:
>
> I had to hunt in the kernel sources for an explanation of what this
> is.. and now it's kind of obvious but....
>
> Here's a good explanation that Peter Bergner provided. Perhaps it can
> become a comment?
>
> "A signal handler is just like any other function compiled by the
> compiler, so if it needs to save/restore the LR, it will save it into
> the 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. If we didn't create a dummy frame, the save of the LR could
> trash the stack frame."
Fair enough, I have added it on this new version.
> I assume the "we don't care..." comment means that this gets overlaid
> on top of the 'current' frame pointer since we only need to inspect
> the context? Perhaps a comment on this would be useful.
The 'don't care' means the rest of signal frame data is not really useful since
I just need the IP for the interrupted symbol. I have added a more explicit
comment.
>
> Why do you need 'fp' if it isn't used?
I don't, it was from a previous version. I removed it.
> Should this properly require #include <sys/ucontext.h>?
Not really.
>
> Is this second block supposed to be:
>
> if (is_sigtramp_address_rt...
>
> If so, rt_signal_frame_32 doesn't have an mctx element, because it
> only goes up to the ucontext. I think the rt_signal_frame_32 needs to
> be larger.
>
> I'd suggest a testcase to test this but it'd require a realtime kernel?
You are correct and the logic should be to test with rt_signal_frame_32, I
have fixed it and added a test to check both __vdso_sigtramp32 and
__vdso_sigtramp_rt32 signal trampolines. Basically for PPC32 __vdso_sigtramp_rt32
is used if SA_SIGINFO is set for sigaction.
> You need to account for the possibility of the transaction memory
> context 'uc_transact' added for POWER8 in order to properly get at
> 'tramp'.
>
> Also, how does this work if the VSX regs are included in the ucontext
> coming from the kernel? Can we even tell? Can we write a test for
> this?
Not really, the TM info is after the GPR register we need.
> Do these require a symbol definition to
> sysdeps/unix/sysv/linux/powerpc/Versions under GLIBC_PRIVATE like we
> do for the other vdso routines?
I thought so, but make check didn't complain about it.
> Care to add a comment on why there isn't a __kernel_sigtramp64 and
> equivalent __vdso_sigtramp64 like there is for powerpc32?
>
> Ryan S. Arnold
>
I added a more explicit comment about it.
Main difference about this patch from previous is I have fixed the
__vdso_sigtramp_rt32 handling for PPC32 and also change the backtrace5
testcase to actually test it.
---
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: Add a new test for SA_SIGINFO option.
--
diff --git a/debug/tst-backtrace5.c b/debug/tst-backtrace5.c
index ca47437..f8984df 100644
--- a/debug/tst-backtrace5.c
+++ b/debug/tst-backtrace5.c
@@ -29,6 +29,7 @@
#include "tst-backtrace.h"
static int do_test (void);
+#define TIMEOUT 5
#define TEST_FUNCTION do_test ()
#include "../test-skeleton.c"
@@ -91,7 +92,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 +101,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 +133,7 @@ fn (int c)
NO_INLINE static int
do_test (void)
{
- fn (2);
+ fn (2, 0);
+ fn (2, SA_SIGINFO);
return ret;
}
diff --git a/sysdeps/powerpc/powerpc32/backtrace.c b/sysdeps/powerpc/powerpc32/backtrace.c
index b4b11dd..cb9c9ca 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 the IP value is at 'uc' 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. */
+};
+
+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..0309493 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,31 @@ 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;
+ unsigned long unused[2];
+ unsigned int tramp[6];
+ /* We don't care about the rest, since the IP value is at 'uc' field. */
+};
+
+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 +81,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..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