questionable checks for flexible array members in c-ubsan.c and tree-vrp.c (was: Re: array bounds, sanitizer, safe programming, and cilk array notation)

[Martin Uecker <uecker at eecs dot berkeley dot edu>]

Martin Uecker <uecker@eecs.berkeley.edu>:
> Marek Polacek <polacek@redhat.com>:
> 
> > > void foo(int (*x)[4])
> > > {
> > > 	(*x)[4] = 5;	// warning
> > > }
> >  
> > This is detected by -fsanitize=object-size, turned on by default in
> > -fsanitize=undefined.  Since it makes use of __builtin_object_size,
> > it is necessary to compile with optimizations turned on.
> 
> Not always.  -fsanitize=object-size  does not seem to detect the
> illegal out-of-bound access by itself, but instead uses some very 
> coarse notion of object size. For example, the following was not
> detected in my tests:

So I found some time to look at this. It seems this should be
detected by -fsanitize=bounds. But it isn't. The problem is in 
'gcc/c-family/c-ubsan.c' in 'ubsan_instrument_bounds'. The
code starting with

 /* Detect flexible array members and suchlike.  */
  tree base = get_base_address (array);
  if (base && (TREE_CODE (base) == INDIRECT_REF
               || TREE_CODE (base) == MEM_REF))
    {

also somehow prevents accesses via pointers to arrays
to be instrumented. This is bad because it eliminates
a lot of useful checks.

The code looks very similar to the code in 'tree-vrp.c' 
where similar checks prevented corresponding compile-time 
warnings. There I recently added a condition 
(warn_array_bounds < 2) to get the warnings atleast
for -Warray-bounds=2.

But looking at both places some more, the code simply seems wrong 
to me. There should be no need at all in the backend to 
somehow detect the case of flexible array members. Not having 
the frontend set an expression for the bound should be enough.

So I removed both code paths completely, and - so far - couldn't
trigger false positives for Warray-bounds or -fsanitize-bounds.
I attached a file with some tests. lines marked with 'ubsan'
are currently detected while lines with 'ubsan!' are only detected
after removing the test for flexible array members.

Am I missing something?

Martin





/* { dg-do compile } */
/* { dg-options "-O3 -Warray-bounds=2" } */

extern void* malloc(unsigned long x);

int e[3];

struct f { int f[3]; };

extern void bar(int v[]);
extern void barn(int n, int v[]);

struct h {

	int i;
	int j[];
};

struct h0 {

	int i;
	int j[0];
};

struct h0b {

	int i;
	int j[0];
	int k;
};

struct h1 {

	int i;
	int j[1];
};

struct h1b {

	int i;
	int j[1];
	int k;
};

struct h3 {

	int i;
	int j[3];
};

struct h3b {

	int i;
	int j[3];
	int k;
};

void foo(int (*a)[3], int n, int (*b)[n])
{
	(*a)[3] = 1;	/* { dg-warning "subscript is above array bound" } */	/* ubsan! */
	a[0][0] = 1;	// ok							
	a[1][0] = 1;	// ok
	a[1][3] = 1;	/* { dg-warning "subscript is above array bound" } */	/* ubsan! */

	(*b)[n] = 1; // !							/* ubsan! */

	int c[3] = { 0 };
	int d[n];

	c[3] = 1;	/* { dg-warning "subscript is above array bound" } */	/* ubsan */
	d[n] = 1;	// !							/* ubsan */

	e[3] = 1;	/* { dg-warning "subscript is above array bound" } */	/* ubsan */

	struct f f;
	f.f[3] = 1;	/* { dg-warning "subscript is above array bound" } */	/* ubsan */

	int m = 3;
	int g[m];
	g[3] = 1;	// !							/* ubsan */

	struct h* h = malloc(sizeof(struct h) + 3 * sizeof(int));
	struct h0* h0 = malloc(sizeof(struct h0) + 3 * sizeof(int));
	struct h1* h1 = malloc(sizeof(struct h1) + 3 * sizeof(int));
	struct h3* h3 = malloc(sizeof(struct h3));

	h->j[3] = 1;	// flexible array member
	h0->j[3] = 1;	// zero-sized array extension
	h1->j[3] = 1;	/* { dg-warning "subscript is above array bound" } */	/* ubsan! */
	h3->j[3] = 1;	/* { dg-warning "subscript is above array bound" } */	/* ubsan! */


	struct h0b* h0b = malloc(sizeof(struct h) + 3 * sizeof(int));
	struct h1b* h1b = malloc(sizeof(struct h1b) + 3 * sizeof(int));
	struct h3b* h3b = malloc(sizeof(struct h3b));
	h0b->j[3] = 1;	// warning ?
	h1b->j[3] = 1;;	/* { dg-warning "subscript is above array bound" } */	/* ubsan */
	h3b->j[3] = 1;;	/* { dg-warning "subscript is above array bound" } */	/* ubsan */

	struct hn {

		int i;
		int j[n];
	} hn;

	hn.j[3] = 1;	// !			/* ubsan */

	struct hnb {

		int i;
		int j[n];
		int k;
	} hnb;

	hnb.j[3] = 1;	// !			/* ubsan */
}


int main()
{
	int a[3];
	foo(&a, 3, &a);
}