This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 08/08] nptl: arm: Fix Race conditions in pthread cancellation (BZ#12683)
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: Phil Blundell <pb at pbcl dot net>
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Thu, 3 Sep 2015 14:30:12 -0300
- Subject: Re: [PATCH 08/08] nptl: arm: Fix Race conditions in pthread cancellation (BZ#12683)
- Authentication-results: sourceware.org; auth=none
- References: <55E4C300 dot 9080800 at linaro dot org> <1441148926 dot 1680 dot 51 dot camel at pbcl dot net> <55E768E8 dot 8020904 at linaro dot org> <1441276001 dot 22688 dot 18 dot camel at pbcl dot net> <55E86CC9 dot 9040507 at linaro dot org> <1441297157 dot 22688 dot 26 dot camel at pbcl dot net>
On 03-09-2015 13:19, Phil Blundell wrote:
> On Thu, 2015-09-03 at 12:52 -0300, Adhemerval Zanella wrote:
>> I can change to:
>>
>> mov lr, pc
>> b __syscall_do_cancel
>>
>> Which explicit state it is a tail cail that do not return.
>
> If you set lr like that then the implication is that it will return
> (since otherwise lr would be unnecessary). A regular tail call would
> just do the branch without changing lr at all.
My first approach was to use compiler output to check how it handle tail
call for ARM. For instance, the code:
--
void __do_cancel () __attribute__ ((noreturn));
long int foo (long int nr);
long int __syscall (long int *d, long int nr)
{
if (*d & 0x1)
__do_cancel ();
return foo (nr);
}
--
generates the 'bl' for __do_cancel with both GCC 4.9 and GCC 6.0.
I did try to use just use a 'b' instruction, but again the tst-cancelx{4,5,17,18}
fails with some early cancellation related to pause and aio functions. If you
can make the tests pass using a 'b' function call please let me know.
>
>> If you check my first message (00/08: nptl: Fix Race conditions in pthread
>> cancellation (BZ#12683)) the idea of this modification it exactly to route
>> *all* cancellable syscall to the __syscall_cancel_arch. It is required
>> because the __syscall_cancel_arch have the global marks the signal cancel
>> handler (sigcancel_handle in nptl/nptl-init.c) will use to check if the
>> instruction pointer falls within the cancellable syscall code.
>
> Ah, I see. Your original message didn't actually say that, but now I
> understand how it's supposed to work. However, I think the amount of
> stack shuffling that you're doing is still rather excessive.
>
> If __syscall_cancel_arch simply needs to be a delineated block of code
> and isn't called from anywhere except PSEUDO then it doesn't necessarily
> need to obey the normal ABI calling conventions. But even if it does, I
> think it should be possible to achieve this result with rather less
> complexity than you seem to have at the moment.
Correct calling sequence should be something like:
long int syscall (...)
if SINGLE_THREAD_P
INLINE_SYSCALL (...)
else
syscall_cancel (...)
I am far from an ARM assembly expert, so if you any suggestion on how to
accomplish with less instruction that current one please let me know.
Right now my approach is to use the current assembly hooks to generate the
cancellable syscalls. My idea for future patches is to just get rid of
this assembly macros and code it with C code instead, auto-generating
the above code.
>
> p.
>
>