dlopen: Fix issues related to NODELETE handling and relocations

  The assumption behind the assert in activate_nodelete was wrong:

Inconsistency detected by ld.so: dl-open.c: 459: activate_nodelete:
Assertion `!imap->l_init_called || imap->l_type != lt_loaded' failed! (edit)

It can happen that an already-loaded object that is in the local
scope is promoted to NODELETE status, via binding to a unique
symbol.

Similarly, it is possible that such NODELETE promotion occurs to
an already-loaded object from the global scope.  This is why the
loop in activate_nodelete has to cover all objects in the namespace
of the new object.

In do_lookup_unique, it could happen that the NODELETE status of
an already-loaded object was overwritten with a pending NODELETE
status.  As a result, if dlopen fails, this could cause a loss of
the NODELETE status of the affected object, eventually resulting
in an incorrect unload.

Fixes commit f63b73814f74032c0e5d0a83300e3d864ef905e5 ("Remove all
loaded objects if dlopen fails, ignoring NODELETE [BZ #20839]").

Tested on x86_64-linux-gnu, i686-linux-gnu, powerpc64le-linux-gnu.

-----
 elf/Makefile                           |  66 +++++++++++-
 elf/dl-lookup.c                        |   6 +-
 elf/dl-open.c                          |  41 +++-----
 elf/tst-dlopen-nodelete-reloc-mod1.c   |  39 ++++++++
 elf/tst-dlopen-nodelete-reloc-mod10.c  |  41 ++++++++
 elf/tst-dlopen-nodelete-reloc-mod11.cc |  49 +++++++++
 elf/tst-dlopen-nodelete-reloc-mod12.cc |  42 ++++++++
 elf/tst-dlopen-nodelete-reloc-mod13.cc |  48 +++++++++
 elf/tst-dlopen-nodelete-reloc-mod13.h  |  24 +++++
 elf/tst-dlopen-nodelete-reloc-mod14.cc |  42 ++++++++
 elf/tst-dlopen-nodelete-reloc-mod15.cc |  41 ++++++++
 elf/tst-dlopen-nodelete-reloc-mod16.c  |  27 +++++
 elf/tst-dlopen-nodelete-reloc-mod17.c  |  19 ++++
 elf/tst-dlopen-nodelete-reloc-mod2.c   |  38 +++++++
 elf/tst-dlopen-nodelete-reloc-mod3.c   |  38 +++++++
 elf/tst-dlopen-nodelete-reloc-mod4.c   |  37 +++++++
 elf/tst-dlopen-nodelete-reloc-mod5.c   |  38 +++++++
 elf/tst-dlopen-nodelete-reloc-mod6.cc  |  42 ++++++++
 elf/tst-dlopen-nodelete-reloc-mod7.cc  |  48 +++++++++
 elf/tst-dlopen-nodelete-reloc-mod8.c   |  41 ++++++++
 elf/tst-dlopen-nodelete-reloc-mod9.cc  |  42 ++++++++
 elf/tst-dlopen-nodelete-reloc.c        | 178 +++++++++++++++++++++++++++++++++
 elf/tst-dlopen-nodelete-reloc.h        |  35 +++++++
 23 files changed, 990 insertions(+), 32 deletions(-)
  

* Florian Weimer:

> The assumption behind the assert in activate_nodelete was wrong:
>
> Inconsistency detected by ld.so: dl-open.c: 459: activate_nodelete:
> Assertion `!imap->l_init_called || imap->l_type != lt_loaded' failed! (edit)
>
> It can happen that an already-loaded object that is in the local
> scope is promoted to NODELETE status, via binding to a unique
> symbol.
>
> Similarly, it is possible that such NODELETE promotion occurs to
> an already-loaded object from the global scope.  This is why the
> loop in activate_nodelete has to cover all objects in the namespace
> of the new object.
>
> In do_lookup_unique, it could happen that the NODELETE status of
> an already-loaded object was overwritten with a pending NODELETE
> status.  As a result, if dlopen fails, this could cause a loss of
> the NODELETE status of the affected object, eventually resulting
> in an incorrect unload.
>
> Fixes commit f63b73814f74032c0e5d0a83300e3d864ef905e5 ("Remove all
> loaded objects if dlopen fails, ignoring NODELETE [BZ #20839]").
>
> Tested on x86_64-linux-gnu, i686-linux-gnu, powerpc64le-linux-gnu.

Ping.  This patch needs review.  It fixes a couple of regressions in the
dlopen NODELETE changes.

  <https://sourceware.org/ml/libc-alpha/2019-12/msg00098.html>

Thanks,
Florian
  

On 03/12/2019 14:16, Florian Weimer wrote:
> The assumption behind the assert in activate_nodelete was wrong:
> 
> Inconsistency detected by ld.so: dl-open.c: 459: activate_nodelete:
> Assertion `!imap->l_init_called || imap->l_type != lt_loaded' failed! (edit)
> 
> It can happen that an already-loaded object that is in the local
> scope is promoted to NODELETE status, via binding to a unique
> symbol.
> 
> Similarly, it is possible that such NODELETE promotion occurs to
> an already-loaded object from the global scope.  This is why the
> loop in activate_nodelete has to cover all objects in the namespace
> of the new object.
> 
> In do_lookup_unique, it could happen that the NODELETE status of
> an already-loaded object was overwritten with a pending NODELETE
> status.  As a result, if dlopen fails, this could cause a loss of
> the NODELETE status of the affected object, eventually resulting
> in an incorrect unload.
> 
> Fixes commit f63b73814f74032c0e5d0a83300e3d864ef905e5 ("Remove all
> loaded objects if dlopen fails, ignoring NODELETE [BZ #20839]").
> 
> Tested on x86_64-linux-gnu, i686-linux-gnu, powerpc64le-linux-gnu.

Wouldn't it need to be rebased on top of 'Remove signal blocking from
dlopen'? I find it confusing trying to review after just reviwing the
patch that refactors l_nodelete flag.

[1] https://sourceware.org/ml/libc-alpha/2019-12/msg00175.html
  

* Adhemerval Zanella:

> On 03/12/2019 14:16, Florian Weimer wrote:
>> The assumption behind the assert in activate_nodelete was wrong:
>> 
>> Inconsistency detected by ld.so: dl-open.c: 459: activate_nodelete:
>> Assertion `!imap->l_init_called || imap->l_type != lt_loaded' failed! (edit)
>> 
>> It can happen that an already-loaded object that is in the local
>> scope is promoted to NODELETE status, via binding to a unique
>> symbol.
>> 
>> Similarly, it is possible that such NODELETE promotion occurs to
>> an already-loaded object from the global scope.  This is why the
>> loop in activate_nodelete has to cover all objects in the namespace
>> of the new object.
>> 
>> In do_lookup_unique, it could happen that the NODELETE status of
>> an already-loaded object was overwritten with a pending NODELETE
>> status.  As a result, if dlopen fails, this could cause a loss of
>> the NODELETE status of the affected object, eventually resulting
>> in an incorrect unload.
>> 
>> Fixes commit f63b73814f74032c0e5d0a83300e3d864ef905e5 ("Remove all
>> loaded objects if dlopen fails, ignoring NODELETE [BZ #20839]").
>> 
>> Tested on x86_64-linux-gnu, i686-linux-gnu, powerpc64le-linux-gnu.
>
> Wouldn't it need to be rebased on top of 'Remove signal blocking from
> dlopen'? I find it confusing trying to review after just reviwing the
> patch that refactors l_nodelete flag.
>
> [1] https://sourceware.org/ml/libc-alpha/2019-12/msg00175.html

No, this patch has to go in first (see the cover letter for the other
patches).

Thank you for helping with reviews!

Florian
  

On 12/3/19 12:16 PM, Florian Weimer wrote:
> The assumption behind the assert in activate_nodelete was wrong:
> 
> Inconsistency detected by ld.so: dl-open.c: 459: activate_nodelete:
> Assertion `!imap->l_init_called || imap->l_type != lt_loaded' failed! (edit)
> 
> It can happen that an already-loaded object that is in the local
> scope is promoted to NODELETE status, via binding to a unique
> symbol.
> 
> Similarly, it is possible that such NODELETE promotion occurs to
> an already-loaded object from the global scope.  This is why the
> loop in activate_nodelete has to cover all objects in the namespace
> of the new object.
> 
> In do_lookup_unique, it could happen that the NODELETE status of
> an already-loaded object was overwritten with a pending NODELETE
> status.  As a result, if dlopen fails, this could cause a loss of
> the NODELETE status of the affected object, eventually resulting
> in an incorrect unload.

Thank you for working through this issue.

I think it's very valuable to have asserts like the original assert that
indicated clearly the expectations we had about current state of the
loader and objects.

Failing to identify the case with STB_GNU_UNIQUE is a normal process of
refining the implementation before release. I'm happy we're doing this
now and not late in January. Thank you for staying on top of the changes
and problems that cropped up in Firefox, Chrome, Calibre and other software
using either signals/dlopen or C++ (STB_GNU_UNIQUE) in ways we didn't
expect.

There are a couple of typos and one nit in mod15 fini to review.

Please post v2, but I expect we're done with that fixup, the rest looks
good to me.

> Fixes commit f63b73814f74032c0e5d0a83300e3d864ef905e5 ("Remove all
> loaded objects if dlopen fails, ignoring NODELETE [BZ #20839]").
> 
> Tested on x86_64-linux-gnu, i686-linux-gnu, powerpc64le-linux-gnu.
> 
> -----
>  elf/Makefile                           |  66 +++++++++++-
>  elf/dl-lookup.c                        |   6 +-
>  elf/dl-open.c                          |  41 +++-----
>  elf/tst-dlopen-nodelete-reloc-mod1.c   |  39 ++++++++
>  elf/tst-dlopen-nodelete-reloc-mod10.c  |  41 ++++++++
>  elf/tst-dlopen-nodelete-reloc-mod11.cc |  49 +++++++++
>  elf/tst-dlopen-nodelete-reloc-mod12.cc |  42 ++++++++
>  elf/tst-dlopen-nodelete-reloc-mod13.cc |  48 +++++++++
>  elf/tst-dlopen-nodelete-reloc-mod13.h  |  24 +++++
>  elf/tst-dlopen-nodelete-reloc-mod14.cc |  42 ++++++++
>  elf/tst-dlopen-nodelete-reloc-mod15.cc |  41 ++++++++
>  elf/tst-dlopen-nodelete-reloc-mod16.c  |  27 +++++
>  elf/tst-dlopen-nodelete-reloc-mod17.c  |  19 ++++
>  elf/tst-dlopen-nodelete-reloc-mod2.c   |  38 +++++++
>  elf/tst-dlopen-nodelete-reloc-mod3.c   |  38 +++++++
>  elf/tst-dlopen-nodelete-reloc-mod4.c   |  37 +++++++
>  elf/tst-dlopen-nodelete-reloc-mod5.c   |  38 +++++++
>  elf/tst-dlopen-nodelete-reloc-mod6.cc  |  42 ++++++++
>  elf/tst-dlopen-nodelete-reloc-mod7.cc  |  48 +++++++++
>  elf/tst-dlopen-nodelete-reloc-mod8.c   |  41 ++++++++
>  elf/tst-dlopen-nodelete-reloc-mod9.cc  |  42 ++++++++
>  elf/tst-dlopen-nodelete-reloc.c        | 178 +++++++++++++++++++++++++++++++++
>  elf/tst-dlopen-nodelete-reloc.h        |  35 +++++++
>  23 files changed, 990 insertions(+), 32 deletions(-)
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index 0debea7759..34573d2a21 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -191,7 +191,7 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
>  	 tst-audit1 tst-audit2 tst-audit8 tst-audit9 \
>  	 tst-addr1 tst-thrlock \
>  	 tst-unique1 tst-unique2 $(if $(CXX),tst-unique3 tst-unique4 \
> -	 tst-nodelete) \
> +	 tst-nodelete tst-dlopen-nodelete-reloc) \

OK. New test.

>  	 tst-initorder tst-initorder2 tst-relsort1 tst-null-argv \
>  	 tst-tlsalign tst-tlsalign-extern tst-nodelete-opened \
>  	 tst-nodelete2 tst-audit11 tst-audit12 tst-dlsym-error tst-noload \
> @@ -271,7 +271,24 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
>  		tst-auditmod9a tst-auditmod9b \
>  		$(if $(CXX),tst-unique3lib tst-unique3lib2 tst-unique4lib \
>  		  tst-nodelete-uniquemod tst-nodelete-rtldmod \
> -		  tst-nodelete-zmod) \
> +		  tst-nodelete-zmod \
> +                  tst-dlopen-nodelete-reloc-mod1 \
> +		  tst-dlopen-nodelete-reloc-mod2 \
> +	          tst-dlopen-nodelete-reloc-mod3 \
> +		  tst-dlopen-nodelete-reloc-mod4 \
> +		  tst-dlopen-nodelete-reloc-mod5 \
> +	          tst-dlopen-nodelete-reloc-mod6 \
> +	          tst-dlopen-nodelete-reloc-mod7 \
> +	          tst-dlopen-nodelete-reloc-mod8 \
> +	          tst-dlopen-nodelete-reloc-mod9 \
> +	          tst-dlopen-nodelete-reloc-mod10 \
> +	          tst-dlopen-nodelete-reloc-mod11 \
> +	          tst-dlopen-nodelete-reloc-mod12 \
> +	          tst-dlopen-nodelete-reloc-mod13 \
> +	          tst-dlopen-nodelete-reloc-mod14 \
> +	          tst-dlopen-nodelete-reloc-mod15 \
> +	          tst-dlopen-nodelete-reloc-mod16 \
> +		  tst-dlopen-nodelete-reloc-mod17) \

OK. Many new test DSOs, 17 to be exact.

>  		tst-initordera1 tst-initorderb1 \
>  		tst-initordera2 tst-initorderb2 \
>  		tst-initordera3 tst-initordera4 \
> @@ -1627,3 +1644,48 @@ $(objpfx)tst-dlopenfailmod1.so: \
>    $(shared-thread-library) $(objpfx)tst-dlopenfaillinkmod.so
>  LDFLAGS-tst-dlopenfaillinkmod.so = -Wl,-soname,tst-dlopenfail-missingmod.so
>  $(objpfx)tst-dlopenfailmod2.so: $(shared-thread-library)
> +
> +$(objpfx)tst-dlopen-nodelete-reloc: $(libdl)

OK. Test needs libdl.

> +$(objpfx)tst-dlopen-nodelete-reloc.out: \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod1.so \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod2.so \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod3.so \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod4.so \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod5.so \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod6.so \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod7.so \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod8.so \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod9.so \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod10.so \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod11.so \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod12.so \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod13.so \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod14.so \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod15.so \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod16.so \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod17.so

OK. Test requires all 17 DSOs to be built.

OK,
mod1
mod2 - nodelete
mod3
mod4 - need mod3
mod5 - need mod4, nodelete
mod6
mod7
mod8 - need libdl
mod9
mod10 - need libdl
mod11
mod12
mod13 - need mod12
mod14
mod15 - need mod14
mod16
mod17 - need mod15, need mod16

> +tst-dlopen-nodelete-reloc-mod2.so-no-z-defs = yes
> +LDFLAGS-tst-dlopen-nodelete-reloc-mod2.so = -Wl,-z,nodelete
> +$(objpfx)tst-dlopen-nodelete-reloc-mod4.so: \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod3.so
> +LDFLAGS-tst-dlopen-nodelete-reloc-mod4.so = -Wl,--no-as-needed
> +$(objpfx)tst-dlopen-nodelete-reloc-mod5.so: \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod4.so
> +LDFLAGS-tst-dlopen-nodelete-reloc-mod5.so = -Wl,-z,nodelete,--no-as-needed
> +tst-dlopen-nodelete-reloc-mod5.so-no-z-defs = yes
> +tst-dlopen-nodelete-reloc-mod7.so-no-z-defs = yes
> +$(objpfx)tst-dlopen-nodelete-reloc-mod8.so: $(libdl)
> +$(objpfx)tst-dlopen-nodelete-reloc-mod10.so: $(libdl)
> +tst-dlopen-nodelete-reloc-mod11.so-no-z-defs = yes
> +$(objpfx)tst-dlopen-nodelete-reloc-mod13.so: \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod12.so
> +$(objpfx)tst-dlopen-nodelete-reloc-mod15.so: \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod14.so
> +tst-dlopen-nodelete-reloc-mod16.so-no-z-defs = yes
> +$(objpfx)tst-dlopen-nodelete-reloc-mod16.so: \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod15.so
> +LDFLAGS-tst-dlopen-nodelete-reloc-mod16.so = -Wl,--no-as-needed
> +$(objpfx)tst-dlopen-nodelete-reloc-mod17.so: \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod15.so \
> +  $(objpfx)tst-dlopen-nodelete-reloc-mod16.so
> +LDFLAGS-tst-dlopen-nodelete-reloc-mod17.so = -Wl,--no-as-needed

OK. DSOs with their collection of various flags.

> diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c
> index a2e85a5568..99846918c3 100644
> --- a/elf/dl-lookup.c
> +++ b/elf/dl-lookup.c
> @@ -311,12 +311,12 @@ do_lookup_unique (const char *undef_name, uint_fast32_t new_hash,
>        enter_unique_sym (entries, size,
>                          new_hash, strtab + sym->st_name, sym, map);
>  
> -      if (map->l_type == lt_loaded)
> +      if (map->l_type == lt_loaded
> +	  && map->l_nodelete == link_map_nodelete_inactive)

OK. Must only do this work if l_nodelete is inactive, otherwise we may have
an object that is *already* marked nodelete. The context that isn't show below
is that we then do this:

 323           if (flags & DL_LOOKUP_FOR_RELOCATE)
 324             map->l_nodelete = link_map_nodelete_pending;
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This would reset the active state.

 325           else
 326             map->l_nodelete = link_map_nodelete_active;


>  	{
>  	  /* Make sure we don't unload this object by
>  	     setting the appropriate flag.  */
> -	  if (__glibc_unlikely (GLRO (dl_debug_mask) & DL_DEBUG_BINDINGS)
> -	      && map->l_nodelete == link_map_nodelete_inactive)
> +	  if (__glibc_unlikely (GLRO (dl_debug_mask) & DL_DEBUG_BINDINGS))

OK. You can always print this now because it's informative, you want to know
which symbol forced this particular object to NODELETE.

>  	    _dl_debug_printf ("\
>  marking %s [%lu] as NODELETE due to unique symbol\n",
>  			      map->l_name, map->l_ns);
> diff --git a/elf/dl-open.c b/elf/dl-open.c
> index df9f29a5e5..56f213323c 100644
> --- a/elf/dl-open.c
> +++ b/elf/dl-open.c
> @@ -433,34 +433,21 @@ TLS generation counter wrapped!  Please report this."));
>     after dlopen failure is not possible, so that _dl_close can clean
>     up objects if necessary.  */
>  static void
> -activate_nodelete (struct link_map *new, int mode)
> +activate_nodelete (struct link_map *new)

OK.

Comparing review in gerrit to local review.

To review this next chunk I did:

pwclient list -w fweimer@redhat.com
pwclient git-am 36479
git difftool -d -t meld HEAD~1

Then I get to see how activate_nodelete is properly converted without the diff
getting in the way.


/* Mark the objects as NODELETE if required.  This is delayed until
   after dlopen failure is not possible, so that _dl_close can clean
   up objects if necessary.  */
static void
activate_nodelete (struct link_map *new)
{
  /* It is necessary to traverse the entire namespace.  References to
     objects in the global scope and unique symbol bindings can force
     NODELETE status for objects outside the local scope.  */
  for (struct link_map *l = GL (dl_ns)[new->l_ns]._ns_loaded; l != NULL;
       l = l->l_next)
    if (l->l_nodelete == link_map_nodelete_pending)
      {
	if (__glibc_unlikely (GLRO (dl_debug_mask) & DL_DEBUG_FILES))
	  _dl_debug_printf ("activating NODELETE for %s [%lu]\n",
			    l->l_name, l->l_ns);

	l->l_nodelete = link_map_nodelete_active;
      }
}

This looks correct to me, it's a walk of the namespace for the object
and it fixes anything loaded in that namespace that's pending. We drop
the mode because it's no longer required.

>  {
> -  if (mode & RTLD_NODELETE || new->l_nodelete == link_map_nodelete_pending)
> -    {
> -      if (__glibc_unlikely (GLRO (dl_debug_mask) & DL_DEBUG_FILES))
> -	_dl_debug_printf ("activating NODELETE for %s [%lu]\n",
> -			  new->l_name, new->l_ns);
> -      new->l_nodelete = link_map_nodelete_active;
> -    }
> +  /* It is necessary to traverse the entire namespace.  References to
> +     objects in the global scope and unique symbol bindings can force
> +     NODELETE status for objects outside the local scope.  */

OK. Yes, this is correct, the STB_GNU_UNIQUE references can force NODELETE
on maps that were already loaded.

> +  for (struct link_map *l = GL (dl_ns)[new->l_ns]._ns_loaded; l != NULL;
> +       l = l->l_next)
> +    if (l->l_nodelete == link_map_nodelete_pending)
> +      {
> +	if (__glibc_unlikely (GLRO (dl_debug_mask) & DL_DEBUG_FILES))
> +	  _dl_debug_printf ("activating NODELETE for %s [%lu]\n",
> +			    l->l_name, l->l_ns);
>  
> -  for (unsigned int i = 0; i < new->l_searchlist.r_nlist; ++i)
> -    {
> -      struct link_map *imap = new->l_searchlist.r_list[i];
> -      if (imap->l_nodelete == link_map_nodelete_pending)
> -	{
> -	  if (__glibc_unlikely (GLRO (dl_debug_mask) & DL_DEBUG_FILES))
> -	    _dl_debug_printf ("activating NODELETE for %s [%lu]\n",
> -			      imap->l_name, imap->l_ns);
> -
> -	  /* Only new objects should have set
> -	     link_map_nodelete_pending.  Existing objects should not
> -	     have gained any new dependencies and therefore cannot
> -	     reach NODELETE status.  */
> -	  assert (!imap->l_init_called || imap->l_type != lt_loaded);
> -
> -	  imap->l_nodelete = link_map_nodelete_active;
> -	}
> -     }
> +	l->l_nodelete = link_map_nodelete_active;
> +      }
>  }
>  

OK.

>  /* struct dl_init_args and call_dl_init are used to call _dl_init with
> @@ -721,7 +708,7 @@ dl_open_worker (void *a)
>       All memory allocations for new objects must have happened
>       before.  */
>  
> -  activate_nodelete (new, mode);
> +  activate_nodelete (new);

OK.

>  
>    /* Second stage after resize_scopes: Actually perform the scope
>       update.  After this, dlsym and lazy binding can bind to new
> diff --git a/elf/tst-dlopen-nodelete-reloc-mod1.c b/elf/tst-dlopen-nodelete-reloc-mod1.c
> new file mode 100644
> index 0000000000..8927a9f851
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc-mod1.c
> @@ -0,0 +1,39 @@
> +/* Test propagation of NODELETE to an already-loaded object via relocation.
> +   Non-NODELETE helper module.
> +   Copyright (C) 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 <stdbool.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +/* Globally exported.  Set by the main program to true before
> +   termination, and used by tst-dlopen-nodelete-reloc-mod2.so to
> +   trigger marking his module as NODELETE (and also for its destructor

s/his/this/g

> +   check).  */
> +bool may_finalize_mod1 = false;
> +
> +static void __attribute__ ((destructor))
> +fini (void)
> +{
> +  if (!may_finalize_mod1)
> +    {
> +      puts ("error: tst-dlopen-nodelete-reloc-mod1.so destructor"
> +            " called too early");
> +      _exit (1);
> +    }
> +}
> diff --git a/elf/tst-dlopen-nodelete-reloc-mod10.c b/elf/tst-dlopen-nodelete-reloc-mod10.c
> new file mode 100644
> index 0000000000..30748b73ec
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc-mod10.c
> @@ -0,0 +1,41 @@
> +/* Helper module to load tst-dlopen-nodelete-reloc-mod11.so.
> +   Copyright (C) 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 <stddef.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +static void *handle;
> +
> +static void __attribute__ ((constructor))
> +init (void)
> +{
> +  handle = dlopen ("tst-dlopen-nodelete-reloc-mod11.so", RTLD_NOW);

OK.

> +  if (handle == NULL)
> +    {
> +      printf ("error: dlopen in module 10: %s\n", dlerror ());
> +      _exit (1);
> +    }
> +}
> +
> +static void __attribute__ ((destructor))
> +fini (void)
> +{
> +  dlclose (handle);
> +}
> diff --git a/elf/tst-dlopen-nodelete-reloc-mod11.cc b/elf/tst-dlopen-nodelete-reloc-mod11.cc
> new file mode 100644
> index 0000000000..48c910403e
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc-mod11.cc
> @@ -0,0 +1,49 @@
> +/* Second module defining a unique symbol (loaded indirectly).
> +   Copyright (C) 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-nodelete-reloc.h"
> +
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +/* Just a flag here, not used for NODELETE processing.  */
> +bool may_finalize_mod11 = false;

OK.

> +
> +/* Trigger the creation of a unique symbol reference.  This should
> +   cause tst-dlopen-nodelete-reloc-mod9.so to be marked as
> +   NODELETE.  */
> +
> +extern template struct unique_symbol<9>;

OK.

> +
> +int
> +global_function_mod11 (void)
> +{
> +  return unique_symbol<9>::value;
> +}
> +
> +static void __attribute__ ((destructor))
> +fini (void)
> +{
> +  if (!may_finalize_mod11)
> +    {
> +      puts ("error: tst-dlopen-nodelete-reloc-mod11.so destructor"
> +            " called too early");
> +      _exit (1);
> +    }

OK.

> +}
> diff --git a/elf/tst-dlopen-nodelete-reloc-mod12.cc b/elf/tst-dlopen-nodelete-reloc-mod12.cc
> new file mode 100644
> index 0000000000..5c093fd02d
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc-mod12.cc
> @@ -0,0 +1,42 @@
> +/* First module for NODELETE test defining a unique symbol (with DT_NEEDED).
> +   Copyright (C) 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-nodelete-reloc.h"
> +
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +/* Just a flag here, not used for NODELETE processing.  */
> +bool may_finalize_mod12 = false;
> +
> +/* Explicit instantiation.  This produces a unique symbol definition
> +   which is not referenced by the library itself, so the library is
> +   not marked NODELETE.  */
> +template struct unique_symbol<12>;
> +
> +static void __attribute__ ((destructor))
> +fini (void)
> +{
> +  if (!may_finalize_mod12)
> +    {
> +      puts ("error: tst-dlopen-nodelete-reloc-mod12.so destructor"
> +            " called too early");
> +      _exit (1);
> +    }
> +}
> diff --git a/elf/tst-dlopen-nodelete-reloc-mod13.cc b/elf/tst-dlopen-nodelete-reloc-mod13.cc
> new file mode 100644
> index 0000000000..caf4fd1cc9
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc-mod13.cc
> @@ -0,0 +1,48 @@
> +/* Second module for NODELETE test defining a unique symbol (with DT_NEEDED).
> +   Copyright (C) 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-nodelete-reloc.h"
> +
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +/* Just a flag here, not used for NODELETE processing.  */
> +bool may_finalize_mod13 = false;
> +
> +extern template struct unique_symbol<12>;
> +
> +/* Trigger the creation of a unique symbol reference.  This should
> +   cause tst-dlopen-nodelete-reloc-mod12.so to be marked as
> +   NODELETE.  */
> +int
> +global_function_mod13 (void)
> +{
> +  return unique_symbol<12>::value;

OK.

> +}
> +
> +static void __attribute__ ((destructor))
> +fini (void)
> +{
> +  if (!may_finalize_mod13)
> +    {
> +      puts ("error: tst-dlopen-nodelete-reloc-mod13.so destructor"
> +            " called too early");
> +      _exit (1);
> +    }
> +}
> diff --git a/elf/tst-dlopen-nodelete-reloc-mod13.h b/elf/tst-dlopen-nodelete-reloc-mod13.h
> new file mode 100644
> index 0000000000..5d338481a3
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc-mod13.h
> @@ -0,0 +1,24 @@
> +/* Inline function which produces a unique symbol.
> +   Copyright (C) 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/>.  */
> +
> +inline char *
> +third_function_with_local_static (void)
> +{
> +  static char local;
> +  return &local;
> +}
> diff --git a/elf/tst-dlopen-nodelete-reloc-mod14.cc b/elf/tst-dlopen-nodelete-reloc-mod14.cc
> new file mode 100644
> index 0000000000..e67621a2a2
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc-mod14.cc
> @@ -0,0 +1,42 @@
> +/* This object must retain NODELETE status after a dlopen failure.
> +   Copyright (C) 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-nodelete-reloc.h"
> +
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +/* Just a flag here, not used for NODELETE processing.  */
> +bool may_finalize_mod14 = false;
> +
> +/* Explicit instantiation.  This produces a unique symbol definition
> +   which is not referenced by the library itself, so the library is
> +   not marked NODELETE.  */
> +template struct unique_symbol<14>;
> +
> +static void __attribute__ ((destructor))
> +fini (void)
> +{
> +  if (!may_finalize_mod14)
> +    {
> +      puts ("error: tst-dlopen-nodelete-reloc-mod14.so destructor"
> +            " called too early");
> +      _exit (1);
> +    }
> +}
> diff --git a/elf/tst-dlopen-nodelete-reloc-mod15.cc b/elf/tst-dlopen-nodelete-reloc-mod15.cc
> new file mode 100644
> index 0000000000..ff7a64edd1
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc-mod15.cc
> @@ -0,0 +1,41 @@
> +/* Helper object to mark tst-dlopen-nodelete-reloc-mod14.so as NODELETE.
> +   Copyright (C) 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-nodelete-reloc.h"
> +
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +extern template struct unique_symbol<14>;
> +
> +/* Trigger the creation of a unique symbol reference.  This should
> +   cause tst-dlopen-nodelete-reloc-mod14.so to be marked as
> +   NODELETE.  */
> +int
> +global_function_mod15 (void)
> +{
> +  return unique_symbol<14>::value;
> +}
> +
> +static void __attribute__ ((destructor))
> +fini (void)
> +{
> +  /* This object is never loaded completely.  */

OK, because we never finish up doing the work because mod16 fails.

> +  puts ("error: tst-dlopen-nodelete-reloc-mod15.so destructor invoked");

Why doesn't this fail the test?

> +}
> diff --git a/elf/tst-dlopen-nodelete-reloc-mod16.c b/elf/tst-dlopen-nodelete-reloc-mod16.c
> new file mode 100644
> index 0000000000..f836f04fb5
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc-mod16.c
> @@ -0,0 +1,27 @@
> +/* Object with an undefined symbol to trigger a relocation failure.
> +   Copyright (C) 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/>.  */
> +
> +/* The reference to undefined_mod16 triggers a relocation failure.  */
> +
> +extern int undefined_mod16;
> +
> +int
> +global_function_mod15 (void)

OK. This interposes global_function_mod15 in mod15.

> +{
> +  return undefined_mod16;
> +}
> diff --git a/elf/tst-dlopen-nodelete-reloc-mod17.c b/elf/tst-dlopen-nodelete-reloc-mod17.c
> new file mode 100644
> index 0000000000..426562edd9
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc-mod17.c
> @@ -0,0 +1,19 @@
> +/* Top-level object with dependency on an object that fails relocation.
> +   Copyright (C) 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/>.  */
> +
> +/* The dependencies do all the work.  */

OK.

> diff --git a/elf/tst-dlopen-nodelete-reloc-mod2.c b/elf/tst-dlopen-nodelete-reloc-mod2.c
> new file mode 100644
> index 0000000000..81ea8e5af2
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc-mod2.c
> @@ -0,0 +1,38 @@
> +/* Test propagation of NODELETE to an already-loaded object via relocation.
> +   NODELETE helper module.
> +   Copyright (C) 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 <stdbool.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +/* Defined in tst-dlopen-nodelete-reloc-mod1.so.  This dependency is
> +   not expressed via DT_NEEDED, so this reference marks the other
> +   object as NODELETE dynamically, during initially relocation.  */
> +extern bool may_finalize_mod1;

OK.

> +
> +static void __attribute__ ((destructor))
> +fini (void)
> +{
> +  if (!may_finalize_mod1)
> +    {
> +      puts ("error: tst-dlopen-nodelete-reloc-mod2.so destructor"
> +            " called too early");
> +      _exit (1);
> +    }
> +}
> diff --git a/elf/tst-dlopen-nodelete-reloc-mod3.c b/elf/tst-dlopen-nodelete-reloc-mod3.c
> new file mode 100644
> index 0000000000..d33f4ec763
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc-mod3.c
> @@ -0,0 +1,38 @@
> +/* Test propagation of NODELETE to an already-loaded object via relocation.
> +   Non-NODELETE helper module.
> +   Copyright (C) 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 <stdbool.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +/* Globally exported.  Set by the main program to true before
> +   termination, and used by tst-dlopen-nodelete-reloc-mod4.so,
> +   tst-dlopen-nodelete-reloc-mod5.so.  */
> +bool may_finalize_mod3 = false;

OK.

> +
> +static void __attribute__ ((destructor))
> +fini (void)
> +{
> +  if (!may_finalize_mod3)
> +    {
> +      puts ("error: tst-dlopen-nodelete-reloc-mod3.so destructor"
> +            " called too early");
> +      _exit (1);

OK.

> +    }
> +}
> diff --git a/elf/tst-dlopen-nodelete-reloc-mod4.c b/elf/tst-dlopen-nodelete-reloc-mod4.c
> new file mode 100644
> index 0000000000..7e6633aebb
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc-mod4.c
> @@ -0,0 +1,37 @@
> +/* Test propagation of NODELETE to an already-loaded object via relocation.
> +   Intermediate helper module.
> +   Copyright (C) 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 <stdbool.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +/* Defined in tst-dlopen-nodelete-reloc-mod3.so.  The dependency is
> +   expressed via DT_NEEDED.  */
> +extern bool may_finalize_mod3;

OK.

> +
> +static void __attribute__ ((destructor))
> +fini (void)
> +{
> +  if (!may_finalize_mod3)
> +    {
> +      puts ("error: tst-dlopen-nodelete-reloc-mod4.so destructor"
> +            " called too early");
> +      _exit (1);
> +    }
> +}
> diff --git a/elf/tst-dlopen-nodelete-reloc-mod5.c b/elf/tst-dlopen-nodelete-reloc-mod5.c
> new file mode 100644
> index 0000000000..f876fa0f97
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc-mod5.c
> @@ -0,0 +1,38 @@
> +/* Test propagation of NODELETE to an already-loaded object via relocation.
> +   NODELETE helper module.
> +   Copyright (C) 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 <stdbool.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +/* Defined in tst-dlopen-nodelete-reloc-mod3.so.  The dependency is
> +   expressed via DT_NEEDED on the intermedia DSO

s/intermedia/intermediate/g

> +   tst-dlopen-nodelete-reloc-mod3.so.  */
> +extern bool may_finalize_mod3;
> +
> +static void __attribute__ ((destructor))
> +fini (void)
> +{
> +  if (!may_finalize_mod3)
> +    {
> +      puts ("error: tst-dlopen-nodelete-reloc-mod5.so destructor"
> +            " called too early");
> +      _exit (1);
> +    }
> +}
> diff --git a/elf/tst-dlopen-nodelete-reloc-mod6.cc b/elf/tst-dlopen-nodelete-reloc-mod6.cc
> new file mode 100644
> index 0000000000..180f5b5842
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc-mod6.cc
> @@ -0,0 +1,42 @@
> +/* First module for NODELETE test defining a unique symbol.
> +   Copyright (C) 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-nodelete-reloc.h"
> +
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +/* Just a flag here, not used for NODELETE processing.  */
> +bool may_finalize_mod6 = false;
> +
> +/* Explicit instantiation.  This produces a unique symbol definition
> +   which is not referenced by the library itself, so the library is
> +   not marked NODELETE.  */
> +template struct unique_symbol<6>;

OK.

> +
> +static void __attribute__ ((destructor))
> +fini (void)
> +{
> +  if (!may_finalize_mod6)
> +    {
> +      puts ("error: tst-dlopen-nodelete-reloc-mod6.so destructor"
> +            " called too early");
> +      _exit (1);
> +    }
> +}
> diff --git a/elf/tst-dlopen-nodelete-reloc-mod7.cc b/elf/tst-dlopen-nodelete-reloc-mod7.cc
> new file mode 100644
> index 0000000000..c85e7c991b
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc-mod7.cc
> @@ -0,0 +1,48 @@
> +/* Second module for NODELETE test defining a unique symbol.
> +   Copyright (C) 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-nodelete-reloc.h"
> +
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +/* Just a flag here, not used for NODELETE processing.  */
> +bool may_finalize_mod7 = false;
> +
> +extern template struct unique_symbol<6>;

OK.

> +
> +/* Trigger the creation of a unique symbol reference.  This should
> +   cause tst-dlopen-nodelete-reloc-mod6.so to be marked as
> +   NODELETE.  */
> +int
> +global_function_mod7 (void)
> +{
> +  return unique_symbol<6>::value;

OK.

> +}
> +
> +static void __attribute__ ((destructor))
> +fini (void)
> +{
> +  if (!may_finalize_mod7)
> +    {
> +      puts ("error: tst-dlopen-nodelete-reloc-mod7.so destructor"
> +            " called too early");
> +      _exit (1);
> +    }
> +}
> diff --git a/elf/tst-dlopen-nodelete-reloc-mod8.c b/elf/tst-dlopen-nodelete-reloc-mod8.c
> new file mode 100644
> index 0000000000..ebb1c35fab
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc-mod8.c
> @@ -0,0 +1,41 @@
> +/* Helper module to load tst-dlopen-nodelete-reloc-mod9.so.
> +   Copyright (C) 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 <stddef.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +static void *handle;
> +
> +static void __attribute__ ((constructor))
> +init (void)
> +{
> +  handle = dlopen ("tst-dlopen-nodelete-reloc-mod9.so", RTLD_NOW);

OK.

> +  if (handle == NULL)
> +    {
> +      printf ("error: dlopen in module 8: %s\n", dlerror ());
> +      _exit (1);
> +    }
> +}
> +
> +static void __attribute__ ((destructor))
> +fini (void)
> +{
> +  dlclose (handle);
> +}
> diff --git a/elf/tst-dlopen-nodelete-reloc-mod9.cc b/elf/tst-dlopen-nodelete-reloc-mod9.cc
> new file mode 100644
> index 0000000000..06fb49cdf7
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc-mod9.cc
> @@ -0,0 +1,42 @@
> +/* First module defining a unique symbol (loaded indirectly).
> +   Copyright (C) 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-nodelete-reloc.h"
> +
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +/* Just a flag here, not used for NODELETE processing.  */
> +bool may_finalize_mod9 = false;
> +
> +/* Explicit instantiation.  This produces a unique symbol definition
> +   which is not referenced by the library itself, so the library is
> +   not marked NODELETE.  */
> +template struct unique_symbol<9>;

OK.

> +
> +static void __attribute__ ((destructor))
> +fini (void)
> +{
> +  if (!may_finalize_mod9)
> +    {
> +      puts ("error: tst-dlopen-nodelete-reloc-mod9.so destructor"
> +            " called too early");
> +      _exit (1);
> +    }
> +}
> diff --git a/elf/tst-dlopen-nodelete-reloc.c b/elf/tst-dlopen-nodelete-reloc.c
> new file mode 100644
> index 0000000000..d3de90a9f5
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc.c
> @@ -0,0 +1,178 @@
> +/* Test interactions of dlopen, NODELETE, and relocations.

This is a lot of good new tests. I don't have any suggestions for more tests.
It's hard for me to know exactly where we might see a problem, but this list
is a good start.

> +   Copyright (C) 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 exercises NODELETE propagation due to data relocations
> +   and unique symbols, and the interaction with already-loaded
> +   objects.  Some test objects are written in C++, to produce unique
> +   symbol definitions.
> +
> +   First test: Global scope variant, data relocation as the NODELETE
> +   trigger.  mod1 is loaded first with a separate dlopen call.
> +
> +      mod2 ---(may_finalize_mod1 relocation dependency)---> mod1
> +    (NODELETE)                                   (marked as NODELETE)

OK. Correct.

My notes:

mod1
- global may_finalize_mod1
- fini check to see if main destructor ran first.

mod2 - nodelete
- reference to may_finalize_mod1


> +
> +   Second test: Local scope variant, data relocation.  mod3 is loaded
> +   first, then mod5.
> +
> +      mod5 ---(DT_NEEDED)--->  mod4  ---(DT_NEEDED)---> mod3
> +    (NODELETE)           (not NODELETE)                  ^
> +        \                                               / (marked as
> +         `--(may_finalize_mod3 relocation dependency)--/   NODELETE)

OK. Correct.

My notes:

mod3
- global may_finalize_mod3

mod4 - need mod3
- reference to may_finalize_mod3

mod5 - need mod4, nodelete
- reference to may_finalize_mod3
- underlinked

> +
> +   Third test: Shared local scope with unique symbol.  mod6 is loaded
> +   first, then mod7.  No explicit dependencies between the two
> +   objects, so first object has to be opened with RTLD_GLOBAL.
> +
> +      mod7 ---(unique symbol)---> mod6
> +                          (marked as NODELETE)

OK. Correct.

My notes:

mod6
- global may_finalize_mod6
- define template struct unique_symbol<6>

mod7
- global may_finalize_mod7
- references template struct unique_symbol<6>


> +
> +   Forth test: Non-shared scopes with unique symbol.  mod8 and mod10
> +   are loaded from the main program.  mod8 loads mod9 from an ELF
> +   constructor, mod10 loads mod11.  There are no DT_NEEDED
> +   dependencies.  mod9 is promoted to the global scope form the main
> +   program.  The unique symbol dependency is:
> +
> +      mod9 ---(unique symbol)---> mod11
> +                          (marked as NODELETE)

OK. Correct.

My notes:

mod8 - need libdl
- dlopen mod9

mod9
- global may_finalize_mod9
- define template struct unique_symbol<9>

mod10 - need libdl
- dlopen mod11

mod11
- global may_finalize_mod11
- references template struct unique_symbol<9>


> +
> +   Fifth test: Shared local scope with unique symbol, like test 3, but
> +   this time, there is also a DT_NEEDED dependency (so no RTLD_GLOBAL
> +   needed):
> +
> +                 DT_NEEDED
> +      mod13 ---(unique symbol)---> mod12
> +                          (marked as NODELETE)
> +

OK. Correct.

My notes:

mod12
- global may_finalize_mod12
- definition template struct unique_symbol<12>

mod13 - need mod12
- global may_finalize_mod13
- references template struct unique_symbol<12> --- Marks mod12 as NODELETE.

> +   Sixth test: NODELETE status is retained after relocation failure
> +   with unique symbol dependency.  The object graph ensures that the
> +   unique symbol binding is processed before the dlopen failure.
> +
> +                                        DT_NEEDED
> +     mod17  --(DT_NEEDED)--> mod15 --(unique symbol)--> mod14
> +       \                       ^                  (RTLD_NODELETE)
> +        \                 (DT_NEEDED)
> +         \                     |
> +          `---(DT_NEEDED)--> mod16
> +                       (fails to relocate)
> +
> +   mod14 must remain NODELETE after opening mod17 failed.  */

OK. Correct.

My notes:

mod14
- global may_finalize_mod14
- definition template struct unique_symbol<14>

mod15 - need mod14
- references template struct unique_symbol<14>
- defines global_function_mod15

mod16
- undefined reference
- defines global_function_mod15

mod17 - need mod15, need mod16


> +
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdbool.h>
> +#include <support/check.h>
> +#include <support/xdlfcn.h>
> +
> +static int
> +do_test (void)
> +{
> +  /* First case: global scope, regular data symbol.  Open the object
> +     which is not NODELETE initially.  */
> +  void *mod1 = xdlopen ("tst-dlopen-nodelete-reloc-mod1.so",
> +                        RTLD_NOW | RTLD_GLOBAL);
> +  /* This is used to indicate that the ELF destructor may be
> +     called.  */
> +  bool *may_finalize_mod1 = xdlsym (mod1, "may_finalize_mod1");

OK. Don't set may_finalize_mod1 to true.

> +  /* Open the NODELETE object.  */
> +  void *mod2 = xdlopen ("tst-dlopen-nodelete-reloc-mod2.so", RTLD_NOW);
> +  /* This has no effect because the DSO is directly marked as
> +     NODELETE.  */
> +  xdlclose (mod2);
> +  /* This has no effect because the DSO has been indirectly marked as
> +     NODELETE due to a relocation dependency.  */
> +  xdlclose (mod1);

OK.

> +
> +  /* Second case: local scope, regular data symbol.  Open the object
> +     which is not NODELETE initially.  */
> +  void *mod3 = xdlopen ("tst-dlopen-nodelete-reloc-mod3.so", RTLD_NOW);
> +  bool *may_finalize_mod3 = xdlsym (mod3, "may_finalize_mod3");

OK. Don't set may_finalize_mod3 to true.

> +  /* Open the NODELETE object.  */
> +  void *mod5 = xdlopen ("tst-dlopen-nodelete-reloc-mod5.so", RTLD_NOW);
> +  /* Again those have no effect because of NODELETE.  */
> +  xdlclose (mod5);
> +  xdlclose (mod3);

OK.

> +
> +  /* Third case: Unique symbol.  */
> +  void *mod6 = xdlopen ("tst-dlopen-nodelete-reloc-mod6.so",
> +                        RTLD_NOW | RTLD_GLOBAL);
> +  bool *may_finalize_mod6 = xdlsym (mod6, "may_finalize_mod6");
> +  void *mod7 = xdlopen ("tst-dlopen-nodelete-reloc-mod7.so", RTLD_NOW);
> +  bool *may_finalize_mod7 = xdlsym (mod7, "may_finalize_mod7");
> +  /* This should not have any effect because of the unique symbol and
> +     the resulting NODELETE status.  */
> +  xdlclose (mod6);
> +  /* mod7 is not NODELETE and can be closed.  */
> +  *may_finalize_mod7 = true;

OK. May be closed.

> +  xdlclose (mod7);

OK.

> +
> +  /* Fourth case: Unique symbol, indirect loading.  */
> +  void *mod8 = xdlopen ("tst-dlopen-nodelete-reloc-mod8.so", RTLD_NOW);
> +  /* Also promote to global scope.  */
> +  void *mod9 = xdlopen ("tst-dlopen-nodelete-reloc-mod9.so",
> +                        RTLD_NOW | RTLD_NOLOAD | RTLD_GLOBAL);

OK.

> +  bool *may_finalize_mod9 = xdlsym (mod9, "may_finalize_mod9");
> +  xdlclose (mod9);              /* Drop mod9 reference.  */
> +  void *mod10 = xdlopen ("tst-dlopen-nodelete-reloc-mod10.so", RTLD_NOW);
> +  void *mod11 = xdlopen ("tst-dlopen-nodelete-reloc-mod11.so",
> +                        RTLD_NOW | RTLD_NOLOAD);
> +  bool *may_finalize_mod11 = xdlsym (mod11, "may_finalize_mod11");
> +  xdlclose (mod11);              /* Drop mod11 reference.  */

OK.

> +  /* mod11 is not NODELETE and can be closed.  */
> +  *may_finalize_mod11 = true;

OK.

> +  /* Trigger closing of mod11, too.  */
> +  xdlclose (mod10);

OK.

> +  /* Does not trigger closing of mod9.  */
> +  xdlclose (mod8);

OK.

> +
> +  /* Fifth case: Unique symbol, with DT_NEEDED dependency.  */
> +  void *mod12 = xdlopen ("tst-dlopen-nodelete-reloc-mod12.so", RTLD_NOW);
> +  bool *may_finalize_mod12 = xdlsym (mod12, "may_finalize_mod12");
> +  void *mod13 = xdlopen ("tst-dlopen-nodelete-reloc-mod13.so", RTLD_NOW);
> +  bool *may_finalize_mod13 = xdlsym (mod13, "may_finalize_mod13");
> +  /* This should not have any effect because of the unique symbol. */
> +  xdlclose (mod12);

OK.

> +  /* mod13 is not NODELETE and can be closed.  */
> +  *may_finalize_mod13 = true;
> +  xdlclose (mod13);

OK.

> +
> +  /* Sixth case: Unique symbol binding must not cause loss of NODELETE
> +     status.  */
> +  void *mod14 = xdlopen ("tst-dlopen-nodelete-reloc-mod14.so",
> +                         RTLD_NOW | RTLD_NODELETE);
> +  bool *may_finalize_mod14 = xdlsym (mod14, "may_finalize_mod14");
> +  TEST_VERIFY (dlopen ("tst-dlopen-nodelete-reloc-mod17.so", RTLD_NOW)
> +               == NULL);
> +  const char *message = dlerror ();
> +  printf ("info: test 6 message: %s\n", message);
> +  /* This must not close the object, it must still be NODELETE.  */
> +  xdlclose (mod14);
> +  xdlopen ("tst-dlopen-nodelete-reloc-mod14.so", RTLD_NOW | RTLD_NOLOAD);

OK.

> +
> +  /* Prepare for process exit.  Destructors for NODELETE objects will
> +     be invoked.  */
> +  *may_finalize_mod1 = true;
> +  *may_finalize_mod3 = true;
> +  *may_finalize_mod6 = true;
> +  *may_finalize_mod9 = true;
> +  *may_finalize_mod12 = true;
> +  *may_finalize_mod14 = true;
OK.

> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/elf/tst-dlopen-nodelete-reloc.h b/elf/tst-dlopen-nodelete-reloc.h
> new file mode 100644
> index 0000000000..8844de6226
> --- /dev/null
> +++ b/elf/tst-dlopen-nodelete-reloc.h
> @@ -0,0 +1,35 @@
> +/* Template to produce unique symbols.
> +   Copyright (C) 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 template produces a unique symbol definition for an explicit
> +   template instantiation (without also incorporating a reference),
> +   and an extern template declaration can be used to reference that
> +   symbol from another object.  The modid parameter is just a
> +   placeholder to create different symbols (because it affects the
> +   name mangling of the static value member).  By convention, it
> +   should match the number of the module that contains the
> +   definition.  */

OK.

> +
> +template <int modid>
> +struct unique_symbol
> +{
> +  static int value;

OK.

> +};
> +
> +template <int modid>
> +int unique_symbol<modid>::value;

OK.

>
  

* Carlos O'Donell:

>> +/* Globally exported.  Set by the main program to true before
>> +   termination, and used by tst-dlopen-nodelete-reloc-mod2.so to
>> +   trigger marking his module as NODELETE (and also for its destructor
>
> s/his/this/g

Fixed.

>> +static void __attribute__ ((destructor))
>> +fini (void)
>> +{
>> +  /* This object is never loaded completely.  */
>
> OK, because we never finish up doing the work because mod16 fails.
>
>> +  puts ("error: tst-dlopen-nodelete-reloc-mod15.so destructor invoked");
>
> Why doesn't this fail the test?

Thinko on my part.  Fixed.

>> diff --git a/elf/tst-dlopen-nodelete-reloc-mod16.c b/elf/tst-dlopen-nodelete-reloc-mod16.c
>> new file mode 100644
>> index 0000000000..f836f04fb5
>> --- /dev/null
>> +++ b/elf/tst-dlopen-nodelete-reloc-mod16.c

>> +int
>> +global_function_mod15 (void)
>
> OK. This interposes global_function_mod15 in mod15.

Ugh.  Fixed as well.


>> diff --git a/elf/tst-dlopen-nodelete-reloc-mod5.c b/elf/tst-dlopen-nodelete-reloc-mod5.c
>> new file mode 100644
>> index 0000000000..f876fa0f97

>> +/* Defined in tst-dlopen-nodelete-reloc-mod3.so.  The dependency is
>> +   expressed via DT_NEEDED on the intermedia DSO
>
> s/intermedia/intermediate/g

Fixed.
>> +   Sixth test: NODELETE status is retained after relocation failure
>> +   with unique symbol dependency.  The object graph ensures that the
>> +   unique symbol binding is processed before the dlopen failure.
>> +
>> +                                        DT_NEEDED
>> +     mod17  --(DT_NEEDED)--> mod15 --(unique symbol)--> mod14
>> +       \                       ^                  (RTLD_NODELETE)
>> +        \                 (DT_NEEDED)
>> +         \                     |
>> +          `---(DT_NEEDED)--> mod16
>> +                       (fails to relocate)
>> +
>> +   mod14 must remain NODELETE after opening mod17 failed.  */
>
> OK. Correct.

I added:

   mod14 is loaded first, and the loading mod17 is attempted.

You indicated off-list that you wouldn't object pushing this now, so
I've done that after retesting.

Thanks,
Florian