[libcxx-commits] [PATCH] D151387: [libc++][libunwind] Fixes to allow GCC 13 to compile libunwind/libc++abi/libc++

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision as: libc++, ldionne.
ldionne added a subscriber: power-llvm-team.
ldionne added a comment.

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.



================
Comment at: libcxx/include/__type_traits/remove_cv.h:22-38
+#if __has_builtin(__remove_cv) && !defined(_LIBCPP_COMPILER_GCC)
+
+template <class _Tp>
+using __remove_cv_t = __remove_cv(_Tp);
+#else
 #if __has_builtin(__remove_cv)
 template <class _Tp>
----------------
```
#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.


================
Comment at: libcxx/include/__type_traits/remove_cvref.h:23
 
-#if __has_builtin(__remove_cvref)
+#if __has_builtin(__remove_cvref) && !defined(_LIBCPP_COMPILER_GCC)
+
----------------
Same comments here.


================
Comment at: libunwind/src/CMakeLists.txt:1
 # Get sources
 
----------------
@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.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151387/new/

https://reviews.llvm.org/D151387