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 v4] ARM: Add SystemTap probes to longjmp and setjmp.


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.


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