Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20170704211418.GQ1627@brightrain.aerifal.cx>
Date: Tue, 4 Jul 2017 17:14:18 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] unify the use of FUTEX_PRIVATE

On Sun, Jun 25, 2017 at 09:29:51AM +0200, Jens Gustedt wrote:
> Hello Rich,
> 
> On Sat, 24 Jun 2017 18:16:30 -0400 Rich Felker <dalias@...c.org> wrote:
> 
> > For now let's just use a macro for FUTEX_PRIVATE.
> 
> Ok, was my guess :)
> 
> > My concern was
> > mainly about whether there are cases where it's assumed that the 128
> > or ^128 translates to FUTEX_PRIVATE by arithmetic ops. The __wait,
> > __timedwait, and __wake interfaces just treat their argument as a
> > boolean now, so I think the only places there might be remaining
> > assumptions of that sort is where a flag bit from the sync object is
> > passed directly into a futex __syscall -- places like the condvar
> > implementation. You've looked at all this code just now so you
> > probably have a better idea if there are any such places;
> 
> All the usages that I found go through a Boolean operation. The only
> place where -maybe- the compiler might shortcut some of it is __wake
> because it is inlined. In any case, by just using the macro the
> compiler see's the same 128.
> 
> > if so I
> > think the assumption that the bits match up should just be replaced by
> > a conditional (e.g. flags & 128 ? 0 : FUTEX_PRIVATE or such).
> 
> There is none in the arch independent code, so you could go with my
> patch as it is, I think.

Yes, I agree. Applying it. Thanks!

> I only found one other place, but which I don't understand (and I am
> not sure that I want to :). This is in
> src/thread/x32/syscall_cp_fixup.c
> where there is a special casing for SYS_futex

This could also be changed, but being that it's arch-specific and not
something people would read when trying to understand the code, I
don't think it really matters. We can change it in another commit if
you really want to.

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.