GCC Bugzilla – Bug 33270
Last modified: 2007-08-31 19:30:51 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:
int inca_and_ret1(int *p_a)
int a = 0;
int *p_a = &a;
(*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 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.
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:
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
*** This bug has been marked as a duplicate of 11751 ***
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
> 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.
> 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).
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 ***
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.