This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v4] ARM: Add SystemTap probes to longjmp and setjmp.
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Will Newton <will dot newton at linaro dot org>, libc-alpha at sourceware dot org
- Date: Wed, 19 Feb 2014 13:36:03 -0500
- Subject: Re: [PATCH v4] ARM: Add SystemTap probes to longjmp and setjmp.
- Authentication-results: sourceware.org; auth=none
- References: <1392830753-3100-1-git-send-email-will dot newton at linaro dot org>
On 02/19/2014 12:25 PM, Will Newton wrote:
> Now the ARM port implements pointer encryption for jmpbufs, gdb needs
> a SystemTap probe point in longjmp to determine the target PC of
> a call to longjmp. This patch implements the probe point in longjmp
> and a similar probe point in setjmp.
>
> In order to have all the appropriate registers available to pass to the
> probe this reorders the layout of jmpbuf, putting the sp and lr registers
> at the start rather than the end, allowing them to be read and
> written sequentially.
>
> Tested on armv7, no new failures in the glibc testsuite and confirmed
> that this fixes the gdb.base/longjmp.exp failures in the gdb testsuite.
Please try to keep vX notes in patches so I can see what's changed
through each revision and double check that matches what I'm expecting.
You have a v4 note below, but I'd like to see those vX comments append
up as you make changes.
This patch looks good to me. Thanks for validating against gdb.
> 2014-01-27 Will Newton <will.newton@linaro.org>
>
> * sysdeps/arm/__longjmp.S: Include stap-probe.h.
> (__longjmp): Restore sp and lr before restoring callee
> saved registers. Add longjmp and longjmp_target
> SystemTap probe point.
> * sysdeps/arm/bits/setjmp.h (__jmp_buf): Update comment.
> * sysdeps/arm/include/bits/setjmp.h (__JMP_BUF_SP):
> Define to zero to match jmpbuf layout.
> * sysdeps/arm/setjmp.S: Include stap-probe.h.
> (__sigsetjmp): Save sp and lr before saving callee
> saved registers. Add setjmp SystemTap probe point.
> ---
> sysdeps/arm/__longjmp.S | 61 ++++++++++++++++++++++++---------------
> sysdeps/arm/bits/setjmp.h | 5 ++--
> sysdeps/arm/include/bits/setjmp.h | 2 +-
> sysdeps/arm/setjmp.S | 12 ++++++--
> 4 files changed, 49 insertions(+), 31 deletions(-)
>
> Changes in v4:
> - Update __jmp_buf comment to match the code
>
> diff --git a/sysdeps/arm/__longjmp.S b/sysdeps/arm/__longjmp.S
> index 27c57a1..08521e5 100644
> --- a/sysdeps/arm/__longjmp.S
> +++ b/sysdeps/arm/__longjmp.S
> @@ -17,6 +17,7 @@
> <http://www.gnu.org/licenses/>. */
>
> #include <sysdep.h>
> +#include <stap-probe.h>
> #include <bits/setjmp.h>
> #include <rtld-global-offsets.h>
> #include <arm-features.h>
> @@ -25,31 +26,35 @@
>
> ENTRY (__longjmp)
> mov ip, r0
> - movs r0, r1 /* get the return value in place */
> - it eq
> - moveq r0, #1 /* can't let setjmp() return zero! */
>
> #ifdef CHECK_SP
> sfi_breg ip, \
> - ldr r4, [\B, #32] /* jmpbuf's sp */
> + ldr r4, [\B] /* jmpbuf's sp */
> cfi_undefined (r4)
> #ifdef PTR_DEMANGLE
> PTR_DEMANGLE (r4, r4, a3, a4)
> #endif
> CHECK_SP (r4)
> #endif
> - sfi_sp sfi_breg ip, \
> - ldmia \B!, JMP_BUF_REGLIST
> +
> #ifdef PTR_DEMANGLE
> ldr a4, [ip], #4
> - PTR_DEMANGLE (a4, a4, a3, a2)
> - mov sp, a4
> - ldr a4, [ip], #4
> - PTR_DEMANGLE2 (lr, a4, a3)
> + PTR_DEMANGLE (a4, a4, a3, r4)
> + cfi_undefined (r4)
> + ldr r4, [ip], #4
> + PTR_DEMANGLE2 (r4, r4, a3)
OK.
> #else
> - ldr sp, [ip], #4
> - ldr lr, [ip], #4
> + ldr a4, [ip], #4
> + ldr r4, [ip], #4
> + cfi_undefined (r4)
> #endif
> + /* longjmp probe expects longjmp first argument (4@r0), second
> + argument (-4@r1), and target address (4@r4), respectively. */
> + LIBC_PROBE (longjmp, 3, 4@r0, -4@r1, 4@r4)
> + mov sp, a4
> + mov lr, r4
> + sfi_sp sfi_breg ip, \
> + ldmia \B!, JMP_BUF_REGLIST
OK.
> cfi_restore (v1)
> cfi_restore (v2)
> cfi_restore (v3)
> @@ -67,27 +72,27 @@ ENTRY (__longjmp)
>
> #ifdef NEED_HWCAP
> # ifdef IS_IN_rtld
> - ldr a2, 1f
> + ldr a4, 1f
> ldr a3, .Lrtld_local_ro
> -0: add a2, pc, a2
> - add a2, a2, a3
> - ldr a2, [a2, #RTLD_GLOBAL_RO_DL_HWCAP_OFFSET]
> +0: add a4, pc, a4
> + add a4, a4, a3
> + ldr a4, [a4, #RTLD_GLOBAL_RO_DL_HWCAP_OFFSET]
> # else
> # ifdef PIC
> - ldr a2, 1f
> + ldr a4, 1f
> ldr a3, .Lrtld_global_ro
> -0: add a2, pc, a2
> - ldr a2, [a2, a3]
> - ldr a2, [a2, #RTLD_GLOBAL_RO_DL_HWCAP_OFFSET]
> +0: add a4, pc, a4
> + ldr a4, [a4, a3]
> + ldr a4, [a4, #RTLD_GLOBAL_RO_DL_HWCAP_OFFSET]
> # else
> - ldr a2, .Lhwcap
> - ldr a2, [a2, #0]
> + ldr a4, .Lhwcap
> + ldr a4, [a4, #0]
> # endif
> # endif
> #endif
>
> #ifdef __SOFTFP__
> - tst a2, #HWCAP_ARM_VFP
> + tst a4, #HWCAP_ARM_VFP
> beq .Lno_vfp
> #endif
>
> @@ -98,7 +103,7 @@ ENTRY (__longjmp)
> .Lno_vfp:
>
> #ifndef ARM_ASSUME_NO_IWMMXT
> - tst a2, #HWCAP_ARM_IWMMXT
> + tst a4, #HWCAP_ARM_IWMMXT
> beq .Lno_iwmmxt
>
> /* Restore the call-preserved iWMMXt registers. */
> @@ -118,6 +123,14 @@ ENTRY (__longjmp)
> .Lno_iwmmxt:
> #endif
>
> + /* longjmp_target probe expects longjmp first argument (4@r0), second
> + argument (-4@r1), and target address (4@r14), respectively. */
> + LIBC_PROBE (longjmp_target, 3, 4@r0, -4@r1, 4@r14)
> +
> + movs r0, r1 /* get the return value in place */
> + it eq
> + moveq r0, #1 /* can't let setjmp() return zero! */
OK.
> +
> DO_RET(lr)
>
> #ifdef NEED_HWCAP
> diff --git a/sysdeps/arm/bits/setjmp.h b/sysdeps/arm/bits/setjmp.h
> index 41423b2..a687359 100644
> --- a/sysdeps/arm/bits/setjmp.h
> +++ b/sysdeps/arm/bits/setjmp.h
> @@ -28,9 +28,8 @@
> /* The exact set of registers saved may depend on the particular core
> in use, as some coprocessor registers may need to be saved. The C
> Library ABI requires that the buffer be 8-byte aligned, and
> - recommends that the buffer contain 64 words. The first 27 words
> - are occupied by v1-v6, sl, fp, sp, pc, and d8-d15. (Note that
> - d8-15 require 17 words, due to the use of fstmx.) */
> + recommends that the buffer contain 64 words. The first 26 words
> + are occupied by sp, lr, v1-v6, sl, fp, and d8-d15. */
OK.
> typedef int __jmp_buf[64] __attribute__((__aligned__ (8)));
> #endif
>
> diff --git a/sysdeps/arm/include/bits/setjmp.h b/sysdeps/arm/include/bits/setjmp.h
> index 220dfe8..5877c1f 100644
> --- a/sysdeps/arm/include/bits/setjmp.h
> +++ b/sysdeps/arm/include/bits/setjmp.h
> @@ -30,7 +30,7 @@
> # define JMP_BUF_REGLIST {v1-v6, sl, fp}
>
> /* Index of __jmp_buf where the sp register resides. */
> -# define __JMP_BUF_SP 8
> +# define __JMP_BUF_SP 0
OK.
> #endif
>
> #endif /* include/bits/setjmp.h */
> diff --git a/sysdeps/arm/setjmp.S b/sysdeps/arm/setjmp.S
> index b0b45ed..5e55ca5 100644
> --- a/sysdeps/arm/setjmp.S
> +++ b/sysdeps/arm/setjmp.S
> @@ -17,6 +17,7 @@
> <http://www.gnu.org/licenses/>. */
>
> #include <sysdep.h>
> +#include <stap-probe.h>
OK.
> #include <bits/setjmp.h>
> #include <rtld-global-offsets.h>
> #include <arm-features.h>
> @@ -27,9 +28,11 @@ ENTRY (__sigsetjmp)
> #endif
> mov ip, r0
>
> - /* Save registers */
> - sfi_breg ip, \
> - stmia \B!, JMP_BUF_REGLIST
> + /* setjmp probe expects sigsetjmp first argument (4@r0), second
> + argument (-4@r1), and target address (4@r14), respectively. */
> + LIBC_PROBE (setjmp, 3, 4@r0, -4@r1, 4@r14)
OK.
> +
> + /* Save sp and lr */
> #ifdef PTR_MANGLE
> mov a4, sp
> PTR_MANGLE2 (a4, a4, a3)
> @@ -40,6 +43,9 @@ ENTRY (__sigsetjmp)
> str sp, [ip], #4
> str lr, [ip], #4
> #endif
> + /* Save registers */
> + sfi_breg ip, \
> + stmia \B!, JMP_BUF_REGLIST
OK.
>
> #if !defined ARM_ASSUME_NO_IWMMXT || defined __SOFTFP__
> # define NEED_HWCAP 1
>
Cheers,
Carlos.