[Bug c++/48814] New: Incorrect scalar increment result

[Bug c++/48814] New: Incorrect scalar increment result

[schaub.johannes at googlemail dot com]

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

           Summary: Incorrect scalar increment result
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: schaub.johannes@googlemail.com


The following test-case should output "2 3", but GCC outputs "1 2":

#include <stdio.h>

int arr[] = {1,2,3,4};
int count = 0;

int incr(){
 return ++count;
}

int main(){  
   arr[count++]=incr();
   printf("%d %d",count,arr[count]);    
   return 0;
}

(it may not be obvious; see
http://stackoverflow.com/questions/5827707/why-this-output/5828578#5828578 ).
Clang correctly handles this case.

[Bug c++/48814] Incorrect scalar increment result

[rguenth at gcc dot gnu.org]

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

--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-04-29 09:56:08 UTC ---
6.5.16/4

"The order of evaluation of the operands is unspecified."


The gimplifier is responsible for this semantic detail of GENERIC (that
matches C for its sequence point rules).

Can you explain the rationale why the behavior is not simply undefined?
The sequence point before the call does not make the evaluation order
of the assignment operands defined.

[Bug c++/48814] Incorrect scalar increment result

[schaub.johannes at googlemail dot com]

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

--- Comment #2 from Johannes Schaub <schaub.johannes at googlemail dot com> 2011-04-29 10:42:12 UTC ---
Since the order of evaluation is undefined it may evaluate "count++" and
"incr()" in any order, as it pleases. 

Since there is a sequence point before entering a function, and before leaving
a function, and since function invocations do not interleave, we know that no
matter whether we evaluate "count++" before or after executing "incr", the side
effects of both "count++" and "++count" do not happen without an intervening
sequence point.

Sequence points have that effect of guaranteeing that side effects of previous
evaluations have been finished, and side effects of subsequent evaluations have
not yet occured.

Undefined behavior would occur if the two side effects would occur without an
intervening sequence point. For example, if we were to replace "incr()" by a
plain "++count".

[Bug c++/48814] Incorrect scalar increment result

[joseph at codesourcery dot com]

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

--- Comment #3 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2011-04-29 12:03:14 UTC ---
This may well be a bug, but it's the sort of case where you want an 
analysis not in terms of sequence points but in terms of the more 
precisely defined sequencing model in C++0x and C1x.  Do you have such an 
analysis?

[Bug c++/48814] Incorrect scalar increment result

[joseph at codesourcery dot com]

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

--- Comment #4 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2011-04-29 12:13:46 UTC ---
I think the relevant wording in the C1X DIS is "With respect to an 
indeterminately-sequenced function call, the operation of postfix ++ is a 
single evaluation."; C++ N3291 has the same wording.

[Bug c++/48814] Incorrect scalar increment result

[schaub.johannes at googlemail dot com]

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

--- Comment #5 from Johannes Schaub <schaub.johannes at googlemail dot com> 2011-04-29 16:20:44 UTC ---
(In reply to comment #4)
> I think the relevant wording in the C1X DIS is "With respect to an 
> indeterminately-sequenced function call, the operation of postfix ++ is a 
> single evaluation."; C++ N3291 has the same wording.

Yes, I agree. This makes it clearer than my C++03 description using sequence
points that GCC is in error.

[Bug middle-end/48814] [4.3/4.4/4.5/4.6/4.7 Regression] Incorrect scalar increment result

[jsm28 at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011.05.02 14:53:42
          Component|c++                         |middle-end
   Target Milestone|---                         |4.3.6
            Summary|Incorrect scalar increment  |[4.3/4.4/4.5/4.6/4.7
                   |result                      |Regression] Incorrect
                   |                            |scalar increment result
     Ever Confirmed|0                           |1

--- Comment #6 from Joseph S. Myers <jsm28 at gcc dot gnu.org> 2011-05-02 14:53:42 UTC ---
Confirming as a gimplifier bug, a regression from 4.0 onwards (presumably
introduced with tree-ssa).

[Bug middle-end/48814] [4.3/4.4/4.5/4.6/4.7 Regression] Incorrect scalar increment result

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-05-02 15:35:01 UTC ---
(In reply to comment #2)
> Since the order of evaluation is undefined it may evaluate "count++" and
> "incr()" in any order, as it pleases. 
> 
> Since there is a sequence point before entering a function, and before leaving
> a function, and since function invocations do not interleave, we know that no
> matter whether we evaluate "count++" before or after executing "incr", the side
> effects of both "count++" and "++count" do not happen without an intervening
> sequence point.

The side-effects yes, but the read in count++ happens at either before
or after incr is executed.  At least that is my reading of undefined
evaluation order.  Does the sequence point before entering a function
make that evaluation order suddenly defined?

Of course since Joseph seems to agree I won't object to fixing it.

[Bug middle-end/48814] [4.3/4.4/4.5/4.6/4.7 Regression] Incorrect scalar increment result

[joseph at codesourcery dot com]

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

--- Comment #8 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2011-05-02 16:24:01 UTC ---
On Mon, 2 May 2011, rguenth at gcc dot gnu.org wrote:

> The side-effects yes, but the read in count++ happens at either before
> or after incr is executed.  At least that is my reading of undefined
> evaluation order.  Does the sequence point before entering a function
> make that evaluation order suddenly defined?

The sequence point wording in C99 and C++03 is inherently confusing and 
doesn't go into much detail about what may or may not interleave, but what 
C1x and C++0x make explicit is that the evaluation of ++, -- and compound 
assignment do not interleave with an indeterminately-sequenced function 
call (and so they may not act by calling the indeterminately sequenced 
function between the read and the write of the operand being updated).  
You can argue about what exactly C99 and C++03 require (and so about 
whether there is a regression), but there is quite clearly now a bug to be 
fixed (and the fix should not depend on the language version).

[Bug middle-end/48814] [4.3/4.4/4.5/4.6/4.7 Regression] Incorrect scalar increment result

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug middle-end/48814] [4.3/4.4/4.5/4.6/4.7 Regression] Incorrect scalar increment result

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.3.6                       |4.4.7

--- Comment #9 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-06-27 12:14:19 UTC ---
4.3 branch is being closed, moving to 4.4.7 target.

[Bug middle-end/48814] [4.4/4.5/4.6/4.7 Regression] Incorrect scalar increment result

[rguenth at gcc dot gnu.org]

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

--- Comment #10 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-09 13:03:25 UTC ---
For

extern void abort (void);
struct S { int i; };
struct S arr[32];
volatile int count = 0;

struct S __attribute__((noinline))
incr (void)
{
  ++count;
}

int main()
{
  arr[count++] = incr ();
  if (count != 2)
    abort ();
  return 0;
}

we can only avoid reading 'count' too many times (once to get at the
index for the store and once for updating its value) if we can
insert a statement inbetween the call and the store to arr[].  Which
we can do only if we are introducing an aggregate temporary - which
might work in C, but does not work in C++ when we require
return-slot-optimization.  Thus, in the face of volatiles and required RSO
this is unfixable.

[Bug middle-end/48814] [4.5/4.6/4.7/4.8 Regression] Incorrect scalar increment result

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.4.7                       |4.5.4

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-03-13 12:47:46 UTC ---
4.4 branch is being closed, moving to 4.5.4 target.

[Bug middle-end/48814] [4.5/4.6/4.7/4.8 Regression] Incorrect scalar increment result

[rguenth at gcc dot gnu.org]

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

--- Comment #12 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-03-16 11:48:54 UTC ---
Author: rguenth
Date: Fri Mar 16 11:48:48 2012
New Revision: 185465

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=185465
Log:
2012-03-16  Richard Guenther  <rguenther@suse.de>
    Kai Tietz  <ktietz@redhat.com>

    PR middle-end/48814
    * gimplify.c (gimplify_self_mod_expr): Evaluate postfix
    side-effects completely in the pre-queue and use a temporary
    for the result.

    * gcc.c-torture/execute/pr48814-1.c: New test.
    * gcc.c-torture/execute/pr48814-2.c: New test.
    * gcc.dg/tree-ssa/assign-1.c: New test.
    * gcc.dg/tree-ssa/assign-2.c: New test.
    * gcc.dg/tree-ssa/assign-3.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr48814-1.c
    trunk/gcc/testsuite/gcc.c-torture/execute/pr48814-2.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/assign-1.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/assign-2.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/assign-3.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/gimplify.c
    trunk/gcc/testsuite/ChangeLog

[Bug middle-end/48814] [4.5/4.6/4.7 Regression] Incorrect scalar increment result

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
      Known to work|                            |4.8.0
         Resolution|                            |FIXED
   Target Milestone|4.5.4                       |4.8.0
            Summary|[4.5/4.6/4.7/4.8            |[4.5/4.6/4.7 Regression]
                   |Regression] Incorrect       |Incorrect scalar increment
                   |scalar increment result     |result
      Known to fail|                            |4.7.1

--- Comment #13 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-03-16 11:49:39 UTC ---
Fixed.  Deliberately not backporting.

[Bug middle-end/48814] [4.5/4.6/4.7 Regression] Incorrect scalar increment result

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |alexcheremkhin at gmail dot
                   |                            |com

--- Comment #14 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-06-14 17:55:29 UTC ---
*** Bug 53656 has been marked as a duplicate of this bug. ***

[Bug middle-end/48814] [4.5/4.6/4.7 Regression] Incorrect scalar increment result

[nmm1 at cam dot ac.uk]

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

Nick Maclaren <nmm1 at cam dot ac.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nmm1 at cam dot ac.uk

--- Comment #15 from Nick Maclaren <nmm1 at cam dot ac.uk> 2013-03-15 23:20:56 UTC ---
I must correct some of the statements here.  This is not a bug, and the
intent of WG14 during the standardisation of C90 was that this should be
undefined behaviour.  This point was raised explicitly - unfortunately,
those of us who wanted this area to be better specified did not
prevail:-(

This is a CHANGE in the C language introduced by C 2011.  gcc may wish
to adopt that, but that is not the same as saying this is a bug.  As
it merely degrades optimisation and does not break any working programs,
it is not a major matter.

The only reason that I am posting this after the decision has been taken
is that there are other changes introduced by C11 that may not be so
benign.  There are many less than in C99, but still quite a lot.  This
should not be used as precedent.