This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, PR 57748] Check for out of bounds access, Part 2
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Bernd Edlinger <bernd dot edlinger at hotmail dot de>
- Cc: Martin Jambor <mjambor at suse dot cz>, Eric Botcazou <ebotcazou at adacore dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 24 Oct 2013 15:08:37 +0200
- Subject: Re: [PATCH, PR 57748] Check for out of bounds access, Part 2
- Authentication-results: sourceware.org; auth=none
- References: <20130910193228 dot GE6732 at virgil dot suse> <2557640 dot s7jgdhfLQs at polaris> <DUB122-W4934D7B6A1D7196E2D6913E41C0 at phx dot gbl> <2243204 dot JyA3NMmzTd at polaris> <DUB122-W20E1531389D7EDA3529F6FE41C0 at phx dot gbl> <DUB122-W4C86060D7688821837731E4020 at phx dot gbl> <CAFiYyc1oKVqEO-xTJ8k9FKWVw_EW2w7Wk=KwdFJr7o1WTESCGg at mail dot gmail dot com> <20131023171106 dot GC22314 at virgil dot suse> <DUB122-W18EE8845FD49B5DF19B227E40C0 at phx dot gbl> <CAFiYyc0HXa40SD6arGF47siq6nHn0DHcbTL6Bbu0iOJhO9DQ6w at mail dot gmail dot com>
On Thu, Oct 24, 2013 at 12:22 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> 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.
And if it is to prevent ICEs then even struct Y { char c[1]; char c2; }
which gets HImode might be used as ((struct Y *)p)->c[1] - invalid
but still shouldn't ICE in any way.
Richard.
> 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.
- References:
- Re: [PATCH, PR 57748] Check for out of bounds access, Part 2
- RE: [PATCH, PR 57748] Check for out of bounds access, Part 2
- Re: [PATCH, PR 57748] Check for out of bounds access, Part 2
- RE: [PATCH, PR 57748] Check for out of bounds access, Part 2
- RE: [PATCH, PR 57748] Check for out of bounds access, Part 2
- Re: [PATCH, PR 57748] Check for out of bounds access, Part 2
- Re: [PATCH, PR 57748] Check for out of bounds access, Part 2
- RE: [PATCH, PR 57748] Check for out of bounds access, Part 2
- Re: [PATCH, PR 57748] Check for out of bounds access, Part 2