[libcxx-commits] [PATCH] D119538: [libcxxabi] [test] Depend on unwind only if available

[Ed Maste via Phabricator via libcxx-commits]

emaste added inline comments.


================
Comment at: libcxxabi/test/CMakeLists.txt:66
   list(APPEND LIBCXXABI_TEST_DEPS cxx)
-  if (LIBCXXABI_USE_LLVM_UNWINDER)
+  if (LIBCXXABI_USE_LLVM_UNWINDER AND TARGET unwind)
     list(APPEND LIBCXXABI_TEST_DEPS unwind)
----------------
mgorny wrote:
> ldionne wrote:
> > mgorny wrote:
> > > ldionne wrote:
> > > > Do I understand correctly that you're passing `LIBCXXABI_USE_LLVM_UNWINDER` but not including `libunwind` in `LLVM_ENABLE_RUNTIMES`? Is there a reason for doing that?
> > > Yes, I'm linking to system-installed libunwind.
> > Why are you specifying `LIBCXXABI_USE_LLVM_UNWINDER` then? That's the part I don't understand:
> > 
> > ```
> > option(LIBCXXABI_USE_LLVM_UNWINDER "Build and use the LLVM unwinder.")
> > ```
> > 
> > You specifically don't want to do that, right?
> > 
> Well, the option enables some conditional code that I presume should be enabled when linking to LLVM libunwind, doesn't it?
It enables cases like
```
#if !defined(LIBCXXABI_USE_LLVM_UNWINDER)
    // Copy the address of _Unwind_Control_Block to r12 so that
    // _Unwind_GetLanguageSpecificData() and _Unwind_GetRegionStart() can
    // return correct address.
    _Unwind_SetGR(context, REG_UCB, reinterpret_cast<uint32_t>(unwind_exception));
#endif
```

In FreeBSD we have the same case @mgorny mentions, there is a system-provided built-in llvm libunwind. I haven't looked into the implication of **not** defining `LIBCXXABI_USE_LLVM_UNWINDER` but using llvm libunwind.


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

https://reviews.llvm.org/D119538