[Fuzzer] Avoid the unnecessary rebuild of the custom libc++

[phosek]

Summary

This changes the add_custom_libcxx macro to resemble the
llvm_ExternalProject_Add. The primary motivation is to avoid
unnecessary libFuzzer rebuilds that are being done on every
Ninja/Make invocation. The libc++ should be only rebuilt whenever
the libc++ source itself changes.

[phosek]

ping?

[vitalybuka]

compiler-rt/cmake/Modules/AddCompilerRT.cmake

530 ↗

(On Diff #133954)

Can you please add into patch description explanation when it is expected to be rebuilt

539 ↗

(On Diff #133954)

Have you considered to achieve the same with just:
BUILD_ALWAYS 0
BUILD_BYPRODUCTS <libs>

541 ↗

(On Diff #133954)

Why do you need USES_TERMINAL_ ?

compiler-rt/lib/fuzzer/CMakeLists.txt

89 ↗

(On Diff #133954)

Can this be just ${dir}/src/libcxx_fuzzer_${arch}/lib/libc++.a ?

compiler-rt/test/tsan/lit.cfg

59 ↗

(On Diff #133954)

why do we need "-bins" suffix?

[morehouse]

Why not just call llvm_ExternalProject_Add rather than duplicating here?

compiler-rt/cmake/Modules/AddCompilerRT.cmake

554 ↗

(On Diff #133954)

Unused if.

[phosek]

In D43213#1016841, @morehouse wrote:

Why not just call llvm_ExternalProject_Add rather than duplicating here?

Don't we still want to support the setup where compiler-rt is built as a standalone project in which case llvm_ExternalProject_Add wouldn't be available?

[morehouse]

In D43213#1020043, @phosek wrote:

In D43213#1016841, @morehouse wrote:

Why not just call llvm_ExternalProject_Add rather than duplicating here?

Don't we still want to support the setup where compiler-rt is built as a standalone project in which case llvm_ExternalProject_Add wouldn't be available?

In that case, llvm_ExternalProject_BuildCmd won't be available either but is still used by this patch.

[phosek]

In D43213#1020058, @morehouse wrote:

In D43213#1020043, @phosek wrote:

In D43213#1016841, @morehouse wrote:

Why not just call llvm_ExternalProject_Add rather than duplicating here?

Don't we still want to support the setup where compiler-rt is built as a standalone project in which case llvm_ExternalProject_Add wouldn't be available?

In that case, llvm_ExternalProject_BuildCmd won't be available either but is still used by this patch.

That's a mistake, thanks for catching that!

[phosek]

compiler-rt/cmake/Modules/AddCompilerRT.cmake

539 ↗

(On Diff #133954)

The problem is that we would need to list all the library artifacts, e.g. libc++.so is only a symlink to libc++.so.1 which is a symlink to libc++.so.1.0, and I don't know if we want to rely on those details here?

541 ↗

(On Diff #133954)

This is not really needed, but now that libc++ is being used as part of the libFuzzer build, I just found it much easier to debug build issues if the libFuzzer's libc++ build output goes to terminal rather than file log which requires extra steps to read, I'm happy to drop this if you don't like it though.

compiler-rt/lib/fuzzer/CMakeLists.txt

89 ↗

(On Diff #133954)

That directory no longer exists, all the binary artifacts now end up in the -bins directory.

compiler-rt/test/tsan/lit.cfg

59 ↗

(On Diff #133954)

I was following the naming convention used by llvm_ExternalProject_Add, but I can drop the "-bins" suffix if you prefer?

[morehouse]

compiler-rt/cmake/Modules/AddCompilerRT.cmake

522 ↗

(On Diff #136029)

Why don't we depend on LIBCXX_DEPS here?

[vitalybuka]

compiler-rt/cmake/Modules/AddCompilerRT.cmake

541 ↗

(On Diff #133954)

up to you.

compiler-rt/lib/fuzzer/CMakeLists.txt

89 ↗

(On Diff #133954)

So why to use ${dir}-bins instead of ${dir}?
I'd prefer the name without suffix here

compiler-rt/test/tsan/lit.cfg

59 ↗

(On Diff #133954)

This is the primary location which will be used for linking? i'd prefer without suffix

cmake/Modules/AddCompilerRT.cmake

macro(add_custom_libcxx name prefix)
   if(NOT COMPILER_RT_LIBCXX_PATH)
     message(FATAL_ERROR "libcxx not found!")
   endif()
 
   cmake_parse_arguments(LIBCXX "USE_TOOLCHAIN" "" "DEPS;CFLAGS;CMAKE_ARGS" ${ARGN})
   foreach(flag ${LIBCXX_CFLAGS})
     set(flagstr "${flagstr} ${flag}")
   endforeach()
-  set(LIBCXX_CFLAGS ${flagstr})
+  set(LIBCXX_C_FLAGS ${flagstr})
+  set(LIBCXX_CXX_FLAGS ${flagstr})
 
   if(LIBCXX_USE_TOOLCHAIN)
     set(compiler_args -DCMAKE_C_COMPILER=${COMPILER_RT_TEST_COMPILER}
                       -DCMAKE_CXX_COMPILER=${COMPILER_RT_TEST_CXX_COMPILER})
     if(NOT COMPILER_RT_STANDALONE_BUILD)
-      set(force_deps DEPENDS clang)
+      set(toolchain_deps $<TARGET_FILE:clang>)
+      set(force_deps DEPENDS $<TARGET_FILE:clang>)
     endif()
   else()
     set(compiler_args -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER}
                       -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER})
   endif()
 
+  set(STAMP_DIR ${prefix}-stamps/)
+  set(BINARY_DIR ${prefix}-bins/)
+
+  add_custom_target(${name}-clear
+    COMMAND ${CMAKE_COMMAND} -E remove_directory ${BINARY_DIR}
+    COMMAND ${CMAKE_COMMAND} -E remove_directory ${STAMP_DIR}
+    COMMENT "Clobbering ${name} build and stamp directories"
+    USES_TERMINAL
+    )
+
+  add_custom_command(
+    OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${name}-clobber-stamp
+    DEPENDS ${LIBCXX_DEPS} ${toolchain_deps}
+    COMMAND ${CMAKE_COMMAND} -E touch ${BINARY_DIR}/CMakeCache.txt
+    COMMAND ${CMAKE_COMMAND} -E touch ${STAMP_DIR}/${name}-mkdir
+    COMMAND ${CMAKE_COMMAND} -E touch ${CMAKE_CURRENT_BINARY_DIR}/${name}-clobber-stamp
+    COMMENT "Clobbering bootstrap build and stamp directories"
+    )
+
+  add_custom_target(${name}-clobber
+    DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/${name}-clobber-stamp)
+
   if(CMAKE_SYSROOT)
     set(sysroot_arg -DCMAKE_SYSROOT=${CMAKE_SYSROOT})
   endif()
 
   ExternalProject_Add(${name}
-    DEPENDS ${LIBCXX_DEPS}
+    DEPENDS ${name}-clobber ${LIBCXX_DEPS}
     PREFIX ${prefix}
     SOURCE_DIR ${COMPILER_RT_LIBCXX_PATH}
+    STAMP_DIR ${STAMP_DIR}
+    BINARY_DIR ${BINARY_DIR}
     CMAKE_ARGS -DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}
                ${compiler_args}
                ${sysroot_arg}
-               -DCMAKE_C_FLAGS=${LIBCXX_CFLAGS}
-               -DCMAKE_CXX_FLAGS=${LIBCXX_CFLAGS}
+               -DCMAKE_C_FLAGS=${LIBCXX_C_FLAGS}
+               -DCMAKE_CXX_FLAGS=${LIBCXX_CXX_FLAGS}
                -DCMAKE_BUILD_TYPE=Release
-               -DCMAKE_INSTALL_PREFIX=<INSTALL_DIR>
+               -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}
                -DLLVM_PATH=${LLVM_MAIN_SRC_DIR}
-               -DLIBCXX_STANDALONE_BUILD=On
+               -DLLVM_BINARY_DIR=${prefix}
+               -DLLVM_LIBRARY_OUTPUT_INTDIR=${prefix}/lib
+               -DLIBCXX_STANDALONE_BUILD=ON
                ${LIBCXX_CMAKE_ARGS}
-    STEP_TARGETS configure build install
-    LOG_BUILD 1
-    LOG_CONFIGURE 1
-    LOG_INSTALL 1
+    INSTALL_COMMAND ""
+    STEP_TARGETS configure build
+    BUILD_ALWAYS 1
+    USES_TERMINAL_CONFIGURE 1
+    USES_TERMINAL_BUILD 1
+    USES_TERMINAL_INSTALL 1
     EXCLUDE_FROM_ALL TRUE
     )
 
-  ExternalProject_Add_Step(${name} force-reconfigure
-    DEPENDERS configure
-    ALWAYS 1
-    )
+  if (CMAKE_GENERATOR MATCHES "Make")
+    set(run_clean "$(MAKE)" "-C" "${BINARY_DIR}" "clean")
+  else()
+    set(run_clean ${CMAKE_COMMAND} --build ${BINARY_DIR} --target clean
+                                   --config "$<CONFIGURATION>")
+  endif()
 
-  ExternalProject_Add_Step(${name} clobber
-    COMMAND ${CMAKE_COMMAND} -E remove_directory <BINARY_DIR>
-    COMMAND ${CMAKE_COMMAND} -E make_directory <BINARY_DIR>
-    COMMENT "Clobberring ${name} build directory..."
-    DEPENDERS configure
+  ExternalProject_Add_Step(${name} clean
+    COMMAND ${run_clean}
+    COMMENT "Cleaning ${name}..."
+    DEPENDEES configure
     ${force_deps}
+    WORKING_DIRECTORY ${BINARY_DIR}
+    EXCLUDE_FROM_MAIN 1
+    USES_TERMINAL 1
     )
+  ExternalProject_Add_StepTargets(${name} clean)
+
+  if(LIBCXX_USE_TOOLCHAIN)
+    add_dependencies(${name}-clean ${name}-clobber)
+    set_target_properties(${name}-clean PROPERTIES
+      SOURCES ${CMAKE_CURRENT_BINARY_DIR}/${name}-clobber-stamp)
+  endif()
 endmacro()
 
 function(rt_externalize_debuginfo name)
   if(NOT COMPILER_RT_EXTERNALIZE_DEBUGINFO)
     return()
   endif()
 
   if(NOT COMPILER_RT_EXTERNALIZE_DEBUGINFO_SKIP_STRIP)

lib/fuzzer/CMakeLists.txt

add_compiler_rt_runtime(clang_rt.fuzzer_no_main
   CFLAGS ${LIBFUZZER_CFLAGS}
   PARENT_TARGET fuzzer)
 
 if("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux" AND COMPILER_RT_LIBCXX_PATH)
   macro(partially_link_libcxx name dir arch)
     set(cxx_${arch}_merge_dir "${CMAKE_CURRENT_BINARY_DIR}/cxx_${arch}_merge.dir")
     file(MAKE_DIRECTORY ${cxx_${arch}_merge_dir})
     add_custom_command(TARGET clang_rt.${name}-${arch} POST_BUILD
-      COMMAND ${CMAKE_LINKER} --whole-archive "$<TARGET_LINKER_FILE:clang_rt.${name}-${arch}>" --no-whole-archive ${dir}/src/libcxx_fuzzer_${arch}-build/lib/libc++.a -r -o ${name}.o
+      COMMAND ${CMAKE_LINKER} --whole-archive "$<TARGET_LINKER_FILE:clang_rt.${name}-${arch}>" --no-whole-archive ${dir}/lib/libc++.a -r -o ${name}.o
       COMMAND ${CMAKE_OBJCOPY} --localize-hidden ${name}.o
       COMMAND ${CMAKE_COMMAND} -E remove "$<TARGET_LINKER_FILE:clang_rt.${name}-${arch}>"
       COMMAND ${CMAKE_AR} qcs "$<TARGET_LINKER_FILE:clang_rt.${name}-${arch}>" ${name}.o
       WORKING_DIRECTORY ${cxx_${arch}_merge_dir}
     )
   endmacro()
 
   foreach(arch ${FUZZER_SUPPORTED_ARCH})
     get_target_flags_for_arch(${arch} TARGET_CFLAGS)
     set(LIBCXX_${arch}_PREFIX ${CMAKE_CURRENT_BINARY_DIR}/libcxx_fuzzer_${arch})
     add_custom_libcxx(libcxx_fuzzer_${arch} ${LIBCXX_${arch}_PREFIX}
       CFLAGS ${TARGET_CFLAGS}
              -D_LIBCPP_ABI_VERSION=Fuzzer
              -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS=1
              -fvisibility=hidden
       CMAKE_ARGS -DLIBCXX_ENABLE_EXCEPTIONS=OFF
                  -DLIBCXX_CXX_ABI=none)
-    target_compile_options(RTfuzzer.${arch} PRIVATE -isystem ${COMPILER_RT_LIBCXX_PATH}/include)
+    target_compile_options(RTfuzzer.${arch} PRIVATE -isystem ${LIBCXX_${arch}_PREFIX}/include/c++/v1)
     add_dependencies(RTfuzzer.${arch} libcxx_fuzzer_${arch}-build)
-    target_compile_options(RTfuzzer_main.${arch} PRIVATE -isystem ${COMPILER_RT_LIBCXX_PATH}/include)
+    target_compile_options(RTfuzzer_main.${arch} PRIVATE -isystem ${LIBCXX_${arch}_PREFIX}/include/c++/v1)
     add_dependencies(RTfuzzer_main.${arch} libcxx_fuzzer_${arch}-build)
     partially_link_libcxx(fuzzer_no_main ${LIBCXX_${arch}_PREFIX} ${arch})
     partially_link_libcxx(fuzzer ${LIBCXX_${arch}_PREFIX} ${arch})
   endforeach()
 endif()
 
 if(COMPILER_RT_INCLUDE_TESTS)
   add_subdirectory(tests)
 endif()

lib/fuzzer/tests/CMakeLists.txt

add_library(${LIBFUZZER_TEST_RUNTIME} STATIC
     ${LIBFUZZER_TEST_RUNTIME_OBJECTS})
   set_target_properties(${LIBFUZZER_TEST_RUNTIME} PROPERTIES
     ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
     FOLDER "Compiler-RT Runtime tests")
 
   if("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux" AND COMPILER_RT_LIBCXX_PATH)
     set(LIBFUZZER_TEST_RUNTIME_DEPS libcxx_fuzzer_${arch}-build)
     set(LIBFUZZER_TEST_RUNTIME_CFLAGS -isystem ${COMPILER_RT_LIBCXX_PATH}/include)
-    set(LIBFUZZER_TEST_RUNTIME_LINK_FLAGS ${LIBCXX_${arch}_PREFIX}/src/libcxx_fuzzer_${arch}-build/lib/libc++.a)
+    set(LIBFUZZER_TEST_RUNTIME_LINK_FLAGS ${LIBCXX_${arch}_PREFIX}/lib/libc++.a)
   endif()
 
   set(FuzzerTestObjects)
   generate_compiler_rt_tests(FuzzerTestObjects
     FuzzerUnitTests "Fuzzer-${arch}-Test" ${arch}
     SOURCES FuzzerUnittest.cpp ${COMPILER_RT_GTEST_SOURCE}
     RUNTIME ${LIBFUZZER_TEST_RUNTIME}
     DEPS gtest ${LIBFUZZER_TEST_RUNTIME_DEPS}
     CFLAGS ${LIBFUZZER_UNITTEST_CFLAGS} ${LIBFUZZER_TEST_RUNTIME_CFLAGS}
     LINK_FLAGS ${LIBFUZZER_UNITTEST_LINK_FLAGS} ${LIBFUZZER_TEST_RUNTIME_LINK_FLAGS})
   set_target_properties(FuzzerUnitTests PROPERTIES
     RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
 endforeach()

lib/msan/tests/CMakeLists.txt

macro(add_msan_tests_for_arch arch kind cflags)
 
   # Instrumented loadable library tests.
   set(MSAN_LOADABLE_SO)
   msan_link_shared(MSAN_LOADABLE_SO "libmsan_loadable" ${arch} "${kind}"
                    OBJECTS ${MSAN_INST_LOADABLE_OBJECTS}
                    DEPS ${MSAN_INST_LOADABLE_OBJECTS})
 
   set(MSAN_TEST_OBJECTS ${MSAN_INST_TEST_OBJECTS} ${MSAN_INST_GTEST})
-  set(MSAN_TEST_DEPS ${MSAN_TEST_OBJECTS} libcxx_msan_${arch}-install
+  set(MSAN_TEST_DEPS ${MSAN_TEST_OBJECTS} libcxx_msan_${arch}-build
                      ${MSAN_LOADABLE_SO})
   if(NOT COMPILER_RT_STANDALONE_BUILD)
     list(APPEND MSAN_TEST_DEPS msan)
   endif()
   get_target_flags_for_arch(${arch} TARGET_LINK_FLAGS)
   add_compiler_rt_test(MsanUnitTests "Msan-${arch}${kind}-Test" ${arch}
     OBJECTS ${MSAN_TEST_OBJECTS} ${MSAN_LIBCXX_SO}
     DEPS ${MSAN_TEST_DEPS}

lib/tsan/CMakeLists.txt

if(COMPILER_RT_LIBCXX_PATH AND
   set(libcxx_tsan_deps)
   foreach(arch ${TSAN_SUPPORTED_ARCH})
     get_target_flags_for_arch(${arch} TARGET_CFLAGS)
     set(LIBCXX_PREFIX ${CMAKE_CURRENT_BINARY_DIR}/libcxx_tsan_${arch})
     add_custom_libcxx(libcxx_tsan_${arch} ${LIBCXX_PREFIX}
       DEPS ${TSAN_RUNTIME_LIBRARIES}
       CFLAGS ${TARGET_CFLAGS} -fsanitize=thread
       USE_TOOLCHAIN)
-    list(APPEND libcxx_tsan_deps libcxx_tsan_${arch}-install)
+    list(APPEND libcxx_tsan_deps libcxx_tsan_${arch}-build)
   endforeach()
 
   add_custom_target(libcxx_tsan DEPENDS ${libcxx_tsan_deps})
 endif()
 
 if(COMPILER_RT_INCLUDE_TESTS)
   add_subdirectory(tests)
 endif()

test/fuzzer/lit.cfg

 import lit.formats
 import sys
 import os
 
 config.name = "LLVMFuzzer"
 config.test_format = lit.formats.ShTest(True)
 config.suffixes = ['.test']
 config.test_source_root = os.path.dirname(__file__)
 
-config.environment['LD_LIBRARY_PATH'] = config.llvm_library_dir
-
 # Choose between lit's internal shell pipeline runner and a real shell.  If
 # LIT_USE_INTERNAL_SHELL is in the environment, we use that as an override.
 use_lit_shell = os.environ.get("LIT_USE_INTERNAL_SHELL")
 if use_lit_shell:
     # 0 is external, "" is default, and everything else is internal.
     execute_external = (use_lit_shell == "0")
 else:
     # Otherwise we default to internal on Windows and external elsewhere, as

 
 config.substitutions.append(('%build_dir', config.cmake_binary_dir))
 libfuzzer_src_root = os.path.join(config.compiler_rt_src_root, "lib", "fuzzer")
 config.substitutions.append(('%libfuzzer_src', libfuzzer_src_root))
 
 def generate_compiler_cmd(is_cpp=True, fuzzer_enabled=True):
   compiler_cmd = config.c_compiler
   if config.clang and config.stdlib == 'libc++':
-    link_cmd = '-stdlib=libc++'
+    link_cmd = '-stdlib=libc++ -Wl,-rpath=%s' % config.llvm_library_dir
   elif config.clang and config.stdlib == 'static-libc++':
-    link_cmd = '-stdlib=libc++ -lc++abi -static-libstdc++'
+    link_cmd = '-stdlib=libc++ -lc++abi -static-libstdc++ -Wl,-rpath=%s' % config.llvm_library_dir
   else:
     link_cmd = '-lc++' if any(x in config.target_triple for x in ('darwin', 'freebsd')) else '-lstdc++'
   std_cmd = '--driver-mode=g++ -std=c++11' if is_cpp else ''
   sanitizers = ['address']
   if fuzzer_enabled:
     sanitizers.append('fuzzer')
   sanitizers_cmd = ('-fsanitize=%s' % ','.join(sanitizers))
   isysroot_cmd = config.osx_sysroot_flag if config.osx_sysroot_flag else ''

test/tsan/lit.cfg

clang_tsan_cflags = (["-fsanitize=thread",
                       extra_cflags +
                       ["-I%s" % tsan_incdir])
 clang_tsan_cxxflags = config.cxx_mode_flags + clang_tsan_cflags + ["-std=c++11"] + ["-I%s" % tsan_incdir]
 # Add additional flags if we're using instrumented libc++.
 # Instrumented libcxx currently not supported on Darwin.
 if config.has_libcxx and config.host_os != 'Darwin':
   # FIXME: Dehardcode this path somehow.
   libcxx_path = os.path.join(config.compiler_rt_obj_root, "lib",
-                             "tsan", "libcxx_tsan_" + config.target_arch)
+                             "tsan", "libcxx_tsan_%s" % config.target_arch)
   libcxx_incdir = os.path.join(libcxx_path, "include", "c++", "v1")
   libcxx_libdir = os.path.join(libcxx_path, "lib")
   libcxx_so = os.path.join(libcxx_libdir, "libc++.so")
   clang_tsan_cxxflags += ["-nostdinc++",
                           "-I%s" % libcxx_incdir,
                           libcxx_so,
                           "-Wl,-rpath=%s" % libcxx_libdir]