[llvm-commits] [PATCH] Rework CMake rules for building ASan unittests

[Alexey Samsonov]

  Yeah, re-using some code is a good thing to do, I've tried to implement some bits of it.

  I'd like to avoid doing all the stuff I want with unittests in one change (in particular, when these changes go in, nothing prevents us from building tests in both 32- and 64-bit mode), but if you'd like to see the final structure, I can go on improving this now.


================
Comment at: lib/asan/tests/CMakeLists.txt:17-23
@@ -16,2 +16,9 @@
 
+set(LLVM_GTEST_PATH ${LLVM_MAIN_SRC_DIR}/utils/unittest/googletest)
+set(GTEST_SOURCE ${LLVM_GTEST_PATH}/gtest-all.cc)
+set(ASAN_GTEST_INCLUDE_CFLAGS
+  -DGTEST_NO_LLVM_RAW_OSTREAM=1
+  -I${LLVM_GTEST_PATH}/include
+)
+
 set(ASAN_UNITTEST_COMMON_CFLAGS
----------------
Chandler Carruth wrote:
> I feel like this should all be abstracted to where the various sanitizer test trees can share it.
Done

================
Comment at: lib/asan/tests/CMakeLists.txt:89-97
@@ -77,28 +88,11 @@
 
-function(add_asan_test testsuite testname)
-  add_unittest(${testsuite} ${testname} ${ARGN})
-  if (APPLE)
-    # Darwin-specific linker flags.
-    set_property(TARGET ${testname} APPEND PROPERTY
-                 LINK_FLAGS "-framework Foundation")
-    target_link_libraries(${testname} clang_rt.asan_osx)
-  elseif (ANDROID)
-    target_link_libraries(${testname} clang_rt.asan-arm-android)
-  elseif (UNIX)
-    # Linux-specific linker flags.
-    set_property(TARGET ${testname} APPEND PROPERTY
-                 LINK_FLAGS "-lpthread -ldl -rdynamic")
-    if(LLVM_BUILD_32_BITS)
-      target_link_libraries(${testname} clang_rt.asan-i386)
-    else()
-      target_link_libraries(${testname} clang_rt.asan-x86_64)
-    endif()
+# This function sets the expected output directory of ASan unittest
+# (it uses the same rules for determining it as Clang/LLVM unittests).
+function(get_test_directory dir)
+  if(CMAKE_BUILD_TYPE)
+    set(OUTPUT_DIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_BUILD_TYPE})
+  else()
+    set(OUTPUT_DIR ${CMAKE_CURRENT_BINARY_DIR})
   endif()
-  set(add_compile_flags "")
-  get_property(compile_flags TARGET ${testname} PROPERTY COMPILE_FLAGS)
-  foreach(arg ${ASAN_UNITTEST_COMMON_CFLAGS})
-    set(add_compile_flags "${add_compile_flags} ${arg}")
-  endforeach(arg ${ASAN_UNITTEST_COMMON_CFLAGS})
-  set_property(TARGET ${testname} PROPERTY COMPILE_FLAGS
-               "${compile_flags} ${add_compile_flags}")
+  set(${dir} ${OUTPUT_DIR} PARENT_SCOPE)
 endfunction()
----------------
Chandler Carruth wrote:
> Same for this, can't this be abstracted?
> 
> This one I feel like could be abstracted into a helper function in LLVM's .cmake files, and shared with all of the gtest cmake bits...
Done

================
Comment at: lib/asan/tests/CMakeLists.txt:100-135
@@ -105,6 +99,38 @@
 
-set(ASAN_NOINST_TEST_SOURCES
-  asan_noinst_test.cc
-  asan_test_main.cc
-)
+# This macro links all the object files provided in ${ARGN} into
+# a single executable, using just-built Clang and ASan-specific
+# link flags.
+# The resulting executable becomes a part of provided test_suite.
+macro(clang_link_asan_test test_suite executable)
+  get_test_directory(OUTPUT_DIR)
+  set(output_bin "${OUTPUT_DIR}/${executable}")
+  add_custom_command(
+    OUTPUT ${output_bin}
+    COMMAND clang ${ARGN} -o "${output_bin}"
+            ${ASAN_LINK_FLAGS}
+    DEPENDS clang ${ASAN_RUNTIME_LIBRARIES} ${ARGN}
+    )
+  add_custom_target(${executable}
+                    DEPENDS ${output_bin})
+  # Make the test suite depend on the binary.
+  add_dependencies(${test_suite} ${executable})
+endmacro()
+
+# This macro is used to compile a source file using just-built Clang
+# with a custom set of compile flags provided in ${ARGN}.
+# It appends a generated object file to the obj_list.
+macro(clang_compile_command obj_list source)
+  get_filename_component(source_rpath ${source} REALPATH)
+  get_filename_component(basename ${source} NAME)
+  set(output_obj "${basename}.o")
+  add_custom_command(
+    OUTPUT ${output_obj}
+    COMMAND clang ${ARGN}
+            -c -o "${output_obj}"
+            ${source_rpath}
+    MAIN_DEPENDENCY ${source}
+    DEPENDS clang gtest ${ASAN_RUNTIME_LIBRARIES} ${ASAN_BLACKLIST_FILE}
+    )
+  list(APPEND ${obj_list} ${output_obj})
+endmacro()
 
----------------
Chandler Carruth wrote:
> Finally this block should be abstracted up to where we can share it.
> 
> I think you should create a CMake module of functions for use in sanitizer tests, and include that module from the top-level compiler-rt CMake file... Make sense?
> 
> Also, we use function more than macro in the LLVM cmake...
OK, I've tried to generalize some compile/link stuff into separate cmake modules.

I was inspired by AddLLVM which consists of macro...


http://llvm-reviews.chandlerc.com/D188