[PATCH] glibc: dlopen RTLD_NOLOAD optimization

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] glibc: dlopen RTLD_NOLOAD optimization

Sourceware - libc-alpha mailing list
https://sourceware.org/bugzilla/show_bug.cgi?id=25615
This change was submitted as an RFC in late June. Since there were no objections to it, we added a test case for "make check" and "make xcheck" and are now formally submitting it.
Our test case addresses Carlos's comments on the bug report.

0001-glibc-dlopen-RTLD_NOLOAD-optimization.patch (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] glibc: dlopen RTLD_NOLOAD optimization

Sourceware - libc-alpha mailing list
On 7/16/20 3:29 PM, Jeremy Stenglein (jstengle) wrote:
> https://sourceware.org/bugzilla/show_bug.cgi?id=25615 This change was
> submitted as an RFC in late June. Since there were no objections to
> it, we added a test case for "make check" and "make xcheck" and are
> now formally submitting it. Our test case addresses Carlos's comments
> on the bug report.

Thanks for re-submitting this.

We are in the middle of the glibc 2.32 release which is going out on
August 3rd. This means we'll likely delay applying this until 2.33 opens
for development. We can review it in the meantime, but there will be a
deay until that time. I hope that is clear and you weren't expecting to
get this included in 2.32.

> From 89548c37e426380fdcf0e8e5833bd1d0a023b20a Mon Sep 17 00:00:00 2001

> From: Conan C Huang <[hidden email]>

> Date: Wed, 6 May 2020 16:47:25 -0400

> Subject: [PATCH] glibc: dlopen RTLD_NOLOAD optimization

>

> This is a fix for GLIBC Bugzilla #25615: dlopen RTLD_NOLOAD

> optimization

>

> When dlopen with RTLD_NOLOAD flag, loader should simply return the

> link_map or NULL and promote flags like RTLD_GLOBAL and RTLD_NODELETE.

>

> Loader shouldn't need to process any dependencies. In dl_open_worker,

> loader should avoid calling _dl_map_object_deps. _dl_map_object_deps

> is very time consuming on low-end platforms with lots of library

> dependencies.

>

> Loader retrains the same functionality during flag promotions:

> RTLD_GLOBAL

> RTLD_NODELETE

>

> https://sourceware.org/bugzilla/show_bug.cgi?id=25615

> ---

>  elf/Makefile                  |  7 +++--

>  elf/dl-open.c                 |  4 ++-

>  elf/tst-dlopen-noload-liba.c  |  9 ++++++

>  elf/tst-dlopen-noload-libb.c  |  8 +++++

>  elf/tst-dlopen-noload-nodel.c |  8 +++++

>  elf/tst-dlopen-noload-share.h |  5 ++++

>  elf/tst-dlopen-noload.c       | 56 +++++++++++++++++++++++++++++++++++

>  7 files changed, 94 insertions(+), 3 deletions(-)

>  create mode 100644 elf/tst-dlopen-noload-liba.c

>  create mode 100644 elf/tst-dlopen-noload-libb.c

>  create mode 100644 elf/tst-dlopen-noload-nodel.c

>  create mode 100644 elf/tst-dlopen-noload-share.h

>  create mode 100644 elf/tst-dlopen-noload.c

>

> diff --git a/elf/Makefile b/elf/Makefile

> index a2c3b12007..28cb787680 100644

> --- a/elf/Makefile

> +++ b/elf/Makefile

> @@ -216,7 +216,7 @@ tests-internal += loadtest unload unload2 circleload1 \

>   neededtest neededtest2 neededtest3 neededtest4 \

>   tst-tls3 tst-tls6 tst-tls7 tst-tls8 tst-dlmopen2 \

>   tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym \

> - tst-create_format1

> + tst-create_format1 tst-dlopen-noload

>  tests-container += tst-pldd tst-dlopen-tlsmodid-container \

>    tst-dlopen-self-container

>  test-srcs = tst-pathopt

> @@ -328,7 +328,8 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \

>   tst-single_threaded-mod3 tst-single_threaded-mod4 \

>   tst-tls-ie-mod0 tst-tls-ie-mod1 tst-tls-ie-mod2 \

>   tst-tls-ie-mod3 tst-tls-ie-mod4 tst-tls-ie-mod5 \

> - tst-tls-ie-mod6

> + tst-tls-ie-mod6 tst-dlopen-noload-libb tst-dlopen-noload-liba \

> + tst-dlopen-noload-nodel

>  

>  # Most modules build with _ISOMAC defined, but those filtered out

>  # depend on internal headers.

> @@ -732,6 +733,8 @@ $(objpfx)tst-tlsalign: $(objpfx)tst-tlsalign-lib.so

>  $(objpfx)tst-nodelete-opened.out: $(objpfx)tst-nodelete-opened-lib.so

>  $(objpfx)tst-nodelete-opened: $(libdl)

>  $(objpfx)tst-noload: $(libdl)

> +$(objpfx)tst-dlopen-noload-liba.so: $(objpfx)tst-dlopen-noload-libb.so

> +$(objpfx)tst-dlopen-noload: $(libdl) $(objpfx)tst-dlopen-noload-libb.so

>  

>  $(objpfx)tst-tlsalign-extern: $(objpfx)tst-tlsalign-vars.o

>  $(objpfx)tst-tlsalign-extern-static: $(objpfx)tst-tlsalign-vars.o

> diff --git a/elf/dl-open.c b/elf/dl-open.c

> index 9b3606c491..cfef39ec53 100644

> --- a/elf/dl-open.c

> +++ b/elf/dl-open.c

> @@ -542,7 +542,9 @@ dl_open_worker (void *a)

>    ++new->l_direct_opencount;

>  

>    /* It was already open.  */

> -  if (__glibc_unlikely (new->l_searchlist.r_list != NULL))

> +  if (__glibc_unlikely (new->l_searchlist.r_list != NULL) ||

> +      (__glibc_unlikely (mode & RTLD_NOLOAD) &&

> +       __glibc_unlikely (!(mode & RTLD_GLOBAL))))

>      {

>        /* Let the user know about the opencount.  */

>        if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES))

> diff --git a/elf/tst-dlopen-noload-liba.c b/elf/tst-dlopen-noload-liba.c

> new file mode 100644

> index 0000000000..8a5937c2f4

> --- /dev/null

> +++ b/elf/tst-dlopen-noload-liba.c

> @@ -0,0 +1,9 @@

> +#include <stdio.h>

> +

> +#include "tst-dlopen-noload-share.h"

> +

> +void dlfunca(void)

> +{

> +    printf("dlfuna \n");

> +    dlfuncb();

> +}

> diff --git a/elf/tst-dlopen-noload-libb.c b/elf/tst-dlopen-noload-libb.c

> new file mode 100644

> index 0000000000..8dd8eb1880

> --- /dev/null

> +++ b/elf/tst-dlopen-noload-libb.c

> @@ -0,0 +1,8 @@

> +#include <stdio.h>

> +

> +#include "tst-dlopen-noload-share.h"

> +

> +void dlfuncb(void)

> +{

> +    printf("dlfunb \n");

> +}

> diff --git a/elf/tst-dlopen-noload-nodel.c b/elf/tst-dlopen-noload-nodel.c

> new file mode 100644

> index 0000000000..0a5ab68db7

> --- /dev/null

> +++ b/elf/tst-dlopen-noload-nodel.c

> @@ -0,0 +1,8 @@

> +#include <stdio.h>

> +

> +#include "tst-dlopen-noload-share.h"

> +

> +void nodel_func(void)

> +{

> +    printf("nodel func\n");

> +}

> diff --git a/elf/tst-dlopen-noload-share.h b/elf/tst-dlopen-noload-share.h

> new file mode 100644

> index 0000000000..6362d08548

> --- /dev/null

> +++ b/elf/tst-dlopen-noload-share.h

> @@ -0,0 +1,5 @@

> +

> +void dlfunc(void);

> +void dlfunca(void);

> +void dlfuncb(void);

> +

> diff --git a/elf/tst-dlopen-noload.c b/elf/tst-dlopen-noload.c

> new file mode 100644

> index 0000000000..9699d9649d

> --- /dev/null

> +++ b/elf/tst-dlopen-noload.c

> @@ -0,0 +1,56 @@

> +#include <stdio.h>

> +

> +void *dlopen(const char *filename, int flags);

> +

> +int dlclose(void *handle);

> +

> +#include <dlfcn.h>

> +

> +void *dlmopen (Lmid_t lmid, const char *filename, int flags);

> +

> +#include "tst-dlopen-noload-share.h"

> +

> +int main(int argc, const char *argv[])

> +{

> +    void *handle, *handle2;

> +    // promote RTLD_NODELETE

> +    printf("dlopen libnodel.so RTLD_LAZY\n");

> +    handle = dlopen("tst-dlopen-noload-nodel.so", RTLD_LAZY);

> +    if (!handle) {

> +        printf("dlopen libnodel.so failed: %s \n", dlerror());

> +        return 1;

> +    }

> +

> +    printf("dlopen libnodel.so RTLD_LAZY|RTLD_NOLOAD|RTLD_NODELETE\n");

> +    handle2 = dlopen("tst-dlopen-noload-nodel.so", RTLD_LAZY|RTLD_NOLOAD|RTLD_NODELETE);

> +    if (!handle2) {

> +        printf("dlopen libnodel.so failed: %s \n", dlerror());

> +        return 1;

> +    }

> +

> +    // gdb print _rtld_global._dl_ns[0]._ns_nloaded should not decrease

> +    dlclose(handle);

> +    dlclose(handle2);

> +

> +    // test RTLD_LOCAL to RTLD_GLOBAL

> +    printf("dlopen libliba.so RTLD_LAZY|RTLD_LOCAL\n");

> +    handle = dlopen("tst-dlopen-noload-liba.so", RTLD_LAZY|RTLD_LOCAL);

> +    if (!handle) {

> +        printf("dlopen libliba.so failed: %s \n", dlerror());

> +        return 1;

> +    }

> +

> +    printf("dlopen liblibb.so RTLD_LAZY|RTLD_NOLOAD|RTLD_GLOBAL\n");

> +    handle = dlopen("tst-dlopen-noload-libb.so", RTLD_LAZY|RTLD_NOLOAD|RTLD_GLOBAL);

> +    if (!handle) {

> +        printf("dlopen liblibb.so failed: %s \n", dlerror());

> +        return 1;

> +    }

> +

> +    // without promotion to RTLD_GLOBAL ltog_func will segfault

> +    dlfuncb();

> +

> +    // won't be able to close because dlfunc is using it

> +    dlclose (handle);

> +    return 0;

> +}

> --

> 2.17.1

>



--
Cheers,
Carlos.

Reply | Threaded
Open this post in threaded view
|

RE: [PATCH] glibc: dlopen RTLD_NOLOAD optimization

Sourceware - libc-alpha mailing list

> We are in the middle of the glibc 2.32 release which is going out on August
> 3rd. This means we'll likely delay applying this until 2.33 opens for
> development. We can review it in the meantime, but there will be a deay
> until that time. I hope that is clear and you weren't expecting to get this
> included in 2.32.

Sure, 2.33 is fine for us.  Thanks.

> -----Original Message-----
> From: Carlos O'Donell <[hidden email]>
> Sent: Thursday, July 16, 2020 3:21 PM
> To: Jeremy Stenglein (jstengle) <[hidden email]>; Daniel Walker
> (danielwa) <[hidden email]>; [hidden email]; Conan
> Huang (conhuang) <[hidden email]>
> Cc: xe-linux-external(mailer list) <[hidden email]>
> Subject: Re: [PATCH] glibc: dlopen RTLD_NOLOAD optimization
>
> On 7/16/20 3:29 PM, Jeremy Stenglein (jstengle) wrote:
> > https://sourceware.org/bugzilla/show_bug.cgi?id=25615 This change was
> > submitted as an RFC in late June. Since there were no objections to
> > it, we added a test case for "make check" and "make xcheck" and are
> > now formally submitting it. Our test case addresses Carlos's comments
> > on the bug report.
>
> Thanks for re-submitting this.
>
> We are in the middle of the glibc 2.32 release which is going out on August
> 3rd. This means we'll likely delay applying this until 2.33 opens for
> development. We can review it in the meantime, but there will be a deay
> until that time. I hope that is clear and you weren't expecting to get this
> included in 2.32.
>
> > From 89548c37e426380fdcf0e8e5833bd1d0a023b20a Mon Sep 17 00:00:00
> 2001
>
> > From: Conan C Huang <[hidden email]>
>
> > Date: Wed, 6 May 2020 16:47:25 -0400
>
> > Subject: [PATCH] glibc: dlopen RTLD_NOLOAD optimization
>
> >
>
> > This is a fix for GLIBC Bugzilla #25615: dlopen RTLD_NOLOAD
>
> > optimization
>
> >
>
> > When dlopen with RTLD_NOLOAD flag, loader should simply return the
>
> > link_map or NULL and promote flags like RTLD_GLOBAL and
> RTLD_NODELETE.
>
> >
>
> > Loader shouldn't need to process any dependencies. In dl_open_worker,
>
> > loader should avoid calling _dl_map_object_deps. _dl_map_object_deps
>
> > is very time consuming on low-end platforms with lots of library
>
> > dependencies.
>
> >
>
> > Loader retrains the same functionality during flag promotions:
>
> > RTLD_GLOBAL
>
> > RTLD_NODELETE
>
> >
>
> > https://sourceware.org/bugzilla/show_bug.cgi?id=25615
>
> > ---
>
> >  elf/Makefile                  |  7 +++--
>
> >  elf/dl-open.c                 |  4 ++-
>
> >  elf/tst-dlopen-noload-liba.c  |  9 ++++++
>
> >  elf/tst-dlopen-noload-libb.c  |  8 +++++
>
> >  elf/tst-dlopen-noload-nodel.c |  8 +++++
>
> >  elf/tst-dlopen-noload-share.h |  5 ++++
>
> >  elf/tst-dlopen-noload.c       | 56
> +++++++++++++++++++++++++++++++++++
>
> >  7 files changed, 94 insertions(+), 3 deletions(-)
>
> >  create mode 100644 elf/tst-dlopen-noload-liba.c
>
> >  create mode 100644 elf/tst-dlopen-noload-libb.c
>
> >  create mode 100644 elf/tst-dlopen-noload-nodel.c
>
> >  create mode 100644 elf/tst-dlopen-noload-share.h
>
> >  create mode 100644 elf/tst-dlopen-noload.c
>
> >
>
> > diff --git a/elf/Makefile b/elf/Makefile
>
> > index a2c3b12007..28cb787680 100644
>
> > --- a/elf/Makefile
>
> > +++ b/elf/Makefile
>
> > @@ -216,7 +216,7 @@ tests-internal += loadtest unload unload2
> > circleload1 \
>
> >   neededtest neededtest2 neededtest3 neededtest4 \
>
> >   tst-tls3 tst-tls6 tst-tls7 tst-tls8 tst-dlmopen2 \
>
> >   tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym \
>
> > - tst-create_format1
>
> > + tst-create_format1 tst-dlopen-noload
>
> >  tests-container += tst-pldd tst-dlopen-tlsmodid-container \
>
> >    tst-dlopen-self-container
>
> >  test-srcs = tst-pathopt
>
> > @@ -328,7 +328,8 @@ modules-names = testobj1 testobj2 testobj3
> > testobj4 testobj5 testobj6 \
>
> >   tst-single_threaded-mod3 tst-single_threaded-mod4 \
>
> >   tst-tls-ie-mod0 tst-tls-ie-mod1 tst-tls-ie-mod2 \
>
> >   tst-tls-ie-mod3 tst-tls-ie-mod4 tst-tls-ie-mod5 \
>
> > - tst-tls-ie-mod6
>
> > + tst-tls-ie-mod6 tst-dlopen-noload-libb tst-dlopen-noload-liba
> \
>
> > + tst-dlopen-noload-nodel
>
> >
>
> >  # Most modules build with _ISOMAC defined, but those filtered out
>
> >  # depend on internal headers.
>
> > @@ -732,6 +733,8 @@ $(objpfx)tst-tlsalign:
> > $(objpfx)tst-tlsalign-lib.so
>
> >  $(objpfx)tst-nodelete-opened.out: $(objpfx)tst-nodelete-opened-lib.so
>
> >  $(objpfx)tst-nodelete-opened: $(libdl)
>
> >  $(objpfx)tst-noload: $(libdl)
>
> > +$(objpfx)tst-dlopen-noload-liba.so:
> > +$(objpfx)tst-dlopen-noload-libb.so
>
> > +$(objpfx)tst-dlopen-noload: $(libdl)
> > +$(objpfx)tst-dlopen-noload-libb.so
>
> >
>
> >  $(objpfx)tst-tlsalign-extern: $(objpfx)tst-tlsalign-vars.o
>
> >  $(objpfx)tst-tlsalign-extern-static: $(objpfx)tst-tlsalign-vars.o
>
> > diff --git a/elf/dl-open.c b/elf/dl-open.c
>
> > index 9b3606c491..cfef39ec53 100644
>
> > --- a/elf/dl-open.c
>
> > +++ b/elf/dl-open.c
>
> > @@ -542,7 +542,9 @@ dl_open_worker (void *a)
>
> >    ++new->l_direct_opencount;
>
> >
>
> >    /* It was already open.  */
>
> > -  if (__glibc_unlikely (new->l_searchlist.r_list != NULL))
>
> > +  if (__glibc_unlikely (new->l_searchlist.r_list != NULL) ||
>
> > +      (__glibc_unlikely (mode & RTLD_NOLOAD) &&
>
> > +       __glibc_unlikely (!(mode & RTLD_GLOBAL))))
>
> >      {
>
> >        /* Let the user know about the opencount.  */
>
> >        if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_FILES))
>
> > diff --git a/elf/tst-dlopen-noload-liba.c
> > b/elf/tst-dlopen-noload-liba.c
>
> > new file mode 100644
>
> > index 0000000000..8a5937c2f4
>
> > --- /dev/null
>
> > +++ b/elf/tst-dlopen-noload-liba.c
>
> > @@ -0,0 +1,9 @@
>
> > +#include <stdio.h>
>
> > +
>
> > +#include "tst-dlopen-noload-share.h"
>
> > +
>
> > +void dlfunca(void)
>
> > +{
>
> > +    printf("dlfuna \n");
>
> > +    dlfuncb();
>
> > +}
>
> > diff --git a/elf/tst-dlopen-noload-libb.c
> > b/elf/tst-dlopen-noload-libb.c
>
> > new file mode 100644
>
> > index 0000000000..8dd8eb1880
>
> > --- /dev/null
>
> > +++ b/elf/tst-dlopen-noload-libb.c
>
> > @@ -0,0 +1,8 @@
>
> > +#include <stdio.h>
>
> > +
>
> > +#include "tst-dlopen-noload-share.h"
>
> > +
>
> > +void dlfuncb(void)
>
> > +{
>
> > +    printf("dlfunb \n");
>
> > +}
>
> > diff --git a/elf/tst-dlopen-noload-nodel.c
> > b/elf/tst-dlopen-noload-nodel.c
>
> > new file mode 100644
>
> > index 0000000000..0a5ab68db7
>
> > --- /dev/null
>
> > +++ b/elf/tst-dlopen-noload-nodel.c
>
> > @@ -0,0 +1,8 @@
>
> > +#include <stdio.h>
>
> > +
>
> > +#include "tst-dlopen-noload-share.h"
>
> > +
>
> > +void nodel_func(void)
>
> > +{
>
> > +    printf("nodel func\n");
>
> > +}
>
> > diff --git a/elf/tst-dlopen-noload-share.h
> > b/elf/tst-dlopen-noload-share.h
>
> > new file mode 100644
>
> > index 0000000000..6362d08548
>
> > --- /dev/null
>
> > +++ b/elf/tst-dlopen-noload-share.h
>
> > @@ -0,0 +1,5 @@
>
> > +
>
> > +void dlfunc(void);
>
> > +void dlfunca(void);
>
> > +void dlfuncb(void);
>
> > +
>
> > diff --git a/elf/tst-dlopen-noload.c b/elf/tst-dlopen-noload.c
>
> > new file mode 100644
>
> > index 0000000000..9699d9649d
>
> > --- /dev/null
>
> > +++ b/elf/tst-dlopen-noload.c
>
> > @@ -0,0 +1,56 @@
>
> > +#include <stdio.h>
>
> > +
>
> > +void *dlopen(const char *filename, int flags);
>
> > +
>
> > +int dlclose(void *handle);
>
> > +
>
> > +#include <dlfcn.h>
>
> > +
>
> > +void *dlmopen (Lmid_t lmid, const char *filename, int flags);
>
> > +
>
> > +#include "tst-dlopen-noload-share.h"
>
> > +
>
> > +int main(int argc, const char *argv[])
>
> > +{
>
> > +    void *handle, *handle2;
>
> > +    // promote RTLD_NODELETE
>
> > +    printf("dlopen libnodel.so RTLD_LAZY\n");
>
> > +    handle = dlopen("tst-dlopen-noload-nodel.so", RTLD_LAZY);
>
> > +    if (!handle) {
>
> > +        printf("dlopen libnodel.so failed: %s \n", dlerror());
>
> > +        return 1;
>
> > +    }
>
> > +
>
> > +    printf("dlopen libnodel.so
> > + RTLD_LAZY|RTLD_NOLOAD|RTLD_NODELETE\n");
>
> > +    handle2 = dlopen("tst-dlopen-noload-nodel.so",
> > + RTLD_LAZY|RTLD_NOLOAD|RTLD_NODELETE);
>
> > +    if (!handle2) {
>
> > +        printf("dlopen libnodel.so failed: %s \n", dlerror());
>
> > +        return 1;
>
> > +    }
>
> > +
>
> > +    // gdb print _rtld_global._dl_ns[0]._ns_nloaded should not
> > + decrease
>
> > +    dlclose(handle);
>
> > +    dlclose(handle2);
>
> > +
>
> > +    // test RTLD_LOCAL to RTLD_GLOBAL
>
> > +    printf("dlopen libliba.so RTLD_LAZY|RTLD_LOCAL\n");
>
> > +    handle = dlopen("tst-dlopen-noload-liba.so",
> > + RTLD_LAZY|RTLD_LOCAL);
>
> > +    if (!handle) {
>
> > +        printf("dlopen libliba.so failed: %s \n", dlerror());
>
> > +        return 1;
>
> > +    }
>
> > +
>
> > +    printf("dlopen liblibb.so RTLD_LAZY|RTLD_NOLOAD|RTLD_GLOBAL\n");
>
> > +    handle = dlopen("tst-dlopen-noload-libb.so",
> > + RTLD_LAZY|RTLD_NOLOAD|RTLD_GLOBAL);
>
> > +    if (!handle) {
>
> > +        printf("dlopen liblibb.so failed: %s \n", dlerror());
>
> > +        return 1;
>
> > +    }
>
> > +
>
> > +    // without promotion to RTLD_GLOBAL ltog_func will segfault
>
> > +    dlfuncb();
>
> > +
>
> > +    // won't be able to close because dlfunc is using it
>
> > +    dlclose (handle);
>
> > +    return 0;
>
> > +}
>
> > --
>
> > 2.17.1
>
> >
>
>
>
> --
> Cheers,
> Carlos.