[compiler-rt] Reapply [compiler-rt] Check for and use -lunwind when linking with -nodefaultlibs (PR #66584)

Martin Storsjö via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 4 01:50:15 PDT 2023


mstorsjo wrote:

> Hmm, maybe. But even if CMake would be fixed to make the former case behave like the latter, what happens in the end when we link the component that we're building, and linking it against `unwind` (assuming the `check_library_exists` did succeed)? We're expecting this to inject a `-lunwind` to link against the toolchain's existing libunwind, but wouldn't it end up linking against the just-built libunwind? I guess that's not bad in itself, but not quite what was intended. If the cmake target `unwind` is a static or shared library, that would kinda work - but as it is a `UTILITY`, what would happen? (I guess that's what's happening within `check_library_exists` in some form?)
> 
> > Perhaps we should file an issue against CMake and include a link in the comment?
> 
> I guess we could - but given the above, I'm not quite sure what we'd do differently here even in the future if we can assume a baseline CMake with this issue fixed. If `TARGET unwind` exists, but is an internal utility target, not an actual shared/static library, we kinda can't link against it anyway?

To follow up on this; I'm not quite sure that this is a bug that reasonably can be resolved in CMake anyway (and taking it further; not a bug at all?). Let's look at a couple of progressive CMake snippets:

```cmake
cmake_minimum_required(VERSION 3.13.4)
project(Test LANGUAGES CXX)
include(CheckLibraryExists)
check_library_exists(unwind _Unwind_RaiseException "" COMPILER_RT_HAS_LIBUNWIND)
add_executable(main main.cpp)
if (COMPILER_RT_HAS_LIBUNWIND)
  target_link_libraries(main PRIVATE unwind)
endif()
```
This would be the primary use. Here we find an `unwind` library in the toolchain, and this ends up linking with `-lunwind`.

If we proceed further with the working case:
```cmake
cmake_minimum_required(VERSION 3.13.4)
project(Test LANGUAGES CXX)
add_library(unwind STATIC unwind.cpp)
include(CheckLibraryExists)
check_library_exists(unwind _Unwind_RaiseException "" COMPILER_RT_HAS_LIBUNWIND)

add_executable(main main.cpp)
if (COMPILER_RT_HAS_LIBUNWIND)
  target_link_libraries(main PRIVATE unwind)
endif()
```
This also works. But in this case, we don't link with `-lunwind` for the toolchain provided unwind library, but we link with `libunwind.a` to pick up the concurrently built library instead.

Now the case that fails:
```cmake
cmake_minimum_required(VERSION 3.13.4)
project(Test LANGUAGES CXX)
add_library(unwind_static STATIC unwind.cpp)
add_custom_target(unwind DEPENDS unwind_static)
include(CheckLibraryExists)
check_library_exists(unwind _Unwind_RaiseException "" COMPILER_RT_HAS_LIBUNWIND)

add_executable(main main.cpp)
if (COMPILER_RT_HAS_LIBUNWIND)
  target_link_libraries(main PRIVATE unwind)
endif()
```

This fails, as discussed earlier, like this:
```
CMake Error at /usr/share/cmake-3.22/Modules/CheckLibraryExists.cmake:72 (try_compile):
  Only libraries may be used as try_compile or try_run IMPORTED
  LINK_LIBRARIES.  Got unwind of type UTILITY.
Call Stack (most recent call first):
  CMakeLists.txt:7 (check_library_exists)
```

Now if we instead skip `check_library_exists` and considers this a bug with that function:
```cmake
cmake_minimum_required(VERSION 3.13.4)
project(Test LANGUAGES CXX)
add_library(unwind_static STATIC unwind.cpp)
add_custom_target(unwind DEPENDS unwind_static)
include(CheckLibraryExists)
# Skipping check_library_exists which fails here
#check_library_exists(unwind _Unwind_RaiseException "" COMPILER_RT_HAS_LIBUNWIND)

add_executable(main main.cpp)
if (TRUE) # COMPILER_RT_HAS_LIBUNWIND
  target_link_libraries(main PRIVATE unwind)
endif()
```

Then we finally end up with a similar error:
```
CMake Error at CMakeLists.txt:12 (target_link_libraries):
  Target "unwind" of type UTILITY may not be linked into another target.  One
  may link only to INTERFACE, OBJECT, STATIC or SHARED libraries, or to
  executables with the ENABLE_EXPORTS property set.
```

So as long as `unwind` exists as a non-library target, we can't use it as a library name as it's not really defined what happens when one links against it.

In the case of compiler-rt right now, we only append it to `CMAKE_REQUIRED_LIBRARIES` - but even then, if we add `unwind` there, when unwind is a utility target, any subsequent `check_library_exists` will fail with the same linker error.

```cmake
cmake_minimum_required(VERSION 3.13.4)
project(Test LANGUAGES CXX)
add_library(unwind_static STATIC unwind.cpp)
add_custom_target(unwind DEPENDS unwind_static)
#add_library(unwind STATIC unwind.cpp)
include(CheckLibraryExists)
#check_library_exists(unwind _Unwind_RaiseException "" COMPILER_RT_HAS_LIBUNWIND)
list(APPEND CMAKE_REQUIRED_LIBRARIES unwind)
check_library_exists(c++ __cxa_throw "" COMPILER_RT_HAS_LIBCXX)
```

Resulting in this:
```
-- Looking for __cxa_throw in c++
CMake Error at /usr/share/cmake-3.22/Modules/CheckLibraryExists.cmake:72 (try_compile):
  Only libraries may be used as try_compile or try_run IMPORTED
  LINK_LIBRARIES.  Got unwind of type UTILITY.
Call Stack (most recent call first):
  CMakeLists.txt:9 (check_library_exists)
```

Thus - there's really no well defined thing to do here. For the cases where `unwind` is a target, but an actual library target, the behaviour seems to be that at configure-time it tries linking against a toolchain provided lib with that name, while at build time it links against the recently built lib instead. I guess one could argue that extend the configure-time behaviour for non-library targets, but for actual build time linking, it doesn't really make sense anyway.

Hence, I don't think this is a proper bug, and even if it would be fixed for `check_library_exists`, we wouldn't be able to use it.

I'll go ahead and land the patch in its current form (I changed the symbol to check to one that has the same name in SjLj builds as well) soon.

https://github.com/llvm/llvm-project/pull/66584


More information about the llvm-commits mailing list