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: Tue, 30 Jun 2015 09:46:40 +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> <55911ECA dot 2060205 at arm dot com> <5591C3AB dot 9000702 at linaro dot org>
On 29/06/15 23:16, Adhemerval Zanella wrote:
> they are not necessary indeed. I have removed them and cancellation
> handlers works as intended, right now the syscall wrappers looks like:
>
> ENTRY (__syscall_cancel_arch)
>
> .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:
> ret
>
> 1:
> b __syscall_do_cancel
>
> END (__syscall_cancel_arch)
>
looks good.
> Indeed it would be much better to add more cleanup in this macros.
> I have simplified to:
>
> # 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) \
> ENTRY (name); \
> SINGLE_THREAD_P(16); \
> cbnz w16, L(pseudo_cancel); \
> DO_CALL (syscall_name, args); \
> b L(pseudo_finish); \
> L(pseudo_cancel): \
> 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); \
> mov x6, x5; \
> mov x5, x4; \
> mov x4, x3; \
> mov x3, x2; \
> mov x2, x1; \
> mov x1, x0; \
> mov x0, SYS_ify (syscall_name); \
> bl JMP_SYSCALL_CANCEL; \
> ldp x29, x30, [sp], 16; \
> cfi_restore (30); \
> cfi_restore (29); \
> cfi_def_cfa (31, 0); \
> L(pseudo_finish): \
> cmn x0, 4095; \
> b.cs L(syscall_error);
>
> # undef PSEUDO_END
> # define PSEUDO_END(name) \
> SYSCALL_ERROR_HANDLER; \
> cfi_endproc; \
> .size name, .-name;
>
> And debug/tst-backtrace{5-6} work as intended as well. What do you think?
>
is it ok to remove the __<syscall>_nocancel internal symbols?
otherwise it looks good.
thanks.