Re: Reviving planned ldso changes

[Rich Felker <dalias@libc.org>]

On Sun, Feb 26, 2017 at 02:04:30AM +0100, Szabolcs Nagy wrote:
> * Rich Felker <dalias@libc.org> [2017-01-15 12:44:38 -0500]:
> >  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);
> > +	/* Construct in dependency order without any recursive state. */
> > +	while (p && !p->constructed) {
> > +		/* The following loop descends into the first dependency
> > +		 * that is neither alredy constructed nor pending
> > +		 * construction due to circular deps, stopping only
> > +		 * when it reaches a dso with no remaining dependencies
> > +		 * to descend into. */
> > +		while (p->deps && p->deps[p->next_dep]) {
> > +			if (!p->deps[p->next_dep]->constructed &&
> > +			    !p->deps[p->next_dep]->next_dep)
> > +				p = p->deps[p->next_dep++];
> > +			else
> > +				p->next_dep++;
> > +		}
> >  		p->constructed = 1;
> >  		decode_vec(p->dynv, dyn, DYN_CNT);
> >  		if (dyn[0] & ((1<<DT_FINI) | (1<<DT_FINI_ARRAY))) {
> > @@ -1233,17 +1246,19 @@ static void do_init_fini(struct dso *p)
> >  			size_t *fn = laddr(p, dyn[DT_INIT_ARRAY]);
> >  			while (n--) ((void (*)(void))*fn++)();
> >  		}
> > -		if (!need_locking && libc.threads_minus_1) {
> > -			need_locking = 1;
> > -			pthread_mutex_lock(&init_fini_lock);
> > -		}
> > -	}
> > -	if (need_locking) pthread_mutex_unlock(&init_fini_lock);
> > +		/* Revisit "parent" dso which caused the just-constructed
> > +		 * dso to be pulled in as a dependency. On the next loop
> > +		 * iteration we will either descend to construct a sibling
> > +		 * of the just-constructed dso, or finish constructing the
> > +		 * parent if no unfinished deps remain. */
> > +		p = p->needed_by;
> > +	}
> 
> i think with
> 
> a.deps: b c
> b.deps: c d
> b.needed_by: a
> c.needed_by: a
> 
> the visiting order starting from a is
> a
> b
> c
> a
> 
> and d never gets constructed.

Are you sure? My understanding of what it does is:

1. Descend a->b->c, construct c, and back up to b.
2. Descend b->d, construct d, and back up to b.
3. Find all of b's deps constructed, construct b, and back up to a.
4. Find all of a's deps constructed, construct a, and end.

I think you have a misunderstanding of "visit order". Nodes are not
visited while descending, only when reaching a point where no further
descent into a non-constructed dep is possible.

Rich