[Bug c/113664] New: False positive warnings with -fno-strict-overflow (-Warray-bounds, -Wstringop-overflow)

[Bug c/113664] New: False positive warnings with -fno-strict-overflow (-Warray-bounds, -Wstringop-overflow)

[stefan at bytereef dot org]

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

            Bug ID: 113664
           Summary: False positive warnings with -fno-strict-overflow
                    (-Warray-bounds, -Wstringop-overflow)
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: stefan at bytereef dot org
  Target Milestone: ---

These false positives only occur in combination with fno-strict-overflow:

================
 -Warray-bounds
================

foo.c
=========================================================
#include <stdio.h>

static char *
f(char *s, int n, char *dot)
{
  switch(n) {
  case 1:
    if (s == dot) {
      *s++ = '.';
    }
    *s++ = '0'; /* fall-through (yes, really!) */
  default:
    if (s == dot) {
      *s++ = '.';
    }
  }

  *s = '\0';
  return s;
}

char *
g(char *s)
{
  return f(s, 1, NULL);
}
=========================================================


$ /home/skrah/gcc/bin/gcc -Wall -O3 -c foo.c
$ /home/skrah/gcc/bin/gcc -Wall -O3 -fno-strict-overflow -c foo.c
In function ‘f’,
    inlined from ‘g’ at foo.c:25:10:
foo.c:11:10: warning: array subscript 0 is outside array bounds of ‘char[0]’
[-Warray-bounds=]
   11 |     *s++ = '0'; /* fall-through (yes, really!) */
      |     ~~~~~^~~~~
In function ‘g’:
cc1: note: source object is likely at address zero



=====================
 -Wstringop-overflow 
=====================

bar.c
=========================================================
#include <stdio.h>

static char *
f(char *s, int n, char *dot)
{
  switch(n) {
  case 1:
    if (s == dot) {
      *s++ = '.';
    }
    *s++ = '0'; /* fall-through (yes, really!) */
  default:
    if (s == dot) {
      *s++ = '.';
    }
  }

  *s = '\0';
  return s;
}

char *
g(char *s)
{
    char sign = '+';
    *s++ = sign;

    return f(s, 1, NULL);
}
=========================================================


$ /home/skrah/gcc/bin/gcc -Wall -O3 -c bar.c
$ /home/skrah/gcc/bin/gcc -Wall -O3 -fno-strict-overflow -c bar.c
In function ‘f’,
    inlined from ‘g’ at bar.c:28:12:
bar.c:11:10: warning: writing 1 byte into a region of size 0
[-Wstringop-overflow=]
   11 |     *s++ = '0'; /* fall-through (yes, really!) */
      |     ~~~~~^~~~~
In function ‘g’:
cc1: note: destination object is likely at address zero




Note that a very small change gives a very different warning.

[Bug tree-optimization/113664] False positive warnings with -fno-strict-overflow (-Warray-bounds, -Wstringop-overflow)

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
-fno-strict-overflow turns on -fwrapv-pointer which allows pointers to wrap
which means if s was non-null, then `s+1` can be still a null pointer ...

And then we go and prop null into dot and s is equal to null at that point. and
then we still generate the code for `*s++ = '.';` in
```
    if (s == dot) {
      *s++ = '.';
    }
```

But it is `*NULL = '.';` due to that.

The warning is very sensitive to the ability to optimization away null pointer
checks in this case. Really `fno-strict-overflow` is normally to workaround
some "undefinedness" in the code and the code should be improved to be fixed.
Using -fwrapv instead also helps because now only signed integer overflows and
not also pointers ...

[Bug tree-optimization/113664] False positive warnings with -fno-strict-overflow (-Warray-bounds, -Wstringop-overflow)

[stefan at bytereef dot org]

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

--- Comment #2 from Stefan Krah <stefan at bytereef dot org> ---
Thanks for the explanation!  I agree that one should not rely on
-fno-strict-overflow. In this case, my project is "vendored" in CPython and
they compile everything with -fno-strict-overflow, so it's out of my control:

https://github.com/python/cpython/issues/108562


mpdecimal itself does not need -fno-strict-overflow.

[Bug tree-optimization/113664] False positive warnings with -fno-strict-overflow (-Warray-bounds, -Wstringop-overflow)

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
https://github.com/python/cpython/issues/96821 is the issue to re-enable
strict-overflow ...

[Bug tree-optimization/113664] False positive warnings with -fno-strict-overflow (-Warray-bounds, -Wstringop-overflow)

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2024-01-30
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed.  As usual it's jump-threading related where we isolate, in the
-Warray-bounds case

MEM[(char *)1B] = 48;

we inline 'f' and then, when s == dot == NULL your code dereferences both
NULL and NULL + 1.

So the diagnostic messages leave a lot to be desired but in the end they
point to a problem in your code which is a guard against a NULL 's'.

The jump threading is different with -fwrapv-pointer, in particular without
it we just get the NULL dereference which we seem to ignore during
array-bound diagnostics.

We later isolate the paths as unreachable but that happens after the
diagnostic.

[Bug tree-optimization/113664] False positive warnings with -fno-strict-overflow (-Warray-bounds, -Wstringop-overflow)

[stefan at bytereef dot org]

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

--- Comment #5 from Stefan Krah <stefan at bytereef dot org> ---
> So the diagnostic messages leave a lot to be desired but in the end
> they point to a problem in your code which is a guard against a NULL 's'.

Hmm, the real code is used to print floating point numbers and integers.
Integers get dot==NULL. It is fine (and desired!) in that case to optimize
away the if clause.

As far as I can see, it is compliant with the C standard.


Even with -fno-strict-overflow one could make the case that the warning
is strange. If "s" wraps around, the allocated output string is too small,
and you have bigger problems.

It is impossible for gcc to detect whether the string size is sufficient,
so IMHO it should not warn.


In essence, since gcc-10 (12?) idioms that were warning-free for 10 years
tend to receive false positive warnings now.

This also applies to -Warray-bounds. I think the Linux kernel disables at
least -Warray-bounds and -Wmaybe-uninitialized.

I think this is becoming a problem, because most projects do not report
false positives but just silently disable the warnings.

[Bug tree-optimization/113664] False positive warnings with -fno-strict-overflow (-Warray-bounds, -Wstringop-overflow)

[stefan at bytereef dot org]

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

--- Comment #6 from Stefan Krah <stefan at bytereef dot org> ---
Sometimes you hear "code should be rewritten" because squashing the warnings
makes it better.

I disagree. I've seen many segfaults introduced in projects that rush
to squash warnings.

Sometimes, analyzers just cannot cope with established idioms. clang-analyzer
for instance hates Knuth's algorithm D (long division). It would be strange to
change that for an analyzer.