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
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Comment Actions
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.
Comment Actions
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?)
Comment Actions
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.)
Comment Actions
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.
Comment Actions
@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.
Comment Actions
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.
Comment Actions
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.
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.
Comment Actions
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.
Comment Actions
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.
Comment Actions
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 -ldlBecause 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.
Comment Actions
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.
Comment Actions
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.