[cfe-dev] [PATCH] Fix for bug 21725: wrong results with union and strict-aliasing

[Daniel Berlin]

On Wed, Mar 18, 2015 at 2:22 AM, Jeroen Dobbelaere <
jeroen.dobbelaere at gmail.com> wrote:

>
>
> On Tue, Mar 17, 2015 at 11:55 PM, Daniel Berlin <dberlin at dberlin.org>
> wrote:
>
>> [..]
>>
>> In theory, the last time i remember, you weren't allow to set one member
>> of a union and read another.
>> But uh, that's not real user code :)
>>
>> (and IIRC, it does not say anything real)
>>
>>
> This is indeed correct. The main issue is that llvm thinks it is allowed
> to reorder 'stores' (as it thinks they are not aliasing),
> so that a legal read afterwards will result in a wrong answer (if an
> optimization or the backend reorders the stores based on the wrong aliasing
> information).
>

I thought both LLVM and GCC guaranteed type punning through unions would
work?


>
>
>
>>> If you access a member (or nested member) of a union, starting from the
>>> union itself, then it depends if the other type is also accessible through
>>> the union.
>>>
>>>
>>> So:
>>>
>>> int foo(union foo* a, float* b, int* c) {
>>>   a->a=1;
>>>   *b=2;
>>>   // compiler must assume a->a and *b can alias
>>>   // compiler must not assume *b and *c alias (access not through union)
>>> }
>>>
>>> (Also see section 3.10 of the c++03 standard;
>>>
>>
>>
>> This, IMHO, does not say what you seem to think it does :)
>>
>> For C++03,  3.10 only includes the word "union" here: "If a program
>> attempts to access the stored value of an object through an lvalue of other
>> than one of the following types the behavior is undefined:
>>
>> — the dynamic type of the object,
>> — a cv-qualified version of the dynamic type of the object,
>> — a type that is the signed or unsigned type corresponding to the dynamic
>> type of the object,
>>  — a type that is the signed or unsigned type corresponding to a
>> cv-qualified version of the dynamic type of the object,
>> — 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),
>>  — a type that is a (possibly cv-qualified) base class type of the
>> dynamic type of the object,
>>  — a char or unsigned char type."
>>
>>
>> C++ standard experts, at least on the GCC side, did not view this as
>> saying "all accesses must have an explicit union access", but that "It must
>> be part of a union type", but about whether you try to access it through a
>> union that doesn't have the right actual types in it.
>>
>> The type of those objects is right the type of the object. There is,
>> IMHO,  nothing illegal about those accesses.
>>
>>
>>
> So, with that interpretation, the mere presence of a 'union { short s; int
> i; }' (in the same or a different compilation unit) should influence the
> (type based) aliasing of all short and int pointers ?
>

It's actually worse than this interpretation, since someone pointed out on
the gcc side that "ihateunions" could be in a different TU, and you'd never
see the union at all.
While I think this is the right interpretation (as do others), that's not
the point.
I'm actually on your side here :)

But remember that Hal said  that if hte language has semantics, we should
follow them.

The whole point of this is essentially to point out that the language
semantics weren't completely thought out here :)
Because of that, our options are either "make up a rule people can follow"
or "disable TBAA".

GCC, and every other compiler i'm aware of, has chosen the former, at least
for C++03 (I don't know what C++11 or 14 say here).

This doesn't look a practical solution to me :(
> That's why I settled on the need of having the full access path available
> as a practical solution (and one that can easily be explained)..
>
> (btw. Do you have a pointer to the discussion on the gcc side ?)
>

It's about 7-8 years old at this point. Let me see if i can find it

>
> What we currently have is definitely wrong. Question is how we want to fix
> it...
>
> So, we have multiple wrongs here.
The first is that the short doesn't appear in the union info. Given that
it's a member of the union, it definitely should appear in the info for
that union.

I'd start there, and see how far that gets us :)


> Greetings,
>
> Jeroen Dobbelaere
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150318/146d4dd9/attachment.html>