| Message ID | 1396351967-1952-1-git-send-email-will.newton@linaro.org |
|---|---|
| State | Accepted |
| Headers | show |
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 >
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
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.
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)