Re: [PATCH, PR 57748] Check for out of bounds access, Part 2

[Richard Biener <richard dot guenther at gmail dot com>]

On Thu, Oct 24, 2013 at 9:15 AM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> Hi Martin,
>
> On Wed, 23 Oct 2013 19:11:06, Martin Jambor wrote:
>> On Wed, Oct 23, 2013 at 06:00:43PM +0200, Richard Biener wrote:
>>>
>>
>> ...
>>
>>> ? And why should the same issue not exist for
>>>
>>> struct { V a[1]; }
>>>
>>
>> indeed, it does and it's simple to trigger (on x86_64):
>>
>> ----------------------------------------
>> /* { dg-do run } */
>>
>> #include <stdlib.h>
>>
>> typedef long long V
>> __attribute__ ((vector_size (2 * sizeof (long long)), may_alias));
>>
>> #if 1
>> typedef struct S {V b[1]; } P __attribute__((aligned (1)));
>> struct __attribute__((packed)) T { char c; P s; };
>> #else
>> typedef struct S {V b[1]; } P;
>> struct T { char c; P s; };
>> #endif
>>
>> void __attribute__((noinline, noclone))
>> good_value (V b)
>> {
>> if (b[0] != 3 || b[1] != 4)
>> __builtin_abort ();
>> }
>>
>> void __attribute__((noinline, noclone))
>> check (P *p)
>> {
>> good_value (p->b[1]);
>> }
>>
>> int
>> main ()
>> {
>> struct T *t = (struct T *) calloc (128, 1);
>> V a = { 3, 4 };
>> t->s.b[1] = a;
>> check (&t->s);
>> free (t);
>> return 0;
>> }
>> ----------------------------------------
>>
>> While I was willing to discount the zero sized array as an
>> insufficiently specified oddity, this seems to be much more serious
>> and potentially common. It seems we really need to be able to detect
>> these out-of-bounds situations and tell lower levels of expander that
>> "whatever mode you think you are expanding, it is actually BLK mode."
>>
>> It's uglier than I thought.
>>
>> Martin
>>
>
> Deja-vu...
>
> Thanks for this test case. This definitely destroys the idea to fix this in stor-layout.c
>
> I think that is common practice to write array[1]; at the end of the structure,
> and use it as a flexible array.

Same for stuff like

 struct X { char c[4]; };

that currently gets SImode.  In alias handling we treat all trailing
arrays as flexible, even if they happen to be nested in sub-structs like for

 struct X { int i; struct Y { char c[4]; } };

too much code out in the wild to disallow this.

> The compute_record_mode has no way to
> decide if that is going to be a flexible array or not.

But it could assign BLKmode to _all_ array types.

Richard.

> Sorry Eric, but now I have no other choice than to go back to my previous version
> of part 2:
>
> http://gcc.gnu.org/ml/gcc-patches/2013-10/msg00222.html
>
> I'd just add Martin's new test case as 57748-4.c.
>
>
> Bernd.