Problem report: Struct aliasing problem causes Thread_Ready_Chain corruption in 4.6.99.3

[Thomas Doerfler (nt)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Pavel,

you are right, on some targets, my structure definition might get other
alignments than intended.

In that case I would like to discuss change the size of the
"Chain_Control" structure. AFAIK, it has previously been defined to hold
three pointers to save some memory (the "permanent_null" pointer ist
shared between the nodes), but maybe it is not SO important. In that
case, Chain_Control might also be defined as follows:

typedef union {
  struct Chain_Node_struct head;
  struct Chain_Node_struct tail;
} Chain_Control;

What would get broken if we change to a 16 byte Chain_Control?

Any comments to this?

wkr,
Thomas.

Pavel Pisa schrieb:
> Hello All,
> 
> On Tuesday 21 November 2006 13:38, Thomas Doerfler wrote:
> 
>>As far as I understood the problem, the following two data structures
>>are used together, but GCC does not understand that they are sometimes
>>aliased:
>>
>>struct Chain_Node_struct {
>>  Chain_Node *next;
>>  Chain_Node *previous;
>>};
>>
>>typedef struct {
>>  Chain_Node *first;
>>  Chain_Node *permanent_null;
>>  Chain_Node *last;
>>} Chain_Control;
>>
>>An example for aliased use is:
> 
> ....
> 
>>Maybe it would help to modify the definition of Chain_Ctronol to
>>something like:
>>
>>typedef union {
>>  struct {
>>    struct Chain_Node_struct head;
>>    void *fill_loc1;
>>  } head_node;
>>
>>  struct {
>>    void *fill_loc1;
>>    struct Chain_Node_struct tail;
>>  } tail_node;
>>} Chain_Control;
>>
>>The names are NOT good, and surely all chain.c/chain.inl functions would
>>have to be adapted but this might be a mechanical search/replace procedure.
>>
>>Any comments to a change like this?
> 
> 
> I have strong feeling that above solution is highly risky.
> There is no guarantee, that fields order in tail_node would not
> be reordered. In the fact, there is high probability, that on some
> targets order would be (4B fill_loc1, 4B spare, 8B tail_node)
> or tail_node would be reordered to same layout as head_node
> to follow alignment rules. I have feeling that that new ARM EABI tries
> to preserve alignment to 8B for example and would expose these problems.
> 
> The next solutions should work with GCC even, that they are not strictly
> correct according to C99 (same union should not be accessed trough
> different pattern than one which is used for it initialization
> - it no problem for GCC, which is prepared for this use)
> 
> typedef union {
>     struct {
>       Chain_Node *first;
>       Chain_Node *permanent_null;
>       Chain_Node *last;
>     } control;
>     struct {
>       struct Chain_Node_struct head;
>     } for_alias_marking_only;
> } Chain_Control;
> 
> If GCC extension allowing unnamed structures and unions fields
> is explored, then definition can be changed like next example
> without any core change requirements
> 
> typedef union {
>     struct {
>       Chain_Node *first;
>       Chain_Node *permanent_null;
>       Chain_Node *last;
>     };
>     struct {
>       struct Chain_Node_struct head;
>     } for_alias_marking_only;
> } Chain_Control;
> 
> I think, that next solution through explicit pointers typecasting
> is unsafe
> 
> 
>>-      Chain_Append(&RTEMS_Malloc_GC_list, (Chain_Node *)ptr);
>>+      Chain_Node *node = (Chain_Node *)ptr;
>>+      Chain_Append(&RTEMS_Malloc_GC_list, node);
> 
> 
> It has no effect on the aliasing rules implications.
> It can have effect only due some different compiler
> resolution/encoding of differently described prescription.
> But it should lead to exactly same code for optimally optimizing
> compiler.
> 
> Best wishes
> 
>             Pavel Pisa
> _______________________________________________
> rtems-users mailing list
> rtems-users at rtems.com
> http://rtems.rtems.org/mailman/listinfo/rtems-users


- --
- --------------------------------------------
IMD Ingenieurbuero fuer Microcomputertechnik
Thomas Doerfler           Herbststrasse 8
D-82178 Puchheim          Germany
email:    Thomas.Doerfler at imd-systems.de
PGP public key available at:
     http://www.imd-systems.de/pgpkey_en.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (MingW32)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFFY1ScwHyg4bDtfjQRAtDcAJsG9spwK7tK0mGEW08boGyvr3jVFACcCM0a
p2KCBSgOvVOzi/4UjPX1iw8=
=6B/d
-----END PGP SIGNATURE-----