[Bug c/56051] New: Wrong expression evaluation

[Bug c/56051] New: Wrong expression evaluation

[olivier.gay at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56051

             Bug #: 56051
           Summary: Wrong expression evaluation
    Classification: Unclassified
           Product: gcc
           Version: 4.7.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: olivier.gay@gmail.com


#include <stdio.h>

int main(void)
{
    int a, s = 8;
    unsigned char data[1] = {0};

    a = data[0] < (unsigned char) (1 << s);
    printf("%d\n", a);

    return 0;
}

The expression assigned to object a evaluates to 1 but I think it should
evaluate to 0. Tested on gcc 4.7.2 / Linux x64.

[Bug c/56051] Wrong expression evaluation

[glisse at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56051

Marc Glisse <glisse at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-01-20
     Ever Confirmed|0                           |1

--- Comment #1 from Marc Glisse <glisse at gcc dot gnu.org> 2013-01-20 12:24:18 UTC ---
fold-const.c:13561 doesn't check that the conversion is not narrowing before
doing the optimization.

[Bug middle-end/56051] Wrong expression evaluation

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56051

Mikael Pettersson <mikpe at it dot uu.se> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mikpe at it dot uu.se

--- Comment #2 from Mikael Pettersson <mikpe at it dot uu.se> 2013-01-20 14:11:20 UTC ---
Technically the test case should use CHAR_BIT not 8.  The bug is present in
every release back to at least 2.95.3.

[Bug tree-optimization/56051] Wrong expression evaluation

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56051

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org
          Component|middle-end                  |tree-optimization
   Target Milestone|---                         |4.6.4

[Bug tree-optimization/56051] Wrong expression evaluation

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56051

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |jakub at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-01-20 16:46:04 UTC ---
Created attachment 29227
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29227
gcc48-pr56051.patch

Untested fix.  As the testcase shows, also a widening conversion can be a
problem, if it extends from signed integral type to wider unsigned one, because
then for Y equal to bitsize of the narrower type - 1 we get more than one bit
set.

On the other side, the optimization doesn't hit when X isn't an ARRAY_REF, but
just an integral variable, because then arg0 and arg1 are swapped.
Guess for 4.9 we should handle those cases too.

[Bug tree-optimization/56051] Wrong expression evaluation

[glisse at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56051

--- Comment #4 from Marc Glisse <glisse at gcc dot gnu.org> 2013-01-20 18:09:40 UTC ---
(In reply to comment #3)
> Untested fix.  As the testcase shows, also a widening conversion can be a
> problem, if it extends from signed integral type to wider unsigned one, because
> then for Y equal to bitsize of the narrower type - 1 we get more than one bit
> set.

I assume this is because of the gcc extension documented as:
"GCC does not use the latitude given in C99 only to treat certain aspects of
signed `<<' as undefined, but this is subject to change."
which prevents from assuming that 1<<n is positive. Maybe that can be changed
at some point, as the documentation suggests? ;-)

PS: sorry for setting it to the middle-end component, I mistakenly believed
that tree-optimization was only for SSA.

[Bug tree-optimization/56051] Wrong expression evaluation

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56051

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-01-20 18:35:20 UTC ---
Yeah, I'm afraid assuming you never do 1 << 31 is going to break simply way too
much code in the wild.

[Bug tree-optimization/56051] Wrong expression evaluation

[glisse at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56051

--- Comment #6 from Marc Glisse <glisse at gcc dot gnu.org> 2013-01-20 20:17:31 UTC ---
(In reply to comment #5)
> Yeah, I'm afraid assuming you never do 1 << 31 is going to break simply way too
> much code in the wild.

I noticed that clang warns for 1 << 31, and with -fsanitize=shift tells you if
there is 1<<n with n==31 in C, but not in C++. And the reason seems to be that
the C++ committee has, in DR1457, retroactively made it legal :-(
(PR52119 also mentions this)

[Bug tree-optimization/56051] Wrong expression evaluation

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56051

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-01-21 17:55:46 UTC ---
Author: jakub
Date: Mon Jan 21 17:55:34 2013
New Revision: 195343

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195343
Log:
    PR tree-optimization/56051
    * fold-const.c (fold_binary_loc): Don't fold
    X < (cast) (1 << Y) into (X >> Y) != 0 if cast is either
    a narrowing conversion, or widening conversion from signed
    to unsigned.

    * gcc.c-torture/execute/pr56051.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr56051.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c
    trunk/gcc/testsuite/ChangeLog

[Bug tree-optimization/56051] Wrong expression evaluation

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56051

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-01-21 18:01:59 UTC ---
Fixed on the trunk so far.

[Bug tree-optimization/56051] Wrong expression evaluation

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56051

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-02-01 14:11:54 UTC ---
Author: jakub
Date: Fri Feb  1 14:11:46 2013
New Revision: 195658

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195658
Log:
    Backported from mainline
    2013-01-21  Jakub Jelinek  <jakub@redhat.com>

    PR tree-optimization/56051
    * fold-const.c (fold_binary_loc): Don't fold
    X < (cast) (1 << Y) into (X >> Y) != 0 if cast is either
    a narrowing conversion, or widening conversion from signed
    to unsigned.

    * gcc.c-torture/execute/pr56051.c: New test.

Added:
    branches/gcc-4_7-branch/gcc/testsuite/gcc.c-torture/execute/pr56051.c
Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/fold-const.c
    branches/gcc-4_7-branch/gcc/testsuite/ChangeLog

[Bug tree-optimization/56051] Wrong expression evaluation

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56051

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.7.3

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-02-01 14:33:08 UTC ---
Fixed for 4.7.3+ too.

[Bug tree-optimization/56051] Wrong expression evaluation

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56051

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|4.6.4                       |4.7.3

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-12 16:29:07 UTC ---
The 4.6 branch has been closed, fixed in GCC 4.7.3.