From owner-freebsd-threads@FreeBSD.ORG Sun Jan 4 11:46:06 2015 Return-Path: Delivered-To: threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id D24262C9; Sun, 4 Jan 2015 11:46:06 +0000 (UTC) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 71897381E; Sun, 4 Jan 2015 11:46:06 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.9/8.14.9) with ESMTP id t04Bk0Nb019720 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 4 Jan 2015 13:46:00 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.9.2 kib.kiev.ua t04Bk0Nb019720 Received: (from kostik@localhost) by tom.home (8.14.9/8.14.9/Submit) id t04Bk0UZ019718; Sun, 4 Jan 2015 13:46:00 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Sun, 4 Jan 2015 13:46:00 +0200 From: Konstantin Belousov To: Jilles Tjoelker Subject: Re: Fixing dlopen("libpthread.so") Message-ID: <20150104114600.GC42409@kib.kiev.ua> References: <20141226165337.GJ1754@kib.kiev.ua> <20150103212837.GC46373@stack.nl> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="yudcn1FV7Hsu/q59" Content-Disposition: inline In-Reply-To: <20150103212837.GC46373@stack.nl> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.0 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on tom.home Cc: threads@freebsd.org, arch@freebsd.org X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 04 Jan 2015 11:46:06 -0000 --yudcn1FV7Hsu/q59 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Jan 03, 2015 at 10:28:37PM +0100, Jilles Tjoelker wrote: >=20 > I think this can work, conceptually. The changes were already committed, right before I got your mail. Still, thank you for the useful review. >=20 > Does this also mean that the order of libc and libthr on the link line > no longer matters? (so SVN r265003 could be reverted?) I think yes, it could. I have similar queries already. IMO it makes sense to wait some time for the changes to settle before even asking for testing of such reversals. >=20 > It looks like this change may introduce more indirection in common paths > which might harm performance. The effect is slight and on the other hand > more single-threaded applications can benefit from non-libthr > performance. >=20 > I noticed a problem in lib/libc/gen/raise.c: it calls the INTERPOS_pause > entry instead of the INTERPOS_raise entry. Alternatively, libc could > call thr_self() and thr_kill() so libthr need not do anything (much like > how lib/libc/gen/sem_new.c works). This was noted already and corrected raise.c committed. I have mixed feeling about the proposal to change raise(3) to use threading syscalls. On the one hand, it indeed works, on the other, there are subtle differences, not all important, WRT the change. E.g., puzzling ktrace output, changed si_code from SI_USER to SI_LWP (is this a bug on its own ?) . >=20 > The number of interposers could be reduced slightly by doing simple work > like wait(), waitpid() and wait3() before instead of after > interposition. This would be an additional change and the current patch > is more like the existing code. Could you, please, explain this ? What exactly should be done before what, and how would it help ? >=20 > In lib/libc/sys/__error.c, __error_selector should be declared static or > __hidden and an accessor function added for libthr, to avoid an > additional pointer chase in a common path. One more PLT and GOT > reference in some situations may be avoided by making a __hidden alias > for __error_unthreaded and making that __error_selector's initial value. __error_unthreaded can be made static, it seems. I added __set_error_selector() and made __error_selector static. >=20 > Although __libc_interposing is properly not exported (with > __libc_interposing_slot accessor), the full benefit is not obtained > because __libc_interposing is not declared __hidden in > lib/libc/include/libc_private.h, so the compiler will generate an > indirect access anyway (and ld will fix it up using a relative > relocation). Done. >=20 > In lib/libthr/thread/thr_private.h, __error_threaded() can be declared > __hidden. Done. >=20 > Sharing __sys_* code between libc and libthr is a net loss (the two > copies of the name, GOT entry, PLT entry and longer instruction > sequences are almost certainly larger than a syscall stub (32 bytes on > amd64)), so I think the best fix for most indirection from > __libc_interposing entries is to give libthr its own copy and mark both > copies hidden, instead of adding hidden aliases now. Do you mean linking syscall stubs (__sys_* trampolines) statically into the libthr, and referencing them directly from the C code ? Yes, this will be huge benefitial, both for libthr and ld-elf.so.1, for rtld it is even kind of required, see r276646 and other uses of __sys. I thought about it. But we need something like libsyscalls.a and libsyscalls_pic.a extracted from libc first. If I am misinterpreting your suggestion, please explain it in more details. diff --git a/lib/libc/include/libc_private.h b/lib/libc/include/libc_privat= e.h index adf342d..ea1a97b 100644 --- a/lib/libc/include/libc_private.h +++ b/lib/libc/include/libc_private.h @@ -173,13 +173,13 @@ typedef pthread_func_t pthread_func_entry_t[2]; =20 extern pthread_func_entry_t __thr_jtable[]; =20 -extern int *(*__error_selector)(void); +void __set_error_selector(int *(*arg)(void)); int _pthread_mutex_init_calloc_cb_stub(pthread_mutex_t *mutex, void *(calloc_cb)(__size_t, __size_t)); =20 typedef int (*interpos_func_t)(void); interpos_func_t *__libc_interposing_slot(int interposno); -extern interpos_func_t __libc_interposing[]; +extern interpos_func_t __libc_interposing[] __hidden; =20 enum { INTERPOS_accept, diff --git a/lib/libc/sys/Symbol.map b/lib/libc/sys/Symbol.map index 7d82361..f1f57ba 100644 --- a/lib/libc/sys/Symbol.map +++ b/lib/libc/sys/Symbol.map @@ -1045,8 +1045,7 @@ FBSDprivate_1.0 { __sys_write; _writev; __sys_writev; - __error_unthreaded; - __error_selector; + __set_error_selector; nlm_syscall; gssd_syscall; __libc_interposing_slot; diff --git a/lib/libc/sys/__error.c b/lib/libc/sys/__error.c index 329d124..28cc31d 100644 --- a/lib/libc/sys/__error.c +++ b/lib/libc/sys/__error.c @@ -32,13 +32,21 @@ __FBSDID("$FreeBSD$"); =20 extern int errno; =20 -int * +static int * __error_unthreaded(void) { - return(&errno); + + return (&errno); } =20 -int *(*__error_selector)(void) =3D __error_unthreaded; +static int *(*__error_selector)(void) =3D __error_unthreaded; + +void +__set_error_selector(int *(*arg)(void)) +{ + + __error_selector =3D arg; +} =20 int * __error(void) diff --git a/lib/libthr/thread/thr_private.h b/lib/libthr/thread/thr_privat= e.h index e8edbe1..0c16b2a 100644 --- a/lib/libthr/thread/thr_private.h +++ b/lib/libthr/thread/thr_private.h @@ -914,7 +914,7 @@ void _thr_tsd_unload(struct dl_phdr_info *phdr_info) __= hidden; void _thr_sigact_unload(struct dl_phdr_info *phdr_info) __hidden; void _thr_stack_fix_protection(struct pthread *thrd); =20 -int *__error_threaded(void); +int *__error_threaded(void) __hidden; void __thr_interpose_libc(void) __hidden; pid_t __thr_fork(void); int __thr_pause(void) __hidden; diff --git a/lib/libthr/thread/thr_syscalls.c b/lib/libthr/thread/thr_sysca= lls.c index 60427d8..a4fe7e8 100644 --- a/lib/libthr/thread/thr_syscalls.c +++ b/lib/libthr/thread/thr_syscalls.c @@ -692,7 +692,7 @@ void __thr_interpose_libc(void) { =20 - __error_selector =3D __error_threaded; + __set_error_selector(__error_threaded); #define SLOT(name) \ *(__libc_interposing_slot(INTERPOS_##name)) =3D \ (interpos_func_t)__thr_##name; --yudcn1FV7Hsu/q59 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJUqSf4AAoJEJDCuSvBvK1BltwQAKbcDFHdJ2kXYZkyrdhTqKRd mBqYUz6xvSeNrfNGmGnK+PpVq5+Oky6vkuE5J0K6eS3mQphzwAflumQake6T13xe RSUz7bmef8y9mSq4fMuJMLI0k3ppFQMV7+tjdORoVOMXV9FB6FUpYNZr51mTEhrg WPtsDOo7vbGZjiDP3WFzsbWkQRftq7G7YpVdA3Ya8FNOSrTDzxdQ1yVMjex/4rP0 9i3GDs1GjgP5tOAjU2XC26/tBU+qvnAgLr0sMPbVkutSx0FZVsiHu4voGcg4zfKd RD0UOe1wF0+GW1NBlfh/aH095ay+V/qs8RFkspIBJ9P+U6mr1VmU+wF1uINyL151 W7VL7AaLxlfhIz2UUsKflN/NKAmSfV6DHo4gIC0eHxVbGOzwbXgb9Kzu2rbj1as2 hFqPZ41A4uMxCSUIXvybkvkOJs7g3eN/v7kHb/n/23eRomQ84yATvopioqhPGxna hdTzqOUlPfIYdudVX4mhUZCYYtoDkz9EmVheIEhXAXMucM/OJWuUhnimw8OX3Qae 9Du6NhNS/catg7RU+Vbc7Z6Rt/69Fd1HbWb4dc01jcFOV96BpGmUiOc0/C2qstIg +X4c9Y5QvbpUxwjPX4pcJOyHL7CoLDTUIlQ2Jzm4weanLlE6hUx/Xq92Dq9MdhZS meOJXHCDL3UMuwWZgHQ0 =JGXq -----END PGP SIGNATURE----- --yudcn1FV7Hsu/q59-- From owner-freebsd-threads@FreeBSD.ORG Sun Jan 4 18:20:31 2015 Return-Path: Delivered-To: threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 67CD4705; Sun, 4 Jan 2015 18:20:31 +0000 (UTC) Received: from mx1.stack.nl (relay04.stack.nl [IPv6:2001:610:1108:5010::107]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client CN "mailhost.stack.nl", Issuer "CA Cert Signing Authority" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id EC4D5355; Sun, 4 Jan 2015 18:20:30 +0000 (UTC) Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131]) by mx1.stack.nl (Postfix) with ESMTP id 37D12B805D; Sun, 4 Jan 2015 19:20:27 +0100 (CET) Received: by snail.stack.nl (Postfix, from userid 1677) id 2272528494; Sun, 4 Jan 2015 19:20:27 +0100 (CET) Date: Sun, 4 Jan 2015 19:20:27 +0100 From: Jilles Tjoelker To: Konstantin Belousov Subject: Re: Fixing dlopen("libpthread.so") Message-ID: <20150104182026.GA61250@stack.nl> References: <20141226165337.GJ1754@kib.kiev.ua> <20150103212837.GC46373@stack.nl> <20150104114600.GC42409@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150104114600.GC42409@kib.kiev.ua> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: threads@freebsd.org, arch@freebsd.org X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 04 Jan 2015 18:20:31 -0000 On Sun, Jan 04, 2015 at 01:46:00PM +0200, Konstantin Belousov wrote: > On Sat, Jan 03, 2015 at 10:28:37PM +0100, Jilles Tjoelker wrote: > > I think this can work, conceptually. > The changes were already committed, right before I got your mail. > Still, thank you for the useful review. > > Does this also mean that the order of libc and libthr on the link line > > no longer matters? (so SVN r265003 could be reverted?) > I think yes, it could. I have similar queries already. > IMO it makes sense to wait some time for the changes to settle before > even asking for testing of such reversals. OK. > > It looks like this change may introduce more indirection in common paths > > which might harm performance. The effect is slight and on the other hand > > more single-threaded applications can benefit from non-libthr > > performance. > > I noticed a problem in lib/libc/gen/raise.c: it calls the INTERPOS_pause > > entry instead of the INTERPOS_raise entry. Alternatively, libc could > > call thr_self() and thr_kill() so libthr need not do anything (much like > > how lib/libc/gen/sem_new.c works). > This was noted already and corrected raise.c committed. > I have mixed feeling about the proposal to change raise(3) to use > threading syscalls. On the one hand, it indeed works, on the other, > there are subtle differences, not all important, WRT the change. E.g., > puzzling ktrace output, changed si_code from SI_USER to SI_LWP (is this > a bug on its own ?) . SI_LWP already occurs with libthr's raise(), and with pthread_kill(pthread_self(), sig); which POSIX says is equivalent. > > The number of interposers could be reduced slightly by doing simple work > > like wait(), waitpid() and wait3() before instead of after > > interposition. This would be an additional change and the current patch > > is more like the existing code. > Could you, please, explain this ? What exactly should be done before what, > and how would it help ? In libc, define the below wait3(). All code in libc that allows libthr to interpose wait3() and in libthr that interposes wait3() can then go away. pid_t wait3(int *istat, int options, struct rusage *rup) { return (((pid_t (*)(pid_t, int *, int, struct rusage *)) __libc_interposing[INTERPOS_wait4])(WAIT_ANY, istat, options, rup)); } This change is also possible for wait() and waitpid(), but not for e.g. sigwait() which has different [EINTR] behaviour than sigtimedwait(). > > In lib/libc/sys/__error.c, __error_selector should be declared static or > > __hidden and an accessor function added for libthr, to avoid an > > additional pointer chase in a common path. One more PLT and GOT > > reference in some situations may be avoided by making a __hidden alias > > for __error_unthreaded and making that __error_selector's initial value. > __error_unthreaded can be made static, it seems. > I added __set_error_selector() and made __error_selector static. > > Although __libc_interposing is properly not exported (with > > __libc_interposing_slot accessor), the full benefit is not obtained > > because __libc_interposing is not declared __hidden in > > lib/libc/include/libc_private.h, so the compiler will generate an > > indirect access anyway (and ld will fix it up using a relative > > relocation). > Done. > > In lib/libthr/thread/thr_private.h, __error_threaded() can be declared > > __hidden. > Done. The changes look good to me. > > Sharing __sys_* code between libc and libthr is a net loss (the two > > copies of the name, GOT entry, PLT entry and longer instruction > > sequences are almost certainly larger than a syscall stub (32 bytes on > > amd64)), so I think the best fix for most indirection from > > __libc_interposing entries is to give libthr its own copy and mark both > > copies hidden, instead of adding hidden aliases now. > Do you mean linking syscall stubs (__sys_* trampolines) statically into > the libthr, and referencing them directly from the C code ? Yes, this > will be huge benefitial, both for libthr and ld-elf.so.1, for rtld it is > even kind of required, see r276646 and other uses of __sys. I thought > about it. But we need something like libsyscalls.a and libsyscalls_pic.a > extracted from libc first. > If I am misinterpreting your suggestion, please explain it in more > details. Your interpretation is correct. -- Jilles Tjoelker From owner-freebsd-threads@FreeBSD.ORG Mon Jan 5 02:37:17 2015 Return-Path: Delivered-To: threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 6DBC6100; Mon, 5 Jan 2015 02:37:17 +0000 (UTC) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id D4DB21225; Mon, 5 Jan 2015 02:37:16 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.9/8.14.9) with ESMTP id t052b8Xr026926 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 5 Jan 2015 04:37:08 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.9.2 kib.kiev.ua t052b8Xr026926 Received: (from kostik@localhost) by tom.home (8.14.9/8.14.9/Submit) id t052b8A7026925; Mon, 5 Jan 2015 04:37:08 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Mon, 5 Jan 2015 04:37:08 +0200 From: Konstantin Belousov To: Jilles Tjoelker Subject: Re: Fixing dlopen("libpthread.so") Message-ID: <20150105023708.GD42409@kib.kiev.ua> References: <20141226165337.GJ1754@kib.kiev.ua> <20150103212837.GC46373@stack.nl> <20150104114600.GC42409@kib.kiev.ua> <20150104182026.GA61250@stack.nl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150104182026.GA61250@stack.nl> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.0 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on tom.home Cc: threads@freebsd.org, arch@freebsd.org X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Jan 2015 02:37:17 -0000 On Sun, Jan 04, 2015 at 07:20:27PM +0100, Jilles Tjoelker wrote: > In libc, define the below wait3(). All code in libc that allows libthr to > interpose wait3() and in libthr that interposes wait3() can then go > away. > > pid_t > wait3(int *istat, int options, struct rusage *rup) > { > > return (((pid_t (*)(pid_t, int *, int, struct rusage *)) > __libc_interposing[INTERPOS_wait4])(WAIT_ANY, istat, options, rup)); > } > > This change is also possible for wait() and waitpid(), but not for e.g. > sigwait() which has different [EINTR] behaviour than sigtimedwait(). > Ok, I did the pass over all such thin wrappers. The list is accept <- calls accept4 now creat \ open <- openat pause <- there were actual bugs, pause called _sigprocmask and _sigsuspend raise <- I converted it to threaded kill sleep <- this one is tricky, but seems to work usleep wait wait3 waitpid Patch is below. > The changes look good to me. Next natural question is about the __error calls through PLT in the .cerror asm. Before the work was committed, libthr interposed __error, which was required for correct operation. Now, we need __error exported, but do we need support its interposing ? This is an implementation detail for errno, I do not see any loss from not allowing to override errno location. diff --git a/lib/libc/compat-43/Symbol.map b/lib/libc/compat-43/Symbol.map index 2827705..bd49f99 100644 --- a/lib/libc/compat-43/Symbol.map +++ b/lib/libc/compat-43/Symbol.map @@ -28,5 +28,4 @@ FBSD_1.2 { FBSDprivate_1.0 { __creat; _creat; - __libc_creat; }; diff --git a/lib/libc/compat-43/creat.c b/lib/libc/compat-43/creat.c index 5ee4531..4545482 100644 --- a/lib/libc/compat-43/creat.c +++ b/lib/libc/compat-43/creat.c @@ -38,21 +38,16 @@ __FBSDID("$FreeBSD$"); #include "un-namespace.h" #include "libc_private.h" -__weak_reference(__libc_creat, __creat); -__weak_reference(__libc_creat, _creat); +__weak_reference(__creat, creat); +__weak_reference(__creat, _creat); #pragma weak creat int -creat(const char *path, mode_t mode) +__creat(const char *path, mode_t mode) { - return (((int (*)(const char *, mode_t)) - __libc_interposing[INTERPOS_creat])(path, mode)); + return (((int (*)(int, const char *, int, ...)) + __libc_interposing[INTERPOS_openat])(AT_FDCWD, path, O_WRONLY | + O_CREAT | O_TRUNC, mode)); } -int -__libc_creat(const char *path, mode_t mode) -{ - - return(__sys_open(path, O_WRONLY | O_CREAT | O_TRUNC, mode)); -} diff --git a/lib/libc/gen/Symbol.map b/lib/libc/gen/Symbol.map index e01a897..ee4d619 100644 --- a/lib/libc/gen/Symbol.map +++ b/lib/libc/gen/Symbol.map @@ -533,14 +533,7 @@ FBSDprivate_1.0 { _libc_sem_post_compat; _libc_sem_getvalue_compat; - __libc_pause; - __libc_raise; - __libc_sleep; __libc_tcdrain; - __libc_usleep; - __libc_wait; - __libc_wait3; - __libc_waitpid; __elf_aux_vector; __pthread_map_stacks_exec; diff --git a/lib/libc/gen/pause.c b/lib/libc/gen/pause.c index 97fbb01..ef48c1c 100644 --- a/lib/libc/gen/pause.c +++ b/lib/libc/gen/pause.c @@ -33,10 +33,8 @@ static char sccsid[] = "@(#)pause.c 8.1 (Berkeley) 6/4/93"; #include __FBSDID("$FreeBSD$"); -#include "namespace.h" #include #include -#include "un-namespace.h" #include "libc_private.h" @@ -44,23 +42,14 @@ __FBSDID("$FreeBSD$"); * Backwards compatible pause. */ int -__libc_pause(void) +__pause(void) { sigset_t oset; - if (_sigprocmask(SIG_BLOCK, NULL, &oset) == -1) + if (sigprocmask(SIG_BLOCK, NULL, &oset) == -1) return (-1); - return (_sigsuspend(&oset)); + return (sigsuspend(&oset)); } -#pragma weak pause -int -pause(void) -{ - - return (((int (*)(void)) - __libc_interposing[INTERPOS_pause])()); -} - -__weak_reference(__libc_pause, __pause); -__weak_reference(__libc_pause, _pause); +__weak_reference(__pause, pause); +__weak_reference(__pause, _pause); diff --git a/lib/libc/gen/raise.c b/lib/libc/gen/raise.c index d605639..994fea5 100644 --- a/lib/libc/gen/raise.c +++ b/lib/libc/gen/raise.c @@ -38,21 +38,15 @@ __FBSDID("$FreeBSD$"); #include "libc_private.h" -__weak_reference(__libc_raise, __raise); -__weak_reference(__libc_raise, _raise); +__weak_reference(__raise, raise); +__weak_reference(__raise, _raise); -#pragma weak raise int -raise(int s) +__raise(int s) { + long id; - return (((int (*)(int)) - __libc_interposing[INTERPOS_raise])(s)); -} - -int -__libc_raise(int s) -{ - - return (kill(getpid(), s)); + if (__sys_thr_self(&id) == -1) + return (-1); + return (__sys_thr_kill(id, s)); } diff --git a/lib/libc/gen/sleep.c b/lib/libc/gen/sleep.c index f558e36..6bb4ecd 100644 --- a/lib/libc/gen/sleep.c +++ b/lib/libc/gen/sleep.c @@ -42,17 +42,8 @@ __FBSDID("$FreeBSD$"); #include "libc_private.h" -#pragma weak sleep unsigned int -sleep(unsigned int seconds) -{ - - return (((unsigned int (*)(unsigned int)) - __libc_interposing[INTERPOS_sleep])(seconds)); -} - -unsigned int -__libc_sleep(unsigned int seconds) +__sleep(unsigned int seconds) { struct timespec time_to_sleep; struct timespec time_remaining; @@ -62,17 +53,19 @@ __libc_sleep(unsigned int seconds) * the maximum value for a time_t is >= INT_MAX. */ if (seconds > INT_MAX) - return (seconds - INT_MAX + __libc_sleep(INT_MAX)); + return (seconds - INT_MAX + __sleep(INT_MAX)); time_to_sleep.tv_sec = seconds; time_to_sleep.tv_nsec = 0; - if (_nanosleep(&time_to_sleep, &time_remaining) != -1) + if (((int (*)(const struct timespec *, struct timespec *)) + __libc_interposing[INTERPOS_nanosleep])( + &time_to_sleep, &time_remaining) != -1) return (0); if (errno != EINTR) return (seconds); /* best guess */ return (time_remaining.tv_sec + - (time_remaining.tv_nsec != 0)); /* round up */ + (time_remaining.tv_nsec != 0)); /* round up */ } -__weak_reference(__libc_sleep, __sleep); -__weak_reference(__libc_sleep, _sleep); +__weak_reference(__sleep, sleep); +__weak_reference(__sleep, _sleep); diff --git a/lib/libc/gen/usleep.c b/lib/libc/gen/usleep.c index 1d58b98..7c35f6c 100644 --- a/lib/libc/gen/usleep.c +++ b/lib/libc/gen/usleep.c @@ -40,24 +40,16 @@ __FBSDID("$FreeBSD$"); #include "libc_private.h" -#pragma weak usleep int -usleep(useconds_t useconds) -{ - - return (((int (*)(useconds_t)) - __libc_interposing[INTERPOS_usleep])(useconds)); -} - -int -__libc_usleep(useconds_t useconds) +__usleep(useconds_t useconds) { struct timespec time_to_sleep; time_to_sleep.tv_nsec = (useconds % 1000000) * 1000; time_to_sleep.tv_sec = useconds / 1000000; - return (_nanosleep(&time_to_sleep, NULL)); + return (((int (*)(const struct timespec *, struct timespec *)) + __libc_interposing[INTERPOS_nanosleep])(&time_to_sleep, NULL)); } -__weak_reference(__libc_usleep, __usleep); -__weak_reference(__libc_usleep, _usleep); +__weak_reference(__usleep, usleep); +__weak_reference(__usleep, _usleep); diff --git a/lib/libc/gen/wait.c b/lib/libc/gen/wait.c index dc1351d..46a3fdd 100644 --- a/lib/libc/gen/wait.c +++ b/lib/libc/gen/wait.c @@ -42,21 +42,13 @@ __FBSDID("$FreeBSD$"); #include "libc_private.h" -#pragma weak wait pid_t -wait(int *istat) +__wait(int *istat) { - return (((pid_t (*)(int *)) - __libc_interposing[INTERPOS_wait])(istat)); + return (((pid_t (*)(pid_t, int *, int, struct rusage *)) + __libc_interposing[INTERPOS_wait4])(WAIT_ANY, istat, 0, NULL)); } -pid_t -__libc_wait(int *istat) -{ - - return (__sys_wait4(WAIT_ANY, istat, 0, NULL)); -} - -__weak_reference(__libc_wait, __wait); -__weak_reference(__libc_wait, _wait); +__weak_reference(__wait, wait); +__weak_reference(__wait, _wait); diff --git a/lib/libc/gen/wait3.c b/lib/libc/gen/wait3.c index 2e116be..965effe 100644 --- a/lib/libc/gen/wait3.c +++ b/lib/libc/gen/wait3.c @@ -42,20 +42,12 @@ __FBSDID("$FreeBSD$"); #include "libc_private.h" -#pragma weak wait3 pid_t -wait3(int *istat, int options, struct rusage *rup) +__wait3(int *istat, int options, struct rusage *rup) { - return (((pid_t (*)(int *, int, struct rusage *)) - __libc_interposing[INTERPOS_wait3])(istat, options, rup)); + return (((pid_t (*)(pid_t, int *, int, struct rusage *)) + __libc_interposing[INTERPOS_wait4])(WAIT_ANY, istat, options, rup)); } -__weak_reference(__libc_wait3, __wait3); - -pid_t -__libc_wait3(int *istat, int options, struct rusage *rup) -{ - - return (__sys_wait4(WAIT_ANY, istat, options, rup)); -} +__weak_reference(__wait3, wait3); diff --git a/lib/libc/gen/waitpid.c b/lib/libc/gen/waitpid.c index 27e920f..5177591 100644 --- a/lib/libc/gen/waitpid.c +++ b/lib/libc/gen/waitpid.c @@ -42,21 +42,13 @@ __FBSDID("$FreeBSD$"); #include "libc_private.h" -#pragma weak waitpid pid_t -waitpid(pid_t pid, int *istat, int options) +__waitpid(pid_t pid, int *istat, int options) { - return (((pid_t (*)(pid_t, int *, int)) - __libc_interposing[INTERPOS_waitpid])(pid, istat, options)); + return (((pid_t (*)(pid_t, int *, int, struct rusage *)) + __libc_interposing[INTERPOS_wait4])(pid, istat, options, NULL)); } -pid_t -__libc_waitpid(pid_t pid, int *istat, int options) -{ - - return (__sys_wait4(pid, istat, options, NULL)); -} - -__weak_reference(__libc_waitpid, __waitpid); -__weak_reference(__libc_waitpid, _waitpid); +__weak_reference(__waitpid, waitpid); +__weak_reference(__waitpid, _waitpid); diff --git a/lib/libc/include/libc_private.h b/lib/libc/include/libc_private.h index ea1a97b..b698553 100644 --- a/lib/libc/include/libc_private.h +++ b/lib/libc/include/libc_private.h @@ -182,22 +182,18 @@ interpos_func_t *__libc_interposing_slot(int interposno); extern interpos_func_t __libc_interposing[] __hidden; enum { - INTERPOS_accept, INTERPOS_accept4, INTERPOS_aio_suspend, INTERPOS_close, INTERPOS_connect, - INTERPOS_creat, INTERPOS_fcntl, INTERPOS_fsync, INTERPOS_fork, INTERPOS_msync, INTERPOS_nanosleep, - INTERPOS_open, INTERPOS_openat, INTERPOS_poll, INTERPOS_pselect, - INTERPOS_raise, INTERPOS_recvfrom, INTERPOS_recvmsg, INTERPOS_select, @@ -212,16 +208,10 @@ enum { INTERPOS_sigwaitinfo, INTERPOS_swapcontext, INTERPOS_system, - INTERPOS_sleep, INTERPOS_tcdrain, - INTERPOS_usleep, - INTERPOS_pause, INTERPOS_read, INTERPOS_readv, - INTERPOS_wait, - INTERPOS_wait3, INTERPOS_wait4, - INTERPOS_waitpid, INTERPOS_write, INTERPOS_writev, INTERPOS__pthread_mutex_init_calloc_cb, @@ -353,23 +343,17 @@ int __sys_sigwait(const __sigset_t *, int *); int __sys_sigwaitinfo(const __sigset_t *, struct __siginfo *); int __sys_swapcontext(struct __ucontext *, const struct __ucontext *); +int __sys_thr_kill(long, int); +int __sys_thr_self(long *); int __sys_truncate(const char *, __off_t); __pid_t __sys_wait4(__pid_t, int *, int, struct rusage *); __ssize_t __sys_write(int, const void *, __size_t); __ssize_t __sys_writev(int, const struct iovec *, int); -int __libc_creat(const char *path, __mode_t mode); -int __libc_pause(void); -int __libc_raise(int); int __libc_sigwait(const __sigset_t * __restrict, int * restrict sig); int __libc_system(const char *); -unsigned int __libc_sleep(unsigned int); int __libc_tcdrain(int); -int __libc_usleep(__useconds_t); -__pid_t __libc_wait(int *); -__pid_t __libc_wait3(int *, int, struct rusage *); -__pid_t __libc_waitpid(__pid_t, int *, int); int __fcntl_compat(int fd, int cmd, ...); /* execve() with PATH processing to implement posix_spawnp() */ diff --git a/lib/libc/sys/accept.c b/lib/libc/sys/accept.c index 38e32f2..8fb945f 100644 --- a/lib/libc/sys/accept.c +++ b/lib/libc/sys/accept.c @@ -38,13 +38,13 @@ __FBSDID("$FreeBSD$"); #include #include "libc_private.h" -__weak_reference(__sys_accept, __accept); +__weak_reference(__accept, accept); #pragma weak accept int -accept(int s, struct sockaddr *addr, socklen_t *addrlen) +__accept(int s, struct sockaddr *addr, socklen_t *addrlen) { - return (((int (*)(int, struct sockaddr *, socklen_t *)) - __libc_interposing[INTERPOS_accept])(s, addr, addrlen)); + return (((int (*)(int, struct sockaddr *, socklen_t *, int)) + __libc_interposing[INTERPOS_accept4])(s, addr, addrlen, 0)); } diff --git a/lib/libc/sys/interposing_table.c b/lib/libc/sys/interposing_table.c index 9987bf0..10d56fd 100644 --- a/lib/libc/sys/interposing_table.c +++ b/lib/libc/sys/interposing_table.c @@ -39,22 +39,18 @@ __FBSDID("$FreeBSD$"); #define SLOT(a, b) \ [INTERPOS_##a] = (interpos_func_t)b interpos_func_t __libc_interposing[INTERPOS_MAX] = { - SLOT(accept, __sys_accept), SLOT(accept4, __sys_accept4), SLOT(aio_suspend, __sys_aio_suspend), SLOT(close, __sys_close), SLOT(connect, __sys_connect), - SLOT(creat, __libc_creat), SLOT(fcntl, __fcntl_compat), SLOT(fsync, __sys_fsync), SLOT(fork, __sys_fork), SLOT(msync, __sys_msync), SLOT(nanosleep, __sys_nanosleep), - SLOT(open, __sys_open), SLOT(openat, __sys_openat), SLOT(poll, __sys_poll), SLOT(pselect, __sys_pselect), - SLOT(raise, __libc_raise), SLOT(read, __sys_read), SLOT(readv, __sys_readv), SLOT(recvfrom, __sys_recvfrom), @@ -71,14 +67,8 @@ interpos_func_t __libc_interposing[INTERPOS_MAX] = { SLOT(sigwaitinfo, __sys_sigwaitinfo), SLOT(swapcontext, __sys_swapcontext), SLOT(system, __libc_system), - SLOT(sleep, __libc_sleep), SLOT(tcdrain, __libc_tcdrain), - SLOT(usleep, __libc_usleep), - SLOT(pause, __libc_pause), - SLOT(wait, __libc_wait), - SLOT(wait3, __libc_wait3), SLOT(wait4, __sys_wait4), - SLOT(waitpid, __libc_waitpid), SLOT(write, __sys_write), SLOT(writev, __sys_writev), SLOT(_pthread_mutex_init_calloc_cb, _pthread_mutex_init_calloc_cb_stub), diff --git a/lib/libc/sys/open.c b/lib/libc/sys/open.c index 57c0324..e0273c6 100644 --- a/lib/libc/sys/open.c +++ b/lib/libc/sys/open.c @@ -54,6 +54,6 @@ open(const char *path, int flags, ...) } else { mode = 0; } - return (((int (*)(const char *, int, ...)) - __libc_interposing[INTERPOS_open])(path, flags, mode)); + return (((int (*)(int, const char *, int, ...)) + __libc_interposing[INTERPOS_openat])(AT_FDCWD, path, flags, mode)); } diff --git a/lib/libthr/thread/thr_private.h b/lib/libthr/thread/thr_private.h index 0c16b2a..d62de98 100644 --- a/lib/libthr/thread/thr_private.h +++ b/lib/libthr/thread/thr_private.h @@ -917,8 +917,6 @@ void _thr_stack_fix_protection(struct pthread *thrd); int *__error_threaded(void) __hidden; void __thr_interpose_libc(void) __hidden; pid_t __thr_fork(void); -int __thr_pause(void) __hidden; -int __thr_raise(int sig); int __thr_setcontext(const ucontext_t *ucp); int __thr_sigaction(int sig, const struct sigaction *act, struct sigaction *oact) __hidden; diff --git a/lib/libthr/thread/thr_sig.c b/lib/libthr/thread/thr_sig.c index 1ec64f0..7cd0f75 100644 --- a/lib/libthr/thread/thr_sig.c +++ b/lib/libthr/thread/thr_sig.c @@ -515,23 +515,6 @@ _thr_signal_deinit(void) } int -__thr_pause(void) -{ - sigset_t oset; - - if (_sigprocmask(SIG_BLOCK, NULL, &oset) == -1) - return (-1); - return (__thr_sigsuspend(&oset)); -} - -int -__thr_raise(int sig) -{ - - return (_thr_send_sig(_get_curthread(), sig)); -} - -int __thr_sigaction(int sig, const struct sigaction *act, struct sigaction *oact) { struct sigaction newact, oldact, oldact2; diff --git a/lib/libthr/thread/thr_syscalls.c b/lib/libthr/thread/thr_syscalls.c index a4fe7e8..d45ecd2 100644 --- a/lib/libthr/thread/thr_syscalls.c +++ b/lib/libthr/thread/thr_syscalls.c @@ -104,24 +104,6 @@ extern int __fcntl_compat(int, int, ...); * If thread is canceled, no socket is created. */ static int -__thr_accept(int s, struct sockaddr *addr, socklen_t *addrlen) -{ - struct pthread *curthread; - int ret; - - curthread = _get_curthread(); - _thr_cancel_enter(curthread); - ret = __sys_accept(s, addr, addrlen); - _thr_cancel_leave(curthread, ret == -1); - - return (ret); -} - -/* - * Cancellation behavior: - * If thread is canceled, no socket is created. - */ -static int __thr_accept4(int s, struct sockaddr *addr, socklen_t *addrlen, int flags) { struct pthread *curthread; @@ -189,25 +171,6 @@ __thr_connect(int fd, const struct sockaddr *name, socklen_t namelen) return (ret); } - -/* - * Cancellation behavior: - * If thread is canceled, file is not created. - */ -static int -__thr_creat(const char *path, mode_t mode) -{ - struct pthread *curthread; - int ret; - - curthread = _get_curthread(); - _thr_cancel_enter(curthread); - ret = __libc_creat(path, mode); - _thr_cancel_leave(curthread, ret == -1); - - return (ret); -} - /* * Cancellation behavior: * According to specification, only F_SETLKW is a cancellation point. @@ -300,35 +263,6 @@ __thr_nanosleep(const struct timespec *time_to_sleep, * If the thread is canceled, file is not opened. */ static int -__thr_open(const char *path, int flags,...) -{ - struct pthread *curthread; - int mode, ret; - va_list ap; - - /* Check if the file is being created: */ - if ((flags & O_CREAT) != 0) { - /* Get the creation mode: */ - va_start(ap, flags); - mode = va_arg(ap, int); - va_end(ap); - } else { - mode = 0; - } - - curthread = _get_curthread(); - _thr_cancel_enter(curthread); - ret = __sys_open(path, flags, mode); - _thr_cancel_leave(curthread, ret == -1); - - return (ret); -} - -/* - * Cancellation behavior: - * If the thread is canceled, file is not opened. - */ -static int __thr_openat(int fd, const char *path, int flags, ...) { struct pthread *curthread; @@ -523,19 +457,6 @@ __thr_sendto(int s, const void *m, size_t l, int f, const struct sockaddr *t, return (ret); } -static unsigned int -__thr_sleep(unsigned int seconds) -{ - struct pthread *curthread; - unsigned int ret; - - curthread = _get_curthread(); - _thr_cancel_enter(curthread); - ret = __libc_sleep(seconds); - _thr_cancel_leave(curthread, 1); - return (ret); -} - static int __thr_system(const char *string) { @@ -567,55 +488,6 @@ __thr_tcdrain(int fd) return (ret); } -static int -__thr_usleep(useconds_t useconds) -{ - struct pthread *curthread; - int ret; - - curthread = _get_curthread(); - _thr_cancel_enter(curthread); - ret = __libc_usleep(useconds); - _thr_cancel_leave(curthread, 1); - return (ret); -} - -/* - * Cancellation behavior: - * Thread may be canceled at start, but if the system call returns - * a child pid, the thread is not canceled. - */ -static pid_t -__thr_wait(int *istat) -{ - struct pthread *curthread; - pid_t ret; - - curthread = _get_curthread(); - _thr_cancel_enter(curthread); - ret = __libc_wait(istat); - _thr_cancel_leave(curthread, ret <= 0); - return (ret); -} - -/* - * Cancellation behavior: - * Thread may be canceled at start, but if the system call returns - * a child pid, the thread is not canceled. - */ -static pid_t -__thr_wait3(int *status, int options, struct rusage *rusage) -{ - struct pthread *curthread; - pid_t ret; - - curthread = _get_curthread(); - _thr_cancel_enter(curthread); - ret = __libc_wait3(status, options, rusage); - _thr_cancel_leave(curthread, ret <= 0); - return (ret); -} - /* * Cancellation behavior: * Thread may be canceled at start, but if the system call returns @@ -636,24 +508,6 @@ __thr_wait4(pid_t pid, int *status, int options, struct rusage *rusage) /* * Cancellation behavior: - * Thread may be canceled at start, but if the system call returns - * a child pid, the thread is not canceled. - */ -static pid_t -__thr_waitpid(pid_t wpid, int *status, int options) -{ - struct pthread *curthread; - pid_t ret; - - curthread = _get_curthread(); - _thr_cancel_enter(curthread); - ret = __libc_waitpid(wpid, status, options); - _thr_cancel_leave(curthread, ret <= 0); - return (ret); -} - -/* - * Cancellation behavior: * Thread may be canceled at start, but if the thread wrote some data, * it is not canceled. */ @@ -696,22 +550,18 @@ __thr_interpose_libc(void) #define SLOT(name) \ *(__libc_interposing_slot(INTERPOS_##name)) = \ (interpos_func_t)__thr_##name; - SLOT(accept); SLOT(accept4); SLOT(aio_suspend); SLOT(close); SLOT(connect); - SLOT(creat); SLOT(fcntl); SLOT(fsync); SLOT(fork); SLOT(msync); SLOT(nanosleep); - SLOT(open); SLOT(openat); SLOT(poll); SLOT(pselect); - SLOT(raise); SLOT(read); SLOT(readv); SLOT(recvfrom); @@ -728,14 +578,8 @@ __thr_interpose_libc(void) SLOT(sigwaitinfo); SLOT(swapcontext); SLOT(system); - SLOT(sleep); SLOT(tcdrain); - SLOT(usleep); - SLOT(pause); - SLOT(wait); - SLOT(wait3); SLOT(wait4); - SLOT(waitpid); SLOT(write); SLOT(writev); #undef SLOT From owner-freebsd-threads@FreeBSD.ORG Tue Jan 6 20:50:49 2015 Return-Path: Delivered-To: threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id A569C92E; Tue, 6 Jan 2015 20:50:49 +0000 (UTC) Received: from mx1.stack.nl (relay02.stack.nl [IPv6:2001:610:1108:5010::104]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client CN "mailhost.stack.nl", Issuer "CA Cert Signing Authority" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 668EA7F3; Tue, 6 Jan 2015 20:50:49 +0000 (UTC) Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131]) by mx1.stack.nl (Postfix) with ESMTP id AA1403592EF; Tue, 6 Jan 2015 21:50:46 +0100 (CET) Received: by snail.stack.nl (Postfix, from userid 1677) id 97EAC28494; Tue, 6 Jan 2015 21:50:46 +0100 (CET) Date: Tue, 6 Jan 2015 21:50:46 +0100 From: Jilles Tjoelker To: Konstantin Belousov Subject: Re: Fixing dlopen("libpthread.so") Message-ID: <20150106205046.GA24971@stack.nl> References: <20141226165337.GJ1754@kib.kiev.ua> <20150103212837.GC46373@stack.nl> <20150104114600.GC42409@kib.kiev.ua> <20150104182026.GA61250@stack.nl> <20150105023708.GD42409@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150105023708.GD42409@kib.kiev.ua> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: threads@freebsd.org, arch@freebsd.org X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 Jan 2015 20:50:49 -0000 On Mon, Jan 05, 2015 at 04:37:08AM +0200, Konstantin Belousov wrote: > On Sun, Jan 04, 2015 at 07:20:27PM +0100, Jilles Tjoelker wrote: > > In libc, define the below wait3(). All code in libc that allows libthr to > > interpose wait3() and in libthr that interposes wait3() can then go > > away. > > pid_t > > wait3(int *istat, int options, struct rusage *rup) > > { > > > > return (((pid_t (*)(pid_t, int *, int, struct rusage *)) > > __libc_interposing[INTERPOS_wait4])(WAIT_ANY, istat, options, rup)); > > } > > This change is also possible for wait() and waitpid(), but not for e.g. > > sigwait() which has different [EINTR] behaviour than sigtimedwait(). > Ok, I did the pass over all such thin wrappers. The list is > accept <- calls accept4 now This is wrong since accept() copies various properties from the listening to the new socket, which accept4() never does. (Note that Linux accept() is equivalent to accept4() with 0 flags, so the linuxolator can use such a shortcut.) > creat \ > open <- openat > pause <- there were actual bugs, pause called _sigprocmask and _sigsuspend > raise <- I converted it to threaded kill > sleep <- this one is tricky, but seems to work > usleep > wait > wait3 > waitpid These look OK. > > The changes look good to me. > Next natural question is about the __error calls through PLT in the > .cerror asm. Before the work was committed, libthr interposed __error, > which was required for correct operation. Now, we need __error exported, > but do we need support its interposing ? This is an implementation > detail for errno, I do not see any loss from not allowing to override > errno location. Indeed, there is no need to allow interposing __error (as with many libc-internal calls to its exported symbols). Additionally, the current namespace.h mechanism could be used to redirect __error calls from C code. Glibc uses macro trickery with the asm labels compiler feature, making libc code see things like int *__error(void) asm("__hidden__error"); and defining the hidden alias somewhere. This also works for compiler-generated calls like to memcpy(). I'm not sure whether it is a good idea to add this extra trickery and depend on the compiler feature. Glibc also has build-time checks that only specifically listed symbols are referenced via the PLT. -- Jilles Tjoelker From owner-freebsd-threads@FreeBSD.ORG Wed Jan 7 03:29:55 2015 Return-Path: Delivered-To: threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 5C0DE824; Wed, 7 Jan 2015 03:29:55 +0000 (UTC) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id BB36E1839; Wed, 7 Jan 2015 03:29:54 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.9/8.14.9) with ESMTP id t073Tgua058112 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 7 Jan 2015 05:29:42 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.9.2 kib.kiev.ua t073Tgua058112 Received: (from kostik@localhost) by tom.home (8.14.9/8.14.9/Submit) id t073TfKw058111; Wed, 7 Jan 2015 05:29:41 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Wed, 7 Jan 2015 05:29:41 +0200 From: Konstantin Belousov To: Jilles Tjoelker Subject: Re: Fixing dlopen("libpthread.so") Message-ID: <20150107032941.GJ42409@kib.kiev.ua> References: <20141226165337.GJ1754@kib.kiev.ua> <20150103212837.GC46373@stack.nl> <20150104114600.GC42409@kib.kiev.ua> <20150104182026.GA61250@stack.nl> <20150105023708.GD42409@kib.kiev.ua> <20150106205046.GA24971@stack.nl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150106205046.GA24971@stack.nl> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.0 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on tom.home Cc: threads@freebsd.org, arch@freebsd.org X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Jan 2015 03:29:55 -0000 On Tue, Jan 06, 2015 at 09:50:46PM +0100, Jilles Tjoelker wrote: > On Mon, Jan 05, 2015 at 04:37:08AM +0200, Konstantin Belousov wrote: > > accept <- calls accept4 now > > This is wrong since accept() copies various properties from the > listening to the new socket, which accept4() never does. (Note that > Linux accept() is equivalent to accept4() with 0 flags, so the > linuxolator can use such a shortcut.) Reverted this part. So it is not possible to emulate accept(2) with accept4(2) ? Does it make sense to add flag(s) to accept4(2) to copy nonblock and async from the listen socket ? > > > creat \ > > open <- openat > > pause <- there were actual bugs, pause called _sigprocmask and _sigsuspend > > raise <- I converted it to threaded kill > > sleep <- this one is tricky, but seems to work > > usleep > > wait > > wait3 > > waitpid > > These look OK. Updated patch is at the end. > > > > The changes look good to me. > > Next natural question is about the __error calls through PLT in the > > .cerror asm. Before the work was committed, libthr interposed __error, > > which was required for correct operation. Now, we need __error exported, > > but do we need support its interposing ? This is an implementation > > detail for errno, I do not see any loss from not allowing to override > > errno location. > > Indeed, there is no need to allow interposing __error (as with many > libc-internal calls to its exported symbols). Additionally, the current > namespace.h mechanism could be used to redirect __error calls from C > code. > > Glibc uses macro trickery with the asm labels compiler feature, making > libc code see things like > int *__error(void) asm("__hidden__error"); > and defining the hidden alias somewhere. This also works for > compiler-generated calls like to memcpy(). I'm not sure whether it is a > good idea to add this extra trickery and depend on the compiler feature. I might look at this after the current change is committed. > Glibc also has build-time checks that only specifically listed symbols > are referenced via the PLT. As well as e.g. libunwind. This would be a nice addition to currently empty set of ABI stability tests. diff --git a/lib/libc/compat-43/Symbol.map b/lib/libc/compat-43/Symbol.map index 2827705..bd49f99 100644 --- a/lib/libc/compat-43/Symbol.map +++ b/lib/libc/compat-43/Symbol.map @@ -28,5 +28,4 @@ FBSD_1.2 { FBSDprivate_1.0 { __creat; _creat; - __libc_creat; }; diff --git a/lib/libc/compat-43/creat.c b/lib/libc/compat-43/creat.c index 5ee4531..4545482 100644 --- a/lib/libc/compat-43/creat.c +++ b/lib/libc/compat-43/creat.c @@ -38,21 +38,16 @@ __FBSDID("$FreeBSD$"); #include "un-namespace.h" #include "libc_private.h" -__weak_reference(__libc_creat, __creat); -__weak_reference(__libc_creat, _creat); +__weak_reference(__creat, creat); +__weak_reference(__creat, _creat); #pragma weak creat int -creat(const char *path, mode_t mode) +__creat(const char *path, mode_t mode) { - return (((int (*)(const char *, mode_t)) - __libc_interposing[INTERPOS_creat])(path, mode)); + return (((int (*)(int, const char *, int, ...)) + __libc_interposing[INTERPOS_openat])(AT_FDCWD, path, O_WRONLY | + O_CREAT | O_TRUNC, mode)); } -int -__libc_creat(const char *path, mode_t mode) -{ - - return(__sys_open(path, O_WRONLY | O_CREAT | O_TRUNC, mode)); -} diff --git a/lib/libc/gen/Symbol.map b/lib/libc/gen/Symbol.map index e01a897..ee4d619 100644 --- a/lib/libc/gen/Symbol.map +++ b/lib/libc/gen/Symbol.map @@ -533,14 +533,7 @@ FBSDprivate_1.0 { _libc_sem_post_compat; _libc_sem_getvalue_compat; - __libc_pause; - __libc_raise; - __libc_sleep; __libc_tcdrain; - __libc_usleep; - __libc_wait; - __libc_wait3; - __libc_waitpid; __elf_aux_vector; __pthread_map_stacks_exec; diff --git a/lib/libc/gen/pause.c b/lib/libc/gen/pause.c index 97fbb01..ef48c1c 100644 --- a/lib/libc/gen/pause.c +++ b/lib/libc/gen/pause.c @@ -33,10 +33,8 @@ static char sccsid[] = "@(#)pause.c 8.1 (Berkeley) 6/4/93"; #include __FBSDID("$FreeBSD$"); -#include "namespace.h" #include #include -#include "un-namespace.h" #include "libc_private.h" @@ -44,23 +42,14 @@ __FBSDID("$FreeBSD$"); * Backwards compatible pause. */ int -__libc_pause(void) +__pause(void) { sigset_t oset; - if (_sigprocmask(SIG_BLOCK, NULL, &oset) == -1) + if (sigprocmask(SIG_BLOCK, NULL, &oset) == -1) return (-1); - return (_sigsuspend(&oset)); + return (sigsuspend(&oset)); } -#pragma weak pause -int -pause(void) -{ - - return (((int (*)(void)) - __libc_interposing[INTERPOS_pause])()); -} - -__weak_reference(__libc_pause, __pause); -__weak_reference(__libc_pause, _pause); +__weak_reference(__pause, pause); +__weak_reference(__pause, _pause); diff --git a/lib/libc/gen/raise.c b/lib/libc/gen/raise.c index d605639..994fea5 100644 --- a/lib/libc/gen/raise.c +++ b/lib/libc/gen/raise.c @@ -38,21 +38,15 @@ __FBSDID("$FreeBSD$"); #include "libc_private.h" -__weak_reference(__libc_raise, __raise); -__weak_reference(__libc_raise, _raise); +__weak_reference(__raise, raise); +__weak_reference(__raise, _raise); -#pragma weak raise int -raise(int s) +__raise(int s) { + long id; - return (((int (*)(int)) - __libc_interposing[INTERPOS_raise])(s)); -} - -int -__libc_raise(int s) -{ - - return (kill(getpid(), s)); + if (__sys_thr_self(&id) == -1) + return (-1); + return (__sys_thr_kill(id, s)); } diff --git a/lib/libc/gen/sleep.c b/lib/libc/gen/sleep.c index f558e36..6bb4ecd 100644 --- a/lib/libc/gen/sleep.c +++ b/lib/libc/gen/sleep.c @@ -42,17 +42,8 @@ __FBSDID("$FreeBSD$"); #include "libc_private.h" -#pragma weak sleep unsigned int -sleep(unsigned int seconds) -{ - - return (((unsigned int (*)(unsigned int)) - __libc_interposing[INTERPOS_sleep])(seconds)); -} - -unsigned int -__libc_sleep(unsigned int seconds) +__sleep(unsigned int seconds) { struct timespec time_to_sleep; struct timespec time_remaining; @@ -62,17 +53,19 @@ __libc_sleep(unsigned int seconds) * the maximum value for a time_t is >= INT_MAX. */ if (seconds > INT_MAX) - return (seconds - INT_MAX + __libc_sleep(INT_MAX)); + return (seconds - INT_MAX + __sleep(INT_MAX)); time_to_sleep.tv_sec = seconds; time_to_sleep.tv_nsec = 0; - if (_nanosleep(&time_to_sleep, &time_remaining) != -1) + if (((int (*)(const struct timespec *, struct timespec *)) + __libc_interposing[INTERPOS_nanosleep])( + &time_to_sleep, &time_remaining) != -1) return (0); if (errno != EINTR) return (seconds); /* best guess */ return (time_remaining.tv_sec + - (time_remaining.tv_nsec != 0)); /* round up */ + (time_remaining.tv_nsec != 0)); /* round up */ } -__weak_reference(__libc_sleep, __sleep); -__weak_reference(__libc_sleep, _sleep); +__weak_reference(__sleep, sleep); +__weak_reference(__sleep, _sleep); diff --git a/lib/libc/gen/usleep.c b/lib/libc/gen/usleep.c index 1d58b98..7c35f6c 100644 --- a/lib/libc/gen/usleep.c +++ b/lib/libc/gen/usleep.c @@ -40,24 +40,16 @@ __FBSDID("$FreeBSD$"); #include "libc_private.h" -#pragma weak usleep int -usleep(useconds_t useconds) -{ - - return (((int (*)(useconds_t)) - __libc_interposing[INTERPOS_usleep])(useconds)); -} - -int -__libc_usleep(useconds_t useconds) +__usleep(useconds_t useconds) { struct timespec time_to_sleep; time_to_sleep.tv_nsec = (useconds % 1000000) * 1000; time_to_sleep.tv_sec = useconds / 1000000; - return (_nanosleep(&time_to_sleep, NULL)); + return (((int (*)(const struct timespec *, struct timespec *)) + __libc_interposing[INTERPOS_nanosleep])(&time_to_sleep, NULL)); } -__weak_reference(__libc_usleep, __usleep); -__weak_reference(__libc_usleep, _usleep); +__weak_reference(__usleep, usleep); +__weak_reference(__usleep, _usleep); diff --git a/lib/libc/gen/wait.c b/lib/libc/gen/wait.c index dc1351d..46a3fdd 100644 --- a/lib/libc/gen/wait.c +++ b/lib/libc/gen/wait.c @@ -42,21 +42,13 @@ __FBSDID("$FreeBSD$"); #include "libc_private.h" -#pragma weak wait pid_t -wait(int *istat) +__wait(int *istat) { - return (((pid_t (*)(int *)) - __libc_interposing[INTERPOS_wait])(istat)); + return (((pid_t (*)(pid_t, int *, int, struct rusage *)) + __libc_interposing[INTERPOS_wait4])(WAIT_ANY, istat, 0, NULL)); } -pid_t -__libc_wait(int *istat) -{ - - return (__sys_wait4(WAIT_ANY, istat, 0, NULL)); -} - -__weak_reference(__libc_wait, __wait); -__weak_reference(__libc_wait, _wait); +__weak_reference(__wait, wait); +__weak_reference(__wait, _wait); diff --git a/lib/libc/gen/wait3.c b/lib/libc/gen/wait3.c index 2e116be..965effe 100644 --- a/lib/libc/gen/wait3.c +++ b/lib/libc/gen/wait3.c @@ -42,20 +42,12 @@ __FBSDID("$FreeBSD$"); #include "libc_private.h" -#pragma weak wait3 pid_t -wait3(int *istat, int options, struct rusage *rup) +__wait3(int *istat, int options, struct rusage *rup) { - return (((pid_t (*)(int *, int, struct rusage *)) - __libc_interposing[INTERPOS_wait3])(istat, options, rup)); + return (((pid_t (*)(pid_t, int *, int, struct rusage *)) + __libc_interposing[INTERPOS_wait4])(WAIT_ANY, istat, options, rup)); } -__weak_reference(__libc_wait3, __wait3); - -pid_t -__libc_wait3(int *istat, int options, struct rusage *rup) -{ - - return (__sys_wait4(WAIT_ANY, istat, options, rup)); -} +__weak_reference(__wait3, wait3); diff --git a/lib/libc/gen/waitpid.c b/lib/libc/gen/waitpid.c index 27e920f..5177591 100644 --- a/lib/libc/gen/waitpid.c +++ b/lib/libc/gen/waitpid.c @@ -42,21 +42,13 @@ __FBSDID("$FreeBSD$"); #include "libc_private.h" -#pragma weak waitpid pid_t -waitpid(pid_t pid, int *istat, int options) +__waitpid(pid_t pid, int *istat, int options) { - return (((pid_t (*)(pid_t, int *, int)) - __libc_interposing[INTERPOS_waitpid])(pid, istat, options)); + return (((pid_t (*)(pid_t, int *, int, struct rusage *)) + __libc_interposing[INTERPOS_wait4])(pid, istat, options, NULL)); } -pid_t -__libc_waitpid(pid_t pid, int *istat, int options) -{ - - return (__sys_wait4(pid, istat, options, NULL)); -} - -__weak_reference(__libc_waitpid, __waitpid); -__weak_reference(__libc_waitpid, _waitpid); +__weak_reference(__waitpid, waitpid); +__weak_reference(__waitpid, _waitpid); diff --git a/lib/libc/include/libc_private.h b/lib/libc/include/libc_private.h index ea1a97b..347b463 100644 --- a/lib/libc/include/libc_private.h +++ b/lib/libc/include/libc_private.h @@ -187,17 +187,14 @@ enum { INTERPOS_aio_suspend, INTERPOS_close, INTERPOS_connect, - INTERPOS_creat, INTERPOS_fcntl, INTERPOS_fsync, INTERPOS_fork, INTERPOS_msync, INTERPOS_nanosleep, - INTERPOS_open, INTERPOS_openat, INTERPOS_poll, INTERPOS_pselect, - INTERPOS_raise, INTERPOS_recvfrom, INTERPOS_recvmsg, INTERPOS_select, @@ -212,16 +209,10 @@ enum { INTERPOS_sigwaitinfo, INTERPOS_swapcontext, INTERPOS_system, - INTERPOS_sleep, INTERPOS_tcdrain, - INTERPOS_usleep, - INTERPOS_pause, INTERPOS_read, INTERPOS_readv, - INTERPOS_wait, - INTERPOS_wait3, INTERPOS_wait4, - INTERPOS_waitpid, INTERPOS_write, INTERPOS_writev, INTERPOS__pthread_mutex_init_calloc_cb, @@ -353,23 +344,17 @@ int __sys_sigwait(const __sigset_t *, int *); int __sys_sigwaitinfo(const __sigset_t *, struct __siginfo *); int __sys_swapcontext(struct __ucontext *, const struct __ucontext *); +int __sys_thr_kill(long, int); +int __sys_thr_self(long *); int __sys_truncate(const char *, __off_t); __pid_t __sys_wait4(__pid_t, int *, int, struct rusage *); __ssize_t __sys_write(int, const void *, __size_t); __ssize_t __sys_writev(int, const struct iovec *, int); -int __libc_creat(const char *path, __mode_t mode); -int __libc_pause(void); -int __libc_raise(int); int __libc_sigwait(const __sigset_t * __restrict, int * restrict sig); int __libc_system(const char *); -unsigned int __libc_sleep(unsigned int); int __libc_tcdrain(int); -int __libc_usleep(__useconds_t); -__pid_t __libc_wait(int *); -__pid_t __libc_wait3(int *, int, struct rusage *); -__pid_t __libc_waitpid(__pid_t, int *, int); int __fcntl_compat(int fd, int cmd, ...); /* execve() with PATH processing to implement posix_spawnp() */ diff --git a/lib/libc/sys/accept.c b/lib/libc/sys/accept.c index 38e32f2..1797783 100644 --- a/lib/libc/sys/accept.c +++ b/lib/libc/sys/accept.c @@ -38,11 +38,11 @@ __FBSDID("$FreeBSD$"); #include #include "libc_private.h" -__weak_reference(__sys_accept, __accept); +__weak_reference(__accept, accept); #pragma weak accept int -accept(int s, struct sockaddr *addr, socklen_t *addrlen) +__accept(int s, struct sockaddr *addr, socklen_t *addrlen) { return (((int (*)(int, struct sockaddr *, socklen_t *)) diff --git a/lib/libc/sys/interposing_table.c b/lib/libc/sys/interposing_table.c index 9987bf0..d303779 100644 --- a/lib/libc/sys/interposing_table.c +++ b/lib/libc/sys/interposing_table.c @@ -44,17 +44,14 @@ interpos_func_t __libc_interposing[INTERPOS_MAX] = { SLOT(aio_suspend, __sys_aio_suspend), SLOT(close, __sys_close), SLOT(connect, __sys_connect), - SLOT(creat, __libc_creat), SLOT(fcntl, __fcntl_compat), SLOT(fsync, __sys_fsync), SLOT(fork, __sys_fork), SLOT(msync, __sys_msync), SLOT(nanosleep, __sys_nanosleep), - SLOT(open, __sys_open), SLOT(openat, __sys_openat), SLOT(poll, __sys_poll), SLOT(pselect, __sys_pselect), - SLOT(raise, __libc_raise), SLOT(read, __sys_read), SLOT(readv, __sys_readv), SLOT(recvfrom, __sys_recvfrom), @@ -71,14 +68,8 @@ interpos_func_t __libc_interposing[INTERPOS_MAX] = { SLOT(sigwaitinfo, __sys_sigwaitinfo), SLOT(swapcontext, __sys_swapcontext), SLOT(system, __libc_system), - SLOT(sleep, __libc_sleep), SLOT(tcdrain, __libc_tcdrain), - SLOT(usleep, __libc_usleep), - SLOT(pause, __libc_pause), - SLOT(wait, __libc_wait), - SLOT(wait3, __libc_wait3), SLOT(wait4, __sys_wait4), - SLOT(waitpid, __libc_waitpid), SLOT(write, __sys_write), SLOT(writev, __sys_writev), SLOT(_pthread_mutex_init_calloc_cb, _pthread_mutex_init_calloc_cb_stub), diff --git a/lib/libc/sys/open.c b/lib/libc/sys/open.c index 57c0324..e0273c6 100644 --- a/lib/libc/sys/open.c +++ b/lib/libc/sys/open.c @@ -54,6 +54,6 @@ open(const char *path, int flags, ...) } else { mode = 0; } - return (((int (*)(const char *, int, ...)) - __libc_interposing[INTERPOS_open])(path, flags, mode)); + return (((int (*)(int, const char *, int, ...)) + __libc_interposing[INTERPOS_openat])(AT_FDCWD, path, flags, mode)); } diff --git a/lib/libthr/thread/thr_private.h b/lib/libthr/thread/thr_private.h index 0c16b2a..d62de98 100644 --- a/lib/libthr/thread/thr_private.h +++ b/lib/libthr/thread/thr_private.h @@ -917,8 +917,6 @@ void _thr_stack_fix_protection(struct pthread *thrd); int *__error_threaded(void) __hidden; void __thr_interpose_libc(void) __hidden; pid_t __thr_fork(void); -int __thr_pause(void) __hidden; -int __thr_raise(int sig); int __thr_setcontext(const ucontext_t *ucp); int __thr_sigaction(int sig, const struct sigaction *act, struct sigaction *oact) __hidden; diff --git a/lib/libthr/thread/thr_sig.c b/lib/libthr/thread/thr_sig.c index 1ec64f0..7cd0f75 100644 --- a/lib/libthr/thread/thr_sig.c +++ b/lib/libthr/thread/thr_sig.c @@ -515,23 +515,6 @@ _thr_signal_deinit(void) } int -__thr_pause(void) -{ - sigset_t oset; - - if (_sigprocmask(SIG_BLOCK, NULL, &oset) == -1) - return (-1); - return (__thr_sigsuspend(&oset)); -} - -int -__thr_raise(int sig) -{ - - return (_thr_send_sig(_get_curthread(), sig)); -} - -int __thr_sigaction(int sig, const struct sigaction *act, struct sigaction *oact) { struct sigaction newact, oldact, oldact2; diff --git a/lib/libthr/thread/thr_syscalls.c b/lib/libthr/thread/thr_syscalls.c index a4fe7e8..06b63c8 100644 --- a/lib/libthr/thread/thr_syscalls.c +++ b/lib/libthr/thread/thr_syscalls.c @@ -99,10 +99,6 @@ __FBSDID("$FreeBSD$"); extern int __fcntl_compat(int, int, ...); #endif -/* - * Cancellation behavior: - * If thread is canceled, no socket is created. - */ static int __thr_accept(int s, struct sockaddr *addr, socklen_t *addrlen) { @@ -189,25 +185,6 @@ __thr_connect(int fd, const struct sockaddr *name, socklen_t namelen) return (ret); } - -/* - * Cancellation behavior: - * If thread is canceled, file is not created. - */ -static int -__thr_creat(const char *path, mode_t mode) -{ - struct pthread *curthread; - int ret; - - curthread = _get_curthread(); - _thr_cancel_enter(curthread); - ret = __libc_creat(path, mode); - _thr_cancel_leave(curthread, ret == -1); - - return (ret); -} - /* * Cancellation behavior: * According to specification, only F_SETLKW is a cancellation point. @@ -300,35 +277,6 @@ __thr_nanosleep(const struct timespec *time_to_sleep, * If the thread is canceled, file is not opened. */ static int -__thr_open(const char *path, int flags,...) -{ - struct pthread *curthread; - int mode, ret; - va_list ap; - - /* Check if the file is being created: */ - if ((flags & O_CREAT) != 0) { - /* Get the creation mode: */ - va_start(ap, flags); - mode = va_arg(ap, int); - va_end(ap); - } else { - mode = 0; - } - - curthread = _get_curthread(); - _thr_cancel_enter(curthread); - ret = __sys_open(path, flags, mode); - _thr_cancel_leave(curthread, ret == -1); - - return (ret); -} - -/* - * Cancellation behavior: - * If the thread is canceled, file is not opened. - */ -static int __thr_openat(int fd, const char *path, int flags, ...) { struct pthread *curthread; @@ -523,19 +471,6 @@ __thr_sendto(int s, const void *m, size_t l, int f, const struct sockaddr *t, return (ret); } -static unsigned int -__thr_sleep(unsigned int seconds) -{ - struct pthread *curthread; - unsigned int ret; - - curthread = _get_curthread(); - _thr_cancel_enter(curthread); - ret = __libc_sleep(seconds); - _thr_cancel_leave(curthread, 1); - return (ret); -} - static int __thr_system(const char *string) { @@ -567,55 +502,6 @@ __thr_tcdrain(int fd) return (ret); } -static int -__thr_usleep(useconds_t useconds) -{ - struct pthread *curthread; - int ret; - - curthread = _get_curthread(); - _thr_cancel_enter(curthread); - ret = __libc_usleep(useconds); - _thr_cancel_leave(curthread, 1); - return (ret); -} - -/* - * Cancellation behavior: - * Thread may be canceled at start, but if the system call returns - * a child pid, the thread is not canceled. - */ -static pid_t -__thr_wait(int *istat) -{ - struct pthread *curthread; - pid_t ret; - - curthread = _get_curthread(); - _thr_cancel_enter(curthread); - ret = __libc_wait(istat); - _thr_cancel_leave(curthread, ret <= 0); - return (ret); -} - -/* - * Cancellation behavior: - * Thread may be canceled at start, but if the system call returns - * a child pid, the thread is not canceled. - */ -static pid_t -__thr_wait3(int *status, int options, struct rusage *rusage) -{ - struct pthread *curthread; - pid_t ret; - - curthread = _get_curthread(); - _thr_cancel_enter(curthread); - ret = __libc_wait3(status, options, rusage); - _thr_cancel_leave(curthread, ret <= 0); - return (ret); -} - /* * Cancellation behavior: * Thread may be canceled at start, but if the system call returns @@ -636,24 +522,6 @@ __thr_wait4(pid_t pid, int *status, int options, struct rusage *rusage) /* * Cancellation behavior: - * Thread may be canceled at start, but if the system call returns - * a child pid, the thread is not canceled. - */ -static pid_t -__thr_waitpid(pid_t wpid, int *status, int options) -{ - struct pthread *curthread; - pid_t ret; - - curthread = _get_curthread(); - _thr_cancel_enter(curthread); - ret = __libc_waitpid(wpid, status, options); - _thr_cancel_leave(curthread, ret <= 0); - return (ret); -} - -/* - * Cancellation behavior: * Thread may be canceled at start, but if the thread wrote some data, * it is not canceled. */ @@ -701,17 +569,14 @@ __thr_interpose_libc(void) SLOT(aio_suspend); SLOT(close); SLOT(connect); - SLOT(creat); SLOT(fcntl); SLOT(fsync); SLOT(fork); SLOT(msync); SLOT(nanosleep); - SLOT(open); SLOT(openat); SLOT(poll); SLOT(pselect); - SLOT(raise); SLOT(read); SLOT(readv); SLOT(recvfrom); @@ -728,14 +593,8 @@ __thr_interpose_libc(void) SLOT(sigwaitinfo); SLOT(swapcontext); SLOT(system); - SLOT(sleep); SLOT(tcdrain); - SLOT(usleep); - SLOT(pause); - SLOT(wait); - SLOT(wait3); SLOT(wait4); - SLOT(waitpid); SLOT(write); SLOT(writev); #undef SLOT From owner-freebsd-threads@FreeBSD.ORG Wed Jan 7 04:01:38 2015 Return-Path: Delivered-To: threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 22076F91; Wed, 7 Jan 2015 04:01:38 +0000 (UTC) Received: from mail.netplex.net (mail.netplex.net [204.213.176.9]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "*.netplex.net", Issuer "RapidSSL CA" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id D6DE01CE8; Wed, 7 Jan 2015 04:01:37 +0000 (UTC) Received: from sea.ntplx.net (sea.ntplx.net [204.213.176.11]) by mail.netplex.net (8.14.9/8.14.9/NETPLEX) with ESMTP id t0741UUR064895; Tue, 6 Jan 2015 23:01:30 -0500 X-Virus-Scanned: by AMaViS and Clam AntiVirus (mail.netplex.net) X-Greylist: Message whitelisted by DRAC access database, not delayed by milter-greylist-4.4.3 (mail.netplex.net [204.213.176.9]); Tue, 06 Jan 2015 23:01:30 -0500 (EST) Date: Tue, 6 Jan 2015 23:01:30 -0500 (EST) From: Daniel Eischen X-X-Sender: eischen@sea.ntplx.net Reply-To: Daniel Eischen To: Konstantin Belousov Subject: Re: Fixing dlopen("libpthread.so") In-Reply-To: <20150107032941.GJ42409@kib.kiev.ua> Message-ID: References: <20141226165337.GJ1754@kib.kiev.ua> <20150103212837.GC46373@stack.nl> <20150104114600.GC42409@kib.kiev.ua> <20150104182026.GA61250@stack.nl> <20150105023708.GD42409@kib.kiev.ua> <20150106205046.GA24971@stack.nl> <20150107032941.GJ42409@kib.kiev.ua> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: threads@freebsd.org, arch@freebsd.org X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Jan 2015 04:01:38 -0000 On Wed, 7 Jan 2015, Konstantin Belousov wrote: > On Tue, Jan 06, 2015 at 09:50:46PM +0100, Jilles Tjoelker wrote: >> On Mon, Jan 05, 2015 at 04:37:08AM +0200, Konstantin Belousov wrote: >>> Next natural question is about the __error calls through PLT in the >>> .cerror asm. Before the work was committed, libthr interposed __error, >>> which was required for correct operation. Now, we need __error exported, >>> but do we need support its interposing ? This is an implementation >>> detail for errno, I do not see any loss from not allowing to override >>> errno location. >> >> Indeed, there is no need to allow interposing __error (as with many >> libc-internal calls to its exported symbols). Additionally, the current >> namespace.h mechanism could be used to redirect __error calls from C >> code. >> >> Glibc uses macro trickery with the asm labels compiler feature, making >> libc code see things like >> int *__error(void) asm("__hidden__error"); >> and defining the hidden alias somewhere. This also works for >> compiler-generated calls like to memcpy(). I'm not sure whether it is a >> good idea to add this extra trickery and depend on the compiler feature. > I might look at this after the current change is committed. I don't quite follow all the above about __error(), but non-C programs need to interface to an __error() that works regardless of threading or not. -- DE From owner-freebsd-threads@FreeBSD.ORG Wed Jan 7 04:21:03 2015 Return-Path: Delivered-To: threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id AD22E28C; Wed, 7 Jan 2015 04:21:03 +0000 (UTC) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 1E8D964054; Wed, 7 Jan 2015 04:21:02 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.9/8.14.9) with ESMTP id t074KsMq070206 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 7 Jan 2015 06:20:54 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.9.2 kib.kiev.ua t074KsMq070206 Received: (from kostik@localhost) by tom.home (8.14.9/8.14.9/Submit) id t074KrMN070204; Wed, 7 Jan 2015 06:20:53 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Wed, 7 Jan 2015 06:20:53 +0200 From: Konstantin Belousov To: Daniel Eischen Subject: Re: Fixing dlopen("libpthread.so") Message-ID: <20150107042053.GK42409@kib.kiev.ua> References: <20141226165337.GJ1754@kib.kiev.ua> <20150103212837.GC46373@stack.nl> <20150104114600.GC42409@kib.kiev.ua> <20150104182026.GA61250@stack.nl> <20150105023708.GD42409@kib.kiev.ua> <20150106205046.GA24971@stack.nl> <20150107032941.GJ42409@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.0 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on tom.home Cc: threads@freebsd.org, arch@freebsd.org X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Jan 2015 04:21:03 -0000 On Tue, Jan 06, 2015 at 11:01:30PM -0500, Daniel Eischen wrote: > On Wed, 7 Jan 2015, Konstantin Belousov wrote: > > > On Tue, Jan 06, 2015 at 09:50:46PM +0100, Jilles Tjoelker wrote: > >> On Mon, Jan 05, 2015 at 04:37:08AM +0200, Konstantin Belousov wrote: > >>> Next natural question is about the __error calls through PLT in the > >>> .cerror asm. Before the work was committed, libthr interposed __error, > >>> which was required for correct operation. Now, we need __error exported, > >>> but do we need support its interposing ? This is an implementation > >>> detail for errno, I do not see any loss from not allowing to override > >>> errno location. > >> > >> Indeed, there is no need to allow interposing __error (as with many > >> libc-internal calls to its exported symbols). Additionally, the current > >> namespace.h mechanism could be used to redirect __error calls from C > >> code. > >> > >> Glibc uses macro trickery with the asm labels compiler feature, making > >> libc code see things like > >> int *__error(void) asm("__hidden__error"); > >> and defining the hidden alias somewhere. This also works for > >> compiler-generated calls like to memcpy(). I'm not sure whether it is a > >> good idea to add this extra trickery and depend on the compiler feature. > > I might look at this after the current change is committed. > > I don't quite follow all the above about __error(), but non-C > programs need to interface to an __error() that works regardless > of threading or not. C programs also need to be able to call __error(), to have working errno. Discussion above talks about the need for the programs to be able to interpose __error(), which I and Jilles agree is excessive. My proposal is to remove PLT indirection in the calls to __error() from _inside_ libc, which makes errno functioning in libc both more reliable and slightly faster. From owner-freebsd-threads@FreeBSD.ORG Wed Jan 7 05:14:58 2015 Return-Path: Delivered-To: threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 7B761682; Wed, 7 Jan 2015 05:14:58 +0000 (UTC) Received: from mail.netplex.net (mail.netplex.net [204.213.176.9]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "*.netplex.net", Issuer "RapidSSL CA" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 3BA4E64709; Wed, 7 Jan 2015 05:14:57 +0000 (UTC) Received: from sea.ntplx.net (sea.ntplx.net [204.213.176.11]) by mail.netplex.net (8.14.9/8.14.9/NETPLEX) with ESMTP id t075EtkD034465; Wed, 7 Jan 2015 00:14:56 -0500 X-Virus-Scanned: by AMaViS and Clam AntiVirus (mail.netplex.net) X-Greylist: Message whitelisted by DRAC access database, not delayed by milter-greylist-4.4.3 (mail.netplex.net [204.213.176.9]); Wed, 07 Jan 2015 00:14:56 -0500 (EST) Date: Wed, 7 Jan 2015 00:14:55 -0500 (EST) From: Daniel Eischen X-X-Sender: eischen@sea.ntplx.net Reply-To: Daniel Eischen To: Konstantin Belousov Subject: Re: Fixing dlopen("libpthread.so") In-Reply-To: <20150107042053.GK42409@kib.kiev.ua> Message-ID: References: <20141226165337.GJ1754@kib.kiev.ua> <20150103212837.GC46373@stack.nl> <20150104114600.GC42409@kib.kiev.ua> <20150104182026.GA61250@stack.nl> <20150105023708.GD42409@kib.kiev.ua> <20150106205046.GA24971@stack.nl> <20150107032941.GJ42409@kib.kiev.ua> <20150107042053.GK42409@kib.kiev.ua> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: threads@freebsd.org, arch@freebsd.org X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Jan 2015 05:14:58 -0000 On Wed, 7 Jan 2015, Konstantin Belousov wrote: > On Tue, Jan 06, 2015 at 11:01:30PM -0500, Daniel Eischen wrote: >> On Wed, 7 Jan 2015, Konstantin Belousov wrote: >> >>> On Tue, Jan 06, 2015 at 09:50:46PM +0100, Jilles Tjoelker wrote: >>>> On Mon, Jan 05, 2015 at 04:37:08AM +0200, Konstantin Belousov wrote: >>>>> Next natural question is about the __error calls through PLT in the >>>>> .cerror asm. Before the work was committed, libthr interposed __error, >>>>> which was required for correct operation. Now, we need __error exported, >>>>> but do we need support its interposing ? This is an implementation >>>>> detail for errno, I do not see any loss from not allowing to override >>>>> errno location. >>>> >>>> Indeed, there is no need to allow interposing __error (as with many >>>> libc-internal calls to its exported symbols). Additionally, the current >>>> namespace.h mechanism could be used to redirect __error calls from C >>>> code. >>>> >>>> Glibc uses macro trickery with the asm labels compiler feature, making >>>> libc code see things like >>>> int *__error(void) asm("__hidden__error"); >>>> and defining the hidden alias somewhere. This also works for >>>> compiler-generated calls like to memcpy(). I'm not sure whether it is a >>>> good idea to add this extra trickery and depend on the compiler feature. >>> I might look at this after the current change is committed. >> >> I don't quite follow all the above about __error(), but non-C >> programs need to interface to an __error() that works regardless >> of threading or not. > > C programs also need to be able to call __error(), to have working > errno. Discussion above talks about the need for the programs to be > able to interpose __error(), which I and Jilles agree is excessive. My > proposal is to remove PLT indirection in the calls to __error() from > _inside_ libc, which makes errno functioning in libc both more reliable > and slightly faster. Fair enough - thanks for the explanation! -- DE