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

[Francisco Jerez]

Davin McCall <davmac at davmac.org> writes:

> 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.
>
Also note that even *p1 and *p2 are allowed to overlap even though they
are of different types because of section 6.5 of C99:

| 7 An object shall have its stored value accessed only by an lvalue
|   expression that has one of the following types:
|[...]
|    - an aggregate or union type that includes one of the aforementioned
|      types among its members (including, recursively, a member of a
|      subaggregate or contained union)[...]

>>   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
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150626/56d33686/attachment.sig>