These are changes to allow GCC 13 to successfully compile the runtimes stack.
Details
- Reviewers
ldionne MaskRay - Group Reviewers
Restricted Project Restricted Project - Commits
- rG3537338d1ab9: [libc++][libunwind] Fixes to allow GCC 13 to compile libunwind/libc++abi/libc++
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
libunwind/src/CMakeLists.txt | ||
---|---|---|
149 | Why is this needed? Shouldn't it work without linking c++abi/standard library? |
libunwind/src/CMakeLists.txt | ||
---|---|---|
149 | Indeed, libunwind must not rely on the C++ standard library - that's a layering violation. What's the issue that this tries to fix? libunwind explicitly doesn't even include C++ standard library headers (building with -nostdinc++) in order to stay clear of any dependencies on it. Not sure if any of the CI configurations would show the issue here, but if we'd have the new CI configuration from D150766 merged, that one most definitely would fail on this change. |
libunwind/src/CMakeLists.txt | ||
---|---|---|
149 | GCC 13 doesn't accept -nostdlib++ in C mode. Given that these flags are passed explicitly, I don't think there should be a problem with invoking the driver in C++ mode. |
This LGTM. I think it should be fine to link libunwind as C++ since we pass -nostdlib++, but please make sure @arichardson and @mstorsjo are on board before landing.
libcxx/include/__type_traits/remove_cv.h | ||
---|---|---|
22–38 | #if __has_builtin(__remove_cv) // GCC can't mangle builtins so they can't appear in an alias directly # if !defined(_LIBCPP_COMPILER_GCC) template <class _Tp> using __remove_cv_t = __remove_cv(_Tp); #else template <class _Tp> struct __libcpp_remove_cv { using type _LIBCPP_NODEBUG = __remove_cv(_Tp); }; template <class _Tp> using __remove_cv_t = typename __libcpp_remove_cv<_Tp>::type; #endif #else template <class _Tp> using __remove_cv_t = __remove_volatile_t<__remove_const_t<_Tp> >; #endif // __has_builtin(__remove_cv) This is equivalent but the logic seems a bit easier to follow. | |
libcxx/include/__type_traits/remove_cvref.h | ||
23 | Same comments here. | |
libunwind/src/CMakeLists.txt | ||
1 | @power-llvm-team Can you please help us understand the failure on AIX at https://buildkite.com/llvm-project/libcxx-ci/builds/24846#01885016-92a1-4955-a7cb-e87fc56b59e5 ? All the tests on AIX are failing with something like: Could not load program /scratch/powerllvm/cpap8008/llvm-project/libcxx-ci/build/aix/test/std/utilities/expected/expected.void/observers/Output/error.pass.cpp.dir/t.tmp.exe: Symbol resolution failed for /scratch/powerllvm/cpap8008/llvm-project/libcxx-ci/build/aix/lib/libunwind.a(libunwind.so.1) because: Symbol unw_getcontext (number 14) is not exported from dependent module /scratch/powerllvm/cpap8008/llvm-project/libcxx-ci/build/aix/lib/libunwind.a(libunwind.so.1). Symbol unw_init_local (number 15) is not exported from dependent module /scratch/powerllvm/cpap8008/llvm-project/libcxx-ci/build/aix/lib/libunwind.a(libunwind.so.1). Symbol unw_step (number 16) is not exported from dependent module /scratch/powerllvm/cpap8008/llvm-project/libcxx-ci/build/aix/lib/libunwind.a(libunwind.so.1). Symbol unw_get_reg (number 17) is not exported from dependent module /scratch/powerllvm/cpap8008/llvm-project/libcxx-ci/build/aix/lib/libunwind.a(libunwind.so.1). Symbol unw_get_fpreg (number 18) is not exported from dependent module /scratch/powerllvm/cpap8008/llvm-project/libcxx-ci/build/aix/lib/libunwind.a(libunwind.so.1). [...] Examine .loader section symbols with the 'dump -Tv' command. error: command failed with exit status: 255 This is likely related to changing the linker language from C to CXX and it seems like suddenly the symbol names exported by libunwind are not correct anymore, but that's weird cause everything should be in extern "C" context anyway. |
libunwind/src/CMakeLists.txt | ||
---|---|---|
149 | This did build fine for me, so consider this objection withdrawn. |
libunwind LGTM, but I suspect the LINKER_LANGUAGE CXX change in libunwind is adding a workaround on top of another one D95875
Newer GCC supports g++ -nostdlib++ but not gcc -nostdlib++. If we don't add -nostdlib++ to link flags, we can keep LINKER_LANGUAGE C.
libunwind/src/CMakeLists.txt | ||
---|---|---|
149 | The -nostdlib++ can be removed. It was initially added for consistency with libcxx and libcxxabi configurations but those are built with CXX. |
@MaskRay I've removed both -nostdinc++ and -nodefaultlibs, since the latter doesn't add anything that we don't re-add anyways. I'll try to make some cleanup patches later, but we should really get this fixed, since LLVM17 should support GCC13.
This is equivalent but the logic seems a bit easier to follow.