Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1409502960.4476.297.camel@eris.loria.fr>
Date: Sun, 31 Aug 2014 18:36:00 +0200
From: Jens Gustedt <jens.gustedt@...ia.fr>
To: musl@...ts.openwall.com
Subject: Re: [PATCH 7/8] add the thrd_xxxxxx functions

Am Sonntag, den 31.08.2014, 12:06 -0400 schrieb Rich Felker:
> On Sun, Aug 31, 2014 at 05:07:22PM +0200, Jens Gustedt wrote:
> > > > > > > Also, since it doesn't depend on anything in pthread_create.c,
> > > > > > 
> > > > > > It does, __THRD_C11 :)
> > > > > 
> > > > > How about naming it to __ATTRP_C11_THREAD and putting it in
> > > > > pthread_impl.h then?
> > > > 
> > > > ok, I'll do that
> > > > 
> > > > But hopefully we didn't overlook any possible use pattern that would
> > > > drag in different versions of the tsd symbols. I am not too
> > > > comfortable with that.
> > > 
> > > What do you mean here? I don't follow.
> > 
> > When trying to separate the two implementations, for a while I
> > struggled with the fact that the game with the dummy functions
> > enforces that pthread_exit and pthread_create must be in the same
> > TU. I found that difficult to deal with, not knowing the code and the
> > interaction between the different TU too well.
> > 
> > (There might be an independent possibility of improvement in
> > readability and maintainability in separating pthread_create and
> > pthread_exit more cleanly.)
> 
> pthread_create inherently pulls in pthread_exit because the start
> function calls the latter. The other direction is not inherent -- for
> example, a single-threaded program could in principle exit via a
> direct call to pthread_exit or cancellation of pthread_self().

so such a hypothetical application would gain some bytes if we'd
separate them :)

> Regarding the tsd weak symbols, the logic is slightly complicated. The
> main thread's tsd pointer can be initialized during the first
> pthread_create call, or by pthread_key_create; it merely needs to be
> valid before any call to pthread_setspecific or pthread_getspecific.
> I'd kind of like to get rid of the initialization path in
> pthread_create, but the one in pthread_key_create cannot find the main
> thread unless the main thread is the only thread in existence when
> it's called.

yes

In any case patch 8 assembles all code that is needed for tsd in one
TU, pthread_exit.c, and should otherwise be no modification to the
existing pthread code.

> In any case, my quick analysis of the weak symbols leads me to believe
> it's fine to split the ones used by pthread_exit and pthread_create
> into separate TUs, but I'm still not convinced this is terribly useful
> to do.

By itself perhaps not, we'll see. As a preparatory step if we want to
separate the two implementations, more likely.

> > > > But I just discovered another such incentive :) You were right, that
> > > > the error handling for thrd_create was not correct for C11, but it
> > > > wasn't my fault :) POSIX (and thus __pthread_create) basically maps
> > > > all errors to EAGAIN, where C11 requires us to distinguish ENOMEM from
> > > > other failures.
> > > > 
> > > > Thus I had to integrate this difference into __pthread_create, which
> > > > was not difficult, but which intrudes even a little bit more into the
> > > > existing code.
> > > 
> > > I think POSIX's EAGAIN is fully equivalent to C11's thrd_nomem: both
> > > should reflect any condition where the thread could not be created due
> > > to a resource exhaustion type failure. While you could argue that
> > > thrd_nomem should only indicate failure of the mmap, not the clone, I
> > > think this would be a useless distinction to callers (both of them are
> > > fundamentally allocation operations) and then you'd be forced to use
> > > thrd_error for clone failures, whereas I would think thrd_error should
> > > be reserved for either erroneous usage (but that's UB anyway) or more
> > > permanent failures (like lack of thread support on the OS).
> > 
> > Having read up a bit, now, I don't think so, for C threads this
> > mapping is not correct.  clone has several different error returns
> > that the actual code correctly maps to EAGAIN, but among them it also
> > has ENOMEM.
> > 
> > So we have possible ENOMEM coming from clone and from mmap, and a
> > bunch of other obscure errors that should go to thrd_error.
> 
> Like what? I see no possible errors except EAGAIN and ENOMEM. The only
> others listed in the man page are EINVAL and EPERM and they pertain to
> invalid arguments that aren't being used by pthread_create.

(I withdraw the "bunch of")

EAGAIN from clone is clearly a distinct error condition than when it
is on ENOMEM. I would not see it covered by what C11 expects as that
error condition. So the EAGAIN from clone should go to thrd_error, I
think, and not be merged with ENOMEM for the C threads implementation.

Jens

-- 
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::



Download attachment "signature.asc" of type "application/pgp-signature" (199 bytes)

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.