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

Mon Jun 13 14:44:10 PDT 2022

phosek added inline comments.

================
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)
+
----------------
mstorsjo wrote:
> 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?
I removed it from this change.  Prior to this change, C++ ABI and unwinder in compiler-rt is controlled by a single option, but that's causing issues. This change introduces a separate option for controlling the unwinder. I'm also working on another change that will introduce a new option for controlling C++ library. When those land, I think we should deprecate and remove the original flag.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115674