[libc++] Add missing XFAIL to tests that need __atomic_* libcalls

[arichardson]

Summary

FreeBSD did not provide the __atomic_* functions as part of the base
system until recently. They were added to libgcc_s in SVN revision r364753
(August 2020), so check for availability of 'non-lockfree-atomics' so that
these tests do not fail unexpectedly on older versions of FreeBSD.

This also removes the #ifndef APPLE from atomic_helpers.h that was used
to work around lack of atomic runtime functions on older Apple platforms
and replaces it with XFAIL: !non-lockfree-atomics.

[emaste]

fine with me

[ldionne]

What is the error that you're seeing? How are you running the test suite?

It looks like you are running against the system library, right? If so, we already have a feature to handle these sorts of XFAILs, it's with_system_cxx_lib=<system>. It's only used by macOS for the time being, but it can (and should) be used by other systems as well.

[arichardson]

In D88818#2328026, @ldionne wrote:

What is the error that you're seeing? How are you running the test suite?

It looks like you are running against the system library, right? If so, we already have a feature to handle these sorts of XFAILs, it's with_system_cxx_lib=<system>. It's only used by macOS for the time being, but it can (and should) be used by other systems as well.

I am running the tests against a just-build libc++: mkdir -p libcxx/build && cd libcxx/build && cmake -GNinja .. && ninja check-cxx
Most of the atomics tests need libatomic since they perform large atomic operations that are not natively supported and therefore call atomic_store/atomic_load, etc. This works on Linux since LinuxLocalTI adds -latomic to the linker flags but FreeBSD does not ship a libatomic.
As of https://reviews.freebsd.org/rS364753 the __atomic atomic functions are included in libcompiler-rt.a and libgcc_s.so.

Here is an example of a failing test with a full command line:

FreeBSD clang version 8.0.1 (tags/RELEASE_801/final 366581) (based on LLVM 8.0.1)
Target: x86_64-unknown-freebsd12
Thread model: posix
InstalledDir: /usr/bin
 "/usr/bin/c++" -cc1 -triple x86_64-unknown-freebsd12 -emit-obj -mrelax-all -disable-free -disable-llvm-verifier -discard-value-names -main-file-name atomic_load_explicit.pass.cpp -mrelocation-model static -mthread-model posix -mdisable-fp-elim -masm-verbose -mconstructor-aliases -munwind-tables -fuse-init-array -target-cpu x86-64 -dwarf-column-info -debugger-tuning=gdb -v -nostdinc++ -resource-dir /usr/lib/clang/8.0.1 -include /exports/users/alr48/sources/upstream-llvm-project/libcxx/build/__config_site -include /exports/users/alr48/sources/upstream-llvm-project/libcxx/test/support/nasty_macros.h -I /exports/users/alr48/sources/upstream-llvm-project/libcxx/include -I /exports/users/alr48/sources/upstream-llvm-project/libcxx/build/include/c++build -I /exports/users/alr48/sources/upstream-llvm-project/libcxx/test/support -D _LIBCPP_DISABLE_AVAILABILITY -D _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -Werror -Wall -Wextra -Wshadow -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-user-defined-literals -Wno-noexcept-type -Wno-atomic-alignment -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -Werror=thread-safety -Wuser-defined-warnings -std=c++2a -fdeprecated-macro -fdebug-compilation-dir /exports/users/alr48/sources/upstream-llvm-project/libcxx/build/test/std/atomics/atomics.types.operations/atomics.types.operations.req -ferror-limit 19 -fmessage-length 0 -fcoroutines-ts -fobjc-runtime=gnustep -fcxx-exceptions -fexceptions -fdiagnostics-show-option -o /tmp/atomic_load_explicit-edb87d.o -x c++ /exports/users/alr48/sources/upstream-llvm-project/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_load_explicit.pass.cpp -faddrsig
clang -cc1 version 8.0.1 based upon LLVM 8.0.1 default target x86_64-unknown-freebsd12.1
#include "..." search starts here:
#include <...> search starts here:
 /exports/users/alr48/sources/upstream-llvm-project/libcxx/include
 /exports/users/alr48/sources/upstream-llvm-project/libcxx/build/include/c++build
 /exports/users/alr48/sources/upstream-llvm-project/libcxx/test/support
 /usr/lib/clang/8.0.1/include
 /usr/include
End of search list.
 "/usr/bin/ld" --eh-frame-hdr -dynamic-linker /libexec/ld-elf.so.1 --hash-style=both --enable-new-dtags -o /exports/users/alr48/sources/upstream-llvm-project/libcxx/build/test/std/atomics/atomics.types.operations/atomics.types.operations.req/Output/atomic_load_explicit.pass.cpp.dir/t.tmp.exe /usr/lib/crt1.o /usr/lib/crti.o /usr/lib/crtbegin.o -L/exports/users/alr48/sources/upstream-llvm-project/libcxx/build/lib -L/usr/lib /tmp/atomic_load_explicit-edb87d.o -rpath /exports/users/alr48/sources/upstream-llvm-project/libcxx/build/lib -lc++ -lc -lm -lpthread -lgcc_s -lcxxrt -lc++experimental /usr/lib/crtend.o /usr/lib/crtn.o
ld: error: undefined symbol: __atomic_load
>>> referenced by atomic_load_explicit.pass.cpp
>>>               /tmp/atomic_load_explicit-edb87d.o:(LargeUserAtomicType std::__1::__cxx_atomic_load<LargeUserAtomicType>(std::__1::__cxx_atomic_base_impl<LargeUserAtomicType> const*, std::__1::memory_order))

ld: error: undefined symbol: __atomic_load
>>> referenced by atomic_load_explicit.pass.cpp
>>>               /tmp/atomic_load_explicit-edb87d.o:(LargeUserAtomicType std::__1::__cxx_atomic_load<LargeUserAtomicType>(std::__1::__cxx_atomic_base_impl<LargeUserAtomicType> const volatile*, std::__1::memory_order))
c++: error: linker command failed with exit code 1 (use -v to see invocation)

As you can see here, there is a -lgcc_s in the linker command, so it will start working on a newer FreeBSD version.

[arichardson]

On macOS these functions appear to be provided by /usr/lib/system/libcompiler_rt.dylib, which I believe is automatically added when you link -lSystem.

[ldionne]

libcxx/utils/libcxx/test/features.py

122 ↗

(On Diff #296145)

This check shouldn't be FreeBSD specific. Instead, we should see whether we can reference these atomic operations generically, by checking the linker output of an appropriate program.

Also, can you explain why the non-lockfree-atomics feature doesn't do what you want?

[arichardson]

Will update to use non-lockfree-atomics

libcxx/utils/libcxx/test/features.py

122 ↗

(On Diff #296145)

I did not see the "non-lockfree-atomic" feature. That should do the same thing by checking whether the source compiles rather than doing it indirectly using a FreeBSD version check.

[arichardson]

Use non-lockfree-atomics instead.

This changes the FreeBSD test results (using system clang, FreeBSD clang version 8.0.1 (tags/RELEASE_801/final 366581) (based on LLVM 8.0.1) from

Unsupported        :  199
Passed             : 6164
Expectedly Failed  :   48
Failed             :   48
Unexpectedly Passed:    2

to

Unsupported        :  199
Passed             : 6164
Expectedly Failed  :   61
Failed             :   35
Unexpectedly Passed:    2

[ldionne]

So... do I understand correctly that you're only seeing issues on older FreeBSDs where libcompiler_rt does not contain __atomic_load & friends? Because macOS is in the same boat as FreeBSD: it doesn't ship libatomic.a (hence the need for the non-lockfree-atomics Lit feature).

You might also want to take a look at libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_helpers.h -- we seem to exclude large types from some tests on __APPLE__ -- would also excluding them from __FreeBsd__ solve your issues?

Requesting changes since this is breaking CI -- I'm not sure yet what we'll have to change, but we'll have to change something.

libcxx/utils/libcxx/test/features.py

43 ↗

(On Diff #304141)

The non-lockfree-atomics feature is defined here, but I assume you found it?

[arichardson]

Rebase on top of D91911 and remove the #ifndef APPLE special case from atomic_helpers.

The CI was failing because the non-lockfree-atomics was being detected as false on Apple platforms due to missing __atomic_is_lockfree even though the other ones were available.

[arichardson]

Rebase for CI

[arichardson]

CI seems happy now. Does this + D91911 look reasonable @ldionne?

[ldionne]

This technically regresses our coverage of atomics on Apple platforms, since those tests are testing both small and large types. I'm OK with that, but I'll make it a priority to investigate shipping support for non-lockfree atomics on Apple platforms.

[arichardson]

In D88818#2425777, @ldionne wrote:

This technically regresses our coverage of atomics on Apple platforms, since those tests are testing both small and large types. I'm OK with that, but I'll make it a priority to investigate shipping support for non-lockfree atomics on Apple platforms.

With these patches the tests still run on my macOS 10.15 laptop, it's just libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_is_lock_free.pass.cpp that no longer runs. In some ways this is now testing more code on Apple platforms since the TestFunctor<LargeUserAtomicType> is no longer ifdef'd away.

[arichardson]

Will commit this once D92302 has landed.

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_strong.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: libcpp-has-no-threads
+// XFAIL: !non-lockfree-atomics
 //  ... assertion fails line 34
 
 // <atomic>
 
 // template <class T>
 //     bool
 //     atomic_compare_exchange_strong(volatile atomic<T>* obj, T* expc, T desr);
 //

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_strong_explicit.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: libcpp-has-no-threads
+// XFAIL: !non-lockfree-atomics
 //  ... assertion fails line 38
 
 // <atomic>
 
 // template <class T>
 //     bool
 //     atomic_compare_exchange_strong_explicit(volatile atomic<T>* obj, T* expc,
 //                                           T desr,

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_weak.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: libcpp-has-no-threads
+// XFAIL: !non-lockfree-atomics
 //  ... assertion fails line 34
 
 // <atomic>
 
 // template <class T>
 //     bool
 //     atomic_compare_exchange_weak(volatile atomic<T>* obj, T* expc, T desr);
 //

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_compare_exchange_weak_explicit.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: libcpp-has-no-threads
+// XFAIL: !non-lockfree-atomics
 //  ... assertion fails line 38
 
 // <atomic>
 
 // template <class T>
 //     bool
 //     atomic_compare_exchange_weak_explicit(volatile atomic<T>* obj, T* expc,
 //                                           T desr,

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_exchange.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: libcpp-has-no-threads
+// XFAIL: !non-lockfree-atomics
 //  ... fails assertion line 31
 
 // <atomic>
 
 // template <class T>
 //     T
 //     atomic_exchange(volatile atomic<T>* obj, T desr);
 //

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_exchange_explicit.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: libcpp-has-no-threads
+// XFAIL: !non-lockfree-atomics
 //  ... assertion fails line 32
 
 // <atomic>
 
 // template <class T>
 //     T
 //     atomic_exchange_explicit(volatile atomic<T>* obj, T desr, memory_order m);
 //

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_helpers.h

#endif
     }
 };
 
 template < template <class TestArg> class TestFunctor >
 struct TestEachAtomicType {
     void operator()() const {
         TestEachIntegralType<TestFunctor>()();
         TestFunctor<UserAtomicType>()();
-#ifndef __APPLE__
         /*
-            These aren't going to be lock-free,
-            so some libatomic.a is necessary.
+            Note: These aren't going to be lock-free,
+            so some libatomic.a is necessary. To handle
+            the case where the support functions are
+            missing, all tests that use this file should add:
+            XFAIL: !non-lockfree-atomics
         */
         TestFunctor<LargeUserAtomicType>()();
-#endif
 /*
     Enable these once we have P0528 
     
         TestFunctor<PaddedUserAtomicType>()();
         TestFunctor<WeirdUserAtomicType>()();
 */
         TestFunctor<int*>()();
         TestFunctor<const int*>()();
         TestFunctor<float>()();
         TestFunctor<double>()();
     }
 };
 
 
 #endif // ATOMIC_HELPER_H

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_init.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: libcpp-has-no-threads
+// XFAIL: !non-lockfree-atomics
 //  ... assertion fails line 36
 
 // <atomic>
 
 // template <class T>
 //     void
 //     atomic_init(volatile atomic<T>* obj, T desr);
 //

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_is_lock_free.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: libcpp-has-no-threads
+// XFAIL: !is-lockfree-runtime-function
 
 // <atomic>
 
 // template <class T>
 //     bool
 //     atomic_is_lock_free(const volatile atomic<T>* obj);
 //
 // template <class T>

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_load.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: libcpp-has-no-threads
+// XFAIL: !non-lockfree-atomics
 //  ... assertion fails line 35
 
 // <atomic>
 
 // template <class T>
 //     T
 //     atomic_load(const volatile atomic<T>* obj);
 //

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_load_explicit.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: libcpp-has-no-threads
+// XFAIL: !non-lockfree-atomics
 //  ... assertion fails line 31
 
 // <atomic>
 
 // template <class T>
 //     T
 //     atomic_load_explicit(const volatile atomic<T>* obj, memory_order m);
 //

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_store.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: libcpp-has-no-threads
+// XFAIL: !non-lockfree-atomics
 
 // <atomic>
 
 // template <class T>
 //     void
 //     atomic_store(volatile atomic<T>* obj, T desr);
 //
 // template <class T>

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_store_explicit.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: libcpp-has-no-threads
+// XFAIL: !non-lockfree-atomics
 
 // <atomic>
 
 // template <class T>
 //     void
 //     atomic_store_explicit(volatile atomic<T>* obj, T desr, memory_order m);
 //
 // template <class T>

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_wait.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: libcpp-has-no-threads
 // XFAIL: c++03
+// XFAIL: !non-lockfree-atomics
 
 // This test requires the dylib support introduced in D68480,
 // which hasn't shipped yet.
 // XFAIL: with_system_cxx_lib=macosx
 // XFAIL: with_system_cxx_lib=macosx10.15
 // XFAIL: with_system_cxx_lib=macosx10.14
 // XFAIL: with_system_cxx_lib=macosx10.13
 // XFAIL: with_system_cxx_lib=macosx10.12