This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 07/08] nptl: aarch64: Fix Race conditions in pthread cancellation (BZ#12683)
- From: Szabolcs Nagy <szabolcs dot nagy at arm dot com>
- To: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>, GNU C Library <libc-alpha at sourceware dot org>
- Date: Mon, 29 Jun 2015 11:32:42 +0100
- Subject: Re: [PATCH 07/08] nptl: aarch64: Fix Race conditions in pthread cancellation (BZ#12683)
- Authentication-results: sourceware.org; auth=none
- References: <558DABCC dot 3020109 at linaro dot org>
On 26/06/15 20:45, Adhemerval Zanella wrote:
> +/* long int [r0] __syscall_cancel_arch (int *cancelhandling [x0],
> + long int nr [x1],
> + long int arg1 [x2],
> + long int arg2 [x3],
> + long int arg3 [x4],
> + long int arg4 [x5],
> + long int arg5 [x6],
> + long int arg6 [x7]) */
> +
> +ENTRY (__syscall_cancel_arch)
> +
> + stp x29, x30, [sp, -16]!
> + cfi_def_cfa_offset (16)
> + cfi_offset (29, -16)
> + cfi_offset (30, -8)
> + add x29, sp, 0
> + cfi_def_cfa_register (29)
> +
you save things on the stack here ...
> + .globl __syscall_cancel_arch_start
> + .type __syscall_cancel_arch_start,@function
> +__syscall_cancel_arch_start:
> +
> + /* if (*cancelhandling & CANCELED_BITMASK)
> + __syscall_do_cancel() */
> + ldr w0, [x0]
> + tbnz w0, 2, 1f
> +
> + /* Issue a 6 argument syscall, the nr [x1] being the syscall
> + number. */
> + mov x8, x1
> + mov x0, x2
> + mov x1, x3
> + mov x2, x4
> + mov x3, x5
> + mov x4, x6
> + mov x5, x7
> + svc 0x0
> +
> + .globl __syscall_cancel_arch_end
> + .type __syscall_cancel_arch_end,@function
> +__syscall_cancel_arch_end:
> +
> + ldp x29, x30, [sp], 16
> + cfi_remember_state
> + cfi_restore (30)
> + cfi_restore (29)
> + cfi_def_cfa (31, 0)
> + ret
> +
> +1:
> + cfi_restore_state
> + b __syscall_do_cancel
> +
... and tail call into a function that does not restore the stack.
i think you can omit saving the frame pointer.
(neither syscall, nor tail call needs it).
> --- a/sysdeps/unix/sysv/linux/aarch64/sysdep-cancel.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/sysdep-cancel.h
> @@ -20,42 +20,50 @@
> #include <tls.h>
> #ifndef __ASSEMBLER__
> # include <nptl/pthreadP.h>
> +# include <sys/ucontext.h>
> #endif
>
> #if IS_IN (libc) || IS_IN (libpthread) || IS_IN (librt)
>
> +# if IS_IN (libc)
> +# define JMP_SYSCALL_CANCEL HIDDEN_JUMPTARGET(__syscall_cancel)
> +# else
> +# define JMP_SYSCALL_CANCEL __syscall_cancel
> +# endif
> +
> # undef PSEUDO
> # define PSEUDO(name, syscall_name, args) \
> - .section ".text"; \
> -ENTRY (__##syscall_name##_nocancel); \
> -.Lpseudo_nocancel: \
> - DO_CALL (syscall_name, args); \
> -.Lpseudo_finish: \
> - cmn x0, 4095; \
> - b.cs .Lsyscall_error; \
> - .subsection 2; \
> - .size __##syscall_name##_nocancel,.-__##syscall_name##_nocancel; \
> + .section ".text"; \
> +ENTRY (__##syscall_name##_nocancel); \
> +L(pseudo_nocancel): \
> + DO_CALL (syscall_name, args); \
> +L(pseudo_finish): \
> + cmn x0, 4095; \
> + b.cs L(syscall_error); \
> + .subsection 2; \
> + .size __##syscall_name##_nocancel,.-__##syscall_name##_nocancel; \
i think there is a problem here that may need independent
fix (not a regression, but worth a mention):
a glibc test (debug/tst-backtrace5) checks if 'read' symbol
is found when read is interrupted and the signal handler
calls backtrace_symbols.
however the interrupt will be in __read_nocancel and that
name is not exported so backtrace does not find it in the
dynamic symbol table so the check fails. (on some other
archs the test pass because the nocancel code is within
the read code, not a separate function).
it's a silly issue so i haven't got around proposing a fix
for this yet.