Opened 7 days ago

Last modified 6 days ago

#26779 assigned defect

atomic_init() and friends failing on clang

Reported by: mikeperry Owned by: nickm
Priority: High Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: jenkins, regression, 032-backport, 033-backport, 034-backport, 035-must
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Clang does not like how we're using atomic_init() and friends in src/lib/thread/threads.h. It looks like these changes were introduced by nickm. I'm not finding a lot of great documentation on these functions, though.

https://jenkins.torproject.org/job/tor-ci-linux-master-clang/3164/ARCHITECTURE=i386,SUITE=sid/consoleFulll
https://jenkins.torproject.org/view/Failed+Unstable/job/tor-ci-linux-0.3.3-clang/lastFailedBuild/ARCHITECTURE=i386,SUITE=sid/console

0.3.4 also, but lost the url in the jenkins maze.

Child Tickets

Change History (7)

comment:1 Changed 7 days ago by weasel

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

comment:2 Changed 6 days ago by nickm

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.

comment:3 Changed 6 days ago by nickm

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

comment:4 Changed 6 days ago by nickm

Keywords: regression 032-backport 033-backport 034-backport 035-must added
Milestone: Tor: 0.3.3.x-finalTor: 0.3.5.x-final
Priority: MediumHigh

comment:5 Changed 6 days ago by nickm

Here is a minimal C file that demonstrates the issue:

#include <stdatomic.h>

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

comment:6 Changed 6 days ago by nickm

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.

Last edited 6 days ago by weasel (previous) (diff)

comment:7 Changed 6 days ago by weasel

Last edited 6 days ago by weasel (previous) (diff)
Note: See TracTickets for help on using tickets.