C11 atomic implementation broken

[marcusva]

Compiling openal-soft with a compiler that supports C11 atomics (in my case clang 3.4.0) is not possible:

...
-- The C compiler identification is Clang 3.4.0
-- The CXX compiler identification is Clang 3.4.0
-- Check for working C compiler: /usr/bin/cc
...
-- Performing Test HAVE_C11_STATIC_ASSERT
-- Performing Test HAVE_C11_STATIC_ASSERT - Success
-- Performing Test HAVE_C11_ALIGNAS
-- Performing Test HAVE_C11_ALIGNAS - Success
-- Performing Test HAVE_C11_ATOMIC
-- Performing Test HAVE_C11_ATOMIC - Success
...
...
[  4%] Building C object CMakeFiles/makehrtf.dir/utils/makehrtf.c.o
[  6%] Building C object CMakeFiles/common.dir/common/atomic.c.o
[  8%] Building C object CMakeFiles/common.dir/common/rwlock.c.o
In file included from /usr/home/marcus/devel/openal-soft-1.16.0/common/atomic.c:4:
/usr/home/marcus/devel/openal-soft-1.16.0/include/atomic.h:19:10: error: address argument to atomic operation must be a pointer to _Atomic type ('volatile int *' invalid)
{ return atomic_exchange(ptr, newval); }
         ^               ~~~
/usr/include/stdatomic.h:350:2: note: expanded from macro 'atomic_exchange'
        atomic_exchange_explicit(object, desired, memory_order_seq_cst)
        ^
/usr/include/stdatomic.h:243:2: note: expanded from macro 'atomic_exchange_explicit'
        __c11_atomic_exchange(object, desired, order)
        ^
...

The definitions in atomic.h

inline int ExchangeInt(volatile int *ptr, int newval)
{ return atomic_exchange(ptr, newval); }
inline void *ExchangePtr(XchgPtr *ptr, void *newval)
{ return atomic_exchange(ptr, newval); }

just pass pointers into atomic_exchange() without a proper cast or a ATOMIC_VAR_INIT() initialization.

[kcat]

Hmm, GCC 4.9 doesn't seem to have an issue with this, although it does appear to be invalid by the standard (you can't use a non-_Atomic variable with the atomic_ methods). Also oddly, C11 atomics doesn't work for me with Clang 3.4.2, because it's trying to use GCC 4.9's stdatomic.h and failing.

In any case, should be fixed now in commit 4b53d0e. Can you confirm?

[marcusva]

Some more issues now with const _Atomic types:

/home/marcus/devel/projects/extern/openal-soft/OpenAL32/alFontsound.c:267:22: error: address argument to atomic operation must be a pointer to non-const _Atomic type ('const _Atomic(struct ALbuffer *) *' invalid)
            buffer = ATOMIC_LOAD(&sound->Buffer);
                     ^
/home/marcus/devel/projects/extern/openal-soft/include/atomic.h:21:38: note: expanded from macro 'ATOMIC_LOAD'
#define ATOMIC_LOAD(_val)            atomic_load(&(_val)->value)
                                     ^
/usr/include/stdatomic.h:362:2: note: expanded from macro 'atomic_load'
        atomic_load_explicit(object, memory_order_seq_cst)
        ^
/usr/include/stdatomic.h:255:2: note: expanded from macro 'atomic_load_explicit'
        __c11_atomic_load(object, order)
        ^
/home/marcus/devel/projects/extern/openal-soft/OpenAL32/alFontsound.c:445:20: error: address argument to atomic operation must be a pointer to non-const _Atomic type ('const _Atomic(struct ALfontsound *) *' invalid)
            link = ATOMIC_LOAD(&sound->Link);
                   ^
/home/marcus/devel/projects/extern/openal-soft/include/atomic.h:21:38: note: expanded from macro 'ATOMIC_LOAD'
#define ATOMIC_LOAD(_val)            atomic_load(&(_val)->value)
                                     ^
/usr/include/stdatomic.h:362:2: note: expanded from macro 'atomic_load'
        atomic_load_explicit(object, memory_order_seq_cst)
        ^
/usr/include/stdatomic.h:255:2: note: expanded from macro 'atomic_load_explicit'
        __c11_atomic_load(object, order)

[kcat]

Seems to be a deficiency in the C11 spec, which they're correcting:
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1807.htm

So there's two options: cast away the const-ness, or only use C atomics when atomic_load accepts const _Atomic variables. I think I'd prefer the latter.

[marcusva]

Agreed. We'll have to wait until the compiler vendors catch up with the change.

[kcat]

Commit 4d19d4f updates the cmake atomic check with a const _Atomic load. You'll probably need to remove CMakeCache.txt to make it properly recheck, but is it good now?

[marcusva]

Sorry for the long response time. Yes, it looks good now.