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. 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. > * c-family/c-ubsan.c (ubsan_instrument_bounds): Remove c-family/ shouldn't be in the ChangeLog entry, instead that part should go into c-family/ChangeLog. > --- a/gcc/c-family/c-ubsan.c > +++ b/gcc/c-family/c-ubsan.c > @@ -293,6 +293,7 @@ ubsan_instrument_bounds (location_t loc, tree array, tree > *index, > tree type = TREE_TYPE (array); > tree domain = TYPE_DOMAIN (type); > > + /* This also takes care of flexilbe array members */ Typo. > +#ifdef __cplusplus > +extern "C" void* malloc(unsigned long x); > +#else > +extern void* malloc(unsigned long x); > +#endif Why are you declaring malloc (wrongly), when it isn't used? Malloc argument is size_t aka __SIZE_TYPE__, not unsigned long. Jakub