This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] ubsan: remove bogus check for flexible array members
- From: Marek Polacek <polacek at redhat dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: Jakub Jelinek <jakub at redhat dot com>, Martin Uecker <uecker at eecs dot berkeley dot edu>, gcc Mailing List <gcc-patches at gcc dot gnu dot org>, Joseph Myers <joseph at codesourcery dot com>
- Date: Thu, 26 Feb 2015 15:57:49 +0100
- Subject: Re: [PATCH] ubsan: remove bogus check for flexible array members
- Authentication-results: sourceware.org; auth=none
- References: <20150225220107 dot 5ce6e4a5 at lemur> <20150226063654 dot GY1746 at tucnak dot redhat dot com> <20150226073235 dot GB22272 at redhat dot com> <CAFiYyc2TD_xOY55qiovQuG3=xSF816zeVJQdD3BfrHMyB0a20g at mail dot gmail dot com>
On Thu, Feb 26, 2015 at 11:08:04AM +0100, Richard Biener wrote:
> > --- gcc/c-family/c-ubsan.c
> > +++ gcc/c-family/c-ubsan.c
> > @@ -303,8 +303,9 @@ ubsan_instrument_bounds (location_t loc, tree array, tree *index,
> >
> > /* Detect flexible array members and suchlike. */
> > tree base = get_base_address (array);
> > - if (base && (TREE_CODE (base) == INDIRECT_REF
> > - || TREE_CODE (base) == MEM_REF))
> > + if (TREE_CODE (array) == COMPONENT_REF
>
> Err - this doesn't detect
>
> int
> main (void)
> {
> int *t = (int *) __builtin_malloc (sizeof (int) * 10);
> int (*a)[1] = (int (*)[1])t;
> (*a)[2] = 1;
> }
>
> that is a trailing array VLA.
>
> What I've definitely seen is
>
> int
> main (void)
> {
> int *t = (int *) __builtin_malloc (sizeof (int) * 9);
> int (*a)[3][3] = (int (*)[3][3])t;
> (*a)[0][9] = 1;
> }
I think we should error on those.
With my patch we'd emit the same -fsanitize=bounds runtime errors as clang
does.
> that is, assume that the array dimension with the fast running
> index "wraps" over into the next (hello SPEC CPU 2006!).
I think they're invoking UB then.
> > + && base && (TREE_CODE (base) == INDIRECT_REF
> > + || TREE_CODE (base) == MEM_REF))
> > {
> > tree next = NULL_TREE;
> > tree cref = array;
> >
> > I think it is a bug that we're playing games on something that is not
> > an element of a structure.
>
> Not sure about this.
The comment says that we're trying to detect a flexible array member
there - and those can't be outside struct. I certainly hadn't anything
else in my mind when I was writing that ;).
Marek