atomic_init() and friends failing on clang

changed milestone to %Tor: 0.3.5.x-final

added 032-backport 033-backport 034-backport 035-must actualpoints::.2 component::core tor/tor jenkins milestone::Tor: 0.3.5.x-final owner::nickm priority::high regression resolution::implemented reviewer::mikeperry severity::normal status::closed type::defect labels

e.g.

23:02:37 In file included from src/lib/compress/compress.c:36:
23:02:37 ./src/lib/thread/threads.h:128:3: error: address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(unsigned int) *' invalid)
23:02:37   atomic_init(&counter->val, 0);
23:02:37   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
23:02:37 /usr/bin/../lib/gcc/i686-linux-gnu/7.3.0/include/stdatomic.h:83:3: note: expanded from macro 'atomic_init'
23:02:37   atomic_store_explicit (PTR, VAL, __ATOMIC_RELAXED)
23:02:37   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
23:02:37 /usr/bin/../lib/gcc/i686-linux-gnu/7.3.0/include/stdatomic.h:126:5: note: expanded from macro 'atomic_store_explicit'
23:02:37     __atomic_store (__atomic_store_ptr, &__atomic_store_tmp, (MO));     \
23:02:37     ^               ~~~~~~~~~~~~~~~~~~
23:02:37 In file included from src/lib/compress/compress.c:36:
23:02:37 ./src/lib/thread/threads.h:140:10: error: address argument to atomic operation must be a pointer to integer or pointer ('atomic_size_t *' (aka '_Atomic(unsigned int) *') invalid)
23:02:37   (void) atomic_fetch_add(&counter->val, add);
23:02:37          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
23:02:37 /usr/bin/../lib/gcc/i686-linux-gnu/7.3.0/include/stdatomic.h:192:36: note: expanded from macro 'atomic_fetch_add'
23:02:37 #define atomic_fetch_add(PTR, VAL) __atomic_fetch_add ((PTR), (VAL),    \
23:02:37                                    ^                   ~~~~~
23:02:37 In file included from src/lib/compress/compress.c:36:
23:02:37 ./src/lib/thread/threads.h:146:10: error: address argument to atomic operation must be a pointer to integer or pointer ('atomic_size_t *' (aka '_Atomic(unsigned int) *') invalid)
23:02:37   (void) atomic_fetch_sub(&counter->val, sub);
23:02:37          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
23:02:37 /usr/bin/../lib/gcc/i686-linux-gnu/7.3.0/include/stdatomic.h:197:36: note: expanded from macro 'atomic_fetch_sub'
23:02:37 #define atomic_fetch_sub(PTR, VAL) __atomic_fetch_sub ((PTR), (VAL),    \
23:02:37                                    ^                   ~~~~~
23:02:37 In file included from src/lib/compress/compress.c:36:
23:02:37 ./src/lib/thread/threads.h:152:10: error: address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(unsigned int) *' invalid)
23:02:37   return atomic_load(&counter->val);
23:02:37          ^~~~~~~~~~~~~~~~~~~~~~~~~~
23:02:37 /usr/bin/../lib/gcc/i686-linux-gnu/7.3.0/include/stdatomic.h:142:27: note: expanded from macro 'atomic_load'
23:02:37 #define atomic_load(PTR)  atomic_load_explicit (PTR, __ATOMIC_SEQ_CST)
23:02:37                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
23:02:37 /usr/bin/../lib/gcc/i686-linux-gnu/7.3.0/include/stdatomic.h:138:5: note: expanded from macro 'atomic_load_explicit'
23:02:37     __atomic_load (__atomic_load_ptr, &__atomic_load_tmp, (MO));        \
23:02:37     ^              ~~~~~~~~~~~~~~~~~
23:02:37 In file included from src/lib/compress/compress.c:36:
23:02:37 ./src/lib/thread/threads.h:158:10: error: address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(unsigned int) *' invalid)
23:02:37   return atomic_exchange(&counter->val, newval);
23:02:37          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
23:02:37 /usr/bin/../lib/gcc/i686-linux-gnu/7.3.0/include/stdatomic.h:157:3: note: expanded from macro 'atomic_exchange'
23:02:37   atomic_exchange_explicit (PTR, VAL, __ATOMIC_SEQ_CST)
23:02:37   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
23:02:37 /usr/bin/../lib/gcc/i686-linux-gnu/7.3.0/include/stdatomic.h:151:5: note: expanded from macro 'atomic_exchange_explicit'
23:02:37     __atomic_exchange (__atomic_exchange_ptr, &__atomic_exchange_val,   \
23:02:37     ^                  ~~~~~~~~~~~~~~~~~~~~~
23:02:37 5 errors generated.
23:02:37 Makefile:8039: recipe for target 'src/lib/compress/compress.o' failed
23:02:37 make[1]: *** [src/lib/compress/compress.o] Error 1

atomic_init() and friends are part of C11; they're declared in stdatomic.h.

It looks like this is maybe 32-bit clang only? Or maybe only some versions of clang? I ask because clang on Travis doesn't object to this code.

And for that matter, I think 32-bit clang didn't object to this code a while ago: these tests used to pass on Jenkins, until a couple of weeks ago: https://jenkins.torproject.org/job/tor-ci-linux-0.3.3-clang/ . We've been using stdatomic.h for much longer than that.

Looking at the debian packages involved, it seems that sid got some new clang packages around Jul 8 maybe, which fits with the timeline.

Trac:
Milestone: Tor: 0.3.3.x-final to Tor: 0.3.5.x-final
Priority: Medium to High
Keywords: N/A deleted, 033-backport, 034-backport, regression, 035-must, 032-backport added

Here is a minimal C file that demonstrates the issue:

#include <stdatomic.h>

void increment(atomic_size_t *arg) {
	atomic_fetch_add(arg, 1);
}

Leading theory after more investigation with weasel: clang is looking at the version of stdatomic.h in /usr/lib/gcc (see error messages above), but it shouldn't be. It wants the one in /usr/lib/llvm-6.0.

I filed https://bugs.debian.org/903709.

I don't see much action on that ticket, so it looks like we should add a workaround.

Branch bug26779_033 should merge into 0.3.3 and 0.3.4 cleanly; branch bug26779_035 merges it into 0.3.5.

See PR at https://github.com/torproject/tor/pull/269 for the 033 version.

Trac:
Status: assigned to needs_review

Trac:
Reviewer: N/A to mikeperry

Ok this seems plausible to me that it detects a working stdatomic, and if not, falls back to our previous mutex-based functions.

I'm setting this to merge ready, but with the caveat that I'm not the best person to review this to ensure that all the stdatomic stuff is sane. But the ifdef changes check out. The 033 branch does appear to replace all usage of HAVE_STDATOMIC_H with HAVE_WORKING_STDATOMIC.

Trac:
Status: needs_review to merge_ready

Merged to 0.2.9 and forward; let's see what CI thinks of it.

Trac:
Resolution: N/A to implemented
Status: merge_ready to closed

Trac:
Actualpoints: N/A to .2

closed

added 1h 36m of time spent

moved to tpo/core/tor#26779 (closed)

atomic_init() and friends failing on clang

Child items ...

Activity