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

[Adhemerval Zanella]

On 30-06-2015 05:46, Szabolcs Nagy wrote:
> 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.

Yes, the idea of 34caaafd1ae38c9295325a1da491d75a92b205b0 is exactly to
remove __<syscall>_nocancel usage.

> 
> thanks.
>