[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

163–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.

168

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

170–174

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

[phosek]

libcxxabi/CMakeLists.txt

163–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.

170–174

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

170–174

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

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
+# 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 libc++ includes.")
-message(STATUS "Libc++abi will be using libc++ includes from ${LIBCXXABI_LIBCXX_INCLUDES}")
+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.")
 
 #===============================================================================
 # Configure System
 #===============================================================================
 
 # Add path for custom modules
 set(CMAKE_MODULE_PATH
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake/Modules"
   ${CMAKE_MODULE_PATH}
   )
 
 if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
+  set(LIBCXXABI_HEADER_DIR ${LLVM_BINARY_DIR})
   set(LIBCXXABI_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)
   set(LIBCXXABI_INSTALL_LIBRARY_DIR lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)
   if(LIBCXX_LIBDIR_SUBDIR)
     string(APPEND LIBCXXABI_LIBRARY_DIR /${LIBCXXABI_LIBDIR_SUBDIR})
     string(APPEND LIBCXXABI_INSTALL_LIBRARY_DIR /${LIBCXXABI_LIBDIR_SUBDIR})
   endif()
 elseif(LLVM_LIBRARY_OUTPUT_INTDIR)
+  set(LIBCXXABI_HEADER_DIR ${LLVM_BINARY_DIR})
   set(LIBCXXABI_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR})
   set(LIBCXXABI_INSTALL_LIBRARY_DIR lib${LIBCXXABI_LIBDIR_SUFFIX})
 else()
+  set(LIBCXXABI_HEADER_DIR ${CMAKE_BINARY_DIR})
   set(LIBCXXABI_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBCXXABI_LIBDIR_SUFFIX})
   set(LIBCXXABI_INSTALL_LIBRARY_DIR lib${LIBCXXABI_LIBDIR_SUFFIX})
 endif()
 
 set(LIBCXXABI_INSTALL_PREFIX "" CACHE STRING "Define libc++abi destination prefix.")
 
 set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LIBCXXABI_LIBRARY_DIR})
 set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LIBCXXABI_LIBRARY_DIR})

libcxxabi/src/CMakeLists.txt

 if (MSVC_IDE OR XCODE)
   # Force them all into the headers dir on MSVC, otherwise they end up at
   # project scope because they don't have extensions.
   if (MSVC_IDE)
     source_group("Header Files" FILES ${LIBCXXABI_HEADERS})
   endif()
 endif()
 
-include_directories("${LIBCXXABI_LIBCXX_INCLUDES}")
-
 # stdlib_stdexcept.cpp depends on libc++ internals.
 include_directories("${LIBCXXABI_LIBCXX_PATH}")
 
 if (LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL)
   add_definitions(-DHAVE___CXA_THREAD_ATEXIT_IMPL)
 endif()
 
 if (LIBCXXABI_ENABLE_FORGIVING_DYNAMIC_CAST)

 
 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

libcxxabi/test/libcxxabi/test/config.py

def configure_compile_flags(self):
             self.cxx.compile_flags += ['-funwind-tables']
         if not self.get_lit_bool('enable_threads', True):
             self.cxx.compile_flags += ['-D_LIBCXXABI_HAS_NO_THREADS']
             self.config.available_features.add('libcxxabi-no-threads')
         super(Configuration, self).configure_compile_flags()
 
     def configure_compile_flags_header_includes(self):
         self.configure_config_site_header()
-        cxx_headers = self.get_lit_conf(
-            'cxx_headers',
-            os.path.join(self.libcxx_src_root, '/include'))
+        cxx_headers = self.get_lit_conf('cxx_headers', None) or \
+            os.path.join(self.libcxxabi_hdr_root, 'include', 'c++', 'v1')
         if cxx_headers == '':
             self.lit_config.note('using the systems c++ headers')
         else:
             self.cxx.compile_flags += ['-nostdinc++']
         if not os.path.isdir(cxx_headers):
             self.lit_config.fatal("cxx_headers='%s' is not a directory."
                                   % cxx_headers)
         self.cxx.compile_flags += ['-I' + cxx_headers]

libcxxabi/test/lit.site.cfg.in

 @AUTO_GEN_COMMENT@
 
 import os
 import site
 
 config.cxx_under_test           = "@CMAKE_CXX_COMPILER@"
 config.project_obj_root         = "@CMAKE_BINARY_DIR@"
+config.libcxxabi_hdr_root       = "@LIBCXXABI_HEADER_DIR@"
 config.libcxxabi_src_root       = "@LIBCXXABI_SOURCE_DIR@"
 config.libcxxabi_obj_root       = "@LIBCXXABI_BINARY_DIR@"
 config.abi_library_root         = "@LIBCXXABI_LIBRARY_DIR@"
 config.libcxx_src_root          = "@LIBCXXABI_LIBCXX_PATH@"
 config.cxx_headers              = "@LIBCXXABI_LIBCXX_INCLUDES@"
 config.libunwind_headers        = "@LIBCXXABI_LIBUNWIND_INCLUDES_INTERNAL@"
 config.cxx_library_root         = "@LIBCXXABI_LIBCXX_LIBRARY_PATH@"
 config.llvm_unwinder            = @LIBCXXABI_USE_LLVM_UNWINDER@