Re: [PATCH v2] elf: Release dl_load_lock before running dlopen constructors (BZ 15686)

> 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