Bug 33270 - Wrong evaluation
Wrong evaluation
Status: RESOLVED DUPLICATE of bug 11751
Product: gcc
Classification: Unclassified
Component: c
4.1.2
: P3 normal
: ---
Assigned To: Not yet assigned to anyone
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-08-31 15:05 UTC by Tim Bruylants
Modified: 2007-08-31 19:30 UTC (History)
56 users (show)

See Also:
Host:
Target: i486-linux-gnu
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Bruylants 2007-08-31 15:05:55 UTC
The following code generates a "1" with gcc-4.1 and generates a "2" with a lot of other compilers (Visual Studio, gcc-3.2, ...). I have read the non-bugs section on http://gcc.gnu.org/bugs.html (also bug 11751) and I have read about "sequence points" on http://c-faq.com/expr/seqpoints.html. Still I feel that this is really a bug rather than a non-bug as in the many examples. Why? Because a function call is also a sequence point.

Here is a small code snippet, that is compiled with no special settings:

#include <stdio.h>

int inca_and_ret1(int *p_a)
{
  ++(*p_a);
  return 1;
}

int main()
{
  int a = 0;
  int *p_a = &a;

  (*p_a) += inca_and_ret1(p_a);

  printf("%d\n", a);

  return 0;
}

The function call to inca_and_ret1 is a sequence point, this the value pointed to by p_a should be correct after the call. And then the expression "(*p_a) += <return value>" should be evaluated (I think).

The example snippet I present here looks very simple and stupid, but I have a similar situation in my code and there it is far less obvious. I stumbled across this problem by accident and I have no clue how many of these problems exist in my code (or other people's code).

The command "gcc -v -save-temps -o bug bug.c" gave the following output:

Using built-in specs.
Target: i486-linux-gnu
Configured with: ../src/configure -v --enable-languages=c,c++,fortran,objc,obj-c++,treelang --prefix=/usr --enable-shared --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --enable-nls --program-suffix=-4.1 --enable-__cxa_atexit --enable-clocale=gnu --enable-libstdcxx-debug --enable-mpfr --enable-checking=release i486-linux-gnu
Thread model: posix
gcc version 4.1.2 (Ubuntu 4.1.2-0ubuntu4)
 /usr/lib/gcc/i486-linux-gnu/4.1.2/cc1 -E -quiet -v bug.c -mtune=generic -fpch-preprocess -o bug.i
ignoring nonexistent directory "/usr/local/include/i486-linux-gnu"
ignoring nonexistent directory "/usr/lib/gcc/i486-linux-gnu/4.1.2/../../../../i486-linux-gnu/include"
ignoring nonexistent directory "/usr/include/i486-linux-gnu"
#include "..." search starts here:
#include <...> search starts here:
 /usr/local/include
 /usr/lib/gcc/i486-linux-gnu/4.1.2/include
 /usr/include
End of search list.
 /usr/lib/gcc/i486-linux-gnu/4.1.2/cc1 -fpreprocessed bug.i -quiet -dumpbase bug.c -mtune=generic -auxbase bug -version -fstack-protector -fstack-protector -o bug.s
GNU C version 4.1.2 (Ubuntu 4.1.2-0ubuntu4) (i486-linux-gnu)
	compiled by GNU C version 4.1.2 (Ubuntu 4.1.2-0ubuntu4).
GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Compiler executable checksum: c0d954aeefbb96d795ff3f6b3b72ef51
 as -V -Qy -o bug.o bug.s
GNU assembler version 2.17.50 (i486-linux-gnu) using BFD version 2.17.50 20070103 Ubuntu
 /usr/lib/gcc/i486-linux-gnu/4.1.2/collect2 --eh-frame-hdr -m elf_i386 --hash-style=both -dynamic-linker /lib/ld-linux.so.2 -o bug /usr/lib/gcc/i486-linux-gnu/4.1.2/../../../../lib/crt1.o /usr/lib/gcc/i486-linux-gnu/4.1.2/../../../../lib/crti.o /usr/lib/gcc/i486-linux-gnu/4.1.2/crtbegin.o -L/usr/lib/gcc/i486-linux-gnu/4.1.2 -L/usr/lib/gcc/i486-linux-gnu/4.1.2 -L/usr/lib/gcc/i486-linux-gnu/4.1.2/../../../../lib -L/lib/../lib -L/usr/lib/../lib bug.o -lgcc --as-needed -lgcc_s --no-as-needed -lc -lgcc --as-needed -lgcc_s --no-as-needed /usr/lib/gcc/i486-linux-gnu/4.1.2/crtend.o /usr/lib/gcc/i486-linux-gnu/4.1.2/../../../../lib/crtn.o
Comment 1 Andrew Pinski 2007-08-31 16:09:10 UTC

*** This bug has been marked as a duplicate of 11751 ***
Comment 2 Ken Raeburn 2007-08-31 19:04:36 UTC
Subject: Re:   New: Wrong evaluation

On Aug 31, 2007, at 11:05, tim dot bruylants at vub dot ac dot be wrote:
> The following code generates a "1" with gcc-4.1 and generates a "2"  
> with a lot
> of other compilers (Visual Studio, gcc-3.2, ...). I have read the  
> non-bugs
> section on http://gcc.gnu.org/bugs.html (also bug 11751) and I have  
> read about
> "sequence points" on http://c-faq.com/expr/seqpoints.html. Still I  
> feel that
> this is really a bug rather than a non-bug as in the many examples.  
> Why?
> Because a function call is also a sequence point.

>   (*p_a) += inca_and_ret1(p_a);

> The function call to inca_and_ret1 is a sequence point, this the  
> value pointed
> to by p_a should be correct after the call. And then the expression  
> "(*p_a) +=
> <return value>" should be evaluated (I think).

The sequence point at the function call means that side-effects in  
the argument list have to be completed before the execution of the  
function body starts.  It doesn't force sequencing on other parts of  
the expression containing the call, specifically when the old value  
of the left-hand side of the "+=" would be read (or when the address  
is computed, if inca_and_ret1 could modify the pointer p_a itself).

Ken
Comment 3 Tim Bruylants 2007-08-31 19:05:04 UTC
Can someone please explain me why this behavior is correct according to the specifications? Isn't the function call issuing a new sequence point? Isn't the ++(*p_a); statement in the function a separate sequence point? Isn't the return a sequence point?

From what I can see is that the statement: "(*p_a) += inca_and_ret1(p_a);" is evaluated in an undefined order. That's ok. No problems there. Except that the (*p_a) is evaluated by already taking the value at that memory location and caching it somewhere, then calling the function, and then performing the math on the cached value, thus skipping the increment inside the function. It feels to me that it is a bug and that it is caused by internal caching of values rather than undefined evaluation order (which indeed is allowed).

And besides the discussion on this being correct behaviour or not, these kind of issues make some compilers (like GCC-4) a real pain to use safely. I know the example here is straight forward and that it is plain easy to see that this is not the best code, but I bumped into this with a much less obvious construction (involving a struct variable). Wouldn't at least a warning be a nice thing to generate in such cases?

Don't get me wrong, I'm not trying to start the discussion about evaluation order all over. I just think this has nothing to do with that.

I hope someone cares enough to explain this case to me and why GCC is correct (or not).(In reply to comment #1)
> 
> *** This bug has been marked as a duplicate of 11751 ***
> 

Comment 4 Tim Bruylants 2007-08-31 19:30:51 UTC
Thank you for this explanation.

A friend of me also found an explaining document on http://www.open-std.org/jtc1/sc22/wg14/www/docs/n926.htm. Looking at 12.4.1 seems to be the case that I am in... and it's undefined.

Kind regards,
Tim