Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

Build failure on RISC-V due missing __atomic_exchange_1 #1427

Open
ricardosalveti opened this issue Oct 29, 2019 · 7 comments
Open

Build failure on RISC-V due missing __atomic_exchange_1 #1427

ricardosalveti opened this issue Oct 29, 2019 · 7 comments

Comments

@ricardosalveti
Copy link

When trying to build aktualizr 2019.9 for QEMU RISC-V 64:

/home/rsalveti/build/lmp/build-lmp/tmp-lmp/work/riscv64-lmp-linux/aktualizr/1.0+gitAUTOINC+a358242d9a-7/recipe-sysroot-native/usr/bin
/riscv64-lmp-linux/../../libexec/riscv64-lmp-linux/gcc/riscv64-lmp-linux/9.2.0/ld: src/libaktualizr/utilities/CMakeFiles/utilities.di
r/sig_handler.cc.o: in function `std::__atomic_base<bool>::compare_exchange_strong(bool&, bool, std::memory_order, std::memory_order)
':
/usr/include/c++/9.2.0/bits/atomic_base.h:502: undefined reference to `__atomic_compare_exchange_1'
/home/rsalveti/build/lmp/build-lmp/tmp-lmp/work/riscv64-lmp-linux/aktualizr/1.0+gitAUTOINC+a358242d9a-7/recipe-sysroot-native/usr/bin/riscv64-lmp-linux/../../libexec/riscv64-lmp-linux/gcc/riscv64-lmp-linux/9.2.0/ld: src/libaktualizr/utilities/CMakeFiles/utilities.dir/sig_handler.cc.o: in function `boost::log::v2_mt_posix::aux::basic_ostringstreambuf<char, std::char_traits<char>, std::allocator<char> >::length_until_boundary(char const*, unsigned long, unsigned long, boost::integral_constant<bool, true>) const':
/usr/include/boost/log/detail/attachable_sstream_buf.hpp:289: undefined reference to `__atomic_exchange_1'
/home/rsalveti/build/lmp/build-lmp/tmp-lmp/work/riscv64-lmp-linux/aktualizr/1.0+gitAUTOINC+a358242d9a-7/recipe-sysroot-native/usr/bin/riscv64-lmp-linux/../../libexec/riscv64-lmp-linux/gcc/riscv64-lmp-linux/9.2.0/ld: src/libaktualizr/utilities/CMakeFiles/utilities.dir/sig_handler.cc.o: in function `std::basic_streambuf<char, std::char_traits<char> >::getloc() const':
/usr/include/c++/9.2.0/streambuf:234: undefined reference to `__atomic_exchange_1'
collect2: error: ld returned 1 exit status

Looks like this happens because not every atomic operation is currently supported by GCC on RISC-V.

Investigating further I found two possible ways to fix this issue:
1 - Change from atomic to atomic_uint (which is currently supported by gcc 9.2):

diff --git a/src/libaktualizr/utilities/sig_handler.cc b/src/libaktualizr/utilities/sig_handler.cc
index 80699c5b..364b5243 100644
--- a/src/libaktualizr/utilities/sig_handler.cc
+++ b/src/libaktualizr/utilities/sig_handler.cc
@@ -3,12 +3,12 @@

 void signal_handler(int sig) {
   (void)sig;
-  bool v = false;
+  unsigned int v = 0;
   // put true if currently set to false
-  SigHandler::signal_marker_.compare_exchange_strong(v, true);
+  SigHandler::signal_marker_.compare_exchange_strong(v, 1);
 }

-std::atomic<bool> SigHandler::signal_marker_;
+std::atomic_uint SigHandler::signal_marker_;

 SigHandler& SigHandler::get() {
   static SigHandler handler;
@@ -30,7 +30,7 @@ void SigHandler::start(const std::function<void()>& on_signal) {

   polling_thread_ = boost::thread([this, on_signal]() {
     while (true) {
-      bool signal = signal_marker_.exchange(false);
+      auto signal = signal_marker_.exchange(0);

       if (signal) {
         LOG_INFO << "received KILL request";
diff --git a/src/libaktualizr/utilities/sig_handler.h b/src/libaktualizr/utilities/sig_handler.h
index 03a4a5aa..37a26860 100644
--- a/src/libaktualizr/utilities/sig_handler.h
+++ b/src/libaktualizr/utilities/sig_handler.h
@@ -26,7 +26,7 @@ class SigHandler {

   boost::thread polling_thread_;
   bool signal_pending;
-  static std::atomic<bool> signal_marker_;
+  static std::atomic_uint signal_marker_;

   boost::mutex m;
   int masked_secs_;

2 - Link to libatomic as it is known to have all the required atomic operations for RISC-V:

find_library(LIBATOMIC atomic)

diff --git a/src/cert_provider/CMakeLists.txt b/src/cert_provider/CMakeLists.txt
index 21f45513..08492491 100644
--- a/src/cert_provider/CMakeLists.txt
+++ b/src/cert_provider/CMakeLists.txt
@@ -19,6 +19,7 @@ target_link_libraries(aktualizr-cert-provider
     ${CURL_LIBRARIES}
     ${OPENSSL_LIBRARIES}
     ${sodium_LIBRARY_RELEASE}
+    ${LIBATOMIC}
     )

 aktualizr_source_file_checks(main.cc)

Option 1 doesn't require an extra library but this can bite us again in the future, option 2 seems safe but will add an extra library to aktualizr.

Any opinions?

@lbonn
Copy link
Contributor

lbonn commented Oct 29, 2019

Interesting...

Do you happen to know what recipes would we use on openembedded? meta/recipes-support/libatomic-ops?

Would be fine with both, but I'd choose the first option right now if we can get away with it.

@eu-siemann
Copy link
Contributor

I don't think we should change a perfectly valid code because of the issues with a specific compiler or platform when we have other options. Someone might change it back when we forget and it will break again.
Maybe we can link with libatomic only when requested by EXTRA_OECMAKE?

@ricardosalveti
Copy link
Author

This is known to happen in multiple architectures and the safe approach is to always link to libatomic, as all the atomic operations are known to be provided there.

@ricardosalveti
Copy link
Author

Some other useful reference as well: riscv-collab/riscv-gnu-toolchain#183 (comment)

@ricardosalveti
Copy link
Author

Do you happen to know what recipes would we use on openembedded? meta/recipes-support/libatomic-ops?

Yeah, that would be the one.

@ricardosalveti
Copy link
Author

ricardosalveti commented Oct 29, 2019

I don't think we should change a perfectly valid code because of the issues with a specific compiler or platform when we have other options. Someone might change it back when we forget and it will break again.
Maybe we can link with libatomic only when requested by EXTRA_OECMAKE?

The problem is that this issue will also show up for people building aktualizr outside OE, so we need to handle this issue as part of the project itself.

Guess the best way would be to add a configure flag to check and use libatomic and then we can use it on such architectures, or that automatically decides what is the best thing to do without such flags.

@XieJiSS
Copy link

XieJiSS commented May 17, 2022

This should be fixed by https://www.mail-archive.com/[email protected]/msg283700.html. We discussed this with the gcc RISC-V dev teams earlier, see the slide for details.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants