[Bug tree-optimization/97185] New: inconsistent builtin elimination for impossible range

[Bug tree-optimization/97185] New: inconsistent builtin elimination for impossible range

[msebor at gcc dot gnu.org]

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

            Bug ID: 97185
           Summary: inconsistent builtin elimination for impossible range
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: msebor at gcc dot gnu.org
  Target Milestone: ---

In the test case below GCC eliminates the memcpy and memmove calls because the
only valid bound they are called with is zero.  But the same optimization isn't
done for the any of the remaining calls, including memset, resulting in
warnings noting that the range is (most likely) invalid.  GCC should be
consistent and either eliminate all the calls (preferably), or none of them.

$ cat z.c && gcc -O2 -S -fdump-tree-optimized=/dev/stdout z.c
void f0 (void *p, const void *q, int n)
{ 
  if (n > 0) return;
  __builtin_memcpy (p, q, n);
}

void f1 (void *p, const void *q, int n)
{
  if (n > 0) return;
  __builtin_memmove (p, q, n);
}

void f2 (char *p, const char *q, int n)
{
  if (n > 0) return;
  __builtin_strncpy (p, q, n);
}

void f3 (void *p, int n)
{ 
  if (n > 0) return;
  __builtin_memset (p, 0, n);
}

void* f4 (const void *p, int n)
{
  if (n > 0) return 0;
  return __builtin_memchr (p, 0, n);
}

int f5 (const void *p, const void *q, int n)
{
  if (n > 0) return 0;
  return __builtin_memcmp (p, q, n);
}


;; Function f0 (f0, funcdef_no=0, decl_uid=1933, cgraph_uid=1, symbol_order=0)

f0 (void * p, const void * q, int n)
{
  <bb 2> [local count: 1073741824]:
  return;

}



;; Function f1 (f1, funcdef_no=1, decl_uid=1938, cgraph_uid=2, symbol_order=1)

f1 (void * p, const void * q, int n)
{
  <bb 2> [local count: 1073741824]:
  return;

}



;; Function f2 (f2, funcdef_no=2, decl_uid=1943, cgraph_uid=3, symbol_order=2)

Removing basic block 5
f2 (char * p, const char * q, int n)
{
  long unsigned int _1;

  <bb 2> [local count: 1073741824]:
  if (n_3(D) > 0)
    goto <bb 4>; [60.08%]
  else
    goto <bb 3>; [39.92%]

  <bb 3> [local count: 428637736]:
  _1 = (long unsigned int) n_3(D);
  __builtin_strncpy (p_5(D), q_6(D), _1); [tail call]

  <bb 4> [local count: 1073741824]:
  return;

}


z.c: In function ‘f2’:
z.c:16:3: warning: ‘__builtin_strncpy’ specified size between
18446744071562067968 and 0 exceeds maximum object size 9223372036854775807
[-Wstringop-overflow=]
   16 |   __builtin_strncpy (p, q, n);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~

;; Function f3 (f3, funcdef_no=3, decl_uid=1947, cgraph_uid=4, symbol_order=3)

Removing basic block 5
f3 (void * p, int n)
{
  long unsigned int _1;

  <bb 2> [local count: 1073741824]:
  if (n_3(D) > 0)
    goto <bb 4>; [60.08%]
  else
    goto <bb 3>; [39.92%]

  <bb 3> [local count: 428637736]:
  _1 = (long unsigned int) n_3(D);
  __builtin_memset (p_5(D), 0, _1); [tail call]

  <bb 4> [local count: 1073741824]:
  return;

}


z.c: In function ‘f3’:
z.c:22:3: warning: ‘__builtin_memset’ specified size between
18446744071562067968 and 18446744073709551615 exceeds maximum object size
9223372036854775807 [-Wstringop-overflow=]
   22 |   __builtin_memset (p, 0, n);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~

;; Function f4 (f4, funcdef_no=4, decl_uid=1951, cgraph_uid=5, symbol_order=4)

Removing basic block 5
f4 (const void * p, int n)
{
  long unsigned int _1;
  void * _2;
  void * _6;

  <bb 2> [local count: 1073741824]:
  if (n_3(D) > 0)
    goto <bb 4>; [23.24%]
  else
    goto <bb 3>; [76.76%]

  <bb 3> [local count: 824204225]:
  _1 = (long unsigned int) n_3(D);
  _6 = __builtin_memchr (p_5(D), 0, _1); [tail call]

  <bb 4> [local count: 1073741824]:
  # _2 = PHI <_6(3), 0B(2)>
  return _2;

}


z.c: In function ‘f4’:
z.c:28:10: warning: ‘__builtin_memchr’ specified bound [18446744071562067968,
0] exceeds maximum object size 9223372036854775807 [-Wstringop-overread]
   28 |   return __builtin_memchr (p, 0, n);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~

;; Function f5 (f5, funcdef_no=5, decl_uid=1956, cgraph_uid=6, symbol_order=5)

Removing basic block 5
f5 (const void * p, const void * q, int n)
{
  long unsigned int _1;
  int _2;
  int _7;

  <bb 2> [local count: 1073741824]:
  if (n_3(D) > 0)
    goto <bb 4>; [42.57%]
  else
    goto <bb 3>; [57.43%]

  <bb 3> [local count: 616649929]:
  _1 = (long unsigned int) n_3(D);
  _7 = __builtin_memcmp (p_5(D), q_6(D), _1); [tail call]

  <bb 4> [local count: 1073741824]:
  # _2 = PHI <_7(3), 0(2)>
  return _2;

}


z.c: In function ‘f5’:
z.c:34:10: warning: ‘__builtin_memcmp’ specified bound [18446744071562067968,
0] exceeds maximum object size 9223372036854775807 [-Wstringop-overread]
   34 |   return __builtin_memcmp (p, q, n);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~

[Bug tree-optimization/97185] inconsistent builtin elimination for impossible range

[siddhesh at gcc dot gnu.org]

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

--- Comment #1 from Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> ---
While the missed optimization ought to be fixed, what's the value of
-Wstringop-* warning on an impossible range, i.e. when low > high?  Shouldn't
it just bail out silently if it detects an impossible range?

[Bug tree-optimization/97185] inconsistent builtin elimination for impossible range

[msebor at gcc dot gnu.org]

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

--- Comment #2 from Martin Sebor <msebor at gcc dot gnu.org> ---
There's a heuristic for ranges of allocation sizes to exclude zero
(size_range_flags) that comes into play here.  The actual range isn't
"impossible" in the sense it's necessarily invalid.  It just means the string
function call is either a no-op or out of bounds, and so can be eliminated as
an optimization.  With the optimization consistently implemented the warning
will also go away (eliminating the calls will prevent sanitizers from detecting
the out of bounds ones, so that might be a consideration).

In general, a low > high range denoted an anti-range before Ranger was
introduced (i.e., ~[high, low]).  With Ranger it's the corresponding union of
two ranges.  Some of the cruft for dealing with anti-ranges is still around,
such as in get_size_range() in pointer-query.cc.  The code should be migrated
to the irange class and the representation probably also updated to print
something more sensible (e.g., the union [MIN, high) U (low, MAX]; we talked
about introducing a pretty-printer % directive for ranges to make the format
consistent across all diagnostics).

[Bug tree-optimization/97185] inconsistent builtin elimination for impossible range

[siddhesh at gcc dot gnu.org]

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

--- Comment #3 from Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> ---
(In reply to Martin Sebor from comment #2)
> There's a heuristic for ranges of allocation sizes to exclude zero
> (size_range_flags) that comes into play here.  The actual range isn't
> "impossible" in the sense it's necessarily invalid.  It just means the
> string function call is either a no-op or out of bounds, and so can be
> eliminated as an optimization.  With the optimization consistently
> implemented the warning will also go away (eliminating the calls will
> prevent sanitizers from detecting the out of bounds ones, so that might be a
> consideration).

Ahh I see it now, I had missed that it was an 'int'.  ISTM that a better idea
would be to *not* optimize away memcpy and memmove in this case, not the other
way around.

> 
> In general, a low > high range denoted an anti-range before Ranger was
> introduced (i.e., ~[high, low]).  With Ranger it's the corresponding union
> of two ranges.  Some of the cruft for dealing with anti-ranges is still
> around, such as in get_size_range() in pointer-query.cc.  The code should be
> migrated to the irange class and the representation probably also updated to
> print something more sensible (e.g., the union [MIN, high) U (low, MAX]; we
> talked about introducing a pretty-printer % directive for ranges to make the
> format consistent across all diagnostics).

That makes sense, thanks.