Re: NSConnection bugs

[Richard Frith-Macdonald]

On 29 Mar 2011, at 10:10, Philippe Roussel wrote:

> Le mardi 29 mars 2011 à 09:25 +0100, Richard Frith-Macdonald a écrit :
>> On 29 Mar 2011, at 01:01, David Chisnall wrote:
>> 
>>> Hello the list,
>>> 
>>> I spent some time with valgrind and found out why NSConnection is so flakey 
>>> on 64-bit platforms.  It seems that we use the same incorrect pattern in a 
>>> lot of places throughout the class.  We probably use it in other places, so 
>>> it would be worth doing a brief audit for this bug before the next release. 
>>>  I had a quick look, and I think that NSConnection is the only place where 
>>> we use GSIMap with something other than pointers.
>>> 
>>> The problem is in statements like this:
>>> 
>>>     node = GSIMapNodeForKey(IreplyMap, (GSIMapKey)seq);
>>> 
>>> Looks fine?  The problem is that (GSIMapKey) is a union (something 
>>> concealed by the fact that it is a macro - unions are difficult to use 
>>> safely, and anything that hides the fact that you are using a union is 
>>> generally a bad thing).  Specifically, it's a problem that GSIMapKey is a 
>>> union of void*, id, unsigned, and int.  On most current 64-bit platforms, 
>>> unsigned and int are 32-bit quantities, while void* and id are 64-bit 
>>> quantities. 
>>> 
>>> The problem, therefore, is that the we are defining 4 bytes of an 8-byte 
>>> quantity and then (in the callee) accessing all 8 bytes.  This results in 
>>> some random behaviour.  The connection.m test was failing because sometimes 
>>> the uninitialised bytes were 0 (so the map was finding the value it was 
>>> looking for) and sometimes they were some other value from the stack.
>> 
>> Well spotted.  I've implemented a fix for that throughout base.
> 
> Seems to break on x86-64 gcc 4.4.3 :
> 
> GSAttributedString.m:147: error: cast to union type from type not present in 
> union

Ah, the problems of not actually having a 64bit development system ... I forgot 
some places where explicit casts are required ... should be fixed now.