Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

seastar does not build with clang-17 + libstdc++ + C++23 #1772

Closed
tchaikov opened this issue Aug 8, 2023 · 13 comments
Closed

seastar does not build with clang-17 + libstdc++ + C++23 #1772

tchaikov opened this issue Aug 8, 2023 · 13 comments

Comments

@tchaikov
Copy link
Contributor

tchaikov commented Aug 8, 2023

cmake -Bbuild/cxx23 -GNinja -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_STANDARD=23

and the build fails with errors like

FAILED: CMakeFiles/seastar.dir/src/core/reactor.cc.o 
/home/kefu/.local/bin/clang++ -DBOOST_NO_CXX98_FUNCTION_BASE -DFMT_SHARED -DSEASTAR_API_LEVEL=7 -DSEASTAR_DEBUG -DSEASTAR_DEBUG_SHARED_PTR -DSEASTAR_DEFAULT_ALLOCATOR -DSEASTAR_DEFERRED_ACTION_REQUIRE_NOEXCEPT -DSEASTAR_HAS_MEMBARRIER -DSEASTAR_HAVE_ASAN_FIBER_SUPPORT -DSEASTAR_HAVE_HWLOC -DSEASTAR_HAVE_NUMA -DSEASTAR_HAVE_URING -DSEASTAR_SCHEDULING_GROUPS_COUNT=16 -DSEASTAR_SHUFFLE_TASK_QUEUE -DSEASTAR_THREAD_STACK_GUARDS -DSEASTAR_TYPE_ERASE_MORE -I/home/kefu/dev/seastar/include -I/home/kefu/dev/seastar/build/debug/gen/include -I/home/kefu/dev/seastar/src -g -std=gnu++23 -U_FORTIFY_SOURCE -DSEASTAR_SSTRING -Wno-error=unused-result "-Wno-error=#warnings" -fstack-clash-protection -UNDEBUG -Wall -Werror -Wimplicit-fallthrough -Wno-array-bounds -Wno-error=deprecated-declarations -fvisibility=hidden -gz -fsanitize=address -fsanitize=undefined -fno-sanitize=vptr -MD -MT CMakeFiles/seastar.dir/src/core/reactor.cc.o -MF CMakeFiles/seastar.dir/src/core/reactor.cc.o.d -o CMakeFiles/seastar.dir/src/core/reactor.cc.o -c /home/kefu/dev/seastar/src/core/reactor.cc
In file included from /home/kefu/dev/seastar/src/core/reactor.cc:29:
In file included from /usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/chrono:45:
In file included from /usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/sstream:40:
In file included from /usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/istream:40:
In file included from /usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/ios:44:
In file included from /usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/ios_base.h:41:
In file included from /usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/locale_classes.h:40:
In file included from /usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/string:58:
In file included from /usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/memory_resource.h:41:
In file included from /usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/uses_allocator_args.h:38:
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/tuple:691:2: error: pack expansion contains parameter pack '_UTypes' that has a different length (1 vs. 2) from outer parameter packs
  691 |         using __convertible = __and_<is_convertible<_UTypes, _Types>...>;
      |         ^~~~~
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/tuple:853:27: note: in instantiation of template type alias '__convertible' requested here
  853 |           = _TCC<true>::template __convertible<_Args...>::value;
      |                                  ^
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/tuple:948:12: note: in instantiation of static data member 'std::tuple<std::tuple<seastar::pollable_fd, seastar::socket_address>>::__convertible<seastar::pollable_fd &, seastar::socket_address &>' requested here
  948 |         explicit(!__convertible<_UElements&...>)
      |                   ^
/home/kefu/dev/seastar/include/seastar/core/future.hh:617:33: note: while substituting deduced template arguments into function template 'tuple' [with _UElements = <seastar::pollable_fd, seastar::socket_address>]
  617 |         this->uninitialized_set(std::forward<A>(a)...);
      |                                 ^
/home/kefu/dev/seastar/include/seastar/core/future.hh:1241:56: note: in instantiation of function template specialization 'seastar::future_state<std::tuple<seastar::pollable_fd, seastar::socket_address>>::future_state<std::tuple<seastar::pollable_fd, seastar::socket_address>>' requested here
 1241 |     future(ready_future_marker m, A&&... a) noexcept : _state(m, std::forward<A>(a)...) { }
      |                                                        ^
/home/kefu/dev/seastar/include/seastar/core/future.hh:1917:12: note: in instantiation of function template specialization 'seastar::future<std::tuple<seastar::pollable_fd, seastar::socket_address>>::future<std::tuple<seastar::pollable_fd, seastar::socket_address>>' requested here
 1917 |     return future<T>(ready_future_marker(), std::forward<A>(value)...);
      |            ^
/home/kefu/dev/seastar/src/core/reactor.cc:297:16: note: in instantiation of function template specialization 'seastar::make_ready_future<std::tuple<seastar::pollable_fd, seastar::socket_address>, std::tuple<seastar::pollable_fd, seastar::socket_address>>' requested here
  297 |         return make_ready_future<std::tuple<pollable_fd, socket_address>>(std::make_tuple(std::move(pfd), std::move(sa)));
      |                ^

this failure can be reproduced with

  • clang-16 (16.0.6) + libstd++ (from GCC-13) + C++23
  • clang-17 (f042890521f314cfd0bb0a3b8399d2c7d1bd526b) + libstd++ (from GCC-13) + C++23

this failure cannot be reproduced with GCC-13 + C++23.

this is very likely a bug in Clang. and it has been reported at llvm/llvm-project#61415

@tchaikov
Copy link
Contributor Author

tchaikov commented Jan 1, 2024

with clang-18 (90c397fc56b7a04dd53cdad8103de1ead9686104), the tree builds just fine. but with clang-17.0.6, the FTBFS still exists.

@tchaikov tchaikov changed the title seastar does not build with clang + libstdc++ + C++23 seastar does not build with clang-17 + libstdc++ + C++23 Jan 1, 2024
@mykaul
Copy link

mykaul commented Jan 2, 2024

with clang-18 (90c397fc56b7a04dd53cdad8103de1ead9686104), the tree builds just fine. but with clang-17.0.6, the FTBFS still exists.

We should really have a GH action that compiles the code with all various supported (or not) compilers...

@tchaikov
Copy link
Contributor Author

tchaikov commented Jan 2, 2024

Probably we can switch to GitHub workflow and use something like https://github.com/aminya/setup-cpp? (was proposed at #1999 (comment) as well)

@mykaul
Copy link

mykaul commented Jan 2, 2024

The only challenge is that it might take ages, so we want to do it selectively (opt-in?).

@tchaikov
Copy link
Contributor Author

tchaikov commented Jan 5, 2024

why switching to another CI would take ages? we just need to

  1. pick a runner image. the reason why we use ubuntu mantic was that it ships the compilers we need to test, but with setup-cpp action, we can just use an LTS release. i'd use ubuntu-latest. see https://github.com/actions/runner-images#available-images
  2. add the github workflow to
    1. install the dependencies with install-dependencies.sh
    2. setup a matrix of: compilers, c++ standards, build (Debug, Release, Dev), and some features (dpdk, C++ modules, for instance)
    3. install the used compiler
    4. build and test

but IMO, the challenge is probably that the priority is not high enough.

@mykaul
Copy link

mykaul commented Jan 7, 2024

If we compile on Github infra, it'll be very slow. If we run on our infra, it'll take resources (which we may or may not have)

@tchaikov
Copy link
Contributor Author

tchaikov commented Jan 22, 2024

If we compile on Github infra, it'll be very slow. If we run on our infra, it'll take resources (which we may or may not have)

not that slow. it took 32m to finish all the builds and tests, including the one with dpdk enabled. see https://github.com/tchaikov/seastar/actions/runs/7606516120, while CircleCI takes 35min in general.

@tchaikov
Copy link
Contributor Author

created #2050 to add the github workflow based test. will drop the circleci based workflow once the github one gets merged.

@tchaikov
Copy link
Contributor Author

please note, clang-18 (230c13d59d0843c3b738920b85c341cc78a61fa9) + libstdc++ (from 13.2.1) + C++23 builds fine.

@tchaikov
Copy link
Contributor Author

i just bisected llvm, it seems it was 128b3b61fe6768c724975fd1df2be0abec848cf6 which fixed the issue. this commit was included by llvm/llvm-project#70548, which in turn fixes llvm/llvm-project#59827 .

@tchaikov
Copy link
Contributor Author

i created https://src.fedoraproject.org/rpms/clang/pull-request/225 to backport this fix to rawhide, hopefully once it gets merged we can backport the patch to f39 and f38.

@tchaikov
Copy link
Contributor Author

tchaikov commented Jan 26, 2024

but, please note, our docker image used for running CI is based on ubuntu:mantic. so in order to get the change included by ubuntu and debian, we need to file a ticket on launchpad and BTS. but a simpler way is just to redo the image, see #2058, and use Clang-18 instead of Clang-17.

@tchaikov
Copy link
Contributor Author

i think this issue has served its purpose. as the fixes in distros packagings or in clang/llvm are not in the scope of Seastar, i am closing this issue.

@tchaikov tchaikov closed this as not planned Won't fix, can't repro, duplicate, stale Jan 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants