[CMake] Use correct visibility for linked libraries in CMake

[phosek]

Summary

When linking shared library, we shouldn't need to export linked
libraries to dependents, but we still need to do that for static
library. We should be explicit about this in target_link_libraries,
otherwise other targets that depend on these such as sanitizers get
repeated (and possibly even conflicting) dependencies.

[phosek]

We hit this issue when trying to link libclang_rt.asan.so with static unwinder but shared libc++abi where we would get two different instances of libunwind, one from direct dependency and one from libc++abi.

[ldionne]

THANKS.

[ldionne]

Wait, actually, doesn't CMake handle that automatically? I think that for STATIC libraries, even PRIVATE dependencies are forwarded to dependents, no?

[phosek]

In D57456#1377406, @ldionne wrote:

Wait, actually, doesn't CMake handle that automatically? I think that for STATIC libraries, even PRIVATE dependencies are forwarded to dependents, no?

Let me try this, if that's true than we could use PRIVATE everywhere.

[phosek]

We can use PRIVATE everywhere, CMake handles static libraries correctly.

benchmarks/CMakeLists.txt

 set(libcxx_benchmark_targets)
 
 function(add_benchmark_test name source_file)
   set(libcxx_target ${name}_libcxx)
   list(APPEND libcxx_benchmark_targets ${libcxx_target})
   add_executable(${libcxx_target} EXCLUDE_FROM_ALL ${source_file})
   add_dependencies(${libcxx_target} cxx cxx-headers google-benchmark-libcxx)
   add_dependencies(cxx-benchmarks ${libcxx_target})
+  target_link_libraries(${libcxx_target} ${LIBCXX_LIBRARIES})
   if (LIBCXX_ENABLE_SHARED)
     target_link_libraries(${libcxx_target} cxx_shared)
   else()
     target_link_libraries(${libcxx_target} cxx_static)
   endif()
   if (TARGET cxx_experimental)
     target_link_libraries(${libcxx_target} cxx_experimental)
   endif()

lib/CMakeLists.txt

 endif()
 
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED ${cxx_shared_sources})
   if(COMMAND llvm_setup_rpath)
     llvm_setup_rpath(cxx_shared)
   endif()
-  target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
+  target_link_libraries(cxx_shared PRIVATE ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
     PROPERTIES
       LINK_FLAGS    "${LIBCXX_LINK_FLAGS}"
       OUTPUT_NAME   "c++"
       VERSION       "${LIBCXX_ABI_VERSION}.0"
       SOVERSION     "${LIBCXX_ABI_VERSION}"
   )
   list(APPEND LIBCXX_BUILD_TARGETS "cxx_shared")
   if (LIBCXX_INSTALL_SHARED_LIBRARY)
     list(APPEND LIBCXX_INSTALL_TARGETS "cxx_shared")
   endif()
   if(WIN32 AND NOT MINGW AND NOT "${CMAKE_HOST_SYSTEM_NAME}" STREQUAL "Windows")
     # Since we most likely do not have a mt.exe replacement, disable the
     # manifest bundling.  This allows a normal cmake invocation to pass which
     # will attempt to use the manifest tool to generate the bundled manifest
     set_target_properties(cxx_shared PROPERTIES
                           APPEND_STRING PROPERTY LINK_FLAGS " /MANIFEST:NO")
   endif()
 endif()
 
 # Build the static library.
 if (LIBCXX_ENABLE_STATIC)
   add_library(cxx_static STATIC ${cxx_static_sources})
-  target_link_libraries(cxx_static ${LIBCXX_LIBRARIES})
+  target_link_libraries(cxx_static PRIVATE ${LIBCXX_LIBRARIES})
   set(CMAKE_STATIC_LIBRARY_PREFIX "lib")
   set_target_properties(cxx_static
     PROPERTIES
       LINK_FLAGS    "${LIBCXX_LINK_FLAGS}"
       OUTPUT_NAME   "c++"
   )
   list(APPEND LIBCXX_BUILD_TARGETS "cxx_static")
   if (LIBCXX_INSTALL_STATIC_LIBRARY)
add_custom_target(install-cxx-stripped
                               ${filesystem_lib_install_target}
                               ${header_install_target}
                       COMMAND "${CMAKE_COMMAND}"
                       -DCMAKE_INSTALL_COMPONENT=cxx
                       -DCMAKE_INSTALL_DO_STRIP=1
                       -P "${LIBCXX_BINARY_DIR}/cmake_install.cmake")
     add_custom_target(install-libcxx DEPENDS install-cxx)
 endif()
+
+# TODO: This is needed by cxx-benchmarks but this variable isn't
+# available outside of the scope of this file so we need to export
+# it. This is not necessarily the cleanest solution.
+set(LIBCXX_LIBRARIES ${LIBCXX_LIBRARIES} PARENT_SCOPE)