This is an archive of the discontinued LLVM Phabricator instance.

[libunwind] Link with -unwindlib=none
AbandonedPublic

Authored by hvdijk on Oct 19 2021, 4:56 PM.

Details

Reviewers
ldionne
mstorsjo
phosek
Group Reviewers
Restricted Project
Summary

When D95875 changed libunwind to link using -nostdlib++ rather than -nodefaultlibs, that missed that clang may now implicitly try to link libunwind against itself. Add -unwindlib=none to avoid that.

Diff Detail

Event Timeline

hvdijk created this revision.Oct 19 2021, 4:56 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 19 2021, 4:56 PM
hvdijk requested review of this revision.Oct 19 2021, 4:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 19 2021, 4:56 PM

Actually, I remembered I had an unupstreamed patch regarding this locally... It turns out that testing check_c_compiler_flag(-unwindlib=none) doesn't work, as the tested flag is added the the compile command (but not to the link command), so the link part of the test command still fails. So to test this, one has to try blindly adding it to CMAKE_REQUIRED_FLAGS, then do a dummy check_c_compiler_flag call to see if it worked, and then revert it if it wasn't supported.

I can post my version of my patch that I found locally, which tries to fix that.

hvdijk added a comment.EditedOct 20 2021, 1:54 AM

Actually, I remembered I had an unupstreamed patch regarding this locally... It turns out that testing check_c_compiler_flag(-unwindlib=none) doesn't work, as the tested flag is added the the compile command (but not to the link command), so the link part of the test command still fails.

The libunwind/src/CMakeLists.txt change sets up the link flag; I've tested 13.0.0 without and with this patch and on the exact system where without this patch it fails to link, with this patch it succeeds. (Edit: I think I may be slightly misunderstanding you though?)

Actually, I remembered I had an unupstreamed patch regarding this locally... It turns out that testing check_c_compiler_flag(-unwindlib=none) doesn't work, as the tested flag is added the the compile command (but not to the link command), so the link part of the test command still fails.

The libunwind/src/CMakeLists.txt change sets up the link flag;

Yes, that's necessary for taking it into use when compiling the project. But the flag is also needed when CMake probes for other things during the build system, and without it, all those tests fail. That's why you add it to CMAKE_REQUIRED_FLAGS.

In my case, with my other workarounds removed and this patch applied, I'm getting this from CMake:

-- Performing Test LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG
-- Performing Test LIBUNWIND_SUPPORTS_NOSTDLIBXX_FLAG - Failed
-- Performing Test LIBUNWIND_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG
-- Performing Test LIBUNWIND_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG - Failed
-- Performing Test LIBUNWIND_SUPPORTS_NODEFAULTLIBS_FLAG
-- Performing Test LIBUNWIND_SUPPORTS_NODEFAULTLIBS_FLAG - Failed

If inspecting CMakeFiles/CMakeError.log to see what happened when testing the -unwindlib=none flag, I find this:

Performing C SOURCE FILE Test LIBUNWIND_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG failed with the following output:
Change Dir: /build/llvm-project/libunwind/build-i686-shared/CMakeFiles/CMakeTmp

Run Build Command(s):/usr/bin/ninja cmTC_94016 && [1/2] Building C object CMakeFiles/cmTC_94016.dir/src.c.obj
[2/2] Linking C executable cmTC_94016.exe
FAILED: cmTC_94016.exe
: && /opt/llvm-mingw/bin/i686-w64-mingw32-clang   CMakeFiles/cmTC_94016.dir/src.c.obj -o cmTC_94016.exe -Wl,--out-implib,libcmTC_94016.dll.a -Wl,--major-image-version,0,--minor-image-version,0  -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 && :
lld: error: unable to find library -lunwind
lld: error: unable to find library -lunwind
clang-13: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

During this particular test, CMake does apply the tested flag, -unwindlib=none, when compiling the test object (CMakeFiles/cmTC_94016.dir/src.c.obj) although that's not visible in the log. But it's not applying that flag when it does linking of the test executable. See e.g. https://gitlab.kitware.com/cmake/cmake/-/issues/15934 for discussion on that issue.

If we just blindly add the flag to CMAKE_REQUIRED_FLAGS as I do in D112126, it will get included when doing tests, both when compiling and linking the test objects, making the tests succeed.

So instead of this:

check_c_compiler_flag(-unwindlib=none CONDITION)
if (CONDITION)
  set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS -unwindlib=none"}
endif()

I do this:

set(CMAKE_REQUIRED_FLAGS_ORIG "${CMAKE_REQUIRED_FLAGS}")
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -unwindlib=none")
check_c_compiler_flag("" CONDITION)
if (NOT CONDITION)
  set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS_ORIG}")
endif()

I'm curious to know how it succeeds in your case, if it's linking in -lunwind explicitly when linking the final libunwind library, but not while trying the check_c_compiler_flag(). Is it a case where -lunwind is added implicitly only when linking as C++ but not C? (As far as I've read the clang driver, if it adds it implicitly for either of them, it should add it for both.)

If inspecting CMakeFiles/CMakeError.log to see what happened when testing the -unwindlib=none flag, I find this:

Performing C SOURCE FILE Test LIBUNWIND_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG failed with the following output:
Change Dir: /build/llvm-project/libunwind/build-i686-shared/CMakeFiles/CMakeTmp

Run Build Command(s):/usr/bin/ninja cmTC_94016 && [1/2] Building C object CMakeFiles/cmTC_94016.dir/src.c.obj
[2/2] Linking C executable cmTC_94016.exe
FAILED: cmTC_94016.exe
: && /opt/llvm-mingw/bin/i686-w64-mingw32-clang   CMakeFiles/cmTC_94016.dir/src.c.obj -o cmTC_94016.exe -Wl,--out-implib,libcmTC_94016.dll.a -Wl,--major-image-version,0,--minor-image-version,0  -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 && :
lld: error: unable to find library -lunwind
lld: error: unable to find library -lunwind
clang-13: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

During this particular test, CMake does apply the tested flag, -unwindlib=none, when compiling the test object (CMakeFiles/cmTC_94016.dir/src.c.obj) although that's not visible in the log. But it's not applying that flag when it does linking of the test executable. See e.g. https://gitlab.kitware.com/cmake/cmake/-/issues/15934 for discussion on that issue.

I'm curious to know how it succeeds in your case, if it's linking in -lunwind explicitly when linking the final libunwind library, but not while trying the check_c_compiler_flag(). Is it a case where -lunwind is added implicitly only when linking as C++ but not C? (As far as I've read the clang driver, if it adds it implicitly for either of them, it should add it for both.)

FWIW, it seems like there's more useful info in CMakeFiles/CMakeError.log about what compile command it did use, when testing check_c_compiler_flag(-unwindlib=none), if using the makefile generator instead of the ninja one - as ninja hides the commands that are run except for the commands that actually resulted in an error.

If inspecting CMakeFiles/CMakeError.log to see what happened when testing the -unwindlib=none flag, I find this:

Performing C SOURCE FILE Test LIBUNWIND_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG failed with the following output:
Change Dir: /build/llvm-project/libunwind/build-i686-shared/CMakeFiles/CMakeTmp

Run Build Command(s):/usr/bin/ninja cmTC_94016 && [1/2] Building C object CMakeFiles/cmTC_94016.dir/src.c.obj
[2/2] Linking C executable cmTC_94016.exe
FAILED: cmTC_94016.exe
: && /opt/llvm-mingw/bin/i686-w64-mingw32-clang   CMakeFiles/cmTC_94016.dir/src.c.obj -o cmTC_94016.exe -Wl,--out-implib,libcmTC_94016.dll.a -Wl,--major-image-version,0,--minor-image-version,0  -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 && :
lld: error: unable to find library -lunwind
lld: error: unable to find library -lunwind
clang-13: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

During this particular test, CMake does apply the tested flag, -unwindlib=none, when compiling the test object (CMakeFiles/cmTC_94016.dir/src.c.obj) although that's not visible in the log. But it's not applying that flag when it does linking of the test executable. See e.g. https://gitlab.kitware.com/cmake/cmake/-/issues/15934 for discussion on that issue.

I'm curious to know how it succeeds in your case, if it's linking in -lunwind explicitly when linking the final libunwind library, but not while trying the check_c_compiler_flag(). Is it a case where -lunwind is added implicitly only when linking as C++ but not C? (As far as I've read the clang driver, if it adds it implicitly for either of them, it should add it for both.)

FWIW, it seems like there's more useful info in CMakeFiles/CMakeError.log about what compile command it did use, when testing check_c_compiler_flag(-unwindlib=none), if using the makefile generator instead of the ninja one - as ninja hides the commands that are run except for the commands that actually resulted in an error.

@mstorsjo CMake introduced CMAKE_REQUIRED_LINK_OPTIONS to address specifically this issue (see https://cmake.org/cmake/help/latest/module/CheckCSourceCompiles.html), but unfortunately it was only introduced in 3.14 and LLVM is still on 3.13.

In my testing, in CMake versions <3.14 CMAKE_REQUIRED_FLAGS apply to link command as well but in >=3.14 you need to use CMAKE_REQUIRED_LINK_OPTIONS so I think we'll need to have conditional logic based on the CMake version.

If inspecting CMakeFiles/CMakeError.log to see what happened when testing the -unwindlib=none flag, I find this:

Performing C SOURCE FILE Test LIBUNWIND_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG failed with the following output:
Change Dir: /build/llvm-project/libunwind/build-i686-shared/CMakeFiles/CMakeTmp

Run Build Command(s):/usr/bin/ninja cmTC_94016 && [1/2] Building C object CMakeFiles/cmTC_94016.dir/src.c.obj
[2/2] Linking C executable cmTC_94016.exe
FAILED: cmTC_94016.exe
: && /opt/llvm-mingw/bin/i686-w64-mingw32-clang   CMakeFiles/cmTC_94016.dir/src.c.obj -o cmTC_94016.exe -Wl,--out-implib,libcmTC_94016.dll.a -Wl,--major-image-version,0,--minor-image-version,0  -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 && :
lld: error: unable to find library -lunwind
lld: error: unable to find library -lunwind
clang-13: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

During this particular test, CMake does apply the tested flag, -unwindlib=none, when compiling the test object (CMakeFiles/cmTC_94016.dir/src.c.obj) although that's not visible in the log. But it's not applying that flag when it does linking of the test executable. See e.g. https://gitlab.kitware.com/cmake/cmake/-/issues/15934 for discussion on that issue.

If we just blindly add the flag to CMAKE_REQUIRED_FLAGS as I do in D112126, it will get included when doing tests, both when compiling and linking the test objects, making the tests succeed.

Keep in mind that this applies to situations where both 1) clang passes -lunwind by default and 2) libunwind is not available. How this gets handled may differ between different CMake versions, but in the one I'm using (3.21.3) that's a hard error by default because the toolchain doesn't work, it requires adding -DCMAKE_TRY_COMPILE_TARGET_TYPE=STATIC_LIBRARY to the CMake options anyway (even in LLVM 12) which disables CMake's link tests. And when that is added, that means there are no longer any link tests performed from CMake that LLVM needs to disable.

Keep in mind that this applies to situations where both 1) clang passes -lunwind by default and 2) libunwind is not available.

Yes, this is exactly the setup that I’m focusing on

How this gets handled may differ between different CMake versions, but in the one I'm using (3.21.3) that's a hard error by default because the toolchain doesn't work, it requires adding -DCMAKE_TRY_COMPILE_TARGET_TYPE=STATIC_LIBRARY to the CMake options anyway (even in LLVM 12) which disables CMake's link tests. And when that is added, that means there are no longer any link tests performed from CMake that LLVM needs to disable.

Oh, I see, so you’re manually passing that option? Ok, that would explain the difference between our outcomes. I don’t add that option to libunwind. As long as the cmake file manages to get the right options (-unwindlib=none and -nostdlib++) into CMAKE_REQUIRED_FLAGS, the rest of the tests from that point succeed.

I’ve experimented with setting -DCMAKE_TRY_COMPILE_TARGET_TYPE=STATIC_LIBRARY when building some of the runtimes, but in my experience, it broke other things (making tests falsely succeed as it doesn’t test linking, enabling things that won’t work in the end).

If you try my version of the patch, does it work if you omit setting -DCMAKE_TRY_COMPILE_TARGET_TYPE=STATIC_LIBRARY? It does for me.

@mstorsjo CMake introduced CMAKE_REQUIRED_LINK_OPTIONS to address specifically this issue (see https://cmake.org/cmake/help/latest/module/CheckCSourceCompiles.html), but unfortunately it was only introduced in 3.14 and LLVM is still on 3.13.

In my testing, in CMake versions <3.14 CMAKE_REQUIRED_FLAGS apply to link command as well but in >=3.14 you need to use CMAKE_REQUIRED_LINK_OPTIONS so I think we'll need to have conditional logic based on the CMake version.

That’s not what I was referring to. If newer versions of cmake change whether CMAKE_REQUIRED_FLAGS apply to linking, that’d be a breaking change that requires opt in with a policy, right?

The thing I’m referring to is that when -unwindlib=none is needed for linking to succeed, one can’t test for it with check_c_compiler_flag (which compiles one object file with the flag, where it has no effect, and then tries linking the object file without the flag), but one has to set it in CMAKE_REQUIRED_FLAGS (so it gets included both when compiling and linking, in check_c_compiler_flag) and run a test with it set, see D112126.

hvdijk added a comment.EditedOct 20 2021, 12:09 PM

If you try my version of the patch, does it work if you omit setting -DCMAKE_TRY_COMPILE_TARGET_TYPE=STATIC_LIBRARY? It does for me.

It does not, it fails as it does unpatched, as it does on earlier versions as well:

-- The C compiler identification is Clang 13.0.0
-- The CXX compiler identification is Clang 13.0.0
-- The ASM compiler identification is Clang
-- Found assembler: /usr/bin/clang
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - failed
-- Check for working C compiler: /usr/bin/clang
-- Check for working C compiler: /usr/bin/clang - broken
CMake Error at /usr/share/cmake-3.21/Modules/CMakeTestCCompiler.cmake:69 (message):
  The C compiler

    "/usr/bin/clang"

  is not able to compile a simple test program.

  It fails with the following output:

    Change Dir: /h/libunwind-13.0.0-build-x64/CMakeFiles/CMakeTmp

    Run Build Command(s):/usr/bin/make -f Makefile cmTC_591df/fast && /usr/bin/make  -f CMakeFiles/cmTC_591df.dir/build.make CMakeFiles/cmTC_591df.dir/build
    make[1]: Entering directory '/h/libunwind-13.0.0-build-x64/CMakeFiles/CMakeTmp'
    Building C object CMakeFiles/cmTC_591df.dir/testCCompiler.c.o
    /usr/bin/clang  -m64   -O2  -MD -MT CMakeFiles/cmTC_591df.dir/testCCompiler.c.o -MF CMakeFiles/cmTC_591df.dir/testCCompiler.c.o.d -o CMakeFiles/cmTC_591df.dir/testCCompiler.c.o -c /h/libunwind-13.0.0-build-x64/CMakeFiles/CMakeTmp/testCCompiler.c
    Linking C executable cmTC_591df
    /usr/bin/cmake -E cmake_link_script CMakeFiles/cmTC_591df.dir/link.txt --verbose=1
    /usr/bin/clang  -m64 -O2  -Wl,-O1 -Wl,--as-needed -s  CMakeFiles/cmTC_591df.dir/testCCompiler.c.o -o cmTC_591df
    ld.lld: error: unable to find library -l:libunwind.so
    ld.lld: error: unable to find library -l:libunwind.so
    clang: error: linker command failed with exit code 1 (use -v to see invocation)
    make[1]: *** [CMakeFiles/cmTC_591df.dir/build.make:100: cmTC_591df] Error 1
    make[1]: Leaving directory '/h/libunwind-13.0.0-build-x64/CMakeFiles/CMakeTmp'
    make: *** [Makefile:127: cmTC_591df/fast] Error 2





  CMake will not be able to correctly generate this project.
Call Stack (most recent call first):
  CMakeLists.txt:24 (project)


-- Configuring incomplete, errors occurred!
See also "/h/libunwind-13.0.0-build-x64/CMakeFiles/CMakeOutput.log".
See also "/h/libunwind-13.0.0-build-x64/CMakeFiles/CMakeError.log".

I’ve experimented with setting -DCMAKE_TRY_COMPILE_TARGET_TYPE=STATIC_LIBRARY when building some of the runtimes, but in my experience, it broke other things (making tests falsely succeed as it doesn’t test linking, enabling things that won’t work in the end).

It's a special option needed in special circumstances, to be handled with care. The situation here is that clang was installed from the split tarball, configured to use compiler-rt and libunwind rather than libgcc and libgcc_s, and that clang is then used to install compiler-rt and libunwind. That means that while configuring compiler-rt and libunwind, the compiler is broken for most link steps by design, but it is good enough to build and install the core compiler-rt bits (libclang_rt.builtins, clang_rt.crtbegin, clang_rt.crtend) and libunwind. Once they are installed, everything else, including the rest of compiler-rt, should then be configued without -DCMAKE_TRY_COMPILE_TARGET_TYPE=STATIC_LIBRARY.

If you try my version of the patch, does it work if you omit setting -DCMAKE_TRY_COMPILE_TARGET_TYPE=STATIC_LIBRARY? It does for me.

It does not, it fails as it does unpatched, as it does on earlier versions as well:

-- Check for working C compiler: /usr/bin/clang
-- Check for working C compiler: /usr/bin/clang - broken
CMake Error at /usr/share/cmake-3.21/Modules/CMakeTestCCompiler.cmake:69 (message):
  The C compiler

    "/usr/bin/clang"

  is not able to compile a simple test program.

Ah, I see. Can you try by adding -DCMAKE_C_COMPILER_WORKS=TRUE -DCMAKE_CXX_COMPILER_WORKS=TRUE? That waives this particular failure.

That means that while configuring compiler-rt and libunwind, the compiler is broken for most link steps by design, but it is good enough to build and install the core compiler-rt bits (libclang_rt.builtins, clang_rt.crtbegin, clang_rt.crtend) and libunwind.

Yes, this is pretty much exactly my setup too (but targeting mingw).

Once they are installed, everything else, including the rest of compiler-rt, should then be configued without -DCMAKE_TRY_COMPILE_TARGET_TYPE=STATIC_LIBRARY.

-DCMAKE_TRY_COMPILE_TARGET_TYPE=STATIC_LIBRARY is too blunt an instrument for this, for my configurations (targeting mingw). If I build libunwind with that set, linking the shared libunwind fails with these errors:

lld: error: unable to find library -lc
lld: error: unable to find library -ldl
lld: error: unable to find library -ldl

Because libunwind does have checks for whether e.g. -lc and -ldl work, and these give false positives with that setting. So if one were to use CMAKE_TRY_COMPILE_TARGET_TYPE=STATIC_LIBRARY, it would have to be turned on and off over specific sections in the cmakefiles. But with D112126 and -DCMAKE_C_COMPILER_WORKS=TRUE -DCMAKE_CXX_COMPILER_WORKS=TRUE, without touching CMAKE_TRY_COMPILE_TARGET_TYPE, it does work for me.

Ah, I see. Can you try by adding -DCMAKE_C_COMPILER_WORKS=TRUE -DCMAKE_CXX_COMPILER_WORKS=TRUE? That waives this particular failure.

I tried that at first (for both compiler-rt and libunwind -- I don't think we should be using different workarounds for the two), that breaks in a different way:

CMake Error at cmake/config-ix.cmake:197 (message):
  Please use architecture with 4 or 8 byte pointers.
Call Stack (most recent call first):
  CMakeLists.txt:251 (include)

This happens because simply telling CMake that linking works will make CMake assume that linking works. It then uses linking in all of its detection steps, so all of its detection steps fail.

-DCMAKE_TRY_COMPILE_TARGET_TYPE=STATIC_LIBRARY is too blunt an instrument for this, for my configurations (targeting mingw). If I build libunwind with that set, linking the shared libunwind fails with these errors:

lld: error: unable to find library -lc
lld: error: unable to find library -ldl
lld: error: unable to find library -ldl

Because libunwind does have checks for whether e.g. -lc and -ldl work, and these give false positives with that setting.

Thanks for the explanation, will have a think about how we can have things work for both of us.

Ah, I see. Can you try by adding -DCMAKE_C_COMPILER_WORKS=TRUE -DCMAKE_CXX_COMPILER_WORKS=TRUE? That waives this particular failure.

I tried that at first (for both compiler-rt and libunwind -- I don't think we should be using different workarounds for the two),

Hmm. I used to use CMAKE_C[XX]_COMPILER_WORKS for compiler-rt, too, but since 8368e4d54c459fe173d76277f17c632478e91add / D91334, compiler-rt/lib/builtins does set CMAKE_TRY_COMPILE_TARGET_TYPE=STATIC_LIBRARY (which works fine there, because it doesn't do any linker testing - as it actually only is building a static library), so I no longer need to set those flags there.

that breaks in a different way:

CMake Error at cmake/config-ix.cmake:197 (message):
  Please use architecture with 4 or 8 byte pointers.
Call Stack (most recent call first):
  CMakeLists.txt:251 (include)

That's curious.

If sidestepping this issue a little - if you'd only add those *_WORKS flags to libunwind, do things work there too?

When running into this error in compiler-rt, were you pointing cmake at compiler-rt/lib/builtins or plain compiler-rt? From some quick grepping, it seems like this only would be triggered by the toplevel compiler-rt. In my setups, I first point cmake at compiler-rt/lib/builtins and build that, then build libunwind, libcxxabi, libcxx, then after that point it at the root of compiler-rt to build all of it.

hvdijk abandoned this revision.Oct 20 2021, 1:38 PM

If sidestepping this issue a little - if you'd only add those *_WORKS flags to libunwind, do things work there too?

When running into this error in compiler-rt, were you pointing cmake at compiler-rt/lib/builtins or plain compiler-rt?

That will be the relevant difference, thanks. I was pointing to compiler-rt and explicitly setting -DCOMPILER_RT_BUILD_SANITIZERS=OFF -DCOMPILER_RT_BUILD_XRAY=OFF -DCOMPILER_RT_BUILD_LIBFUZZER=OFF -DCOMPILER_RT_BUILD_PROFILE=OFF -DCOMPILER_RT_BUILD_MEMPROF=OFF -DCOMPILER_RT_BUILD_ORC=OFF. I'll try soon whether configuring from lib/builtins directly, and using *_WORKS for libunwind, works for me too, but based on what you've said so far, if it doesn't yet, I'm sure we can find a way to make it work. I'm happy to pre-emptively abandon this revision, will follow up on yours once I can verify that everything works.