[PATCH] D139782: [scudo][standalone] Use CheckAtomic to decide to link to libatomic

[Ben Wolsieffer via Phabricator via llvm-commits]

benwolsieffer created this revision.
benwolsieffer added reviewers: vitalybuka, MaskRay.
Herald added subscribers: Enna1, StephenFan, atanasyan, cryptoad, kristof.beyls, arichardson, sdardis.
Herald added a project: All.
benwolsieffer requested review of this revision.
Herald added projects: Sanitizers, LLVM.
Herald added subscribers: llvm-commits, Sanitizers.

Standalone scudo uses the atomic operation builtin functions, which require
linking to libatomic on some platforms. Currently, this is done in an ad-hoc
manner. MIPS platforms always link to libatomic, and the tests are always linked
to it as well. libatomic is required on base ARMv6 (but not ARMv6K), but it is
currently not linked, causing the build to fail.

This patch replaces this ad-hoc logic with the CheckAtomic CMake module already
used in other parts of LLVM. The CheckAtomic module checks whether std::atomic
requires libatomic, which is not strictly the same as checking the atomic
builtins, but should have the same results as far as I know. If this is
problematic, a custom version of CheckAtomic could be used to specifically test
the builtins.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139782

Files:
  compiler-rt/lib/scudo/standalone/CMakeLists.txt
  compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
  llvm/cmake/modules/CheckAtomic.cmake


Index: llvm/cmake/modules/CheckAtomic.cmake
===================================================================
--- llvm/cmake/modules/CheckAtomic.cmake
+++ llvm/cmake/modules/CheckAtomic.cmake
@@ -2,6 +2,7 @@
 
 INCLUDE(CheckCXXSourceCompiles)
 INCLUDE(CheckLibraryExists)
+INCLUDE(DetermineGCCCompatible)
 
 # Sometimes linking against libatomic is required for atomic ops, if
 # the platform doesn't support lock-free atomics.
Index: compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
===================================================================
--- compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
+++ compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
@@ -37,13 +37,9 @@
 set(SCUDO_UNITTEST_LINK_FLAGS
   ${COMPILER_RT_UNITTEST_LINK_FLAGS}
   ${COMPILER_RT_UNWINDER_LINK_LIBS}
-  ${SANITIZER_TEST_CXX_LIBRARIES})
+  ${SANITIZER_TEST_CXX_LIBRARIES}
+  ${LLVM_ATOMIC_LIB})
 list(APPEND SCUDO_UNITTEST_LINK_FLAGS -pthread -no-pie)
-# Linking against libatomic is required with some compilers
-check_library_exists(atomic __atomic_load_8 "" COMPILER_RT_HAS_LIBATOMIC)
-if (COMPILER_RT_HAS_LIBATOMIC)
-  list(APPEND SCUDO_UNITTEST_LINK_FLAGS -latomic)
-endif()
 
 set(SCUDO_TEST_HEADERS
   scudo_unit_test.h
Index: compiler-rt/lib/scudo/standalone/CMakeLists.txt
===================================================================
--- compiler-rt/lib/scudo/standalone/CMakeLists.txt
+++ compiler-rt/lib/scudo/standalone/CMakeLists.txt
@@ -1,5 +1,7 @@
 add_compiler_rt_component(scudo_standalone)
 
+include(CheckAtomic)
+
 include_directories(../.. include)
 
 set(SCUDO_CFLAGS)
@@ -28,7 +30,7 @@
   list(APPEND SCUDO_CFLAGS -O3)
 endif()
 
-set(SCUDO_LINK_FLAGS)
+set(SCUDO_LINK_FLAGS ${LLVM_ATOMIC_LIB})
 
 list(APPEND SCUDO_LINK_FLAGS -Wl,-z,defs,-z,now,-z,relro)
 
@@ -153,10 +155,6 @@
 
 append_list_if(FUCHSIA zircon SCUDO_LINK_LIBS)
 
-if(COMPILER_RT_DEFAULT_TARGET_ARCH MATCHES "mips|mips64|mipsel|mips64el")
-  list(APPEND SCUDO_LINK_LIBS atomic)
-endif()
-
 if(COMPILER_RT_HAS_SCUDO_STANDALONE)
   add_compiler_rt_object_libraries(RTScudoStandalone
     ARCHS ${SCUDO_STANDALONE_SUPPORTED_ARCH}


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D139782.481893.patch
Type: text/x-patch
Size: 2123 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221211/bfcaac03/attachment.bin>