Fwd: Problem report: Struct aliasing problem causes Thread_Ready_Chain corruption in 4.6.99.3

[Joel Sherrill]

Ralf Corsepius wrote:
> On Tue, 2006-11-21 at 17:16 +0100, Thomas Doerfler wrote:
>   
>> Ralf,
>>
>> Ralf Corsepius schrieb:
>>     
>>> On Tue, 2006-11-21 at 16:38 +0100, Thomas Doerfler wrote:
>>>
>>>       
>>>> Therefore your patch in malloc.c is IMHO identical to the previous
>>>> explicit cast.
>>>>         
>>> Please prove Joel's patch to be broken.
>>>       
>> maybe it is my limited horizon, but I don't see the difference applied
>> by the following patch:
>>
>>    if (_System_state_Is_up(_System_state_Get())) {
>>      if ((_Thread_Dispatch_disable_level > 0) || (_ISR_Nest_level > 0)) {
>> -      Chain_Append(&RTEMS_Malloc_GC_list, (Chain_Node *)ptr);
>> +      Chain_Node *node = (Chain_Node *)ptr;
>> +      Chain_Append(&RTEMS_Malloc_GC_list, node);
>>        return;
>>
>> The intermediate local variable "node" can easily be optimized by the
>> compiler. For the compiler, it should be obvious, that "node" is a
>> direct alias for "(Chain_Node *)ptr". So I don't see ANY reason, what
>> this additional variable would change in the compiler's behavior.
>>     
> Right, ... that's why I have been asking Peer and you to prove an
> example really exposing issue.
>
> I am having difficulties in seeing what should be broken with this code.
> But I see suspicious spots in chain.inl.
>
>   
I don't think anything is wrong with this code.  All I said was that in 
the initial
batch of punning warning removals we added intermediate variables to make
gcc a bit happier.

It's VERY VERY likely to be unneeded.

But what _Chain_Head and _Chain_Tail do is probably the culprit.
>> It MAY BE, that this change produces different code, but I don't see,
>> why the code should be more correct.
>>     
> ACK.
>
>   
For some reason, I don't see able to find one of the examples where gcc 
actually
complained that adding an intermediate variable helped.  I recall it was 
code
that took a pointer of one type, converted it to void *, and then 
converted it
to something else.  This pattern was usually in BSP or device driver code.

--joel

> Ralf
>
>
>