-
Notifications
You must be signed in to change notification settings - Fork 36.2k
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
util: Work around ParseHex gcc cross compiler bug #27218
The head ref may contain hidden characters: "2303-util-workaround-gcc-\u{1F539}"
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
To test on a fresh install of Ubuntu/Debian with gcc-11 or later: #25227 (comment) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK fa8481b, tested on Ubuntu 22.04, gcc 11.3 for the riscv64-linux-gnu
host.,
ACK fa8481b Tested cross compiling through depends on Ubuntu Jammy (22.04) with GCC 11 on these hosts: |
Does anyone have a link to an upstream issue? Is this a regression in GCC 11, that has been fixed in 12/13? |
It is not fixed in gcc-12, see #25227 (comment). gcc-13 exists, so it may be possible to test. I have not attempted to reduce the test case, nor did I dig deeper to find out if this is a compiler bug or bug in our code, because I don't think we have a choice here: If there are two equivalent versions of the same code, but one doesn't compile, we should pick the one that compiles. |
To clarify, even if the workaround is not needed in gcc-13 or gcc-14, I think we shouldn't revert it. I see it as a permanent workaround. |
Still broken in GCC 13.0.1. |
fa8481b util: Work around ParseHex gcc cross compiler bug (MarcoFalke) Pull request description: I fail to see how an explicit `ParseHex` template instantiation fails to also instantiate `TryParseHex`. Nonetheless, to work around a compiler bug, change the explicit instantiation from `ParseHex` to `TryParseHex`. (`ParseHex` is inline anyway and will be instantiated by the compiler either way). Fixes bitcoin#25227 (comment) : ``` CXXLD bitcoind /usr/lib/gcc-cross/powerpc64le-linux-gnu/11/../../../../powerpc64le-linux-gnu/bin/ld: libbitcoin_node.a(libbitcoin_node_a-net_processing.o): in function `(anonymous namespace)::PeerManagerImpl::ProcessMessage(CNode&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, CDataStream&, std::chrono::duration<long, std::ratio<1l, 1000000l> >, std::atomic<bool> const&)': net_processing.cpp:(.text+0x29660): undefined reference to `std::optional<std::vector<unsigned char, std::allocator<unsigned char> > > TryParseHex<unsigned char>(std::basic_string_view<char, std::char_traits<char> >)' /usr/lib/gcc-cross/powerpc64le-linux-gnu/11/../../../../powerpc64le-linux-gnu/bin/ld: libbitcoin_node.a(libbitcoin_node_a-rest.o): in function `rest_getutxos(std::any const&, HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)': rest.cpp:(.text+0x83b4): undefined reference to `std::optional<std::vector<unsigned char, std::allocator<unsigned char> > > TryParseHex<unsigned char>(std::basic_string_view<char, std::char_traits<char> >)' /usr/lib/gcc-cross/powerpc64le-linux-gnu/11/../../../../powerpc64le-linux-gnu/bin/ld: libbitcoin_node.a(libbitcoin_node_a-torcontrol.o): in function `std::vector<unsigned char, std::allocator<unsigned char> > ParseHex<unsigned char>(std::basic_string_view<char, std::char_traits<char> >)': torcontrol.cpp:(.text._Z8ParseHexIhESt6vectorIT_SaIS1_EESt17basic_string_viewIcSt11char_traitsIcEE[_Z8ParseHexIhESt6vectorIT_SaIS1_EESt17basic_string_viewIcSt11char_traitsIcEE]+0x2c): undefined reference to `std::optional<std::vector<unsigned char, std::allocator<unsigned char> > > TryParseHex<unsigned char>(std::basic_string_view<char, std::char_traits<char> >)' /usr/lib/gcc-cross/powerpc64le-linux-gnu/11/../../../../powerpc64le-linux-gnu/bin/ld: libbitcoin_common.a(libbitcoin_common_a-external_signer.o): in function `ExternalSigner::SignTransaction(PartiallySignedTransaction&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&)': external_signer.cpp:(.text+0x8d84): undefined reference to `std::optional<std::vector<unsigned char, std::allocator<unsigned char> > > TryParseHex<unsigned char>(std::basic_string_view<char, std::char_traits<char> >)' collect2: error: ld returned 1 exit status ACKs for top commit: gruve-p: ACK bitcoin@fa8481b hebasto: ACK fa8481b, tested on Ubuntu 22.04, gcc 11.3 for the `riscv64-linux-gnu` host., Tree-SHA512: 53efa424e7e18d85a2c9ac2267b9370ae3594d9be73da5135a3a79bf07ab50fcc5357cbde09dc0b2a9eb78d78ec37beae0c9f876333b568e678b9d0067bc9e4e
I fail to see how an explicit
ParseHex
template instantiation fails to also instantiateTryParseHex
.Nonetheless, to work around a compiler bug, change the explicit instantiation from
ParseHex
toTryParseHex
. (ParseHex
is inline anyway and will be instantiated by the compiler either way).Fixes #25227 (comment) :