Skip to content

Commit

Permalink
[libc++] Simplify the configuration of the C++ ABI library
Browse files Browse the repository at this point in the history
This commit removes support for building against the system libc++abi,
which was supported on Apple platforms. This is basically never what we
want to do, since libc++ and libc++abi are coupled and building a trunk
libc++ against an older libc++abi can lead to incompatibilities (and
good luck debugging them!). It might have made some sense to support
that when the monorepo did not exist, however I don't think this is
anything but a footgun nowadays.

Furthermore, based on the newly-made assumption that we're building
against the monorepo libc++abi, we can simplify the search path logic
for finding libc++abi.

This area of our build system has a lot of technical debt accumulated,
and it's surprisingly difficult to change. We've tried different things
and failed several times in the past. I did test this change on our
Docker image for the build bots and on Apple platforms, however it is
possible that this breaks some unknown configuration, in which case it
should be fine to revert this (so we can try again!).
  • Loading branch information
ldionne committed Apr 2, 2020
1 parent cbd3969 commit 61e8973
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 44 deletions.
32 changes: 3 additions & 29 deletions libcxx/CMakeLists.txt
Expand Up @@ -149,45 +149,19 @@ set(LIBCXX_LIBCPPABI_VERSION "2" CACHE STRING "Version of libc++abi's ABI to re-
Note that this is not related to the version of libc++'s ABI itself!")

# ABI Library options ---------------------------------------------------------
set(LIBCXX_CXX_ABI "default" CACHE STRING
"Specify C++ ABI library to use.")
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})

# FIXME: This is a temporary hack to get the buildbots working while D63883 is in flight.
# Without this all the bots fail while building libc++
if (DEFINED ENV{USER})
if (("$ENV{USER}" STREQUAL "buildbot") OR (("$ENV{USER}" STREQUAL "llvmbb") OR ("$ENV{USER}" STREQUAL "buildslave")))
if (LIBCXX_CXX_ABI STREQUAL "libcxxabi" AND NOT DEFINED LIBCXX_CXX_ABI_INCLUDE_PATHS)
message(WARNING "OVERRIDING BUILDBOT CONFIG")
set(LIBCXX_CXX_ABI "default" CACHE STRING "FIXME" FORCE)
endif()
endif()
endif()
# 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")
elseif (NOT LIBCXX_STANDALONE_BUILD OR HAVE_LIBCXXABI)
set(LIBCXX_CXX_ABI_LIBNAME "libcxxabi")
else()
set(LIBCXX_CXX_ABI_LIBNAME "default")
endif()
Expand Down
18 changes: 3 additions & 15 deletions libcxx/cmake/Modules/HandleLibCXXABI.cmake
Expand Up @@ -102,24 +102,12 @@ if ("${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libstdc++" OR
"${_LIBSUPCXX_LIBNAME}" "${_LIBSUPCXX_LIBNAME}" "${_LIBSUPCXX_INCLUDE_FILES}" "bits"
)
elseif ("${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libcxxabi")
if (LIBCXX_CXX_ABI_INTREE)
# 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()
if (LIBCXX_CXX_ABI_SYSTEM)
set(HEADERS "")
else()
set(HEADERS "cxxabi.h;__cxxabi_config.h")
endif()
set(LIBCXX_CXX_ABI_INCLUDE_PATHS "${LIBCXX_SOURCE_DIR}/../libcxxabi/include")
setup_abi_lib(
"-DLIBCXX_BUILDING_LIBCXXABI"
"${CXXABI_SHARED_LIBNAME}" "${CXXABI_STATIC_LIBNAME}" "${HEADERS}" "")
"cxxabi_shared" "cxxabi_static" "cxxabi.h;__cxxabi_config.h" "")

This comment has been minimized.

Copy link
@tambry

tambry Apr 2, 2020

Contributor

This breaks standalone builds, where cxxabi_shared and cxxabi_static don't exist as targets.

elseif ("${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libcxxrt")
set(LIBCXX_CXX_ABI_INCLUDE_PATHS "/usr/include/c++/v1")
setup_abi_lib(
"-DLIBCXXRT"
"cxxrt" "cxxrt" "cxxabi.h;unwind.h;unwind-arm.h;unwind-itanium.h" ""
Expand Down

0 comments on commit 61e8973

Please sign in to comment.