[PATCH] Fix C++ strict-aliasing issues with memcpy folding

[Richard Guenther]

On Tue, 2 Feb 2010, Gabriel Dos Reis wrote:

> On Tue, Feb 2, 2010 at 6:44 AM, Richard Guenther <rguenther@suse.de> wrote:
> > On Tue, 2 Feb 2010, Gabriel Dos Reis wrote:
> >> >
> >> > Basically the middle-end sees
> >> >
> >> > Â new (&x.m.mem[0]) Y;
> >> > Â ((Y *)&x.m.mem[0])->foo = 1;
> >> > Â tem = x;
> >
> > ... the copy-assignment is seen as a load from 'x' and a store to 'tem'
> > by the middle-end. Â Supposedly this is what the frontend emits
> > for copy-assignments of POD types.
> 
> OK.  So, the middle-end does not care about the distinction
> between sub-objects and complete objects?

Correct.

> If I understand what you are saying so far, it looks to me as
> if the g++ front-ends is not constructing trees with sufficient
> information for the middle-end not to get confused.

Correct.

> >> > and the store does not conflict with the load of x as far
> >> > as TBAA is concerned (there is no Y in the alias-subsets of X).
> >> > So the scheduler ends up re-ordering the accesses.
> >> >
> >> > Thus I'm looking for a way to claim that the copy as above
> >> > invokes undefined behavior (because of that in-place construction),
> >> > and that the only valid way to copy 'x' is by using memcpy
> >> > (independed on that being broken by the middle-end).
> >>
> >> Is there a way to tell the middle end that in the assignment
> >>
> >> Â  Â  tem = x;
> >>
> >> both members of x.m are indistinguishable in terms of aliasing?
> >> Or would that be too pessimistic an assumption for the optimizer?
> >
> > That wouldn't be even enough. Â Or I don't understand what you
> > mean with indistinguishable.
> >
> >> > So, is the copy tem = x above indeed invoking undefined behavior?
> >>
> >> Technically, that assignment is undefined behaviour in C++03, but
> >> I think a compiler that goes by that would be terrible from practical
> >> point of view -- that omission was not intended, as indicated in the DR.
> >> That will be fixed soon, by requiring the compiler to do a memcpy().
> >
> > So every copy-assignment of a POD type will have to be a memcpy?
> 
> The suggestion of memcpy() for
> 
>     tem = x;
> 
> is only of copy-n-assignments of union objects.  This is because at the
> point of the assignment, we don't necessarily have the information about
> which branch of the union was last assigned to -- so, a memberwise
> assignmnet would be invalid.  For other POD types,  I think the language
> is sufficiently clear to keep whatever we currently  have working.

Ok, so if you have a union embedded in a struct like

  struct X {
    union { ... } u;
    int i;
  } a, b;

then

  a = b;

would be translated into

  memcpy (&a, &b, sizeof (a));

?  or would it be

  memcpy (&a.u, &b.u, sizeof (a.u));
  a.i = b.i;

?  I suppose the latter.

If other POD types are supposed to be fine then you probably share
Marks view of disallowing in-place construction of arbitrary
types in arbitrary storage with a declared type?  Thus only
allow it to be constructed in place of a union, supposedly with
a char[] member?

It would be nice to clarify the standard here.

> >> Now, I do not know whether the g++ front-end will insert that memcpy(),
> >> or if it would let the middle end do it. Â Either way, I think g++ today
> >> should treat that assignment as making 'tem.m' have the same active
> >> dynamic type as 'x.m'. Â Now, you're the middle-end expert, and I have
> >> no clue about how difficult that is to teach GCC with the current
> >> infrastructure.
> >
> > It would be the frontends responsibility to translate tem = x
> > with using memcpy as only the frontend knows that the copy has
> > to conflict with stores through arbitrary types.
> 
> OK, that makes sense.  Assuming the front-ends issues memcpy for
> union objects, would we still be running into this issue?  That is I'm
> trying to understand whether there are other aspects that we may have
> forgotten (from the middle-end point of view.)

Well, if it would be undefined to do

 struct X { int i[2]; } a, b;
 double *p = new (&a.i[0]) double;
 *p = 1.
 b = a;
 return *(double *)&b.i[0];

because we place the double not into a union then yes, I think
issueing memcpy for union objects should be enough.

Note that the middle-end is fine with

 union { int i; short s; float f; } a, b;
 a.i = 0;
 b = a;
 return b.i;

The active member of a is transferred to b.

It is just that if there is a type constructed at the place of the
union that is not in the type list of its members (as in the
original example), then we are lost.  Note that placing a char
member in the union doesn't fix this from the middle-end - only
using memcpy for the assignment does.

> > Whether using memcpy (or any other middle-end notion with the
> > same effect) or simply disabling type-based aliasing for C++ will
> > end up more efficient remains to be seen ...
> 
> OK. (It would be a pity if we had to disable TBAA for g++.)

Indeed.  People would just complain again that C++ is slower than C.

Richard.