Possible bug in v12

[goran_ceric]

We may have identified a compiler bug specific to the Intel v12 compiler.

test program is as follows. Aficionados will recognize it's a code fragment derived from the widely used Mersenne Twister random number generator; We expect the bug to manifest in many implementations of the MT:

---

#include

#include

#include

#include

int

main(void)

{

uint32_t mt[24];

uint32_t y;

int z;

uint32_t mag01[2] = { 0x0, 0x9908b0df };

for (z = 0; z < 24; z++) mt = z;

for (z = 0; z < 12; z++)

{

y = (mt & 0x80000000) | (mt[z+1] & 0x7fffffff);

mt = y ^ mag01[y & 0x1];

}

if (mt[0] == 2567483614) printf("correct\\n");

else if (mt[0] == 1) printf("FAIL\\n");

}

---

icc -O -o bug bug.c; ./bug

FAIL

icc -g -o bug bug.c; ./bug

correct

The problem is at mag01[y & 0x1]

We believe that the Intel compiler *may* (depending on optimization level and surrounding code context) be attempting to cast the unsigned y to a signed int before it evaluates y&0x1. Casting a large unsigned int to signed int may overflow, and in C99, this overflow behavior is undefined; I believe here it's resulting in a 0 on overflow.

That's really bad here, because this mag01[ y&0x1] incantation is an efficient (avoiding a conditional branch) implementation of the logic:

if (y is odd) mt = y ^ 0x9908b0df;

so if y is a large odd number, and icc casts it to signed int, overflows, and results in 0, then it's changing y from odd to even, and the result of the algorithm is changed.

Our understanding of the C99 standard is that this would be a compiler bug. The standard says that array indices may be any integer type -- signed or unsigned. The compiler shouldn't be casting y to signed int. We've fixed the problem by using mag01[(int) (y & 0x1)], to force the order of evaluation the way it should be, but I'd like to know if our interpretation is correct...

[mecej4]

The 12.1 versions of the compiler (IA32 and Intel64) both return "correct", regardless of which optimization options from -O, -O2, -O3 are used.

What is the complete version number of the compiler that you used for which "FAIL" was printed out?

[goran_ceric]

Version 12.1.0.233 Build 20110811 installed as part of cluster studio

Prompted by your response, I found aVersion 12.1.1.256 Build 20111011 installed as part of composerxe install and when compiled with this version, it returns "correct".

Thanks

[SergeyKostrov]

>>...attempting to cast the unsigned y to a signed int before it evaluates y&0x1. Casting a large unsigned int
>>to signed int may overflow, and in C99, this overflow behavior is undefined;

Could you imaging a case like:

...
somearray[ ( int )-128 ] = -128;
...

Even if a C/C++compiler could compile it, possibly with some warning, it is the example of out of bound
access and anAccess Violation exception could happen. But, it could also work if there is an another
array located in memorybefore somearray[xxx].

I'm not suprised that problems related to acast'unsigned int' ->'int' are "alive". I hadsimilar
problems with Watcom C/C++ compiler v7.xmany years agoand withMinGW v3.4.2about two
weeks ago.

Best regards,
Sergey

[goran_ceric]

I am not sure what you are getting at.C standard allows negative indices.

[SergeyKostrov]

>>...C standard allows negative indices...

Eventually itwill cause an Access Violation in an application. Take a look at two screenshots below.

Screenshot 1:



Screenshot 2:

[SergeyKostrov]

By the way, MinGW v3.4.2 gives a warning on that line of code ( on a 32-bit platform ):

...
if( mt[0] == 2567483614 )
...

"Warning: This decimal constant is unsigned only in ISO C90"

Note:'2567483614' is greater than '2147483648' ( 2^31 ).

[goran_ceric]

Thanks Sergey.

Sure, you can have a negative array index that causes a pointer to get dereferenced out of bounds. But you can also have a negative array index into legit memory because you know what you're doing in memory. C99 standard explicitly allows negative array indices... (they must be "integral" types, signed or unsigned).

In any case, what I am interested in here is why the compiler is apparently erroneously casting a large positive signed value to unsigned (which can create a negative number) before the binary "and" operation that is supposed to convert the index to 0/1. And it seems to be very version specific when it comes to Intel compiler. I'll see if I can chase this down with their support.

[SergeyKostrov]

>>...Sure, you can have a negative array index that causes a pointer to get dereferenced out of bounds. But
>>you can also have a negative array index into legit memory because you know what you're doing in
>>memory...

Exactly! This is some kind of "hacking technique".

>>...why the compiler is apparently erroneously casting a large positive signed value to unsigned (which can
>>create a negative number)... I'll see if I can chase this down with their support...

I'm afraid that we still don't hear anything from Intel's software engineers. I thinkyou need to
provide two fragments of assembler codes from different versions ofIntelC/C++ compiler to prove
the statementa "possible bug".

Best regards,
Sergey

[ILevi1]

Sergey,

Using negative array indices is not hacking.

For example, if you have:

char p[200];

Then p is always pointing to the array start and accessing p[-5] will give access violation.

But if you have:

char *pp = &p[100];

Then writing:

pp[-5] = 'X';

Is perfectly valid code.

In other words, you need to remember that array pointer does not always have to point at the beginning of an array.

I tested the code with Intel Compiler 12.1.1.062 Build 20111011 (Windows), and regardless of optimization options I always get "correct" as an answer. To compile on Windows I had to remove #include and add typedef unsigned int uint32_t.

[SergeyKostrov]

Agree. I forgot about that way of accessing an array. Just testedyour Test-Caseand it works.

Best regards,
Sergey

[goran_ceric]

Thanks Igor. I will open a case with support re:Version 12.1.0.233 Build 20110811. All I want at this point is for them to say that it is/was a bug and that it was fixed in later version or provide some kind of an explanation if they don't think it's a bug.