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

[mstorsjo]

Summary

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.

[mstorsjo]

This exposes some preexisting warnings in some build configs, that weren’t caught before as LIBCXX_ENABLE_WERROR didn’t end up having any effect. The arm builders warnings should be fixed by D124371.

The AIX 32 bit build produces these warnings:

/scratch/powerllvm/cpap8006/llvm-project/libcxx-ci/build/aix/include/c++/v1/atomic:1005:12: error: large atomic operation may incur significant performance penalty; the access size (8 bytes) exceeds the max lock-free size (4  bytes) [-Werror,-Watomic-alignment]
    return __c11_atomic_fetch_add(&__a->__a_value, __delta, static_cast<__memory_order_underlying_t>(__order));

@daltenty - what do you want to do about it? Add LIBCXX_ENABLE_WERROR=OFF in the AIX build config (which also would affect the 64 bit build), do silence it differently somehow?

[ldionne]

Thanks for noticing this! I had indeed noticed some warnings when we built libc++, but didn't realize this would mess with our detection of -Werror's availability.

LGTM once we've fixed the issues that prevent the CI from passing.

[phosek]

libcxx/cmake/config-ix.cmake

15–16

Should we drop this check as well since this is done in runtimes/CMakeLists.txt?

libunwind/cmake/config-ix.cmake

12–13

Ditto

[mstorsjo]

libcxx/cmake/config-ix.cmake

15–16

I guess we could do that too, now that using runtimes/ is mandatory. In one sense, it's a bit of a bigger step than before though - then libcxx would start relying on specific cmake variables being set, without the code for setting the variables residing in libcxx itself.

[mstorsjo]

libcxx/cmake/config-ix.cmake

15–16

Actually, we can't drop it just like that without further changes. In runtimes/CMakeLists.txt we only check for this option if it seems to be needed (if linking fails without it), while libcxx uses the result of this check if LIBCXXABI_USE_LLVM_UNWINDER is set.

[phosek]

LGTM

[mstorsjo]

Rebased to rerun CI, with hopefully only the AIX 32 bit issue left.

[daltenty]

@daltenty - what do you want to do about it? Add LIBCXX_ENABLE_WERROR=OFF in the AIX build config (which also would affect the 64 bit build), do silence it differently somehow?

Seems like we could add a diagnostic ignore pragma for -Watomic-alignment to atomic.cpp under a defined(_AIX) && defined(__64BIT__) guard to unblock this (alternatively I'm ok with setting LIBCXX_ENABLE_WERROR=OFF temporarily if that's preferred while we investigate the cause)

[ldionne]

Let's land D124519 and rebase this on top.

[mstorsjo]

Rebased, rerunning CI

libcxx/cmake/config-ix.cmake

 include(CMakePushCheckState)
 include(CheckLibraryExists)
 include(LLVMCheckCompilerLinkerFlag)
 include(CheckCCompilerFlag)
 include(CheckCXXCompilerFlag)
 include(CheckCSourceCompiles)
 
 # The compiler driver may be implicitly trying to link against libunwind.
 # This is normally ok (libcxx relies on an unwinder), but if libunwind is
 # built in the same cmake invocation as libcxx and we've got
 # LIBCXXABI_USE_LLVM_UNWINDER set, we'd be linking against the just-built
 # libunwind (and the compiler implicit -lunwind wouldn't succeed as the newly
 # 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
   # let the default linking take care of that.
   set(LIBCXX_HAS_C_LIB NO)
 else()
   check_library_exists(c fopen "" LIBCXX_HAS_C_LIB)
 endif()
 

libunwind/cmake/config-ix.cmake

 include(CMakePushCheckState)
 include(CheckCCompilerFlag)
 include(CheckCXXCompilerFlag)
 include(CheckLibraryExists)
 include(LLVMCheckCompilerLinkerFlag)
 include(CheckSymbolExists)
 include(CheckCSourceCompiles)
 
 # The compiler driver may be implicitly trying to link against libunwind, which
 # 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)
 
 if (NOT LIBUNWIND_USE_COMPILER_RT)
   if (ANDROID)
     check_library_exists(gcc __gcc_personality_v0 "" LIBUNWIND_HAS_GCC_LIB)
   else ()
     check_library_exists(gcc_s __gcc_personality_v0 "" LIBUNWIND_HAS_GCC_S_LIB)
     check_library_exists(gcc __absvdi2 "" LIBUNWIND_HAS_GCC_LIB)