Hi, 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. Regression tested. 2015-02-25 Martin Uecker <uec...@eecs.berkeley.edu> gcc/ * c-family/c-ubsan.c (ubsan_instrument_bounds): Remove bogus check for flexible array members. gcc/testsuite/ * gcc.dg/ubsan/bounds-2.c: New * c-c++-common/ubsan/bounds-8.c: New diff --git a/gcc/c-family/c-ubsan.c b/gcc/c-family/c-ubsan.c index 90d59c0..dba3bbc 100644 --- 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 */ if (domain == NULL_TREE || TYPE_MAX_VALUE (domain) == NULL_TREE) return NULL_TREE; @@ -301,36 +302,6 @@ ubsan_instrument_bounds (location_t loc, tree array, tree *index, bound = fold_build2 (PLUS_EXPR, TREE_TYPE (bound), bound, build_int_cst (TREE_TYPE (bound), 1)); - /* Detect flexible array members and suchlike. */ - tree base = get_base_address (array); - if (base && (TREE_CODE (base) == INDIRECT_REF - || TREE_CODE (base) == MEM_REF)) - { - tree next = NULL_TREE; - tree cref = array; - - /* Walk all structs/unions. */ - while (TREE_CODE (cref) == COMPONENT_REF) - { - if (TREE_CODE (TREE_TYPE (TREE_OPERAND (cref, 0))) == RECORD_TYPE) - for (next = DECL_CHAIN (TREE_OPERAND (cref, 1)); - next && TREE_CODE (next) != FIELD_DECL; - next = DECL_CHAIN (next)) - ; - if (next) - /* Not a last element. Instrument it. */ - break; - /* Ok, this is the last field of the structure/union. But the - aggregate containing the field must be the last field too, - recursively. */ - cref = TREE_OPERAND (cref, 0); - } - if (!next) - /* Don't instrument this flexible array member-like array in non-strict - -fsanitize=bounds mode. */ - return NULL_TREE; - } - /* Don't emit instrumentation in the most common cases. */ tree idx = NULL_TREE; if (TREE_CODE (*index) == INTEGER_CST) diff --git a/gcc/testsuite/c-c++-common/ubsan/bounds-8.c b/gcc/testsuite/c-c++-common/ubsan/bounds-8.c new file mode 100644 index 0000000..554693f --- /dev/null +++ b/gcc/testsuite/c-c++-common/ubsan/bounds-8.c @@ -0,0 +1,26 @@ +/* { dg-do run } */ +/* { dg-options "-O3 -fsanitize=bounds" } */ +/* Origin: Martin Uecker <uec...@eecs.berkeley.edu> */ + +#ifdef __cplusplus +extern "C" void* malloc(unsigned long x); +#else +extern void* malloc(unsigned long x); +#endif + +void foo(int (*a)[3]) +{ + (*a)[3] = 1; // error + a[0][0] = 1; // ok + a[1][0] = 1; // ok + a[1][4] = 1; // error +} + +int main() +{ + int a[20]; + foo((int (*)[3])&a); +} + +/* { dg-output "index 3 out of bounds for type 'int \\\[3\\\]'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*index 4 out of bounds for type 'int \\\[3\\\]'" } */ diff --git a/gcc/testsuite/gcc.dg/ubsan/bounds-2.c b/gcc/testsuite/gcc.dg/ubsan/bounds-2.c new file mode 100644 index 0000000..722c54e --- /dev/null +++ b/gcc/testsuite/gcc.dg/ubsan/bounds-2.c @@ -0,0 +1,18 @@ +/* { dg-do run } */ +/* { dg-options "-O3 -fsanitize=bounds" } */ +/* Origin: Martin Uecker <uec...@eecs.berkeley.edu> */ + +extern void* malloc(unsigned long x); + +void foo(int n, int (*b)[n]) +{ + (*b)[n] = 1; // error +} + +int main() +{ + int a[20]; + foo(3, (int (*)[3])&a); +} + +/* { dg-output "index 3 out of bounds for type 'int \\\[\\\*\\\]'" } */