[libcxxabi] Use cxx-headers target to consume libcxx headers

[phosek]

Summary

Rather than including libc++ include dir, use the cxx-headers target.

[compnerd]

libcxxabi/CMakeLists.txt

159–175

Why not make this if (NOT TARGET cxx-headers)? You don't really care so much about the standalone target as much as you do about the cxx-headers target existing.

164

I believe that with the suggestion below, this check is unnecessary as CMake will verify that the path exists and is a directory.

166–170

There is no need for this complicated special handling, why not just let CMake handle it?

[phosek]

libcxxabi/CMakeLists.txt

159–175

The issue is that depending on the order in which users put projects in LLVM_ENABLE_PROJECTS or LLVM_ENABLE_RUNTIMES, cxx-headers target may not yet exist, see also D97314 for a related discussion.

166–170

We tried to use that approach in the past but it broke the runtimes build with what appeared to be a CMake bug where it would simply drop the include on the floor on certain platforms like macOS, see D82702 for some of the background discussion.

We could try and revisit this but I'd prefer to do it in a follow up change.

[phosek]

@ldionne do you have any thoughts on this change? This was split from D97572.

[ldionne]

libcxxabi/CMakeLists.txt

166–170

Can you please create that follow-up change right now and make it a child of this patch?

[ldionne]

Actually, I accepted a bit too fast. I'm fine with the patch but there seems to be a problem with it, since CI is failing.

[phosek]

In D98367#2635024, @ldionne wrote:

Actually, I accepted a bit too fast. I'm fine with the patch but there seems to be a problem with it, since CI is failing.

It seems to be passing now.

[uabelho]

Hi,

I don't know what is happening at all, but I noticed that when I do "ninja check-runtimes" with this patch it fails with

[0/1] Running all regression tests
llvm-lit: /repo/uabelho/master-github/llvm/build-all-builtins/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/libcxxabi/test/lit.site.cfg:60: note: Using configuration variant: libcxxabi
llvm-lit: /repo/uabelho/master-github/libcxx/utils/libcxx/test/config.py:632: note: Setting target_triple to x86_64-unknown-linux-gnu
llvm-lit: /repo/uabelho/master-github/libcxxabi/test/libcxxabi/test/config.py:67: fatal: cxx_headers='/repo/uabelho/master-github/llvm/build-all-builtins/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/include/c++/v1' is not a directory.

The directory
/repo/uabelho/master-github/llvm/build-all-builtins/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/include/c++/v1
doesn't exist. The dir
/repo/uabelho/master-github/llvm/build-all-builtins/runtimes/runtimes-x86_64-unknown-linux-gnu-bins
exists, but there is no "include" in it.
If I search for "v1" in my build dir I find

build-all-builtins/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/compiler-rt/lib/fuzzer/libcxx_fuzzer_x86_64/include/c++/v1
build-all-builtins/include/c++/v1

Any idea what's up?

[bjope]

libcxxabi/test/libcxxabi/test/config.py

61 ↗

(On Diff #332755)

Looking at the same problem as @uabelho asked about. It works better if changing this line to
os.path.join(self.cxx_library_root, '..', '..', '..', 'include', 'c++', 'v1')
but maybe that isn't getting the intended headers?

[phosek]

What's your CMake configuration? I'd like to try and reproduce this locally so I can investigate.

[vvereschaka]

Hi @phosek ,

we got the same thing on the win-x-arm cross toolchain builders as described by @uabelho

cmake --build . --target check-cxxabi
...

-- Build files have been written to: C:/buildbot/temp/build/runtimes/runtimes-bins
[4/5] cmd.exe /C "cd /D C:\buildbot\temp\build\runtimes\runtimes-bin...uild/runtimes/runtimes-bins/ --target check-cxxabi --config Release"
[0/1] Running libcxxabi tests
llvm-lit.py: c:\buildbot\temp\build\runtimes\runtimes-bins\libcxxabi\test\lit.site.cfg:60: note: Using configuration variant: libcxxabi
llvm-lit.py: C:\buildbot\temp\llvm-project\libcxx\utils\libcxx\test\target_info.py:152: note: inferred target_info as: 'libcxx.test.target_info.LinuxRemoteTI'
llvm-lit.py: C:\buildbot\temp\llvm-project\libcxx\utils\libcxx\test\config.py:632: note: Setting target_triple to aarch64-linux-gnu
llvm-lit.py: C:\buildbot\temp\llvm-project\libcxxabi\test\libcxxabi\test\config.py:67: fatal: cxx_headers='C:/buildbot/temp/build/runtimes/runtimes-bins\include\c++\v1' is not a directory.
FAILED: libcxxabi/test/CMakeFiles/check-cxxabi
cmd.exe /C "cd /D C:\buildbot\temp\build\runtimes\runtimes-bins\libcxxabi\test && C:\Python38\python.exe C:/buildbot/temp/build/./bin/llvm-lit.py -vv -v -vv C:/buildbot/temp/build/runtimes/runtimes-bins/libcxxabi/test"
ninja: build stopped: subcommand failed.
FAILED: runtimes/CMakeFiles/check-cxxabi
cmd.exe /C "cd /D C:\buildbot\temp\build\runtimes\runtimes-bins && "C:\Program Files\CMake\bin\cmake.exe" --build C:/buildbot/temp/build/runtimes/runtimes-bins/ --target check-cxxabi --config Release"
ninja: build stopped: subcommand failed.

The <build_root>/runtimes/runtimes-bins folder does not contain include/c++/v1. There is only <build-root>/include/c++/v1.

Builders:
aarch64: https://lab.llvm.org/buildbot/#/builders/119/builds/2894
armv7: https://lab.llvm.org/buildbot/#/builders/60/builds/2484

[uabelho]

In D98367#2648239, @phosek wrote:

What's your CMake configuration? I'd like to try and reproduce this locally so I can investigate.

For me it fails with

CC='/proj/flexasic/app/llvm/8.0/bin/clang -march=corei7' CXX='/proj/flexasic/app/llvm/8.0/bin/clang++ -march=corei7' LDFLAGS='-L/proj/flexasic/app/llvm/8.0/lib64 -Wl,-R/proj/flexasic/app/llvm/8.0/lib64:/proj/bbi_twh/wh_bbi/x86_64-Linux3/z3/4.8.8-1/lib64' PATH=/proj/flexasic/app/ninja/1.8.2/SLED11-64/bin:$PATH  /app/vbuild/RHEL7-x86_64/cmake/3.16.4/bin/cmake /repo/uabelho/master-github/llvm --debug-trycompile -G Ninja -DCMAKE_MAKE_PROGRAM=/proj/flexasic/app/ninja/1.8.2/SLED11-64/bin/ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_INSTALL_PREFIX=/compiler-clang -DLLVM_APPEND_VC_REV=OFF -DLLVM_CCACHE_BUILD=OFF -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_ENABLE_WERROR=ON -DLLVM_ENABLE_PROJECTS='clang;clang-tools-extra;lld' -DLLVM_BUILTIN_TARGETS=default -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON -DLLVM_ENABLE_RUNTIMES='libcxx;libcxxabi;libunwind;compiler-rt' -DRUNTIMES_x86_64-unknown-linux-gnu_SANITIZER_USE_STATIC_LLVM_UNWINDER=ON -DRUNTIMES_x86_64-unknown-linux-gnu_SANITIZER_USE_STATIC_CXX_ABI=ON -DRUNTIMES_x86_64-unknown-linux-gnu_COMPILER_RT_USE_BUILTINS_LIBRARY=ON -DRUNTIMES_x86_64-unknown-linux-gnu_COMPILER_RT_CAN_EXECUTE_TESTS=OFF -DRUNTIMES_x86_64-unknown-linux-gnu_LIBCXX_HAS_ATOMIC_LIB=0 -DRUNTIMES_x86_64-unknown-linux-gnu_COMPILER_RT_BUILD_XRAY=OFF -DLLVM_RUNTIME_TARGETS='x86_64-unknown-linux-gnu' -DLLVM_ENABLE_Z3_SOLVER=ON -DLLVM_Z3_INSTALL_DIR=/proj/bbi_twh/wh_bbi/x86_64-Linux3/z3/4.8.8-1 -DLLVM_ENABLE_LIBPFM=OFF -DCLANG_TOOLING_BUILD_AST_INTROSPECTION=OFF

ninja check-runtimes

Since there are also buildbots failing, perhaps we can revert while investigating unless there is a very quick fix?

[phosek]

@ldionne the latest diff should address the runtimes build test failure.

[vvereschaka]

Hi @phosek,
I have tested the updated diff on Windows-x-ARM cross builder and it works fine. Thank you.

libcxxabi/CMakeLists.txt

 option(LIBCXXABI_SILENT_TERMINATE "Set this to make the terminate handler default to a silent alternative" OFF)
 option(LIBCXXABI_NON_DEMANGLING_TERMINATE "Set this to make the terminate handler
 avoid demangling" OFF)
 
 if (NOT LIBCXXABI_ENABLE_SHARED AND NOT LIBCXXABI_ENABLE_STATIC)
   message(FATAL_ERROR "libc++abi must be built as either a shared or static library.")
 endif()
 
-set(LIBCXXABI_LIBCXX_INCLUDES "${LIBCXXABI_LIBCXX_PATH}/include" CACHE PATH
-    "Specify path to libc++ includes.")
-message(STATUS "Libc++abi will be using libc++ includes from ${LIBCXXABI_LIBCXX_INCLUDES}")
+# TODO: This is a workaround for the fact that Standalone builds can't use
+# targets from the other runtimes (so the cxx-headers target doesn't exist).
+set(LIBCXXABI_LIBCXX_INCLUDES "" CACHE PATH
+    "Specify path to the installed libc++ includes. This is only relevant for Standalone builds, where the location of the headers can't be determined automatically.")
+if (LIBCXXABI_STANDALONE_BUILD)
+  if (NOT IS_DIRECTORY ${LIBCXXABI_LIBCXX_INCLUDES})
+    message(FATAL_ERROR
+      "LIBCXXABI_LIBCXX_INCLUDES=${LIBCXXABI_LIBCXX_INCLUDES} is not a valid directory. "
+      "Please provide the path to where the libc++ headers have been installed.")
+  endif()
+  add_library(cxx-headers INTERFACE)
+  if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC" OR "${CMAKE_CXX_SIMULATE_ID}" STREQUAL "MSVC")
+    target_compile_options(cxx-headers INTERFACE /I "${LIBCXXABI_LIBCXX_INCLUDES}")
+  else()
+    target_compile_options(cxx-headers INTERFACE -I "${LIBCXXABI_LIBCXX_INCLUDES}")
+  endif()
+endif()
 
 option(LIBCXXABI_HERMETIC_STATIC_LIBRARY
   "Do not export any symbols from the static library." OFF)
 
 set(LIBCXXABI_TEST_CONFIG "${CMAKE_CURRENT_SOURCE_DIR}/test/lit.site.cfg.in" CACHE STRING
     "The Lit testing configuration to use when running the tests.")
 set(LIBCXXABI_TEST_PARAMS "" CACHE STRING
     "A list of parameters to run the Lit test suite with.")

 
 # Threading
 if (NOT LIBCXXABI_ENABLE_THREADS)
   if (LIBCXXABI_HAS_PTHREAD_API)
     message(FATAL_ERROR "LIBCXXABI_HAS_PTHREAD_API can only"
                         " be set to ON when LIBCXXABI_ENABLE_THREADS"
                         " is also set to ON.")
   endif()
-  if (LIBCXXABI_HAS_WIN32_THREAD_API)
-    message(FATAL_ERROR "LIBCXXABI_HAS_WIN32_THREAD_API can only"
-                        " be set to ON when LIBCXXABI_ENABLE_THREADS"
-                        " is also set to ON.")
-  endif()
   if (LIBCXXABI_HAS_EXTERNAL_THREAD_API)
     message(FATAL_ERROR "LIBCXXABI_HAS_EXTERNAL_THREAD_API can only"
                         " be set to ON when LIBCXXABI_ENABLE_THREADS"
                         " is also set to ON.")
   endif()
   if (LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY)
     message(FATAL_ERROR "LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY can only"
                         " be set to ON when LIBCXXABI_ENABLE_THREADS"
                         " is also set to ON.")
   endif()
   add_definitions(-D_LIBCXXABI_HAS_NO_THREADS)
   add_definitions(-D_LIBCPP_HAS_NO_THREADS)
 endif()
 
 if (LIBCXXABI_HAS_EXTERNAL_THREAD_API)
   if (LIBCXXABI_HAS_PTHREAD_API)
     message(FATAL_ERROR "The options LIBCXXABI_HAS_EXTERNAL_THREAD_API"
                         " and LIBCXXABI_HAS_PTHREAD_API cannot be both"
                         " set to ON at the same time.")
   endif()
-  if (LIBCXXABI_HAS_WIN32_THREAD_API)
-    message(FATAL_ERROR "The options LIBCXXABI_HAS_EXTERNAL_THREAD_API"
-                        " and LIBCXXABI_HAS_WIN32_THREAD_API cannot be both"
-                        " set to ON at the same time.")
-  endif()
   if (LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY)
     message(FATAL_ERROR "The options LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY"
                         " and LIBCXXABI_HAS_EXTERNAL_THREAD_API cannot be both"
                         " set to ON at the same time.")
   endif()
 endif()
 
-if (LIBCXXABI_HAS_PTHREAD_API)
-  if (LIBCXXABI_HAS_WIN32_THREAD_API)
-    message(FATAL_ERROR "The options LIBCXXABI_HAS_PTHREAD_API"
-            "and LIBCXXABI_HAS_WIN32_THREAD_API cannot be both"
-            "set to ON at the same time.")
-  endif()
-endif()
-
 if (LLVM_ENABLE_MODULES)
   # Ignore that the rest of the modules flags are now unused.
   add_compile_flags_if_supported(-Wno-unused-command-line-argument)
   add_compile_flags(-fno-modules)
 endif()
 
 set(LIBCXXABI_HAS_UNDEFINED_SYMBOLS OFF)
 if ((NOT LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS)
else()
     string(REPLACE "-Wl,-z,defs" "" CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS}")
   endif()
 endif()
 
 if (LIBCXXABI_HAS_PTHREAD_API)
   add_definitions(-D_LIBCPP_HAS_THREAD_API_PTHREAD)
 endif()
 
-if (LIBCXXABI_HAS_WIN32_THREAD_API)
-  add_definitions(-D_LIBCPP_HAS_THREAD_API_WIN32)
-endif()
-
 if (LIBCXXABI_HAS_EXTERNAL_THREAD_API)
   add_definitions(-D_LIBCPP_HAS_THREAD_API_EXTERNAL)
 endif()
 
 if (LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY)
   add_definitions(-D_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL)
 endif()
 

libcxxabi/src/CMakeLists.txt

 
 if (NOT TARGET pstl::ParallelSTL)
   message(STATUS "Could not find ParallelSTL, libc++abi will not attempt to use it but the build may fail if the libc++ in use needs it to be available.")
 endif()
 
 # Build the shared library.
 if (LIBCXXABI_ENABLE_SHARED)
   add_library(cxxabi_shared SHARED ${LIBCXXABI_SOURCES} ${LIBCXXABI_HEADERS})
-  target_link_libraries(cxxabi_shared PRIVATE ${LIBCXXABI_SHARED_LIBRARIES} ${LIBCXXABI_LIBRARIES})
+  target_link_libraries(cxxabi_shared PRIVATE cxx-headers ${LIBCXXABI_SHARED_LIBRARIES} ${LIBCXXABI_LIBRARIES})
   if (TARGET pstl::ParallelSTL)
     target_link_libraries(cxxabi_shared PUBLIC pstl::ParallelSTL)
   endif()
   set_target_properties(cxxabi_shared
                         PROPERTIES
                           CXX_EXTENSIONS
                             OFF
                           CXX_STANDARD
if (LIBCXXABI_ENABLE_EXCEPTIONS)
       endif()
     endif()
   endif()
 endif()
 
 # Build the static library.
 if (LIBCXXABI_ENABLE_STATIC)
   add_library(cxxabi_static STATIC ${LIBCXXABI_SOURCES} ${LIBCXXABI_HEADERS})
-  target_link_libraries(cxxabi_static PRIVATE ${LIBCXXABI_STATIC_LIBRARIES} ${LIBCXXABI_LIBRARIES})
+  target_link_libraries(cxxabi_static PRIVATE cxx-headers ${LIBCXXABI_STATIC_LIBRARIES} ${LIBCXXABI_LIBRARIES})
   if (TARGET pstl::ParallelSTL)
     target_link_libraries(cxxabi_static PUBLIC pstl::ParallelSTL)
   endif()
   set_target_properties(cxxabi_static
                         PROPERTIES
                           CXX_EXTENSIONS
                             OFF
                           CXX_STANDARD