Re: Reviving planned ldso changes

[Rich Felker <dalias@libc.org>]

On Wed, Jan 04, 2017 at 01:06:40AM -0500, Rich Felker wrote:
> diff --git a/ldso/dynlink.c b/ldso/dynlink.c
> index c689084..cb82b2c 100644
> --- a/ldso/dynlink.c
> +++ b/ldso/dynlink.c
> @@ -67,6 +67,7 @@ struct dso {
>  	char constructed;
>  	char kernel_mapped;
>  	struct dso **deps, *needed_by;
> +	size_t next_dep;
>  	char *rpath_orig, *rpath;
>  	struct tls_module tls;
>  	size_t tls_id;
> @@ -1211,13 +1212,14 @@ void __libc_exit_fini()
>  static void do_init_fini(struct dso *p)
>  {
>  	size_t dyn[DYN_CNT];
> -	int need_locking = libc.threads_minus_1;
> -	/* Allow recursive calls that arise when a library calls
> -	 * dlopen from one of its constructors, but block any
> -	 * other threads until all ctors have finished. */
> -	if (need_locking) pthread_mutex_lock(&init_fini_lock);
> -	for (; p; p=p->prev) {
> -		if (p->constructed) continue;
> +	pthread_mutex_lock(&init_fini_lock);
> +	while (!p->constructed) {
> +		while (p->deps[p->next_dep] && p->deps[p->next_dep]->constructed)
> +			p->next_dep++;
> +		if (p->deps[p->next_dep]) {
> +			p = p->deps[p->next_dep++];
> +			continue;
> +		}

I think this logic is probably broken in the case of circular
dependencies, and will end up skipping ctors for some deps due to the
increment in this line:

+			p = p->deps[p->next_dep++];

which happens before the new p's ctors have actually run. Omitting the
increment here, however, would turn it into an infinite loop. We need
some way to detect this case and let the dso with an indirect
dependency on itself run its ctors once all non-circular deps have
been satisfied. I don't now the right algorithm for this right off,
though; suggestions would be welcome.

Rich