This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]