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

[Francisco Jerez]

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), the result of the cast expression is
not, 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),
so this code seems valid to me (though admittedly it's far from obvious
and introduces another kind of UB).

> "((const struct exec_node **)n)[0] = foo;"
>
> ...so it can appear on the left-hand side of an assignment, which
> makes it an lvalue.
> _______________________________________________
> 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/6020e8b3/attachment.sig>