From: Louis Dionne Date: Fri, 15 May 2020 18:54:27 +0000 (-0400) Subject: [libc++] Always generate a __config_site header X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=53623d4aa7105dd431b388c6fddea3a71b52ab14;p=platform%2Fupstream%2Fllvm.git [libc++] Always generate a __config_site header Before this patch, the __config_site header was only generated when at least one __config_site macro needed to be defined. This lead to two different code paths in how libc++ is configured, depending on whether a __config_site header was generated or not. After this patch, the __config_site is always generated, but it can be empty in case there are no macros to define in it. More context on why this change is important -------------------------------------------- In addition to being confusing, this double-code-path situation lead to broken code being checked in undetected in 2405bd689815, which introduced the LIBCXX_HAS_MERGED_TYPEINFO_NAMES_DEFAULT CMake setting. Specifically, the _LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT <__config_site> macro was supposed NOT to be defined unless LIBCXX_HAS_MERGED_TYPEINFO_NAMES_DEFAULT was specified explicitly on the CMake command line. Instead, what happened is that it was defined to 0 if it wasn't specified explicitly and a <__config_site> header was generated. And defining that macro to 0 had the important effect of using the non-unique RTTI comparison implementation, which changes the ABI. This change in behavior wasn't noticed because the <__config_site> header is not generated by default. However, the Apple configuration does cause a <__config_site> header to be generated, which lead to the wrong RTTI implementation being used, and to https://llvm.org/PR45549. We came close to an ABI break in the dylib, but were saved due to a downstream-only change that overrode the decision of the <__config_site> for the purpose of RTTI comparisons in libc++abi. This is an incredible luck that we should not rely on ever again. While the problem itself was fixed with 2464d8135e2a by setting LIBCXX_HAS_MERGED_TYPEINFO_NAMES_DEFAULT explicitly in the Apple CMake cache and then in d0fcdcd28f95 by making the setting less brittle, the point still is that we should have had a single code path from the beginning. Unlike most normal libraries, the macros that configure libc++ are really complex, there's a lot of them and they control important properties of the C++ runtime. There must be a single code path for that, and it must be simple and robust. Differential Revision: https://reviews.llvm.org/D80927 --- diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt index 7508840..f884456 100644 --- a/libcxx/CMakeLists.txt +++ b/libcxx/CMakeLists.txt @@ -874,22 +874,15 @@ if (DEFINED WIN32 AND LIBCXX_ENABLE_STATIC AND NOT LIBCXX_ENABLE_SHARED) endif() set(site_config_path "${LIBCXX_BINARY_DIR}/__config_site") -if (LIBCXX_NEEDS_SITE_CONFIG) - configure_file("include/__config_site.in" - "${site_config_path}" - @ONLY) -elseif(EXISTS "${site_config_path}") - message(STATUS "Removing stale site configuration ${site_config_path}") - file(REMOVE "${site_config_path}") -endif() +configure_file("include/__config_site.in" + "${site_config_path}" + @ONLY) function(cxx_add_config_site target) - if (LIBCXX_NEEDS_SITE_CONFIG) - if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC" OR "${CMAKE_CXX_SIMULATE_ID}" STREQUAL "MSVC") - target_compile_options(${target} PUBLIC /FI "${site_config_path}") - else() - target_compile_options(${target} PUBLIC -include "${site_config_path}") - endif() + if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC" OR "${CMAKE_CXX_SIMULATE_ID}" STREQUAL "MSVC") + target_compile_options(${target} PUBLIC /FI "${site_config_path}") + else() + target_compile_options(${target} PUBLIC -include "${site_config_path}") endif() endfunction() diff --git a/libcxx/benchmarks/CMakeLists.txt b/libcxx/benchmarks/CMakeLists.txt index bd38de9..f012ccc 100644 --- a/libcxx/benchmarks/CMakeLists.txt +++ b/libcxx/benchmarks/CMakeLists.txt @@ -18,9 +18,7 @@ if (DEFINED LIBCXX_CXX_ABI_LIBRARY_PATH) -L${LIBCXX_CXX_ABI_LIBRARY_PATH} -Wl,-rpath,${LIBCXX_CXX_ABI_LIBRARY_PATH}) endif() -if (LIBCXX_NEEDS_SITE_CONFIG) - list(APPEND BENCHMARK_LIBCXX_COMPILE_FLAGS -include "${LIBCXX_BINARY_DIR}/__config_site") -endif() +list(APPEND BENCHMARK_LIBCXX_COMPILE_FLAGS -include "${LIBCXX_BINARY_DIR}/__config_site") split_list(BENCHMARK_LIBCXX_COMPILE_FLAGS) ExternalProject_Add(google-benchmark-libcxx diff --git a/libcxx/cmake/Modules/HandleLibcxxFlags.cmake b/libcxx/cmake/Modules/HandleLibcxxFlags.cmake index b0fb98b..a5b4df6 100644 --- a/libcxx/cmake/Modules/HandleLibcxxFlags.cmake +++ b/libcxx/cmake/Modules/HandleLibcxxFlags.cmake @@ -88,20 +88,17 @@ endmacro() macro(config_define_if condition def) if (${condition}) set(${def} ON) - set(LIBCXX_NEEDS_SITE_CONFIG ON) endif() endmacro() macro(config_define_if_not condition def) if (NOT ${condition}) set(${def} ON) - set(LIBCXX_NEEDS_SITE_CONFIG ON) endif() endmacro() macro(config_define value def) set(${def} ${value}) - set(LIBCXX_NEEDS_SITE_CONFIG ON) endmacro() # Add a list of flags to all of 'CMAKE_CXX_FLAGS', 'CMAKE_C_FLAGS', diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt index 3625dd9..c913ef9 100644 --- a/libcxx/include/CMakeLists.txt +++ b/libcxx/include/CMakeLists.txt @@ -184,27 +184,20 @@ if(LIBCXX_INSTALL_SUPPORT_HEADERS) ) endif() -if (LIBCXX_NEEDS_SITE_CONFIG) - # Generate a custom __config header. The new header is created - # by prepending __config_site to the current __config header. - add_custom_command(OUTPUT ${LIBCXX_BINARY_DIR}/__generated_config - COMMAND ${Python3_EXECUTABLE} ${LIBCXX_SOURCE_DIR}/utils/cat_files.py - ${LIBCXX_BINARY_DIR}/__config_site - ${LIBCXX_SOURCE_DIR}/include/__config - -o ${LIBCXX_BINARY_DIR}/__generated_config - DEPENDS ${LIBCXX_SOURCE_DIR}/include/__config - ${LIBCXX_BINARY_DIR}/__config_site - ) - # Add a target that executes the generation commands. - add_custom_target(cxx-generated-config ALL - DEPENDS ${LIBCXX_BINARY_DIR}/__generated_config) - set(generated_config_deps cxx-generated-config) -else() - set(files - ${files} - __config - ) -endif() +# Generate a custom __config header. The new header is created +# by prepending __config_site to the current __config header. +add_custom_command(OUTPUT ${LIBCXX_BINARY_DIR}/__generated_config + COMMAND ${Python3_EXECUTABLE} ${LIBCXX_SOURCE_DIR}/utils/cat_files.py + ${LIBCXX_BINARY_DIR}/__config_site + ${LIBCXX_SOURCE_DIR}/include/__config + -o ${LIBCXX_BINARY_DIR}/__generated_config + DEPENDS ${LIBCXX_SOURCE_DIR}/include/__config + ${LIBCXX_BINARY_DIR}/__config_site +) +# Add a target that executes the generation commands. +add_custom_target(cxx-generated-config ALL + DEPENDS ${LIBCXX_BINARY_DIR}/__generated_config) +set(generated_config_deps cxx-generated-config) # In some build configurations (like bootstrapping clang), we need to be able to # install the libcxx headers before the CMake configuration for libcxx runs. Making @@ -228,16 +221,14 @@ if(NOT LIBCXX_USING_INSTALLED_LLVM AND LIBCXX_HEADER_DIR) list(APPEND out_files ${dst}) endforeach() - if (LIBCXX_NEEDS_SITE_CONFIG) - # Copy the generated header as __config into build directory. - set(src ${LIBCXX_BINARY_DIR}/__generated_config) - set(dst ${output_dir}/__config) - add_custom_command(OUTPUT ${dst} - DEPENDS ${src} ${generated_config_deps} - COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst} - COMMENT "Copying CXX __config") - list(APPEND out_files ${dst}) - endif() + # Copy the generated header as __config into build directory. + set(src ${LIBCXX_BINARY_DIR}/__generated_config) + set(dst ${output_dir}/__config) + add_custom_command(OUTPUT ${dst} + DEPENDS ${src} ${generated_config_deps} + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst} + COMMENT "Copying CXX __config") + list(APPEND out_files ${dst}) add_custom_target(${CXX_HEADER_TARGET} ALL DEPENDS ${out_files} ${LIBCXX_CXX_ABI_HEADER_TARGET}) else() @@ -255,14 +246,12 @@ if (LIBCXX_INSTALL_HEADERS) ) endforeach() - if (LIBCXX_NEEDS_SITE_CONFIG) - # Install the generated header as __config. - install(FILES ${LIBCXX_BINARY_DIR}/__generated_config - DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1 - PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ - RENAME __config - COMPONENT ${CXX_HEADER_TARGET}) - endif() + # Install the generated header as __config. + install(FILES ${LIBCXX_BINARY_DIR}/__generated_config + DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1 + PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ + RENAME __config + COMPONENT ${CXX_HEADER_TARGET}) if (NOT CMAKE_CONFIGURATION_TYPES) add_custom_target(install-${CXX_HEADER_TARGET}