[Bug c/96369] New: Wrong evaluation order of || operator

[Bug c/96369] New: Wrong evaluation order of || operator

[frank_yzg at outlook dot com]

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

            Bug ID: 96369
           Summary: Wrong evaluation order of || operator
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: frank_yzg at outlook dot com
  Target Milestone: ---

The following code snippet:

#include <stdio.h> 
int main(void)
{
    const long ONE = 1L;
    long y = 0L;
    long x = ((long) (ONE || (y = 1L)) % 8L);
    printf("x = %ld, y = %ld\n", x, y);    // with -O1/-O2/-O3 flag, gcc prints
x = 1, y = 1 where clang prints x = 1, y = 0
}

> $ /usr/gcc-trunk/bin/gcc -O2 -Wall -Wextra bug.c -o a.out
> $ ./a.out
> x = 1, y = 1

If the first operand of a logical-OR operation has a nonzero value, the second
operand is not evaluated. Therefore (y = 1L) should not be evaluated. It should
print y = 0 instead of y = 1.

This bug appears in GCC-4.8, GCC-6.4.0, GCC-7.5.0, GCC-8.4.0, GCC-9.3.0,
GCC-10.2.0 and GCC-11.0.0 20200501.

[Bug tree-optimization/96369] Wrong evaluation order of || operator

[glisse at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-07-29
           Keywords|                            |wrong-code
          Component|c                           |tree-optimization
             Status|UNCONFIRMED                 |NEW

[Bug c/96369] Wrong evaluation order of || operator

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |11.0, 4.8.5, 7.5.0
          Component|tree-optimization           |c

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Works with -O0, already failed with -O1 in .original:

    long int x = y = 1;, 1;

works with the C++ frontend so a frontend specific "misoptimization".

[Bug tree-optimization/96369] [8/9/10/11 Regression] Wrong evaluation order of || operator

[jsm28 at gcc dot gnu.org]

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

Joseph S. Myers <jsm28 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c                           |tree-optimization
            Summary|Wrong evaluation order of   |[8/9/10/11 Regression]
                   ||| operator                 |Wrong evaluation order of
                   |                            ||| operator

--- Comment #2 from Joseph S. Myers <jsm28 at gcc dot gnu.org> ---
Actually it appears the wrong code is being introduced somewhere in
fold_range_test (I didn't trace it further than that); the only front-end
involvement is that the front end does folding as part of conversions at
various points.

Testing with old compilers suggests this appeared between 4.8.1 and 4.8.3
(quite possibly through some backported change perturbing the exact sequence of
calls to folding functions rather than the actual underlying cause being
introduced in whatever backport).

[Bug tree-optimization/96369] [8/9/10/11 Regression] Wrong evaluation order of || operator

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
I have a patch.

[Bug tree-optimization/96369] [8/9/10/11 Regression] Wrong evaluation order of || operator

[cvs-commit at gcc dot gnu.org]

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

--- 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:10231958fcfb13bc4847729eba21470c101b4a88

commit r11-2450-g10231958fcfb13bc4847729eba21470c101b4a88
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Jul 31 08:41:56 2020 +0200

    middle-end/96369 - fix missed short-circuiting during range folding

    This makes the special case of constant evaluated LHS for a
    short-circuiting or/and explicit rather than doing range
    merging and eventually exposing a side-effect that shouldn't be
    evaluated.

    2020-07-31  Richard Biener  <rguenther@suse.de>

            PR middle-end/96369
            * fold-const.c (fold_range_test): Special-case constant
            LHS for short-circuiting operations.

            * c-c++-common/pr96369.c: New testcase.

[Bug tree-optimization/96369] [8/9/10 Regression] Wrong evaluation order of || operator

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |11.0
            Summary|[8/9/10/11 Regression]      |[8/9/10 Regression] Wrong
                   |Wrong evaluation order of   |evaluation order of ||
                   ||| operator                 |operator
      Known to fail|11.0                        |10.2.0

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

[Bug tree-optimization/96369] [8/9/10 Regression] Wrong evaluation order of || operator

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2
   Target Milestone|---                         |8.5

[Bug tree-optimization/96369] [8/9/10 Regression] Wrong evaluation order of || operator

[cvs-commit at gcc dot gnu.org]

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

--- 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:d3a0ef7a523fb4cdeabfbb9fab56bfcf8d21ffd5

commit r10-8745-gd3a0ef7a523fb4cdeabfbb9fab56bfcf8d21ffd5
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Jul 31 08:41:56 2020 +0200

    middle-end/96369 - fix missed short-circuiting during range folding

    This makes the special case of constant evaluated LHS for a
    short-circuiting or/and explicit rather than doing range
    merging and eventually exposing a side-effect that shouldn't be
    evaluated.

    2020-07-31  Richard Biener  <rguenther@suse.de>

            PR middle-end/96369
            * fold-const.c (fold_range_test): Special-case constant
            LHS for short-circuiting operations.

            * c-c++-common/pr96369.c: New testcase.

    (cherry picked from commit 10231958fcfb13bc4847729eba21470c101b4a88)

[Bug tree-optimization/96369] [8/9 Regression] Wrong evaluation order of || operator

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:505767905735a3c8f171c140108ee263f9fdcad6

commit r9-9093-g505767905735a3c8f171c140108ee263f9fdcad6
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Jul 31 08:41:56 2020 +0200

    middle-end/96369 - fix missed short-circuiting during range folding

    This makes the special case of constant evaluated LHS for a
    short-circuiting or/and explicit rather than doing range
    merging and eventually exposing a side-effect that shouldn't be
    evaluated.

    2020-07-31  Richard Biener  <rguenther@suse.de>

            PR middle-end/96369
            * fold-const.c (fold_range_test): Special-case constant
            LHS for short-circuiting operations.

            * c-c++-common/pr96369.c: New testcase.

    (cherry picked from commit 10231958fcfb13bc4847729eba21470c101b4a88)

[Bug tree-optimization/96369] [8 Regression] Wrong evaluation order of || operator

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:371ae12cf3b22795246a5707017f07257b5cbc97

commit r8-10803-g371ae12cf3b22795246a5707017f07257b5cbc97
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Jul 31 08:41:56 2020 +0200

    middle-end/96369 - fix missed short-circuiting during range folding

    This makes the special case of constant evaluated LHS for a
    short-circuiting or/and explicit rather than doing range
    merging and eventually exposing a side-effect that shouldn't be
    evaluated.

    2020-07-31  Richard Biener  <rguenther@suse.de>

            PR middle-end/96369
            * fold-const.c (fold_range_test): Special-case constant
            LHS for short-circuiting operations.

            * c-c++-common/pr96369.c: New testcase.

    (cherry picked from commit 505767905735a3c8f171c140108ee263f9fdcad6)

[Bug tree-optimization/96369] [8 Regression] Wrong evaluation order of || operator

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
      Known to fail|                            |8.4.0
      Known to work|                            |8.4.1
             Status|ASSIGNED                    |RESOLVED

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