[libunwind] f8da28f - [runtimes] [cmake] Fix -Werror detection in common build configs

[Martin Storsjö via cfe-commits]

Author: Martin Storsjö
Date: 2022-05-12T22:22:15+03:00
New Revision: f8da28f5228857e905eedb248ac82c939777f9df

URL: https://github.com/llvm/llvm-project/commit/f8da28f5228857e905eedb248ac82c939777f9df
DIFF: https://github.com/llvm/llvm-project/commit/f8da28f5228857e905eedb248ac82c939777f9df.diff

LOG: [runtimes] [cmake] Fix -Werror detection in common build configs

We add `--unwindlib=none` to `CMAKE_REQUIRED_FLAGS`
to make sure that builds with a yet-incomplete toolchain succeed,
to avoid linker failures about missing unwindlib.

When this option is added to `CMAKE_REQUIRED_FLAGS`, it gets added to
both compile and link commands in CMake compile tests. If
`--unwindlib=none` is included in compilation commands, it causes
warnings about unused arguments, as the flag only is relevant for
linking.

Due to the warnings in CMake tests, the later CMake test for the
`-Werror` option failed (as the tested `-Werror` option caused the
preexisting warning due to unused `--unwindlib=none` to become a
hard error). Therefore, most CI configurations that build with
`LIBCXX_ENABLE_WERROR` didn't actually end up enabling `-Werror`
after all.

When looking at the CI build log of recent CI builds, they do
end up printing:

    -- Performing Test LIBCXX_SUPPORTS_WERROR_FLAG
    -- Performing Test LIBCXX_SUPPORTS_WERROR_FLAG - Failed
    -- Performing Test LIBCXX_SUPPORTS_WX_FLAG
    -- Performing Test LIBCXX_SUPPORTS_WX_FLAG - Failed

Thus while the configurations are meant to error out on warnings,
they actually haven't done that, due to the interaction of these
options.

To fix this, remove the individual cases of adding `--unwindlib=none`
into `CMAKE_REQUIRED_FLAGS` in libcxx and libunwind.
`runtimes/CMakeLists.txt` still adds `--unwindlib=none` if needed, but
not otherwise. (The same issue with enabling `-Werror` does remain
if `--unwindlib=none` strictly is needed though - that can be fixed
separately afterwards.)

These individual cases in libunwind and libcxx were added while
standalone builds of the runtimes still were supported - but no longer
are necessary now.

Differential Revision: https://reviews.llvm.org/D124375

Added: 
    

Modified: 
    libcxx/cmake/config-ix.cmake
    libunwind/cmake/config-ix.cmake

Removed: 
    


################################################################################
diff  --git a/libcxx/cmake/config-ix.cmake b/libcxx/cmake/config-ix.cmake
index 093eb6b31c2c3..209e6214a4718 100644
--- a/libcxx/cmake/config-ix.cmake
+++ b/libcxx/cmake/config-ix.cmake
@@ -13,9 +13,6 @@ include(CheckCSourceCompiles)
 # built libunwind isn't installed yet). For those cases, it'd be good to
 # link with --uwnindlib=none. Check if that option works.
 llvm_check_compiler_linker_flag(C "--unwindlib=none" CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG)
-if (CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG)
-  set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} --unwindlib=none")
-endif()
 
 if(WIN32 AND NOT MINGW)
   # NOTE(compnerd) this is technically a lie, there is msvcrt, but for now, lets

diff  --git a/libunwind/cmake/config-ix.cmake b/libunwind/cmake/config-ix.cmake
index 34786eff32c17..f716532fdf1b5 100644
--- a/libunwind/cmake/config-ix.cmake
+++ b/libunwind/cmake/config-ix.cmake
@@ -10,9 +10,6 @@ include(CheckCSourceCompiles)
 # might not work if libunwind doesn't exist yet. Try to check if
 # --unwindlib=none is supported, and use that if possible.
 llvm_check_compiler_linker_flag(C "--unwindlib=none" CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG)
-if (CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG)
-  set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} --unwindlib=none")
-endif()
 
 check_library_exists(c fopen "" LIBUNWIND_HAS_C_LIB)