From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1030.google.com (mail-pj1-x1030.google.com [IPv6:2607:f8b0:4864:20::1030]) by sourceware.org (Postfix) with ESMTPS id 69CCB385743B; Tue, 17 Aug 2021 20:26:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 69CCB385743B Received: by mail-pj1-x1030.google.com with SMTP id m24-20020a17090a7f98b0290178b1a81700so679620pjl.4; Tue, 17 Aug 2021 13:26:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=8Eof9b0qWipfbfTzAoiSie9geERLfnmVlvN9zKMN+p8=; b=Kn0OZA4MqHOPgazp2+Wwgz/QXZ5STRp0cAEaTrj/ND8sh0orYUJY8FX6Os4vYmitLU oYS6zdWG9hVmaxtq4stHp2spPLshiLjcEwCzQTxL+8uEwXznCpTUMwBvZrzzWAlYERhH +eymCKFwKmnVHwTRTREoJi05L9UolN5IZWrjTD26ENviMUGY56CUGokEBA6TTvAFdAsl kuTHImfpjRQAcxDohwAHmgASwZMHE/Sg04OzoLyiU7AQem1Hbdoe93m22eSb8J7DfU5r yOustuZW2Ux2CUMqdjzJ0RAXC3UYYozAtrDdhW5y7fTqyEMu3S/RHcWYjXfs6PXUVeC8 6qNA== X-Gm-Message-State: AOAM531EuygWjKU3xF2W7Lv6hTEZQeN8ldaNqbeHUHZdH8c49s3hlHLS JQ4mrE8st5cafq/e0ztbaM0DRHYjtoEA+3WEdRI= X-Google-Smtp-Source: ABdhPJwIFpEeccxi4Q5DNADfcKgN89hX8D1AJUK//ujgwHuqJvo4+pjNBl1jMj2+k9oGbZxrQBsA8XnBAsh3wvhWYRQ= X-Received: by 2002:a17:902:e291:b0:12d:9d9b:7e5b with SMTP id o17-20020a170902e29100b0012d9d9b7e5bmr4122211plc.4.1629231992554; Tue, 17 Aug 2021 13:26:32 -0700 (PDT) MIME-Version: 1.0 References: <20210817010629.593479-1-hjl.tools@gmail.com> <20210817010629.593479-3-hjl.tools@gmail.com> <20210817174413.GG1633923@zorba> In-Reply-To: <20210817174413.GG1633923@zorba> From: "H.J. Lu" Date: Tue, 17 Aug 2021 13:25:56 -0700 Message-ID: Subject: Re: [PATCH v2 2/2] Extend struct r_debug to support multiple namespaces To: Daniel Walker Cc: GNU C Library , GDB Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3030.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Aug 2021 20:26:44 -0000 On Tue, Aug 17, 2021 at 10:44 AM Daniel Walker wrote: > > 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 : > > > > 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 > > + > > +-- > > +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 > > + . */ > > + > > +#include > > + > > +/* 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 > > + . */ > > + > > +#include > > + > > +#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 _dl_debug_initialize can be called multiple times like _dl_debug_initialize (0, nsid)->r_state = RT_CONSISTENT; I will update my patch to fix it. -- H.J.