[cfe-dev] [PATCH] libc++: help diagnosing the following std::atomic compile error

[Richard Smith]

On Mon, May 20, 2013 at 5:55 PM, Howard Hinnant <hhinnant at apple.com> wrote:

> Please review:
>
> I've enclosed a proposed patch for fixing <atomic>.  This patch changes us
> from using _Atomic(T) for the stored type, to just T, but aligned to
> _Atomic(T).  It also switches from the __c11_atomic intrinsics to the gcc
> __atomic intrinsics.  This change exposed several silly bugs in the test
> suite.  But most importantly it is meant to allow std::atomic to be
> instantiated with trivially copyable class types.
>
> The enclosed patch passes all (modified) tests on Apple's Mountain Lion
> except one:
>
> passed 1 tests in /Users/hhinnant/Development/temp_libcxx/test/atomics
> passed 2 tests in
> /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.fences
> passed 11 tests in
> /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.flag
> passed 1 tests in
> /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.general
> passed 1 tests in
> /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.lockfree
> passed 2 tests in
> /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.order
> passed 1 tests in
> /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.syn
> passed 5 tests in
> /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.generic
> passed 1 tests in
> /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations
> passed 1 tests in
> /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.arith
> passed 1 tests in
> /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.general
> passed 1 tests in
> /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.pointer
> Undefined symbols for architecture x86_64:
>   "___atomic_is_lock_free", referenced from:
>       void test<A>() in atomic_is_lock_free-cA4Y5d.o
> ld: symbol(s) not found for architecture x86_64
> clang: error: linker command failed with exit code 1 (use -v to see
> invocation)
> /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.req/atomic_is_lock_free.pass.cpp
> failed to compile
> Compile line was: xcrun clang++ -stdlib=libc++ -std=c++11
> -I/Users/hhinnant/Development/temp_libcxx/test/support
> -I/Users/hhinnant/Development/temp_libcxx/include
> -L/Users/hhinnant/Development/temp_libcxx/lib -D_LIBCPP_STD_VER=13
> -I/Users/hhinnant/Development/temp_libcxx/test/support
> -I/Users/hhinnant/Development/temp_libcxx/include
> -L/Users/hhinnant/Development/temp_libcxx/lib atomic_is_lock_free.pass.cpp
> failed 1 tests in
> /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.req
> passed 22 tests in
> /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.req
> passed 1 tests in
> /Users/hhinnant/Development/temp_libcxx/test/atomics/atomics.types.operations/atomics.types.operations.templ
> ****************************************************
> Results for /Users/hhinnant/Development/temp_libcxx/test/atomics:
> using Apple LLVM version 4.2 (clang-425.1.16) (based on LLVM 3.2svn)
> Target: x86_64-apple-darwin12.3.0
> Thread model: posix
> with -stdlib=libc++ -std=c++11
> -I/Users/hhinnant/Development/temp_libcxx/test/support
> -I/Users/hhinnant/Development/temp_libcxx/include
> -L/Users/hhinnant/Development/temp_libcxx/lib -D_LIBCPP_STD_VER=13
> -I/Users/hhinnant/Development/temp_libcxx/test/support
> -I/Users/hhinnant/Development/temp_libcxx/include
> -L/Users/hhinnant/Development/temp_libcxx/lib
> ----------------------------------------------------
> sections without tests   : 0
> sections with failures   : 1
> sections without failures: 13
>                        +   ----
> total number of sections : 14
> ----------------------------------------------------
> number of tests failed   : 1
> number of tests passed   : 51
>                        +   ----
> total number of tests    : 52
> ****************************************************
>
> Comments welcomed.  We need more clang expertise here than libc++
> expertise.


The patch looks basically right. However, since you get the alignment of
the inner object correct, you should pass 0 as the second parameter to
__atomic_is_lock_free (that should remove your one remaining link error).
Also, your placement-new expressions aren't robust against an overloaded
operator& on _Tp.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20130520/00526260/attachment.html>