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] nptl: Implement pthread_self in libc.so


On 12/15/2017 05:51 AM, Carlos O'Donell wrote:
On 12/14/2017 10:37 AM, Florian Weimer wrote:
All binaries use TLS and thus need a properly set up TCB, so we can
simply return its address directly, instead of forwarding to the
libpthread implementation from libc.

Could you expand on this a bit?

Yes, all binaries use TLS by virtue of errno.

Yes, because we need errno setup we need a TCB setup.

However, internally we never call pthread_self, we use THREAD_SELF,
and access the thread register directly. So there is no internal cost
paid for pthread_self.

I don't understand this. The existing implementation has a cost because of the redirect from libc.so to libpthread.so:

Dump of assembler code for function pthread_self:
   <+0>:	mov    0x2b505a(%rip),%eax # <__libc_pthread_functions_init>
   <+6>:	test   %eax,%eax
   <+8>:	je     0x7ffff76f0690 <pthread_self+32>
   <+10>:	mov    0x2b4fb7(%rip),%rax # <__libc_pthread_functions+280>
   <+17>:	ror    $0x11,%rax
   <+21>:	xor    %fs:0x30,%rax
   <+30>:	jmpq   *%rax
   <+32>:	xor    %eax,%eax
   <+34>:	retq

Compare this to the implementation in libpthread (which is what the new libc.so implementation looks like as well):

Dump of assembler code for function __pthread_self:
   <+0>:	mov    %fs:0x10,%rax
   <+9>:	retq

The redirect implementation returns zero as a pthread_t value, which is not correct if libpthread is loaded later (something this redirect functionality is there to support, but which is currently defective in various ways).

The only cost paid is if the application binary calls pthread_self to
use that descriptor in some way, and to use it in some way means calling
a pthread function with it.

It's also common to use it for logging, although that is a slight abuse. The range of permitted uses is certainly limited because pthread_t is an unsigned long, and any direct manipulation is undefined.

The stub implementation is broken, so we can't move more functionality from libpthread to libc until it is fixed anyway.

Are there thread functions we might use in a program not linked against
libpthread e.g. the pthread_atfork changes we have in Fedora?

pthread_thread_number_np depends on this change in practical terms.

The layout of struct pthread_functions is not changed by this commit,
although the pointer used to store the address of pthread_self is now
unused.

Why?

To preserve the internal ABI. It seem to have very little cost. I can remove the field, of course.

To avoid linking against the libpthread function in all cases, we would
have to bump the symbol version of libpthread in libc.so and supply a
compat symbol.

Could you expand on what this might look like?

We'd add pthread_self@@GLIBC_2.27 to glibc and turn the existing version into a compat symbol. Both would be backed by the same implementation (at the same address). Hopefully, this will encourage link editors not to use the compatibility symbol in libpthread—it then has a completely different version, so it should be treated as unrelated.

This commit does not do that because the function
implementation is so small, so the overhead by two active copies of the
same function might well be smaller than the increase in symbol table
size.

Can we please measure that to be sure?

I don't see how. Is it really necessary? The new implementation in libc is clearly an improvement.

The link editor should probably be fixed not to link against compatibility symbols in certain cases.

Thanks,
Florian


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