[CMake] Use generator expression to get in-tree libc++ path

[phosek]

Summary

When using the in-tree libc++, we should be using the full path to
ensure that we're using the right library and not accidentally pick up
the system library.

[abrachet]

compiler-rt/CMakeLists.txt

540 ↗

(On Diff #403065)

Can $<BOOL:$<TARGET_NAME_IF_EXISTS:${cxx_target}>> ever evaluate false here?

compiler-rt/lib/memprof/tests/CMakeLists.txt

43

Stylistically should this instead append to MEMPROF_UNITTEST_LINK_FLAGS? Everywhere else you have added SANITIZER_TEST_CXX_LIBRARIES to their respective *LINK_FLAGS.

[ldionne]

I don't know compiler-rt much, but this looks reasonable.

[phosek]

compiler-rt/CMakeLists.txt

540 ↗

(On Diff #403065)

If the target doesn't exist, $<TARGET_NAME_IF_EXISTS:${cxx_target}> will evaluate to empty string and $<BOOL:$<TARGET_NAME_IF_EXISTS:${cxx_target}>> should evaluate that to false.

compiler-rt/lib/memprof/tests/CMakeLists.txt

43

Good question, I think that target_link_libraries is the correct solution because it handles other aspects like visibility (e.g. PRIVATE, PUBLIC). The reason we only use it here is because this is the only test that uses add_executable, all other tests use the generate_compiler_rt_tests function which internally uses add_compiler_rt_test which handles the compilation and linking manually via add_custom_command (this is so that it can use a custom compiler rather than CMAKE_{C,CXX}_COMPILER), see https://github.com/llvm/llvm-project/blob/db0631096e59c71b6f6b034306327b42d9184184/compiler-rt/cmake/Modules/AddCompilerRT.cmake#L497.

What I'm considering as a follow up is to (1) change MemProfUnitTests to also use generate_compiler_rt_tests for consistency with other tests and (2) change add_compiler_rt_test to use add_executable and other common CMake infrastructure.

[tambre]

compiler-rt/CMakeLists.txt

542–545 ↗

(On Diff #403401)

Why the explicit -l? Per the documentation it's considered as a linker flag while without as a plain library name (unless a target or a full path). I'd wager we mean the latter, so would be nice to use that.

[phosek]

compiler-rt/CMakeLists.txt

542–545 ↗

(On Diff #403401)

generate_compiler_rt_tests doesn't use target_link_libraries, it directly invokes the compiler which wouldn't handle plain library name unless it's a full path.

compiler-rt/lib/memprof/tests/CMakeLists.txt

 
 set(MEMPROF_UNIT_TEST_HEADERS
   ${MEMPROF_HEADERS})
 
 if(NOT WIN32)
   list(APPEND MEMPROF_UNITTEST_LINK_FLAGS -pthread)
 endif()
 
+set(MEMPROF_UNITTEST_LINK_LIBRARIES "dl")
 foreach(lib ${SANITIZER_TEST_CXX_LIBRARIES})
-  list(APPEND MEMPROF_UNITTEST_LINK_FLAGS -l${lib})
+  set(libname "$<$<BOOL:$<TARGET_NAME_IF_EXISTS:${lib}>>:$<TARGET_LINKER_FILE:${lib}>>")
+  list(APPEND MEMPROF_UNITTEST_LINK_LIBRARIES "${libname}")
 endforeach()
 
 if(COMPILER_RT_DEFAULT_TARGET_ARCH IN_LIST MEMPROF_SUPPORTED_ARCH)
   # MemProf unit tests are only run on the host machine.
   set(arch ${COMPILER_RT_DEFAULT_TARGET_ARCH})
 
   add_executable(MemProfUnitTests 
     ${MEMPROF_UNITTESTS}
     ${COMPILER_RT_GTEST_SOURCE}
     ${COMPILER_RT_GMOCK_SOURCE}
     ${MEMPROF_SOURCES}
     $<TARGET_OBJECTS:RTSanitizerCommon.${arch}>
     $<TARGET_OBJECTS:RTSanitizerCommonCoverage.${arch}>
     $<TARGET_OBJECTS:RTSanitizerCommonLibc.${arch}>
     $<TARGET_OBJECTS:RTSanitizerCommonSymbolizer.${arch}>)
   set_target_compile_flags(MemProfUnitTests ${MEMPROF_UNITTEST_CFLAGS})
   set_target_link_flags(MemProfUnitTests ${MEMPROF_UNITTEST_LINK_FLAGS})
-  target_link_libraries(MemProfUnitTests dl)
+  target_link_libraries(MemProfUnitTests ${MEMPROF_UNITTEST_LINK_LIBRARIES})
 
   if (TARGET cxx-headers OR HAVE_LIBCXX)
     add_dependencies(MemProfUnitTests cxx-headers)
   endif()
 
   set_target_properties(MemProfUnitTests PROPERTIES
     RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
 endif()