Fix PR80533

On Thu, 27 Apr 2017, Alexander Monakov wrote:

> On Thu, 27 Apr 2017, Richard Biener wrote:
> > struct q { int n; long o[100]; };
> > struct r { int n; long o[0]; };
> > 
> > union {
> >     struct r r;
> >     struct q q;
> > } u;
> > 
> > int foo (int i, int j)
> > {
> >   long *q = u.r.o;
> >   u.r.o[i/j] = 1;
> >   return q[2];
> > }
> > 
> > but nothing convinced scheduling to move the load before the store ;)
> > The two memory references are seen as not aliasing though.  Stupid
> > scheduler.
> 
> On x86 there's an anti-dependence on %eax that prevents the second scheduler
> from performing the breaking motion; with -fschedule-insns, pre-RA scheduler
> is actually able to move the load too early, but then IRA immediately undoes
> that.  Also, -fselective-scheduling2 is able to move the load early via renaming
> (and uncovers the miscompile, as nothing undoes it later).
> 
> Applying division to the result of the load, rather than the address of the
> store, also produces code demonstrating the miscompile:
> 
> struct q { int n; long o[100]; };
> struct r { int n; long o[0]; };
> 
> union {
>     struct r r;
>     struct q q;
> } u;
> 
> int foo (int i, int j)
> {
>   long *q = u.r.o;
>   u.r.o[i] = 1;
>   return q[2]/j;
> }

Thanks.  It also is still miscompiled because array_at_struct_end_p
is somewhat confused as to what its semantics are supposed to be.
Multiple callers (including the new one) assume when it returns false
they can trust the array domain while in reality when it returns false
it says we can constrain the access even if only because we know an
underlying decls size ...

It also raises the question whether for

struct X
{
  double x;
  int a[1];
} x;

x.a is an array at struct end -- there's enough padding to provide
space for x.a[1] even though its size doesn't include that (and
whether it can depends on the alignment of 'double').  This is
sort-of what happens for the union case above -- u.r.o can be
extended into the padding (which is quite large due to u.q.o).
The question is whether we allow such access to padding in general
(not only when the array is at struct end).  Likewise do we allow, in

struct Y
{
  struct X[4][4] a;
} y;

for y.a[i][j].a[k] k == 3?  that is, allow the "last" element of
an array to be flexibly extended?  Or do we instead allow
y.a[i][j] with j == 4, thus the last dimension of a multidimensional
array to be "over-allocated"?  Or do we just allow i > 3?

There's another PR where array-bound warnings are confused by
the weird answer from array_at_struct_end_p and two other users
I skimmed would generate wrong code because they trust the array
domain after it.

Looks like I have to refactor that a bit, like with the following
which makes things a bit more explicit, and _not_ allowing to
use padding appart when using flexarrays (though that can't
happen, you can't instantiate those with decls) or the zero-size
array GNU extension.

It also corrects IMHO wrong behavior with VIEW_CONVERT_EXPRs
and the overly pessimistic handling of arrays of structs with
arrays at struct end or multi-dimensional arrays when not
dealing with the outermost dimension.

So I'm testing the following.

Richard.

2017-04-28  Richard Biener  <rguenther@suse.de>

	PR middle-end/80533
	* tree.c (array_at_struct_end_p): Handle arrays at struct
	end with size zero more conservatively.  Refactor and treat
	arrays of arrays or aggregates more strict.  Fix
	VIEW_CONVERT_EXPR handling.

	* gcc.dg/torture/pr80533.c: New testcase.

Fix PR80533

Commit Message

Comments

Patch