stringop-overflow warning with GCC 14

achow101 commented at 6:24 pm on May 15, 2024: member

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

 0In file included from /usr/include/c++/14.1.1/string:51,
 1                 from ../../../src/crypto/sha256.h:10,
 2                 from ../../../src/hash.h:12,
 3                 from ../../../src/pubkey.h:10,
 4                 from ../../../src/addresstype.h:9,
 5                 from ../../../src/wallet/wallet.h:9,
 6                 from ../../../src/wallet/test/wallet_tests.cpp:5:
 7In 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]’,
 8    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,
 9    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,
10    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,
11    inlined from ‘void wallet::wallet_tests::PollutePubKey(CPubKey&)’ at ../../../src/wallet/test/wallet_tests.cpp:503:14:
12/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=]
13  952 |         *__first = __tmp;
14      |         ~~~~~~~~~^~~~~~~
15cc1plus: all warnings being treated as errors

Expected behaviour

Should build without warnings.

Steps to reproduce

Build with GCC 14.1.1

0$ gcc --version
1gcc (GCC) 14.1.1 20240507
2Copyright (C) 2024 Free Software Foundation, Inc.
3This is free software; see the source for copying conditions.  There is NO
4warranty; 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 commented at 6:28 pm on May 15, 2024: member

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

willcl-ark added the label Build system on May 16, 2024

laanwj commented at 9:21 am on May 17, 2024: member

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 commented at 6:31 pm on May 17, 2024: member

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 commented at 6:46 pm on May 17, 2024: member

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.

fanquake closed this on May 22, 2024

stringop-overflow warning with GCC 14 #30114