Re: [PATCH v2 2/2] Extend struct r_debug to support multiple namespaces

On Mon, Aug 16, 2021 at 06:06:29PM -0700, H.J. Lu wrote:
> Glibc does not provide an interface for debugger to access libraries
> loaded in multiple namespaces via dlmopen.
> 
> The current rtld-debugger interface is described in the file:
> 
> elf/rtld-debugger-interface.txt
> 
> under the "Standard debugger interface" heading.  This interface only
> provides access to the first link-map (LM_ID_BASE).
> 
> Based on the patch from Conan C Huang <[email protected]>:
> 
> https://sourceware.org/pipermail/libc-alpha/2020-June/115448.html
> 
> 1. Bump r_version to 2.  This triggers the GDB bug:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=28236
> 
> 2. Add struct r_debug_extended to extend struct r_debug into a linked-list,
> where each element correlates to an unique namespace.
> 3. Add a hidden symbol, _r_debug_extended, for struct r_debug_extended.
> 4. Provide the compatibility symbol, _r_debug, with size of struct r_debug,
> as an alise of _r_debug_extended, for programs which reference _r_debug.

It's very similar to patches we have already created internally.

One comment below,

> ---
>  csu/Makefile                    |  3 ++
>  csu/rtld-sizes.sym              |  4 ++
>  elf/Makefile                    | 22 ++++++++++-
>  elf/dl-close.c                  |  2 +-
>  elf/dl-debug-symbols-gen.c      | 24 +++++++++++
>  elf/dl-debug-symbols.S          | 31 +++++++++++++++
>  elf/dl-debug.c                  | 29 +++++++-------
>  elf/dl-load.c                   |  2 +-
>  elf/dl-open.c                   |  2 +-
>  elf/dl-reloc-static-pie.c       |  2 +-
>  elf/link.h                      | 70 +++++++++++++++++++++++++--------
>  elf/rtld-debugger-interface.txt | 14 +++++++
>  elf/rtld.c                      |  4 +-
>  elf/tst-dlmopen4.c              | 68 ++++++++++++++++++++++++++++++++
>  include/link.h                  |  4 ++
>  sysdeps/generic/ldsodefs.h      |  5 ++-
>  16 files changed, 245 insertions(+), 41 deletions(-)
>  create mode 100644 csu/rtld-sizes.sym
>  create mode 100644 elf/dl-debug-symbols-gen.c
>  create mode 100644 elf/dl-debug-symbols.S
>  create mode 100644 elf/tst-dlmopen4.c
> 
> diff --git a/csu/Makefile b/csu/Makefile
> index 3054329cea..e2390e4a7d 100644
> --- a/csu/Makefile
> +++ b/csu/Makefile
> @@ -88,6 +88,9 @@ endif
>  before-compile += $(objpfx)abi-tag.h
>  generated += abi-tag.h
>  
> +# Put it here to generate it earlier.
> +gen-as-const-headers += rtld-sizes.sym
> +
>  # These are the special initializer/finalizer files.  They are always the
>  # first and last file in the link.  crti.o ... crtn.o define the global
>  # "functions" _init and _fini to run the .init and .fini sections.
> diff --git a/csu/rtld-sizes.sym b/csu/rtld-sizes.sym
> new file mode 100644
> index 0000000000..40dd8edaec
> --- /dev/null
> +++ b/csu/rtld-sizes.sym
> @@ -0,0 +1,4 @@
> +#include <link.h>
> +
> +--
> +COMPAT_R_DEBUG_SIZE	sizeof (struct r_debug)
> diff --git a/elf/Makefile b/elf/Makefile
> index 725537c40b..1444a53405 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -35,7 +35,8 @@ dl-routines	= $(addprefix dl-,load lookup object reloc deps \
>  				  execstack open close trampoline \
>  				  exception sort-maps lookup-direct \
>  				  call-libc-early-init write \
> -				  thread_gscope_wait tls_init_tp)
> +				  thread_gscope_wait tls_init_tp \
> +				  debug-symbols)
>  ifeq (yes,$(use-ldconfig))
>  dl-routines += dl-cache
>  endif
> @@ -203,7 +204,7 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
>  	 tst-tls16 tst-tls17 tst-tls18 tst-tls19 tst-tls-dlinfo \
>  	 tst-align tst-align2 \
>  	 tst-dlmodcount tst-dlopenrpath tst-deep1 \
> -	 tst-dlmopen1 tst-dlmopen3 \
> +	 tst-dlmopen1 tst-dlmopen3 tst-dlmopen4 \
>  	 unload3 unload4 unload5 unload6 unload7 unload8 tst-global1 order2 \
>  	 tst-audit1 tst-audit2 tst-audit8 tst-audit9 \
>  	 tst-addr1 tst-thrlock \
> @@ -672,6 +673,21 @@ LC_ALL=C sed $(ldd-rewrite) < $< \
>  endef
>  endif
>  
> +ifeq ($(build-shared),yes)
> +generated += dl-debug-compat-symbols.os dl-debug-compat-symbols.o
> +
> +libof-dl-debug-compat-symbols = rtld
> +
> +$(objpfx)dl-debug-compat-symbols.os: dl-debug-symbols-gen.c
> +	$(compile-command.c) -S
> +
> +$(objpfx)dl-debug-compat-symbols.o: dl-debug-symbols-gen.c
> +	$(compile-command.c) -S
> +
> +$(objpfx)dl-debug-symbols.os: $(objpfx)dl-debug-compat-symbols.os
> +$(objpfx)dl-debug-symbols.o: $(objpfx)dl-debug-compat-symbols.o
> +endif
> +
>  $(objpfx)ldd: ldd.bash.in $(common-objpfx)soversions.mk \
>  	      $(common-objpfx)config.make
>  	$(gen-ldd)
> @@ -1242,6 +1258,8 @@ $(objpfx)tst-dlmopen2.out: $(objpfx)tst-dlmopen1mod.so
>  
>  $(objpfx)tst-dlmopen3.out: $(objpfx)tst-dlmopen1mod.so
>  
> +$(objpfx)tst-dlmopen4.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-close.c b/elf/dl-close.c
> index f39001cab9..f59ffdd666 100644
> --- a/elf/dl-close.c
> +++ b/elf/dl-close.c
> @@ -500,7 +500,7 @@ _dl_close_worker (struct link_map *map, bool force)
>  #endif
>  
>    /* Notify the debugger we are about to remove some loaded objects.  */
> -  struct r_debug *r = _dl_debug_initialize (0, nsid);
> +  struct r_debug_extended *r = _dl_debug_initialize (0, nsid);
>    r->r_state = RT_DELETE;
>    _dl_debug_state ();
>    LIBC_PROBE (unmap_start, 2, nsid, r);
> diff --git a/elf/dl-debug-symbols-gen.c b/elf/dl-debug-symbols-gen.c
> new file mode 100644
> index 0000000000..2406260bcb
> --- /dev/null
> +++ b/elf/dl-debug-symbols-gen.c
> @@ -0,0 +1,24 @@
> +/* Generate the _r_debug_extended symbol used to communicate dynamic
> +   linker state to the debugger at runtime.
> +   Copyright (C) 2021 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 <link.h>
> +
> +/* This structure communicates dl state to the debugger.  The debugger
> +   finds it via the DT_DEBUG entry in the dynamic section.  */
> +struct r_debug_extended _r_debug_extended;
> diff --git a/elf/dl-debug-symbols.S b/elf/dl-debug-symbols.S
> new file mode 100644
> index 0000000000..0966b172ab
> --- /dev/null
> +++ b/elf/dl-debug-symbols.S
> @@ -0,0 +1,31 @@
> +/* Define symbols used to communicate dynamic linker state to the
> +   debugger at runtime.
> +   Copyright (C) 2021 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 <rtld-sizes.h>
> +
> +#ifdef SHARED
> +# include "dl-debug-compat-symbols.os"
> +#else
> +# include "dl-debug-compat-symbols.o"
> +#endif
> +
> +/* Define the compatibility symbol, _r_debug, with size of struct r_debug,
> +   as an alias of _r_debug_extended.  */
> +declare_object_symbol_alias (_r_debug, _r_debug_extended,
> +			     COMPAT_R_DEBUG_SIZE);
> diff --git a/elf/dl-debug.c b/elf/dl-debug.c
> index 2cd5f09753..9e884a5648 100644
> --- a/elf/dl-debug.c
> +++ b/elf/dl-debug.c
> @@ -30,34 +30,35 @@ extern const int verify_link_map_members[(VERIFY_MEMBER (l_addr)
>  					  && VERIFY_MEMBER (l_prev))
>  					 ? 1 : -1];
>  
> -/* This structure communicates dl state to the debugger.  The debugger
> -   normally finds it via the DT_DEBUG entry in the dynamic section, but in
> -   a statically-linked program there is no dynamic section for the debugger
> -   to examine and it looks for this particular symbol name.  */
> -struct r_debug _r_debug;
> -
> -
>  /* Initialize _r_debug if it has not already been done.  The argument is
>     the run-time load address of the dynamic linker, to be put in
>     _r_debug.r_ldbase.  Returns the address of _r_debug.  */
>  
> -struct r_debug *
> +struct r_debug_extended *
>  _dl_debug_initialize (ElfW(Addr) ldbase, Lmid_t ns)
>  {
> -  struct r_debug *r;
> +  struct r_debug_extended *r, *rp;
>  
>    if (ns == LM_ID_BASE)
> -    r = &_r_debug;
> +    r = &_r_debug_extended;
>    else
> -    r = &GL(dl_ns)[ns]._ns_debug;
> +    {
> +      r = &GL(dl_ns)[ns]._ns_debug;
> +      rp = &GL(dl_ns)[ns - 1]._ns_debug;
> +      rp->r_next = r;
> +      if (ns - 1 == LM_ID_BASE)
> +        _r_debug_extended.r_next = r;
> +    }

I'm not sure why, but we have an issue assigning the dl_ns[ns - 1] when ns -1
was equal to LM_ID_BASE. I don't know what the issue was but we had tests fail
as a result of it. In my case I add an else clause and only set r_next when it
wasn't LM_ID_BASE, and for debugging it didn't matter since the structure was
all that mattered.

Daniel