From: Daniel Walker <danielwa@cisco.com>
To: "H.J. Lu" <hjl.tools@gmail.com>
Cc: libc-alpha@sourceware.org, gdb@sourceware.org
Subject: Re: [PATCH v2 2/2] Extend struct r_debug to support multiple namespaces
Date: Tue, 17 Aug 2021 10:44:13 -0700 [thread overview]
Message-ID: <20210817174413.GG1633923@zorba> (raw)
In-Reply-To: <20210817010629.593479-3-hjl.tools@gmail.com>
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 <conhuang@cisco.com>:
>
> 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
next prev parent reply other threads:[~2021-08-17 17:44 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-17 1:06 [PATCH v2 0/2] " H.J. Lu
2021-08-17 1:06 ` [PATCH v2 1/2] Add declare_object_symbol_alias for assembly codes [BZ #28128] H.J. Lu
2021-08-17 1:06 ` [PATCH v2 2/2] Extend struct r_debug to support multiple namespaces H.J. Lu
2021-08-17 17:44 ` Daniel Walker [this message]
2021-08-17 20:25 ` H.J. Lu
2021-08-18 0:14 ` H.J. Lu
2021-08-17 17:57 ` Daniel Walker
2021-08-17 20:26 ` H.J. Lu
2021-08-18 0:21 ` H.J. Lu
2021-08-18 2:36 ` H.J. Lu
2021-08-18 13:34 ` H.J. Lu
2021-08-17 17:57 ` [PATCH v2 0/2] " Joseph Myers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210817174413.GG1633923@zorba \
--to=danielwa@cisco.com \
--cc=gdb@sourceware.org \
--cc=hjl.tools@gmail.com \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).