[Mesa-dev] [PATCH] Fix strict-aliasing violations in GLSL shader list implementation

[Davin McCall]

On 24/06/15 03:35, Ian Romanick wrote:
> On 06/23/2015 07:01 PM, Dave Airlie wrote:
>>
>> Oh we've always had aliasing problems this is just one, you can't
>> expect one person to fix them all at once.
> But "With this patch, I can build a working (though perhaps not 100%
> bug-free) Mesa without using -fno-strict-aliasing during compilation."
> is a pretty strong statement that doesn't completely jibe many years of
> Mesa using -fno-strict-aliasing before exec_list was added.

I suppose the reason for that is that an aliasing violation doesn't 
necessarily cause the compiler to do something other than what the 
programmer wanted - it just raises the potential for that to happen. It 
so happens that the exec_list implementation caused issues in my build. 
Having said that, the statement wasn't intended to be particularly 
strong. Read it as "I, personally, with my particular compiler version 
and options, was able to produce a binary that appeared to work". I 
wasn't necessarily proposing that -fno-strict-aliasing should be removed 
from all Mesa builds after applying my patch, thought it could certainly 
be considered.

(There's also the fact that GCC makes considerably more effort now to 
behave "correctly", when it detects an aliasing violation, than it did 
several years back. It used to be trivial to demonstrate aliasing issues 
by showing the assembly output of a small piece of code; it's no longer 
so easy).

>
>> But making headway is a good thing.
>>
>> You can't have
>> struct exec_list *p;
>> struct exec_node *p2 = (struct exec_list *)p
>>
>> And do things with p2 and hope that p will get them, because
>> the compiler wants to store things its doing to p in registers,
>> and when you go and do something in p2 it can't work out it's the
>> same thing, so it has to spill/reload.
> Which I think is different from what Davin was saying, but I may be
> misunderstanding the whole thing.  That's why I'd like to see spec
> language.

Dave has it right, though he uses a slightly different example to me. In 
his example, p and p2 (if they are de-referenced) must not alias 
according to the C99 rules. So, If they are de-referenced, the compiler 
can assume that they do not alias, which means it can re-order reads and 
writes to the two pointers. Consider:

     p2->next = NULL;
     p->head = p2;

(I know this code doesn't make much sense in the context of actual use 
of exec_list, but bear with me). Both are trying to assign to p->head, 
but one does so directly where the other does it via an exec_node *. 
These assignments might be re-ordered by the compiler, because it 
believes the pointers cannot alias.

>    The part that really gets me is that this is across a
> function boundary... that's generally a sacred line, so I'm surprised
> that the compiler is allowed to disregard what it's told in that scenario.

In the language standard, function boundaries really don't mean much. 
Module boundaries used to be much stronger, but link-time optimization 
weakens those, too.

> I'd also like to see assembly dumps with and without
> -fno-strict-aliasing of some place where this goes wrong.  If you,
> Davin, or someone else can point out a specific function that actually
> does the wrong thing, I can generate assembly myself.

I'll see if I can pinpoint where it's going wrong. It may take me a 
little time. Bear in mind that the compiler version and options have a 
significant effect.

> For that matter... how the heck is the ralloc (or any memory allocator)
> implementation valid?

There are some ways to work around the aliasing restrictions. 'char *' 
is allowed to alias anything. Unions can be used for aliasing, with some 
restrictions. 'memcpy' can copy bytes from any object into any other object.

Davin