Message ID | 20220610163552.3587064-2-adhemerval.zanella@linaro.org |
---|---|
State | New |
Headers | show |
Series | Simplify internal single-threaded usage | expand |
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. 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. 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 >
> 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. > > 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
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 >
> On 16 Jun 2022, at 15:30, Fangrui Song <maskray@google.com> wrote: > > 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.) The glibc will still continue to use the __libc_single_threaded value internally, the __libc_dlsym is used to initialize __libc_external_single_threaded which is used to update the external one solely to avoid update not being visible on architectures that use copy relocations. This is actually not needed on some architectures, aarch64 for instance, but without it x86 does not see the __libc_single_thread being updated. > > 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. > It can not be a read-only value because it should be updated by pthread_create.
On 2022-06-16, Adhemerval Zanella wrote: > > >> On 16 Jun 2022, at 15:30, Fangrui Song <maskray@google.com> wrote: >> >> 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.) > >The glibc will still continue to use the __libc_single_threaded value internally, the >__libc_dlsym is used to initialize __libc_external_single_threaded which is used to >update the external one solely to avoid update not being visible on architectures that >use copy relocations. Yes, I know. My point is that to access the possibly external definition (in the presence of a copy relocation), we can access the STV_DEFAULT symbol, instead of using __libc_dlsym. The compiler generates a GOT-generating relocation in -fPIC mode and the linker will create a GLOB_DAT dynamic relocation because the symbol is preemptible/interposable (see https://maskray.me/blog/2020-11-15-explain-gnu-linker-options#bsymbolic-and---dynamic-list how a symbol is considered preemptible). ld.so resolves GLOB_DAT to the copy relocated definition in the executable (if exists). >This is actually not needed on some architectures, aarch64 for instance, but without >it x86 does not see the __libc_single_thread being updated. __libc_single_thread is in a public header sys/single_threaded.h and is used by libstdc++ ext/atomicity.h. A user program compiled with -fno-pic will typically access __libc_single_thread with an absolute or PC-relative relocation. The linker will resolve this relocation to a copy relocation if __libc_single_thread is defined in libc.so.6. aarch64 -fno-pic uses PC-relative relocations. I think some variants of mips avoid -fno-pic code generation which may lead to copy relocation. clang -fno-direct-access-external-data and gcc -mno-direct-extern-access avoids absolute/PC-relative relocation in -fno-pic/-fpie mode, but the options are by no means popular. >> >> 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. >> > >It can not be a read-only value because it should be updated by pthread_create. > I mean that the executable does not write to __libc_single_threaded. Its write to __libc_single_threaded will not be visible to libc.so.6. If libc.so.6 decides to write the STV_HIDDEN __libc_single_threaded, it needs to write the STV_DEFAULT __libc_single_threaded to update the possibly copy relocated definition.
Hi,
So long story short, what happens is that the magic macros effectively emit both global
and hidden versions of the same symbol which can be accessed without using dl_sym.
>>>> I think this whole scheme can be greatly simplified.
Yes, I would suggest to keep it simple and just explicitly add a separate hidden symbol
for use inside GLIBC (eg. __libc_single_thread_internal).
Cheers,
Wilco
* Adhemerval Zanella via Libc-alpha: > 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 wrote this before, but: There is no need to cache the pointer because we need it at most once. We can just do the lookup in pthread_create. RTLD_DEFAULT is wrong here because we need the scope of the main program, where the copy relocations are, so: __libc_dlsym (GL (dl_ns)[LM_ID_BASE]._ns_loaded, "__libc_single_threaded"); and no __libc_dlsym changes are needed. Thanks, Florian
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 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;