Dereferencing type-punned pointer will break strict-aliasing rules

Question

I used the following piece of code to read data from files as part of a larger program.

double data_read(FILE *stream,int code) {
        char data[8];
        switch(code) {
        case 0x08:
            return (unsigned char)fgetc(stream);
        case 0x09:
            return (signed char)fgetc(stream);
        case 0x0b:
            data[1] = fgetc(stream);
            data[0] = fgetc(stream);
            return *(short*)data;
        case 0x0c:
            for(int i=3;i>=0;i--)
                data[i] = fgetc(stream);
            return *(int*)data;
        case 0x0d:
            for(int i=3;i>=0;i--)
                data[i] = fgetc(stream);
            return *(float*)data;
        case 0x0e:
            for(int i=7;i>=0;i--)
                data[i] = fgetc(stream);
            return *(double*)data;
        }
        die("data read failed");
        return 1;
    }

Now I am told to use -O2 and I get following gcc warning: warning: dereferencing type-punned pointer will break strict-aliasing rules

Googleing I found two orthogonal answers:

• Concluding: there's no need to worry; gcc tries to be more law obedient than the actual law.

vs

• So basically if you have an int* and a float* they are not allowed to point to the same memory location. If your code does not respect this, then the compiler's optimizer will most likely break your code.

In the end I don't want to ignore the warnings. What would you recommend?

[update] I substituted the toy example with the real function.

Answer 1

This doc summarizes the situation: http://dbp-consulting.com/tutorials/StrictAliasing.html

There are several different solutions there, but the most portable/safe one is to use memcpy(). (The function calls may be optimized out, so it's not as inefficient as it appears.) For example, replace this:

return *(short*)data;

With this:

short temp;
memcpy(&temp, data, sizeof(temp));
return temp;

Answer 2

The authors of the C Standard wanted to let compiler writers generate efficient code in circumstances where it would be theoretically possible but unlikely that a global variable might have its value accessed using a seemingly-unrelated pointer. The idea wasn't to forbid type punning by casting and dereferencing a pointer in a single expression, but rather to say that given something like:

int x;
int foo(double *d)
{
  x++;
  *d=1234;
  return x;
}

a compiler would be entitled to assume that the write to *d won't affect x. The authors of the Standard wanted to list situations where a function like the above that received a pointer from an unknown source would have to assume that it might alias a seemingly-unrelated global, without requiring that types perfectly match. Unfortunately, while the rationale strongly suggests that authors of the Standard intended to describe a standard for minimum conformance in cases where a compiler would otherwise have no reason to believe that things might alias, the rule fails to require that compilers recognize aliasing in cases where it is obvious and the authors of gcc have decided that they'd rather generate the smallest program it can while conforming to the poorly-written language of the Standard, than generate code which is actually useful, and instead of recognizing aliasing in cases where it's obvious (while still being able to assume that things that don't look like they'll alias, won't) they'd rather require that programmers use memcpy, thus requiring a compiler to allow for the possibility that pointers of unknown origin might alias just about anything, thus impeding optimization.

Answer 3

It looks a lot as if you really want to use fread:

int data;
fread(&data, sizeof(data), 1, stream);

That said, if you do want to go the route of reading chars, then reinterpreting them as an int, the safe way to do it in C (but not in C++) is to use a union:

union
{
    char theChars[4];
    int theInt;
} myunion;

for(int i=0; i<4; i++)
    myunion.theChars[i] = fgetc(stream);
return myunion.theInt;

I'm not sure why the length of data in your original code is 3. I assume you wanted 4 bytes; at least I don't know of any systems where an int is 3 bytes.

Note that both your code and mine are highly non-portable.

Edit: If you want to read ints of various lengths from a file, portably, try something like this:

unsigned result=0;
for(int i=0; i<4; i++)
    result = (result << 8) | fgetc(stream);

(Note: In a real program, you would additionally want to test the return value of fgetc() against EOF.)

This reads a 4-byte unsigned from the file in little-endian format, regardless of what the endianness of the system is. It should work on just about any system where an unsigned is at least 4 bytes.

If you want to be endian-neutral, don't use pointers or unions; use bit-shifts instead.

Answer 4

The problem occurs because you access a char-array through a double*:

char data[8];
...
return *(double*)data;

But gcc assumes that your program will never access variables though pointers of different type. This assumption is called strict-aliasing and allows the compiler to make some optimizations:

If the compiler knows that your *(double*) can in no way overlap with data[], it's allowed to all sorts of things like reordering your code into:

return *(double*)data;
for(int i=7;i>=0;i--)
    data[i] = fgetc(stream);

The loop is most likely optimized away and you end up with just:

return *(double*)data;

Which leaves your data[] uninitialized. In this particular case the compiler might be able to see that your pointers overlap, but if you had declared it char* data, it could have given bugs.

But, the strict-aliasing rule says that a char* and void* can point at any type. So you can rewrite it into:

double data;
...
*(((char*)&data) + i) = fgetc(stream);
...
return data;

Strict aliasing warnings are really important to understand or fix. They cause the kinds of bugs that are impossible to reproduce in-house because they occur only on one particular compiler on one particular operating system on one particular machine and only on full-moon and once a year, etc.

Answer 5

Using a union is not the correct thing to do here. Reading from an unwritten member of the union is undefined - i.e. the compiler is free to perform optimisations that will break your code (like optimising away the write).

Answer 6

Apparently the standard allows sizeof(char*) to be different from sizeof(int*) so gcc complains when you try a direct cast. void* is a little special in that everything can be converted back and forth to and from void*. In practice I don't know many architecture/compiler where a pointer is not always the same for all types but gcc is right to emit a warning even if it is annoying.

I think the safe way would be

int i, *p = &i;
char *q = (char*)&p[0];

or

char *q = (char*)(void*)p;

You can also try this and see what you get:

char *q = reinterpret_cast<char*>(p);

Answer 7

Basically you can read gcc's message as guy you are looking for trouble, don't say I didn't warn ya.

Casting a three byte character array to an int is one of the worst things I have seen, ever. Normally your int has at least 4 bytes. So for the fourth (and maybe more if int is wider) you get random data. And then you cast all of this to a double.

Just do none of that. The aliasing problem that gcc warns about is innocent compared to what you are doing.