On Mon, May 13, 2013 at 2:20 PM, LANGLOIS Olivier PIS -EXT
<olivier.pis.langlois@xxxxxxxxxxxxxxxxxxxx> wrote:
> I have just been hit by something:
>
> lano1106@hpmini ~/dev/gcc-test $ g++ --version
> g++ (GCC) 4.8.0 20130502 (prerelease)
> Copyright (C) 2013 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
> lano1106@hpmini ~/dev/gcc-test $ g++ -O2 -o test1 test1.cpp test1_init.cpp
> lano1106@hpmini ~/dev/gcc-test $ ./test1
> item 0
> a: 1
> lano1106@hpmini ~/dev/gcc-test $ g++ -O1 -o test1 test1.cpp test1_init.cpp
> lano1106@hpmini ~/dev/gcc-test $ ./test1
> item 0
> a: 1
> lano1106@hpmini ~/dev/gcc-test $ g++ -O0 -o test1 test1.cpp test1_init.cpp
> lano1106@hpmini ~/dev/gcc-test $ ./test1
> item 0
> a: 1
> item 1
> a: 2
> lano1106@hpmini ~/dev/gcc-test $ cat test1.h
>
> struct A
> {
> int a;
> int b;
> int c;
> };
>
> struct B
> {
> int numelem;
> /*
> * Old C trick to define a dynamically sizable array just by allocating
> * sizeof(B) + (numelem-1)*sizeof(A) memory.
> */
> A item[1];
> };
>
> void initArr(B *p);
>
> lano1106@hpmini ~/dev/gcc-test $ cat test1_init.cpp
> #include "test1.h"
>
> void initArr(B *p)
> {
> p->numelem = 2;
> p->item[0].a = 1;
> p->item[1].a = 2;
> }
>
> lano1106@hpmini ~/dev/gcc-test $ cat test1.cpp
> /*
> * Author: Olivier Langlois <olivier@xxxxxxxxxxxxxx>
> *
> * Purpose: Small test to highlight gcc optimization bug
> */
>
> #include <stdio.h>
> #include <string.h>
> #include "test1.h"
>
> /*
> * Create a B array with the intent of only using the first item.
> * The 19 other items sole purpose is to create a buffer large enough
> * to accomodate A array needs.
> */
> #define MAXBLEN 20
>
> int main(int argc, char *argv[])
> {
> B arr[MAXBLEN];
> memset(arr,0,sizeof(arr));
>
> initArr(arr);
>
> for( int i = 0; i < arr[0].numelem; ++i )
> {
> printf( "item %d\n"
> " a: %d\n",
> i,
> arr[0].item[i].a);
> }
>
> return 0;
> }
>
> From gcc website, this is not a bug:
>
> Loops do not terminate
>
> This is often caused by out-of-bound array accesses or by signed integer overflow which both result in undefined behavior according to the ISO C standard. For example
>
> int
> SATD (int* diff, int use_hadamard)
> {
> int k, satd = 0, m[16], dd, d[16];
> ...
> for (dd=d[k=0]; k<16; dd=d[++k])
> satd += (dd < 0 ? -dd : dd);
>
> accesses d[16] before the loop is exited with the k<16 check. This causes the compiler to optimize away the exit test because the new value of k must be in the range [0, 15] according to ISO C.
>
> GCC starting with version 4.8 has a new option -fno-aggressive-loop-optimizations that may help here. If it does, then this is a clear sign that your code is not conforming to ISO C and it is not a GCC bug.
>
> I am surprised that I didn't hit the problem before but I am seriously considering using '-fno-aggressive-loop-optimizations' in my own makepkg.conf. I just want to test others feeling on this discovery to see if it wouldn't be a good idea to make the switch standard in Arch...
The only time the switch makes a difference is when the program is
already incorrect. I really doubt Arch is going to enable a flag
slowing down all programs to make invalid programs behave
*differently* (not necessary as they were intended to behave, just
*differently*).
GCC is correctly noticing a situation that would result in undefined
behaviour, and optimizing based on the assumption that it never
happens. The solution is to write valid code not relying on undefined
behaviour - accessing beyond the end of an array is undefined
behaviour regardless of whether there's more allocated memory.
C99 has this feature as a flexible-length array member using `foo
array[];` and that might be valid C++11 but I'm not sure and I don't
feel like digging through the standard. Using `foo array[0]` will also
work because it's a GNU extension, but keep in mind that you've left
the land of standard C++.
Compilers are going to get better and better at optimizing away code
that's not needed if the program is assumed to be correct. I recommend
using another language if you don't want to worry about incorrect code
that seems to work now breaking from future optimizations.
