|
Message-ID: <20170104193627.GO1555@brightrain.aerifal.cx>
Date: Wed, 4 Jan 2017 14:36:27 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: Reviving planned ldso changes
On Wed, Jan 04, 2017 at 01:22:03AM -0500, Rich Felker wrote:
> 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.
Here's a v2 of the patch with the above issues fixed, and some
comments that hopefully make it make sense. I still think there's more
logic needed to allow concurrent ctors from unrelated dlopen in
multiple threads, though.
Rich
View attachment "ctor_dep_order_v2.diff" of type "text/plain" (2682 bytes)
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.