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: Martin Jambor <mjambor at suse dot cz>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: Bernd Edlinger <bernd dot edlinger at hotmail dot de>, Eric Botcazou <ebotcazou at adacore dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 23 Oct 2013 19:11:06 +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>
Hi,
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
> which is also "flexible enough" that we support accesses beyond it
> via a pointer? That struct also has V2DImode. And this is all
> because
>
> /* If this field is the whole struct, remember its mode so
> that, say, we can put a double in a class into a DF
> register instead of forcing it to live in the stack. */
> if (simple_cst_equal (TYPE_SIZE (type), DECL_SIZE (field)))
> mode = DECL_MODE (field);
>
> which is IMHO ok.
>
> > Boot-strapped and regression-tested on x86_64-linux-gnu.
> >
> > Ok for trunk?
>
> Well, I'm not so sure. And if so then I'd prefer martins original
> patch, rejecting all zero-sized fields. But then only if you
> consider it a "real" field.
>
> Of course I blame those that added the zero-sized array extension
> for all this mess ... maybe we can reduce it by rejecting
> zero-sized arrays that are not at the end of a structure - which means
> we can demote them to flexible arrays with a NULL TYPE_SIZE
> which would completely side-step this issue in stor-layout.c.
>
> Richard.
>
> > Regards
> > 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