stringop-overflow warning with GCC 14

[achow101]

Is there an existing issue for this?

• I have searched the existing issues

Current behaviour

When building with GCC 14, I noticed a new warning which is causing -Werror builds to fail

In file included from /usr/include/c++/14.1.1/string:51,
                 from ../../../src/crypto/sha256.h:10,
                 from ../../../src/hash.h:12,
                 from ../../../src/pubkey.h:10,
                 from ../../../src/addresstype.h:9,
                 from ../../../src/wallet/wallet.h:9,
                 from ../../../src/wallet/test/wallet_tests.cpp:5:
In function ‘constexpr typename __gnu_cxx::__enable_if<std::__is_scalar<_Tp>::__value, void>::__type std::__fill_a1(_ForwardIterator, _ForwardIterator, const _Tp&) [with _ForwardIterator = unsigned char*; _Tp = int]’,
    inlined from ‘constexpr void std::__fill_a1(__gnu_cxx::__normal_iterator<_Iterator, _Container>, __gnu_cxx::__normal_iterator<_Iterator, _Container>, const _Tp&) [with _Ite = unsigned char*; _Cont = vector<unsigned char>; _Tp = int]’ at /usr/include/c++/14.1.1/bits/stl_algobase.h:981:21,
    inlined from ‘constexpr void std::__fill_a(_FIte, _FIte, const _Tp&) [with _FIte = __gnu_cxx::__normal_iterator<unsigned char*, vector<unsigned char> >; _Tp = int]’ at /usr/include/c++/14.1.1/bits/stl_algobase.h:998:21,
    inlined from ‘constexpr void std::fill(_ForwardIterator, _ForwardIterator, const _Tp&) [with _ForwardIterator = __gnu_cxx::__normal_iterator<unsigned char*, vector<unsigned char> >; _Tp = int]’ at /usr/include/c++/14.1.1/bits/stl_algobase.h:1029:20,
    inlined from ‘void wallet::wallet_tests::PollutePubKey(CPubKey&)’ at ../../../src/wallet/test/wallet_tests.cpp:503:14:
/usr/include/c++/14.1.1/bits/stl_algobase.h:952:18: error: ‘void* __builtin_memset(void*, int, long unsigned int)’ specified bound 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
  952 |         *__first = __tmp;
      |         ~~~~~~~~~^~~~~~~
cc1plus: all warnings being treated as errors

Expected behaviour

Should build without warnings.

Steps to reproduce

Build with GCC 14.1.1

$ gcc --version
gcc (GCC) 14.1.1 20240507
Copyright (C) 2024 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Relevant log output

No response

How did you obtain Bitcoin Core

Compiled from source

What version of Bitcoin Core are you using?

master@7a40f2a3f1cf744d136ecf534979114e79c5e71d

Operating system and version

Arch

Machine specifications

No response

[achow101]

I think might be a GCC bug, I can't figure out what is actually wrong with this code that would trigger the warning.

[laanwj]

Looks like it derives that .begin() is the same as .end() which would bring the size to -1. This would require CPubKey::GetLen to return 0, which is possible if the first byte is invalid. Maybe it'd make sense to make it return 1 in that case because there's always the first byte?

[achow101]

Looks like it derives that .begin() is the same as .end() which would bring the size to -1. This would require CPubKey::GetLen to return 0, which is possible if the first byte is invalid. Maybe it'd make sense to make it return 1 in that case because there's always the first byte?

That worked, but since CPubKey is used in a ton of places, I'm a little bit hesitant to change GetLen since that may have unexpected side effects. However, I do have a fix that seems to work that changes to using insert() instead of std::fill.

[laanwj]

Yes, could also handle it at another level, though it's strange for a class to return begin()==end() when there's actually one byte in it.

Edit: but yes it's too risky to change this just to work around a compiler warning in the tests.