[Bug c++/109717] New: -Warray-bound error with gnu++20 and fmt library

[Bug c++/109717] New: -Warray-bound error with gnu++20 and fmt library

[psmith at gnu dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109717

            Bug ID: 109717
           Summary: -Warray-bound error with gnu++20 and fmt library
           Product: gcc
           Version: 13.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: psmith at gnu dot org
  Target Milestone: ---

Created attachment 54983
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54983&action=edit
fmt1.i preprocessor output (compressed)

I found SO MANY issues related to -Warray-bound, many of them reported with GCC
11 or so.  I can't tell if this is a duplicate or not, although this issue
doesn't reproduce for me with GCC 11.3.

I have built my own GCC 13.1 from source on x86_64 GNU/Linux (as I've been
doing for >10 years) and it works great except for one thing.  When I use some
parts of the fmt 9.1.0 library, and "-O2 -Werror-builds -std=gnu++20" (removing
any one of those, or changing to -std=gnu++17, makes the error go away).

I try to compile this:

  #include <vector>
  #include <iterator>
  #include <fmt/format.h>

  void add_info(std::vector<char>& buf)
  {
      fmt::format_to(std::back_inserter(buf), "hello {}", "there");
  }

and I get this output:

$ g++-13.1 -I/data/src/build/common/fmt/include -std=gnu++20 -Warray-bounds -O2
-c -o /tmp/fmt1.o /tmp/fmt1.cpp

In file included from
/data/src/build/x86_64-linux/cc/unknown/x86_64-unknown-linux-gnu/include/c++/13.1.0/vector:62,
                 from /tmp/fmt1.cpp:1:
In static member function 'static constexpr _Up* std::__copy_move<_IsMove,
true, std::random_access_iterator_tag>::__copy_m(_Tp*, _Tp*, _Up*) [with _Tp =
unsigned int; _Up = unsigned int; bool _IsMove = false]',
    inlined from 'constexpr _OI std::__copy_move_a2(_II, _II, _OI) [with bool
_IsMove = false; _II = unsigned int*; _OI = unsigned int*]' at
/data/src/build/x86_64-linux/cc/unknown/x86_64-unknown-linux-gnu/include/c++/13.1.0/bits/stl_algobase.h:506:30,
    inlined from 'constexpr _OI std::__copy_move_a1(_II, _II, _OI) [with bool
_IsMove = false; _II = unsigned int*; _OI = unsigned int*]' at
/data/src/build/x86_64-linux/cc/unknown/x86_64-unknown-linux-gnu/include/c++/13.1.0/bits/stl_algobase.h:533:42,
    inlined from 'constexpr _OI std::__copy_move_a(_II, _II, _OI) [with bool
_IsMove = false; _II = unsigned int*; _OI = unsigned int*]' at
/data/src/build/x86_64-linux/cc/unknown/x86_64-unknown-linux-gnu/include/c++/13.1.0/bits/stl_algobase.h:540:31,
    inlined from 'constexpr _OI std::copy(_II, _II, _OI) [with _II = unsigned
int*; _OI = unsigned int*]' at
/data/src/build/x86_64-linux/cc/unknown/x86_64-unknown-linux-gnu/include/c++/13.1.0/bits/stl_algobase.h:633:7,
    inlined from 'static _ForwardIterator
std::__uninitialized_copy<true>::__uninit_copy(_InputIterator, _InputIterator,
_ForwardIterator) [with _InputIterator = unsigned int*; _ForwardIterator =
unsigned int*]' at
/data/src/build/x86_64-linux/cc/unknown/x86_64-unknown-linux-gnu/include/c++/13.1.0/bits/stl_uninitialized.h:147:27,
    inlined from '_ForwardIterator std::uninitialized_copy(_InputIterator,
_InputIterator, _ForwardIterator) [with _InputIterator = unsigned int*;
_ForwardIterator = unsigned int*]' at
/data/src/build/x86_64-linux/cc/unknown/x86_64-unknown-linux-gnu/include/c++/13.1.0/bits/stl_uninitialized.h:185:15,
    inlined from 'constexpr void fmt::v9::basic_memory_buffer<T, SIZE,
Allocator>::grow(size_t) [with T = unsigned int; long unsigned int SIZE = 32;
Allocator = std::allocator<unsigned int>]' at
/data/src/build/common/fmt/include/fmt/format.h:925:26,
    inlined from 'constexpr void
fmt::v9::detail::buffer<T>::try_reserve(size_t) [with T = unsigned int]' at
/data/src/build/common/fmt/include/fmt/core.h:928:39,
    inlined from 'constexpr void
fmt::v9::detail::buffer<T>::try_reserve(size_t) [with T = unsigned int]' at
/data/src/build/common/fmt/include/fmt/core.h:927:24,
    inlined from 'constexpr void fmt::v9::detail::buffer<T>::try_resize(size_t)
[with T = unsigned int]' at
/data/src/build/common/fmt/include/fmt/core.h:919:16,
    inlined from 'constexpr void fmt::v9::basic_memory_buffer<T, SIZE,
Allocator>::resize(size_t) [with T = unsigned int; long unsigned int SIZE = 32;
Allocator = std::allocator<unsigned int>]' at
/data/src/build/common/fmt/include/fmt/format.h:897:63,
    inlined from 'constexpr void fmt::v9::detail::bigint::assign(UInt) [with
UInt = long unsigned int; typename std::enable_if<(std::is_same<UInt, long
unsigned int>::value || std::is_same<UInt, __int128 unsigned>::value),
int>::type <anonymous> = 0]' at
/data/src/build/common/fmt/include/fmt/format.h:2792:19,
    inlined from 'constexpr void fmt::v9::detail::bigint::operator=(Int) [with
Int = int]' at /data/src/build/common/fmt/include/fmt/format.h:2813:11,
    inlined from 'constexpr void fmt::v9::detail::bigint::assign_pow10(int)' at
/data/src/build/common/fmt/include/fmt/format.h:2886:32:
/data/src/build/x86_64-linux/cc/unknown/x86_64-unknown-linux-gnu/include/c++/13.1.0/bits/stl_algobase.h:437:30:
warning: 'void* __builtin_memmove(void*, const void*, long unsigned int)'
forming offset 4 is out of the bounds [0, 4] [-Warray-bounds=]
  437 |             __builtin_memmove(__result, __first, sizeof(_Tp) * _Num);
      |             ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

It's very strange that it says "offset 4" is out of bounds for [0, 4] but maybe
I just don't understand the error.

The code in fmt/format.h line 2886 is:

  // Assigns pow(10, exp) to this bigint.
  FMT_CONSTEXPR20 void assign_pow10(int exp) {
    FMT_ASSERT(exp >= 0, "");
    if (exp == 0) return *this = 1;   //<== line 2886

If I don't use the fmt::format_to() function (just normal fmt::print etc.) then
it works fine.

Also I should point out that for some reason I can't reproduce this with
godbolt using GCC 13.1 and fmt 9.1.0.  I also can't reproduce it with my
previous build, GCC 11.3 (but all else the same).

I'll add the postprocessed output (where I still see the above error) as an
attachment.

[Bug c++/109717] -Warray-bound error with gnu++20 and fmt library

[psmith at gnu dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109717

--- Comment #1 from Paul Smith <psmith at gnu dot org> ---
This same test case also causes spurious errors for -Wstringop-overflow :(

If I use this compile line with the same source file, I get these errors:

$ g++-13.1 -I/data/src/build/common/fmt/include -std=gnu++20
-Wstringop-overflow -O2 -c -o /tmp/fmt1.o /tmp/fmt1.cpp

In file included from
/data/src/build/x86_64-linux/cc/unknown/x86_64-unknown-linux-gnu/include/c++/13.1.0/vector:62,
                 from /tmp/fmt1.cpp:1:
In static member function 'static constexpr _Up* std::__copy_move<_IsMove,
true, std::random_access_iterator_tag>::__copy_m(_Tp*, _Tp*, _Up*) [with _Tp =
unsigned int; _Up = unsigned int; bool _IsMove = false]',
    inlined from 'constexpr _OI std::__copy_move_a2(_II, _II, _OI) [with bool
_IsMove = false; _II = unsigned int*; _OI = unsigned int*]' at
/data/src/build/x86_64-linux/cc/unknown/x86_64-unknown-linux-gnu/include/c++/13.1.0/bits/stl_algobase.h:506:30,
    inlined from 'constexpr _OI std::__copy_move_a1(_II, _II, _OI) [with bool
_IsMove = false; _II = unsigned int*; _OI = unsigned int*]' at
/data/src/build/x86_64-linux/cc/unknown/x86_64-unknown-linux-gnu/include/c++/13.1.0/bits/stl_algobase.h:533:42,
    inlined from 'constexpr _OI std::__copy_move_a(_II, _II, _OI) [with bool
_IsMove = false; _II = unsigned int*; _OI = unsigned int*]' at
/data/src/build/x86_64-linux/cc/unknown/x86_64-unknown-linux-gnu/include/c++/13.1.0/bits/stl_algobase.h:540:31,
    inlined from 'constexpr _OI std::copy(_II, _II, _OI) [with _II = unsigned
int*; _OI = unsigned int*]' at
/data/src/build/x86_64-linux/cc/unknown/x86_64-unknown-linux-gnu/include/c++/13.1.0/bits/stl_algobase.h:633:7,
    inlined from 'static _ForwardIterator
std::__uninitialized_copy<true>::__uninit_copy(_InputIterator, _InputIterator,
_ForwardIterator) [with _InputIterator = unsigned int*; _ForwardIterator =
unsigned int*]' at
/data/src/build/x86_64-linux/cc/unknown/x86_64-unknown-linux-gnu/include/c++/13.1.0/bits/stl_uninitialized.h:147:27,
    inlined from '_ForwardIterator std::uninitialized_copy(_InputIterator,
_InputIterator, _ForwardIterator) [with _InputIterator = unsigned int*;
_ForwardIterator = unsigned int*]' at
/data/src/build/x86_64-linux/cc/unknown/x86_64-unknown-linux-gnu/include/c++/13.1.0/bits/stl_uninitialized.h:185:15,
    inlined from 'constexpr void fmt::v9::basic_memory_buffer<T, SIZE,
Allocator>::grow(size_t) [with T = unsigned int; long unsigned int SIZE = 32;
Allocator = std::allocator<unsigned int>]' at
/data/src/build/common/fmt/include/fmt/format.h:925:26,
    inlined from 'constexpr void
fmt::v9::detail::buffer<T>::try_reserve(size_t) [with T = unsigned int]' at
/data/src/build/common/fmt/include/fmt/core.h:928:39,
    inlined from 'constexpr void
fmt::v9::detail::buffer<T>::try_reserve(size_t) [with T = unsigned int]' at
/data/src/build/common/fmt/include/fmt/core.h:927:24,
    inlined from 'constexpr void fmt::v9::detail::buffer<T>::try_resize(size_t)
[with T = unsigned int]' at
/data/src/build/common/fmt/include/fmt/core.h:919:16,
    inlined from 'constexpr void fmt::v9::basic_memory_buffer<T, SIZE,
Allocator>::resize(size_t) [with T = unsigned int; long unsigned int SIZE = 32;
Allocator = std::allocator<unsigned int>]' at
/data/src/build/common/fmt/include/fmt/format.h:897:63,
    inlined from 'constexpr void fmt::v9::detail::bigint::assign(UInt) [with
UInt = long unsigned int; typename std::enable_if<(std::is_same<UInt, long
unsigned int>::value || std::is_same<UInt, __int128 unsigned>::value),
int>::type <anonymous> = 0]' at
/data/src/build/common/fmt/include/fmt/format.h:2792:19,
    inlined from 'constexpr void fmt::v9::detail::bigint::operator=(Int) [with
Int = int]' at /data/src/build/common/fmt/include/fmt/format.h:2813:11,
    inlined from 'constexpr void fmt::v9::detail::bigint::assign_pow10(int)' at
/data/src/build/common/fmt/include/fmt/format.h:2886:32:
/data/src/build/x86_64-linux/cc/unknown/x86_64-unknown-linux-gnu/include/c++/13.1.0/bits/stl_algobase.h:437:30:
warning: 'void* __builtin_memmove(void*, const void*, long unsigned int)'
writing between 5 and 9223372036854775807 bytes into a region of size 4
overflows the destination [-Wstringop-overflow=]
  437 |             __builtin_memmove(__result, __first, sizeof(_Tp) * _Num);
      |             ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Again that same test works fine with GCC 11.3.  It seems that overflow
detection in GCC 13.1 is really broken.

[Bug c++/109717] -Warray-bound error with gnu++20 and fmt library

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109717

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
It's not actually broken, it's just paranoid, and the overflow detection
triggers for dead code.

The optimizer analyzes different paths through the code separately, and fails
to detect that some of those paths are unreachable, and then warns that they
would overflow if those paths were taken.

[Bug tree-optimization/109717] -Warray-bound error with gnu++20 and fmt library

[psmith at gnu dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109717

--- Comment #3 from Paul Smith <psmith at gnu dot org> ---
OK, well, we don't have to say "broken"; all I know is that perfectly
respectable code that used to work without triggering these warnings in older
versions of GCC, and with older -std=c++..., is now failing in GCC 13.1 /
-std=c++20 widely enough that I must disable these warnings, which is
unfortunate.

[Bug tree-optimization/109717] -Warray-bound error with gnu++20 and fmt library

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109717

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Some of the warnings were always present, but suppressed unless you used
-Wsystem-headers

Now they're issued even when the "problem" is in a system header.

This is considered progress ;-)

[Bug libstdc++/109717] -Warray-bound error with gnu++20 and fmt library

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109717

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jwakely.gcc at gmail dot com,
                   |                            |rguenth at gcc dot gnu.org
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
          Component|tree-optimization           |libstdc++
   Last reconfirmed|                            |2023-05-04

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
So we see

<bb 6> [local count: 14986075]:
_60 = operator new (4);
_43 = MEM[(const struct buffer *)this_10(D)].size_;
_44 = _43 * 4;
_63 = (signed long) _44;
if (_63 > 4)
  goto <bb 7>; [90.00%]
else
  goto <bb 8>; [10.00%]

<bb 7> [local count: 12138721]:
__builtin_memmove (_60, pretmp_118, _44);
goto <bb 10>; [100.00%]

and if _63 > 4 then the memmove will write out of bounds of the allocated
storage.  The stringop-overflow diagnostic is about the same thing.

There's eventually a missed optimization that we do not discover this
path as not taken, but I do not see any write to size_ in the function.
There's

<bb 5> [local count: 272474101]:
*pretmp_118 = 1;
if (pretmp_154 == 0)
  goto <bb 6>; [50.00%]
else
  goto <bb 12>; [50.00%]

and

<bb 4> [local count: 148083751]:
pretmp_118 = MEM[(struct buffer *)this_10(D)].ptr_;
pretmp_154 = MEM[(struct buffer *)this_10(D)].capacity_;
if (exp_6(D) == 0)
  goto <bb 5>; [20.24%]
else
  goto <bb 13>; [79.76%]

but given capacity is zero on this path size_ is probably zero as well
(but that's not visible here).

An assertion in the library that size_ <= capacity_ would maybe help.

To sum up, there's nothing the diagnostic code can do here - it simply
doesn't have sufficient information to prune this diagnostic.

[Bug tree-optimization/109717] -Warray-bound error with gnu++20 and fmt library

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109717

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|libstdc++                   |tree-optimization

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
That code is in the {fmt} library, not libstdc++.

[Bug tree-optimization/109717] -Warray-bound error with gnu++20 and fmt library

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109717

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
And requiring every library to annotate every assumption causes regressions
like PR 109703, where the cure is worse than the disease.

A wrong-code bug is worse than a false positive warning, but the warnings are
forcing people to modify correct code.

[Bug tree-optimization/109717] -Warray-bound error with gnu++20 and fmt library

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109717

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dmalcolm at gcc dot gnu.org

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #7)
> And requiring every library to annotate every assumption causes regressions
> like PR 109703, where the cure is worse than the disease.
> 
> A wrong-code bug is worse than a false positive warning, but the warnings
> are forcing people to modify correct code.

I agree, but then I do not see how to reliably diagnose real problems.  There
is no way GCC can prove a line of code will be executed (the whole function
could be dead).  And given C++ abstraction and high branch density the
amount of IL not under conditional execution in a function is about zero.

Annotating the library also gives way to better optimization.

It might be an interesting experiment to use coverage data to prune
diagnostics on locations that are not covered.  All our late diagnostics
could be put into a separate -Wall.  That of course requires we can
somehow reliably get coverage of a program and also distinguish the
various copies of the same location passes like jump threading create ...

Might be also interesting to steer static analysis to those "interesting"
paths.

One requirement would probably be that we put all late diagnostics on
a common point in the pass pipeline, otherwise adding discriminators for
all "variants" of the IL we run into makes the coverage data explode
(much easier for the analyzer here).

[Bug tree-optimization/109717] -Warray-bound error with gnu++20 and fmt library

[psmith at gnu dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109717

--- Comment #9 from Paul Smith <psmith at gnu dot org> ---
> Now they're issued even when the "problem" is in a system header.

Oh interesting: I have been in the habit of including all my 3rdparty library
headers using -isystem to avoid worrying about warnings/errors in them.


Regarding this issue I understand the rock versus hard place situation.  If
there's an assert or something that can help, I can suggest it to the fmt
folks; they have been receptive to adding compiler-specific tricks in the past.
 If I can find the time I'll try to test this by editing my copy.  I still find
it strange that I can't reproduce this failure using the GDB 13.1 / fmt 9.1.0
libraries available on godbolt: it works fine there.

It would be great if this warning could be split into "this is a definite
problem" versus "this might be a problem", as some others have been, but I
understand that could be complex and a lot of work.