| Message ID | 20230410195907.4123869-8-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix Race conditions in pthread cancellation [BZ#12683] | expand |
On 4/10/23 2:58 PM, Adhemerval Zanella via Libc-alpha wrote: > By adding the required syscall_cancel.S. > > Checked on powerpc64le-linux-gnu, powerpc64-linux-gnu and > powerpc-linux-gnu. > --- Thanks. A couple trivial comments, but otherwise LGTM. Reviewed-by: Paul E. Murphy <murphyp@linux.ibm.com> > +ENTRY (__syscall_cancel_arch) > + > + .globl __syscall_cancel_arch_start > +__syscall_cancel_arch_start: > + > + /* if (*cancelhandling & CANCELED_BITMASK) > + __syscall_do_cancel() */ > + lwz r0,0(r3) > + andi. r0,r0,TCB_CANCELED_BITMASK > + bne- 1f Really trivial opinion nit, the branch hint should be removed. > + > + /* Issue a 6 argument syscall, the nr [r4] being the syscall > + number. */ > + mr r0,r4 > + mr r3,r5 > + mr r4,r6 > + mr r5,r7 > + mr r6,r8 > + mr r7,r9 > + mr r8,r10 > + sc For consistency with the other syscall usage, can this also use the appropriate wrappers for using scv? > + > + .globl __syscall_cancel_arch_end > +__syscall_cancel_arch_end: > + > + bnslr+ > + neg r3,r3 > + blr > + > + /* Although the __syscall_do_cancel do not return, we need to stack > + being set correctly for unwind. */ > +1: > + TAIL_CALL_NO_RETURN (__syscall_do_cancel
On 14/04/23 12:27, Paul E Murphy wrote: > > > On 4/10/23 2:58 PM, Adhemerval Zanella via Libc-alpha wrote: >> By adding the required syscall_cancel.S. >> >> Checked on powerpc64le-linux-gnu, powerpc64-linux-gnu and >> powerpc-linux-gnu. >> --- > Thanks. A couple trivial comments, but otherwise LGTM. > > Reviewed-by: Paul E. Murphy <murphyp@linux.ibm.com> > >> +ENTRY (__syscall_cancel_arch) >> + >> + .globl __syscall_cancel_arch_start >> +__syscall_cancel_arch_start: >> + >> + /* if (*cancelhandling & CANCELED_BITMASK) >> + __syscall_do_cancel() */ >> + lwz r0,0(r3) >> + andi. r0,r0,TCB_CANCELED_BITMASK >> + bne- 1f > Really trivial opinion nit, the branch hint should be removed. Ack. > >> + >> + /* Issue a 6 argument syscall, the nr [r4] being the syscall >> + number. */ >> + mr r0,r4 >> + mr r3,r5 >> + mr r4,r6 >> + mr r5,r7 >> + mr r6,r8 >> + mr r7,r9 >> + mr r8,r10 >> + sc > For consistency with the other syscall usage, can this also use the appropriate wrappers for using scv? That is tricky for similar reasons i386 and i64 have to use the old syscall mechanism instead of the vDSO: the cancel syscall bridge need to have a mark just after the syscall instruction to know whether it has any side-effects. Having two syscall mechanism means essentially two marks depending of the syscall used. I think it should be possible to two two marks for powerpc, one for default 'sc' and another for 'svc'. It would require an arch-specific cancellation-pc-check.h for powerpc. > >> + >> + .globl __syscall_cancel_arch_end >> +__syscall_cancel_arch_end: >> + >> + bnslr+ >> + neg r3,r3 >> + blr >> + >> + /* Although the __syscall_do_cancel do not return, we need to stack >> + being set correctly for unwind. */ >> +1: >> + TAIL_CALL_NO_RETURN (__syscall_do_cancel
diff --git a/sysdeps/powerpc/powerpc32/sysdep.h b/sysdeps/powerpc/powerpc32/sysdep.h index 095a726765..df67e3516a 100644 --- a/sysdeps/powerpc/powerpc32/sysdep.h +++ b/sysdeps/powerpc/powerpc32/sysdep.h @@ -104,6 +104,9 @@ GOT_LABEL: ; \ # define JUMPTARGET(name) name #endif +#define TAIL_CALL_NO_RETURN(__func) \ + b __func@local + #if defined SHARED && defined PIC && !defined NO_HIDDEN # undef HIDDEN_JUMPTARGET # define HIDDEN_JUMPTARGET(name) __GI_##name##@local diff --git a/sysdeps/powerpc/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h index ce92d8b3d2..1815131dc2 100644 --- a/sysdeps/powerpc/powerpc64/sysdep.h +++ b/sysdeps/powerpc/powerpc64/sysdep.h @@ -352,6 +352,25 @@ LT_LABELSUFFIX(name,_name_end): ; \ ENTRY (name); \ DO_CALL (SYS_ify (syscall_name)) +#ifdef SHARED +# define TAIL_CALL_NO_RETURN(__func) \ + b JUMPTARGET(__func) +#else +# define TAIL_CALL_NO_RETURN(__func) \ + .ifdef .Local ## __func; \ + b .Local ## __func; \ + .else; \ +.Local ## __func: \ + mflr 0; \ + std 0,FRAME_LR_SAVE(1); \ + stdu 1,-FRAME_MIN_SIZE(1); \ + cfi_adjust_cfa_offset(FRAME_MIN_SIZE); \ + cfi_offset(lr,FRAME_LR_SAVE); \ + bl JUMPTARGET(__func); \ + nop; \ + .endif +#endif + #ifdef SHARED #define TAIL_CALL_SYSCALL_ERROR \ b JUMPTARGET (NOTOC (__syscall_error)) diff --git a/sysdeps/unix/sysv/linux/powerpc/syscall_cancel.S b/sysdeps/unix/sysv/linux/powerpc/syscall_cancel.S new file mode 100644 index 0000000000..7aa2b77caa --- /dev/null +++ b/sysdeps/unix/sysv/linux/powerpc/syscall_cancel.S @@ -0,0 +1,65 @@ +/* Cancellable syscall wrapper. Linux/powerpc version. + Copyright (C) 2023 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <sysdep.h> +#include <descr-const.h> + +/* long int [r3] __syscall_cancel_arch (int *cancelhandling [r3], + long int nr [r4], + long int arg1 [r5], + long int arg2 [r6], + long int arg3 [r7], + long int arg4 [r8], + long int arg5 [r9], + long int arg6 [r10]) */ + +ENTRY (__syscall_cancel_arch) + + .globl __syscall_cancel_arch_start +__syscall_cancel_arch_start: + + /* if (*cancelhandling & CANCELED_BITMASK) + __syscall_do_cancel() */ + lwz r0,0(r3) + andi. r0,r0,TCB_CANCELED_BITMASK + bne- 1f + + /* Issue a 6 argument syscall, the nr [r4] being the syscall + number. */ + mr r0,r4 + mr r3,r5 + mr r4,r6 + mr r5,r7 + mr r6,r8 + mr r7,r9 + mr r8,r10 + sc + + .globl __syscall_cancel_arch_end +__syscall_cancel_arch_end: + + bnslr+ + neg r3,r3 + blr + + /* Although the __syscall_do_cancel do not return, we need to stack + being set correctly for unwind. */ +1: + TAIL_CALL_NO_RETURN (__syscall_do_cancel) + +END (__syscall_cancel_arch)