[compiler-rt] [builtins] Don't build __atomic_load etc. by default.

[efriedma]

Summary

The locks need to be implemented in a unique shared library to work correctly, so they shouldn't be part of libclang_rt.builtins.a, except in specialized scenarios without any dynamically linked code.

Not sure how this hasn't led to any bug reports yet.

[jfb]

Wow this is amazing!

[devernay]

I ran into an issue because of this commit, but only on i386/OS X 10.6 (undefined symbol ___atomic_load). x86_64 goes fine.
I agree that it should be in a shared lib, so why not move it to a shared lib?

[ldionne]

Does this mean that we're never actually including the atomic builtins in any of the Clang releases?

I'm trying to investigate what the state of things is because right now, code like the following doesn't work out-of-the-box on Clang/Linux nor on AppleClang/macOS:

cat <<EOF | clang++ -xc++ - 
#include <atomic>
struct Large { int storage[100]; };
std::atomic<Large> x;
int main(int, char**) { return x.is_lock_free(); }
EOF

/tmp/--754274.o: In function `std::atomic<Large>::is_lock_free() const':
-:(.text._ZNKSt6atomicI5LargeE12is_lock_freeEv[_ZNKSt6atomicI5LargeE12is_lock_freeEv]+0x19): undefined reference to `__atomic_is_lock_free'

Furthermore, none of the libclang_rt.xxx.yyy.so or libclang_rt.xxx.yyy.a libraries shipped alongside Clang appear to contain __atomic_is_lock_free. Where is the implementation expected to get it from?

[ldionne]

In other words, it appears that this commit turned a "code behaves incorrectly at runtime" error into a "code won't link" error. I guess it's somewhat better... but we should really figure out how to both provide the functionality *and* have it behave correctly.

[efriedma]

On Linux, the operating system ships with libatomic. I think it should be getting linked in by default? If it isn't, that a problem with the link command.

Not sure what the situation is like on other operating systems.

[ldionne]

In D47606#2880596, @efriedma wrote:

On Linux, the operating system ships with libatomic. I think it should be getting linked in by default? If it isn't, that a problem with the link command.

No, it's not linked by default, you need to add -latomic (but that's another issue, as you say). So does that mean the intent on Linux is for Clang to use the OS-shipped /usr/lib/<triple>/libatomic.so instead of its own libclang_rt?

Not sure what the situation is like on other operating systems.

On Darwin, we don't ship libatomic.so anywhere. It seems like we also don't ship the atomic builtins as part of <xcode-toolchain>/usr/lib/clang/<version>/lib/darwin/libclang_rt.osx.a, which is where I would normally expect those (I think?).

So IIUC, there would be two bugs here:

1. Clang doesn't automatically add -latomic when linking on Linux. Note that GCC behaves the same, so it is possible that I'll find out there's a good reason for it.
2. We should ship the atomic builtins in some form on Darwin, and make sure those are picked up by the driver.

Do you agree?

[efriedma]

In D47606#2880804, @ldionne wrote:

In D47606#2880596, @efriedma wrote:

On Linux, the operating system ships with libatomic. I think it should be getting linked in by default? If it isn't, that a problem with the link command.

No, it's not linked by default, you need to add -latomic (but that's another issue, as you say). So does that mean the intent on Linux is for Clang to use the OS-shipped /usr/lib/<triple>/libatomic.so instead of its own libclang_rt?

Yes. This is necessary to be ABI-compatible with gcc.

Not sure what the situation is like on other operating systems.

On Darwin, we don't ship libatomic.so anywhere. It seems like we also don't ship the atomic builtins as part of <xcode-toolchain>/usr/lib/clang/<version>/lib/darwin/libclang_rt.osx.a, which is where I would normally expect those (I think?).

So IIUC, there would be two bugs here:

1. Clang doesn't automatically add -latomic when linking on Linux. Note that GCC behaves the same, so it is possible that I'll find out there's a good reason for it.
2. We should ship the atomic builtins in some form on Darwin, and make sure those are picked up by the driver.

Do you agree?

Yes, that sounds right.

[ldionne]

In D47606#2881161, @efriedma wrote:

In D47606#2880804, @ldionne wrote:

On Darwin, we don't ship libatomic.so anywhere. It seems like we also don't ship the atomic builtins as part of <xcode-toolchain>/usr/lib/clang/<version>/lib/darwin/libclang_rt.osx.a, which is where I would normally expect those (I think?).

So IIUC, there would be two bugs here:

1. Clang doesn't automatically add -latomic when linking on Linux. Note that GCC behaves the same, so it is possible that I'll find out there's a good reason for it.
2. We should ship the atomic builtins in some form on Darwin, and make sure those are picked up by the driver.

Do you agree?

Yes, that sounds right.

Just to circle back on this, it looks like we do ship compiler-rt on Darwin, and we do ship the atomic builtins. It's just that we didn't ship *all* of them because our libraries were a bit outdated. I'll fix that, but I think that's all that needs fixing on Apple platforms at least.

lib/builtins/CMakeLists.txt

set(GENERIC_TF_SOURCES
   floatunditf.c
   floatunsitf.c
   floatuntitf.c
   multc3.c
   trunctfdf2.c
   trunctfsf2.c)
 
 option(COMPILER_RT_EXCLUDE_ATOMIC_BUILTIN
-  "Skip the atomic builtin (this may be needed if system headers are unavailable)"
-  Off)
+  "Skip the atomic builtin (these should normally be provided by a shared library)"
+  On)
 
 if(NOT FUCHSIA AND NOT COMPILER_RT_BAREMETAL_BUILD)
   set(GENERIC_SOURCES
     ${GENERIC_SOURCES}
     emutls.c 
     enable_execute_stack.c
     eprintf.c)
 endif()