Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 22 Nov 2014 18:30:22 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1

On Sun, Nov 23, 2014 at 12:09:55AM +0100, Jens Gustedt wrote:
> No, not all atomics get translated directly into assembler, some need
> auxiliary functions. This is for all types that don't have atomic
> support in the processor, the assembler is missing the instruction,
> .... or the type is simply not a basic type such as a struct or so.
> 
> This then usually results in atomic types that are not "lockfree". As
> far as I see Gcc implements them with some sort of hash table of locks
> that uses the address of the protected object as hash key.
> 
> With gcc 4.9 on my machine I have 204 symbols exported by libatomic.a
> 
> This hash table uses pthread_mutex_t to lock the access. This is where
> I'd like to have a replacement. C mutex's (mtx_t) would already be a
> progress, but I actually think that atomic_flag should be enough.

atomic_flag is not viable for this because it does not have a
wait/wake mechanism. You'd be spinning, which means in processes with
different priorities involved, you could easily get deadlock if the
lower-priority thread got suspended while holding the lock. You really
do need mutexes.

> > > > For clang the support is complete for versions >= 3.1. GCC added the
> > > > __atomic functions in 4.7 but not _Atomic until 4.9 so from 4.1 it will
> > > > work correctly so long as you don't use VLAs together with things like
> > > > the ++ operator in arguments and is subject to the same limitations as
> > > > 4.7.
> > > 
> > > I don't follow here. Could you explain a bit more?
> > > 
> > > I have the impression that your patch can't work at all with gcc,
> > > since it is missing _Atomic and you need the typedef's with _Atomic in
> > > them. Then, what is the connection with VLA? and the ++ operator?
> > 
> > GCC was nice enough to add all the __atomic stuff sans _Atomic for 4.7.
> > The _Atomic keyword didn't come along until 4.9. So basically you can
> > trivially implement the function-like macros for 4.7 but you won't have
> > 4.7.
> 
> What has all of this to do with VLA? I am lost.

The operands of __typeof__ and sizeof get evaluated when they have VLA
type. I think this is the problem.

> > > > +typedef enum {
> > > > +    memory_order_relaxed = 0,
> > > > +    memory_order_consume = 1,
> > > > +    memory_order_acquire = 2,
> > > > +    memory_order_release = 3,
> > > > +    memory_order_acq_rel = 4,
> > > > +    memory_order_seq_cst = 5
> > > > +} memory_order;
> > > 
> > > I'd prefer to provide the corresponding macros as gcc defines them
> > > with #ifdef and then just use these macros here
> 
> > I'd prefer to just keep them as these constants.
> 
> You really have to be careful here, because this is part of the ABI
> and the compiler provides the values for this, so you should use
> these.

I was the one why requested doing them explicitly. They're part of the
ABI and can't change, so pretending they're something that could
change is unnecessary and obfuscating (IMO).

> > I would go so far as to
> > say that this is a valid interpretation of the standard.
> > http://port70.net/~nsz/c/c11/n1570.html#7.17.3
> 
> sure it is a valid interpretation, this is not the question. We don't
> live in an isolated world, and so those choices are not ours to make.

The choice was made by whichever of GCC and clang first implemented
them and added them to the ABI. Now they're fixed with the values they
were added with.

> > > > +typedef _Atomic _Bool atomic_flag;
> > > 
> > > that has chances to be wrong base type, probably this would be better
> > > 
> > > typedef _Atomic int atomic_flag;
> > > 
> > > at least you'd really have to be sure about the ABI that later
> > > versions of the corresponding compiler suppose
> > 
> > The GCC test_and_set function explicitly takes a bool * and both GCC and
> > Clang implement it as a _Bool. GCC of course makes it significantly more
> > complicated than that and will on occasion make it an unsigned char but
> > nonetheless bool is the correct type here.
> 
> I think you missed the point here. It is much, much different than you
> describe, I looked it up for gcc. It is a struct that contains such
> types. Don't mix these up, typedef to a basic type and typedef to a
> struct are two completely different things, they may have very
> different sizeof and alignment properties, because of padding.

Yes, I recall we looked into this mess dissucsing it on IRC...

> > I have changed it to be an atomic_bool in a struct as both GCC and Clang
> > has it in a struct. Presumably to separate it from the generic _Atomic
> > stuff.
> 
> Again, since we want to have ABI compatibility, it is not your choice
> to make. You'd simply have to stick to the choice that gcc made. So
> you have to copy the declaration of the struct, including all the
> ifdef fuzz.

I'd have to look at it again, but IIRC only one case of the #ifdef
mess was actually possible. The others were for hypothetical archs
without real atomics which we can't support anyway.

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.