gokturk created this revision.
gokturk added reviewers: ilya-biryukov, nridge, kadircet, beanz, compnerd.
Herald added subscribers: cfe-commits, usaxena95, s.egerton, lenary, PkmX, jfb, 
arphaman, jkorous, simoncook, mgorny.
Herald added a project: clang.

The CheckAtomic module performs two tests to determine if passing
'-latomic' to the linker is required: one for 64-bit atomics, and
another for non-64-bit atomics. clangd only uses the result from
HAVE_CXX_ATOMICS64_WITHOUT_LIB. This is incomplete because there are
uses of non-64-bit atomics in the code, such as the ReplyOnce::Replied
of type std::atomic<bool> defined in clangd/ClangdLSPServer.cpp.

It is possible to have a requirement for an explicit '-latomic' for
non-64-bit atomics and not have for atomics64. One example is the
RISC-V rv64 (64-bit) architecture with the 'A' (atomic) extension,
where the host compiler gcc will convert any 64-bit atomic operation
to their hardware assembly equivalents, giving the impression that
'-latomic' is not required, while failing on 8-bit atomic operations
as there is no hardware support for that and linking against libatomic
is necessary.

As a matter of fact, clang-tools-extra (commit
5c40544fa40bfb85ec888b6a03421b3905e4a4e7) fails to compile with:

  
/usr/lib/gcc/riscv64-unknown-linux-gnu/9.1.0/../../../../riscv64-unknown-linux-gnu/bin/ld:
 tools/extra/clangd/CMakeFiles/obj.clangDaemon.dir/ClangdLSPServer.cpp.o: in 
function `.L0 '
  :
  
ClangdLSPServer.cpp:(.text._ZN5clang6clangd15ClangdLSPServer14MessageHandler9ReplyOnceclEN4llvm8ExpectedINS4_4json5ValueEEE[_ZN5clang6clangd15ClangdLSPServer14MessageHandler9ReplyOnceclEN4llvm8ExpectedINS4_4json5ValueEEE]+0x2a):
 undefined reference to `__atomic_exchange_1'
  collect2: error: ld returned 1 exit status

Fix by also checking for the result of HAVE_CXX_ATOMICS_WITHOUT_LIB.

See also: https://reviews.llvm.org/D68964


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69869

Files:
  clang-tools-extra/clangd/CMakeLists.txt


Index: clang-tools-extra/clangd/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -30,7 +30,7 @@
 endif()
 
 set(CLANGD_ATOMIC_LIB "")
-if(NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB)
+if(NOT HAVE_CXX_ATOMICS_WITHOUT_LIB OR NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB)
   list(APPEND CLANGD_ATOMIC_LIB "atomic")
 endif()
 


Index: clang-tools-extra/clangd/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -30,7 +30,7 @@
 endif()
 
 set(CLANGD_ATOMIC_LIB "")
-if(NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB)
+if(NOT HAVE_CXX_ATOMICS_WITHOUT_LIB OR NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB)
   list(APPEND CLANGD_ATOMIC_LIB "atomic")
 endif()
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to