On Thu, Mar 30, 2023 at 1:40 PM Andrew MacLeod via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> I committed it. ran it again for fun. sigh. Looks like its also
> triggering another issue now in g++.dg/warn/Wstringop-overflow-4.C
> where its issuing:
libstdc++v3's 23_containers/vector/bool/allocator/copy.cc fails most
likely in a similar way.
In file included from
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/algorithm:60,^M
from
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu/bits/stdc++.h:51,^M
from <command-line>:^M
In static member function 'static _Up* std::__copy_move<_IsMove, true,
std::random_access_iterator_tag>::__copy_m(_Tp*, _Tp*, _Up*) [with _Tp
= long unsigned int; _Up = long unsigned int; bool _IsMove =
false]',^M
inlined from '_OI std::__copy_move_a2(_II, _II, _OI) [with bool
_IsMove = false; _II = long unsigned int*; _OI = long unsigned int*]'
at
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_algobase.h:506,^M
inlined from '_OI std::__copy_move_a1(_II, _II, _OI) [with bool
_IsMove = false; _II = long unsigned int*; _OI = long unsigned int*]'
at
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_algobase.h:533,^M
inlined from '_OI std::__copy_move_a(_II, _II, _OI) [with bool
_IsMove = false; _II = long unsigned int*; _OI = long unsigned int*]'
at
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_algobase.h:540,^M
inlined from '_OI std::copy(_II, _II, _OI) [with _II = long
unsigned int*; _OI = long unsigned int*]' at
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_algobase.h:633,^M
inlined from 'std::vector<bool, _Alloc>::iterator
std::vector<bool, _Alloc>::_M_copy_aligned(const_iterator,
const_iterator, iterator) [with _Alloc =
__gnu_test::propagating_allocator<bool, false>]' at
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_bvector.h:1303,^M
inlined from 'void std::vector<bool,
_Alloc>::_M_insert_aux(iterator, bool) [with _Alloc =
__gnu_test::propagating_allocator<bool, false>]' at
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/vector.tcc:945,^M
inlined from 'void std::vector<bool, _Alloc>::push_back(bool)
[with _Alloc = __gnu_test::propagating_allocator<bool, false>]' at
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_bvector.h:1121,^M
inlined from 'void test01()' at
/home/apinski/src/upstream-gcc/gcc/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/copy.cc:33:^M
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_algobase.h:437:
warning: 'void* __builtin_memmove(void*, const void*, long unsigned
int)' writing between 9 and 9223372036854775807 bytes into a region of
size 8 overflows the destination [-Wstringop-overflow=]^M
In file included from
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu/bits/c++allocator.h:33,^M
from
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/allocator.h:46,^M
from
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/string:43,^M
from
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bitset:52,^M
from
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu/bits/stdc++.h:52:^M
In member function '_Tp*
std::__new_allocator<_Tp>::allocate(size_type, const void*) [with _Tp
= long unsigned int]',^M
inlined from 'static _Tp*
std::allocator_traits<std::allocator<_CharT>
>::allocate(allocator_type&, size_type) [with _Tp = long unsigned
int]' at
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/alloc_traits.h:482,^M
inlined from '__gnu_test::uneq_allocator<Tp, Alloc>::pointer
__gnu_test::uneq_allocator<Tp, Alloc>::allocate(size_type, const
void*) [with Tp = long unsigned int; Alloc = std::allocator<long
unsigned int>]' at
/home/apinski/src/upstream-gcc/gcc/libstdc++-v3/testsuite/util/testsuite_allocator.h:360,^M
inlined from 'static std::allocator_traits<
<template-parameter-1-1> >::pointer std::allocator_traits<
<template-parameter-1-1> >::allocate(_Alloc&, size_type) [with _Alloc
= __gnu_test::propagating_allocator<long unsigned int, false,
std::allocator<long unsigned int> >]' at
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/alloc_traits.h:333,^M
inlined from 'std::_Bvector_base<_Alloc>::_Bit_pointer
std::_Bvector_base<_Alloc>::_M_allocate(std::size_t) [with _Alloc =
__gnu_test::propagating_allocator<bool, false>]' at
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_bvector.h:643,^M
inlined from 'void std::vector<bool,
_Alloc>::_M_insert_aux(iterator, bool) [with _Alloc =
__gnu_test::propagating_allocator<bool, false>]' at
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/vector.tcc:943,^M
inlined from 'void std::vector<bool, _Alloc>::push_back(bool)
[with _Alloc = __gnu_test::propagating_allocator<bool, false>]' at
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_bvector.h:1121,^M
inlined from 'void test01()' at
/home/apinski/src/upstream-gcc/gcc/libstdc++-v3/testsuite/23_containers/vector/bool/allocator/copy.cc:33:^M
/home/apinski/src/upstream-gcc/gcc/objdir/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/new_allocator.h:147:
note: destination object of size 8 allocated by 'operator new'^M
Thanks,
Andrew Pinski
Thanks,
Andrew Pinski
>
> b.C: In function ‘void test_strcpy_new_int16_t(size_t, const size_t*)’:
> b.C:76:12: warning: ‘void* __builtin_memcpy(void*, const void*, long
> unsigned int)’ writing 3 bytes into a region of size 0
> [-Wstringop-overflow=]
> 76 | strcpy (d, s); \
> | ~~~~~~~^~~~~~
> b.C:90:3: note: in expansion of macro ‘T’
> 90 | T (S (2), new int16_t[r_imin_imax + 1]); // { dg-bogus "into
> a region of size" "pr106120" { xfail { ilp32 && c++98_only } } }
>
>
> previously, by VRP2 time we hadn't figured out the edge case, couldn't
> recalculate _29 and iftmp.1_38 was unknown.
>
> iftmp.1_38 = _29 * 2;
> _40 = operator new [] (iftmp.1_38);
> __builtin_memcpy (_40, &MEM <const char[37]> [(void
> *)"0123456789abcdefghijklmnopqrstuvwxyz" + 34B], 3);
>
> Now, by VRP2 we have figured it out...
>
> _40 = operator new [] (0);
> __builtin_memcpy (_40, &MEM <const char[37]> [(void
> *)"0123456789abcdefghijklmnopqrstuvwxyz" + 34B], 3);
>
> And that info is available earlier for the warnings, just hasn't been
> explcitly exposed:
>
> Do we want to change the cfail to always? something like:
>
> diff --git a/gcc/testsuite/g++.dg/warn/Wstringop-overflow-4.C
> b/gcc/testsuite/g++.dg/warn/Wstringop-overflow-4.C
> index 35fb59e0232..faad5bed074 100644
> --- a/gcc/testsuite/g++.dg/warn/Wstringop-overflow-4.C
> +++ b/gcc/testsuite/g++.dg/warn/Wstringop-overflow-4.C
> @@ -141,7 +141,7 @@ void test_strcpy_new_int16_t (size_t n, const size_t
> vals[])
>
> int r_imin_imax = SR (INT_MIN, INT_MAX);
> T (S (1), new int16_t[r_imin_imax]);
> - T (S (2), new int16_t[r_imin_imax + 1]); // { dg-bogus "into a region
> of size" "pr106120" { xfail { ilp32 && c++98_only } } }
> + T (S (2), new int16_t[r_imin_imax + 1]); // { dg-bogus "into a region
> of size" "pr106120" { xfail { c++98_only } } }
> T (S (9), new int16_t[r_imin_imax * 2 + 1]);
>
> int r_0_imax = SR (0, INT_MAX);
>
>
> Of course, I dont know what this is doing on other arches... perhaps
> wait for the fallout to be complete?
>
> Andrew
>
> On 3/30/23 12:05, Jakub Jelinek wrote:
> > On Thu, Mar 30, 2023 at 11:58:19AM -0400, Andrew MacLeod wrote:
> >> On 3/30/23 09:41, Jakub Jelinek wrote:
> >>> On Wed, Mar 29, 2023 at 01:22:27PM -0400, Andrew MacLeod wrote:
> >>>> however, as seems to be the case often, better ranges result in, I now
> >>>> get:
> >>>>
> >>>> FAIL: 23_containers/vector/bool/allocator/copy.cc (test for excess
> >>>> errors)
> >>> Our middle-end warnings are just badly designed :(, the better value
> >>> ranges
> >>> are, the more false positives they have.
> >>>
> >>>> commit 358d0ca44faf2e20fbacd0f74386308b5ca52cd4
> >>>> Author: Andrew MacLeod <amacl...@redhat.com>
> >>>> Date: Tue Mar 28 12:16:34 2023 -0400
> >>>>
> >>>> Add recursive GORI recompuations with a depth limit.
> >>> LGTM for trunk, let's do with the regression incrementally.
> >>> Or as Richard mentioned on IRC, one possibility would be to force this
> >>> param temporarily to 1 (or whatever matches previous behavior) for the
> >>> diagnostic range queries).
> >>>
> >>> You need a ChangeLog entry though...
> >>>
> >> Attached. I also removed the bogus warning in Walloc-13.c that no longer
> >> happens
> >>
> >> So incrementally deal with it.. what? just let it fail?
> > For today? Yes.
> >
> > Ok for trunk.
> >
> >> commit debb8ce1f9b9d5a72d88d0ae90a6b4da5130ff59
> >> Author: Andrew MacLeod <amacl...@redhat.com>
> >> Date: Tue Mar 28 12:16:34 2023 -0400
> >>
> >> Add recursive GORI recompuations with a depth limit.
> >>
> >> PR tree-optimization/109154
> >> gcc/
> >> * gimple-range-gori.cc (gori_compute::may_recompute_p): Add
> >> depth limit.
> >> * gimple-range-gori.h (may_recompute_p): Add depth param.
> >> * params.opt (ranger-recompute-depth): New param.
> >>
> >> gcc/testsuite/
> >> * gcc.dg/Walloca-13.c: Remove bogus warning that is now fixed.
> > Jakub
> >
>
