[cfe-commits] [PATCH] Fix struct assignment from "piecemeal" aggregates

[Eli Friedman]

On Fri, Jun 17, 2011 at 11:21 AM, Douglas Gregor <dgregor at apple.com> wrote:
> The fix I added in r133261 to solve the miscompilation of
>
>  typedef struct S { int x,y; } S;
>  S x(struct S s) { s = (S){s.y,s.x}; return s; }
>
> was terribly unsatisfying, because it worked around a bug in the compilation of aggregate assignment expressions, e.g.,
>
>  lhs = rhs
>
> As an optimization, IR generation computes the address of the the lhs and then directly initializes 'lhs' with the value of rhs. This works great except when the expression 'rhs' actually builds a structure piece-by-piece, as in
>
>        s = (S){s.y,s.x}
>
> and individual pieces refer back to the lhs, because those individual pieces are written immediately to the lhs. No temporary structure for the rhs is ever created or allocated. In the example, the function x(), when given a structure { 1,  2 }, will end up returning { 2, 2 } rather than the intended { 2, 1 }.
>
> The fix in r133261 basically forced us to always create a temporary and memcpy when generating a non-POD compound literal, but that's a hack: the real bug is that assignment is directly initializing the lhs from the rhs when it isn't safe to do so.
>
> The attached patch fixes the root of the problem: by detecting when the rhs is something that will build an aggregate piecemeal, we can generate safer code that creates a temporary for the rhs and then copies it over. In the common case (rhs builds a full aggregate value in one shot), we still perform the optimization.
>
> Comments appreciated!

I'm sort of uncomfortable with the blacklist approach... are you sure
you're actually catching all the unsafe cases?

While we're working this stuff out, it might be nice to also work out
if we're doing the right thing for sret calls...

-Eli