From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/13582 Path: news.gmane.org!.POSTED!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: pthread_key_create bug? Date: Mon, 7 Jan 2019 19:00:18 -0500 Message-ID: <20190108000018.GZ23599@brightrain.aerifal.cx> References: <84B22C11-AA93-47FD-8352-A4F18B19F689@gmail.com> <5c1d9cf6-1b53-7082-5dee-8673ed4c55e9@adelielinux.org> <20190107021128.GW23599@brightrain.aerifal.cx> <20190107171327.GD29911@voyager> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: blaine.gmane.org 1546905508 13950 195.159.176.226 (7 Jan 2019 23:58:28 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Mon, 7 Jan 2019 23:58:28 +0000 (UTC) User-Agent: Mutt/1.5.21 (2010-09-15) To: musl@lists.openwall.com Original-X-From: musl-return-13598-gllmg-musl=m.gmane.org@lists.openwall.com Tue Jan 08 00:58:23 2019 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1ggemk-0003UF-6b for gllmg-musl@m.gmane.org; Tue, 08 Jan 2019 00:58:22 +0100 Original-Received: (qmail 19977 invoked by uid 550); 8 Jan 2019 00:00:30 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 19959 invoked from network); 8 Jan 2019 00:00:30 -0000 Content-Disposition: inline In-Reply-To: <20190107171327.GD29911@voyager> Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:13582 Archived-At: On Mon, Jan 07, 2019 at 06:13:28PM +0100, Markus Wichmann wrote: > On Sun, Jan 06, 2019 at 09:11:28PM -0500, Rich Felker wrote: > > See commit 84d061d5a31c9c773e29e1e2b1ffe8cb9557bc58. > > > > Rich > > Speaking of that commit: Am I missing something? The log message of that > commit says that pthread_key_delete was stuffed into another file to > avoid __synccall() being pulled into programs not referencing > pthread_key_delete(). Yet pthread_key_create.c contains > clean_dirty_tsd(), which contains a hard dependency on > __pthread_key_delete_synccall(), which will do the synccall, and thus > pull everything in. > > I appreciate that the function in question can never be called unless > pthread_key_delete() is used, but the linker can't know that. The > compiler can't see code of the other compilation units and thus has to > generate code containing a reference to __pthread_key_delete_synccall(), > and the linker can't see that the reference is unreachable unless > __pthread_key_delete() is linked in... > > Wait a minute. If we made that a weak reference, that would already > suffice. Wouldn't even necessarily need an alias, since it is > unreachable without pthread_key_delete() anyway. > > Is the attached patch acceptable? > > Ciao, > Markus > >From 851f8bb89d9fae664a0d6018fce251ab64d737b7 Mon Sep 17 00:00:00 2001 > From: Markus Wichmann > Date: Mon, 7 Jan 2019 18:07:34 +0100 > Subject: [PATCH 4/4] Add weak reference to reduce static size. > > Commit 84d061d5a31c9c773e29e1e2b1ffe8cb9557bc58 claimed to try and > reduce the static size for programs referencing pthread_key_create() but > not pthread_key_delete(). Unfortunately, the critical reference was > still strong, so the __synccall() mechanism was pulled in, anyway. This > commit makes the reference weak, so it is resolved to NULL if > pthread_key_delete() is not used, but in that case the function is > unreachable, anyway. > --- > src/thread/pthread_key_create.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/src/thread/pthread_key_create.c b/src/thread/pthread_key_create.c > index e26f199c..31ed0826 100644 > --- a/src/thread/pthread_key_create.c > +++ b/src/thread/pthread_key_create.c > @@ -35,6 +35,8 @@ static void clean_dirty_tsd_callback(void *p) > if (args->caller == self) args->ret = 0; > } > > +extern hidden weak void __pthread_key_delete_synccall(void (*f)(void *), void *p); > + > static int clean_dirty_tsd(void) > { > struct cleanup_args args = { > -- > 2.19.1 > I think you're right, though we don't generally use weak references in musl on the basis (perhaps somewhat dubious) that they're an additional toolchain feature that might cause problems reusing the code in non-ELF contexts (this may affect midipix; I'm not sure). That's why weak aliases to dummy functions are used for this purpose everywhere else. Also: weak references to hidden functions historically had some breakage with respect to generating unresolvable-at-ld-time pc-relative references to the symbol. Thus their use might break some gcc/binutils versions too. Rich