Problem report: Struct aliasing problem causes Thread_Ready_Chain corruption in 4.6.99.3

[Pavel Pisa]

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