Rename and split elf/tst-dlopen-aout collection of tests (was: Re: [PATCH] Split self-dlopen tests from elf/tst-dlopen-aout)

Message ID 87pniwzhsb.fsf_-_@oldenburg2.str.redhat.com
State New
Headers show
Series
  • Rename and split elf/tst-dlopen-aout collection of tests (was: Re: [PATCH] Split self-dlopen tests from elf/tst-dlopen-aout)
Related show

Commit Message

Florian Weimer Oct. 16, 2019, 6:06 p.m.
* Zack Weinberg:

> On Wed, Oct 16, 2019 at 11:06 AM Florian Weimer <fweimer@redhat.com> wrote:

>>

>> From the beginning, elf/tst-dlopen-aout has exercised two different

>> bugs: (a) failure to report errors for a dlopen of the executable

>> itself in some cases (bug 24900) and (b) incorrect rollback of the

>> TLS modid allocation in case of a dlopen failure (bug 16634).

>>

>> This patch retains elf/tst-dlopen-aout for (b) and introduces a new

>> set of tests, elf/tst-dlopen-self, for (a).  The elf/tst-dlopen-aout

>> tests use the elf/tst-dlopen-self binaries (or iconv), so they are

>> no longer self-dlopen tests.

>

> I don't know the dynamic linker well enough to review this change in

> detail.  I like the idea of splitting up these two tests.  I want to

> suggest that the (b) test should be renamed to something more

> descriptive at the same time as (a) is moved to its own binary.

> tst-dlopen-aout sounds like it tests something to do with an ELF

> executable trying to dlopen() an a.out-format shared library.  (I

> don't know if that was ever a thing that worked, but I could easily

> believe that some ancient attempt to make it work had backward

> compatibility implications that we're still stuck with, at least on

> the older ABIs.)


Oh right, very good point.  I faced the same confusion when I first
dealt with the test, but forgot about it.  Updated patch below.

Thanks,
Florian

8<------------------------------------------------------------------8<
From the beginning, elf/tst-dlopen-aout has exercised two different
bugs: (a) failure to report errors for a dlopen of the executable
itself in some cases (bug 24900) and (b) incorrect rollback of the
TLS modid allocation in case of a dlopen failure (bug 16634).

This commit replaces the test with elf/tst-dlopen-self for (a) and
elf/tst-dlopen-tlsmodid for (b).  The latter tests use the
elf/tst-dlopen-self binaries (or iconv) with dlopen, so they are
no longer self-dlopen tests.

Tested on x86_64-linux-gnu and i686-linux-gnu, with a toolchain that
does not default to PIE.

-----
 elf/Makefile                                       | 32 +++++++++----
 elf/tst-dlopen-self-container.c                    | 19 ++++++++
 ...open-aout-container.c => tst-dlopen-self-pie.c} |  4 +-
 elf/tst-dlopen-self.c                              | 55 ++++++++++++++++++++++
 elf/tst-dlopen-tlsmodid-container.c                | 39 +++++++++++++++
 ...dlopen-aout-pie.c => tst-dlopen-tlsmodid-pie.c} |  5 +-
 elf/tst-dlopen-tlsmodid.c                          | 25 ++++++++++
 elf/{tst-dlopen-aout.c => tst-dlopen-tlsmodid.h}   | 22 +++++----
 8 files changed, 177 insertions(+), 24 deletions(-)

Comments

Carlos O'Donell Oct. 16, 2019, 9:12 p.m. | #1
On 10/16/19 2:06 PM, Florian Weimer wrote:
> * Zack Weinberg:

> 

>> On Wed, Oct 16, 2019 at 11:06 AM Florian Weimer <fweimer@redhat.com> wrote:

>>>

>>> From the beginning, elf/tst-dlopen-aout has exercised two different

>>> bugs: (a) failure to report errors for a dlopen of the executable

>>> itself in some cases (bug 24900) and (b) incorrect rollback of the

>>> TLS modid allocation in case of a dlopen failure (bug 16634).

>>>

>>> This patch retains elf/tst-dlopen-aout for (b) and introduces a new

>>> set of tests, elf/tst-dlopen-self, for (a).  The elf/tst-dlopen-aout

>>> tests use the elf/tst-dlopen-self binaries (or iconv), so they are

>>> no longer self-dlopen tests.

>>

>> I don't know the dynamic linker well enough to review this change in

>> detail.  I like the idea of splitting up these two tests.  I want to

>> suggest that the (b) test should be renamed to something more

>> descriptive at the same time as (a) is moved to its own binary.

>> tst-dlopen-aout sounds like it tests something to do with an ELF

>> executable trying to dlopen() an a.out-format shared library.  (I

>> don't know if that was ever a thing that worked, but I could easily

>> believe that some ancient attempt to make it work had backward

>> compatibility implications that we're still stuck with, at least on

>> the older ABIs.)

> 

> Oh right, very good point.  I faced the same confusion when I first

> dealt with the test, but forgot about it.  Updated patch below.

> 


OK for master (minor typo in comment).

The test split looks good to me. I evaluated that 24900 was the minimum
I would expect for a test case that needs to test that particularly code
path and it looks good to me. You don't need threads, or any TLS usage,
you just want self-dlopen.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>



> 

> 8<------------------------------------------------------------------8<

> From the beginning, elf/tst-dlopen-aout has exercised two different

> bugs: (a) failure to report errors for a dlopen of the executable

> itself in some cases (bug 24900) and (b) incorrect rollback of the

> TLS modid allocation in case of a dlopen failure (bug 16634).

> 

> This commit replaces the test with elf/tst-dlopen-self for (a) and

> elf/tst-dlopen-tlsmodid for (b).  The latter tests use the

> elf/tst-dlopen-self binaries (or iconv) with dlopen, so they are

> no longer self-dlopen tests.

> 

> Tested on x86_64-linux-gnu and i686-linux-gnu, with a toolchain that

> does not default to PIE.


OK.

> -----

>  elf/Makefile                                       | 32 +++++++++----

>  elf/tst-dlopen-self-container.c                    | 19 ++++++++

>  ...open-aout-container.c => tst-dlopen-self-pie.c} |  4 +-

>  elf/tst-dlopen-self.c                              | 55 ++++++++++++++++++++++

>  elf/tst-dlopen-tlsmodid-container.c                | 39 +++++++++++++++

>  ...dlopen-aout-pie.c => tst-dlopen-tlsmodid-pie.c} |  5 +-

>  elf/tst-dlopen-tlsmodid.c                          | 25 ++++++++++

>  elf/{tst-dlopen-aout.c => tst-dlopen-tlsmodid.h}   | 22 +++++----

>  8 files changed, 177 insertions(+), 24 deletions(-)


OK. 

So we split like this:

tst-dlopen-aout -> tst-dlopen-tlsmodid + tst-dlopen-self (explicitly marked no-pie)

tst-dlopen-aout-container -> tst-dlopen-tlsmodid-container + tst-dlopen-self-container

tst-dlopen-aout-pie -> tst-dlopen-tlsmodid-pie + tst-dlopen-self-pie

Looks good to me.

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

> index dea51ca182..5e4cdb494f 100644

> --- a/elf/Makefile

> +++ b/elf/Makefile

> @@ -192,14 +192,16 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \

>  	 tst-latepthread tst-tls-manydynamic tst-nodelete-dlclose \

>  	 tst-debug1 tst-main1 tst-absolute-sym tst-absolute-zero tst-big-note \

>  	 tst-unwind-ctor tst-unwind-main tst-audit13 \

> -	 tst-sonamemove-link tst-sonamemove-dlopen tst-dlopen-aout

> +	 tst-sonamemove-link tst-sonamemove-dlopen tst-dlopen-tlsmodid \

> +	 tst-dlopen-self


OK. Replace old test with two new tests.

>  #	 reldep9

>  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

> -tests-container += tst-pldd tst-dlopen-aout-container

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

> +  tst-dlopen-self-container


OK. Two containers which exercise $ORIGIN.

>  test-srcs = tst-pathopt

>  selinux-enabled := $(shell cat /selinux/enforce 2> /dev/null)

>  ifneq ($(selinux-enabled),1)

> @@ -308,8 +310,9 @@ test-xfail-tst-protected1b = yes

>  endif

>  ifeq (yesyes,$(have-fpie)$(build-shared))

>  modules-names += tst-piemod1

> -tests += tst-pie1 tst-pie2 tst-dlopen-pie tst-dlopen-aout-pie

> -tests-pie += tst-pie1 tst-pie2 tst-dlopen-aout-pie

> +tests += tst-pie1 tst-pie2 tst-dlopen-pie tst-dlopen-tlsmodid-pie \

> +  tst-dlopen-self-pie


OK. Two PIE tests.

> +tests-pie += tst-pie1 tst-pie2 tst-dlopen-tlsmodid-pie tst-dlopen-self-pie

>  ifeq (yes,$(have-protected-data))

>  tests += vismain

>  tests-pie += vismain

> @@ -1268,12 +1271,21 @@ $(objpfx)tst-addr1: $(libdl)

>  

>  $(objpfx)tst-thrlock: $(libdl) $(shared-thread-library)

>  

> -tst-tst-dlopen-aout-no-pie = yes

> -$(objpfx)tst-dlopen-aout: $(libdl) $(shared-thread-library)

> -CFLAGS-tst-dlopen-aout-pie.c += $(pie-ccflag)

> -$(objpfx)tst-dlopen-aout-pie: $(libdl) $(shared-thread-library)

> -$(objpfx)tst-dlopen-aout-container: $(libdl) $(shared-thread-library)

> -LDFLAGS-tst-dlopen-aout-container += -Wl,-rpath,\$$ORIGIN

> +tst-tst-dlopen-tlsmodid-no-pie = yes

> +$(objpfx)tst-dlopen-tlsmodid: $(libdl) $(shared-thread-library)

> +$(objpfx)tst-dlopen-tlsmodid.out: $(objpfx)tst-dlopen-self

> +CFLAGS-tst-dlopen-tlsmodid-pie.c += $(pie-ccflag)

> +$(objpfx)tst-dlopen-tlsmodid-pie: $(libdl) $(shared-thread-library)

> +$(objpfx)tst-dlopen-tlsmodid-pie.out: $(objpfx)tst-dlopen-self-pie

> +$(objpfx)tst-dlopen-tlsmodid-container: $(libdl) $(shared-thread-library)

> +LDFLAGS-tst-dlopen-tlsmodid-container += -Wl,-rpath,\$$ORIGIN

> +

> +tst-tst-dlopen-self-no-pie = yes

> +$(objpfx)tst-dlopen-self: $(libdl)

> +CFLAGS-tst-dlopen-self-pie.c += $(pie-ccflag)

> +$(objpfx)tst-dlopen-self-pie: $(libdl)

> +$(objpfx)tst-dlopen-self-container: $(libdl)

> +LDFLAGS-tst-dlopen-self-container += -Wl,-rpath,\$$ORIGIN

>  

>  CFLAGS-ifuncmain1pic.c += $(pic-ccflag)

>  CFLAGS-ifuncmain1picstatic.c += $(pic-ccflag)

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

> new file mode 100644

> index 0000000000..b1db238dec

> --- /dev/null

> +++ b/elf/tst-dlopen-self-container.c

> @@ -0,0 +1,19 @@

> +/* Check dlopen'ing the executable itself fails (bug 24900); container version.

> +   Copyright (C) 2014-2019 Free Software Foundation, Inc.

> +   This file is part of the GNU C Library.

> +

> +   The GNU C Library is free software; you can redistribute it and/or

> +   modify it under the terms of the GNU Lesser General Public

> +   License as published by the Free Software Foundation; either

> +   version 2.1 of the License, or (at your option) any later version.

> +

> +   The GNU C Library is distributed in the hope that it will be useful,

> +   but WITHOUT ANY WARRANTY; without even the implied warranty of

> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU

> +   Lesser General Public License for more details.

> +

> +   You should have received a copy of the GNU Lesser General Public

> +   License along with the GNU C Library; if not, see

> +   <https://www.gnu.org/licenses/>.  */

> +

> +#include "tst-dlopen-self.c"


OK.

> diff --git a/elf/tst-dlopen-aout-container.c b/elf/tst-dlopen-self-pie.c

> similarity index 87%

> rename from elf/tst-dlopen-aout-container.c

> rename to elf/tst-dlopen-self-pie.c

> index 702dbe04c4..855bccc5b5 100644

> --- a/elf/tst-dlopen-aout-container.c

> +++ b/elf/tst-dlopen-self-pie.c

> @@ -1,4 +1,4 @@

> -/* Test case for BZ #16634 and BZ#24900.  Container version.

> +/* Check that dlopen'ing the executable itself fails (bug 24900); PIE version.


OK.

>     Copyright (C) 2014-2019 Free Software Foundation, Inc.

>     This file is part of the GNU C Library.

>  

> @@ -16,4 +16,4 @@

>     License along with the GNU C Library; if not, see

>     <https://www.gnu.org/licenses/>.  */

>  

> -#include "tst-dlopen-aout.c"

> +#include "tst-dlopen-self.c"


OK.

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

> new file mode 100644

> index 0000000000..310f1b09d4

> --- /dev/null

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

> @@ -0,0 +1,55 @@

> +/* Check that dlopen'ing the executable itself fails (bug 24900).

> +   Copyright (C) 2014-2019 Free Software Foundation, Inc.

> +   This file is part of the GNU C Library.

> +

> +   The GNU C Library is free software; you can redistribute it and/or

> +   modify it under the terms of the GNU Lesser General Public

> +   License as published by the Free Software Foundation; either

> +   version 2.1 of the License, or (at your option) any later version.

> +

> +   The GNU C Library is distributed in the hope that it will be useful,

> +   but WITHOUT ANY WARRANTY; without even the implied warranty of

> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU

> +   Lesser General Public License for more details.

> +

> +   You should have received a copy of the GNU Lesser General Public

> +   License along with the GNU C Library; if not, see

> +   <https://www.gnu.org/licenses/>.  */

> +

> +#include <dlfcn.h>

> +#include <stdlib.h>

> +#include <string.h>

> +#include <support/check.h>

> +

> +/* Call dlopen and check that fails with an error message indicating

> +   an attempt to open an ET_EXEC or PIE object.  */

> +static void

> +check_dlopen_failure (const char *path)

> +{

> +  void *handle = dlopen (path, RTLD_LAZY);

> +  if (handle != NULL)

> +    FAIL_EXIT1 ("dlopen succeeded unexpectedly: %s", path);

> +

> +  const char *message = dlerror ();

> +  TEST_VERIFY_EXIT (message != NULL);

> +  if ((strstr (message,

> +	       "cannot dynamically load position-independent executable")

> +       == NULL)

> +      && strstr (message, "cannot dynamically load executable") == NULL)

> +    FAIL_EXIT1 ("invalid dlopen error message: \"%s\"", message);

> +}


OK. Matches original test.

> +

> +static int

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

> +{

> +  check_dlopen_failure (argv[0]);

> +

> +  char *full_path = realpath (argv[0], NULL);

> +  check_dlopen_failure  (full_path);

> +  free (full_path);

> +

> +  return 0;

> +}


OK. No threads, just the dlopen failure. This is the minimal test case for bug 24900.

> +

> +#define TEST_FUNCTION_ARGV do_test

> +#include <support/test-driver.c>

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

> new file mode 100644

> index 0000000000..7654fcb214

> --- /dev/null

> +++ b/elf/tst-dlopen-tlsmodid-container.c

> @@ -0,0 +1,39 @@

> +/* Test case for BZ #16634.  Container version.

> +   Copyright (C) 2014-2019 Free Software Foundation, Inc.

> +   This file is part of the GNU C Library.

> +

> +   The GNU C Library is free software; you can redistribute it and/or

> +   modify it under the terms of the GNU Lesser General Public

> +   License as published by the Free Software Foundation; either

> +   version 2.1 of the License, or (at your option) any later version.

> +

> +   The GNU C Library is distributed in the hope that it will be useful,

> +   but WITHOUT ANY WARRANTY; without even the implied warranty of

> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU

> +   Lesser General Public License for more details.

> +

> +   You should have received a copy of the GNU Lesser General Public

> +   License along with the GNU C Library; if not, see

> +   <https://www.gnu.org/licenses/>.  */

> +

> +/* This test use the iconv program as the test binary.  */


s/use/uses/g.

OK, using any binary inside the build is OK.

> +

> +#include <stdlib.h>

> +#include <support/support.h>

> +

> +static char *iconv_path;

> +

> +static __attribute__ ((constructor)) void

> +iconv_path_init (void)

> +{

> +  iconv_path = xasprintf ("%s/iconv", support_bindir_prefix);

> +}

> +

> +static __attribute__ ((destructor)) void

> +iconv_path_fini (void)

> +{

> +  free (iconv_path);

> +}



OK.

> +

> +#define TST_DLOPEN_TLSMODID_PATH iconv_path

> +#include "tst-dlopen-tlsmodid.h"


OK.

> diff --git a/elf/tst-dlopen-aout-pie.c b/elf/tst-dlopen-tlsmodid-pie.c

> similarity index 85%

> rename from elf/tst-dlopen-aout-pie.c

> rename to elf/tst-dlopen-tlsmodid-pie.c

> index 8d2009c0f3..db1176b12c 100644

> --- a/elf/tst-dlopen-aout-pie.c

> +++ b/elf/tst-dlopen-tlsmodid-pie.c

> @@ -1,4 +1,4 @@

> -/* Test case for BZ #16634 and BZ#24900.  PIE version.

> +/* Test case for BZ #16634.  PIE version.


OK.

>     Copyright (C) 2014-2019 Free Software Foundation, Inc.

>     This file is part of the GNU C Library.

>  

> @@ -16,4 +16,5 @@

>     License along with the GNU C Library; if not, see

>     <http://www.gnu.org/licenses/>.  */

>  

> -#include "tst-dlopen-aout.c"

> +#define TST_DLOPEN_TLSMODID_PATH "tst-dlopen-self-pie"

> +#include "tst-dlopen-tlsmodid.h"


OK.

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

> new file mode 100644

> index 0000000000..165edcb552

> --- /dev/null

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

> @@ -0,0 +1,25 @@

> +/* Test case for BZ #16634.  Non-PIE version.


OK.

> +

> +   Verify that incorrectly dlopen()ing an executable without

> +   __RTLD_OPENEXEC does not cause assertion in ld.so, and that it

> +   actually results in an error.

> +

> +   Copyright (C) 2014-2019 Free Software Foundation, Inc.

> +   This file is part of the GNU C Library.

> +

> +   The GNU C Library is free software; you can redistribute it and/or

> +   modify it under the terms of the GNU Lesser General Public

> +   License as published by the Free Software Foundation; either

> +   version 2.1 of the License, or (at your option) any later version.

> +

> +   The GNU C Library is distributed in the hope that it will be useful,

> +   but WITHOUT ANY WARRANTY; without even the implied warranty of

> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU

> +   Lesser General Public License for more details.

> +

> +   You should have received a copy of the GNU Lesser General Public

> +   License along with the GNU C Library; if not, see

> +   <https://www.gnu.org/licenses/>.  */

> +

> +#define TST_DLOPEN_TLSMODID_PATH "tst-dlopen-self"

> +#include "tst-dlopen-tlsmodid.h"


OK.

> diff --git a/elf/tst-dlopen-aout.c b/elf/tst-dlopen-tlsmodid.h

> similarity index 77%

> rename from elf/tst-dlopen-aout.c

> rename to elf/tst-dlopen-tlsmodid.h

> index b86d082bc1..c747cb1491 100644

> --- a/elf/tst-dlopen-aout.c

> +++ b/elf/tst-dlopen-tlsmodid.h

> @@ -1,4 +1,5 @@

> -/* Test case for BZ #16634 and BZ#24900.

> +/* Common code for tst-dlopen-tlsmodid, tst-dlopen-tlsmodid-pie,

> +   tst-dlopen-tlsmodid-container.


OK.

>  

>     Verify that incorrectly dlopen()ing an executable without

>     __RTLD_OPENEXEC does not cause assertion in ld.so, and that it

> @@ -21,6 +22,9 @@

>     License along with the GNU C Library; if not, see

>     <https://www.gnu.org/licenses/>.  */

>  

> +/* Before including this file, the macro TST_DLOPEN_TLSMODID_PATH must

> +   be defined, to specify the path used for the open operation.  */

> +


OK.

>  #include <dlfcn.h>

>  #include <pthread.h>

>  #include <stdio.h>

> @@ -38,14 +42,14 @@ fn (void *p)

>    return p;

>  }

>  

> -/* Call dlopen on PATH and check that fails with an error message

> -   indicating an attempt to open an ET_EXEC or PIE object.  */

> +/* Call dlopen and check that fails with an error message indicating

> +   an attempt to open an ET_EXEC or PIE object.  */

>  static void

> -check_dlopen_failure (const char *path)

> +check_dlopen_failure (void)

>  {

> -  void *handle = dlopen (path, RTLD_LAZY);

> +  void *handle = dlopen (TST_DLOPEN_TLSMODID_PATH, RTLD_LAZY);

>    if (handle != NULL)

> -    FAIL_EXIT1 ("dlopen succeeded unexpectedly: %s", path);

> +    FAIL_EXIT1 ("dlopen succeeded unexpectedly: %s", TST_DLOPEN_TLSMODID_PATH);

>  

>    const char *message = dlerror ();

>    TEST_VERIFY_EXIT (message != NULL);

> @@ -65,7 +69,7 @@ do_test (int argc, char *argv[])

>      {

>        pthread_t thr;

>  

> -      check_dlopen_failure (argv[0]);

> +      check_dlopen_failure ();


OK.

>  

>        /* We create threads to force TLS allocation, which triggers

>  	 the original bug i.e. running out of surplus slotinfo entries

> @@ -74,9 +78,7 @@ do_test (int argc, char *argv[])

>        xpthread_join (thr);

>      }

>  

> -  /* The elf subdirectory (or $ORIGIN in the container case) is on the

> -     library search path.  */

> -  check_dlopen_failure ("tst-dlopen-aout");

> +  check_dlopen_failure ();


OK. Same test as before but made common.

>  

>    return 0;

>  }

> 



-- 
Cheers,
Carlos.

Patch

diff --git a/elf/Makefile b/elf/Makefile
index dea51ca182..5e4cdb494f 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -192,14 +192,16 @@  tests += restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-latepthread tst-tls-manydynamic tst-nodelete-dlclose \
 	 tst-debug1 tst-main1 tst-absolute-sym tst-absolute-zero tst-big-note \
 	 tst-unwind-ctor tst-unwind-main tst-audit13 \
-	 tst-sonamemove-link tst-sonamemove-dlopen tst-dlopen-aout
+	 tst-sonamemove-link tst-sonamemove-dlopen tst-dlopen-tlsmodid \
+	 tst-dlopen-self
 #	 reldep9
 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
-tests-container += tst-pldd tst-dlopen-aout-container
+tests-container += tst-pldd tst-dlopen-tlsmodid-container \
+  tst-dlopen-self-container
 test-srcs = tst-pathopt
 selinux-enabled := $(shell cat /selinux/enforce 2> /dev/null)
 ifneq ($(selinux-enabled),1)
@@ -308,8 +310,9 @@  test-xfail-tst-protected1b = yes
 endif
 ifeq (yesyes,$(have-fpie)$(build-shared))
 modules-names += tst-piemod1
-tests += tst-pie1 tst-pie2 tst-dlopen-pie tst-dlopen-aout-pie
-tests-pie += tst-pie1 tst-pie2 tst-dlopen-aout-pie
+tests += tst-pie1 tst-pie2 tst-dlopen-pie tst-dlopen-tlsmodid-pie \
+  tst-dlopen-self-pie
+tests-pie += tst-pie1 tst-pie2 tst-dlopen-tlsmodid-pie tst-dlopen-self-pie
 ifeq (yes,$(have-protected-data))
 tests += vismain
 tests-pie += vismain
@@ -1268,12 +1271,21 @@  $(objpfx)tst-addr1: $(libdl)
 
 $(objpfx)tst-thrlock: $(libdl) $(shared-thread-library)
 
-tst-tst-dlopen-aout-no-pie = yes
-$(objpfx)tst-dlopen-aout: $(libdl) $(shared-thread-library)
-CFLAGS-tst-dlopen-aout-pie.c += $(pie-ccflag)
-$(objpfx)tst-dlopen-aout-pie: $(libdl) $(shared-thread-library)
-$(objpfx)tst-dlopen-aout-container: $(libdl) $(shared-thread-library)
-LDFLAGS-tst-dlopen-aout-container += -Wl,-rpath,\$$ORIGIN
+tst-tst-dlopen-tlsmodid-no-pie = yes
+$(objpfx)tst-dlopen-tlsmodid: $(libdl) $(shared-thread-library)
+$(objpfx)tst-dlopen-tlsmodid.out: $(objpfx)tst-dlopen-self
+CFLAGS-tst-dlopen-tlsmodid-pie.c += $(pie-ccflag)
+$(objpfx)tst-dlopen-tlsmodid-pie: $(libdl) $(shared-thread-library)
+$(objpfx)tst-dlopen-tlsmodid-pie.out: $(objpfx)tst-dlopen-self-pie
+$(objpfx)tst-dlopen-tlsmodid-container: $(libdl) $(shared-thread-library)
+LDFLAGS-tst-dlopen-tlsmodid-container += -Wl,-rpath,\$$ORIGIN
+
+tst-tst-dlopen-self-no-pie = yes
+$(objpfx)tst-dlopen-self: $(libdl)
+CFLAGS-tst-dlopen-self-pie.c += $(pie-ccflag)
+$(objpfx)tst-dlopen-self-pie: $(libdl)
+$(objpfx)tst-dlopen-self-container: $(libdl)
+LDFLAGS-tst-dlopen-self-container += -Wl,-rpath,\$$ORIGIN
 
 CFLAGS-ifuncmain1pic.c += $(pic-ccflag)
 CFLAGS-ifuncmain1picstatic.c += $(pic-ccflag)
diff --git a/elf/tst-dlopen-self-container.c b/elf/tst-dlopen-self-container.c
new file mode 100644
index 0000000000..b1db238dec
--- /dev/null
+++ b/elf/tst-dlopen-self-container.c
@@ -0,0 +1,19 @@ 
+/* Check dlopen'ing the executable itself fails (bug 24900); container version.
+   Copyright (C) 2014-2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include "tst-dlopen-self.c"
diff --git a/elf/tst-dlopen-aout-container.c b/elf/tst-dlopen-self-pie.c
similarity index 87%
rename from elf/tst-dlopen-aout-container.c
rename to elf/tst-dlopen-self-pie.c
index 702dbe04c4..855bccc5b5 100644
--- a/elf/tst-dlopen-aout-container.c
+++ b/elf/tst-dlopen-self-pie.c
@@ -1,4 +1,4 @@ 
-/* Test case for BZ #16634 and BZ#24900.  Container version.
+/* Check that dlopen'ing the executable itself fails (bug 24900); PIE version.
    Copyright (C) 2014-2019 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
@@ -16,4 +16,4 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include "tst-dlopen-aout.c"
+#include "tst-dlopen-self.c"
diff --git a/elf/tst-dlopen-self.c b/elf/tst-dlopen-self.c
new file mode 100644
index 0000000000..310f1b09d4
--- /dev/null
+++ b/elf/tst-dlopen-self.c
@@ -0,0 +1,55 @@ 
+/* Check that dlopen'ing the executable itself fails (bug 24900).
+   Copyright (C) 2014-2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <dlfcn.h>
+#include <stdlib.h>
+#include <string.h>
+#include <support/check.h>
+
+/* Call dlopen and check that fails with an error message indicating
+   an attempt to open an ET_EXEC or PIE object.  */
+static void
+check_dlopen_failure (const char *path)
+{
+  void *handle = dlopen (path, RTLD_LAZY);
+  if (handle != NULL)
+    FAIL_EXIT1 ("dlopen succeeded unexpectedly: %s", path);
+
+  const char *message = dlerror ();
+  TEST_VERIFY_EXIT (message != NULL);
+  if ((strstr (message,
+	       "cannot dynamically load position-independent executable")
+       == NULL)
+      && strstr (message, "cannot dynamically load executable") == NULL)
+    FAIL_EXIT1 ("invalid dlopen error message: \"%s\"", message);
+}
+
+static int
+do_test (int argc, char *argv[])
+{
+  check_dlopen_failure (argv[0]);
+
+  char *full_path = realpath (argv[0], NULL);
+  check_dlopen_failure  (full_path);
+  free (full_path);
+
+  return 0;
+}
+
+#define TEST_FUNCTION_ARGV do_test
+#include <support/test-driver.c>
diff --git a/elf/tst-dlopen-tlsmodid-container.c b/elf/tst-dlopen-tlsmodid-container.c
new file mode 100644
index 0000000000..7654fcb214
--- /dev/null
+++ b/elf/tst-dlopen-tlsmodid-container.c
@@ -0,0 +1,39 @@ 
+/* Test case for BZ #16634.  Container version.
+   Copyright (C) 2014-2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+/* This test use the iconv program as the test binary.  */
+
+#include <stdlib.h>
+#include <support/support.h>
+
+static char *iconv_path;
+
+static __attribute__ ((constructor)) void
+iconv_path_init (void)
+{
+  iconv_path = xasprintf ("%s/iconv", support_bindir_prefix);
+}
+
+static __attribute__ ((destructor)) void
+iconv_path_fini (void)
+{
+  free (iconv_path);
+}
+
+#define TST_DLOPEN_TLSMODID_PATH iconv_path
+#include "tst-dlopen-tlsmodid.h"
diff --git a/elf/tst-dlopen-aout-pie.c b/elf/tst-dlopen-tlsmodid-pie.c
similarity index 85%
rename from elf/tst-dlopen-aout-pie.c
rename to elf/tst-dlopen-tlsmodid-pie.c
index 8d2009c0f3..db1176b12c 100644
--- a/elf/tst-dlopen-aout-pie.c
+++ b/elf/tst-dlopen-tlsmodid-pie.c
@@ -1,4 +1,4 @@ 
-/* Test case for BZ #16634 and BZ#24900.  PIE version.
+/* Test case for BZ #16634.  PIE version.
    Copyright (C) 2014-2019 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
@@ -16,4 +16,5 @@ 
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include "tst-dlopen-aout.c"
+#define TST_DLOPEN_TLSMODID_PATH "tst-dlopen-self-pie"
+#include "tst-dlopen-tlsmodid.h"
diff --git a/elf/tst-dlopen-tlsmodid.c b/elf/tst-dlopen-tlsmodid.c
new file mode 100644
index 0000000000..165edcb552
--- /dev/null
+++ b/elf/tst-dlopen-tlsmodid.c
@@ -0,0 +1,25 @@ 
+/* Test case for BZ #16634.  Non-PIE version.
+
+   Verify that incorrectly dlopen()ing an executable without
+   __RTLD_OPENEXEC does not cause assertion in ld.so, and that it
+   actually results in an error.
+
+   Copyright (C) 2014-2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#define TST_DLOPEN_TLSMODID_PATH "tst-dlopen-self"
+#include "tst-dlopen-tlsmodid.h"
diff --git a/elf/tst-dlopen-aout.c b/elf/tst-dlopen-tlsmodid.h
similarity index 77%
rename from elf/tst-dlopen-aout.c
rename to elf/tst-dlopen-tlsmodid.h
index b86d082bc1..c747cb1491 100644
--- a/elf/tst-dlopen-aout.c
+++ b/elf/tst-dlopen-tlsmodid.h
@@ -1,4 +1,5 @@ 
-/* Test case for BZ #16634 and BZ#24900.
+/* Common code for tst-dlopen-tlsmodid, tst-dlopen-tlsmodid-pie,
+   tst-dlopen-tlsmodid-container.
 
    Verify that incorrectly dlopen()ing an executable without
    __RTLD_OPENEXEC does not cause assertion in ld.so, and that it
@@ -21,6 +22,9 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+/* Before including this file, the macro TST_DLOPEN_TLSMODID_PATH must
+   be defined, to specify the path used for the open operation.  */
+
 #include <dlfcn.h>
 #include <pthread.h>
 #include <stdio.h>
@@ -38,14 +42,14 @@  fn (void *p)
   return p;
 }
 
-/* Call dlopen on PATH and check that fails with an error message
-   indicating an attempt to open an ET_EXEC or PIE object.  */
+/* Call dlopen and check that fails with an error message indicating
+   an attempt to open an ET_EXEC or PIE object.  */
 static void
-check_dlopen_failure (const char *path)
+check_dlopen_failure (void)
 {
-  void *handle = dlopen (path, RTLD_LAZY);
+  void *handle = dlopen (TST_DLOPEN_TLSMODID_PATH, RTLD_LAZY);
   if (handle != NULL)
-    FAIL_EXIT1 ("dlopen succeeded unexpectedly: %s", path);
+    FAIL_EXIT1 ("dlopen succeeded unexpectedly: %s", TST_DLOPEN_TLSMODID_PATH);
 
   const char *message = dlerror ();
   TEST_VERIFY_EXIT (message != NULL);
@@ -65,7 +69,7 @@  do_test (int argc, char *argv[])
     {
       pthread_t thr;
 
-      check_dlopen_failure (argv[0]);
+      check_dlopen_failure ();
 
       /* We create threads to force TLS allocation, which triggers
 	 the original bug i.e. running out of surplus slotinfo entries
@@ -74,9 +78,7 @@  do_test (int argc, char *argv[])
       xpthread_join (thr);
     }
 
-  /* The elf subdirectory (or $ORIGIN in the container case) is on the
-     library search path.  */
-  check_dlopen_failure ("tst-dlopen-aout");
+  check_dlopen_failure ();
 
   return 0;
 }