[v2,1/3] aarch64: Re-implement setcontext without rt_sigreturn syscall

Message ID 1396351967-1952-1-git-send-email-will.newton@linaro.org
State Accepted
Headers show

Commit Message

Will Newton April 1, 2014, 11:32 a.m.
The current implementation of setcontext uses rt_sigreturn to restore
the contents of registers. This contrasts with the way most other
architectures implement setcontext:

  powerpc64, mips, tile:

  Call rt_sigreturn if context was created by a call to a signal handler,
  otherwise restore in user code.

  powerpc32:

  Call swapcontext system call and don't call sigreturn or rt_sigreturn.

  x86_64, sparc, hppa, sh, ia64, m68k, s390, arm:

  Only support restoring "synchronous" contexts, that is contexts
  created by getcontext, and restoring in user code and don't call
  sigreturn or rt_sigreturn.

  alpha:

  Call sigreturn (but not rt_sigreturn) in all cases to do the restore.

The text of the setcontext manpage suggests that the requirement to be
able to restore a signal handler created context has been dropped from
SUSv2:

  If  the context was obtained by a call to a signal handler, then old
  standard text says that "program execution continues with the program
  instruction following the instruction interrupted by the signal".
  However, this sentence was removed in SUSv2, and the present verdict
  is "the result is unspecified".

Implementing setcontext by calling rt_sigreturn unconditionally causes
problems when used with sigaltstack as in BZ #16629. On this basis it
seems that aarch64 is broken and that new ports should only support
restoring contexts created with getcontext and do not need to call
rt_sigreturn at all.

This patch re-implements the aarch64 setcontext function to restore
the context in user code in a similar manner to x86_64 and other ports.

ChangeLog:

2014-04-01  Will Newton  <will.newton@linaro.org>

	[BZ #16629]
	* sysdeps/unix/sysv/linux/aarch64/setcontext.S (__setcontext):
	Re-implement to restore registers in user code and avoid
	rt_sigreturn system call.
---
 NEWS                                         |   6 +-
 sysdeps/unix/sysv/linux/aarch64/setcontext.S | 147 +++++++++++++++++----------
 2 files changed, 95 insertions(+), 58 deletions(-)

Changes in v2:
 - Added NEWS entry
 - Fixed bug in handling sigmask value
 - Improved commit message

Comments

Will Newton April 17, 2014, 8:35 a.m. | #1
On 1 April 2014 12:32, Will Newton <will.newton@linaro.org> wrote:
> The current implementation of setcontext uses rt_sigreturn to restore
> the contents of registers. This contrasts with the way most other
> architectures implement setcontext:
>
>   powerpc64, mips, tile:
>
>   Call rt_sigreturn if context was created by a call to a signal handler,
>   otherwise restore in user code.
>
>   powerpc32:
>
>   Call swapcontext system call and don't call sigreturn or rt_sigreturn.
>
>   x86_64, sparc, hppa, sh, ia64, m68k, s390, arm:
>
>   Only support restoring "synchronous" contexts, that is contexts
>   created by getcontext, and restoring in user code and don't call
>   sigreturn or rt_sigreturn.
>
>   alpha:
>
>   Call sigreturn (but not rt_sigreturn) in all cases to do the restore.
>
> The text of the setcontext manpage suggests that the requirement to be
> able to restore a signal handler created context has been dropped from
> SUSv2:
>
>   If  the context was obtained by a call to a signal handler, then old
>   standard text says that "program execution continues with the program
>   instruction following the instruction interrupted by the signal".
>   However, this sentence was removed in SUSv2, and the present verdict
>   is "the result is unspecified".
>
> Implementing setcontext by calling rt_sigreturn unconditionally causes
> problems when used with sigaltstack as in BZ #16629. On this basis it
> seems that aarch64 is broken and that new ports should only support
> restoring contexts created with getcontext and do not need to call
> rt_sigreturn at all.
>
> This patch re-implements the aarch64 setcontext function to restore
> the context in user code in a similar manner to x86_64 and other ports.
>
> ChangeLog:
>
> 2014-04-01  Will Newton  <will.newton@linaro.org>
>
>         [BZ #16629]
>         * sysdeps/unix/sysv/linux/aarch64/setcontext.S (__setcontext):
>         Re-implement to restore registers in user code and avoid
>         rt_sigreturn system call.
> ---
>  NEWS                                         |   6 +-
>  sysdeps/unix/sysv/linux/aarch64/setcontext.S | 147 +++++++++++++++++----------
>  2 files changed, 95 insertions(+), 58 deletions(-)
>
> Changes in v2:
>  - Added NEWS entry
>  - Fixed bug in handling sigmask value
>  - Improved commit message

Ping?

> diff --git a/NEWS b/NEWS
> index 1574ab4..219198d 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -11,9 +11,9 @@ Version 2.20
>
>    6804, 15347, 15804, 15894, 16002, 16198, 16284, 16348, 16349, 16357,
>    16362, 16447, 16532, 16545, 16574, 16599, 16600, 16609, 16610, 16611,
> -  16613, 16623, 16632, 16634, 16639, 16642, 16648, 16649, 16670, 16674,
> -  16677, 16680, 16683, 16689, 16695, 16701, 16706, 16707, 16712, 16713,
> -  16714, 16731, 16743, 16758, 16759, 16760, 16770.
> +  16613, 16623, 16629, 16632, 16634, 16639, 16642, 16648, 16649, 16670,
> +  16674, 16677, 16680, 16683, 16689, 16695, 16701, 16706, 16707, 16712,
> +  16713, 16714, 16731, 16743, 16758, 16759, 16760, 16770.
>
>  * Running the testsuite no longer terminates as soon as a test fails.
>    Instead, a file tests.sum (xtests.sum from "make xcheck") is generated,
> diff --git a/sysdeps/unix/sysv/linux/aarch64/setcontext.S b/sysdeps/unix/sysv/linux/aarch64/setcontext.S
> index d220c41..aef5149 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/setcontext.S
> +++ b/sysdeps/unix/sysv/linux/aarch64/setcontext.S
> @@ -22,68 +22,105 @@
>  #include "ucontext_i.h"
>  #include "ucontext-internal.h"
>
> -/* int setcontext (const ucontext_t *ucp) */
> +/*  int __setcontext (const ucontext_t *ucp)
>
> -       .text
> -
> -ENTRY(__setcontext)
> -
> -       /* Create a signal frame on the stack:
> -
> -               fp
> -               lr
> -               ...
> -          sp-> rt_sigframe
> -        */
> -
> -       stp     x29, x30, [sp, -16]!
> -       cfi_adjust_cfa_offset (16)
> -       cfi_rel_offset (x29, 0)
> -       cfi_rel_offset (x30, 8)
> -
> -        mov     x29, sp
> -       cfi_def_cfa_register (x29)
> -
> -       /* Allocate space for the sigcontext.  */
> -       mov     w3, #((RT_SIGFRAME_SIZE + SP_ALIGN_SIZE) & SP_ALIGN_MASK)
> -       sub     sp, sp, x3
> +  Restores the machine context in UCP and thereby resumes execution
> +  in that context.
>
> -       /* Compute the base address of the ucontext structure.  */
> -       add     x1, sp, #RT_SIGFRAME_UCONTEXT
> +  This implementation is intended to be used for *synchronous* context
> +  switches only.  Therefore, it does not have to restore anything
> +  other than the PRESERVED state.  */
>
> -       /* Only ucontext is required in the frame, *copy* it in.  */
> -
> -#if UCONTEXT_SIZE % 16
> -#error The implementation of setcontext.S assumes sizeof(ucontext_t) % 16 == 0
> -#endif
> -
> -       mov     x2, #UCONTEXT_SIZE / 16
> -0:
> -       ldp     x3, x4, [x0], #16
> -       stp     x3, x4, [x1], #16
> -       sub     x2, x2, 1
> -       cbnz    x2, 0b
> +       .text
>
> -       /* rt_sigreturn () -- no arguments, sp points to struct rt_sigframe.  */
> -       mov     x8, SYS_ify (rt_sigreturn)
> +ENTRY (__setcontext)
> +       /* Save a copy of UCP.  */
> +       mov     x9, x0
> +
> +       /* Set the signal mask with
> +          rt_sigprocmask (SIG_SETMASK, mask, NULL, _NSIG/8).  */
> +       mov     x0, #SIG_SETMASK
> +       add     x1, x9, #UCONTEXT_SIGMASK
> +       mov     x2, #0
> +       mov     x3, #_NSIG8
> +       mov     x8, SYS_ify (rt_sigprocmask)
>         svc     0
> -
> -       /* Ooops we failed.  Recover the stack */
> -
> -       mov     sp, x29
> -       cfi_def_cfa_register (sp)
> -
> -        ldp     x29, x30, [sp], 16
> -       cfi_adjust_cfa_offset (16)
> -       cfi_restore (x29)
> -       cfi_restore (x30)
> -       b       C_SYMBOL_NAME(__syscall_error)
> -
> +       cbz     x0, 1f
> +       b       C_SYMBOL_NAME (__syscall_error)
> +1:
> +       /* Restore the general purpose registers.  */
> +       mov     x0, x9
> +       cfi_def_cfa (x0, 0)
> +       cfi_offset (x18, oX0 + 18 * SZREG)
> +       cfi_offset (x19, oX0 + 19 * SZREG)
> +       cfi_offset (x20, oX0 + 20 * SZREG)
> +       cfi_offset (x21, oX0 + 21 * SZREG)
> +       cfi_offset (x22, oX0 + 22 * SZREG)
> +       cfi_offset (x23, oX0 + 23 * SZREG)
> +       cfi_offset (x24, oX0 + 24 * SZREG)
> +       cfi_offset (x25, oX0 + 25 * SZREG)
> +       cfi_offset (x26, oX0 + 26 * SZREG)
> +       cfi_offset (x27, oX0 + 27 * SZREG)
> +       cfi_offset (x28, oX0 + 28 * SZREG)
> +       cfi_offset (x29, oX0 + 29 * SZREG)
> +       cfi_offset (x30, oX0 + 30 * SZREG)
> +
> +       cfi_offset ( d8, oV0 + 8 * SZVREG)
> +       cfi_offset ( d9, oV0 + 9 * SZVREG)
> +       cfi_offset (d10, oV0 + 10 * SZVREG)
> +       cfi_offset (d11, oV0 + 11 * SZVREG)
> +       cfi_offset (d12, oV0 + 12 * SZVREG)
> +       cfi_offset (d13, oV0 + 13 * SZVREG)
> +       cfi_offset (d14, oV0 + 14 * SZVREG)
> +       cfi_offset (d15, oV0 + 15 * SZVREG)
> +       ldp     x18, x19, [x0, oX0 + 18 * SZREG]
> +       ldp     x20, x21, [x0, oX0 + 20 * SZREG]
> +       ldp     x22, x23, [x0, oX0 + 22 * SZREG]
> +       ldp     x24, x25, [x0, oX0 + 24 * SZREG]
> +       ldp     x26, x27, [x0, oX0 + 26 * SZREG]
> +       ldp     x28, x29, [x0, oX0 + 28 * SZREG]
> +       ldr     x30,      [x0, oX0 + 30 * SZREG]
> +       ldr     x2, [x0, oSP]
> +       mov     sp, x2
> +
> +       /* Check for FP SIMD context.  */
> +       add     x2, x0, #oEXTENSION
> +
> +       mov     w3, #(FPSIMD_MAGIC & 0xffff)
> +       movk    w3, #(FPSIMD_MAGIC >> 16), lsl #16
> +       ldr     w1, [x2, #oHEAD + oMAGIC]
> +       cmp     w1, w3
> +       b.ne    2f
> +
> +       /* Restore the FP SIMD context.  */
> +       add     x3, x2, #oV0 + 8 * SZVREG
> +       ldp      d8,  d9, [x3], #2 * SZVREG
> +       ldp     d10, d11, [x3], #2 * SZVREG
> +       ldp     d12, d13, [x3], #2 * SZVREG
> +       ldp     d14, d15, [x3], #2 * SZVREG
> +
> +       add     x3, x2, oFPSR
> +
> +       ldr     w4, [x3]
> +       msr     fpsr, x4
> +
> +       ldr     w4, [x3, oFPCR - oFPSR]
> +       msr     fpcr, x4
> +
> +2:
> +       ldr     x16, [x0, oPC]
> +       /* Restore arg registers.  */
> +       ldp     x2, x3, [x0, oX0 + 2 * SZREG]
> +       ldp     x4, x5, [x0, oX0 + 4 * SZREG]
> +       ldp     x6, x7, [x0, oX0 + 6 * SZREG]
> +       ldp     x0, x1, [x0, oX0 + 0 * SZREG]
> +       /* Jump to the new pc value.  */
> +       br      x16
>  PSEUDO_END (__setcontext)
>  weak_alias (__setcontext, setcontext)
>
> -ENTRY(__startcontext)
> +ENTRY (__startcontext)
>         mov     x0, x19
>         cbnz    x0, __setcontext
> -1:     b       HIDDEN_JUMPTARGET(_exit)
> -END(__startcontext)
> +1:     b       HIDDEN_JUMPTARGET (_exit)
> +END (__startcontext)
> --
> 1.8.1.4
>
Marcus Shawcroft April 17, 2014, 9:02 a.m. | #2
On 1 April 2014 12:32, Will Newton <will.newton@linaro.org> wrote:

> ChangeLog:
>
> 2014-04-01  Will Newton  <will.newton@linaro.org>
>
>         [BZ #16629]
>         * sysdeps/unix/sysv/linux/aarch64/setcontext.S (__setcontext):
>         Re-implement to restore registers in user code and avoid
>         rt_sigreturn system call.

OK, but, with reference to:

https://sourceware.org/ml/libc-alpha/2014-03/msg00594.html

would you mind adding a comment to the context recovery code spelling
out that we don;t handle kernel created contexts therefore it is safe
to assume a fixed context layout?

Cheers
/Marcus
Will Newton April 17, 2014, 10:42 a.m. | #3
On 17 April 2014 10:02, Marcus Shawcroft <marcus.shawcroft@gmail.com> wrote:
> On 1 April 2014 12:32, Will Newton <will.newton@linaro.org> wrote:
>
>> ChangeLog:
>>
>> 2014-04-01  Will Newton  <will.newton@linaro.org>
>>
>>         [BZ #16629]
>>         * sysdeps/unix/sysv/linux/aarch64/setcontext.S (__setcontext):
>>         Re-implement to restore registers in user code and avoid
>>         rt_sigreturn system call.
>
> OK, but, with reference to:
>
> https://sourceware.org/ml/libc-alpha/2014-03/msg00594.html
>
> would you mind adding a comment to the context recovery code spelling
> out that we don;t handle kernel created contexts therefore it is safe
> to assume a fixed context layout?

Thanks Marcus, committed with a comment added.

Patch

diff --git a/NEWS b/NEWS
index 1574ab4..219198d 100644
--- a/NEWS
+++ b/NEWS
@@ -11,9 +11,9 @@  Version 2.20
 
   6804, 15347, 15804, 15894, 16002, 16198, 16284, 16348, 16349, 16357,
   16362, 16447, 16532, 16545, 16574, 16599, 16600, 16609, 16610, 16611,
-  16613, 16623, 16632, 16634, 16639, 16642, 16648, 16649, 16670, 16674,
-  16677, 16680, 16683, 16689, 16695, 16701, 16706, 16707, 16712, 16713,
-  16714, 16731, 16743, 16758, 16759, 16760, 16770.
+  16613, 16623, 16629, 16632, 16634, 16639, 16642, 16648, 16649, 16670,
+  16674, 16677, 16680, 16683, 16689, 16695, 16701, 16706, 16707, 16712,
+  16713, 16714, 16731, 16743, 16758, 16759, 16760, 16770.
 
 * Running the testsuite no longer terminates as soon as a test fails.
   Instead, a file tests.sum (xtests.sum from "make xcheck") is generated,
diff --git a/sysdeps/unix/sysv/linux/aarch64/setcontext.S b/sysdeps/unix/sysv/linux/aarch64/setcontext.S
index d220c41..aef5149 100644
--- a/sysdeps/unix/sysv/linux/aarch64/setcontext.S
+++ b/sysdeps/unix/sysv/linux/aarch64/setcontext.S
@@ -22,68 +22,105 @@ 
 #include "ucontext_i.h"
 #include "ucontext-internal.h"
 
-/* int setcontext (const ucontext_t *ucp) */
+/*  int __setcontext (const ucontext_t *ucp)
 
-	.text
-
-ENTRY(__setcontext)
-
-	/* Create a signal frame on the stack:
-
-		fp
-		lr
-		...
-	   sp-> rt_sigframe
-	 */
-
-	stp     x29, x30, [sp, -16]!
-	cfi_adjust_cfa_offset (16)
-	cfi_rel_offset (x29, 0)
-	cfi_rel_offset (x30, 8)
-
-        mov     x29, sp
-	cfi_def_cfa_register (x29)
-
-	/* Allocate space for the sigcontext.  */
-	mov	w3, #((RT_SIGFRAME_SIZE + SP_ALIGN_SIZE) & SP_ALIGN_MASK)
-	sub	sp, sp,	x3
+  Restores the machine context in UCP and thereby resumes execution
+  in that context.
 
-	/* Compute the base address of the ucontext structure.  */
-	add	x1, sp, #RT_SIGFRAME_UCONTEXT
+  This implementation is intended to be used for *synchronous* context
+  switches only.  Therefore, it does not have to restore anything
+  other than the PRESERVED state.  */
 
-	/* Only ucontext is required in the frame, *copy* it in.  */
-
-#if UCONTEXT_SIZE % 16
-#error The implementation of setcontext.S assumes sizeof(ucontext_t) % 16 == 0
-#endif
-
-	mov	x2, #UCONTEXT_SIZE / 16
-0:
-	ldp	x3, x4, [x0], #16
-	stp	x3, x4, [x1], #16
-	sub	x2, x2, 1
-	cbnz	x2, 0b
+	.text
 
-	/* rt_sigreturn () -- no arguments, sp points to struct rt_sigframe.  */
-	mov	x8, SYS_ify (rt_sigreturn)
+ENTRY (__setcontext)
+	/* Save a copy of UCP.  */
+	mov	x9, x0
+
+	/* Set the signal mask with
+	   rt_sigprocmask (SIG_SETMASK, mask, NULL, _NSIG/8).  */
+	mov	x0, #SIG_SETMASK
+	add	x1, x9, #UCONTEXT_SIGMASK
+	mov	x2, #0
+	mov	x3, #_NSIG8
+	mov	x8, SYS_ify (rt_sigprocmask)
 	svc	0
-
-	/* Ooops we failed.  Recover the stack */
-
-	mov	sp, x29
-	cfi_def_cfa_register (sp)
-
-        ldp     x29, x30, [sp], 16
-	cfi_adjust_cfa_offset (16)
-	cfi_restore (x29)
-	cfi_restore (x30)
-	b	C_SYMBOL_NAME(__syscall_error)
-
+	cbz	x0, 1f
+	b	C_SYMBOL_NAME (__syscall_error)
+1:
+	/* Restore the general purpose registers.  */
+	mov	x0, x9
+	cfi_def_cfa (x0, 0)
+	cfi_offset (x18, oX0 + 18 * SZREG)
+	cfi_offset (x19, oX0 + 19 * SZREG)
+	cfi_offset (x20, oX0 + 20 * SZREG)
+	cfi_offset (x21, oX0 + 21 * SZREG)
+	cfi_offset (x22, oX0 + 22 * SZREG)
+	cfi_offset (x23, oX0 + 23 * SZREG)
+	cfi_offset (x24, oX0 + 24 * SZREG)
+	cfi_offset (x25, oX0 + 25 * SZREG)
+	cfi_offset (x26, oX0 + 26 * SZREG)
+	cfi_offset (x27, oX0 + 27 * SZREG)
+	cfi_offset (x28, oX0 + 28 * SZREG)
+	cfi_offset (x29, oX0 + 29 * SZREG)
+	cfi_offset (x30, oX0 + 30 * SZREG)
+
+	cfi_offset ( d8, oV0 + 8 * SZVREG)
+	cfi_offset ( d9, oV0 + 9 * SZVREG)
+	cfi_offset (d10, oV0 + 10 * SZVREG)
+	cfi_offset (d11, oV0 + 11 * SZVREG)
+	cfi_offset (d12, oV0 + 12 * SZVREG)
+	cfi_offset (d13, oV0 + 13 * SZVREG)
+	cfi_offset (d14, oV0 + 14 * SZVREG)
+	cfi_offset (d15, oV0 + 15 * SZVREG)
+	ldp	x18, x19, [x0, oX0 + 18 * SZREG]
+	ldp	x20, x21, [x0, oX0 + 20 * SZREG]
+	ldp	x22, x23, [x0, oX0 + 22 * SZREG]
+	ldp	x24, x25, [x0, oX0 + 24 * SZREG]
+	ldp	x26, x27, [x0, oX0 + 26 * SZREG]
+	ldp	x28, x29, [x0, oX0 + 28 * SZREG]
+	ldr     x30,      [x0, oX0 + 30 * SZREG]
+	ldr     x2, [x0, oSP]
+	mov	sp, x2
+
+	/* Check for FP SIMD context.  */
+	add     x2, x0, #oEXTENSION
+
+	mov	w3, #(FPSIMD_MAGIC & 0xffff)
+	movk	w3, #(FPSIMD_MAGIC >> 16), lsl #16
+	ldr	w1, [x2, #oHEAD + oMAGIC]
+	cmp	w1, w3
+	b.ne	2f
+
+	/* Restore the FP SIMD context.  */
+	add	x3, x2, #oV0 + 8 * SZVREG
+	ldp	 d8,  d9, [x3], #2 * SZVREG
+	ldp	d10, d11, [x3], #2 * SZVREG
+	ldp	d12, d13, [x3], #2 * SZVREG
+	ldp	d14, d15, [x3], #2 * SZVREG
+
+	add	x3, x2, oFPSR
+
+	ldr	w4, [x3]
+	msr	fpsr, x4
+
+	ldr	w4, [x3, oFPCR - oFPSR]
+	msr	fpcr, x4
+
+2:
+	ldr     x16, [x0, oPC]
+	/* Restore arg registers.  */
+	ldp	x2, x3, [x0, oX0 + 2 * SZREG]
+	ldp	x4, x5, [x0, oX0 + 4 * SZREG]
+	ldp	x6, x7, [x0, oX0 + 6 * SZREG]
+	ldp	x0, x1, [x0, oX0 + 0 * SZREG]
+	/* Jump to the new pc value.  */
+	br	x16
 PSEUDO_END (__setcontext)
 weak_alias (__setcontext, setcontext)
 
-ENTRY(__startcontext)
+ENTRY (__startcontext)
 	mov	x0, x19
 	cbnz	x0, __setcontext
-1:	b       HIDDEN_JUMPTARGET(_exit)
-END(__startcontext)
+1:	b       HIDDEN_JUMPTARGET (_exit)
+END (__startcontext)