[libunwind] [llvm] [runtimes] remove workaround for old CMake when setting `--unwindlib=none` (PR #93429)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 26 00:00:11 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/5] [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/5] 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/5] [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/5] [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/5] [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()
More information about the llvm-commits
mailing list