Reapply [compiler-rt] Check for and use -lunwind when linking with -nodefaultlibs

[mstorsjo]

If libc++ is available and should be used as the ubsan C++ ABI library, the check for libc++ might fail if libc++ is a static library, as the -nodefaultlibs flag inhibits a potential compiler default -lunwind.

Just like the -nodefaultlibs configuration tests for and manually adds a bunch of compiler default libraries, look for -lunwind too.

This is a reland of #65912.

[mstorsjo]

Previously this failed with the following error:

-- Looking for _Unwind_RaiseException in unwind
-- Looking for _Unwind_RaiseException in unwind - not found
CMake Error at /usr/share/cmake-3.25/Modules/CheckLibraryExists.cmake:71 (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): 
  /b/sanitizer-aarch64-linux-fuzzer/build/llvm-project/compiler-rt/cmake/config-ix.cmake:66 (check_library_exists)
  /b/sanitizer-aarch64-linux-fuzzer/build/llvm-project/compiler-rt/CMakeLists.txt:281 (include)

I tried fixing it by wrapping it all in an if (NOT TARGET unwind) - which works, but feels rather ugly...

[mstorsjo]

Ping @petrhosek

[petrhosek]

I'm surprised that CMake is trying to use targets defined in the same build in try_compile. What would happen if this a LIBRARY target like unwind_shared or unwind_static? That target hasn't been built yet (it's built by the build we're configuring). Is this a CMake issue or is there a well defined behavior for this case?

[s-barannikov]

Can this use a different symbol? This symbol does not exist when SJLJ exceptions are enabled.

[mstorsjo]

Oh - good point. I guess we could check for _Unwind_GetRegionStart or _Unwind_GetLanguageSpecificData - both of those exist without any extra name mangling for both ehabi, sjlj, seh and regular dwarf.

[mstorsjo]

I'm surprised that CMake is trying to use targets defined in the same build in try_compile. What would happen if this a LIBRARY target like unwind_shared or unwind_static? That target hasn't been built yet (it's built by the build we're configuring). Is this a CMake issue or is there a well defined behavior for this case?

I tested replacing this with a check for unwind_shared, and that proceeded just fine (and it tested linking right away even if the target unwind_shared wasn't found).

Thus, if the unwind target wouldn't be of the type UTILITY, this might work - but would that cause other cmake confusion at some point? If the target/library entity unwind both means a preexisting library (link by passing -lunwind) and a library that gets built during the build (where it instead adds dependencies on it, builds that before using it, and links against the newly generated file instead of necessarily using literally -lunwind).

[mstorsjo]

Ping @petrhosek, what do you think about this?

[petrhosek]

I'm surprised that CMake is trying to use targets defined in the same build in try_compile. What would happen if this a LIBRARY target like unwind_shared or unwind_static? That target hasn't been built yet (it's built by the build we're configuring). Is this a CMake issue or is there a well defined behavior for this case?

I tested replacing this with a check for unwind_shared, and that proceeded just fine (and it tested linking right away even if the target unwind_shared wasn't found).

Thus, if the unwind target wouldn't be of the type UTILITY, this might work - but would that cause other cmake confusion at some point? If the target/library entity unwind both means a preexisting library (link by passing -lunwind) and a library that gets built during the build (where it instead adds dependencies on it, builds that before using it, and links against the newly generated file instead of necessarily using literally -lunwind).

Thanks, I tried it locally as well to better understand the issue.

With the following:

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)

I get the same error:

CMake Error at cmake/linux-x64/share/cmake-3.25/Modules/CheckLibraryExists.cmake:71 (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)

However, with the following:

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)

CMake fails to find libunwind:

/usr/bin/ld: cannot find -lunwind: No such file or directory

To me this really looks like a bug in CMake since the behavior is inconsistent. Specifically, I don't think CMake should consider the previously defined targets when performing checks regardless of their type.

Perhaps we should file an issue against CMake and include a link in the comment?

[mstorsjo]

With the following:

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)

I get the same error:

CMake Error at cmake/linux-x64/share/cmake-3.25/Modules/CheckLibraryExists.cmake:71 (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)

However, with the following:

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)

CMake fails to find libunwind:

/usr/bin/ld: cannot find -lunwind: No such file or directory

To me this really looks like a bug in CMake since the behavior is inconsistent. Specifically, I don't think CMake should consider the previously defined targets when performing checks regardless of their type.

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?

[petrhosek]

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?)

I was thinking about always ignoring previously defined targets, regardless of the type, since I cannot think of a case where using a previously defined target in a check call would be desirable.

[mstorsjo]

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_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_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_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_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_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.

[jpalus]

Note that one of the implications of this change is that for example on x86_64 test for __i386__ symbol existence will fail if 32-bit version of libunwind is not installed:

llvm-project/compiler-rt/cmake/base-config-ix.cmake

Line 209 in 43b2b2e

test_target_arch(i386 __i386__ "-m32")

Although 32-bit libunwind does not seem to be required.

[mstorsjo]

Hmm, so you have an x86_64 libunwind, and enough 32 bit libraries, so that compilation with -m32 works, but as we detected the x86_64 libunwind we now also require linking libunwind in all other tests, which makes the test for -m32 fail suddenly.

That's unfortunate...

@petrhosek Do you have any good ideas about how to handle this? In principle, I think the whole handling of multiple archs (like compiler-rt does, when building both x86_64 and i386 in the same cmake invocation) to be kind of wrong - ideally each arch build should be a separate cmake configuration of all the runtimes, so that all the tests are done specifically for that arch. But untangling those bits of compiler-rt is outside of what I want to take on.