[Bug c/107890] New: UB on integer overflow impacts code flow

[Bug c/107890] New: UB on integer overflow impacts code flow

[gcc at pkh dot me]

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

            Bug ID: 107890
           Summary: UB on integer overflow impacts code flow
           Product: gcc
           Version: 12.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gcc at pkh dot me
  Target Milestone: ---

Following is a code that is sensible to a signed integer overflow. I was under
the impression that this kind of undefined behavior essentially meant that the
value of that integer could become unreliable. But apparently this is not
limited to the value of said integer, it can also dramatically impact the code
flow.

Here is the pathological code:

    #include <stdint.h>
    #include <stdio.h>
    #include <stdlib.h>

    uint8_t tab[0x1ff + 1];

    uint8_t f(int32_t x)
    {
        if (x < 0)
            return 0;
        int32_t i = x * 0x1ff / 0xffff;
        if (i >= 0 && i < sizeof(tab)) {
            printf("tab[%d] looks safe because %d is between [0;%d[\n", i, i,
(int)sizeof(tab));
            return tab[i];
        }

        return 0;
    }

    int main(int ac, char **av)
    {
        return f(atoi(av[1]));
    }

Triggering an overflow actually enters the printf/dereference scope, violating
the protective condition and thus causing a crash:

    % cc -Wall -O2 overflow.c -o overflow && ./overflow 50000000
    tab[62183] looks safe because 62183 is between [0;512[
    zsh: segmentation fault (core dumped)  ./overflow 50000000

I feel extremely uncomfortable about an integer overflow actually impacting
something else than the integer itself. Is it expected or is this a bug?

[Bug c/107890] UB on integer overflow impacts code flow

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
>I was under the impression that this kind of undefined behavior essentially meant that the value of that integer could become unreliable.

Your impression is incorrect. Once undefined behavior happens, anything can
happen. 

This is why things like -fsanitize=undefined is there now.

[Bug c/107890] UB on integer overflow impacts code flow

[redi at gcc dot gnu.org]

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

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
You should read https://blog.regehr.org/archives/213

[Bug c/107890] UB on integer overflow impacts code flow

[muecker at gwdg dot de]

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

Martin Uecker <muecker at gwdg dot de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |muecker at gwdg dot de

--- Comment #3 from Martin Uecker <muecker at gwdg dot de> ---

Of course, instead of using the standard as an excuse, we could also try to
make the compiler less of a footgun. 

Even if this is standard conforming, it is still a severe usability issue with
safety implications and I do not think we should simply close such bugs.

[Bug c/107890] UB on integer overflow impacts code flow

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
There is always a trade off here.
We made the decision that signed integer overflow is undefined because we want
to do optimizations. Gcc does provide an option which makes them behave well
defined at runtime, -fwrapv . This is similar to strict aliasing with respect
to optimizations in the sense it is hard to decide if the optimizations is
being done will break what people assumptions are. This is part of the reason
why there is a specifications (standard) so people can write to it.
There are other undefined behavior that gcc has started to take advantage of
(e.g. in c++ if there is no return with a value in a function with that returns
non-void). The question is where do you draw the line with respect to undefined
behaviors, the answer is complex sometimes, especially if you are optimizing.

In this example the range of x is known to be positive so multiplying by
another positive # gives a positive result and then dividing by a positive
value still is positive. The check for negative result is optimized away.

[Bug c/107890] UB on integer overflow impacts code flow

[egallager at gcc dot gnu.org]

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

Eric Gallager <egallager at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |egallager at gcc dot gnu.org
           Keywords|                            |diagnostic

--- Comment #5 from Eric Gallager <egallager at gcc dot gnu.org> ---
What I wonder is, why doesn't -Wstrict-overflow=5 catch this? That's supposed
to be its strictest level, right?

$ /usr/local/bin/gcc -Wall -Wextra -pedantic -Wstrict-overflow=5 -O2 107890.c
107890.c: In function 'f':
107890.c:13:25: warning: comparison of integer expressions of different
signedness: 'int32_t' {aka 'int'} and 'long unsigned int' [-Wsign-compare]
   13 |         if (i >= 0 && i < sizeof(tab)) {
      |                         ^
107890.c: In function 'main':
107890.c:21:18: warning: unused parameter 'ac' [-Wunused-parameter]
   21 |     int main(int ac, char **av)
      |              ~~~~^~
$