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

[Davin McCall]

On 26/06/15 15:29, Erik Faye-Lund wrote:
> 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:
>>>> [...]
>>> 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.

A cast by itself simply can't cause an aliasing violation, because it 
doesn't "access the stored value of an object". The aliasing violation 
has to happen either before or after the cast.

>
> Aliasing *is* hard. But let's not go shopping for that reason.

Agreed, though maybe this is getting to the point where we should take 
it off-list.

>
>> 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).

'n' is an lvalue, yes. Its type is 'struct exec node *' (disregarding 
const for now). So when the object that is in 'n' is accessed, it is 
accessed via an appropriate type. The cast does not itself cause an 
access and the cast expression as a whole is not an lvalue.

The fact that n is an lvalue does not mean that it can appear anywhere 
on the left-hand-side of an assignment expression; the entire 
left-hand-side expression must be an lvalue. So when you then say:

> You can write stuff like:
>
> "((const struct exec_node **)n)[0] = foo;"
>
> ...so it can appear on the left-hand side of an assignment, which
> makes it an lvalue.

... it is not quite correct. The fact that you could put n itself on the 
left-hand side of an assignment (eg "n = NULL") makes it an lvalue, not 
the fact that it appears *within* a larger expression that is on the 
left-hand side of an assignment.

In fact, the whole of '((const struct exec_node **)n)[0]' is an lvalue. 
However, '(const struct exec_node **)n' is not an lvalue.

I do believe you have some sort of misconception, and I'm trying hard to 
understand the nature of this misconception. Let's start with some stuff 
that I hope we can agree on (ignore the 'const' for now as it doesn't 
make much difference, unless you disagree?). Assume that the type of the 
variable 'n' is 'struct exec_node *'.

1. The expression '((struct exec_node **)n)[0] = foo' accesses two 
objects - one is that represented by the variable 'n', and another that 
is pointed-to by the pointer value in 'n'.
2. In (1), the first access is a read, the other is due to the 
assignment operator.
3. The object represented by the variable 'n' is not modified by the 
expression (unless 'n' has been made to point to itself).
4. The type of the 'n' object is 'struct exec_node *'
5. The type of the sub-expression 'n' is also 'struct exec_node *'.
6. The type of the sub-expression '(struct exec_node **)n' is 'struct 
exec_node **'.
7. The type of the whole expression, '((struct exec_node **)n)[0] = 
foo', is 'struct exec_node *'.

I think you would say the following (though I'd disagree):
8. The expression '((struct exec_node **)n)[0] = foo' necessarily causes 
an aliasing violation, assuming it doesn't cause undefined behavior for 
any other reason (i.e. assuming n is not null and is a valid pointer etc).
9. The statement 'return ((struct exec_node **)n)[0];' necessarily 
causes an aliasing violation, assuming it doesn't cause undefined 
behavior for any other reason.
10. The reason that (6) and (7) cause an aliasing violation is 
essentially the same.

Now if you do indeed believe (6) is true, the difficulty I'm having is 
understanding why. So, some questions:

Do you also believe the following would violate strict aliasing (and if 
so, on which line)?

    struct exec_node ** m = (struct exec_node *) n;
    return m[0];


How about this (and on which line)?

    struct exec_node ** m = (struct exec_node *) n;
    n = NULL;
    return m[0];


Davin

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150626/9f51b141/attachment-0001.html>