https://github.com/h-vetinari updated https://github.com/llvm/llvm-project/pull/93429
>From 8c1b899aa174b107fece1edbf99eaf261bdea516 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <mar...@martin.st> Date: Mon, 25 Apr 2022 09:45:22 +0300 Subject: [PATCH 1/7] [runtimes] [CMake] Use CMAKE_REQUIRED_LINK_OPTIONS to simplify handling of the --unwindlib=none option This avoids passing the option unnecessarily to compilation commands (where it causes warnings). This fails in practice with libunwind, where setting CMAKE_TRY_COMPILE_TARGET_TYPE to STATIC_LIBRARY breaks it, as the option from CMAKE_REQUIRED_LINK_OPTIONS ends up passed to the "ar" tool too. --- libunwind/CMakeLists.txt | 3 +++ runtimes/CMakeLists.txt | 22 +--------------------- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt index b22ade0a7d71e..3d2fadca9d2ec 100644 --- a/libunwind/CMakeLists.txt +++ b/libunwind/CMakeLists.txt @@ -221,9 +221,12 @@ add_cxx_compile_flags_if_supported(-EHsc) # This leads to libunwind not being built with this flag, which makes # libunwind quite useless in this setup. set(_previous_CMAKE_TRY_COMPILE_TARGET_TYPE ${CMAKE_TRY_COMPILE_TARGET_TYPE}) +set(_previous_CMAKE_REQUIRED_LINK_OPTIONS ${CMAKE_REQUIRED_LINK_OPTIONS}) set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY) +set(CMAKE_REQUIRED_LINK_OPTIONS) add_compile_flags_if_supported(-funwind-tables) set(CMAKE_TRY_COMPILE_TARGET_TYPE ${_previous_CMAKE_TRY_COMPILE_TARGET_TYPE}) +set(CMAKE_REQUIRED_LINK_OPTIONS ${_previous_CMAKE_REQUIRED_LINK_OPTIONS}) if (LIBUNWIND_USES_ARM_EHABI AND NOT CXX_SUPPORTS_FUNWIND_TABLES_FLAG) message(SEND_ERROR "The -funwind-tables flag must be supported " diff --git a/runtimes/CMakeLists.txt b/runtimes/CMakeLists.txt index 24f4851169591..8f909322c9a98 100644 --- a/runtimes/CMakeLists.txt +++ b/runtimes/CMakeLists.txt @@ -116,27 +116,7 @@ filter_prefixed("${CMAKE_ASM_IMPLICIT_INCLUDE_DIRECTORIES}" ${LLVM_BINARY_DIR} C # brittle. We should ideally move this to runtimes/CMakeLists.txt. llvm_check_compiler_linker_flag(C "--unwindlib=none" CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG) if (CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG) - set(ORIG_CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS}") - set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} --unwindlib=none") - # TODO: When we can require CMake 3.14, we should use - # CMAKE_REQUIRED_LINK_OPTIONS here. Until then, we need a workaround: - # When using CMAKE_REQUIRED_FLAGS, this option gets added both to - # compilation and linking commands. That causes warnings in the - # compilation commands during cmake tests. This is normally benign, but - # when testing whether -Werror works, that test fails (due to the - # preexisting warning). - # - # Therefore, before we can use CMAKE_REQUIRED_LINK_OPTIONS, check if we - # can use --start-no-unused-arguments to silence the warnings about - # --unwindlib=none during compilation. - # - # We must first add --unwindlib=none to CMAKE_REQUIRED_FLAGS above, to - # allow this subsequent test to succeed, then rewrite CMAKE_REQUIRED_FLAGS - # below. - check_c_compiler_flag("--start-no-unused-arguments" C_SUPPORTS_START_NO_UNUSED_ARGUMENTS) - if (C_SUPPORTS_START_NO_UNUSED_ARGUMENTS) - set(CMAKE_REQUIRED_FLAGS "${ORIG_CMAKE_REQUIRED_FLAGS} --start-no-unused-arguments --unwindlib=none --end-no-unused-arguments") - endif() + list(APPEND CMAKE_REQUIRED_LINK_OPTIONS "--unwindlib=none") endif() # Disable use of the installed C++ standard library when building runtimes. >From 816e9e6d81ac12537879406e0495fc80394a1a66 Mon Sep 17 00:00:00 2001 From: "H. Vetinari" <h.vetin...@gmx.com> Date: Thu, 20 Jun 2024 23:18:51 +1100 Subject: [PATCH 2/7] add comment (and CMake issue reference) about incompatible options --- libunwind/CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt index 3d2fadca9d2ec..d84f8fa6ff954 100644 --- a/libunwind/CMakeLists.txt +++ b/libunwind/CMakeLists.txt @@ -220,6 +220,10 @@ add_cxx_compile_flags_if_supported(-EHsc) # # This leads to libunwind not being built with this flag, which makes # libunwind quite useless in this setup. +# +# NOTE: we need to work around https://gitlab.kitware.com/cmake/cmake/-/issues/23454 +# because CMAKE_REQUIRED_LINK_OPTIONS (c.f. CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG) +# is incompatible with CMAKE_TRY_COMPILE_TARGET_TYPE==STATIC_LIBRARY. set(_previous_CMAKE_TRY_COMPILE_TARGET_TYPE ${CMAKE_TRY_COMPILE_TARGET_TYPE}) set(_previous_CMAKE_REQUIRED_LINK_OPTIONS ${CMAKE_REQUIRED_LINK_OPTIONS}) set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY) >From 3f917d22bdcd8b398cf7162563547418a056ecec Mon Sep 17 00:00:00 2001 From: "H. Vetinari" <h.vetin...@gmx.com> Date: Thu, 20 Jun 2024 23:18:51 +1100 Subject: [PATCH 3/7] [cmake] move check for `-fno-exceptions` to "safe zone" w.r.t. interference between CMAKE_REQUIRED_LINK_OPTIONS and static libraries --- libunwind/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt index d84f8fa6ff954..d8aad8d73befa 100644 --- a/libunwind/CMakeLists.txt +++ b/libunwind/CMakeLists.txt @@ -229,6 +229,7 @@ set(_previous_CMAKE_REQUIRED_LINK_OPTIONS ${CMAKE_REQUIRED_LINK_OPTIONS}) set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY) set(CMAKE_REQUIRED_LINK_OPTIONS) add_compile_flags_if_supported(-funwind-tables) +add_cxx_compile_flags_if_supported(-fno-exceptions) set(CMAKE_TRY_COMPILE_TARGET_TYPE ${_previous_CMAKE_TRY_COMPILE_TARGET_TYPE}) set(CMAKE_REQUIRED_LINK_OPTIONS ${_previous_CMAKE_REQUIRED_LINK_OPTIONS}) @@ -237,7 +238,6 @@ if (LIBUNWIND_USES_ARM_EHABI AND NOT CXX_SUPPORTS_FUNWIND_TABLES_FLAG) "because this target uses ARM Exception Handling ABI") endif() -add_cxx_compile_flags_if_supported(-fno-exceptions) add_cxx_compile_flags_if_supported(-fno-rtti) # Ensure that we don't depend on C++ standard library. >From a8dd830691e433f79738c4ce8258b122f2cb2f77 Mon Sep 17 00:00:00 2001 From: "H. Vetinari" <h.vetin...@gmx.com> Date: Wed, 26 Jun 2024 15:40:51 +1100 Subject: [PATCH 4/7] [cmake] same for `-fno-rtti` --- libunwind/CMakeLists.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt index d8aad8d73befa..6dd2f4a813fd9 100644 --- a/libunwind/CMakeLists.txt +++ b/libunwind/CMakeLists.txt @@ -230,6 +230,7 @@ set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY) set(CMAKE_REQUIRED_LINK_OPTIONS) add_compile_flags_if_supported(-funwind-tables) add_cxx_compile_flags_if_supported(-fno-exceptions) +add_cxx_compile_flags_if_supported(-fno-rtti) set(CMAKE_TRY_COMPILE_TARGET_TYPE ${_previous_CMAKE_TRY_COMPILE_TARGET_TYPE}) set(CMAKE_REQUIRED_LINK_OPTIONS ${_previous_CMAKE_REQUIRED_LINK_OPTIONS}) @@ -238,8 +239,6 @@ if (LIBUNWIND_USES_ARM_EHABI AND NOT CXX_SUPPORTS_FUNWIND_TABLES_FLAG) "because this target uses ARM Exception Handling ABI") endif() -add_cxx_compile_flags_if_supported(-fno-rtti) - # Ensure that we don't depend on C++ standard library. if (CXX_SUPPORTS_NOSTDINCXX_FLAG) list(APPEND LIBUNWIND_COMPILE_FLAGS -nostdinc++) >From 7444457b1e3df1d923891d301b507a3909e21b09 Mon Sep 17 00:00:00 2001 From: "H. Vetinari" <h.vetin...@gmx.com> Date: Wed, 26 Jun 2024 17:47:05 +1100 Subject: [PATCH 5/7] [cmake] run _all_ flag checks in runtimes/CMakeLists.txt without linker --- libunwind/CMakeLists.txt | 49 ++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt index 6dd2f4a813fd9..ebef3fdcfe86a 100644 --- a/libunwind/CMakeLists.txt +++ b/libunwind/CMakeLists.txt @@ -175,6 +175,23 @@ if (LIBUNWIND_USE_COMPILER_RT AND NOT LIBUNWIND_HAS_NODEFAULTLIBS_FLAG) list(APPEND LIBUNWIND_LINK_FLAGS "-rtlib=compiler-rt") endif() +# Disable linker for running CMake checks +# +# This was originally added because when building libunwind for ARM Linux, +# we need to pass the -funwind-tables flag in order for it to work properly +# with ARM EHABI. However, this produces a false negative when performing CMake +# checks, causing libunwind to not be built with this flag. +# +# A similar dynamic occurred with the advent of CMAKE_REQUIRED_LINK_OPTIONS +# (c.f. CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG), which causes flag checks for static +# targets to fail due to https://gitlab.kitware.com/cmake/cmake/-/issues/23454. +# Thus, to avoid failures with static targets, we cache the target type here, +# and reset it after the various flag support checks have been performed. +set(_previous_CMAKE_TRY_COMPILE_TARGET_TYPE ${CMAKE_TRY_COMPILE_TARGET_TYPE}) +set(_previous_CMAKE_REQUIRED_LINK_OPTIONS ${CMAKE_REQUIRED_LINK_OPTIONS}) +set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY) +set(CMAKE_REQUIRED_LINK_OPTIONS) + add_compile_flags_if_supported(-Werror=return-type) if (LIBUNWIND_ENABLE_CET) @@ -207,38 +224,16 @@ endif() add_cxx_compile_flags_if_supported(-fstrict-aliasing) add_cxx_compile_flags_if_supported(-EHsc) -# Don't run the linker in this CMake check. -# -# The reason why this was added is that when building libunwind for -# ARM Linux, we need to pass the -funwind-tables flag in order for it to -# work properly with ARM EHABI. -# -# However, when performing CMake checks, adding this flag causes the check -# to produce a false negative, because the compiler generates calls -# to __aeabi_unwind_cpp_pr0, which is defined in libunwind itself, -# which isn't built yet, so the linker complains about undefined symbols. -# -# This leads to libunwind not being built with this flag, which makes -# libunwind quite useless in this setup. -# -# NOTE: we need to work around https://gitlab.kitware.com/cmake/cmake/-/issues/23454 -# because CMAKE_REQUIRED_LINK_OPTIONS (c.f. CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG) -# is incompatible with CMAKE_TRY_COMPILE_TARGET_TYPE==STATIC_LIBRARY. -set(_previous_CMAKE_TRY_COMPILE_TARGET_TYPE ${CMAKE_TRY_COMPILE_TARGET_TYPE}) -set(_previous_CMAKE_REQUIRED_LINK_OPTIONS ${CMAKE_REQUIRED_LINK_OPTIONS}) -set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY) -set(CMAKE_REQUIRED_LINK_OPTIONS) add_compile_flags_if_supported(-funwind-tables) -add_cxx_compile_flags_if_supported(-fno-exceptions) -add_cxx_compile_flags_if_supported(-fno-rtti) -set(CMAKE_TRY_COMPILE_TARGET_TYPE ${_previous_CMAKE_TRY_COMPILE_TARGET_TYPE}) -set(CMAKE_REQUIRED_LINK_OPTIONS ${_previous_CMAKE_REQUIRED_LINK_OPTIONS}) if (LIBUNWIND_USES_ARM_EHABI AND NOT CXX_SUPPORTS_FUNWIND_TABLES_FLAG) message(SEND_ERROR "The -funwind-tables flag must be supported " "because this target uses ARM Exception Handling ABI") endif() +add_cxx_compile_flags_if_supported(-fno-exceptions) +add_cxx_compile_flags_if_supported(-fno-rtti) + # Ensure that we don't depend on C++ standard library. if (CXX_SUPPORTS_NOSTDINCXX_FLAG) list(APPEND LIBUNWIND_COMPILE_FLAGS -nostdinc++) @@ -292,6 +287,10 @@ if (LIBUNWIND_ENABLE_ARM_WMMX) add_compile_flags(-D__ARM_WMMX) endif() +# reset CMAKE_TRY_COMPILE_TARGET_TYPE & CMAKE_REQUIRED_LINK_OPTIONS after flag checks +set(CMAKE_TRY_COMPILE_TARGET_TYPE ${_previous_CMAKE_TRY_COMPILE_TARGET_TYPE}) +set(CMAKE_REQUIRED_LINK_OPTIONS ${_previous_CMAKE_REQUIRED_LINK_OPTIONS}) + if(LIBUNWIND_IS_BAREMETAL) add_compile_definitions(_LIBUNWIND_IS_BAREMETAL) endif() >From 216799acde01688067ea3f0782b73ce4bd6551a2 Mon Sep 17 00:00:00 2001 From: "H. Vetinari" <h.vetin...@gmx.com> Date: Wed, 26 Jun 2024 21:18:36 +1100 Subject: [PATCH 6/7] [cmake] also do static-vs-linker work-around for cxx_add_warning_flags --- libunwind/CMakeLists.txt | 8 ++++---- runtimes/cmake/Modules/WarningFlags.cmake | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt index ebef3fdcfe86a..c787a560470ad 100644 --- a/libunwind/CMakeLists.txt +++ b/libunwind/CMakeLists.txt @@ -234,6 +234,10 @@ endif() add_cxx_compile_flags_if_supported(-fno-exceptions) add_cxx_compile_flags_if_supported(-fno-rtti) +# reset CMAKE_TRY_COMPILE_TARGET_TYPE & CMAKE_REQUIRED_LINK_OPTIONS after flag checks +set(CMAKE_TRY_COMPILE_TARGET_TYPE ${_previous_CMAKE_TRY_COMPILE_TARGET_TYPE}) +set(CMAKE_REQUIRED_LINK_OPTIONS ${_previous_CMAKE_REQUIRED_LINK_OPTIONS}) + # Ensure that we don't depend on C++ standard library. if (CXX_SUPPORTS_NOSTDINCXX_FLAG) list(APPEND LIBUNWIND_COMPILE_FLAGS -nostdinc++) @@ -287,10 +291,6 @@ if (LIBUNWIND_ENABLE_ARM_WMMX) add_compile_flags(-D__ARM_WMMX) endif() -# reset CMAKE_TRY_COMPILE_TARGET_TYPE & CMAKE_REQUIRED_LINK_OPTIONS after flag checks -set(CMAKE_TRY_COMPILE_TARGET_TYPE ${_previous_CMAKE_TRY_COMPILE_TARGET_TYPE}) -set(CMAKE_REQUIRED_LINK_OPTIONS ${_previous_CMAKE_REQUIRED_LINK_OPTIONS}) - if(LIBUNWIND_IS_BAREMETAL) add_compile_definitions(_LIBUNWIND_IS_BAREMETAL) endif() diff --git a/runtimes/cmake/Modules/WarningFlags.cmake b/runtimes/cmake/Modules/WarningFlags.cmake index d06409841dc9d..2330ad2c0cc6f 100644 --- a/runtimes/cmake/Modules/WarningFlags.cmake +++ b/runtimes/cmake/Modules/WarningFlags.cmake @@ -3,6 +3,18 @@ include(HandleFlags) # Warning flags =============================================================== function(cxx_add_warning_flags target enable_werror enable_pedantic) target_compile_definitions(${target} PUBLIC -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) + + # Disable linker for CMake flag compatibility checks + # + # Due to https://gitlab.kitware.com/cmake/cmake/-/issues/23454, we need to + # disable CMAKE_REQUIRED_LINK_OPTIONS (c.f. CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG), + # for static targets; cache the target type here, and reset it after the various + # checks have been performed. + set(_previous_CMAKE_TRY_COMPILE_TARGET_TYPE ${CMAKE_TRY_COMPILE_TARGET_TYPE}) + set(_previous_CMAKE_REQUIRED_LINK_OPTIONS ${CMAKE_REQUIRED_LINK_OPTIONS}) + set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY) + set(CMAKE_REQUIRED_LINK_OPTIONS) + if (MSVC) # -W4 is the cl.exe/clang-cl equivalent of -Wall. (In cl.exe and clang-cl, # -Wall is equivalent to -Weverything in GCC style compiler drivers.) @@ -74,4 +86,8 @@ function(cxx_add_warning_flags target enable_werror enable_pedantic) if (${enable_pedantic}) target_add_compile_flags_if_supported(${target} PRIVATE -pedantic) endif() + + # reset CMAKE_TRY_COMPILE_TARGET_TYPE & CMAKE_REQUIRED_LINK_OPTIONS after flag checks + set(CMAKE_TRY_COMPILE_TARGET_TYPE ${_previous_CMAKE_TRY_COMPILE_TARGET_TYPE}) + set(CMAKE_REQUIRED_LINK_OPTIONS ${_previous_CMAKE_REQUIRED_LINK_OPTIONS}) endfunction() >From 47e7688b74a723eb4105a9bfb41c024665adda5d Mon Sep 17 00:00:00 2001 From: "H. Vetinari" <h.vetin...@gmx.com> Date: Thu, 27 Jun 2024 09:30:05 +1100 Subject: [PATCH 7/7] [cmake] also do static-vs-linker work-around for target_add_compile_flags_if_supported --- runtimes/cmake/Modules/HandleFlags.cmake | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/runtimes/cmake/Modules/HandleFlags.cmake b/runtimes/cmake/Modules/HandleFlags.cmake index 4a62b67169e4d..6bb3d92043b46 100644 --- a/runtimes/cmake/Modules/HandleFlags.cmake +++ b/runtimes/cmake/Modules/HandleFlags.cmake @@ -103,6 +103,18 @@ endmacro() # For each specified flag, add that compile flag to the provided target. # The flags are added with the given visibility, i.e. PUBLIC|PRIVATE|INTERFACE. function(target_add_compile_flags_if_supported target visibility) + + # Disable linker for CMake flag compatibility checks + # + # Due to https://gitlab.kitware.com/cmake/cmake/-/issues/23454, we need to + # disable CMAKE_REQUIRED_LINK_OPTIONS (c.f. CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG), + # for static targets; cache the target type here, and reset it after the various + # checks have been performed. + set(_previous_CMAKE_TRY_COMPILE_TARGET_TYPE ${CMAKE_TRY_COMPILE_TARGET_TYPE}) + set(_previous_CMAKE_REQUIRED_LINK_OPTIONS ${CMAKE_REQUIRED_LINK_OPTIONS}) + set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY) + set(CMAKE_REQUIRED_LINK_OPTIONS) + foreach(flag ${ARGN}) mangle_name("${flag}" flagname) check_cxx_compiler_flag("${flag}" "CXX_SUPPORTS_${flagname}_FLAG") @@ -110,4 +122,8 @@ function(target_add_compile_flags_if_supported target visibility) target_compile_options(${target} ${visibility} ${flag}) endif() endforeach() + + # reset CMAKE_TRY_COMPILE_TARGET_TYPE & CMAKE_REQUIRED_LINK_OPTIONS after flag checks + set(CMAKE_TRY_COMPILE_TARGET_TYPE ${_previous_CMAKE_TRY_COMPILE_TARGET_TYPE}) + set(CMAKE_REQUIRED_LINK_OPTIONS ${_previous_CMAKE_REQUIRED_LINK_OPTIONS}) endfunction() _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits