Strict aliasing and chains revisited

[Sebastian Huber]

On 10/21/2010 05:42 PM, Till Straumann wrote:
>  On 10/21/2010 12:51 AM, Sebastian Huber wrote:
>> On 10/21/2010 01:49 AM, Till Straumann wrote:
>>>   I didn't think about this carefully but I guess a clean solution
>>> would be sacrificing the space for 1 pointer and make the
>>> Chain_Control a proper struct comprised of 2 Chain_Nodes.
>> The chain is a pivot data structure in RTEMS and some operations are
>> not that
>> efficient if you sacrifice the node overlap.  Also chain controls are
>> used in
>> great numbers (one ready chain per task priority).  We should keep
>> that structure.
>>
>>> A hacked solution would be making Chain_Control a (hopefully properly
>>> packed)
>>>
>>> union Chain_Control {
>>>     struct {
>>>        Chain_Node node;
>>>        Chain_Node *fill;
>>>     } head;
>>>     struct {
>>>        Chain_Node  *fill;
>>>        Chain_Node  node;
>>>     } tail;
>>> } Chain_Control;
>> Yes, this is an alternative to the casts.  I have no preference here.
> I don't think the casts remove the alias problem. The fact
> that it doesn't seem to happen doesn't guarantee that the
> code is correct (gcc behavior changes).
> 
> I would feel more comfortable with a proper declaration,
> i.e., avoiding type punning.

Ok, I changed the Chain_Control definition to:

typedef union {
    struct {
      Chain_Node Node;
      Chain_Node *fill;
    } Head;

    struct {
      Chain_Node *fill;
      Chain_Node Node;
    } Tail;
} __attribute__ ((packed)) Chain_Control;

Test suites pass on ARM, MIPS and SPARC.  One problem with this change is that
the field names change, and this may break applications which use this internal
API.

>> Till, what do you think about the change in watchdoginsert.c?
> If there are compiler memory  barriers in place then
> there should be no problem. You mentioned the barrier
> in ISR_Flash() -- is there also a barrier before entry
> to the routine? This might not be necessary - I'm travelling
> and don't have time + access to the code for proper
> analysis, sorry - but we should double-check.
[...]

Yes, before the _ISR_Flash() there is an _ISR_Disable() which is also a memory
barrier.

-- 
Sebastian Huber, embedded brains GmbH

Address : Obere Lagerstr. 30, D-82178 Puchheim, Germany
Phone   : +49 89 18 90 80 79-6
Fax     : +49 89 18 90 80 79-9
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.