This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 08/08] nptl: arm: Fix Race conditions in pthread cancellation (BZ#12683)



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.
> 
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]