Re: [PATCH 1/8] nptl: Perform signal initialization upon pthread_create

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 21/05/2021 17:15, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>>> @@ -106,4 +172,8 @@ versioned_symbol (libc, __pthread_cancel, pthread_cancel, GLIBC_2_34);
>>>  compat_symbol (libpthread, __pthread_cancel, pthread_cancel, GLIBC_2_0);
>>>  #endif
>>>  
>>> -PTHREAD_STATIC_FN_REQUIRE (__pthread_create)
>>> +/* Ensure that the unwinder is always linked in (the __pthread_unwind
>>> +   reference from __do_cancel is weak).  Use ___pthread_unwind_next
>>> +   (three underscores) to produce a strong reference to the same
>>> +   file.  */
>>> +PTHREAD_STATIC_FN_REQUIRE (___pthread_unwind_next)
>>
>> Why __pthread_unwind is marked as weak now? Would be simpler to unmark
>> is as weak and avoid the PTHREAD_STATIC_FN_REQUIRE?
> 
> It's related to the compiled-in cancellation points.  The references to
> __pthread_enable_asynccancel are strong, and that disassembles to:
> 
> 0000000000000000 <__pthread_enable_asynccancel>:
>    0:	mov    %fs:0x10,%rcx
>    9:	mov    %fs:0x308,%r8d
>   12:	mov    %r8d,%edx
>   15:	or     $0x2,%edx
>   18:	cmp    %edx,%r8d
>   1b:	je     35 <__pthread_enable_asynccancel+0x35>
>   1d:	mov    %r8d,%eax
>   20:	lock cmpxchg %edx,0x308(%rcx)
>   28:	cmp    %eax,%r8d
>   2b:	jne    40 <__pthread_enable_asynccancel+0x40>
>   2d:	and    $0xffffffbb,%edx
>   30:	cmp    $0xa,%edx
>   33:	je     45 <__pthread_enable_asynccancel+0x45>
>   35:	mov    %r8d,%eax
>   38:	retq   
>   39:	nopl   0x0(%rax)
>   40:	mov    %eax,%r8d
>   43:	jmp    12 <__pthread_enable_asynccancel+0x12>
>   45:	push   %rax
>   46:	movq   $0xffffffffffffffff,%fs:0x630
>   53:	mov    %fs:0x10,%rax
>   5c:	lock orl $0x10,0x308(%rax)
>   64:	mov    %fs:0x300,%rdi
>   6d:	callq  72 <__pthread_enable_asynccancel+0x72>
> 			6e: R_X86_64_PLT32	__pthread_unwind-0x4
>   72:	data16 nopw %cs:0x0(%rax,%rax,1)
>   7d:	nopl   (%rax)
> 
> And the unwind reference is weak:
> 
>    17: 0000000000000000      0 NOTYPE  WEAK   HIDDEN     UNDEF __pthread_unwind
> 
> There's no address check because the cancellation state in the TCB can
> only change if pthread_cancel is linked in, and that really should pull
> in the unwinder, too.  That's why I added the PTHREAD_STATIC_FN_REQUIRE.
> Without it, I think some of the existing tests will fail (the
> pthread_create PTHREAD_STATIC_FN_REQUIREs are apparently not quite
> correct).
> 
> I think it's always been this way, so that the unwinder is optional in
> static links (in theory, in practice it is still pulled in by the bits
> that are compiled with -fexceptions due to the _Unwind_Resume construct,
> hence the need for elf/static-stubs.c).

Right, it does seem unnecessary to pull the unwinder for SYSCALL_CANCEL.