[CMake] Cleanup add_compiler_rt_object_library to be platform-agnostic

[beanz]

Summary

This change takes darwin-specific goop that was scattered around CMakeLists files and spread between add_compiler_rt_object_library and add_compiler_rt_darwin_object_library and moves it all under add_compiler_rt_object_library.

The goal of this is to try to push platform handling as low in the utility functions as possible.

[rsmith]

cmake/Modules/AddCompilerRT.cmake

5–6

This comment needs to be updated.

lib/asan/CMakeLists.txt

79

It seems weird for the dynamic library target to have the name RTAsan for APPLE but to be named RTAsan_dynamic everywhere else. Is it possible to merge these two targets and move them out of the if here? (Not as part of this change, just curious if we can unify this further.)

lib/interception/CMakeLists.txt

14–19

Do you still need the if here? Both arms look the same (other than the OS part which is ignored for the non-APPLE case).

lib/lsan/CMakeLists.txt

20–27

Likewise, can you move this part out of the if?

[beanz]

I will send updated patches in a minute once I finish testing them.

cmake/Modules/AddCompilerRT.cmake

5–6

Will do.

lib/asan/CMakeLists.txt

79

I hope to reduce this further in another patch. I really want to get to the point where there is no platform handling in the CMakeLists files for libraries.

lib/interception/CMakeLists.txt

14–19

Yea, I can get rid of the if here. I was going to do that in a later patch, because I didn't realize the OS parameter was the only difference.

lib/lsan/CMakeLists.txt

20–27

Yep, I can do that here too. That will get rid of the Apple side of this conditional.

[beanz]

Updating patches based on feedback from Richard Smith.

• Updated comments on add_compiler_rt_object_library
• Moved a few calls to add_compiler_rt_object_library out of their 'if' blocks

[rnk]

Seems like a nice win. :)

cmake/Modules/AddCompilerRT.cmake

5–6

I think this needs more updating, it's no longer adding a single object library, it actually creates an object library for each supported OS or architecture. add_compiler_rt_object_libraries might be a better name too.

8

Can you make this a function instead of a macro? Then you don't leak all these variables into the parent scope.

[samsonov]

lib/lsan/CMakeLists.txt

22

It's weird that we now use SANITIZER_COMMON_SUPPORTED_DARWIN_OS on non-Darwin platforms.

[beanz]

I'll send updated patches shortly.

cmake/Modules/AddCompilerRT.cmake

5–6

Will do.

8

Will do.

lib/lsan/CMakeLists.txt

22

This is weird, but I see it as a temporary weirdness. I'm hoping this will all fold away. If you have a preference on how to make it less weird I'm open to suggestions.

My thought, was to eventually just change that to SANITIZER_COMMON_SUPPORTED_OS, and to set it appropriately for whatever OS you are targeting.

[beanz]

Updating based on feedback from Reid.

[rnk]

lgtm

[samsonov]

LGTM, but please address comments below.

lib/interception/CMakeLists.txt

15

Here as well. Or rename SANITIZER_COMMON_SUPPORTED_DARWIN_OS to smth. more appropriate first.

lib/lsan/CMakeLists.txt

22

I'd still use

if (APPLE)

clause here and avoid using reference to DARWIN_OS on Linux.

cmake/Modules/AddCompilerRT.cmake

 include(AddLLVM)
 include(ExternalProject)
 include(LLVMParseArguments)
 include(CompilerRTUtils)
 
-# Tries to add "object library" target for a given architecture
-# with name "<name>.<arch>" if architecture can be targeted.
-# add_compiler_rt_object_library(<name> <arch>
-#                                SOURCES <source files>
-#                                CFLAGS <compile flags>
-#                                DEFS <compile definitions>)
-macro(add_compiler_rt_object_library name arch)
-  if(CAN_TARGET_${arch})
-    parse_arguments(LIB "SOURCES;CFLAGS;DEFS" "" ${ARGN})
-    add_library(${name}.${arch} OBJECT ${LIB_SOURCES})
-    set_target_compile_flags(${name}.${arch}
-      ${CMAKE_CXX_FLAGS} ${TARGET_${arch}_CFLAGS} ${LIB_CFLAGS})
-    set_property(TARGET ${name}.${arch} APPEND PROPERTY
-      COMPILE_DEFINITIONS ${LIB_DEFS})
+# Tries to add an "object library" target for a given list of OSs and/or
+# architectures with name "<name>.<arch>" for non-Darwin platforms if
+# architecture can be targeted, and "<name>.<os>" for Darwin platforms.
+# add_compiler_rt_object_libraries(<name>
+#                                  OS <os>
+#                                  ARCH <arch>
+#                                  SOURCES <source files>
+#                                  CFLAGS <compile flags>
+#                                  DEFS <compile definitions>)
+function(add_compiler_rt_object_libraries name)
+  parse_arguments(LIB "OS;ARCH;SOURCES;CFLAGS;DEFS" "" ${ARGN})
+  set(libnames)
+  if(APPLE)
+    foreach(os ${LIB_OS})
+      set(libname "${name}.${os}")
+      set(libnames ${libnames} ${libname})
+      set(extra_cflags_${libname} ${DARWIN_${os}_CFLAGS})
+    endforeach()
   else()
+    foreach(arch ${LIB_ARCH})
+      set(libname "${name}.${arch}")
+      set(libnames ${libnames} ${libname})
+      set(extra_cflags_${libname} ${TARGET_${arch}_CFLAGS})
+      if(NOT CAN_TARGET_${arch})
         message(FATAL_ERROR "Archtecture ${arch} can't be targeted")
+        return()
+      endif()
+    endforeach()
   endif()
-endmacro()
   
-# Same as above, but adds universal osx library for either OSX or iOS simulator
-# with name "<name>.<os>" targeting multiple architectures.
-# add_compiler_rt_darwin_object_library(<name> <os> ARCH <architectures>
-#                                                   SOURCES <source files>
-#                                                   CFLAGS <compile flags>
-#                                                   DEFS <compile definitions>)
-macro(add_compiler_rt_darwin_object_library name os)
-  parse_arguments(LIB "ARCH;SOURCES;CFLAGS;DEFS" "" ${ARGN})
-  set(libname "${name}.${os}")
+  foreach(libname ${libnames})
     add_library(${libname} OBJECT ${LIB_SOURCES})
-  set_target_compile_flags(${libname} ${LIB_CFLAGS} ${DARWIN_${os}_CFLAGS})
-  set_target_properties(${libname} PROPERTIES OSX_ARCHITECTURES "${LIB_ARCH}")
+    set_target_compile_flags(${libname}
+      ${CMAKE_CXX_FLAGS} ${extra_cflags_${libname}} ${LIB_CFLAGS})
     set_property(TARGET ${libname} APPEND PROPERTY
       COMPILE_DEFINITIONS ${LIB_DEFS})
-endmacro()
+    if(APPLE)
+      set_target_properties(${libname} PROPERTIES OSX_ARCHITECTURES "${LIB_ARCH}")
+    endif()
+  endforeach()
+endfunction()
 
 # Adds static or shared runtime for a given architecture and puts it in the
 # proper directory in the build and install trees.
 # add_compiler_rt_runtime(<name> <arch> {STATIC,SHARED}
 #                         SOURCES <source files>
 #                         CFLAGS <compile flags>
 #                         DEFS <compile definitions>
 #                         OUTPUT_NAME <output library name>)

lib/asan/CMakeLists.txt

 append_list_if(COMPILER_RT_HAS_LIBM m ASAN_DYNAMIC_LIBS)
 append_list_if(COMPILER_RT_HAS_LIBPTHREAD pthread ASAN_DYNAMIC_LIBS)
 append_list_if(COMPILER_RT_HAS_LIBSTDCXX stdc++ ASAN_DYNAMIC_LIBS)
 
 append_list_if(ANDROID log ASAN_DYNAMIC_LIBS)
 
 # Compile ASan sources into an object library.
 if(APPLE)
-  foreach(os ${SANITIZER_COMMON_SUPPORTED_DARWIN_OS})
-    add_compiler_rt_darwin_object_library(RTAsan ${os}
-      ARCH ${ASAN_SUPPORTED_ARCH}
-      SOURCES ${ASAN_SOURCES} ${ASAN_CXX_SOURCES}
-      CFLAGS ${ASAN_DYNAMIC_CFLAGS}
-      DEFS ${ASAN_DYNAMIC_DEFINITIONS})
-  endforeach()
+  add_compiler_rt_object_libraries(RTAsan
+    OS ${SANITIZER_COMMON_SUPPORTED_DARWIN_OS}
 else()
-  foreach(arch ${ASAN_SUPPORTED_ARCH})
-    add_compiler_rt_object_library(RTAsan ${arch}
-      SOURCES ${ASAN_SOURCES} CFLAGS ${ASAN_CFLAGS}
-      DEFS ${ASAN_COMMON_DEFINITIONS})
-    add_compiler_rt_object_library(RTAsan_cxx ${arch}
-      SOURCES ${ASAN_CXX_SOURCES} CFLAGS ${ASAN_CFLAGS}
-      DEFS ${ASAN_COMMON_DEFINITIONS})
-    add_compiler_rt_object_library(RTAsan_preinit ${arch}
-      SOURCES ${ASAN_PREINIT_SOURCES} CFLAGS ${ASAN_CFLAGS}
-      DEFS ${ASAN_COMMON_DEFINITIONS})
-    add_compiler_rt_object_library(RTAsan_dynamic ${arch}
-      SOURCES ${ASAN_SOURCES} ${ASAN_CXX_SOURCES}
-      CFLAGS ${ASAN_DYNAMIC_CFLAGS}
-      DEFS ${ASAN_DYNAMIC_DEFINITIONS})
+  add_compiler_rt_object_libraries(RTAsan 
+    ARCH ${ASAN_SUPPORTED_ARCH}
+  add_compiler_rt_object_libraries(RTAsan_cxx 
+    ARCH ${ASAN_SUPPORTED_ARCH}
+  add_compiler_rt_object_libraries(RTAsan_preinit 
+    ARCH ${ASAN_SUPPORTED_ARCH}
+  add_compiler_rt_object_libraries(RTAsan_dynamic 
+    ARCH ${ASAN_SUPPORTED_ARCH}
 
-    file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/dummy.cc "")
-    add_compiler_rt_object_library(RTAsan_dynamic_version_script_dummy ${arch}
-      SOURCES ${CMAKE_CURRENT_BINARY_DIR}/dummy.cc
-      CFLAGS ${ASAN_DYNAMIC_CFLAGS}
-      DEFS ${ASAN_DYNAMIC_DEFINITIONS})
-  endforeach()
+  add_compiler_rt_object_libraries(RTAsan_dynamic_version_script_dummy
+    ARCH ${ASAN_SUPPORTED_ARCH}
 endif()
 
 # Build ASan runtimes shipped with Clang.
 add_custom_target(asan)
 if(APPLE)
   foreach (os ${SANITIZER_COMMON_SUPPORTED_DARWIN_OS})
     add_compiler_rt_darwin_dynamic_runtime(clang_rt.asan_${os}_dynamic ${os}
       ARCH ${ASAN_SUPPORTED_ARCH}

lib/interception/CMakeLists.txt

 # Build for the runtime interception helper library.
 
 set(INTERCEPTION_SOURCES
   interception_linux.cc
   interception_mac.cc
   interception_win.cc
   interception_type_test.cc
   )
 
 include_directories(..)
 
 set(INTERCEPTION_CFLAGS ${SANITIZER_COMMON_CFLAGS})
 append_no_rtti_flag(INTERCEPTION_CFLAGS)
 
-if(APPLE)
-  # Build universal binary on APPLE.
-  foreach(os ${SANITIZER_COMMON_SUPPORTED_DARWIN_OS})
-    add_compiler_rt_darwin_object_library(RTInterception ${os}
-      ARCH ${SANITIZER_COMMON_SUPPORTED_ARCH}
-      SOURCES ${INTERCEPTION_SOURCES}
-      CFLAGS ${INTERCEPTION_CFLAGS})
-  endforeach()
-else()
-  # Otherwise, build separate libraries for each target.
-  foreach(arch ${SANITIZER_COMMON_SUPPORTED_ARCH})
-    add_compiler_rt_object_library(RTInterception ${arch}
-      SOURCES ${INTERCEPTION_SOURCES} CFLAGS ${INTERCEPTION_CFLAGS})
-  endforeach()
-endif()
+add_compiler_rt_object_libraries(RTInterception
+    OS ${SANITIZER_COMMON_SUPPORTED_DARWIN_OS}

lib/lsan/CMakeLists.txt

set(LSAN_SOURCES
   lsan_allocator.cc
   lsan_interceptors.cc
   lsan_preinit.cc
   lsan_thread.cc)
 
 set(LSAN_SRC_DIR ${CMAKE_CURRENT_SOURCE_DIR})
 
 add_custom_target(lsan)
 
-if(APPLE)
-  foreach(os ${SANITIZER_COMMON_SUPPORTED_DARWIN_OS})
-    add_compiler_rt_darwin_object_library(RTLSanCommon ${os}
-      ARCH ${LSAN_COMMON_SUPPORTED_ARCH}
-      SOURCES ${LSAN_COMMON_SOURCES}
-      CFLAGS ${LSAN_CFLAGS})
-  endforeach()
-else()
-  foreach(arch ${LSAN_COMMON_SUPPORTED_ARCH})
-    add_compiler_rt_object_library(RTLSanCommon ${arch}
-      SOURCES ${LSAN_COMMON_SOURCES}
-      CFLAGS ${LSAN_CFLAGS})
-  endforeach()
+add_compiler_rt_object_libraries(RTLSanCommon
+    OS ${SANITIZER_COMMON_SUPPORTED_DARWIN_OS}
 
+if(NOT APPLE)
   foreach(arch ${LSAN_SUPPORTED_ARCH})
     add_compiler_rt_runtime(clang_rt.lsan-${arch} ${arch} STATIC
       SOURCES ${LSAN_SOURCES}
               $<TARGET_OBJECTS:RTInterception.${arch}>
               $<TARGET_OBJECTS:RTSanitizerCommon.${arch}>
               $<TARGET_OBJECTS:RTSanitizerCommonLibc.${arch}>
               $<TARGET_OBJECTS:RTLSanCommon.${arch}>
       CFLAGS ${LSAN_CFLAGS})
     add_dependencies(lsan clang_rt.lsan-${arch})
   endforeach()
 endif()
 
 add_dependencies(compiler-rt lsan)

lib/sanitizer_common/CMakeLists.txt

append_list_if(SANITIZER_LIMIT_FRAME_SIZE -Wframe-larger-than=570
                SANITIZER_CFLAGS)
 append_list_if(COMPILER_RT_HAS_WGLOBAL_CONSTRUCTORS_FLAG -Wglobal-constructors
                SANITIZER_CFLAGS)
 
 add_custom_target(sanitizer_common)
 set(SANITIZER_RUNTIME_LIBRARIES)
 if(APPLE)
   # Build universal binary on APPLE.
-  foreach(os ${SANITIZER_COMMON_SUPPORTED_DARWIN_OS})
-    add_compiler_rt_darwin_object_library(RTSanitizerCommon ${os}
-      ARCH ${SANITIZER_COMMON_SUPPORTED_ARCH}
-      SOURCES ${SANITIZER_SOURCES} ${SANITIZER_LIBCDEP_SOURCES}
-      CFLAGS ${SANITIZER_CFLAGS}
-      DEFS ${SANITIZER_COMMON_DEFINITIONS})
+
+  add_compiler_rt_object_libraries(RTSanitizerCommon
+    OS ${SANITIZER_COMMON_SUPPORTED_DARWIN_OS}
+  foreach(os ${SANITIZER_COMMON_SUPPORTED_DARWIN_OS})
     list(APPEND SANITIZER_RUNTIME_LIBRARIES RTSanitizerCommon.${os})
   endforeach()
 else()
   # Otherwise, build separate libraries for each target.
-  foreach(arch ${SANITIZER_COMMON_SUPPORTED_ARCH})
-    add_compiler_rt_object_library(RTSanitizerCommon ${arch}
-      SOURCES ${SANITIZER_SOURCES} CFLAGS ${SANITIZER_CFLAGS}
-      DEFS ${SANITIZER_COMMON_DEFINITIONS})
-    add_compiler_rt_object_library(RTSanitizerCommonLibc ${arch}
-      SOURCES ${SANITIZER_LIBCDEP_SOURCES} CFLAGS ${SANITIZER_CFLAGS}
-      DEFS ${SANITIZER_COMMON_DEFINITIONS})
+  
+  add_compiler_rt_object_libraries(RTSanitizerCommon
+    ARCH ${SANITIZER_COMMON_SUPPORTED_ARCH}
+  add_compiler_rt_object_libraries(RTSanitizerCommonLibc
+    ARCH ${SANITIZER_COMMON_SUPPORTED_ARCH}
+  foreach(arch ${SANITIZER_COMMON_SUPPORTED_ARCH})
     list(APPEND SANITIZER_RUNTIME_LIBRARIES RTSanitizerCommon.${arch}
                                             RTSanitizerCommonLibc.${arch})
   endforeach()
 endif()
 
 add_dependencies(compiler-rt sanitizer_common)
 
 # Unit tests for common sanitizer runtime.
 if(COMPILER_RT_INCLUDE_TESTS)
   add_subdirectory(tests)
 endif()

lib/tsan/dd/CMakeLists.txt

if(CAN_TARGET_x86_64 AND UNIX AND NOT APPLE AND NOT ANDROID)
   add_compiler_rt_runtime(clang_rt.dd-${arch} ${arch} STATIC
     SOURCES ${DD_SOURCES}
             $<TARGET_OBJECTS:RTInterception.${arch}>
             $<TARGET_OBJECTS:RTSanitizerCommon.${arch}>
             $<TARGET_OBJECTS:RTSanitizerCommonLibc.${arch}>
     CFLAGS ${DD_CFLAGS})
   add_dependencies(dd clang_rt.dd-${arch})
 
-  add_compiler_rt_object_library(RTDD ${arch}
+  add_compiler_rt_object_libraries(RTDD
+    ARCH ${arch}
     SOURCES ${DD_SOURCES} CFLAGS ${DD_CFLAGS})
 
   add_compiler_rt_runtime(clang_rt.dyndd-${arch} ${arch} SHARED
     SOURCES $<TARGET_OBJECTS:RTDD.${arch}>
             $<TARGET_OBJECTS:RTInterception.${arch}>
             $<TARGET_OBJECTS:RTSanitizerCommon.${arch}>
             $<TARGET_OBJECTS:RTSanitizerCommonLibc.${arch}>)
   target_link_libraries(clang_rt.dyndd-${arch} ${DD_LINKLIBS})
   add_dependencies(dd clang_rt.dyndd-${arch})
 endif()
 
 add_dependencies(compiler-rt dd)

lib/ubsan/CMakeLists.txt

 append_no_rtti_flag(UBSAN_CFLAGS)
 set(UBSAN_STANDALONE_CFLAGS ${SANITIZER_COMMON_CFLAGS})
 append_no_rtti_flag(UBSAN_STANDALONE_CFLAGS)
 set(UBSAN_CXXFLAGS ${SANITIZER_COMMON_CFLAGS})
 
 add_custom_target(ubsan)
 
 if(APPLE)
-  foreach(os ${SANITIZER_COMMON_SUPPORTED_DARWIN_OS})
-    # Common parts of UBSan runtime.
-    add_compiler_rt_darwin_object_library(RTUbsan ${os}
-      ARCH ${UBSAN_COMMON_SUPPORTED_ARCH}
-      SOURCES ${UBSAN_SOURCES} ${UBSAN_CXX_SOURCES}
-      CFLAGS ${UBSAN_CXXFLAGS})
+  add_compiler_rt_object_libraries(RTUbsan
+    OS ${SANITIZER_COMMON_SUPPORTED_DARWIN_OS}
 
-    if(COMPILER_RT_HAS_UBSAN)
-      # Initializer of standalone UBSan runtime.
-      add_compiler_rt_darwin_object_library(RTUbsan_standalone ${os}
-        ARCH ${UBSAN_SUPPORTED_ARCH}
-        SOURCES ${UBSAN_STANDALONE_SOURCES}
-        CFLAGS ${UBSAN_STANDALONE_CFLAGS})
-
+    add_compiler_rt_object_libraries(RTUbsan_standalone
+      OS ${SANITIZER_COMMON_SUPPORTED_DARWIN_OS}
+    foreach(os ${SANITIZER_COMMON_SUPPORTED_DARWIN_OS})
       add_compiler_rt_darwin_dynamic_runtime(clang_rt.ubsan_${os}_dynamic ${os}
         ARCH ${UBSAN_SUPPORTED_ARCH}
         SOURCES $<TARGET_OBJECTS:RTUbsan.${os}>
                 $<TARGET_OBJECTS:RTUbsan_standalone.${os}>
                 $<TARGET_OBJECTS:RTSanitizerCommon.${os}>)
 
       add_dependencies(ubsan clang_rt.ubsan_${os}_dynamic)
-    endif()
     endforeach()
+  endif()
+
 else()
   # Common parts of UBSan runtime.
-  foreach(arch ${UBSAN_COMMON_SUPPORTED_ARCH})
-    add_compiler_rt_object_library(RTUbsan ${arch}
-      SOURCES ${UBSAN_SOURCES} CFLAGS ${UBSAN_CFLAGS})
-    # C++-specific parts of UBSan runtime. Requires a C++ ABI library.
-    add_compiler_rt_object_library(RTUbsan_cxx ${arch}
-      SOURCES ${UBSAN_CXX_SOURCES} CFLAGS ${UBSAN_CXXFLAGS})
-  endforeach()
+  add_compiler_rt_object_libraries(RTUbsan
+    ARCH ${UBSAN_COMMON_SUPPORTED_ARCH}
+  add_compiler_rt_object_libraries(RTUbsan_cxx
+    ARCH ${UBSAN_COMMON_SUPPORTED_ARCH}
 
   if(COMPILER_RT_HAS_UBSAN)
-    foreach(arch ${UBSAN_SUPPORTED_ARCH})
-      # Initializer of standalone UBSan runtime.
-      add_compiler_rt_object_library(RTUbsan_standalone ${arch}
-        SOURCES ${UBSAN_STANDALONE_SOURCES} CFLAGS ${UBSAN_STANDALONE_CFLAGS})
+    add_compiler_rt_object_libraries(RTUbsan_standalone
+      ARCH ${UBSAN_SUPPORTED_ARCH}
     
+    foreach(arch ${UBSAN_SUPPORTED_ARCH})
       # Standalone UBSan runtimes.
       add_compiler_rt_runtime(clang_rt.ubsan_standalone-${arch} ${arch} STATIC
         SOURCES $<TARGET_OBJECTS:RTSanitizerCommon.${arch}>
                 $<TARGET_OBJECTS:RTSanitizerCommonLibc.${arch}>
                 $<TARGET_OBJECTS:RTUbsan.${arch}>
                 $<TARGET_OBJECTS:RTUbsan_standalone.${arch}>
         CFLAGS ${UBSAN_CFLAGS})
       add_compiler_rt_runtime(clang_rt.ubsan_standalone_cxx-${arch} ${arch} STATIC