Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 23 Nov 2014 00:09:55 +0100
From: Jens Gustedt <jens.gustedt@...ia.fr>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] Add stdatomic.h for clang>=3.1 and gcc>=4.1

Hello,

Am Samstag, den 22.11.2014, 21:52 +0100 schrieb Joakim Sindholt:
> On Sun, 2014-11-09 at 18:11 +0100, Jens Gustedt wrote:
> > I would propose some rules that should be respected by such a header
> > file:
> > 
> >  - all syntax that this supports should be correct C11 syntax, but it
> >    may be a restricted version, that is not all C11 might be
> >    supported. In such a case the compilation should fail.
> > 
> >  - all C11 syntax that was not in C99 and that compiles with this
> >    header should compile to correct executables with provable atomic
> >    sematic as defined by C11
> > 
> >  - all produced object code must be ABI compatible with the newer
> >    versions of the same compiler
> 
> Agreed.
> 
> I'm sorry it's taken so long to respond but I didn't want to do it from
> my phone and I haven't been home in a while.
> 
> > Your file doesn't provide support for atomics for the non-basic
> > types, but which the C11 standard requests? How to deal with them? Do
> > you plan to implement libatomic.a ? I think that would be a good idea,
> > because the gcc implementation is bogus and relies on
> > pthread_mutex_t instead of atomic_flag.
> 
> I'm not sure what you mean here? Does __atomic_stuff in GCC not get
> translated into assembly level atomics? I just disassembled a program
> compiled with this revised version of stdatomic.h and GCC 4.7 and I see
> no special calls and plenty of atomic asm.
> And I do wish I had the time to go and implement atomic functions for
> libc.a but I'm not really sure how to do it right now.

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.

> > Would we'd have to mimic the different interfaces on a per compiler
> > base?
> 
> As it stands right now clang has their __c11_atomic stuff and gcc has
> __atomic which doesn't mimic C11 exactly but close enough. Other
> compilers will probably provide one or the other but the problem here is
> obviously that we might end up trampling on a compiler's own
> implementation.
> There are 2 reasons I propose this be included in musl at some point
> after rigorous testing:
> 1) clang provides full C11 atomic support as of 3.1 but doesn't ship
> stdatomic.h until 3.6, which is not even released yet.
> 2) It would be ideal if we were to provide stdatomic.h support for
> compilers that do not have it built in.
> Number 2 has a few problems as it requires generic function-like macros
> and I cannot see how to implement it properly without compiler-specific
> features. That might be alright too since we can at least support old
> GCC.

In effect, for the similar feature in P99 I did that with gcc'ish
extensions, so it only works for gcc and its cousins (clang, icc).

But even without such stuff you could get things working at least for
the basic types by switching with respect to the size of the base
type. Here only 1, 2, 4, and 8 are interesting, so not a big deal. If
you want to map to assembler directly (in absence of sync_ builtins)
you could probably chose the right word size for the assembler
instruction.

> > Generally, at least for a start, I think it would be a good idea to
> > separate the support for different compilers, and in particular don't
> > claim support for a compiler which you didn't even test :)
> 
> In my defense, I did say it was only for personal testing purposes. I'm
> not insane enough to propose committing this to musl without testing.
> 
> > Having something for earlier versions of clang would already be a good
> > start.
> > 
> > Some more comments in the text, but I didn't do a complete review,
> > yet, less did I do tests.
> > 
> > Jens
> > 
> > Am Sonntag, den 09.11.2014, 13:53 +0100 schrieb Joakim Sindholt:
> > > GCC and Clang ship their own stdatomic.h headers but not for all the
> > > versions mentioned above. They ship many other standard headers too that
> > > musl also ships (stdbool/def/int/...).
> > > 
> > > In the end musl should be a complete libc with all standard headers and
> > > this is one of them. It is up to the individual programmer to ensure
> > > that his or her compiler is properly supported by for example checking
> > > __STD_NO_ATOMICS__ and __STDC_VERSION__ >= 201112L.
> > 
> > As said above checking these would have to give the assertion that the
> > platform supports all atomics, library and compiler, which we
> > shouldn't give lighthearted
> > 
> > > 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.

> > > +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 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.

> > > +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.

> 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.

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.