From: Rich Felker <dalias@libc.org>
To: musl@lists.openwall.com
Subject: Re: [PATCH 3/9] use weak symbols for the POSIX functions that will be used by C threads
Date: Sat, 6 Sep 2014 14:52:46 -0400 [thread overview]
Message-ID: <20140906185246.GW23797@brightrain.aerifal.cx> (raw)
In-Reply-To: <d408051f223ecc194b28f8df1f113a30427d17f6.1409524413.git.Jens.Gustedt@inria.fr>
On Mon, Sep 01, 2014 at 12:46:23AM +0200, Jens Gustedt wrote:
> The intent of this is to avoid name space pollution of the C threads
> implementation.
I found a few bugs in this which I'm fixing and about to commit if all
goes well:
> diff --git a/src/mman/mprotect.c b/src/mman/mprotect.c
> index f488486..535787b 100644
> --- a/src/mman/mprotect.c
> +++ b/src/mman/mprotect.c
> @@ -2,10 +2,12 @@
> #include "libc.h"
> #include "syscall.h"
>
> -int mprotect(void *addr, size_t len, int prot)
> +int __mprotect(void *addr, size_t len, int prot)
> {
> size_t start, end;
> start = (size_t)addr & -PAGE_SIZE;
> end = (size_t)((char *)addr + len + PAGE_SIZE-1) & -PAGE_SIZE;
> return syscall(SYS_mprotect, start, end-start, prot);
> }
> +
> +weak_alias(__mprotect, mprotect);
I tested and inlining the syscall does not make pthread_create.o
smaller. It would eliminate a dependency on one extra file, so I might
make the change later, but for now I'm just doing it your way in the
interest of getting things committed without major edits to the
patches.
> diff --git a/src/thread/__timedwait.c b/src/thread/__timedwait.c
> index d6f1233..3d62272 100644
> --- a/src/thread/__timedwait.c
> +++ b/src/thread/__timedwait.c
> @@ -4,6 +4,9 @@
> #include "futex.h"
> #include "syscall.h"
>
> +int __pthread_setcancelstate(int new, int *old);
> +int __clock_gettime(clockid_t clk, struct timespec *ts);
I've removed arg names from prototypes like this just for consistency
with how it's done elsewhere.
> @@ -139,7 +143,7 @@ static void init_file_lock(FILE *f)
>
> void *__copy_tls(unsigned char *);
>
> -int pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict attrp, void *(*entry)(void *), void *restrict arg)
> +static int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict attrp, void *(*entry)(void *), void *restrict arg)
This static is wrong if the goal is to go ahead and get the files in
the form where they're "ready to use" for C11. I saw it was reverted
in the later patch, but I'm just taking it out here.
> diff --git a/src/thread/pthread_getspecific.c b/src/thread/pthread_getspecific.c
> index b2a282c..bfc4294 100644
> --- a/src/thread/pthread_getspecific.c
> +++ b/src/thread/pthread_getspecific.c
> @@ -1,7 +1,10 @@
> #include "pthread_impl.h"
>
> -void *pthread_getspecific(pthread_key_t k)
> +static void *__pthread_getspecific(pthread_key_t k)
> {
> struct pthread *self = __pthread_self();
> return self->tsd[k];
> }
> +
> +weak_alias(__pthread_getspecific, pthread_getspecific);
> +weak_alias(__pthread_getspecific, tss_get);
This actually goes ahead and implements one C11 function, which is not
horrible, but mildly inconsistent. I think I'll move it to the later
patch that's actually supposed to implement these functions.
> diff --git a/src/thread/pthread_mutex_trylock.c b/src/thread/pthread_mutex_trylock.c
> index e851517..9be7930 100644
> --- a/src/thread/pthread_mutex_trylock.c
> +++ b/src/thread/pthread_mutex_trylock.c
> @@ -50,9 +50,11 @@ int __pthread_mutex_trylock_owner(pthread_mutex_t *m)
> return 0;
> }
>
> -int pthread_mutex_trylock(pthread_mutex_t *m)
> +static int __pthread_mutex_trylock(pthread_mutex_t *m)
> {
> if ((m->_m_type&15) == PTHREAD_MUTEX_NORMAL)
> return a_cas(&m->_m_lock, 0, EBUSY) & EBUSY;
> return __pthread_mutex_trylock_owner(m);
> }
> +
> +weak_alias(__pthread_mutex_trylock, pthread_mutex_trylock);
This static looks like an actual bug that would break the patch series
as a whole; I can't see where it's made visible for mtx_trylock to use
later. Removing static here.
Rich
next prev parent reply other threads:[~2014-09-06 18:52 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-31 22:45 [PATCH 0/9] C thread patch series, v. 8.6 and 9.7 Jens Gustedt
2014-08-31 22:45 ` [PATCH 1/9] interface additions for the C thread implementation Jens Gustedt
2014-09-07 0:21 ` Rich Felker
2014-09-07 9:13 ` Jens Gustedt
2014-09-07 10:05 ` Alexander Monakov
2014-09-07 11:16 ` Jens Gustedt
2014-09-07 11:31 ` Alexander Monakov
2014-09-07 11:32 ` Rich Felker
2014-09-07 14:45 ` Jens Gustedt
2014-09-07 15:16 ` Rich Felker
2014-09-07 16:51 ` Jens Gustedt
2014-09-07 16:55 ` Rich Felker
2014-09-07 1:19 ` Rich Felker
2014-08-31 22:46 ` [PATCH 2/9] additions to src/time and some implied minor changes here and there Jens Gustedt
2014-09-06 17:44 ` Rich Felker
2014-08-31 22:46 ` [PATCH 3/9] use weak symbols for the POSIX functions that will be used by C threads Jens Gustedt
2014-09-06 18:52 ` Rich Felker [this message]
2014-08-31 22:46 ` [PATCH 4/9] add the functions for tss_t and once_flag Jens Gustedt
2014-08-31 22:46 ` [PATCH 5/9] add the functions for mtx_t Jens Gustedt
2014-09-07 1:51 ` Rich Felker
2014-09-07 1:54 ` Rich Felker
2014-08-31 22:47 ` [PATCH 6/9] add the functions for cnd_t Jens Gustedt
2014-08-31 22:47 ` [PATCH 7/9] add the thrd_xxxxxx functions Jens Gustedt
2014-09-07 14:24 ` Rich Felker
2014-09-07 14:52 ` Jens Gustedt
2014-09-07 15:17 ` Rich Felker
2014-08-31 22:47 ` [PATCH 8/9] separate pthread_create and pthread_exit in two different TU Jens Gustedt
2014-08-31 22:48 ` [PATCH 9/9] Separate pthread_create and thrd_create Jens Gustedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140906185246.GW23797@brightrain.aerifal.cx \
--to=dalias@libc.org \
--cc=musl@lists.openwall.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).