Bug 38929 - Optimisation with inline function causes invalid behaviour
Optimisation with inline function causes invalid behaviour
Status: RESOLVED DUPLICATE of bug 35634
Product: gcc
Classification: Unclassified
Component: c
4.3.2
: P3 normal
: ---
Assigned To: Not yet assigned to anyone
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-01-21 16:42 UTC by Neil Brown
Modified: 2009-01-21 23:30 UTC (History)
8 users (show)

See Also:
Host: i486-linux-gnu
Target: i486-linux-gnu
Build: i486-linux-gnu
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 Neil Brown 2009-01-21 16:42:35 UTC
I have a program with an addition function for int8_t that is intended to check for overflow.  Regardless of whether the test works, I get different results from the program if I compile it with -O1 vs compiling with -O2.  It appears that GCC is performing optimisations it shouldn't across the process boundaries that lead to the wrong behaviour.  In particular, GCC is effectively saying that it has the value 128 in an int8_t.  This is not specifically a printf problem, as the overflow is not set with -O1, and is set with -O2.  Running with -O1, the program runs all the tests successfully.  With -O2, there is a failure, where GCC says it is adding the int8_t values 0 and 128 during the loop.  Compare that with the earlier lines where it correctly says it is trying to add 0 and -128 in the standalone tests.  Here's the complete code of the reduced test-case (the .i file is no different apart from the included headers, and the includes are all standard C headers):

#include <stddef.h>
#include <stdbool.h>
#include <stdint.h>
#include <limits.h>
#include <stdio.h>

int g_overflow;

static inline int8_t add_int8_t (int8_t, int8_t, const char *);
static inline int8_t add_int8_t (int8_t a, int8_t b, const char *pos) {
	printf("int8_t : adding %d and %d\n", (int)a, (int)b);
	if (((b<1)&&(-128-b<=a)) || ((b>=1)&&(127-b>=a))) {
		return a + b;
	} else {
		g_overflow = 1; return 0;
	}
}

int main(int argc, char** argv)
{
	g_overflow = 0;
	const int8_t result = add_int8_t(-128,0,"");
	printf("-128 + 0, result: %d, overflow: %d", (int)result, g_overflow);

	g_overflow = 0;
	const int8_t resultB = add_int8_t(0,-128,"");
	printf("-128 + 0, result: %d, overflow: %d", (int)resultB, g_overflow);

	// Test commutativity:
	int done_x_once = 0;
	for (int8_t x = 0; !(x == 0 && done_x_once == 1);x++) {
		done_x_once = 1;
		int done_y_once = 0;
		for (int8_t y = 0; !(y == 0 && done_y_once == 1);y++) {
			done_y_once = 1;
			
			g_overflow = 0;
			const int8_t r0 = add_int8_t(x,y,"");
			const int overflow_earlier = g_overflow;
			
			g_overflow = 0;
			const int8_t r1 = add_int8_t(y,x,"");
			const int overflow_later = g_overflow;
			
			if ((overflow_later == 1 && overflow_earlier == 1) ||
			  (overflow_earlier == 0 && overflow_later == 0 && r0 == r1)) {
				//Passed
			} else {
				printf("Comm failed, non-commutative with %d, %d (overflow: %d, %d) (res: %d, %d)\n",
				  (int)x, (int)y, overflow_earlier, overflow_later, r0, r1);
				goto failed_comm;
			}
		}
	}
	failed_comm:;
	
	
	return 0;
}


Here is my gcc -v output (I'm using the latest standard GCC on Ubuntu):

Using built-in specs.
Target: i486-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 4.3.2-1ubuntu11' --with-bugurl=file:///usr/share/doc/gcc-4.3/README.Bugs --enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr --enable-shared --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --enable-nls --with-gxx-include-dir=/usr/include/c++/4.3 --program-suffix=-4.3 --enable-clocale=gnu --enable-libstdcxx-debug --enable-objc-gc --enable-mpfr --enable-targets=all --enable-checking=release --build=i486-linux-gnu --host=i486-linux-gnu --target=i486-linux-gnu
Thread model: posix
gcc version 4.3.2 (Ubuntu 4.3.2-1ubuntu11)

Command-line to get correct behaviour (pasting the above code into gccbug.c) is: gcc -std=gnu99 gccbug.c -O1
Command-line to trigger the bug is: gcc -std=gnu99 gccbug.c -O2

There is no compiler output, and adding -Wall still gives no compiler output.
Comment 1 Neil Brown 2009-01-21 17:03:44 UTC
I tried fiddling with various optimisations.  With -O1 the code works fine.  With -O2 the code breaks.  With -O2 -fno-tree-vrp it works fine again, so that may help in finding the bug.
Comment 2 Neil Brown 2009-01-21 17:13:27 UTC
The -fno-tree-vrp flag does stop the problem on the testcase I submitted.  However, on the full program I drew the testcase from, it only modifies the behaviour to a different invalid problem, so it is perhaps an interaction of optimisations.
Comment 3 Neil Brown 2009-01-21 17:36:09 UTC
The main optimisation that seems to affect this example, is, -fstrict-overflow.  So to reiterate:

Works: gcc gccbug.c -std=gnu99 -O1
Doesn't work: gcc gccbug.c -std=gnu99 -O2
Doesn't work: gcc gccbug.c -std=gnu99 -O2 -fno-strict-overflow
Works: gcc gccbug.c -O2 -std=gnu99 -O2 -fno-strict-overflow -fno-tree-vrp

I can see that the assumptions involved in strict-overflow may cause the overflow test to behave differently, but it is still the case that under the optimisations, one printf says -128 for the int8_t value, and the other (when passed in the same value from a different call) says 128, and therefore I am still confident that this is a bug.
Comment 4 Richard Biener 2009-01-21 20:26:50 UTC
You overflow x and y in main() which invokes undefined behavior.
Comment 5 joseph@codesourcery.com 2009-01-21 23:27:38 UTC
Subject: Re:  Optimisation with inline function causes invalid
 behaviour

On Wed, 21 Jan 2009, rguenth at gcc dot gnu dot org wrote:

> You overflow x and y in main() which invokes undefined behavior.

Actually, I don't think this is undefined behavior; I think it's the same 
as bug 35634.  There is no arithmetic on int8_t in C; the values are 
promoted to int (defined), arithmetic is done on int (which doesn't 
overflow for the promoted values) and if the values are then stored back 
in int8_t they are converted back (where the implementation-defined 
conversions are documented, and it's implementation-defined not 
undefined).  But we know in bug 35634 that GCC is wrongly treating 
increment/decrement of types such as int8_t as having undefined behavior, 
by not representing the intermediate promotions and truncations.

Comment 6 Andrew Pinski 2009-01-21 23:28:25 UTC
Reopening to ...
Comment 7 Andrew Pinski 2009-01-21 23:30:14 UTC
Yes it is a dup, replacing ++ with +=1 allows this to work correctly.

*** This bug has been marked as a duplicate of 35634 ***