[Bug tree-optimization/109350] New: FAIL: g++.dg/warn/Wstringop-overflow-4.C

[Bug tree-optimization/109350] New: FAIL: g++.dg/warn/Wstringop-overflow-4.C

[rguenth at gcc dot gnu.org]

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

            Bug ID: 109350
           Summary: FAIL: g++.dg/warn/Wstringop-overflow-4.C
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rguenth at gcc dot gnu.org
  Target Milestone: ---

The

  T (S (2), new int16_t[r_imin_imax + 1]); // { dg-bogus "into a region of
size" "pr106120" { xfail { ilp32 && c++98_only } } }

subtest now FAILs.

Reduced testcase:

#include "../../gcc.dg/range.h"

#define INT_MAX     __INT_MAX__
#define INT_MIN     (-INT_MAX - 1)

extern "C" char* strcpy (char*, const char*);

void sink (void*);

#define S36 "0123456789abcdefghijklmnopqrstuvwxyz"
#define S(N) (S36 + sizeof S36 - N - 1)

#define T(src, alloc) do {                      \
    const char *s = src;                        \
    char *d = (char*)alloc;                     \
    strcpy (d, s);                              \
    sink (d);                                   \
  } while (0)


#ifdef __INT16_TYPE__

// Hack around PR 92829.
#define XUR(min, max) \
  (++idx, (vals[idx] < min || max < vals[idx] ? min : vals[idx]))

typedef __INT16_TYPE__ int16_t;

void test_strcpy_new_int16_t (size_t n, const size_t vals[])
{
  size_t idx = 0;

  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 (9), new int16_t[r_imin_imax * 2 + 1]);

}

[Bug tree-optimization/109350] FAIL: g++.dg/warn/Wstringop-overflow-4.C

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
             Blocks|                            |88443

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
We have

<bb 2> [local count: 1073741824]:
_28 ={v} signed_value_source;
_4 = (unsigned long) _28;
_9 = _4 + 2147483648;
if (_9 > 4294967295)
  goto <bb 8>; [50.00%]
else
  goto <bb 3>; [50.00%]

<bb 3> [local count: 536870913]:
r_imin_imax_8 = (int) _28;
_31 = r_imin_imax_8 + 1;
_29 = (sizetype) _31;
if (_4 <= 4611686018427387900)
  goto <bb 4>; [50.00%]
else
  goto <bb 5>; [50.00%]

<bb 5> [local count: 268435458]:
_13 = operator new [] (18446744073709551615);
__builtin_memcpy (_13, &MEM <const char[37]> [(void
*)"0123456789abcdefghijklmnopqrstuvwxyz" + 35B], 2);
sink (_13);
if (_29 <= 4611686018427387900)
  goto <bb 9>; [100.00%]
else
  goto <bb 7>; [0.00%]

<bb 9> [local count: 0]:
iftmp.1_38 = _29 * 2;
_40 = operator new [] (iftmp.1_38);
__builtin_memcpy (_40, &MEM <const char[37]> [(void
*)"0123456789abcdefghijklmnopqrstuvwxyz" + 34B], 3);

and we're again down into get_size_range of the operator new[] argument
iftmp.1_38:

pointer-query.cc:507

    /* Determine the largest valid range size, including zero.  */
    if (!get_size_range (qry, size, stmt, r, SR_ALLOW_ZERO | SR_USE_LARGEST))
      return NULL_TREE;

where we compute

[irange] long unsigned int [0, 0] NONZERO 0x0$27 = void

for the range.  I have to make up my mind if that's correct, but ranger
debug prints the same conclusion so it doesn't seem to be an artifact of
using legacy value_range.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88443
[Bug 88443] [meta-bug] bogus/missing -Wstringop-overflow warnings

[Bug tree-optimization/109350] FAIL: g++.dg/warn/Wstringop-overflow-4.C

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amacleod at redhat dot com

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Hmm, the same reproduces with r_imin_imax as ptrdiff_t but the IL is a bit
more obvious:

<bb 2> [local count: 1073741824]:
_27 ={v} signed_value_source;
_4 = (unsigned long) _27;
_8 = _4 + 2147483648;
if (_8 > 4294967295)
  goto <bb 8>; [50.00%]
else
  goto <bb 3>; [50.00%]

<bb 3> [local count: 536870913]:
_30 = _27 + 1;
_28 = (sizetype) _30;
if (_4 <= 4611686018427387900)
  goto <bb 4>; [50.00%]
else
  goto <bb 5>; [50.00%]

<bb 5> [local count: 268435458]:
_12 = operator new [] (18446744073709551615);
__builtin_memcpy (_12, &MEM <const char[37]> [(void
*)"0123456789abcdefghijklmnopqrstuvwxyz" + 35B], 2);
sink (_12);
if (_28 <= 4611686018427387900)
  goto <bb 9>; [100.00%]
else
  goto <bb 7>; [0.00%]

<bb 9> [local count: 0]:
iftmp.2_37 = _28 * 2;
_39 = operator new [] (iftmp.2_37);
__builtin_memcpy (_39, &MEM <const char[37]> [(void
*)"0123456789abcdefghijklmnopqrstuvwxyz" + 34B], 3);

so we have (unsigned long)[int_min, int_max] > 4611686018427387900
&& (unsigned long)[int_min+1, int_max+1] <= 4611686018427387900 to
constrain _4.  I don't see how we can arrive at [0,0] for iftmp.2_37.

In fact if I put this into a separate testcase like

void __attribute__((noipa))
foo (long signed_value_source)
{
  unsigned long temu = signed_value_source;
  if (temu + 2147483648 > 4294967295)
    ;
  else
    {
     long tems = signed_value_source + 1;
     unsigned long temu2 = tems;
     if (temu > 4611686018427387900)
       if (temu2 <= 4611686018427387900)
         {
           unsigned long iftmp = temu2 * 2;
           if (iftmp == 0)
             __builtin_abort ();
         }
    }
}

then we optimize this to

  <bb 2> [local count: 1073741824]:
  temu_3 = (long unsigned int) signed_value_source_2(D);
  _1 = temu_3 + 2147483648;
  if (_1 > 4294967295)
    goto <bb 5>; [50.00%]
  else
    goto <bb 3>; [50.00%]

  <bb 3> [local count: 536870913]:
  if (signed_value_source_2(D) == -1)
    goto <bb 4>; [0.00%]
  else
    goto <bb 5>; [100.00%]

  <bb 4> [count: 0]:
  __builtin_abort ();

and the outer if doesn't change the inner range result.

[Bug tree-optimization/109350] FAIL: g++.dg/warn/Wstringop-overflow-4.C

[amacleod at redhat dot com]

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

--- Comment #3 from Andrew Macleod <amacleod at redhat dot com> ---
On 3/31/23 03:17, rguenth at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109350
>
> Richard Biener <rguenth at gcc dot gnu.org> changed:
>
>             What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                   CC|                            |amacleod at redhat dot com
>
> --- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
> Hmm, the same reproduces with r_imin_imax as ptrdiff_t but the IL is a bit
> more obvious:
>
> <bb 2> [local count: 1073741824]:
> _27 ={v} signed_value_source;
> _4 = (unsigned long) _27;
> _8 = _4 + 2147483648;
> if (_8 > 4294967295)
>    goto <bb 8>; [50.00%]
> else
>    goto <bb 3>; [50.00%]
>
> <bb 3> [local count: 536870913]:
> _30 = _27 + 1;
> _28 = (sizetype) _30;
> if (_4 <= 4611686018427387900)
>    goto <bb 4>; [50.00%]
> else
>    goto <bb 5>; [50.00%]
>
> <bb 5> [local count: 268435458]:
> _12 = operator new [] (18446744073709551615);
> __builtin_memcpy (_12, &MEM <const char[37]> [(void
> *)"0123456789abcdefghijklmnopqrstuvwxyz" + 35B], 2);
> sink (_12);
> if (_28 <= 4611686018427387900)
>    goto <bb 9>; [100.00%]
> else
>    goto <bb 7>; [0.00%]
>
> <bb 9> [local count: 0]:
> iftmp.2_37 = _28 * 2;
> _39 = operator new [] (iftmp.2_37);
> __builtin_memcpy (_39, &MEM <const char[37]> [(void
> *)"0123456789abcdefghijklmnopqrstuvwxyz" + 34B], 3);
>
> so we have (unsigned long)[int_min, int_max] > 4611686018427387900
> && (unsigned long)[int_min+1, int_max+1] <= 4611686018427387900 to
> constrain _4.  I don't see how we can arrive at [0,0] for iftmp.2_37.


Looking at what ranger produces for vrp2 (same code just a few passes 
later):

=========== BB 2 ============
Imports: _28
Exports: _4  _10  _28
          _4 : _28(I)
          _10 : _4  _28(I)
Partial equiv (_4 pe64 _28)
Relational : (_10 != _4)
     <bb 2> [local count: 1073741824]:
     _28 ={v} signed_value_source;
     _4 = (unsigned long) _28;
     _10 = _4 + 2147483648;
     if (_10 > 4294967295)
       goto <bb 8>; [50.00%]
     else
       goto <bb 3>; [50.00%]

2->3  (F) _4 :  [irange] unsigned long [0, 
2147483647][18446744071562067968, +INF]
2->3  (F) _10 :         [irange] unsigned long [0, 4294967295] NONZERO 
0xffffffff
2->3  (F) _28 :         [irange] long int [-2147483648, 2147483647]

on entry top BB3 , _28 has the full range of a signed int in a long int 
body.


=========== BB 3 ============

_4      [irange] unsigned long [0, 2147483647][18446744071562067968, +INF]
_28     [irange] long int [-2147483648, 2147483647]
Partial equiv (r_imin_imax_8 pe32 _28)
Relational : (_31 > r_imin_imax_8)
     <bb 3> [local count: 536870913]:
     r_imin_imax_8 = (int) _28;                           << THIs is 
varying, which is why it isnt printed anywhere
     _31 = r_imin_imax_8 + 1;      <<   signed traps on overflowt,  so 
this would be [min+1, +INF]
     _29 = (sizetype) _31;          <<  sizetype is a larger unsigned 
object,so the possible values for it are [0, 
2147483647][18446744071562067969, +INF]
     if (_4 <= 4611686018427387900)       << this leaves _4 with 
possible values of [18446744071562067968, +INF] on the FALSE branch.
       goto <bb 4>; [50.00%]
     else
       goto <bb 5>; [50.00%]


_29 : [irange] sizetype [0, 2147483647][18446744071562067969, +INF]
_31 : [irange] int [-2147483647, +INF]

When we recalculate values based on the range of _4 on the false 
branch,  intersected with their knowns ranges, it comes up with this


3->5  (F) _4 :  [irange] unsigned long [18446744071562067968, +INF]
3->5  (F) r_imin_imax_8 :       [irange] int [-INF, -1]
3->5  (F) _28 :         [irange] long int [-2147483648, -1]
3->5  (F) _29 :         [irange] sizetype [0, 0][18446744071562067969, +INF]
3->5  (F) _31 :         [irange] int [-2147483647, 0]

when we feed that value of _4 into

[18446744071562067968, +INF] = (unsigned long)28

in BB2, we discover the only possible valiues of _28 are [-2147483648, 
-1] on this branch.
We now go an recalculate r_imin_imax_8, _31 and _29 based on this new 
value of _28 and come up with those ranges

that means when we get to bb5, and see
     if (_29 <= 4611686018427387900)
       goto <bb 9>; [100.00%]

the only possible value of _29 on this branch is a [0,0]   And thats a 
direct result of _31 = [-INF, -1] + 1  before _20 is created with the cast.

yikes.  talk about convoluted...


>
> In fact if I put this into a separate testcase like
>
> void __attribute__((noipa))
> foo (long signed_value_source)
> {
>    unsigned long temu = signed_value_source;
>    if (temu + 2147483648 > 4294967295)
>      ;
>    else
>      {
>       long tems = signed_value_source + 1;
>       unsigned long temu2 = tems;
>       if (temu > 4611686018427387900)
>         if (temu2 <= 4611686018427387900)
>           {
>             unsigned long iftmp = temu2 * 2;
>             if (iftmp == 0)
>               __builtin_abort ();
>           }
>      }
> }
>
> then we optimize this to
>
>    <bb 2> [local count: 1073741824]:
>    temu_3 = (long unsigned int) signed_value_source_2(D);
>    _1 = temu_3 + 2147483648;
>    if (_1 > 4294967295)
>      goto <bb 5>; [50.00%]
>    else
>      goto <bb 3>; [50.00%]
>
>    <bb 3> [local count: 536870913]:
>    if (signed_value_source_2(D) == -1)
>      goto <bb 4>; [0.00%]
>    else
>      goto <bb 5>; [100.00%]
>
>    <bb 4> [count: 0]:
>    __builtin_abort ();
>
> and the outer if doesn't change the inner range result.
>
I bet if we used temu_3 at the abort point it would.  I changed it to 
bar (temu) from the abort:

            if (iftmp == 0)
              bar (temu);

 From EVRP:

   temu_5 = (long unsigned int) signed_value_source_4(D);
   _1 = temu_5 + 2147483648;
   if (_1 > 4294967295)
     goto <bb 6>; [INV]
   else
     goto <bb 3>; [INV]

   <bb 3> :
   tems_7 = signed_value_source_4(D) + 1;
   temu2_8 = (long unsigned int) tems_7;
   if (temu_5 > 4611686018427387900)
     goto <bb 4>; [INV]
   else
     goto <bb 6>; [INV]

   <bb 4> :
   if (temu2_8 <= 4611686018427387900)
     goto <bb 5>; [INV]
   else
     goto <bb 6>; [INV]

   <bb 5> :
   bar (-1);


Andrew

[Bug tree-optimization/109350] FAIL: g++.dg/warn/Wstringop-overflow-4.C

[amacleod at redhat dot com]

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

--- Comment #4 from Andrew Macleod <amacleod at redhat dot com> ---
Perhaps its clearer (HA!) if I turn the IL into a C program:
This is what the code sequence we are seeing effectively does:

int need_beer(int value);
int need_big_beer(unsigned long value);

int beer(int value)
{
  unsigned long x = value;
  unsigned long y = x + 0x80000000;
  if (y < 0xFFFFFFFF)
    {
      int r = x;
      int t = r + 1;
      unsigned long z = t;
      if (x >  0x3FFFFFFFFFFFFFFC)
        {
          need_beer (r);
          if (z <= 0x3FFFFFFFFFFFFFFC)
            need_big_beer (z);
        }
    }
}


and with relevant outgoing edges:
    <bb 2> :
    x_3 = (long unsigned int) value_2(D);
    y_4 = x_3 + 2147483648;
    if (y_4 != 4294967295)
      goto <bb 3>; [INV]
    else
      goto <bb 6>; [INV]

x_3 : [irange] long unsigned int [0, 2147483647][18446744071562067968, +INF]
y_4 : [irange] long unsigned int [0, 4294967295] NONZERO 0xffffffff
2->3  (T) value_2(D) :  [irange] int [-INF, 2147483646]
2->3  (T) x_3 :         [irange] long unsigned int [0,
2147483646][18446744071562067968, +INF]
2->3  (T) y_4 :         [irange] long unsigned int [0, 4294967294] NONZERO
0xffffffff

    <bb 3> :
    t_7 = value_2(D) + 1;
    z_8 = (long unsigned int) t_7;
    if (x_3 > 4611686018427387900)
      goto <bb 4>; [INV]
    else
      goto <bb 6>; [INV]

t_7 : [irange] int [-2147483647, +INF]
z_8 : [irange] long unsigned int [0, 2147483647][18446744071562067969, +INF]
3->4  (T) value_2(D) :  [irange] int [-INF, -1]
3->4  (T) x_3 :         [irange] long unsigned int [18446744071562067968, +INF]
3->4  (T) t_7 :         [irange] int [-2147483647, 0]
3->4  (T) z_8 :         [irange] long unsigned int [0, 0][18446744071562067969,
+INF]

    <bb 4> :
    need_beer (value_2(D));
    if (z_8 <= 4611686018427387900)
      goto <bb 5>; [INV]
    else
      goto <bb 6>; [INV]

4->5  (T) value_2(D) :  [irange] int [-1, -1]
4->5  (T) z_8 :         [irange] long unsigned int [0, 0] NONZERO 0x0

    <bb 5> :
    need_big_beer (0);