[PATCH 0/4] _r_debug copy relocation support

[PATCH 0/4] _r_debug copy relocation support

[Florian Weimer]

I implemented this because we received a bug that the _r_debug extension
mechanism broke dyninst.  But it turns out it wasn't because of a copy
relocation: dyninst has its own interposing *definition* of _r_debug, so
the patch series doesn't solve this.  It can only handle an interposing
definition in the main executable, not one in shared object (although I
guess in theory we could fix this).

The first two patches are independently useful, and the second two
patches could be used if we ever need to implement copy relocation
support for _r_debug.

Tested on i686-linux-gnu and x86_64-linux-gnu.

Thanks,
Florian

Florian Weimer (4):
  elf: Introduce separate _r_debug_array variable
  elf: Introduce _dl_debug_change_state
  elf: Support version-less lookup in _dl_lookup_direct
  elf: Restore support for _r_debug copy relocations

 elf/Makefile               |   8 +++
 elf/dl-close.c             |   6 +-
 elf/dl-debug.c             | 138 ++++++++++++++++++++++++++-----------
 elf/dl-load.c              |   6 +-
 elf/dl-lookup-direct.c     |   5 ++
 elf/dl-open.c              |   5 +-
 elf/rtld.c                 |  10 +--
 elf/tst-dlmopen4-nonpic.c  |   2 +
 elf/tst-dlmopen4-pic.c     |   2 +
 elf/tst-dlmopen4.c         |  22 ++++++
 sysdeps/generic/ldsodefs.h |  27 ++++++--
 11 files changed, 168 insertions(+), 63 deletions(-)
 create mode 100644 elf/tst-dlmopen4-nonpic.c
 create mode 100644 elf/tst-dlmopen4-pic.c


base-commit: 9702a7901e18460e8ffc5f56a493d41294a8e936
-- 
2.33.1

[PATCH 1/4] elf: Introduce separate _r_debug_array variable

[Florian Weimer]

It replaces the ns_debug member of the namespaces.  Previously,
the base namespace had an unused ns_debug member.

This change also fixes a concurrency issue: _dl_debug_initialize only
updates r_next of the previous namespace's r_debug after the new
r_debug is initialized, so that only the initialized version is
observed.
---
 elf/dl-debug.c             | 91 +++++++++++++++++++++-----------------
 sysdeps/generic/ldsodefs.h |  2 -
 2 files changed, 50 insertions(+), 43 deletions(-)

diff --git a/elf/dl-debug.c b/elf/dl-debug.c
index f637d4bb8d..649386d5a6 100644
--- a/elf/dl-debug.c
+++ b/elf/dl-debug.c
@@ -30,17 +30,37 @@ extern const int verify_link_map_members[(VERIFY_MEMBER (l_addr)
 					  && VERIFY_MEMBER (l_prev))
 					 ? 1 : -1];
 
+#ifdef SHARED
+/* r_debug structs for secondary namespaces.  The first namespace is
+   handled separately because its r_debug structure must overlap with
+   the public _r_debug symbol, so the first array element corresponds
+   to LM_ID_BASE + 1.  See elf/dl-debug-symbols.S.  */
+struct r_debug_extended _r_debug_array[DL_NNS - 1];
+
+/* Return the r_debug object for the namespace NS.  */
+static inline struct r_debug_extended *
+get_rdebug (Lmid_t ns)
+{
+  if (ns == LM_ID_BASE)
+    return &_r_debug_extended;
+  else
+    return  &_r_debug_array[ns - 1];
+}
+#else /* !SHARED */
+static inline struct r_debug_extended *
+get_rdebug (Lmid_t ns)
+{
+  return &_r_debug_extended; /* There is just one namespace.  */
+}
+#endif  /* !SHARED */
+
 /* Update the `r_map' member and return the address of `struct r_debug'
    of the namespace NS. */
 
 struct r_debug *
 _dl_debug_update (Lmid_t ns)
 {
-  struct r_debug_extended *r;
-  if (ns == LM_ID_BASE)
-    r = &_r_debug_extended;
-  else
-    r = &GL(dl_ns)[ns]._ns_debug;
+  struct r_debug_extended *r = get_rdebug (ns);
   if (r->base.r_map == NULL)
     atomic_store_release (&r->base.r_map,
 			  (void *) GL(dl_ns)[ns]._ns_loaded);
@@ -54,34 +74,7 @@ _dl_debug_update (Lmid_t ns)
 struct r_debug *
 _dl_debug_initialize (ElfW(Addr) ldbase, Lmid_t ns)
 {
-  struct r_debug_extended *r, **pp = NULL;
-
-  if (ns == LM_ID_BASE)
-    {
-      r = &_r_debug_extended;
-      /* Initialize r_version to 1.  */
-      if (_r_debug_extended.base.r_version == 0)
-	_r_debug_extended.base.r_version = 1;
-    }
-  else if (DL_NNS > 1)
-    {
-      r = &GL(dl_ns)[ns]._ns_debug;
-      if (r->base.r_brk == 0)
-	{
-	  /* Add the new namespace to the linked list.  After a namespace
-	     is initialized, r_brk becomes non-zero.  A namespace becomes
-	     empty (r_map == NULL) when it is unused.  But it is never
-	     removed from the linked list.  */
-	  struct r_debug_extended *p;
-	  for (pp = &_r_debug_extended.r_next;
-	       (p = *pp) != NULL;
-	       pp = &p->r_next)
-	    ;
-
-	  r->base.r_version = 2;
-	}
-    }
-
+  struct r_debug_extended *r = get_rdebug (ns);
   if (r->base.r_brk == 0)
     {
       /* Tell the debugger where to find the map of loaded objects.
@@ -89,20 +82,36 @@ _dl_debug_initialize (ElfW(Addr) ldbase, Lmid_t ns)
 	 only once.  */
       r->base.r_ldbase = ldbase ?: _r_debug_extended.base.r_ldbase;
       r->base.r_brk = (ElfW(Addr)) &_dl_debug_state;
-      r->r_next = NULL;
+
+#ifdef SHARED
+      /* Add the new namespace to the linked list.  This assumes that
+	 namespaces are allocated in increasing order.  After a
+	 namespace is initialized, r_brk becomes non-zero.  A
+	 namespace becomes empty (r_map == NULL) when it is unused.
+	 But it is never removed from the linked list.  */
+
+      if (ns != LM_ID_BASE)
+	{
+	  r->base.r_version = 2;
+	  if (ns - 1 == LM_ID_BASE)
+	    {
+	      atomic_store_release (&_r_debug_extended.r_next, r);
+	      /* Now there are multiple namespaces.  */
+	      atomic_store_release (&_r_debug_extended.base.r_version, 2);
+	    }
+	  else
+	    /* Update r_debug_extended of the previous namespace.  */
+	    atomic_store_release (&_r_debug_array[ns - 2].r_next, r);
+	}
+      else
+#endif /* SHARED */
+	r->base.r_version = 1;
     }
 
   if (r->base.r_map == NULL)
     atomic_store_release (&r->base.r_map,
 			  (void *) GL(dl_ns)[ns]._ns_loaded);
 
-  if (pp != NULL)
-    {
-      atomic_store_release (pp, r);
-      /* Bump r_version to 2 for the new namespace.  */
-      atomic_store_release (&_r_debug_extended.base.r_version, 2);
-    }
-
   return &r->base;
 }
 
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index c26860430c..a9189f5501 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -368,8 +368,6 @@ struct rtld_global
       size_t n_elements;
       void (*free) (void *);
     } _ns_unique_sym_table;
-    /* Keep track of changes to each namespace' list.  */
-    struct r_debug_extended _ns_debug;
   } _dl_ns[DL_NNS];
   /* One higher than index of last used namespace.  */
   EXTERN size_t _dl_nns;
-- 
2.33.1

[PATCH 2/4] elf: Introduce _dl_debug_change_state

[Florian Weimer]

It combines updating r_state with the debugger notification.

The change to  _dl_open introduces an additional debugger notification
for dlmopen, but debuggers are expected to ignore it.
---
 elf/dl-close.c             |  6 ++----
 elf/dl-debug.c             |  7 +++++++
 elf/dl-load.c              |  6 ++----
 elf/dl-open.c              |  5 ++---
 elf/rtld.c                 |  6 ++----
 sysdeps/generic/ldsodefs.h | 14 ++++++++++++--
 6 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/elf/dl-close.c b/elf/dl-close.c
index 4f5cfcc1c3..660a46e8ed 100644
--- a/elf/dl-close.c
+++ b/elf/dl-close.c
@@ -495,8 +495,7 @@ _dl_close_worker (struct link_map *map, bool force)
 
   /* Notify the debugger we are about to remove some loaded objects.  */
   struct r_debug *r = _dl_debug_update (nsid);
-  r->r_state = RT_DELETE;
-  _dl_debug_state ();
+  _dl_debug_change_state (r, RT_DELETE);
   LIBC_PROBE (unmap_start, 2, nsid, r);
 
   if (unload_global)
@@ -820,8 +819,7 @@ _dl_close_worker (struct link_map *map, bool force)
     while (GL(dl_ns)[GL(dl_nns) - 1]._ns_loaded == NULL);
 
   /* Notify the debugger those objects are finalized and gone.  */
-  r->r_state = RT_CONSISTENT;
-  _dl_debug_state ();
+  _dl_debug_change_state (r, RT_CONSISTENT);
   LIBC_PROBE (unmap_complete, 2, nsid, r);
 
   /* Recheck if we need to retry, release the lock.  */
diff --git a/elf/dl-debug.c b/elf/dl-debug.c
index 649386d5a6..f840a1b922 100644
--- a/elf/dl-debug.c
+++ b/elf/dl-debug.c
@@ -67,6 +67,13 @@ _dl_debug_update (Lmid_t ns)
   return &r->base;
 }
 
+void
+_dl_debug_change_state (struct r_debug *r, int state)
+{
+  atomic_store_release (&r->r_state, state);
+  _dl_debug_state ();
+}
+
 /* Initialize _r_debug_extended for the namespace NS.  LDBASE is the
    run-time load address of the dynamic linker, to be put in
    _r_debug_extended.r_ldbase.  Return the address of _r_debug.  */
diff --git a/elf/dl-load.c b/elf/dl-load.c
index 2a1443387f..94b5bd619a 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -987,8 +987,7 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
 
 	  if (make_consistent && r != NULL)
 	    {
-	      r->r_state = RT_CONSISTENT;
-	      _dl_debug_state ();
+	      _dl_debug_change_state (r, RT_CONSISTENT);
 	      LIBC_PROBE (map_failed, 2, nsid, r);
 	    }
 
@@ -1506,8 +1505,7 @@ cannot enable executable stack as shared object requires");
       /* Notify the debugger we have added some objects.  We need to
 	 call _dl_debug_initialize in a static program in case dynamic
 	 linking has not been used before.  */
-      r->r_state = RT_ADD;
-      _dl_debug_state ();
+      _dl_debug_change_state (r, RT_ADD);
       LIBC_PROBE (map_start, 2, nsid, r);
       make_consistent = true;
     }
diff --git a/elf/dl-open.c b/elf/dl-open.c
index 6ea5dd2457..545037a031 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -634,8 +634,7 @@ dl_open_worker_begin (void *a)
 
   /* Notify the debugger all new objects are now ready to go.  */
   struct r_debug *r = _dl_debug_update (args->nsid);
-  r->r_state = RT_CONSISTENT;
-  _dl_debug_state ();
+  _dl_debug_change_state (r, RT_CONSISTENT);
   LIBC_PROBE (map_complete, 3, args->nsid, r, new);
 
   _dl_open_check (new);
@@ -863,7 +862,7 @@ no more namespaces available for dlmopen()"));
 	  ++GL(dl_nns);
 	}
 
-      _dl_debug_update (nsid)->r_state = RT_CONSISTENT;
+      _dl_debug_change_state (_dl_debug_update (nsid), RT_CONSISTENT);
     }
   /* Never allow loading a DSO in a namespace which is empty.  Such
      direct placements is only causing problems.  Also don't allow
diff --git a/elf/rtld.c b/elf/rtld.c
index 4b09e84b0d..dd4173c2ca 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1807,8 +1807,7 @@ dl_main (const ElfW(Phdr) *phdr,
 #endif
 
   /* We start adding objects.  */
-  r->r_state = RT_ADD;
-  _dl_debug_state ();
+  _dl_debug_change_state (r, RT_ADD);
   LIBC_PROBE (init_start, 2, LM_ID_BASE, r);
 
   /* Auditing checkpoint: we are ready to signal that the initial map
@@ -2527,8 +2526,7 @@ dl_main (const ElfW(Phdr) *phdr,
   /* Notify the debugger all new objects are now ready to go.  We must re-get
      the address since by now the variable might be in another object.  */
   r = _dl_debug_update (LM_ID_BASE);
-  r->r_state = RT_CONSISTENT;
-  _dl_debug_state ();
+  _dl_debug_change_state (r, RT_CONSISTENT);
   LIBC_PROBE (init_complete, 2, LM_ID_BASE, r);
 
 #if defined USE_LDCONFIG && !defined MAP_COPY
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index a9189f5501..9efd5506b6 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -1125,8 +1125,14 @@ extern void _dl_debug_state (void);
 rtld_hidden_proto (_dl_debug_state)
 
 /* Initialize `struct r_debug_extended' for the namespace NS.  LDBASE
-   is the run-time load address of the dynamic linker, to be put in the
-   `r_ldbase' member.  Return the address of the structure.  */
+   is the run-time load address of the dynamic linker, to be put in
+   the `r_ldbase' member.
+
+   This function returns the address of the r_debug structure for the
+   namespace.  This is not merely a convenience or optimization, but
+   it is necessary for the LIBC_PROBE Systemtap/debugger probes to
+   work reliably: direct variable access can create probes that tools
+   cannot consume.  */
 extern struct r_debug *_dl_debug_initialize (ElfW(Addr) ldbase, Lmid_t ns)
      attribute_hidden;
 
@@ -1134,6 +1140,10 @@ extern struct r_debug *_dl_debug_initialize (ElfW(Addr) ldbase, Lmid_t ns)
    of the namespace NS.  */
 extern struct r_debug *_dl_debug_update (Lmid_t ns) attribute_hidden;
 
+/* Updates R->r_state to STATE and notifies the debugger by calling
+   _dl_debug_state.  */
+void _dl_debug_change_state (struct r_debug *r, int state) attribute_hidden;
+
 /* Initialize the basic data structure for the search paths.  SOURCE
    is either "LD_LIBRARY_PATH" or "--library-path".
    GLIBC_HWCAPS_PREPEND adds additional glibc-hwcaps subdirectories to
-- 
2.33.1

[PATCH 3/4] elf: Support version-less lookup in _dl_lookup_direct

[Florian Weimer]

---
 elf/dl-lookup-direct.c     | 5 +++++
 sysdeps/generic/ldsodefs.h | 7 ++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/elf/dl-lookup-direct.c b/elf/dl-lookup-direct.c
index 08ac4d4104..6617962c4d 100644
--- a/elf/dl-lookup-direct.c
+++ b/elf/dl-lookup-direct.c
@@ -54,6 +54,11 @@ check_match (const struct link_map *const map, const char *const undef_name,
     /* Not the symbol we are looking for.  */
     return NULL;
 
+  if (map->l_versyms == NULL || version == NULL)
+    /* No symbol versioning information available, or no symbol
+       version requested.  Accepted the symbol.  */
+    return sym;
+
   ElfW(Half) ndx = map->l_versyms[symidx] & 0x7fff;
   if (map->l_versions[ndx].hash != version_hash
       || strcmp (map->l_versions[ndx].name, version) != 0)
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index 9efd5506b6..8ff06d6b02 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -1051,9 +1051,10 @@ extern lookup_t _dl_lookup_symbol_x (const char *undef,
    MAP) for the symbol UNDEF_NAME, with GNU hash NEW_HASH (computed
    with dl_new_hash), symbol version VERSION, and symbol version hash
    VERSION_HASH (computed with _dl_elf_hash).  Returns a pointer to
-   the symbol table entry in MAP on success, or NULL on failure.  MAP
-   must have symbol versioning information, or otherwise the result is
-   undefined.  */
+   the symbol table entry in MAP on success, or NULL on failure.
+
+   If VERSION is NULL or MAP does not have symbol versioning
+   information, any symbol version is accepted.  */
 const ElfW(Sym) *_dl_lookup_direct (struct link_map *map,
 				    const char *undef_name,
 				    uint32_t new_hash,
-- 
2.33.1

[PATCH 4/4] elf: Restore support for _r_debug copy relocations

[Florian Weimer]

The changes in commit a93d9e03a31ec14405cb3a09aa95413b67067380
("Extend struct r_debug to support multiple namespaces [BZ #15971]")
break the dyninst dynamic instrumentation tool.

It turns out it is rather hard to use the proposed handshake
for accessing _r_debug via DT_DEBUG.  Therefore, this commit restores
copy relocation support by adjusting the copy of _r_debug in the
main executable if a copy relocation is in play.
---
 elf/Makefile               |  8 +++++++
 elf/dl-debug.c             | 46 +++++++++++++++++++++++++++++++++++---
 elf/rtld.c                 |  4 ++++
 elf/tst-dlmopen4-nonpic.c  |  2 ++
 elf/tst-dlmopen4-pic.c     |  2 ++
 elf/tst-dlmopen4.c         | 22 ++++++++++++++++++
 sysdeps/generic/ldsodefs.h |  4 ++++
 7 files changed, 85 insertions(+), 3 deletions(-)
 create mode 100644 elf/tst-dlmopen4-nonpic.c
 create mode 100644 elf/tst-dlmopen4-pic.c

diff --git a/elf/Makefile b/elf/Makefile
index fe42caeb0e..000f9311ca 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -210,6 +210,7 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-align tst-align2 tst-align3 \
 	 tst-dlmodcount tst-dlopenrpath tst-deep1 \
 	 tst-dlmopen1 tst-dlmopen3 tst-dlmopen4 \
+	 tst-dlmopen4-pic tst-dlmopen4-nonpic \
 	 unload3 unload4 unload5 unload6 unload7 unload8 tst-global1 order2 \
 	 tst-audit1 tst-audit2 tst-audit8 tst-audit9 \
 	 tst-addr1 tst-thrlock \
@@ -1308,6 +1309,13 @@ $(objpfx)tst-dlmopen3.out: $(objpfx)tst-dlmopen1mod.so
 
 $(objpfx)tst-dlmopen4.out: $(objpfx)tst-dlmopen1mod.so
 
+CFLAGS-tst-dlmopen4-pic.c += -fPIC
+$(objpfx)tst-dlmopen4-pic.out: $(objpfx)tst-dlmopen1mod.so
+
+CFLAGS-tst-dlmopen4-nonpic.c += -fno-pie
+tst-dlmopen4-nonpic-no-pie = yes
+$(objpfx)tst-dlmopen4-nonpic.out: $(objpfx)tst-dlmopen1mod.so
+
 $(objpfx)tst-audit1.out: $(objpfx)tst-auditmod1.so
 tst-audit1-ENV = LD_AUDIT=$(objpfx)tst-auditmod1.so
 
diff --git a/elf/dl-debug.c b/elf/dl-debug.c
index f840a1b922..1aa79bc9b5 100644
--- a/elf/dl-debug.c
+++ b/elf/dl-debug.c
@@ -37,6 +37,32 @@ extern const int verify_link_map_members[(VERIFY_MEMBER (l_addr)
    to LM_ID_BASE + 1.  See elf/dl-debug-symbols.S.  */
 struct r_debug_extended _r_debug_array[DL_NNS - 1];
 
+/* If not null, pointer to the _r_debug in the main executable.  */
+static struct r_debug *_r_debug_main;
+
+void
+_dl_debug_post_relocate (struct link_map *main_map)
+{
+  const ElfW(Sym) *sym = _dl_lookup_direct (main_map, "_r_debug", 0x5475103c,
+					    NULL, 0);
+  if (sym != NULL && sym->st_size >= sizeof (struct r_debug))
+    {
+      struct r_debug *main_r_debug = DL_SYMBOL_ADDRESS (main_map, sym);
+      if (main_r_debug != &_r_debug_extended.base)
+	{
+	  /* The extended version of the struct is not available in
+	     the main executable because a copy relocation has been
+	     used.  r_map etc. have already been copied as part of the
+	     copy relocation processing.  */
+	  main_r_debug->r_version = 1;
+
+          /* Record that dual updates of the initial link map are
+             required.  */
+          _r_debug_main = main_r_debug;
+	}
+    }
+}
+
 /* Return the r_debug object for the namespace NS.  */
 static inline struct r_debug_extended *
 get_rdebug (Lmid_t ns)
@@ -71,6 +97,11 @@ void
 _dl_debug_change_state (struct r_debug *r, int state)
 {
   atomic_store_release (&r->r_state, state);
+#ifdef SHARED
+  if (r == &_r_debug_extended.base && _r_debug_main != NULL)
+    /* Update the copy-relocation of _r_debug.  */
+    atomic_store_release (&_r_debug_main->r_state, state);
+#endif
   _dl_debug_state ();
 }
 
@@ -103,7 +134,9 @@ _dl_debug_initialize (ElfW(Addr) ldbase, Lmid_t ns)
 	  if (ns - 1 == LM_ID_BASE)
 	    {
 	      atomic_store_release (&_r_debug_extended.r_next, r);
-	      /* Now there are multiple namespaces.  */
+	      /* Now there are multiple namespaces.  Note that this
+		 deliberately does not update the copy in the main
+		 executable (if it exists).  */
 	      atomic_store_release (&_r_debug_extended.base.r_version, 2);
 	    }
 	  else
@@ -116,8 +149,15 @@ _dl_debug_initialize (ElfW(Addr) ldbase, Lmid_t ns)
     }
 
   if (r->base.r_map == NULL)
-    atomic_store_release (&r->base.r_map,
-			  (void *) GL(dl_ns)[ns]._ns_loaded);
+    {
+      struct link_map *l = (void *) GL(dl_ns)[ns]._ns_loaded;
+      atomic_store_release (&r->base.r_map, l);
+#ifdef SHARED
+      if (ns == LM_ID_BASE && _r_debug_main != NULL)
+	/* Update the copy-relocation of _r_debug.  */
+	atomic_store_release (&_r_debug_main->r_map, l);
+#endif
+    }
 
   return &r->base;
 }
diff --git a/elf/rtld.c b/elf/rtld.c
index dd4173c2ca..ae3514eade 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -2428,6 +2428,7 @@ dl_main (const ElfW(Phdr) *phdr,
 	  if (l->l_tls_blocksize != 0 && tls_init_tp_called)
 	    _dl_add_to_slotinfo (l, true);
 	}
+
       rtld_timer_stop (&relocate_time, start);
 
       /* Now enable profiling if needed.  Like the previous call,
@@ -2439,6 +2440,9 @@ dl_main (const ElfW(Phdr) *phdr,
 	_dl_start_profile ();
     }
 
+  /* Update _r_debug if necessary.  */
+  _dl_debug_post_relocate (main_map);
+
   if ((!was_tls_init_tp_called && GL(dl_tls_max_dtv_idx) > 0)
       || count_modids != _dl_count_modids ())
     ++GL(dl_tls_generation);
diff --git a/elf/tst-dlmopen4-nonpic.c b/elf/tst-dlmopen4-nonpic.c
new file mode 100644
index 0000000000..ad4e409953
--- /dev/null
+++ b/elf/tst-dlmopen4-nonpic.c
@@ -0,0 +1,2 @@
+#define BUILD_FOR_NONPIC
+#include "tst-dlmopen4.c"
diff --git a/elf/tst-dlmopen4-pic.c b/elf/tst-dlmopen4-pic.c
new file mode 100644
index 0000000000..919fa85c25
--- /dev/null
+++ b/elf/tst-dlmopen4-pic.c
@@ -0,0 +1,2 @@
+#define BUILD_FOR_PIC
+#include "tst-dlmopen4.c"
diff --git a/elf/tst-dlmopen4.c b/elf/tst-dlmopen4.c
index 3fe150e50b..633addf419 100644
--- a/elf/tst-dlmopen4.c
+++ b/elf/tst-dlmopen4.c
@@ -53,6 +53,15 @@ do_test (void)
   TEST_COMPARE (debug->base.r_version, 1);
   TEST_VERIFY_EXIT (debug->r_next == NULL);
 
+#ifdef BUILD_FOR_PIC
+  /* In a PIC build, using _r_debug directly should give us the same
+     object.  */
+  TEST_VERIFY (&_r_debug == &debug->base);
+#endif
+#ifdef BUILD_FOR_NONPIC
+  TEST_COMPARE (_r_debug.r_version, 1);
+#endif
+
   void *h = xdlmopen (LM_ID_NEWLM, "$ORIGIN/tst-dlmopen1mod.so",
 		      RTLD_LAZY);
 
@@ -64,6 +73,19 @@ do_test (void)
   const char *name = basename (debug->r_next->base.r_map->l_name);
   TEST_COMPARE_STRING (name, "tst-dlmopen1mod.so");
 
+#ifdef BUILD_FOR_NONPIC
+  /* If a copy relocation is used, it must be at version 1.  */
+  if (&_r_debug != &debug->base)
+    {
+      TEST_COMPARE (_r_debug.r_version, 1);
+      TEST_COMPARE ((uintptr_t) _r_debug.r_map,
+		    (uintptr_t) debug->base.r_map);
+      TEST_COMPARE (_r_debug.r_brk, debug->base.r_brk);
+      TEST_COMPARE (_r_debug.r_state, debug->base.r_state);
+      TEST_COMPARE (_r_debug.r_ldbase, debug->base.r_ldbase);
+    }
+#endif
+
   xdlclose (h);
 
   return 0;
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index 8ff06d6b02..4658636d71 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -1137,6 +1137,10 @@ rtld_hidden_proto (_dl_debug_state)
 extern struct r_debug *_dl_debug_initialize (ElfW(Addr) ldbase, Lmid_t ns)
      attribute_hidden;
 
+/* This is called after relocation processing to handle a potential
+   copy relocation for _r_debug.  */
+void _dl_debug_post_relocate (struct link_map *main_map) attribute_hidden;
+
 /* Update the `r_map' member and return the address of `struct r_debug'
    of the namespace NS.  */
 extern struct r_debug *_dl_debug_update (Lmid_t ns) attribute_hidden;
-- 
2.33.1

Re: [PATCH 0/4] _r_debug copy relocation support

[H.J. Lu]

On Thu, Dec 23, 2021 at 10:43 AM Florian Weimer via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> I implemented this because we received a bug that the _r_debug extension
> mechanism broke dyninst.  But it turns out it wasn't because of a copy
> relocation: dyninst has its own interposing *definition* of _r_debug, so
> the patch series doesn't solve this.  It can only handle an interposing
> definition in the main executable, not one in shared object (although I
> guess in theory we could fix this).

Why doesn't GDB suffer from this issue?

> The first two patches are independently useful, and the second two
> patches could be used if we ever need to implement copy relocation
> support for _r_debug.
>
> Tested on i686-linux-gnu and x86_64-linux-gnu.
>
> Thanks,
> Florian
>
> Florian Weimer (4):
>   elf: Introduce separate _r_debug_array variable
>   elf: Introduce _dl_debug_change_state
>   elf: Support version-less lookup in _dl_lookup_direct
>   elf: Restore support for _r_debug copy relocations
>
>  elf/Makefile               |   8 +++
>  elf/dl-close.c             |   6 +-
>  elf/dl-debug.c             | 138 ++++++++++++++++++++++++++-----------
>  elf/dl-load.c              |   6 +-
>  elf/dl-lookup-direct.c     |   5 ++
>  elf/dl-open.c              |   5 +-
>  elf/rtld.c                 |  10 +--
>  elf/tst-dlmopen4-nonpic.c  |   2 +
>  elf/tst-dlmopen4-pic.c     |   2 +
>  elf/tst-dlmopen4.c         |  22 ++++++
>  sysdeps/generic/ldsodefs.h |  27 ++++++--
>  11 files changed, 168 insertions(+), 63 deletions(-)
>  create mode 100644 elf/tst-dlmopen4-nonpic.c
>  create mode 100644 elf/tst-dlmopen4-pic.c
>
>
> base-commit: 9702a7901e18460e8ffc5f56a493d41294a8e936
> --
> 2.33.1
>


-- 
H.J.

Re: [PATCH 0/4] _r_debug copy relocation support

[Florian Weimer]

* H. J. Lu:

> On Thu, Dec 23, 2021 at 10:43 AM Florian Weimer via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>> I implemented this because we received a bug that the _r_debug extension
>> mechanism broke dyninst.  But it turns out it wasn't because of a copy
>> relocation: dyninst has its own interposing *definition* of _r_debug, so
>> the patch series doesn't solve this.  It can only handle an interposing
>> definition in the main executable, not one in shared object (although I
>> guess in theory we could fix this).
>
> Why doesn't GDB suffer from this issue?

I expect that GDB uses DT_DEBUG, and that's not subject to
interposition, as before.

Thanks,
Florian

Re: [PATCH 0/4] _r_debug copy relocation support

[H.J. Lu]

On Thu, Dec 23, 2021 at 11:59 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Thu, Dec 23, 2021 at 10:43 AM Florian Weimer via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> >>
> >> I implemented this because we received a bug that the _r_debug extension
> >> mechanism broke dyninst.  But it turns out it wasn't because of a copy
> >> relocation: dyninst has its own interposing *definition* of _r_debug, so
> >> the patch series doesn't solve this.  It can only handle an interposing
> >> definition in the main executable, not one in shared object (although I
> >> guess in theory we could fix this).
> >
> > Why doesn't GDB suffer from this issue?
>
> I expect that GDB uses DT_DEBUG, and that's not subject to
> interposition, as before.
>

Can dyninst also use DT_DEBUG?

-- 
H.J.

Re: [PATCH 0/4] _r_debug copy relocation support

[Florian Weimer]

* H. J. Lu:

> On Thu, Dec 23, 2021 at 11:59 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * H. J. Lu:
>>
>> > On Thu, Dec 23, 2021 at 10:43 AM Florian Weimer via Libc-alpha
>> > <libc-alpha@sourceware.org> wrote:
>> >>
>> >> I implemented this because we received a bug that the _r_debug extension
>> >> mechanism broke dyninst.  But it turns out it wasn't because of a copy
>> >> relocation: dyninst has its own interposing *definition* of _r_debug, so
>> >> the patch series doesn't solve this.  It can only handle an interposing
>> >> definition in the main executable, not one in shared object (although I
>> >> guess in theory we could fix this).
>> >
>> > Why doesn't GDB suffer from this issue?
>>
>> I expect that GDB uses DT_DEBUG, and that's not subject to
>> interposition, as before.
>
> Can dyninst also use DT_DEBUG?

Not easily.  They can keep using _r_debug if they just remove their
local definition.

Thanks,
Florian

Re: [PATCH 0/4] _r_debug copy relocation support

[H.J. Lu]

On Thu, Dec 23, 2021 at 1:00 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Thu, Dec 23, 2021 at 11:59 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * H. J. Lu:
> >>
> >> > On Thu, Dec 23, 2021 at 10:43 AM Florian Weimer via Libc-alpha
> >> > <libc-alpha@sourceware.org> wrote:
> >> >>
> >> >> I implemented this because we received a bug that the _r_debug extension
> >> >> mechanism broke dyninst.  But it turns out it wasn't because of a copy
> >> >> relocation: dyninst has its own interposing *definition* of _r_debug, so
> >> >> the patch series doesn't solve this.  It can only handle an interposing
> >> >> definition in the main executable, not one in shared object (although I
> >> >> guess in theory we could fix this).
> >> >
> >> > Why doesn't GDB suffer from this issue?
> >>
> >> I expect that GDB uses DT_DEBUG, and that's not subject to
> >> interposition, as before.
> >
> > Can dyninst also use DT_DEBUG?
>
> Not easily.  They can keep using _r_debug if they just remove their
> local definition.

DT_DEBUG is easily accessible.  I'd rather fix dyninst than add hacks
in ld.so.

-- 
H.J.