Re: [PATCH v2] elf: Release dl_load_lock before running dlopen constructors (BZ 15686)
Artem Proskurnev <[email protected]>
| Newsgroups | gmane.comp.lib.glibc.alpha |
|---|---|
| Message-ID | <[email protected]> |
>> diff --git a/elf/dl-init.c b/elf/dl-init.c >> index bd85bacdc1..250f7cc175 100644 >> --- a/elf/dl-init.c >> +++ b/elf/dl-init.c >> @@ -21,27 +21,24 @@ >> #include <ldsodefs.h> >> #include <elf-initfini.h> >> >> +#if !IS_IN (rtld) >> +# include <stdint.h> >> +/* pthread_once_t is int; we use the l_init_once field in struct >> + link_map (also int, zero from calloc matches PTHREAD_ONCE_INIT). */ >> +extern int __pthread_once (int *once_control, void (*init_routine) (void)); >> + >> +/* Argument for call_init_once_cb. Set by call_init before calling >> + __pthread_once, which invokes the callback synchronously (either >> + immediately, or after blocking). */ >> +static __thread struct link_map *call_init_once_arg; >> + >> +static void call_init_once_cb (void); >> +#endif > I don't think this change is effective because elf/dl-init.c is not > built at all for dynamically linked libc.so, only for libc.a and ld.so. > > I think a fallible pthread_once variant with a closure pointer would be > generally useful. It could be used here to avoid the thread-local > variable. > > We cannot use ELF TLS in ld.so, but we can access fields in struct > pthread. See rtld_catch. > >> diff --git a/include/link.h b/include/link.h >> index 8f851d2212..e539535a44 100644 >> --- a/include/link.h >> +++ b/include/link.h >> @@ -346,6 +346,12 @@ struct link_map >> size_t l_relro_size; >> >> unsigned long long int l_serial; >> + >> + /* Per-DSO once-initialization control for constructor execution. >> + Used as pthread_once_t (PTHREAD_ONCE_INIT == 0, matching >> + calloc). Only accessed from the libc.so build of dl-init.c >> + (IS_IN (rtld) path is single-threaded). */ >> + int l_init_once; >> }; > I don't think this will build on Hurd. It doesn't have futexes, so > pthread_once_t contains a spinlock and isn't just an int. > > We'll need additional tests that show the synchronization is working as > expected. > > My concern regarding the removal of a lock that applications could > previously rely on remains. > > Thanks, > Florian > I got it. Indeed I did it wrong when I used IS_IN (rtld). This is a significantly more complex patch. But I will try to implement it. Thank you very much for your response! Artem