[PATCH] D115674: [CMake][compiler-rt] Provide a dedicated option for LLVM unwinder

Mon Jun 13 13:59:20 PDT 2022

mstorsjo added inline comments.

================
Comment at: compiler-rt/CMakeLists.txt:514
+if(COMPILER_RT_USE_LLVM_UNWINDER)
+  # TODO: set --unwindlib=none after https://reviews.llvm.org/D115674 lands.
+  if (COMPILER_RT_ENABLE_STATIC_UNWINDER)
----------------
This todo refers to this change itself.

================
Comment at: compiler-rt/CMakeLists.txt:512
+    # default libunwind (which may be missing still).
+    append_list_if(CXX_SUPPORTS_UNWINDLIB_NONE_FLAG --unwindlib=none SANITIZER_COMMON_LINK_FLAGS)
+
----------------
phosek wrote:
> mstorsjo wrote:
> > I presume we should only do this if we actually know we're linking against a libunwind that is built in the same cmake invocation, i.e. either of the `TARGET unwind_* OR HAVE_LIBUNWIND` cases below, otherwise this does break things.
> Yes, I hope this could be more cleanly addressed by D126909.
Hmm, this feels a bit nonobvious, when this here isn't directly related to the `COMPILER_RT_USE_LLVM_UNWINDER` variable. Is it the case that this only gets called when `COMPILER_RT_USE_LLVM_UNWINDER` is true, or can you end up with other kinds of mismatches?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115674