[Bug middle-end/103483] [12 regression] context-sensitive ranges change triggers stringop-overread

[msebor at gcc dot gnu.org]

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

--- Comment #17 from Martin Sebor <msebor at gcc dot gnu.org> ---
Jaosn: this is how all middle-end warnings have always behaved.  They trigger
on invalid statements present in the IL.  A statement is considered invalid
when any of its operands is out of bounds or in some other way not valid for it
(e.g., null in -Wnonnull, or not pointing to the first byte allocated on the
heap in -Wfree-nonheap-pointer, or not matching an allocation call in
-Wmismatched-dealloc).  Warnings don't differentiate between constant operands
or those in some range.  We have been making more extensive use of ranges since
get_range_info() was introduced, but prior to that, -Warray-bounds made use of
ranges as well (thanks to VRP).  Even in warnings that don't use ranges,
constant operands need not be literals: they can be reduced to constants from
ranges by various transformations (jump threading is just one of them).  For
more background please see my two-part article from 2019: Understanding GCC
Warnings:

  https://developers.redhat.com/blog/2019/03/13/understanding-gcc-warnings
 
https://developers.redhat.com/blog/2019/03/13/understanding-gcc-warnings-part-2

It's easy to derive examples from the one in comment #12 or comment #15 showing
other similar warnings: replacing the memcpy() call with an array subscript
triggers a -Warray-bounds; snprintf() triggers -Wformat-truncation; malloc()
triggers -Walloc-size-larger-than; etc.  See below.  (I also showed an example
with -Wnonnull in comment #13.  It's issued on the same basis.)

It might seem like the common denominator in all these instances is ranges, but
they're a red herring.  The same effect can be demonstrated without them.  The
root cause behind them all is that (again) warnings are designed to trigger for
apparently reachable invalid IL.  See pr54202 for an example from 2012 with
-Wfree-nonheap-object.  The warning is simply based on what the pointer points
to, irrespective of the conditions under which the invalid statement is
evaluated.

If you consider any of the warnings above false positives you must consider as
such all of them.  It makes no sense to do something about just a subset of
them and not the rest.  And to avoid them altogether you have to disable (or at
least seriously cripple) all those we've ever added into the middle end.  You
could, for example, only warn in statements that are reached unconditionally
from function entry.  Removing them them all from -Wall and making them opt-in
would reduce the number of complaints but only as a result of the number of
users explicitly enabling the warnings, without actually improving anything
(besides, by being included in -Wall most already are opt-in).  In any event,
any of these alternatives would compromise the security improvements we have
invested so much in over the years.  The best solution, in my view, is to show
users the conditionals under which the invalid statements can be reached.  I
hoped to be able to do that by extending Ranger
(https://gcc.gnu.org/pipermail/gcc/2021-December/237922.html) but it could also
be done by rolling a range propagation engine just for warnings (like for the
static analyzer).

char *sink;
__attribute__ ((noipa))
int mystrlen (const char *p)
{
  return __builtin_strlen (p);
}

inline void copy(const char *p)
{
  int L = mystrlen (p);
  if (L < 5)
    /* Small string magic. */;
  else
   *sink = p[L];
}
void f()
{ 
  copy ("12");
} 

In function ‘copy’,
    inlined from ‘f’ at a.c:18:3:
a.c:14:13: warning: array subscript [5, 2147483647] is outside array bounds of
‘char[3]’ [-Warray-bounds]
   14 |    *sink = p[L];
      |            ~^~~

char *sink;
__attribute__ ((noipa))
int mystrlen (const char *p)
{
  return __builtin_strlen (p);
}

inline void copy(const char *p)
{
  int L = mystrlen (p);
  if (L < 5)
    /* Small string magic. */;
  else
    __builtin_snprintf (sink, 5, "%*s", L, p);
}
void f()
{ 
  copy ("12");
} 

a.c: In function ‘f’:
a.c:14:38: warning: ‘__builtin_snprintf’ output truncated before the last
format character [-Wformat-truncation=]
   14 |     __builtin_snprintf (sink, 5, "%*s", L, p);
      |                                      ^
In function ‘copy’,
    inlined from ‘f’ at a.c:18:3:
a.c:14:5: note: ‘__builtin_snprintf’ output between 6 and 2147483648 bytes into
a destination of size 5
   14 |     __builtin_snprintf (sink, 5, "%*s", L, p);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


$ gcc -O2 -S -Walloc-size-larger-than=4 a.c
har *sink;
__attribute__ ((noipa))
int mystrlen (const char *p)
{
  return __builtin_strlen (p);
}

inline void copy(const char *p)
{
  int L = mystrlen (p);
  if (L < 5)
    /* Small string magic. */;
  else
    sink = __builtin_malloc (L);
}
void f()
{ 
  copy ("12");
} 

In function ‘copy’,
    inlined from ‘f’ at a.c:18:3:
a.c:14:12: warning: argument 1 range [5, 2147483647] exceeds maximum object
size 4 [-Walloc-size-larger-than=]
   14 |     sink = __builtin_malloc (L);
      |            ^~~~~~~~~~~~~~~~~~~~
a.c:14:12: note: in a call to built-in allocation function ‘__builtin_malloc’