Bug 106093 - [12/13/14 Regression] False positive -Wstringop-overflow with -O3 when resizing std::vector
Summary: [12/13/14 Regression] False positive -Wstringop-overflow with -O3 when resizi...
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 12.1.0
: P2 normal
Target Milestone: 12.4
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks: Wstringop-overflow
  Show dependency treegraph
 
Reported: 2022-06-26 21:31 UTC by Johannes Altmanninger
Modified: 2023-05-14 18:54 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work: 11.3.0
Known to fail:
Last reconfirmed: 2023-01-16 00:00:00


Attachments
preprocessed reproducer (69.26 KB, text/plain)
2022-06-26 21:31 UTC, Johannes Altmanninger
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Johannes Altmanninger 2022-06-26 21:31:23 UTC
Created attachment 53202 [details]
preprocessed reproducer

Very similar to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83239 and others. This one reproduces on 12.1.0 but not on 11.2.0.

cat > repro.cpp << EOF
#include <vector>

template <typename T> struct Allocator {
  using value_type = T;

  Allocator() = default;

  T *allocate(unsigned long n) {
    return reinterpret_cast<T *>(::operator new(sizeof(T) * n));
  }

  void deallocate(T *ptr, unsigned long n) { ::operator delete(ptr); }
};

static std::vector<char, Allocator<char>> m_stream{};
void read_available() {
    m_stream.resize(1);
}
EOF

attached the preprocessed output

$ g++ -std=c++2a repro.cpp -S -O3

In function ‘constexpr decltype (::new(void*(0)) _Tp) std::construct_at(_Tp*, _Args&& ...) [with _Tp = char; _Args = {char}]’,
    inlined from ‘static constexpr std::_Require<std::__and_<std::__not_<typename std::allocator_traits< <template-parameter-1-1> >::__construct_helper<_Tp, _Args>::type>, std::is_constructible<_Tp, _Args ...> > > std::allocator_traits< <template-parameter-1-1> >::_S_construct(_Alloc&, _Tp*, _Args&& ...) [with _Tp = char; _Args = {char}; _Alloc = Allocator<char>]’ at /usr/include/c++/12.1.0/bits/alloc_traits.h:263:21,
    inlined from ‘static constexpr decltype (std::allocator_traits< <template-parameter-1-1> >::_S_construct(__a, __p, (forward<_Args>)(std::allocator_traits< <template-parameter-1-1> >::construct::__args)...)) std::allocator_traits< <template-parameter-1-1> >::construct(_Alloc&, _Tp*, _Args&& ...) [with _Tp = char; _Args = {char}; _Alloc = Allocator<char>]’ at /usr/include/c++/12.1.0/bits/alloc_traits.h:364:16,
    inlined from ‘constexpr void std::__relocate_object_a(_Tp*, _Up*, _Allocator&) [with _Tp = char; _Up = char; _Allocator = Allocator<char>]’ at /usr/include/c++/12.1.0/bits/stl_uninitialized.h:1064:26,
    inlined from ‘constexpr _ForwardIterator std::__relocate_a_1(_InputIterator, _InputIterator, _ForwardIterator, _Allocator&) [with _InputIterator = char*; _ForwardIterator = char*; _Allocator = Allocator<char>]’ at /usr/include/c++/12.1.0/bits/stl_uninitialized.h:1092:26,
    inlined from ‘constexpr _ForwardIterator std::__relocate_a(_InputIterator, _InputIterator, _ForwardIterator, _Allocator&) [with _InputIterator = char*; _ForwardIterator = char*; _Allocator = Allocator<char>]’ at /usr/include/c++/12.1.0/bits/stl_uninitialized.h:1133:33,
    inlined from ‘static constexpr std::vector<_Tp, _Alloc>::pointer std::vector<_Tp, _Alloc>::_S_relocate(pointer, pointer, pointer, _Tp_alloc_type&) [with _Tp = char; _Alloc = Allocator<char>]’ at /usr/include/c++/12.1.0/bits/stl_vector.h:504:26,
    inlined from ‘constexpr void std::vector<_Tp, _Alloc>::_M_default_append(size_type) [with _Tp = char; _Alloc = Allocator<char>]’ at /usr/include/c++/12.1.0/bits/vector.tcc:663:16,
    inlined from ‘constexpr void std::vector<_Tp, _Alloc>::resize(size_type) [with _Tp = char; _Alloc = Allocator<char>]’ at /usr/include/c++/12.1.0/bits/stl_vector.h:1011:21,
    inlined from ‘void read_available()’ at t/repro/repro.cc:17:20:
/usr/include/c++/12.1.0/bits/stl_construct.h:97:14: warning: writing 8 bytes into a region of size 1 [-Wstringop-overflow=]
   97 |     { return ::new((void*)__location) _Tp(std::forward<_Args>(__args)...); }
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Comment 1 Richard Biener 2023-01-16 10:53:26 UTC
Confirmed.

(gdb) p debug_gimple_stmt (stmt)
# .MEM_120 = VDEF <.MEM_81>
MEM <vector(8) char> [(char *)vectp.79_117] = vect__18.77_116;

it's possibly a missed optimization for the vectorizer introduced compute
of the number of iterations.  We have

<bb 7> [local count: 58465242]:
_23 = operator new (1);
*_23 = 0;
__cur_29 = _23 + 1;
if (_3 != _4)
  goto <bb 8>; [89.00%]

<bb 8> [local count: 52034065]:
_15 = (unsigned long) _3;
_38 = (unsigned long) _4;
_36 = _15 - _38;
_54 = _36 + 18446744073709551615;
_10 = _54 > 6;
if (_10 != 0)
  goto <bb 9>; [64.00%]

so we allocate 1 byte but then compute the iteration as difference from _4 and _3 which are computed from

_3 = m_stream.D.31893._M_impl.D.31166._M_finish;
_4 = m_stream.D.31893._M_impl.D.31166._M_start;

in particular the new allocated storage is processed but the old size is used?!
Comment 2 Jonathan Wakely 2023-01-16 13:14:23 UTC
(In reply to Richard Biener from comment #1)
> in particular the new allocated storage is processed but the old size is
> used?!

Yes, that seems correct.

We're resizing the vector from N to N+1 (where N happens to be 0). The new size is greater than the existing capacity, so we allocate N+1 elements, then copy the existing N elements into the new storage (which uses old_finish - old_start).

Afterwards, we would actually construct the new element in the new storage, but the warning happens while just copying the elements from the old storage to the new.
Comment 3 Jonathan Wakely 2023-01-16 13:17:34 UTC
(In reply to Jonathan Wakely from comment #2)
> Afterwards, we would actually construct the new element in the new storage,
> but the warning happens while just copying the elements from the old storage
> to the new.

Actually we already created the new element first (vector.tcc:668) and then we copy the old elements (vector.tcc:676).

Either way, it's correct that the loop copying the old elements uses the old size.
Comment 4 Richard Biener 2023-05-08 12:24:54 UTC
GCC 12.3 is being released, retargeting bugs to GCC 12.4.