Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 26 Feb 2017 02:04:30 +0100
From: Szabolcs Nagy <nsz@...t70.net>
To: musl@...ts.openwall.com
Subject: Re: Reviving planned ldso changes

* Rich Felker <dalias@...c.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.

i was looking for the dfs stack (how you track back
on the path you descended into the dependency tree),
the needed_by entry might not point to the parent
dso through which you arrived somewhere.

i'm not sure what the right fix is, if needed_by
is not used elsewhere then it could be set during
traversal, but there might be other ways.

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.