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

[Erik Faye-Lund]

On Fri, Jun 26, 2015 at 5:25 PM, Francisco Jerez <currojerez at riseup.net> wrote:
> Erik Faye-Lund <kusmabite at gmail.com> writes:
>
>> On Fri, Jun 26, 2015 at 4:53 PM, Francisco Jerez <currojerez at riseup.net> wrote:
>>> Erik Faye-Lund <kusmabite at gmail.com> writes:
>>>
>>>> On Fri, Jun 26, 2015 at 4:16 PM, Davin McCall <davmac at davmac.org> wrote:
>>>>> On 26/06/15 14:53, Erik Faye-Lund wrote:
>>>>>>
>>>>>> On Fri, Jun 26, 2015 at 3:05 PM, Davin McCall <davmac at davmac.org> wrote:
>>>>>>>
>>>>>>> On 26/06/15 12:55, Erik Faye-Lund wrote:
>>>>>>>
>>>>>>> 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.
>>>>>>>
>>>>>>>
>>>>>>> It is not a mistake, and the strict aliasing rules are not about the
>>>>>>> combination of these two things.
>>>>>>
>>>>>> It is. In fact, it's not even possible to violate strict-aliasing
>>>>>> without doing at least two operations. You cannot validate operations
>>>>>> in a vacuum, because that's not how strict-aliasing is defined.
>>>>>
>>>>>
>>>>> Any pointer dereference can violate strict aliasing - that's one operation.
>>>>> If you mean that it's first necessary to construct a pointer value in such a
>>>>> way that de-referencing it will be an aliasing violation, then yes, I agree
>>>>> with this statement.
>>>>>
>>>>
>>>> Yes, I mean exactly the latter. You cannot look at one operation in
>>>> isolation, you need to look at the whole program.
>>>>
>>>>>>
>>>>>>> As I have pointed out, with your reading,
>>>>>>> pretty much any pointer cast constitutes an aliasing violation.
>>>>>>>
>>>>>> No, only those violating the strict aliasing rules I posted before.
>>>>>
>>>>>
>>>>> ... which would only allow changing const/volatile qualifiers, not the
>>>>> pointed-to type.
>>>>>
>>>>
>>>> You can change the pointed to type in terms of signedness, you can
>>>> cast it to a compatible type, you can cast a void-pointer or
>>>> char-pointer to any type. But you need to make sure you don't violate
>>>> the strict-aliasing rules in some other way while doing the latter.
>>>>
>>>> Aliasing *is* hard. But let's not go shopping for that reason.
>>>>
>>>>> Your reading also disallows casting an 'int' variable to type 'long',
>>>>> because that isn't on the list.
>>>>>
>>>>>>
>>>>>>> The strict aliasing rules specify what kind of reference you can use to
>>>>>>> access an object of a particular type. They say nothing about how that
>>>>>>> reference is obtained.
>>>>>>
>>>>>> Which means that it applies regardless of how you obtain it.
>>>>>
>>>>>
>>>>> Yes.
>>>>>
>>>>>> "If a program attempts to access the stored value of an object through
>>>>>> a glvalue of other than one of the following types the behavior is
>>>>>> undefined"
>>>>>>
>>>>>> It says "if a *program* attempts", not "if a *statement* attempts" or
>>>>>> "if an *opreation* attempts". This is a whole-program deal, not
>>>>>> limited to one operation in isolation.
>>>>>
>>>>>
>>>>> The key part of the wording is "through a glvalue":
>>>>>
>>>>> "If a program attempts to access the stored value of an object *through
>>>>> a glvalue* of other than one of the following types ..."
>>>>
>>>> This is exactly what makes this invalid AFAICT, see below.
>>>>
>>>>> Going back to the original example:
>>>>>
>>>>>    return ((const struct exec_node **)n)[0]
>>>>>
>>>>> The glvalue used to access the object in n is n itself. (I do not think that
>>>>> '(const struct exec_node **)n' is even a glvalue).
>>>>
>>>> Bur 'n' *is* an lvalue, which also makes it an glvalue (for reference,
>>>> a glvalue is a "generalized lvalue", which means that it's either an
>>>> lvalue or an xvalue). You can write stuff like:
>>>>
>>>
>>> "n" is indeed an lvalue (which in no way aliases the storage of any
>>> exec_node or exec_list object)
>>
>> '(const struct exec_node **)n' is an lvalue who alias the storage to n.
>>
>>> , the result of the cast expression is
>>> not,
>>
>> The result of the cast is also an lvalue. You can assign to a casted pointer.
>>
> No, you can't, you can only assign to the result of dereferencing the
> result of a casted pointer.  See the footnote in section 6.5.4 of the
> C99 spec:
>
> | 89) A cast does not yield an lvalue. Thus, a cast to a qualified type
> |     has the same effect as a cast to the unqualified version of the type.
>
>>> and the result of the subscript expression is again an lvalue but
>>> of a type (exec_node *) which may legitimately alias an exec_node or
>>> exec_list object (because of the text from C99 6.5/7 I quoted earlier),
>>
>> Again, as I just said in response to the other mail. I don't see how
>> that section is relevant in this case.
>
> OK, I've replied to this point on the other thread.

Yeah, I'm totally satisfied with the explanation :)

Sorry for making a big fuzz about it!