[haiku-development] Re: Disabling Strict Aliasing for GCC4 Builds

A follow up

Sorry, turned out not to be as short as I originally though. If you 
don't really care about strict aliasing, you can safely skip this 
message...

Real world example of a strict aliasing rule violation causing 
unexpected behaviour: the Chart demo app in Haiku. When using Chart on 
a GCC4 compiled Haiku that has strict aliasing enabled (i.e. prior to 
r25016), you will notice that it doesn't draw anything (testing under 
QEMU at least). When compiling Chart there are three warnings 
concerning a break of strict aliasing rules. One is in ChartWindow.cpp 
with the code:

BHandler *handler;
...
message->FindPointer("source", (void**)&handler);

This doesn't really look too wrong and in practice apparently doesn't 
hinder functionallity either. I just show it here to point out what I 
find quite limiting about strict aliasing rules. The "correct" way (and 
this is only a GCC extension it turns out and not even officially 
allowed by the standard) of doing that would involve doing the type 
punning from BHandler ** to void ** for the FindPointer() call through 
a union. This would result in the following code:

        union {
                BHandler *handlerPointer;
                void *voidPointer;
        } u;

        message->FindPointer("source", &u.voidPointer);

And every instance where the handler pointer is accessed would then of 
course need to be "u.handlerPointer". One could alternatively copy the 
handler pointer out of the union after the call though so no further 
changes to the function would be necessary.

Now this is not what is causing the drawing problems anyway. The cause 
for that resides in ChartRender.cpp and is an implementation of a 
simple approximation square root function:

float b_sqrt(float x) {
        uint32 val;
        float y,z,t;
        float flottant, tampon;

        flottant = x;
        val = *((uint32*)&flottant);
        val >>= 1;
        val += 0x1FC00000L;
        *((uint32*)&tampon) = val;
        y = tampon;
        z = y*y+x;
        t = y*y-x;
        y *= (float)4.0;
        x = z*z;
        t = t*t;
        y = z*y;
        t = (float)2.0*x-t;
        return t/y;
}

This code does break strict aliasing in two places (it also breaks 
coding style guidelines, but it's Be sample code...). First when 
retrieving the uint32 representation of the float through "val = *
((uint32*)&flottant);" and the second time when setting the float 
variable to the modified uint32 value again "*((uint32*)&tampon) = 
val;". This generates the other two warnings. Appart from obviously 
depending on a certain in-memory layout of a float, this code does 
indeed break when strict aliasing is enabled. The compiler obviously 
optimizes this function with the strict aliasing assumption which 
causes these two sections to fail. Note that the uint32 variable "val" 
and the float variables are not actually aliasing eachother, they are 
stored in different memory locations. What is breaking the rules is the 
"*((uint32 *)&flottant" part, as it makes a float pointer pointing to a 
memory location (&flottant), and then makes a uint32 pointer out of it 
pointing to the same memory location. This results in two pointers of 
different (and incompatible) types pointing to the same location, which 
obviously breaks the assumption that two pointers of different type 
cannot do exactly that.

The above could be corrected in some different ways. For example a 
union approach could be used where the float and the uint32 variables 
would simply be put into the same union and then either of the union 
members is modified:

{
        ...
        union {
                float flottant;
                uint32 val;
        } u;

        u.flottant = x;
        u.val >>= 1;
        u.val += 0x1FC00000L;
        y = u.flottant;
        ...
}

Which actually simplifies the code even and therefore is probably a 
good thing to do anyway.
One could probably also implement it as actually moving around the 
memory locations. For example using a macro around memcpy to make 
things a bit handier:

#define COPY_VALUES(x, y) \
        memcpy(&x, &y, MIN(sizeof(x), sizeof(y)))

And then use it like:

{
        ...
        flottant = x;
        COPY_VALUES(val, flottant);
        val >>= 1;
        val += 0x1FC00000L;
        COPY_VALUES(tampon, val);
        y = tampon;
        ...
}

But I am in fact not really sure if this doesn't just hide the problem 
(as it works I suppose it doesn't).

As you see there are ways to get these situations corrected. But as 
should be obvious from the first example of the FindPointer() method, 
we will have lots of these cases. And I am not yet convinced that I 
really like to do that kind of workaround in such common code.

Regards
Michael

Other related posts: