[libcxx-commits] [PATCH] D142957: [CMake][runtimes] Redo the --unwindlib=none and -nostdlib++ checks

Wed Feb 1 14:41:33 PST 2023

mstorsjo added inline comments.

================
Comment at: runtimes/CMakeLists.txt:112-113
   # --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)
----------------
mstorsjo wrote:
> phosek wrote:
> > @mstorsjo do you know what's the reasoning behind this? I was wondering if we could add `--unwindlib=none` and `-nostdlib++` unconditionally to simplify this logic and make the behavior more deterministic.
> Sorry, I don't remember all the details exactly - but I think the issue was exposed in the libc++ CI builds, for the ASAN/TSAN builds there. I guess the easiest would be to put up a test patch that adds it unconditionally, and see what issues we hit in CI.
I tried running a patch through CI, removing the `if (NOT LLVM_RUNTIMES_LINKING_WORKS)` condition from around adding `--unwindlib=none`, and it no longer broke anything. So that's a bit unsettling...

I dug up the original case where I added this, https://reviews.llvm.org/D113253#3112444, and the CI build that failed before I added that check:
https://buildkite.com/llvm-project/libcxx-ci/builds/6433

There, essentially all cmake tests failed after adding the option, and this then broke the cmake configure in libunwind (otherwise it would probably have been a severely broken build anyway).

I'm not quite sure what has changed since, that makes it no longer break though. I guess it would be possible to bisect somehow...

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142957