[Bug sanitizer/109107] New: UBsan since GCC-8 misses an integer-overflow

[Bug sanitizer/109107] New: UBsan since GCC-8 misses an integer-overflow

[shaohua.li at inf dot ethz.ch]

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

            Bug ID: 109107
           Summary: UBsan since GCC-8 misses an integer-overflow
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: sanitizer
          Assignee: unassigned at gcc dot gnu.org
          Reporter: shaohua.li at inf dot ethz.ch
                CC: dodji at gcc dot gnu.org, dvyukov at gcc dot gnu.org,
                    jakub at gcc dot gnu.org, kcc at gcc dot gnu.org, marxin at gcc dot gnu.org
  Target Milestone: ---

For the following code, UBsan since GCC-8 at -O1 and above misses the integer
overflow. GCC-7 can detect it at all opt levels.

Clang can detect it at all opt levels.

Compiler explorer: https://godbolt.org/z/Pez4jd5aj

% cat a.c
int a = -2147468918, c;
const long b = 676540;
int main() { 
    c = 1 + a - (short)b; 
    return c;
}
%
% gcc-tk -O1 -fsanitize=undefined a.c &&./a.out
%
% gcc-7 -O1 -fsanitize=undefined a.c &&./a.out
/a.c:4:15: runtime error: signed integer overflow: -2147468918 - 21179 cannot
be represented in type 'int'
%

[Bug sanitizer/109107] [10/11/12/13 Regression] UBsan since GCC-8 misses an integer-overflow

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|UBsan since GCC-8 misses an |[10/11/12/13 Regression]
                   |integer-overflow            |UBsan since GCC-8 misses an
                   |                            |integer-overflow
           Priority|P3                          |P2
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2023-03-13
                 CC|                            |mpolacek at gcc dot gnu.org
   Target Milestone|---                         |10.5
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Confirmed.  Started with r8-1516-ged73f46f30cabe so not a bug of PR108995 or
PR109090.

commit ed73f46f30cabeea4de64e7cce0682a7a610ffb6
Author: Marc Glisse <marc.glisse@inria.fr>
Date:   Wed Jun 21 13:16:27 2017 +0200

    NOP conversions in X+CST+CST

[Bug sanitizer/109107] [10/11/12/13 Regression] UBsan since GCC-8 misses an integer-overflow

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |mpolacek at gcc dot gnu.org

--- Comment #2 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Hopefully just a missing TYPE_OVERFLOW_SANITIZED.  Let me see...

[Bug sanitizer/109107] [10/11/12/13 Regression] UBsan since GCC-8 misses an integer-overflow

[mpolacek at gcc dot gnu.org]

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

--- Comment #3 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
A similar problem:

#define INT_MIN (-__INT_MAX__ - 1)
int a = INT_MIN;
const int b = 676540;
int
main ()
{
  int c = a - 1 + (int) (short) b;
  return c;
}

for which I think we need:

--- a/gcc/fold-const.cc
+++ b/gcc/fold-const.cc
@@ -11319,7 +11319,8 @@ fold_binary_loc (location_t loc, enum tree_code code,
tree type,
     And, we need to make sure type is not saturating.  */

       if ((! FLOAT_TYPE_P (type) || flag_associative_math)
-     && !TYPE_SATURATING (type))
+     && !TYPE_SATURATING (type)
+     && !TYPE_OVERFLOW_SANITIZED (type))
    {
      tree var0, minus_var0, con0, minus_con0, lit0, minus_lit0;
      tree var1, minus_var1, con1, minus_con1, lit1, minus_lit1;

[Bug sanitizer/109107] [10/11/12/13 Regression] UBsan since GCC-8 misses an integer-overflow

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Marek Polacek <mpolacek@gcc.gnu.org>:

https://gcc.gnu.org/g:c1aca26b707471ce8051bd03b3fb2217bcdf2df0

commit r13-7001-gc1aca26b707471ce8051bd03b3fb2217bcdf2df0
Author: Marek Polacek <polacek@redhat.com>
Date:   Mon Mar 13 18:50:25 2023 -0400

    sanitizer: missing signed integer overflow errors [PR109107]

    Here we're failing to detect a signed overflow with -O because match.pd,
    since r8-1516, transforms

      c = (a + 1) - (int) (short int) b;

    into

      c = (int) ((unsigned int) a + 4294946117);

    wrongly eliding the overflow.  This kind of problems is usually
    avoided by using TYPE_OVERFLOW_SANITIZED in the appropriate place.
    The first match.pd hunk in the patch fixes it.  I've constructed
    a testcase for each of the surrounding cases as well.  Then I
    noticed that fold_binary_loc/associate has the same problem, so I've
    added a TYPE_OVERFLOW_SANITIZED there as well (it may be too coarse,
    sorry).  Then I found yet another problem, but instead of fixing it
    now I've opened 109134.  I could probably go on and find a dozen more.

            PR sanitizer/109107

    gcc/ChangeLog:

            * fold-const.cc (fold_binary_loc): Use TYPE_OVERFLOW_SANITIZED
            when associating.
            * match.pd: Use TYPE_OVERFLOW_SANITIZED.

    gcc/testsuite/ChangeLog:

            * c-c++-common/ubsan/pr109107-1.c: New test.
            * c-c++-common/ubsan/pr109107-2.c: New test.
            * c-c++-common/ubsan/pr109107-3.c: New test.
            * c-c++-common/ubsan/pr109107-4.c: New test.

[Bug sanitizer/109107] [10/11/12/13 Regression] UBsan since GCC-8 misses an integer-overflow

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #5 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Fixed for GCC 13.

[Bug sanitizer/109107] [10/11/12/13 Regression] UBsan since GCC-8 misses an integer-overflow

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.5                        |13.0