[Bug c/95141] New: Incorrect integer overflow warning message for bitand expression

[Bug c/95141] New: Incorrect integer overflow warning message for bitand expression

[yadongh at vt dot edu]

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

            Bug ID: 95141
           Summary: Incorrect integer overflow warning message for bitand
                    expression
           Product: gcc
           Version: 8.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: yadongh at vt dot edu
  Target Milestone: ---

C code:
#include <stdint.h>

uint64_t test(uint8_t IA1)
{
  return (uint8_t)(IA1 & 158) & 1UL;
}


Command:
gcc -c test.c


Warning message:
test.c: In function ‘test’:
test.c:5:31: warning: integer overflow in expression ‘(long unsigned int)IA1 &
158 & 1’ of type ‘long unsigned int’ results in ‘0’ [-Woverflow]
   return (uint8_t)(IA1 & 158) & 1UL;
          ~~~~~~~~~~~~~~~~~~~~~^~~~~


gcc -v output:
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/8/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 8.3.0-6'
--with-bugurl=file:///usr/share/doc/gcc-8/README.Bugs
--enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++ --prefix=/usr
--with-gcc-
major-version-only --program-suffix=-8 --program-prefix=x86_64-linux-gnu-
--enable-shared --enable-linker-build-id --libexecdir=/usr/lib
--without-included-gettext --enable-threads=posix --libdir=/usr/lib --enab
le-nls --enable-bootstrap --enable-clocale=gnu --enable-libstdcxx-debug
--enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new
--enable-gnu-unique-object --disable-vtable-verify --enable-libmpx --enable-pl
ugin --enable-default-pie --with-system-zlib --with-target-system-zlib
--enable-objc-gc=auto --enable-multiarch --disable-werror --with-arch-32=i686
--with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-mult
ilib --with-tune=generic --enable-offload-targets=nvptx-none
--without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu
--host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 8.3.0 (Debian 8.3.0-6)

Note that I find this reproducible starting from GCC 7.1 to 10.1 (on GodBolt).


Platform:
Debian 10 Linux


Problem statement:
There are a lot of explicit/implicit integer type casts here, but in no way I
think integer overflow can happen. Note that essentially we are returning zero
here as 158 & 1 is just zero.
Some other interesting observations:
(uint8_t)(IA1 & 159) & 1UL;      --- No Warning
(uint8_t)(IA1 & 158U) & 1UL;     --- No Warning
(uint8_t)(IA1 & 254) & 1UL;      --- Warning
(uint8_t)(IA1 & 2) & 1UL;        --- No Warning

[Bug c/95141] [8/9/10/11 Regression] Incorrect integer overflow warning message for bitand expression

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org
   Last reconfirmed|                            |2020-05-15
   Target Milestone|---                         |8.5
     Ever confirmed|0                           |1
            Summary|Incorrect integer overflow  |[8/9/10/11 Regression]
                   |warning message for bitand  |Incorrect integer overflow
                   |expression                  |warning message for bitand
                   |                            |expression
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r7-7544-g2d143ba8cfef7ef480c639882fd5518b7afd822b

[Bug c/95141] [8/9/10/11 Regression] Incorrect integer overflow warning message for bitand expression

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
So there's already (OVF) at

((long unsigned int) IA1 & 158(OVF)) & 1

but we only check

375           if (TREE_OVERFLOW_P (ret)
376               && !TREE_OVERFLOW_P (op0)
377               && !TREE_OVERFLOW_P (op1))
378             overflow_warning (EXPR_LOC_OR_LOC (expr, input_location), ret,
expr);

which doesn't catch the pre-existing overflow on op0 which then propagates.

The original overflow is introduced folding short 158 to signed char -98(OVF)
via convert.c:do_narrow:

437           /* We should do away with all this once we have a proper
438              type promotion/demotion pass, see PR45397.  */
439           expr = maybe_fold_build2_loc (dofold, loc, ex_form, typex,
440                                         convert (typex, arg0),
441                                         convert (typex, arg1));

specifically the convert (typex, arg1).

Now TREE_OVERFLOW in general is quite a fragile thing, but it's tempting to
adjust the overflow_warning guard for this case ...

The do_narrow code also specifically looks for overflow cases that matter
and does not perform narrowing then so clearing TREE_OVERFLOW there would
also look reasonable.

Thus like the following?  Should be cheaper than adding walk_tree to the
diagnostic guard.

diff --git a/gcc/convert.c b/gcc/convert.c
index 42509c518a9..ed00ded1a89 100644
--- a/gcc/convert.c
+++ b/gcc/convert.c
@@ -436,9 +436,16 @@ do_narrow (location_t loc,
        }
       /* We should do away with all this once we have a proper
         type promotion/demotion pass, see PR45397.  */
+      /* Above we checked for all cases where overflow matters, avoid
+        geneating overflowed constants here which otherwise propagate
+        and cause spurious warnings, see PR95141.  */
+      tree converted_arg1 = convert (typex, arg1);
+      if (TREE_OVERFLOW_P (converted_arg1)
+         && !TREE_OVERFLOW_P (arg1))
+       converted_arg1 = drop_tree_overflow (converted_arg1);
       expr = maybe_fold_build2_loc (dofold, loc, ex_form, typex,
                                    convert (typex, arg0),
-                                   convert (typex, arg1));
+                                   converted_arg1);
       return convert (type, expr);
     }

[Bug c/95141] [8/9/10/11 Regression] Incorrect integer overflow warning message for bitand expression

[rguenth at gcc dot gnu.org]

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

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #2)
> So there's already (OVF) at
> 
> ((long unsigned int) IA1 & 158(OVF)) & 1
> 
> but we only check
> 
> 375           if (TREE_OVERFLOW_P (ret)
> 376               && !TREE_OVERFLOW_P (op0)
> 377               && !TREE_OVERFLOW_P (op1))
> 378             overflow_warning (EXPR_LOC_OR_LOC (expr, input_location),
> ret, expr);
> 
> which doesn't catch the pre-existing overflow on op0 which then propagates.
> 
> The original overflow is introduced folding short 158 to signed char -98(OVF)
> via convert.c:do_narrow:
> 
> 437           /* We should do away with all this once we have a proper
> 438              type promotion/demotion pass, see PR45397.  */
> 439           expr = maybe_fold_build2_loc (dofold, loc, ex_form, typex,
> 440                                         convert (typex, arg0),
> 441                                         convert (typex, arg1));
> 
> specifically the convert (typex, arg1).
> 
> Now TREE_OVERFLOW in general is quite a fragile thing, but it's tempting to
> adjust the overflow_warning guard for this case ...
> 
> The do_narrow code also specifically looks for overflow cases that matter
> and does not perform narrowing then so clearing TREE_OVERFLOW there would
> also look reasonable.
> 
> Thus like the following?  Should be cheaper than adding walk_tree to the
> diagnostic guard.
> 
> diff --git a/gcc/convert.c b/gcc/convert.c
> index 42509c518a9..ed00ded1a89 100644
> --- a/gcc/convert.c
> +++ b/gcc/convert.c
> @@ -436,9 +436,16 @@ do_narrow (location_t loc,
>         }
>        /* We should do away with all this once we have a proper
>          type promotion/demotion pass, see PR45397.  */
> +      /* Above we checked for all cases where overflow matters, avoid
> +        geneating overflowed constants here which otherwise propagate
> +        and cause spurious warnings, see PR95141.  */
> +      tree converted_arg1 = convert (typex, arg1);
> +      if (TREE_OVERFLOW_P (converted_arg1)
> +         && !TREE_OVERFLOW_P (arg1))
> +       converted_arg1 = drop_tree_overflow (converted_arg1);
>        expr = maybe_fold_build2_loc (dofold, loc, ex_form, typex,
>                                     convert (typex, arg0),
> -                                   convert (typex, arg1));
> +                                   converted_arg1);
>        return convert (type, expr);
>      }

Regresses

FAIL: gcc.dg/overflow-warn-5.c  (test for warnings, line 6)

which looks like a useful warning to preserve.  Lame "walk-tree" variant
catching this case:

diff --git a/gcc/c/c-fold.c b/gcc/c/c-fold.c
index 63becfeaf2c..bd21d247051 100644
--- a/gcc/c/c-fold.c
+++ b/gcc/c/c-fold.c
@@ -374,6 +374,7 @@ c_fully_fold_internal (tree expr, bool in_init, bool
*maybe_const_operands,
        ret = fold (expr);
       if (TREE_OVERFLOW_P (ret)
          && !TREE_OVERFLOW_P (op0)
+         && !(BINARY_CLASS_P (op0) && TREE_OVERFLOW_P (TREE_OPERAND (op0, 1)))
          && !TREE_OVERFLOW_P (op1))
        overflow_warning (EXPR_LOC_OR_LOC (expr, input_location), ret, expr);
       if (code == LSHIFT_EXPR

[Bug c/95141] [8/9/10/11 Regression] Incorrect integer overflow warning message for bitand expression

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:4a88caf21a0a85129f6c985ca13ba3eb54ff5366

commit r11-509-g4a88caf21a0a85129f6c985ca13ba3eb54ff5366
Author: Richard Biener <rguenther@suse.de>
Date:   Tue May 19 07:58:33 2020 +0200

    c/95141 - fix bogus integer overflow warning

    This fixes an integer overflow warning that ultimatively happens because
    of TREE_OVERFLOW propagating through transforms and the existing guard
    against this,

    375           if (TREE_OVERFLOW_P (ret)
    376               && !TREE_OVERFLOW_P (op0)
    377               && !TREE_OVERFLOW_P (op1))
    378             overflow_warning (EXPR_LOC_OR_LOC (expr, input_location,

    being insufficient.  Rather than trying to use sth like walk_tree to
    exhaustively walk operands (with the possibility of introducing
    quadraticness when folding larger expressions recursively) the
    following amends the above with an ad-hoc test for a binary op0
    with a possibly constant op1.

    2020-05-30  Richard Biener  <rguenther@suse.de>

            PR c/95141
    gcc/c
            * c-fold.c (c_fully_fold_internal): Enhance guard on
            overflow_warning.

    gcc/testsuite
            * gcc.dg/pr95141.c: New testcase.

[Bug c/95141] [8/9/10 Regression] Incorrect integer overflow warning message for bitand expression

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |10.1.0
            Summary|[8/9/10/11 Regression]      |[8/9/10 Regression]
                   |Incorrect integer overflow  |Incorrect integer overflow
                   |warning message for bitand  |warning message for bitand
                   |expression                  |expression
      Known to work|                            |11.0

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed on trunk sofar.

[Bug c/95141] [8/9/10 Regression] Incorrect integer overflow warning message for bitand expression

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Richard Biener
<rguenth@gcc.gnu.org>:

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

commit r10-8351-gcc08609b62ed37d279c986ebb9cb8294eb4e0e3b
Author: Richard Biener <rguenther@suse.de>
Date:   Tue May 19 07:58:33 2020 +0200

    c/95141 - fix bogus integer overflow warning

    This fixes an integer overflow warning that ultimatively happens because
    of TREE_OVERFLOW propagating through transforms and the existing guard
    against this,

    375           if (TREE_OVERFLOW_P (ret)
    376               && !TREE_OVERFLOW_P (op0)
    377               && !TREE_OVERFLOW_P (op1))
    378             overflow_warning (EXPR_LOC_OR_LOC (expr, input_location,

    being insufficient.  Rather than trying to use sth like walk_tree to
    exhaustively walk operands (with the possibility of introducing
    quadraticness when folding larger expressions recursively) the
    following amends the above with an ad-hoc test for a binary op0
    with a possibly constant op1.

    2020-05-30  Richard Biener  <rguenther@suse.de>

            PR c/95141
    gcc/c
            * c-fold.c (c_fully_fold_internal): Enhance guard on
            overflow_warning.

    gcc/testsuite
            * gcc.dg/pr95141.c: New testcase.

[Bug c/95141] [8/9 Regression] Incorrect integer overflow warning message for bitand expression

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2
            Summary|[8/9/10 Regression]         |[8/9 Regression] Incorrect
                   |Incorrect integer overflow  |integer overflow warning
                   |warning message for bitand  |message for bitand
                   |expression                  |expression
      Known to work|                            |10.1.1

[Bug c/95141] [9 Regression] Incorrect integer overflow warning message for bitand expression

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|8.5                         |9.4

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 8 branch is being closed.

[Bug c/95141] [9 Regression] Incorrect integer overflow warning message for bitand expression

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|9.4                         |9.5

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 9.4 is being released, retargeting bugs to GCC 9.5.

[Bug c/95141] [9 Regression] Incorrect integer overflow warning message for bitand expression

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED
   Target Milestone|9.5                         |10.2
      Known to fail|                            |9.5.0

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed for GCC 10.2.