Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build fails without -latomic on Clang 8.0 #5865

Closed
mqudsi opened this issue May 9, 2019 · 7 comments
Closed

Build fails without -latomic on Clang 8.0 #5865

mqudsi opened this issue May 9, 2019 · 7 comments
Assignees
Labels
Milestone

Comments

@mqudsi
Copy link
Contributor

mqudsi commented May 9, 2019

master doesn't compile under Clang 8.0 (Ubuntu 18.04.1 LTS) due to missing __atomic_* symbols such as __atomic_load and __atomic_store, which are provided by the GCC libatomic.

[6/7] Linking CXX executable fish                                                                                                                                FAILED: fish
: && /usr/local/bin/sc++  -march=native -msse4a -mavx2 -mfma -msha -fdiagnostics-color=always -O2 -g  -rdynamic CMakeFiles/fish.dir/src/fish.cpp.o  -o fish  libfishlib.a -lcurses -pthread -ldl -lpcre2-32 && :
/usr/local/bin/ld: libfishlib.a(wutil.cpp.o): in function `safe_strerror(int)':
/mnt/d/rand/fish/build/../src/wutil.cpp:334: warning: `sys_errlist' is deprecated; use `strerror' or `strerror_r' instead
/usr/local/bin/ld: /mnt/d/rand/fish/build/../src/wutil.cpp:334: warning: `sys_nerr' is deprecated; use `strerror' or `strerror_r' instead
/usr/local/bin/ld: libfishlib.a(common.cpp.o): in function `std::atomic<winsize>::load(std::memory_order) const':
/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/atomic:250: undefined reference to `__atomic_load'
/usr/local/bin/ld: /usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/atomic:250: undefined reference to `__atomic_load'
/usr/local/bin/ld: /usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/atomic:250: undefined reference to `__atomic_load'
/usr/local/bin/ld: libfishlib.a(common.cpp.o): in function `std::atomic<winsize>::store(winsize, std::memory_order)':
/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/atomic:239: undefined reference to `__atomic_store'
clang: error: linker command failed with exit code 1 (use -v to see invocation)

The following hack fixes the build:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 0179915f..192a12f3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -143,7 +143,7 @@ ADD_LIBRARY(fishlib STATIC ${FISH_SRCS})
 TARGET_SOURCES(fishlib PRIVATE ${FISH_HEADERS})
 TARGET_LINK_LIBRARIES(fishlib
   ${CURSES_LIBRARY} ${CURSES_EXTRA_LIBRARY} Threads::Threads ${CMAKE_DL_LIBS}
-  ${PCRE2_LIB} ${Intl_LIBRARIES})
+  ${PCRE2_LIB} ${Intl_LIBRARIES} "atomic")
 
 # Define fish.
 ADD_EXECUTABLE(fish src/fish.cpp)

I'm not sure what the correct way of testing for -latomic and using it is with CMake.

Build logs:
CMakeError.log
CMakeOutput.log

@zanchey zanchey added the cmake label May 9, 2019
@zanchey zanchey added this to the fish 3.1.0 milestone May 9, 2019
@zanchey
Copy link
Member

zanchey commented May 9, 2019

Is sc++ some custom compiler?

The right thing to do is to make a CheckAtomic function, similar to https://github.com/llvm-mirror/llvm/blob/master/cmake/modules/CheckAtomic.cmake

@mqudsi
Copy link
Contributor Author

mqudsi commented May 10, 2019

No, it's just exec ccache c++ to speed up compilation, where c++ is pointing at clang++-8.

ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue May 12, 2019
ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue May 12, 2019
ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue May 12, 2019
ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue May 12, 2019
ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue May 12, 2019
ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue May 12, 2019
atomic<winsize> requires linking libatomic on some platforms which is
annoying. Remove the one use.

Fixes fish-shell#5865
@zanchey zanchey reopened this Dec 17, 2019
@zanchey
Copy link
Member

zanchey commented Dec 17, 2019

This is still an issue as of 8bf9f52, though only on armv7l:

/usr/bin/ld: libfishlib.a(history.cpp.o): in function `history_t::add_pending_with_file_detection(std::__cxx11::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> > const&, std::__cxx11::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> > const&)':
/usr/include/c++/8/bits/atomic_base.h:514: undefined reference to `__atomic_fetch_add_8'
/usr/bin/ld: libfishlib.a(proc.cpp.o): in function `internal_proc_t::internal_proc_t()':
/usr/include/c++/8/bits/atomic_base.h:296: undefined reference to `__atomic_fetch_add_8'
/usr/bin/ld: libfishlib.a(reader.cpp.o): in function `reader_run_count()':
/usr/include/c++/8/bits/atomic_base.h:396: undefined reference to `__atomic_load_8'
/usr/bin/ld: libfishlib.a(reader.cpp.o): in function `reader_read(parser_t&, int, io_chain_t const&)':
/usr/include/c++/8/bits/atomic_base.h:514: undefined reference to `__atomic_fetch_add_8'
collect2: error: ld returned 1 exit status

(gold's error messages are much less useful than GNU ld)

@mqudsi
Copy link
Contributor Author

mqudsi commented Dec 18, 2019

I can confirm that it's not an issue on ARMv8; I spun up an armv8 instance on AWS and it's not an issue there (mainly because I missed the v7 part of your post):

root@freebsd:/usr/home/ec2-user/fish-shell # uname -a
FreeBSD freebsd 12.1-RELEASE FreeBSD 12.1-RELEASE r354233 GENERIC  arm64
root@freebsd:/usr/home/ec2-user/fish-shell # ldd build/fish
build/fish:
        libncurses.so.8 => /lib/libncurses.so.8 (0x403c1000)
        libintl.so.8 => /usr/local/lib/libintl.so.8 (0x40432000)
        libc++.so.1 => /usr/lib/libc++.so.1 (0x40473000)
        libcxxrt.so.1 => /lib/libcxxrt.so.1 (0x40566000)
        libm.so.5 => /lib/libm.so.5 (0x405ab000)
        libthr.so.3 => /lib/libthr.so.3 (0x4061c000)
        libc.so.7 => /lib/libc.so.7 (0x40678000)
        libgcc_s.so.1 => /lib/libgcc_s.so.1 (0x40a3f000)
root@freebsd:/usr/home/ec2-user/fish-shell #

What's your armv7 test platform, @zanchey?

@zanchey
Copy link
Member

zanchey commented Dec 19, 2019

Linux titan 4.19.75-v7l+ #1270 SMP Tue Sep 24 18:51:41 BST 2019 armv7l GNU/Linux (Raspbian GNU/Linux 10)

Probably this is a 32-bit issue.

@zanchey zanchey self-assigned this Dec 20, 2019
zanchey added a commit that referenced this issue Dec 20, 2019
780bac6 did not actually successfully
compile on any platforms, leading to -latomic always being added
(including on platforms it does not exist on).

Work on #5865.
@zanchey zanchey reopened this Dec 20, 2019
@zanchey
Copy link
Member

zanchey commented Dec 20, 2019

The test for this still isn't right - libatomic is not available on RHEL 6/7, but the test fails there.

@zanchey
Copy link
Member

zanchey commented Dec 21, 2019

Looks like this is finally working in 45633f4

@zanchey zanchey closed this as completed Dec 21, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants