Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20201222014030.GU534@brightrain.aerifal.cx>
Date: Mon, 21 Dec 2020 20:40:31 -0500
From: Rich Felker <dalias@...c.org>
To: Jesse DeGuire <jesse.a.deguire@...il.com>
Cc: musl@...ts.openwall.com, Markus Wichmann <nullplan@....net>
Subject: Re: Musl on Cortex-M Devices

On Mon, Dec 21, 2020 at 07:00:02PM -0500, Jesse DeGuire wrote:
> On Fri, Dec 18, 2020 at 3:35 PM Rich Felker <dalias@...c.org> wrote:
> >
> > On Fri, Dec 18, 2020 at 08:38:49PM +0100, Markus Wichmann wrote:
> > > On Wed, Dec 16, 2020 at 07:23:49PM -0500, Rich Felker wrote:
> > > > On Wed, Dec 16, 2020 at 06:43:15PM -0500, Jesse DeGuire wrote:
> > > > > diff --git a/src/thread/arm/__unmapself.s b/src/thread/arm/__unmapself..s
> > > > > index 29c2d07b..fe0406e3 100644
> > > > > --- a/src/thread/arm/__unmapself.s
> > > > > +++ b/src/thread/arm/__unmapself.s
> > > > > @@ -3,7 +3,7 @@
> > > > >  .global __unmapself
> > > > >  .type   __unmapself,%function
> > > > >  __unmapself:
> > > > > - mov r7,#91
> > > > > + movs r7,#91
> > > > >   svc 0
> > > > > - mov r7,#1
> > > > > + movs r7,#1
> > > > >   svc 0
> > > >
> > > > OK, but this file can't be used on nommu anyway. Instead the generic C
> > > > version has to be used.
> > > >
> > >
> > > Speaking of that (C) version, maybe someone should note down somewhere
> > > that it is only safe to call if simultaneous calls from other threads
> > > are precluded somehow. That function uses global variables, so if
> > > multiple threads call it at the same time, there will be blood (well,
> > > there will be clobbering in any case). Which is fulfilled at the moment:
> > > The only call site of __unmapself() is pthread_exit(), after getting the
> > > thread list lock. That is a global lock, so further threads also
> > > exitting would have to queue up there. But I think it is rather
> > > non-intuitive that the thread-list lock happens to also protect the
> > > global variables in __unmapself.c. And that is a property of the C
> > > version not shared by the assembler versions, which are all thread-safe.
> >
> > Prior to commit 8f11e6127fe93093f81a52b15bb1537edc3fc8af,
> > __unmapself.c had its own lock and had to explicitly modify the exit
> > futex address to unlock that lock. Now it can (and must, since neither
> > can be unlocked before the kernel-level task exits) share the thread
> > list lock for this purpose.
> >
> > FWIW there's arguably no reason to even keep the asm versions of this
> > function now that the thread list lock exists. We could just remove
> > them all.
> 
> Should I just go ahead and remove this so the generic C version is
> used instead since I'm already here?

Keep what you've got for now. If we decide to remove it globally it's
trivial to merge that change, but the removal is logically unrelated
to the M-profile work and will be done as a separate commit if it's
done.

Rich

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.