* [PATCH] gdb: Workaround stringop-overread warning in debuginfod-support.c on powerpc64 @ 2022-05-11 22:52 Mark Wielaard 2022-05-12 0:49 ` Simon Marchi 0 siblings, 1 reply; 7+ messages in thread From: Mark Wielaard @ 2022-05-11 22:52 UTC (permalink / raw) To: gdb-patches; +Cc: Mark Wielaard Just like on s390x with g++ 11.2.1, ppc64le with g++ 11.3.1 produces a spurious warning for stringop-overread in debuginfod_is_enabled for url_view. Also suppress it on powerpc64. gdb/ChangeLog: * debuginfod-support.c (debuginfod_is_enabled): Use DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD on powerpc64. --- Found with the new fedora-ppc64le builder: https://builder.sourceware.org/buildbot/#/builders/gdb-fedora-ppc64le Alternatively we could just suppress this warning for any architecture. gdb/debuginfod-support.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c index f2a31ea1952..6dc08fc29b6 100644 --- a/gdb/debuginfod-support.c +++ b/gdb/debuginfod-support.c @@ -193,15 +193,15 @@ debuginfod_is_enabled () if (off == gdb::string_view::npos) break; url_view = url_view.substr (off); -#if defined (__s390x__) - /* g++ 11.2.1 on s390x seems convinced url_view might be of - SIZE_MAX length. And so complains because the length of - an array can only be PTRDIFF_MAX. */ +#if defined (__s390x__) || defined (__powerpc64__) + /* g++ 11.2.1 on s390x and g++ 11.3.1 on ppc64le seem convinced + url_view might be of SIZE_MAX length. And so complains + because the length of an array can only be PTRDIFF_MAX. */ DIAGNOSTIC_PUSH DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD #endif off = url_view.find_first_of (' '); -#if defined (__s390x__) +#if defined (__s390x__) || defined (__powerpc64__) DIAGNOSTIC_POP #endif gdb_printf -- 2.30.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] gdb: Workaround stringop-overread warning in debuginfod-support.c on powerpc64 2022-05-11 22:52 [PATCH] gdb: Workaround stringop-overread warning in debuginfod-support.c on powerpc64 Mark Wielaard @ 2022-05-12 0:49 ` Simon Marchi 2022-05-31 9:05 ` Mark Wielaard 0 siblings, 1 reply; 7+ messages in thread From: Simon Marchi @ 2022-05-12 0:49 UTC (permalink / raw) To: Mark Wielaard, gdb-patches On 2022-05-11 18:52, Mark Wielaard wrote: > Just like on s390x with g++ 11.2.1, ppc64le with g++ 11.3.1 produces a > spurious warning for stringop-overread in debuginfod_is_enabled > for url_view. Also suppress it on powerpc64. > > gdb/ChangeLog: > > * debuginfod-support.c (debuginfod_is_enabled): Use > DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD on powerpc64. LGTM. > --- > > Found with the new fedora-ppc64le builder: > https://builder.sourceware.org/buildbot/#/builders/gdb-fedora-ppc64le > > Alternatively we could just suppress this warning for any architecture. Let's make a deal, if you find a 3rd architecture where this is the case we do that, alright? ;) Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] gdb: Workaround stringop-overread warning in debuginfod-support.c on powerpc64 2022-05-12 0:49 ` Simon Marchi @ 2022-05-31 9:05 ` Mark Wielaard 2022-05-31 9:46 ` Pedro Alves 0 siblings, 1 reply; 7+ messages in thread From: Mark Wielaard @ 2022-05-31 9:05 UTC (permalink / raw) To: Simon Marchi, gdb-patches [-- Attachment #1: Type: text/plain, Size: 433 bytes --] Hi Simon, On Wed, 2022-05-11 at 20:49 -0400, Simon Marchi wrote: > On 2022-05-11 18:52, Mark Wielaard wrote: > > Alternatively we could just suppress this warning for any > > architecture. > > Let's make a deal, if you find a 3rd architecture where this is the > case we do that, alright? ;) Got another on hppa: https://sourceware.org/bugzilla/show_bug.cgi?id=29198 So, ok to push the attached? Thanks, Mark [-- Attachment #2: 0001-gdb-Always-suppress-stringop-overread-warning-in-deb.patch --] [-- Type: text/x-patch, Size: 1767 bytes --] From 0a1bd604d3037272d89c7c3e9ddc02e513e1027a Mon Sep 17 00:00:00 2001 From: Mark Wielaard <mark@klomp.org> Date: Tue, 31 May 2022 11:00:06 +0200 Subject: [PATCH] gdb: Always suppress stringop-overread warning in debuginfod-support.c Just like on s390x with g++ 11.2.1 and ppc64le with g++ 11.3.1 g++ 11 on hppa produces a spurious warning for stringop-overread in debuginfod_is_enabled for url_view. Just always suppress it on all arches. https://sourceware.org/bugzilla/show_bug.cgi?id=29198 gdb/ChangeLog: * debuginfod-support.c (debuginfod_is_enabled): Always use DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD. --- gdb/debuginfod-support.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c index 6dc08fc29b6..bd355b67352 100644 --- a/gdb/debuginfod-support.c +++ b/gdb/debuginfod-support.c @@ -193,17 +193,14 @@ debuginfod_is_enabled () if (off == gdb::string_view::npos) break; url_view = url_view.substr (off); -#if defined (__s390x__) || defined (__powerpc64__) - /* g++ 11.2.1 on s390x and g++ 11.3.1 on ppc64le seem convinced - url_view might be of SIZE_MAX length. And so complains - because the length of an array can only be PTRDIFF_MAX. */ + /* g++ 11.2.1 on s390x, g++q 11.3.1 on ppc64le and g++ 11 on + hppa seem convinced url_view might be of SIZE_MAX length. + And so complains because the length of an array can only + be PTRDIFF_MAX. */ DIAGNOSTIC_PUSH DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD -#endif off = url_view.find_first_of (' '); -#if defined (__s390x__) || defined (__powerpc64__) DIAGNOSTIC_POP -#endif gdb_printf (_(" <%ps>\n"), styled_string (file_name_style.style (), -- 2.18.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] gdb: Workaround stringop-overread warning in debuginfod-support.c on powerpc64 2022-05-31 9:05 ` Mark Wielaard @ 2022-05-31 9:46 ` Pedro Alves 2022-05-31 10:51 ` Mark Wielaard 0 siblings, 1 reply; 7+ messages in thread From: Pedro Alves @ 2022-05-31 9:46 UTC (permalink / raw) To: Mark Wielaard, Simon Marchi, gdb-patches On 2022-05-31 10:05, Mark Wielaard wrote: > + /* g++ 11.2.1 on s390x, g++q 11.3.1 on ppc64le and g++ 11 on > + hppa seem convinced url_view might be of SIZE_MAX length. > + And so complains because the length of an array can only > + be PTRDIFF_MAX. */ I wonder whether sticking in an assert so that the compiler knows the size of array, would make the warning go away: diff --git c/gdb/debuginfod-support.c w/gdb/debuginfod-support.c index 6dc08fc29b6..d6fab39eac8 100644 --- c/gdb/debuginfod-support.c +++ w/gdb/debuginfod-support.c @@ -187,6 +187,8 @@ debuginfod_is_enabled () "from the following URLs:\n")); gdb::string_view url_view (urls); + gdb_assert (url_view.size () < PTRDIFF_MAX); + while (true) { size_t off = url_view.find_first_not_of (' '); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] gdb: Workaround stringop-overread warning in debuginfod-support.c on powerpc64 2022-05-31 9:46 ` Pedro Alves @ 2022-05-31 10:51 ` Mark Wielaard 2022-06-06 11:27 ` Mark Wielaard 0 siblings, 1 reply; 7+ messages in thread From: Mark Wielaard @ 2022-05-31 10:51 UTC (permalink / raw) To: Pedro Alves, Simon Marchi, gdb-patches Hi Pedro, On Tue, 2022-05-31 at 10:46 +0100, Pedro Alves wrote: > On 2022-05-31 10:05, Mark Wielaard wrote: > > + /* g++ 11.2.1 on s390x, g++q 11.3.1 on ppc64le and g++ 11 on > > + hppa seem convinced url_view might be of SIZE_MAX length. > > + And so complains because the length of an array can only > > + be PTRDIFF_MAX. */ > > I wonder whether sticking in an assert so that the compiler knows the > size of array, would make the warning go away: If only we had the try buildbots running already, then we could more easily try (because one of them, fedora-s390x, does show this issue). I'll try (pun intended) to get that up by next week. > diff --git c/gdb/debuginfod-support.c w/gdb/debuginfod-support.c > index 6dc08fc29b6..d6fab39eac8 100644 > --- c/gdb/debuginfod-support.c > +++ w/gdb/debuginfod-support.c > @@ -187,6 +187,8 @@ debuginfod_is_enabled () > "from the following URLs:\n")); > > gdb::string_view url_view (urls); > + gdb_assert (url_view.size () < PTRDIFF_MAX); > + > while (true) > { > size_t off = url_view.find_first_not_of (' '); I tried by hand, but that does result in the same warning (with the DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD removed): CXX debuginfod-support.o 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:197: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)); | ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~ cc1plus: all warnings being treated as errors make[1]: *** [Makefile:1895: debuginfod-support.o] Error 1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] gdb: Workaround stringop-overread warning in debuginfod-support.c on powerpc64 2022-05-31 10:51 ` Mark Wielaard @ 2022-06-06 11:27 ` Mark Wielaard 2022-06-06 16:42 ` Pedro Alves 0 siblings, 1 reply; 7+ messages in thread From: Mark Wielaard @ 2022-06-06 11:27 UTC (permalink / raw) To: Pedro Alves, Simon Marchi, gdb-patches Hi, On Tue, 2022-05-31 at 12:51 +0200, Mark Wielaard wrote: > On Tue, 2022-05-31 at 10:46 +0100, Pedro Alves wrote: > > I wonder whether sticking in an assert so that the compiler > > knows the size of array, would make the warning go away: > > > diff --git c/gdb/debuginfod-support.c w/gdb/debuginfod-support.c > > index 6dc08fc29b6..d6fab39eac8 100644 > > --- c/gdb/debuginfod-support.c > > +++ w/gdb/debuginfod-support.c > > @@ -187,6 +187,8 @@ debuginfod_is_enabled () > > "from the following URLs:\n")); > > > > gdb::string_view url_view (urls); > > + gdb_assert (url_view.size () < PTRDIFF_MAX); > > + > > while (true) > > { > > size_t off = url_view.find_first_not_of (' '); > > I tried by hand, but that does result in the same warning (with the > DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD removed): So, since that didn't work, OK to push the original patch that simply suppresses the diagnostic always? Thanks, Mark ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] gdb: Workaround stringop-overread warning in debuginfod-support.c on powerpc64 2022-06-06 11:27 ` Mark Wielaard @ 2022-06-06 16:42 ` Pedro Alves 0 siblings, 0 replies; 7+ messages in thread From: Pedro Alves @ 2022-06-06 16:42 UTC (permalink / raw) To: Mark Wielaard, Simon Marchi, gdb-patches On 2022-06-06 12:27, Mark Wielaard wrote: > Hi, > > On Tue, 2022-05-31 at 12:51 +0200, Mark Wielaard wrote: >> On Tue, 2022-05-31 at 10:46 +0100, Pedro Alves wrote: >>> I wonder whether sticking in an assert so that the compiler >>> knows the size of array, would make the warning go away: >> >>> diff --git c/gdb/debuginfod-support.c w/gdb/debuginfod-support.c >>> index 6dc08fc29b6..d6fab39eac8 100644 >>> --- c/gdb/debuginfod-support.c >>> +++ w/gdb/debuginfod-support.c >>> @@ -187,6 +187,8 @@ debuginfod_is_enabled () >>> "from the following URLs:\n")); >>> >>> gdb::string_view url_view (urls); >>> + gdb_assert (url_view.size () < PTRDIFF_MAX); >>> + >>> while (true) >>> { >>> size_t off = url_view.find_first_not_of (' '); >> >> I tried by hand, but that does result in the same warning (with the >> DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD removed): > > So, since that didn't work, OK to push the original patch that simply > suppresses the diagnostic always? The code currently looks like: gdb::string_view url_view (urls); while (true) { size_t off = url_view.find_first_not_of (' '); if (off == gdb::string_view::npos) break; url_view = url_view.substr (off); #if defined (__s390x__) || defined (__powerpc64__) /* g++ 11.2.1 on s390x and g++ 11.3.1 on ppc64le seem convinced url_view might be of SIZE_MAX length. And so complains because the length of an array can only be PTRDIFF_MAX. */ DIAGNOSTIC_PUSH DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD #endif off = url_view.find_first_of (' '); #if defined (__s390x__) || defined (__powerpc64__) DIAGNOSTIC_POP #endif I suppose that the fact that the warning appears in the find_first_of call, and not the find_first_not_of call above was already an indication that the assert at that spot wouldn't work, otherwise, I'd think we'd be seeing a similar warning in the find_first_not_of call. I suspect that for the gdb_assert to work, it would need to be put at the same spot where the DIAGNOSTIC_PUSH is, after the url_view.substr. The theory being that substr returns a new string_view and it's that one that would need the assert. Like: gdb::string_view url_view (urls); while (true) { size_t off = url_view.find_first_not_of (' '); if (off == gdb::string_view::npos) break; url_view = url_view.substr (off); + gdb_assert (url_view.size () < PTRDIFF_MAX); off = url_view.find_first_of (' '); OTOH, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97185 suggests that the root issue is with not eliminating builtin calls when sizes are 0, so maybe the right thing to do would be to assert that the string_view isn't empty, like: diff --git c/gdb/debuginfod-support.c w/gdb/debuginfod-support.c index 6dc08fc29b6..9a4301f018c 100644 --- c/gdb/debuginfod-support.c +++ w/gdb/debuginfod-support.c @@ -193,17 +193,8 @@ debuginfod_is_enabled () if (off == gdb::string_view::npos) break; url_view = url_view.substr (off); -#if defined (__s390x__) || defined (__powerpc64__) - /* g++ 11.2.1 on s390x and g++ 11.3.1 on ppc64le seem convinced - url_view might be of SIZE_MAX length. And so complains - because the length of an array can only be PTRDIFF_MAX. */ - DIAGNOSTIC_PUSH - DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD -#endif + gdb_assert (!url_view.empty ()); off = url_view.find_first_of (' '); -#if defined (__s390x__) || defined (__powerpc64__) - DIAGNOSTIC_POP -#endif Anyhow, your patch is OK with me. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-06-06 16:42 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-05-11 22:52 [PATCH] gdb: Workaround stringop-overread warning in debuginfod-support.c on powerpc64 Mark Wielaard 2022-05-12 0:49 ` Simon Marchi 2022-05-31 9:05 ` Mark Wielaard 2022-05-31 9:46 ` Pedro Alves 2022-05-31 10:51 ` Mark Wielaard 2022-06-06 11:27 ` Mark Wielaard 2022-06-06 16:42 ` Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).