[runtimes][CMake] Drop the check to see if linker works

[phosek]

Summary

This isn't needed anymore.

[mstorsjo]

LGTM - I've also noticed that this no longer is necessary. And unifying the behaviour (so that the --unwindlib=none flag always gets passed, instead of only passing it in the rare cases of fully from-scratch builds with an incomplete toolchain) is certainly good.

However I'm quite curious about how this now works, when it didn't before. I'll see if I can manage to bisect down the change in behaviour...

[mstorsjo]

Ok, now I've bisected the issue and I think I understand what's happening (or what used to happen). The issue was resolved by b3df14b6c98702ce50401fd039852787373e4676 / D110005.

For the Generic-asan libcxx CI configuration, we pass LLVM_USE_SANITIZER="Address" (via a cmake cache file). Within the call to include(HandleLLVMOptions), this adds -fsanitize=address to CMAKE_C_FLAGS and CMAKE_CXX_FLAGS.

The combination -fsanitize-address --unwindlib=none makes all further linking checks fail. However - in cmake/config-ix.cmake in all of libcxx/libcxxabi/libunwind, we try to add an option to counter this. This, somewhat condensed, does this:

llvm_check_compiler_linker_flag(C "-nostdlib++" LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG)
if (LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG)
  set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nostdlib++")
else()
  llvm_check_compiler_linker_flag(C "-nodefaultlibs" LIBUNWIND_SUPPORTS_NODEFAULTLIBS_FLAG)
  if (LIBUNWIND_SUPPORTS_NODEFAULTLIBS_FLAG)
    set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nodefaultlibs")
  endif()
endif()

if (LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG OR LIBUNWIND_SUPPORTS_NODEFAULTLIBS_FLAG)
  if (CMAKE_C_FLAGS MATCHES -fsanitize OR CMAKE_CXX_FLAGS MATCHES -fsanitize)
    set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fno-sanitize=all")

However since all linking tests fail, the check for LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG fails, and we end up never adding -fno-sanitize=all. This got fixed by D110005, by reusing the same variable CXX_SUPPORTS_NOSTDLIBXX_FLAG which we already initialized within runtimes/CMakeLists.txt before -fsanitize was added.

Hence, this works, but is still a bit brittle, since it relies on project specific code within */cmake/config-ix.cmake. As future improvement, I guess it could be useful to move/generalize such code, at least code for adding -fno-sanitize=all to CMAKE_REQUIRED_FLAGS, into runtimes/CMakeLists.txt, to make sure that it gets set up before we try to configure the individual projects.

[phosek]

In D144440#4140827, @mstorsjo wrote:

Ok, now I've bisected the issue and I think I understand what's happening (or what used to happen). The issue was resolved by b3df14b6c98702ce50401fd039852787373e4676 / D110005.

For the Generic-asan libcxx CI configuration, we pass LLVM_USE_SANITIZER="Address" (via a cmake cache file). Within the call to include(HandleLLVMOptions), this adds -fsanitize=address to CMAKE_C_FLAGS and CMAKE_CXX_FLAGS.

The combination -fsanitize-address --unwindlib=none makes all further linking checks fail. However - in cmake/config-ix.cmake in all of libcxx/libcxxabi/libunwind, we try to add an option to counter this. This, somewhat condensed, does this:

llvm_check_compiler_linker_flag(C "-nostdlib++" LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG)
if (LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG)
  set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nostdlib++")
else()
  llvm_check_compiler_linker_flag(C "-nodefaultlibs" LIBUNWIND_SUPPORTS_NODEFAULTLIBS_FLAG)
  if (LIBUNWIND_SUPPORTS_NODEFAULTLIBS_FLAG)
    set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nodefaultlibs")
  endif()
endif()

if (LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG OR LIBUNWIND_SUPPORTS_NODEFAULTLIBS_FLAG)
  if (CMAKE_C_FLAGS MATCHES -fsanitize OR CMAKE_CXX_FLAGS MATCHES -fsanitize)
    set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fno-sanitize=all")

However since all linking tests fail, the check for LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG fails, and we end up never adding -fno-sanitize=all. This got fixed by D110005, by reusing the same variable CXX_SUPPORTS_NOSTDLIBXX_FLAG which we already initialized within runtimes/CMakeLists.txt before -fsanitize was added.

Hence, this works, but is still a bit brittle, since it relies on project specific code within */cmake/config-ix.cmake. As future improvement, I guess it could be useful to move/generalize such code, at least code for adding -fno-sanitize=all to CMAKE_REQUIRED_FLAGS, into runtimes/CMakeLists.txt, to make sure that it gets set up before we try to configure the individual projects.

Thanks for looking into this. I left a TODO comment so this is captured and will look into address this more properly.

runtimes/CMakeLists.txt

function(filter_prefixed list prefix outvar)
   endforeach()
   set(${outvar} ${result} PARENT_SCOPE)
 endfunction()
 
 filter_prefixed("${CMAKE_C_IMPLICIT_INCLUDE_DIRECTORIES}" ${LLVM_BINARY_DIR} CMAKE_C_IMPLICIT_INCLUDE_DIRECTORIES)
 filter_prefixed("${CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES}" ${LLVM_BINARY_DIR} CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES)
 filter_prefixed("${CMAKE_ASM_IMPLICIT_INCLUDE_DIRECTORIES}" ${LLVM_BINARY_DIR} CMAKE_ASM_IMPLICIT_INCLUDE_DIRECTORIES)
 
-check_c_compiler_flag("" LLVM_RUNTIMES_LINKING_WORKS)
-if (NOT LLVM_RUNTIMES_LINKING_WORKS)
-  # 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.
-  # Don't add this if not necessary to fix linking, as it can break using
-  # e.g. ASAN/TSAN.
-  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()
-  endif()
-endif()
+#
+# TODO: Note that this is problematic when LLVM_USE_SANITIZER is used
+# because some sanitizers require the unwinder and so the combination of
+# -fsanitize=... --unwindlib=none will always result in a linking error.
+# Currently, we counteract this issue by adding -fno-sanitize=all flag in
+# the project specific code within */cmake/config-ix.cmake files but that's
+# brittle. We should ideally move this to runtimes/CMakeLists.txt.
 
 # Disable use of the installed C++ standard library when building runtimes.
 # Check for -nostdlib++ first; if there's no C++ standard library yet,
 # all check_cxx_compiler_flag commands will fail until we add -nostdlib++
 # (or -nodefaultlibs).
 # TODO: Switch to check_linker_flag after raise the minimum CMake version past 3.14.
 check_cxx_compiler_flag(-nostdlib++ CXX_SUPPORTS_NOSTDLIBXX_FLAG)
 if (CXX_SUPPORTS_NOSTDLIBXX_FLAG)