On Thu, Feb 26, 2015 at 07:36:54AM +0100, Jakub Jelinek wrote: > On Wed, Feb 25, 2015 at 10:01:07PM -0800, Martin Uecker wrote: > > this patch removes a bogus check for flexible array members > > which prevents array references to be instrumented in some > > interesting cases. Arrays accessed through pointers are now > > instrumented correctly. > > > > The check was unnecessary because flexible arrays are not > > instrumented anyway because of > > TYPE_MAX_VALUE (domain) == NULL_TREE. > > No, it is not bogus nor unnecessary. > This isn't about just real flexible arrays, but similar constructs, > C++ doesn't have flexible array members, nor C89, so people use the > GNU extension of struct S { ... ; char a[0]; } instead, or > use char a[1]; as the last member and still expect to be able to access > s->a[i] for i > 0 say on heap allocations etc. Yes, in other words, your patch would make a difference here:
int main (void) { struct S { int i; char a[1]; }; struct S *t = (struct S *) __builtin_malloc (sizeof (struct S) + 10); t->a[2] = 1; } (bounds-1.c test case should test this, too.) > I think we were talking at some point about having a strict-bounds or > something similar that would accept only real flexible array members and > nothing else and be more pedantic, at the disadvantage of diagnosing tons > of real-world code that is supported by GCC. > > Perhaps if the reference is just an array, not a struct containing > flexible-array-member-like array, we could consider it strict always, > but that is certainly not what your patch does. Martin, can you try whether this (untested) does what you're after? --- 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 + && 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. Marek