Opened 5 years ago

Last modified 3 years ago

#58712 assigned defect

macports-clangs > 6.0 are missing atomic builtins on i386

Reported by: devernay (Frédéric Devernay) Owned by: macports-tickets@…
Priority: Normal Milestone:
Component: ports Version:
Keywords: Cc: larryv (Lawrence Velázquez), jeremyhu (Jeremy Huddleston Sequoia)
Port: clang-8.0 clang-9.0 clang-10 clang-devel

Description

the default value of COMPILER_RT_EXCLUDE_ATOMIC_BUILTIN in compiler-rt/lib/builtins/CMakeLists.txt changed from "off" in clang-6.0 to "on" in clang-7.0 and clang-8.0: https://github.com/llvm-mirror/compiler-rt/blob/release_60/lib/builtins/CMakeLists.txt#L177 https://github.com/llvm-mirror/compiler-rt/blob/release_70/lib/builtins/CMakeLists.txt#L177 https://github.com/llvm-mirror/compiler-rt/blob/release_80/lib/builtins/CMakeLists.txt#L177

The reason is probably that the x86_64 compiler now has assembly instructions for these builtins.

However, i386 does not seem to have these, and this causes undefined symbols when compiling for i386 (or universal):

Undefined symbols for architecture i386:
  "___atomic_load", referenced from:
      _.omp_outlined..468 in CImgExpression.o
      _.omp_outlined..608 in CImgExpression.o
ld: symbol(s) not found for architecture i386

When compiling universal, I only get errors from the i386 side.

When checking the lib contents:

nm -arch i386  /Volumes/opt/local-libc++/libexec/llvm-8.0/lib/clang/8.0.0/lib/darwin/libclang_rt.osx.a|fgrep atomic_load
<nothing>
nm -arch i386  /Volumes/opt/local-libc++/libexec/llvm-6.0/lib/clang/6.0.1/lib/darwin/libclang_rt.osx.a |fgrep atomic_load
00000000 T ___atomic_load
000004f0 T ___atomic_load_1
00000510 T ___atomic_load_2
00000530 T ___atomic_load_4
00000550 T ___atomic_load_8

This should be signaled upstream, of course, but it's become complicated to submit llvm bugs.

I'll check if re-enabling this fixes it. After all, it's just one more file (atomic.o) in a static lib, it should not hurt and it just makes the lib a few Kb more fat.

Change History (19)

comment:1 Changed 5 years ago by devernay (Frédéric Devernay)

Here's the commit from Jun 14 2018 that changed that default value.I understand his point (having it in a static lib may cause duplicate symbols when loading two shared objects that have that symbol), but nobody did that shared library after all. https://github.com/llvm-mirror/compiler-rt/commit/e8b47a537f4c849e66e450dadddfbfe03d1f06fd

comment:2 Changed 5 years ago by kencu (Ken)

interesting. I guess with Apple's deprecating i386 it's up to us to fix this for Darwin.

Help me understand better where x86_64 is finding these symbols; maybe we can fix it there?

Otherwise...what shared library could we put it in? The only one I can think of that is always linked in at the moment is libSystem...

Or, for +universal builds, we could put it back in libruntime, but that sorta defeats the point of the fix, I guess...

comment:3 Changed 5 years ago by mf2k (Frank Schima)

Cc: larryv added; jeremyhu removed
Owner: set to jeremyhu
Port: clang-7.0,clang-8.0clang-7.0 clang-8.0
Status: newassigned

comment:4 Changed 5 years ago by kencu (Ken)

A pure i386 build of clang-8.0 does build on the buildbot <http://packages.macports.org/clang-8.0/>.

comment:5 Changed 5 years ago by devernay (Frédéric Devernay)

Yes, it builds, but can you check if that i386 build has the atomic_load functions using

nm -arch i386  /Volumes/opt/local-libc++/libexec/llvm-8.0/lib/clang/8.0.0/lib/darwin/libclang_rt.osx.a|fgrep atomic_load

By the way, I checked and these fixes add the missing symbols. I put it in the "<= 10.6" section: https://github.com/NatronGitHub/Natron/blob/RB-2.3/tools/MacPorts/lang/llvm-7.0/Portfile.patch https://github.com/NatronGitHub/Natron/blob/RB-2.3/tools/MacPorts/lang/llvm-8.0/Portfile.patch

The author of the commit that changes this behavior is right, but he did not provide a proper fix.

comment:6 Changed 5 years ago by kencu (Ken)

comment:7 Changed 4 years ago by kencu (Ken)

It is not super easy to have the clang_rt configure one way for x86_64 and another way for i386, as it is presently built in one go using one cmake configure phase and multiple arch flags. We'd have to -- I guess -- build clang_rt using the muniversal portgroup or something like it, and then configure it differently for each section.

We can of course set up the clang i386 builds to do this, which solves one part of it. But +universal builds will still be missing the i386 symbols I believe.

Looking for inspiration here... I could add them in manually somehow for i386 I guess, or turn off the intrisics completely.

comment:8 Changed 4 years ago by kencu (Ken)

Port: clang-9.0 clang-10 clang-devel added
Summary: clang-7.0 and clang-8.0 are missing atomic builtins on i386macports-clangs > 6.0 are missing atomic builtins on i386

comment:9 Changed 4 years ago by kencu (Ken)

NB:

they cannot be shipped in a statically linked compiler-support library, as they have state which must be shared amongst all DSOs loaded in the program. They must be provided in a shared library used by all objects.

These functions should never have been in a static library, and enabling them to be there is just wrong, so turning off the instrinsics is not the way to go.

Version 0, edited 4 years ago by kencu (Ken) (next)

comment:10 Changed 4 years ago by kencu (Ken)

Noted that libgccN comes with a perfectly wonderful libatomic.dylib that seems to have all the missing functions.

That's a pretty low-hanging fruit.

comment:11 Changed 4 years ago by kencu (Ken)

Cc: jeremyhu added; kencu removed
Owner: changed from jeremyhu to kencu

comment:12 Changed 4 years ago by kencu (Ken)

I think we need to do something here. This i386 atomics issue does keep coming around. Upstream llvm turned this off because for technical reasons it is better implemented as a dylib than as a linked in static implementation.

A number of other projects have run into this, and they have (for the most part) just put the atomics back into the static compiler_rt.a library (where it used to be). That is easy -- toggle the option, which still exists, as above.

Otherwise we can -- require an appropriate version of libgcc at runtime and either manually or automatically add the link library to libatomic.1.dylib. Or - find some other, non-gcc implementation of libatomic.dylib somewhere that we want to use instead of the one in libgcc, and somehow have it work in with our clang libraries.

I'm leaning towards just toggling it back on. If we do that, with the x64_64 and arm64 builds be affected (ie will they use the older, non-favoured static lib calls) or will they continue to use the new intrinsic calls and be unaffected? If the 64 bit systems are unaffected, I would suggest we just toggle the atomics back on for i386 and be done with it.

comment:14 Changed 4 years ago by devernay (Frédéric Devernay)

If the compiler emits native instructions for atomics, the linker won't use the definitions in libclang_rt.osx.a, so setting COMPILER_RT_EXCLUDE_ATOMIC_BUILTIN=off should do any bad, isn't it?

comment:15 Changed 4 years ago by kencu (Ken)

I think you're 100% right there.

comment:16 Changed 4 years ago by kencu (Ken)

as usual

comment:17 Changed 3 years ago by ken-cunningham-webuse

In c53a09036eb6c71a52055965b4d300441f22da5a/macports-ports (master):

clang-7.0: allow compiler-rt to build atomic builtins

these were previously on by default. We need atomics for
32bit atomic support. 64bit atomics are supported by
the compiler/processor but 32bit are not.

see: #58712

comment:18 Changed 3 years ago by kencu (Ken)

Port: clang-7.0 removed

comment:19 Changed 3 years ago by kencu (Ken)

Owner: changed from kencu to macports-tickets@…
Note: See TracTickets for help on using tickets.