Re: gdb builder status (Was: Adding binutils to the GNU Toolchain buildbot on sourceware)

On 2022-05-01 15:44, Mark Wielaard wrote:
> Hi,
> 
> On Fri, Apr 29, 2022 at 10:04:22PM +0200, Mark Wielaard via Overseers wrote:
>> The fedora-s390x failure looks as follows:
>> https://builder.sourceware.org/buildbot/#/builders/gdb-fedora-s390x
>>
>> In file included from /usr/include/c++/11/string:40,
>>                  from ../../binutils-gdb/gdb/../gdbsupport/ptid.h:36,
>>                  from ../../binutils-gdb/gdb/../gdbsupport/common-defs.h:198,
>>                  from ../../binutils-gdb/gdb/defs.h:28,
>>                  from ../../binutils-gdb/gdb/debuginfod-support.c:19:
>> In static member function ‘static constexpr const char_type* std::char_traits<char>::find(const char_type*, std::size_t, const char_type&)’,
>>     inlined from ‘constexpr std::basic_string_view<_CharT, _Traits>::size_type std::basic_string_view<_CharT, _Traits>::find(_CharT, std::basic_string_view<_CharT, _Traits>::size_type) const [with _CharT = char; _Traits = std::char_traits<char>]’ at /usr/include/c++/11/bits/string_view.tcc:87:41,
>>     inlined from ‘constexpr std::basic_string_view<_CharT, _Traits>::size_type std::basic_string_view<_CharT, _Traits>::find_first_of(_CharT, std::basic_string_view<_CharT, _Traits>::size_type) const [with _CharT = char; _Traits = std::char_traits<char>]’ at /usr/include/c++/11/string_view:431:26,
>>     inlined from ‘bool debuginfod_is_enabled()’ at ../../binutils-gdb/gdb/debuginfod-support.c:194:33:
>> /usr/include/c++/11/bits/char_traits.h:413:62: error: ‘void* __builtin_memchr(const void*, int, long unsigned int)’ specified bound 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overread]
>>   413 |         return static_cast<const char_type*>(__builtin_memchr(__s, __a, __n));
>>       |                                              ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
>>
> 
> This is really weird. I don't know why, but for some reason g++ (GCC)
> 11.2.1 20220127 (Red Hat 11.2.1-9) on s390x seems convinced the string
> might be of SIZE_MAX lenght. And so complains because the length of an
> array can only be PTRDIFF_MAX. The following "fixes" it:
> 
> diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c
> index 4ce2e786..d4f8a55c 100644
> --- a/gdb/debuginfod-support.c
> +++ b/gdb/debuginfod-support.c
> @@ -190,7 +190,10 @@ debuginfod_is_enabled ()
>           size_t off = url_view.find_first_not_of (' ');
>           if (off == gdb::string_view::npos)
>             break;
> -         url_view = url_view.substr (off);
> +         /* Use PTRDIFF_MAX otherwise g++ might (wrongly) believe
> +            the string might be SIZE_MAX and warn for specified bound
> +            exceeding maximum object size on find.  */
> +         url_view = url_view.substr (off, PTRDIFF_MAX);
>           off = url_view.find_first_of (' ');
>           gdb_printf
>             (_("  <%ps>\n"),
> 
> Is that a reasonable workaround?

Hi Mark,

If it's really just the diagnostic that is bogus, my preference would be
to try to silence the diagnostic and not modify the code.

It would require adding a DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD to
include/diagnostics.h.

I would also limit the disabling to just that arch, like:

#if defined (__s390x__) || defined (__s390__)
# DIAGNOSTIC_PUSH
# DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD
#endif
// the code
#if defined (__s390x__) || defined (__s390__)
# DIAGNOSTIC_POP
#endif

And if that bug ever gets fixed in gcc (let's say in gcc 13), I would
change the condition (hopefully we remember) to only disable the warning
on gcc <= 12.  This way, when someone sees this in 20 years, when we
don't care about gcc 12 anymore, they'll know they can remove the
workaround.

Sometimes this doesn't work though, as it would require putting pragma
in the libstdc++ header, which we obviously can't do.  In that case I
would fall back to what you propose.

Simon