[PATCH v2 1/4] misc: Optimize internal usage of __libc_single_threaded

[Fangrui Song]

On 2022-06-16, Adhemerval Zanella wrote:
>
>
>> On 16 Jun 2022, at 00:15, Fangrui Song <maskray@google.com> wrote:
>>
>> On 2022-06-10, Adhemerval Zanella via Libc-alpha wrote:
>>> By adding an internal hidden_def alias to avoid the GOT indirection.
>>> On some architecture, __libc_single_thread may be accessed through
>>> copy relocations and thus it requires to update both copies.
>>>
>>> To obtain the correct address of the __libc_single_thread,
>>> __libc_dlsym is extended to support RTLD_DEFAULT. It searches
>>> through all scope instead of default local ones.
>>>
>>> Checked on x86_64-linux-gnu and i686-linux-gnu.
>>> ---
>>> elf/dl-libc.c | 20 ++++++++++++++++++--
>>> elf/libc_early_init.c | 9 +++++++++
>>> include/sys/single_threaded.h | 11 +++++++++++
>>> misc/single_threaded.c | 2 ++
>>> nptl/pthread_create.c | 6 +++++-
>>> 5 files changed, 45 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/elf/dl-libc.c b/elf/dl-libc.c
>>> index 266e068da6..e64f4b9910 100644
>>> --- a/elf/dl-libc.c
>>> +++ b/elf/dl-libc.c
>>> @@ -16,6 +16,7 @@
>>> License along with the GNU C Library; if not, see
>>> <https://www.gnu.org/licenses/>. */
>>>
>>> +#include <assert.h>
>>> #include <dlfcn.h>
>>> #include <stdlib.h>
>>> #include <ldsodefs.h>
>>> @@ -72,6 +73,7 @@ struct do_dlsym_args
>>> /* Arguments to do_dlsym. */
>>> struct link_map *map;
>>> const char *name;
>>> + const void *caller_dlsym;
>>>
>>> /* Return values of do_dlsym. */
>>> lookup_t loadbase;
>>> @@ -102,8 +104,21 @@ do_dlsym (void *ptr)
>>> {
>>> struct do_dlsym_args *args = (struct do_dlsym_args *) ptr;
>>> args->ref = NULL;
>>> - args->loadbase = GLRO(dl_lookup_symbol_x) (args->name, args->map, &args->ref,
>>> -					 args->map->l_local_scope, NULL, 0,
>>> + struct link_map *match = args->map;
>>> + struct r_scope_elem **scope;
>>> + if (args->map == RTLD_DEFAULT)
>>> + {
>>> + ElfW(Addr) caller = (ElfW(Addr)) args->caller_dlsym;
>>> + match = _dl_find_dso_for_object (caller);
>>> + /* It is only used internally, so caller should be always recognized. */
>>> + assert (match != NULL);
>>> + scope = match->l_scope;
>>> + }
>>> + else
>>> + scope = args->map->l_local_scope;
>>> +
>>> + args->loadbase = GLRO(dl_lookup_symbol_x) (args->name, match, &args->ref,
>>> +					 scope, NULL, 0,
>>> 					 DL_LOOKUP_RETURN_NEWEST, NULL);
>>> }
>>>
>>> @@ -182,6 +197,7 @@ __libc_dlsym (void *map, const char *name)
>>> struct do_dlsym_args args;
>>> args.map = map;
>>> args.name = name;
>>> + args.caller_dlsym = RETURN_ADDRESS (0);
>>>
>>> #ifdef SHARED
>>> if (GLRO (dl_dlfcn_hook) != NULL)
>>> diff --git a/elf/libc_early_init.c b/elf/libc_early_init.c
>>> index 3c4a19cf6b..7cc2997122 100644
>>> --- a/elf/libc_early_init.c
>>> +++ b/elf/libc_early_init.c
>>> @@ -16,7 +16,9 @@
>>> License along with the GNU C Library; if not, see
>>> <https://www.gnu.org/licenses/>. */
>>>
>>> +#include <assert.h>
>>> #include <ctype.h>
>>> +#include <dlfcn.h>
>>> #include <elision-conf.h>
>>> #include <libc-early-init.h>
>>> #include <libc-internal.h>
>>> @@ -38,6 +40,13 @@ __libc_early_init (_Bool initial)
>>> __libc_single_threaded = initial;
>>>
>>> #ifdef SHARED
>>> + /* __libc_single_threaded can be accessed through copy relocations, so it
>>> + requires to update the external copy. */
>>> + __libc_external_single_threaded = __libc_dlsym (RTLD_DEFAULT,
>>> +						 "__libc_single_threaded");
>>> + assert (__libc_external_single_threaded != NULL);
>>> + *__libc_external_single_threaded = initial;
>>> +
>>> __libc_initial = initial;
>>> #endif
>>
>> I think this whole scheme can be greatly simplified.
>>
>> Under the hood,
>>
>> extern __typeof (__libc_single_threaded) __EI___libc_single_threaded __asm__("" "__libc_single_threaded"); extern __typeof (__libc_single_threaded) __EI___libc_single_threaded __attribute__((alias ("" "__GI___libc_single_threaded"))) __attribute__ ((__copy__ (__libc_single_threaded)));
>> * __libc_single_threaded is the STV_HIDDEN symbol with the asm name "__GI___libc_single_threaded"
>> * __EI___libc_single_threaded is the STV_DEFAULT symbol with the asm name "__libc_single_threaded". It aliases __libc_single_threaded.
>
>In fact libc_hidden_proto for SHARED will result in:
>
>extern __typeof (__libc_single_threaded) __libc_single_threaded __asm__ ("" "__GI___libc_single_threaded") __attribute__ ((visibility ("hidden")));;
>
>So using __libc_single_threaded will be the most simple way.

The code tries to read the initial value of the copy relocated __libc_single_threaded.
Accessing the STV_DEFAULT symbol does the trick and is simpler than __libc_dlsym.
(Accessing the STV_HIDDEN symbol just accesses libc.so's own copy.)

Note: The copy relocated __libc_single_threaded likely has a value of zero and is read-only from the executable.
I assume that this is a conservative and safe value.

>>
>> We can just access __EI___libc_single_threaded which will lead to a GOT
>> indirection (R_*_GLOB_DAT). This can avoid a __libc_dlsym call.
>
>The idea is exactly to avoid the GOT indirection.
>
>>
>> I can see that accessing the __EI declaration is currently inconvenient
>> because include/libc-symbols.h does not seem to provide a convenient
>> macro, but that be added.
>>
>>> diff --git a/include/sys/single_threaded.h b/include/sys/single_threaded.h
>>> index 18f6972482..258b01e0b2 100644
>>> --- a/include/sys/single_threaded.h
>>> +++ b/include/sys/single_threaded.h
>>> @@ -1 +1,12 @@
>>> #include <misc/sys/single_threaded.h>
>>> +
>>> +#ifndef _ISOMAC
>>> +
>>> +libc_hidden_proto (__libc_single_threaded);
>>> +
>>> +# ifdef SHARED
>>> +extern __typeof (__libc_single_threaded) *__libc_external_single_threaded
>>> + attribute_hidden;
>>> +# endif
>>> +
>>> +#endif
>>> diff --git a/misc/single_threaded.c b/misc/single_threaded.c
>>> index 96ada9137b..201d86a273 100644
>>> --- a/misc/single_threaded.c
>>> +++ b/misc/single_threaded.c
>>> @@ -22,6 +22,8 @@
>>> __libc_early_init (as false for inner libcs). */
>>> #ifdef SHARED
>>> char __libc_single_threaded;
>>> +__typeof (__libc_single_threaded) *__libc_external_single_threaded;
>>> #else
>>> char __libc_single_threaded = 1;
>>> #endif
>>> +libc_hidden_data_def (__libc_single_threaded)
>>> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
>>> index e7a099acb7..5633d01c62 100644
>>> --- a/nptl/pthread_create.c
>>> +++ b/nptl/pthread_create.c
>>> @@ -627,7 +627,11 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr,
>>> if (__libc_single_threaded)
>>> {
>>> late_init ();
>>> - __libc_single_threaded = 0;
>>> + __libc_single_threaded =
>>> +#ifdef SHARED
>>> + *__libc_external_single_threaded =
>>> +#endif
>>> +	0;
>>> }
>>>
>>> const struct pthread_attr *iattr = (struct pthread_attr *) attr;
>>> --
>>> 2.34.1
>