[libcxx-commits] [PATCH] D144440: [runtimes][CMake] Drop the check to see if linker works

[Petr Hosek via Phabricator via libcxx-commits]

phosek added a comment.
Herald added a subscriber: ekilmer.

In D144440#4140827 <https://reviews.llvm.org/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 <https://reviews.llvm.org/rGb3df14b6c98702ce50401fd039852787373e4676> / D110005 <https://reviews.llvm.org/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 <https://reviews.llvm.org/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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144440/new/

https://reviews.llvm.org/D144440