[Mesa-dev] [PATCH v2] glsls: Modify exec_list to avoid strict-aliasing violations

[Davin McCall]

On 26/06/15 14:31, Eirik Byrkjeflot Anonsen wrote:
> Erik Faye-Lund <kusmabite at gmail.com> writes:
>
>> On Fri, Jun 26, 2015 at 1:23 PM, Davin McCall <davmac at davmac.org> wrote:
>>> On 26/06/15 12:03, Davin McCall wrote:
>>>> ... The stored value of 'n' is not accessed by any other type than the
>>>> type of n itself. This value is then cast to a different pointer type. You
>>>> are mistaken if you think that the cast accesses the stored value of n. The
>>>> other "stored value" access that it occurs in that expression is to the
>>>> object pointed at by the result of the cast. [...]:
>>>
>>> I'm sorry, I think that was phrased somewhat abrasively, which I did not
>>> intend. Let me try this part again. If we by break up the expression in
>>> order of evaluation:
>>>
>>> From:
>>>     return ((const struct exec_node **)n)[0]
>>>
>>> In order of evaluation:
>>>
>>> n
>>> - which accesses the stored value of n, i.e. a value of type 'struct exec
>>> node *', via n, which is obviously of that type.
>>>
>>> (const struct exec_node **)n
>>>   - which casts that value, after it has been retrieved, to another type. If
>>> this were an aliasing violation, then casting any pointer variable to
>>> another type would be an aliasing violation; this is clearly not the case.
>>>
>>> ((const struct exec_node **)n)[0]
>>> - which de-references the result of the above cast, thereby accessing a
>>> stored value of type 'exec node *' using a glvalue of type 'exec node *'.
>> I think breaking this up is a mistake, because the strict-aliasing
>> rules is explicitly about the *combination* of these two things.
>>
>> You *are* accessing the underlying memory of 'n' through a different
>> type, and this is what strict aliasing is all about. But it takes two
>> steps, a single step isn't enough to do so.
>>
>> Those other spec-quotes doesn't undo the strict-aliasing definitions;
>> knowing how things are laid out in memory doesn't mean the compiler
>> cannot assume two differently typed variables doesn't overlap.
> So basically, you're saying that e.g.:
>
>     p->next = a;
>     q = exec_node_get_next_const(p);
>
> is equivalent to:
>
>     exec_node * p1 = p;
>     exec_node ** p2 = (exec_node**)p;
>     p1->next = a;
>     q = p2[0];

It is, once the patch is applied (or if strict aliasing is disabled).

> And at this point p1 and p2 are different types, so the compiler can
> freely assume that p1 and p2 are non-overlapping.

p1 and p2 are two separate variables and of course they are 
non-overlapping, but *p1 and **p2 are the same type and so may overlap.

>   Thus the two
> assignments can be "safely" reordered. Sounds plausible to me.

The assignments are to 'p1->next' and 'p2[0]', which *are* the same type 
(struct exec_node *) and therefore the assignments *cannot* be 
reordered. It is exactly this that I rely on in my patch to resolve the 
aliasing issue with the current code.

> And note that casting via void* won't help. "p == (void*)p" compares a
> variable of type "exec_node*" to a variable of type "void*", and thus
> there's no strict-aliasing problem. But "p == (exec_node**)(void*)p"
> compares an "exec_node*" to an "exec_node**" and thus the compiler can
> assume that they are not the same.

The compiler cannot assume pointers are not the same based on their 
type, unless the pointers are de-referenced, which they are not in the 
example you just gave. Strictly speaking C99 doesn't even allow the 
comparison of 'p == (exec_node**)(void*)p', but all the compilers I know 
of allow it, and AFAIK give the same result as if one operand were cast 
to the same type as the other.

Davin