[libunwind] [llvm] [runtimes] remove workaround for old CMake when setting `--unwindlib=none` (PR #93429)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 26 15:30:35 PDT 2024
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?= <martin at 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.vetinari at 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.vetinari at 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.vetinari at 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.vetinari at 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.vetinari at 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.vetinari at 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()
More information about the cfe-commits
mailing list