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

[Till Straumann]

**BEWARE**

Thomas is right. And after reading the aforementioned article
I tried their example (simplified):

inline void
writethree(int *p)
{
short *palias = (short*)p;
   /* compiler assumes 'palias' and 'p' *dont* address the same memory 
region */
   *palias = 3;
}

int
testalias()
{
int value = 0xdeadbeef;
    writethree(&value);
    return value;
}

When I compile (gcc-4.1.2, gcc-4.1.1) this with

-fstrict-aliasing -O -Wstrict-aliasing=2

then the call to 'writethree' gets optimized away
**w/o warning** (since 'palias' is not a valid alias of  'p').

I must admit that I didn't fully understand the
strict-alias rule until today and I'm a bit shocked.
I'm sure it breaks code I've written.

Until every cast has been hand-checked carefully
I see no way around -fno-strict-aliasing

(And, yes, the 'intermediate variable fix' seems to
not fix anything except silencing the warning).

If gcc sees enough of the source code (e.g. when inlining)
to know exactly what the 'original' pointers were
(prior to any casting and using intermediate variables)
then it 'ruthlessly' enforces aliasing rules no
matter what casting you do:

Consider this:

typedef union {
    int   i;
    short s;
} u2;

int rtn3()
{
int   rval = 3;
u2    *pu  = &rval;
short *ps = (short*)pu;
    *ps = 0xdead;
    return rval;
}

Even though casting a int* into u2* is legal
(u2 contains a int member) and u2* to short*
is, too (u2 also contains a short member)
gcc still optimizes *ps=0xdead away because
it says: at the end of the day, ps must not be
an alias of  &rval and it knows that both
point to the same address.
OTOH, setting

pu->s = 0xdead;

would not go away.

If code was not written from the ground up with the
strict-aliasing rule in mind then it seems hard to
ensure the rule is not violated.

-- Till

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.
>
> It MAY BE, that this change produces different code, but I don't see,
> why the code should be more correct.
>
> wkr,
> Thomas.
>
>   
>> Ralf
>>
>>
>>     
>
>
>