[libc++] Do not cleverly link against libc++abi just because it happens to be there

[ldionne]

Summary

Otherwise, when libcxxabi is not an enabled project in the monorepo, we
get a link error because we try to link against non-existent cxxabi_shared.

More generally, we shouldn't change the behavior of the build based on
implicit things like whether a file happens to be at a specific path or
not.

[ldionne]

This solves the problem that currently, NOT specifying LLVM_ENABLE_PROJECTS=libcxxabi and building libc++ will fail because cxxabi_shared can't be found.

Still an open question is whether we need to retain the in-tree include paths of libc++abi, or whether linking against the cxxabi_shared target is enough. @EricWF, can you remind me why we install libc++abi headers in include/c++/v1 again?

[thakis]

Here's my take on this, which I hadn't had time to follow up on: https://reviews.llvm.org/D62060 Maybe it provides at least some mild entertainment.

[ldionne]

In D63883#1561467, @thakis wrote:

Here's my take on this, which I hadn't had time to follow up on: https://reviews.llvm.org/D62060 Maybe it provides at least some mild entertainment.

Your approach is fine too, but I like mine better because it gets rid of LIBCXX_CXX_ABI_INTREE and removes more complexity than yours.

[mgorny]

Am I guessing correctly that this broke the buildbots (again)?

http://lab.llvm.org:8011/builders/netbsd-amd64/builds/20712/steps/ninja%20build%20local/logs/stdio

[vitalybuka]

Reverted with r365359

[ldionne]

Yes. Eric told me the bots were now specifying the ABI library to be libc++abi explicitly, but apparently that's not true (or at least not for some of them).

[ldionne]

I'll coordinate with him to check this in again once the build bots have the fix.

[ldionne]

Okay, so what happened is that it usually takes about a week for changes to the build bot configuration to actually be reflected on the bots. I should have known that. I'll wait before I recommit this.

[vitalybuka]

Sanitizers use sh scripts which can be checked out and executed on most linux dev workstations.
http://llvm.org/viewvc/llvm-project/zorg/trunk/zorg/buildbot/builders/sanitizers/
Changes to these scripts are effective immediately.

[mgorny]

Could you tell me specifically what change to buildbot is requested? I must've missed the original thread.

[ldionne]

In D63883#1574119, @mgorny wrote:

Could you tell me specifically what change to buildbot is requested? I must've missed the original thread.

The build bots must specify -DLIBCXX_CXX_ABI=libcxxabi at the CMake configure step. Otherwise, they'll end up using the default ABI library on Linux, which uses the xxx_fallback.ipp files from libc++, and that doesn't implement everything we need.

[mgorny]

In D63883#1574132, @ldionne wrote:

The build bots must specify -DLIBCXX_CXX_ABI=libcxxabi at the CMake configure step. Otherwise, they'll end up using the default ABI library on Linux, which uses the xxx_fallback.ipp files from libc++, and that doesn't implement everything we need.

Thanks. I've added that to NetBSD buildbot, so feel free to recommit once others are ready.

[smeenai]

I think it's worth advertising the required configuration change (to explicitly say -DLIBCXX_CXX_ABI=libcxxabi) in the documentation and/or the release notes, otherwise the behavior change will be surprising.

[ldionne]

I'll monitor the bots, but if someone gets here because their build is broken by this commit, it's fine to revert it (but please notify me)!

For some reason, this change appears to be very trick to land.

libcxx/CMakeLists.txt

 # ABI Library options ---------------------------------------------------------
 set(LIBCXX_CXX_ABI "default" CACHE STRING
     "Specify C++ ABI library to use.")
 set(CXXABIS none default libcxxabi libcxxrt libstdc++ libsupc++ vcruntime)
 set_property(CACHE LIBCXX_CXX_ABI PROPERTY STRINGS ;${CXXABIS})
 
 # Setup the default options if LIBCXX_CXX_ABI is not specified.
 if (LIBCXX_CXX_ABI STREQUAL "default")
-  find_path(
-    LIBCXX_LIBCXXABI_INCLUDES_INTERNAL cxxabi.h
-    PATHS ${LLVM_MAIN_SRC_DIR}/projects/libcxxabi/include
-          ${LLVM_MAIN_SRC_DIR}/runtimes/libcxxabi/include
-          ${LLVM_MAIN_SRC_DIR}/../libcxxabi/include
-    NO_DEFAULT_PATH
-    NO_CMAKE_FIND_ROOT_PATH
-  )
   if (LIBCXX_TARGETING_MSVC)
     # FIXME: Figure out how to configure the ABI library on Windows.
     set(LIBCXX_CXX_ABI_LIBNAME "vcruntime")
-  elseif ((NOT LIBCXX_STANDALONE_BUILD OR HAVE_LIBCXXABI) AND
-          IS_DIRECTORY "${LIBCXX_LIBCXXABI_INCLUDES_INTERNAL}")
-    set(LIBCXX_CXX_ABI_LIBNAME "libcxxabi")
-    set(LIBCXX_CXX_ABI_INCLUDE_PATHS "${LIBCXX_LIBCXXABI_INCLUDES_INTERNAL}")
-    set(LIBCXX_CXX_ABI_INTREE 1)
   elseif (APPLE)
     set(LIBCXX_CXX_ABI_LIBNAME "libcxxabi")
     set(LIBCXX_CXX_ABI_SYSTEM 1)
   elseif (${CMAKE_SYSTEM_NAME} MATCHES "FreeBSD")
     set(LIBCXX_CXX_ABI_LIBNAME "libcxxrt")
     set(LIBCXX_CXX_ABI_INCLUDE_PATHS "/usr/include/c++/v1")
   else()
     set(LIBCXX_CXX_ABI_LIBNAME "default")

libcxx/cmake/Modules/HandleLibCXXABI.cmake

else()
     set(_LIBSUPCXX_DEFINES "")
     set(_LIBSUPCXX_LIBNAME supc++)
   endif()
   setup_abi_lib(
     "-D__GLIBCXX__ ${_LIBSUPCXX_DEFINES}"
     "${_LIBSUPCXX_LIBNAME}" "${_LIBSUPCXX_LIBNAME}" "${_LIBSUPCXX_INCLUDE_FILES}" "bits"
     )
 elseif ("${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libcxxabi")
-  if (LIBCXX_CXX_ABI_INTREE)
+  if (TARGET cxxabi_shared AND TARGET cxxabi_static)
     # Link against just-built "cxxabi" target.
     set(CXXABI_SHARED_LIBNAME cxxabi_shared)
     set(CXXABI_STATIC_LIBNAME cxxabi_static)
   else()
     # Assume c++abi is installed in the system, rely on -lc++abi link flag.
     set(CXXABI_SHARED_LIBNAME "c++abi")
     set(CXXABI_STATIC_LIBNAME "c++abi")
   endif()