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
