Review Request: Fix KPixmapCache crash (probably because of pointer aliasing in unions)

[Johannes Sixt]

Michael Pyne schrieb:
> On Sunday 24 May 2009 20:09:23 Christoph Feck wrote:
>> Attempts to fix KPixmapCrash, pardon, KPixmapCache. This is my first
>> submission to reviewboard to learn how it works (and I already learned that
>> you have to retype the Description???).
>>
>> According to the comments in the (previous) code, there are problems using
>> the pointers in the union, so I removed them, and replaced them with a
>> single pointer and casts at the appropriate places.
>>
>>
>> This addresses bug 182026.
>>     https://bugs.kde.org/show_bug.cgi?id=182026
> 
> Well I lost my KWallet and thence my super-secure review board password.  
> Luckily for me there is absolutely no way I can find to reset my password so 
> I'll resort to good-old-fashioned mailing lists.
> 
> I wrote the code in question, including the immortal comment "This probably 
> breaks some C++ aliasing rule :-("
> 
> Turns out I was right =D
> 
> I had actually after writing the comment got myself under the impression that 
> the char* memory in the union made type-punning OK but apparently that doesn't 
> allow you to actually ever dereference the pointer and expect it to work.  
> Weird.
> 
> I say ship it

No, don't ship it. The new code is just as wrong as the old code was.

The new code uses the idiom

    *(some_type*)foo

where foo is of some_other_type. This breaks aliasing rules. The old code
only hid the type cast in a union:

     union bla {
        char* memory;
        some_type* a;
        some_other_type* b;
     };

The correct solution is to change the union to

     union bla {
        some_type a;
        some_other_type b;
     };
     bla* memory;

and use that appropriately.

-- Hannes

-- 
"Atomic objects are neither active nor radioactive." --
Doc.No N2798 (Working Draft, Standard for Programming Language C++)