public inbox for libc-ports@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2] ARM: Add SystemTap probes to longjmp and setjmp.
@ 2014-02-05  9:56 Will Newton
  2014-02-06 16:26 ` Carlos O'Donell
  2014-02-06 22:11 ` Roland McGrath
  0 siblings, 2 replies; 11+ messages in thread
From: Will Newton @ 2014-02-05  9:56 UTC (permalink / raw)
  To: libc-ports; +Cc: patches

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 points 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.

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.

ports/ChangeLog.arm:

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/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.
---
 ports/sysdeps/arm/__longjmp.S           | 61 ++++++++++++++++++++-------------
 ports/sysdeps/arm/include/bits/setjmp.h |  2 +-
 ports/sysdeps/arm/setjmp.S              | 12 +++++--
 3 files changed, 47 insertions(+), 28 deletions(-)

Changes in v2:
 - Add longjmp_target probe
 - Move longjmp probe up to point before restore of sp/lr

diff --git a/ports/sysdeps/arm/__longjmp.S b/ports/sysdeps/arm/__longjmp.S
index 27c57a1..08521e5 100644
--- a/ports/sysdeps/arm/__longjmp.S
+++ b/ports/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)
 #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
 	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! */
+
 	DO_RET(lr)
 
 #ifdef NEED_HWCAP
diff --git a/ports/sysdeps/arm/include/bits/setjmp.h b/ports/sysdeps/arm/include/bits/setjmp.h
index 220dfe8..5877c1f 100644
--- a/ports/sysdeps/arm/include/bits/setjmp.h
+++ b/ports/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
 #endif
 
 #endif  /* include/bits/setjmp.h */
diff --git a/ports/sysdeps/arm/setjmp.S b/ports/sysdeps/arm/setjmp.S
index b0b45ed..5e55ca5 100644
--- a/ports/sysdeps/arm/setjmp.S
+++ b/ports/sysdeps/arm/setjmp.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>
@@ -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)
+
+	/* 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
 
 #if !defined ARM_ASSUME_NO_IWMMXT || defined __SOFTFP__
 # define NEED_HWCAP 1
-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] ARM: Add SystemTap probes to longjmp and setjmp.
  2014-02-05  9:56 [PATCH v2] ARM: Add SystemTap probes to longjmp and setjmp Will Newton
@ 2014-02-06 16:26 ` Carlos O'Donell
  2014-02-06 16:41   ` Joseph S. Myers
  2014-02-06 16:48   ` Will Newton
  2014-02-06 22:11 ` Roland McGrath
  1 sibling, 2 replies; 11+ messages in thread
From: Carlos O'Donell @ 2014-02-06 16:26 UTC (permalink / raw)
  To: Will Newton, libc-ports; +Cc: patches

On 02/05/2014 04:56 AM, 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 points 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.
> 
> 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.

This looks good to me.

If it's considered a bug, please check this in immediately and CC Allan
to keep him in the loop for these last minute bug fixes.

We are about to freeze so the 2.19 branch can be cut. If there is anything
else like this please bring it to his attention immediately.
 
> ports/ChangeLog.arm:
> 
> 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/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.
> ---
>  ports/sysdeps/arm/__longjmp.S           | 61 ++++++++++++++++++++-------------
>  ports/sysdeps/arm/include/bits/setjmp.h |  2 +-
>  ports/sysdeps/arm/setjmp.S              | 12 +++++--
>  3 files changed, 47 insertions(+), 28 deletions(-)
> 
> Changes in v2:
>  - Add longjmp_target probe
>  - Move longjmp probe up to point before restore of sp/lr
> 
> diff --git a/ports/sysdeps/arm/__longjmp.S b/ports/sysdeps/arm/__longjmp.S
> index 27c57a1..08521e5 100644
> --- a/ports/sysdeps/arm/__longjmp.S
> +++ b/ports/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)
>  #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
>  	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! */
> +
>  	DO_RET(lr)
>  
>  #ifdef NEED_HWCAP
> diff --git a/ports/sysdeps/arm/include/bits/setjmp.h b/ports/sysdeps/arm/include/bits/setjmp.h
> index 220dfe8..5877c1f 100644
> --- a/ports/sysdeps/arm/include/bits/setjmp.h
> +++ b/ports/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
>  #endif
>  
>  #endif  /* include/bits/setjmp.h */
> diff --git a/ports/sysdeps/arm/setjmp.S b/ports/sysdeps/arm/setjmp.S
> index b0b45ed..5e55ca5 100644
> --- a/ports/sysdeps/arm/setjmp.S
> +++ b/ports/sysdeps/arm/setjmp.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>
> @@ -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)
> +
> +	/* 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
>  
>  #if !defined ARM_ASSUME_NO_IWMMXT || defined __SOFTFP__
>  # define NEED_HWCAP 1
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] ARM: Add SystemTap probes to longjmp and setjmp.
  2014-02-06 16:26 ` Carlos O'Donell
@ 2014-02-06 16:41   ` Joseph S. Myers
  2014-02-06 16:48   ` Will Newton
  1 sibling, 0 replies; 11+ messages in thread
From: Joseph S. Myers @ 2014-02-06 16:41 UTC (permalink / raw)
  To: Carlos O'Donell; +Cc: Will Newton, libc-ports, patches

On Thu, 6 Feb 2014, Carlos O'Donell wrote:

> This looks good to me.
> 
> If it's considered a bug, please check this in immediately and CC Allan
> to keep him in the loop for these last minute bug fixes.

It's certainly not a bug fix and is completely inappropriate for 2.19.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] ARM: Add SystemTap probes to longjmp and setjmp.
  2014-02-06 16:26 ` Carlos O'Donell
  2014-02-06 16:41   ` Joseph S. Myers
@ 2014-02-06 16:48   ` Will Newton
  2014-02-06 16:54     ` Carlos O'Donell
  1 sibling, 1 reply; 11+ messages in thread
From: Will Newton @ 2014-02-06 16:48 UTC (permalink / raw)
  To: Carlos O'Donell; +Cc: libc-ports, Patch Tracking

On 6 February 2014 16:26, Carlos O'Donell <carlos@redhat.com> wrote:
> On 02/05/2014 04:56 AM, 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 points 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.
>>
>> 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.
>
> This looks good to me.
>
> If it's considered a bug, please check this in immediately and CC Allan
> to keep him in the loop for these last minute bug fixes.
>
> We are about to freeze so the 2.19 branch can be cut. If there is anything
> else like this please bring it to his attention immediately.

Sorry I should have been clearer, I am proposing this patch for 2.20.

It is a regression of functionality to add pointer encryption without
this patch as gdb now cannot deal with longjmp calls as well as it
could previously but I agree with Jospeh that it's not appropriate to
push a change of this nature so late in the freeze.

-- 
Will Newton
Toolchain Working Group, Linaro

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] ARM: Add SystemTap probes to longjmp and setjmp.
  2014-02-06 16:48   ` Will Newton
@ 2014-02-06 16:54     ` Carlos O'Donell
  0 siblings, 0 replies; 11+ messages in thread
From: Carlos O'Donell @ 2014-02-06 16:54 UTC (permalink / raw)
  To: Will Newton; +Cc: libc-ports, Patch Tracking

On 02/06/2014 11:48 AM, Will Newton wrote:
> On 6 February 2014 16:26, Carlos O'Donell <carlos@redhat.com> wrote:
>> On 02/05/2014 04:56 AM, 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 points 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.
>>>
>>> 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.
>>
>> This looks good to me.
>>
>> If it's considered a bug, please check this in immediately and CC Allan
>> to keep him in the loop for these last minute bug fixes.
>>
>> We are about to freeze so the 2.19 branch can be cut. If there is anything
>> else like this please bring it to his attention immediately.
> 
> Sorry I should have been clearer, I am proposing this patch for 2.20.
> 
> It is a regression of functionality to add pointer encryption without
> this patch as gdb now cannot deal with longjmp calls as well as it
> could previously but I agree with Jospeh that it's not appropriate to
> push a change of this nature so late in the freeze.

As long as gdb isn't broken by not having the patch then you are
correct that this is inappropriate for 2.19.

Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] ARM: Add SystemTap probes to longjmp and setjmp.
  2014-02-05  9:56 [PATCH v2] ARM: Add SystemTap probes to longjmp and setjmp Will Newton
  2014-02-06 16:26 ` Carlos O'Donell
@ 2014-02-06 22:11 ` Roland McGrath
  2014-02-07 12:38   ` Will Newton
  1 sibling, 1 reply; 11+ messages in thread
From: Roland McGrath @ 2014-02-06 22:11 UTC (permalink / raw)
  To: Will Newton; +Cc: libc-ports, patches

Can you clarify why you want to change the jmp_buf layout?
I don't see why any change you might want to make would necessitate that.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] ARM: Add SystemTap probes to longjmp and setjmp.
  2014-02-06 22:11 ` Roland McGrath
@ 2014-02-07 12:38   ` Will Newton
  2014-02-07 14:16     ` Andreas Schwab
  0 siblings, 1 reply; 11+ messages in thread
From: Will Newton @ 2014-02-07 12:38 UTC (permalink / raw)
  To: Roland McGrath; +Cc: libc-ports, Patch Tracking

On 6 February 2014 22:11, Roland McGrath <roland@hack.frob.com> wrote:
> Can you clarify why you want to change the jmp_buf layout?
> I don't see why any change you might want to make would necessitate that.

In order to have the correct registers live I need to free up a temp
register which involves loading lr and sp earlier than the
call-preserved register set. I could do this by offsetting into the
jmpbuf but it seems more natural just to move lr and sp to the front
of the jmp_buf and load registers in ascending order. As far as I am
aware the layout of jmp_buf is completely opaque so I don't see why
this would cause any problems?

-- 
Will Newton
Toolchain Working Group, Linaro

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] ARM: Add SystemTap probes to longjmp and setjmp.
  2014-02-07 12:38   ` Will Newton
@ 2014-02-07 14:16     ` Andreas Schwab
  2014-02-07 15:45       ` Jonathan S. Shapiro
                         ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Andreas Schwab @ 2014-02-07 14:16 UTC (permalink / raw)
  To: Will Newton; +Cc: Roland McGrath, libc-ports, Patch Tracking

Will Newton <will.newton@linaro.org> writes:

> In order to have the correct registers live I need to free up a temp
> register which involves loading lr and sp earlier than the
> call-preserved register set. I could do this by offsetting into the
> jmpbuf but it seems more natural just to move lr and sp to the front
> of the jmp_buf and load registers in ascending order. As far as I am
> aware the layout of jmp_buf is completely opaque so I don't see why
> this would cause any problems?

According to ports/sysdeps/arm/bits/setjmp.h the layout of jmp_buf is
part of the ABI.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] ARM: Add SystemTap probes to longjmp and setjmp.
  2014-02-07 14:16     ` Andreas Schwab
@ 2014-02-07 15:45       ` Jonathan S. Shapiro
  2014-02-07 17:04       ` Joseph S. Myers
       [not found]       ` <CAAP=3QP6_TvyFdpmO9Or5E2=NFCdcUVrCGBHT3rMozRXLT4mmw@mail.gmail.com>
  2 siblings, 0 replies; 11+ messages in thread
From: Jonathan S. Shapiro @ 2014-02-07 15:45 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Will Newton, Roland McGrath, libc-ports, Patch Tracking

On Fri, Feb 7, 2014 at 6:16 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> According to ports/sysdeps/arm/bits/setjmp.h the layout of jmp_buf is
> part of the ABI.

Yes. The layout of jmp_buf is part of the ABI. And not just the libc ABI,
but for some platforms it's part of the platform standard (e.g. SVID
specifies it). In consequence, changing the structure layout isn't an
option in either the current libc release or any foreseeable future libc
release. The patch point needs to be reworked so as to operate given the
current structure layout.

On the bright side, reworking the code to fit the current layout means that
(a) this can be integrated sooner, and (b) we don't have to accept a small
regression in GDB functionality in order to implement encrypted pointers.


Jonathan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] ARM: Add SystemTap probes to longjmp and setjmp.
  2014-02-07 14:16     ` Andreas Schwab
  2014-02-07 15:45       ` Jonathan S. Shapiro
@ 2014-02-07 17:04       ` Joseph S. Myers
       [not found]       ` <CAAP=3QP6_TvyFdpmO9Or5E2=NFCdcUVrCGBHT3rMozRXLT4mmw@mail.gmail.com>
  2 siblings, 0 replies; 11+ messages in thread
From: Joseph S. Myers @ 2014-02-07 17:04 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Will Newton, Roland McGrath, libc-ports, Patch Tracking

On Fri, 7 Feb 2014, Andreas Schwab wrote:

> Will Newton <will.newton@linaro.org> writes:
> 
> > In order to have the correct registers live I need to free up a temp
> > register which involves loading lr and sp earlier than the
> > call-preserved register set. I could do this by offsetting into the
> > jmpbuf but it seems more natural just to move lr and sp to the front
> > of the jmp_buf and load registers in ascending order. As far as I am
> > aware the layout of jmp_buf is completely opaque so I don't see why
> > this would cause any problems?
> 
> According to ports/sysdeps/arm/bits/setjmp.h the layout of jmp_buf is
> part of the ABI.

No, it's not.  The ABI requirement is 8-byte alignment.  I'm not sure 
where "recommends that the buffer contain 64 words" comes from; it doesn't 
seem to be in the current version of CLIBABI, or any version I checked 
back to 2005, but maybe it was in some prerelease version.  "The first 27 
words are occupied by" is a statement of fact[*], that any patch changing 
the layout needs to update, not a statement of an ABI requirement.  
CLIBABI says "The type and size of jmp_buf are defined by setjmp.h. Its 
contents are opaque, so setjmp and longjmp must be from the same library, 
and called out of line.", and that opaqueness leaves us free to change the 
layout between releases, if there is some reason to do so - setjmp and 
longjmp always need to come from the same library.

[*] The references to 27 and 17 are inaccurate - we haven't been using 
fstmx since:

2009-10-22  Andrew Stubbs  <ams@codesourcery.com>
            Julian Brown  <julian@codesourcery.com>

        * sysdeps/arm/eabi/setjmp.S (__sigsetjmp): Replace deprecated
        instruction fstmiax with vstmia.
        Correct register conflict and comment.
        * sysdeps/arm/eabi/__longjmp.S (__longjmp): Use vldmia not fldmiax.
        Don't clobber r1/a2 register before testing IWMMXT hwcap.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] ARM: Add SystemTap probes to longjmp and setjmp.
       [not found]       ` <CAAP=3QP6_TvyFdpmO9Or5E2=NFCdcUVrCGBHT3rMozRXLT4mmw@mail.gmail.com>
@ 2014-02-10  8:54         ` Will Newton
  0 siblings, 0 replies; 11+ messages in thread
From: Will Newton @ 2014-02-10  8:54 UTC (permalink / raw)
  To: Jonathan S. Shapiro
  Cc: Andreas Schwab, Roland McGrath, libc-ports, Patch Tracking

On 7 February 2014 15:27, Jonathan S. Shapiro <shap@eros-os.org> wrote:

> Yes. The layout of jmp_buf is part of the ABI. And not just the libc ABI,
> but for some platforms it's part of the platform standard (e.g. SVID
> specifies it). In consequence, changing the structure layout isn't an option
> in either the current libc release or any foreseeable future libc release.
> The patch point needs to be reworked so as to operate given the current
> structure layout.

The layout has changed in previous releases so I am not convinced this is true.

> On the bright side, reworking the code to fit the current layout means that
> (a) this can be integrated sooner, and (b) we don't have to accept a small
> regression in GDB functionality in order to implement encrypted pointers.

The GDB functionality regression is orthogonal to the layout change.
The gdb issue is caused by the fact that the saved lr in the jmp_buf
is now no longer a straightforward code pointer. The longjmp SystemTap
probe added by this patch unencrypts lr and hands it to gdb so gdb can
set a break there.

-- 
Will Newton
Toolchain Working Group, Linaro

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2014-02-10  8:54 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-05  9:56 [PATCH v2] ARM: Add SystemTap probes to longjmp and setjmp Will Newton
2014-02-06 16:26 ` Carlos O'Donell
2014-02-06 16:41   ` Joseph S. Myers
2014-02-06 16:48   ` Will Newton
2014-02-06 16:54     ` Carlos O'Donell
2014-02-06 22:11 ` Roland McGrath
2014-02-07 12:38   ` Will Newton
2014-02-07 14:16     ` Andreas Schwab
2014-02-07 15:45       ` Jonathan S. Shapiro
2014-02-07 17:04       ` Joseph S. Myers
     [not found]       ` <CAAP=3QP6_TvyFdpmO9Or5E2=NFCdcUVrCGBHT3rMozRXLT4mmw@mail.gmail.com>
2014-02-10  8:54         ` Will Newton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).