[v5,07/22] powerpc: Fix Race conditions in pthread cancellation [BZ#12683]
Checks
| Context |
Check |
Description |
| dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
Commit Message
By adding the required syscall_cancel.S.
Checked on powerpc64le-linux-gnu, powerpc64-linux-gnu and
powerpc-linux-gnu.
---
sysdeps/powerpc/powerpc32/sysdep.h | 3 +
sysdeps/powerpc/powerpc64/sysdep.h | 19 ++++++
.../unix/sysv/linux/powerpc/syscall_cancel.S | 65 +++++++++++++++++++
3 files changed, 87 insertions(+)
create mode 100644 sysdeps/unix/sysv/linux/powerpc/syscall_cancel.S
Comments
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
@@ -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
@@ -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))
new file mode 100644
@@ -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)