[libcxxabi] [test] Depend on unwind only if available

[mgorny]

Summary

When building libcxxabi via LLVM_ENABLE_RUNTIMES=libcxxabi the CMake
invocation fails because of missing "unwind" target. However,
if the extraneous dependency is removed, the library builds just fine
against installed libunwind and tests work fine. To fix this,
add the dependency only if the target actually exists.

[ldionne]

libcxxabi/test/CMakeLists.txt

66

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?

[mgorny]

libcxxabi/test/CMakeLists.txt

66

Yes, I'm linking to system-installed libunwind.

[ldionne]

libcxxabi/test/CMakeLists.txt

66

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?

[mgorny]

libcxxabi/test/CMakeLists.txt

66

Well, the option enables some conditional code that I presume should be enabled when linking to LLVM libunwind, doesn't it?

[emaste]

libcxxabi/test/CMakeLists.txt

66

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.

[ldionne]

libcxxabi/test/CMakeLists.txt

66

I see, thanks for the explanation. So IIUC this means you're also building against a LLVM-provided unwind.h header, right? In the longer term, what would you think if libunwind provided a _LIBUNWIND_VERSION macro in its header that you can query for (like we do for libc++ and libc++abi)?

[mgorny]

libcxxabi/test/CMakeLists.txt

66

Our systems do have unwind.h from llvm-libunwind or "nongnu" libunwind but at this point I'm not really sure whether it is picked up by the compiler vs the one supplied along with the compiler.

I don't really mind some extra macro but I don't really see why that would be a question for me. I'm merely packaging it, not writing software against libunwind.

[emaste]

libcxxabi/test/CMakeLists.txt

66

It was a bit of a mess in FreeBSD; we do now install LLVM's unwind.h as of https://reviews.freebsd.org/D34065. Prior to this change we had a copy of HP libunwind unwind.h in the tree but it wasn't installed.

Also agree with @mgorny, I think a _LIBUNWIND_VERSION is reasonable but it doesn't really matter to me per se.

[ldionne]

libcxxabi/test/CMakeLists.txt

66

Sorry, I didn't explain properly. My point is that if you install the correct unwind.h header and it has a _LIBUNWIND_VERSION macro defined in it, this code in libc++abi could simply query if defined(_LIBUNWIND_VERSION) instead of having to be told that it's using the LLVM libunwind by means of setting LIBCXXABI_USE_LLVM_UNWINDER. That would solve the problem that this patch tries to solve in a cleaner way IMO.

[emaste]

libcxxabi/test/CMakeLists.txt

66

Ah, yes.

[ldionne]

libcxxabi/test/CMakeLists.txt

66

See D121015

libcxxabi/test/CMakeLists.txt

 endif()
 
 if (LIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY)
   list(APPEND LIBCXXABI_TEST_DEPS cxx_external_threads)
 endif()
 
 if (NOT LIBCXXABI_STANDALONE_BUILD)
   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)
   endif()
 endif()
 
 set(AUTO_GEN_COMMENT "## Autogenerated by libcxxabi configuration.\n# Do not edit!")
 set(SERIALIZED_LIT_PARAMS "# Lit parameters serialized here for llvm-lit to pick them up\n")
 
 macro(serialize_lit_param param value)