From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 4F007385841A for ; Mon, 6 Sep 2021 09:39:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4F007385841A Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-152-2MJZV4EcPiOM6mHl3sCa1w-1; Mon, 06 Sep 2021 05:39:13 -0400 X-MC-Unique: 2MJZV4EcPiOM6mHl3sCa1w-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 9352A107ACC7; Mon, 6 Sep 2021 09:39:12 +0000 (UTC) Received: from oldenburg.str.redhat.com (unknown [10.39.194.140]) by smtp.corp.redhat.com (Postfix) with ESMTPS id CC74869FC2; Mon, 6 Sep 2021 09:39:10 +0000 (UTC) From: Florian Weimer To: "H.J. Lu" Cc: libc-alpha@sourceware.org, gdb@sourceware.org, libc-coord@lists.openwall.com, Daniel Walker Subject: Re: [PATCH v6 2/2] Extend struct r_debug to support multiple namespaces References: <20210830173844.458727-1-hjl.tools@gmail.com> <20210830173844.458727-3-hjl.tools@gmail.com> Date: Mon, 06 Sep 2021 11:39:08 +0200 In-Reply-To: <20210830173844.458727-3-hjl.tools@gmail.com> (H. J. Lu's message of "Mon, 30 Aug 2021 10:38:44 -0700") Message-ID: <874kay11s3.fsf@oldenburg.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-14.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=unavailable 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: Mon, 06 Sep 2021 09:39:18 -0000 * H. J. Lu: > +* The r_version update in the debugger interface makes the glibc binary > + incompatible with GDB binaries built without the following commits: > + > + c0154a4a21a gdb: Don't assume r_ldsomap when r_version > 1 on Linux > + 4eb629d50d4 gdbserver: Check r_version < 1 for Linux debugger interface Does this incompatibility happen even if audit modules and dlmopen are not used? > +ifeq ($(build-shared),yes) > +# dl-debug-compat-symbols.os and dl-debug-compat-symbols.o are assembly > +# codes included in dl-debug-symbols.S. > +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 This puts the assember output from the compiler through the preprocessor. That seems to be brittle. I think you would have to preprocess the manually written fragment separately. However, I think we are overdesigning things here. The following in dl-debug-symbols-gen.c should work (and the file should have a different name then): /* Alias _r_debug to a prefix of _r_debug_extended. */ asm (".set _r_debug, _r_debug_extended\n\t" ".type _r_debug, %object\n\t" ".symver _r_debug_extended, _r_debug@@" FIRST_VERSION_ld__r_debug_STRING); #if __WORDSIZE == 64 _Static_assert (sizeof (struct r_debug) == 40, "sizeof (struct r_debug)"); asm (".size _r_debug, 40"); #else _Static_assert (sizeof (struct r_debug) == 20, "sizeof (struct r_debug)"); asm (".size _r_debug, 20"); #endif It's not exactly pretty, but at least it's obvious what is going on. (Extended asm with input operands is not supported outside of functions.) And it's not that the size of struct _r_debug is going to change. > diff --git a/elf/dl-close.c b/elf/dl-close.c > index f39001cab9..43873d2543 100644 > --- a/elf/dl-close.c > +++ b/elf/dl-close.c > @@ -709,6 +709,27 @@ _dl_close_worker (struct link_map *map, bool force) > assert (nsid != LM_ID_BASE); > ns->_ns_loaded = imap->l_next; > > + if (ns->_ns_loaded == NULL) > + { > + /* Remove the empty namespace from the namespace linked > + list. */ > + struct r_debug_extended **pp, *p; > + > + for (pp = &_r_debug_extended.r_next; > + (p = *pp) != NULL; > + pp = &p->r_next) > + if (p == &ns->_ns_debug) > + { > + /* Remove the empty namespace. */ > + *pp = p->r_next; > + > + /* Clear r_version to indicate that it is > + unused. */ > + p->base.r_version = 0; > + break; > + } > + } Is this necessary? It makes concurrent access to the list harder and does not save any memory. > diff --git a/elf/rtld-debugger-interface.txt b/elf/rtld-debugger-interface.txt > index 61bc99e4b0..f6aaa28706 100644 > --- a/elf/rtld-debugger-interface.txt > +++ b/elf/rtld-debugger-interface.txt > @@ -9,6 +9,9 @@ structure can be found. > > The r_debug structure contains (amongst others) the following fields: > > + int r_version: > + Version number for this protocol. It should be greater than 0. > + It seems to me that r_version starts out as 0 and is initialized later. Maybe this should be described here, and tested in the test case. Thanks, Florian