From patchwork Mon May 30 13:46:48 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 627830 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3rJHwj2Xczz9t3Z for ; Mon, 30 May 2016 23:47:01 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b=CwtnhsOU; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:references:cc:from:message-id:date :mime-version:in-reply-to:content-type; q=dns; s=default; b=pVQJ laP3+ZnrJ+jliPsuPhiNblM4yfDOpNlcfPtFVCZ7g5vYrJhvffY7vSW3xXnDYM7m hYSRHUFVM9ci22iQbccWRZUoPRKAuk/P73+FpoUlZDp9591scPwBatkccPMjVrJm zXMM7I3wBbRT8Sr/9yKSUnCdAHisJ1WZ5fb9uTg= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:references:cc:from:message-id:date :mime-version:in-reply-to:content-type; s=default; bh=/JX3Iy1YHH p8ek/FqbjpZEnI0m8=; b=CwtnhsOUAj6jD/cmXcqQbxMhQmmniLdGQs1jnapv6A 8BYBnev/pQ5A1EVMP6ipxxUacduaV6jtqtE/q+kgRO4iKZg76fjWtrbiQwpahzzu 48l8WO1h2b6ApPGSSqiaEMoAa0MOAtIqZut4nsAM7I9a+9yDrEGIozH2ixytVzwJ A= Received: (qmail 32282 invoked by alias); 30 May 2016 13:46:56 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 32272 invoked by uid 89); 30 May 2016 13:46:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=BAYES_20, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Nothing, Renamed, worthwhile, historical X-HELO: mx1.redhat.com Subject: Re: [PATCH] fork in libpthread cannot use IFUNC resolver [BZ #19861] To: Roland McGrath References: <20160524091019.269A04222879F@oldenburg.str.redhat.com> <20160527195123.919292C3D1E@topped-with-meat.com> Cc: libc-alpha@sourceware.org From: Florian Weimer Message-ID: <6d5b1d1d-ccee-3928-48e8-8547ddee3f03@redhat.com> Date: Mon, 30 May 2016 15:46:48 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <20160527195123.919292C3D1E@topped-with-meat.com> On 05/27/2016 09:51 PM, Roland McGrath wrote: >> [BZ #19861] >> Do not use IFUNC resolver with potentially unrelcated symbol. > > typo: "unrelocated" > >> --- a/nptl/pt-fork.c >> +++ b/nptl/pt-fork.c >> @@ -25,48 +25,20 @@ >> the historical ABI requires it. For static linking, there is no need to >> provide anything here--the libc version will be linked in. For shared >> library ABI compatibility, there must be __fork and fork symbols in >> - libpthread.so; so we define them using IFUNC to redirect to the libc >> - function. */ >> + libpthread.so. */ > > I think it's worthwhile for this comment to address the IFUNC issue. > e.g., "It's tempting to avoid the tail-call overhead in the shared library > case by using IFUNC here, but ..." > >> -DEFINE_FORK (fork_ifunc) >> +strong_alias (fork_compat, fork_ifunc) >> compat_symbol (libpthread, fork_ifunc, fork, GLIBC_2_0); >> >> -DEFINE_FORK (__fork_ifunc) >> +strong_alias (fork_compat, __fork_ifunc) >> compat_symbol (libpthread, __fork_ifunc, __fork, GLIBC_2_0); > > Nothing that remains should have "ifunc" in the name. Thanks. I believe I have addressed your comments. Florian fork in libpthread cannot use IFUNC resolver [BZ #19861] This commit only addresses the fork case, the vfork case has to be a tail call, which is why the generic code needs an IFUNC resolver there. 2016-05-30 Florian Weimer [BZ #19861] Do not use IFUNC resolver with potentially unrelocated symbol. * nptl/pt-fork.c [HAVE_IFUNC]: Remove. (DEFINE_FORK): Remove macro and inline definition. (fork_alias): Renamed from fork_ifunc. (__fork_alias): Renamed from __fork_ifunc. diff --git a/nptl/pt-fork.c b/nptl/pt-fork.c index b65d6b4..db9b61d 100644 --- a/nptl/pt-fork.c +++ b/nptl/pt-fork.c @@ -25,48 +25,25 @@ the historical ABI requires it. For static linking, there is no need to provide anything here--the libc version will be linked in. For shared library ABI compatibility, there must be __fork and fork symbols in - libpthread.so; so we define them using IFUNC to redirect to the libc - function. */ + libpthread.so. + + With an IFUNC resolver, it would be possible to avoid the + indirection, but the IFUNC resolver might run before the + __libc_fork symbol has been relocated, in which case the IFUNC + resolver would not be able to provide the correct address. */ #if SHLIB_COMPAT (libpthread, GLIBC_2_0, GLIBC_2_22) -# if HAVE_IFUNC - -static __typeof (fork) * -__attribute__ ((used)) -fork_resolve (void) -{ - return &__libc_fork; -} - -# ifdef HAVE_ASM_SET_DIRECTIVE -# define DEFINE_FORK(name) \ - asm (".set " #name ", fork_resolve\n" \ - ".globl " #name "\n" \ - ".type " #name ", %gnu_indirect_function"); -# else -# define DEFINE_FORK(name) \ - asm (#name " = fork_resolve\n" \ - ".globl " #name "\n" \ - ".type " #name ", %gnu_indirect_function"); -# endif - -# else /* !HAVE_IFUNC */ - static pid_t __attribute__ ((used)) fork_compat (void) { return __libc_fork (); } -# define DEFINE_FORK(name) strong_alias (fork_compat, name) +strong_alias (fork_compat, fork_alias) +compat_symbol (libpthread, fork_alias, fork, GLIBC_2_0); -# endif /* HAVE_IFUNC */ - -DEFINE_FORK (fork_ifunc) -compat_symbol (libpthread, fork_ifunc, fork, GLIBC_2_0); - -DEFINE_FORK (__fork_ifunc) -compat_symbol (libpthread, __fork_ifunc, __fork, GLIBC_2_0); +strong_alias (fork_compat, __fork_alias) +compat_symbol (libpthread, __fork_alias, __fork, GLIBC_2_0); #endif