From ca873b0e5aafd6c0fce9e00719671c6aa25c708b Mon Sep 17 00:00:00 2001 From: Simon Baldwin Date: Thu, 1 May 2008 19:03:32 +0000 Subject: [PATCH] c-common.h (warn_array_subscript_range): New function. * c-common.h (warn_array_subscript_range): New function. * c-common.c (warn_array_subscript_range): Ditto. * tree-vrp.c (check_array_ref): Corrected code to agree with comment, ignoring only arrays of size 0 or size 1. * c-typeck.c (build_array_ref): Call warn_array_subscript_range. * testsuite/gcc.dg/Warray-bounds.c: Updated for frontend warnings, additional tests for arrays of size 0 and size 1. * testsuite/g++.dg/warn/Warray-bounds.c: Ditto. * testsuite/gcc.dg/Warray-bounds-noopt.c: New testcase. * testsuite/g++.dg/warn/Warray-bounds-noopt.c: Ditto. * typeck.c (build_array_ref): Call warn_array_subscript_range. From-SVN: r134865 --- gcc/ChangeLog | 8 ++ gcc/c-common.c | 54 ++++++++ gcc/c-common.h | 1 + gcc/c-typeck.c | 14 +- gcc/cp/ChangeLog | 4 + gcc/cp/typeck.c | 14 +- gcc/doc/invoke.texi | 9 +- gcc/testsuite/ChangeLog | 8 ++ .../g++.dg/warn/Warray-bounds-noopt.C | 123 ++++++++++++++++++ gcc/testsuite/g++.dg/warn/Warray-bounds.C | 42 +++++- gcc/testsuite/gcc.dg/Warray-bounds-noopt.c | 123 ++++++++++++++++++ gcc/testsuite/gcc.dg/Warray-bounds.c | 42 +++++- gcc/tree-vrp.c | 4 +- 13 files changed, 425 insertions(+), 21 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/Warray-bounds-noopt.C create mode 100644 gcc/testsuite/gcc.dg/Warray-bounds-noopt.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 0ffe99b9ebe..fc9095efa6c 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,11 @@ +2008-05-01 Simon Baldwin + + * c-common.h (warn_array_subscript_range): New function. + * c-common.c (warn_array_subscript_range): Ditto. + * tree-vrp.c (check_array_ref): Corrected code to agree with + comment, ignoring only arrays of size 0 or size 1. + * c-typeck.c (build_array_ref): Call warn_array_subscript_range. + 2008-05-01 H.J. Lu * config/i386/i386.c (ix86_builtin_type): Replace diff --git a/gcc/c-common.c b/gcc/c-common.c index 58585239680..3fa16492658 100644 --- a/gcc/c-common.c +++ b/gcc/c-common.c @@ -7452,6 +7452,60 @@ warn_array_subscript_with_type_char (tree index) warning (OPT_Wchar_subscripts, "array subscript has type %"); } +/* Warn about obvious array bounds errors for fixed size arrays that + are indexed by a constant. This is a subset of similar checks in + tree-vrp.c; by doing this here we can get some level of checking + from non-optimized, non-vrp compilation. Returns true if a warning + is issued. */ + +bool +warn_array_subscript_range (const_tree array, const_tree index) +{ + if (skip_evaluation == 0 + && TREE_CODE (TREE_TYPE (array)) == ARRAY_TYPE + && TYPE_DOMAIN (TREE_TYPE (array)) && TREE_CODE (index) == INTEGER_CST) + { + const_tree max_index; + + max_index = TYPE_MAX_VALUE (TYPE_DOMAIN (TREE_TYPE (array))); + if (max_index && TREE_CODE (max_index) == INTEGER_CST + && tree_int_cst_lt (max_index, index) + && !tree_int_cst_equal (index, max_index) + /* Always allow off-by-one. */ + && !tree_int_cst_equal (int_const_binop (PLUS_EXPR, + max_index, + integer_one_node, + 0), + index) + /* Accesses after the end of arrays of size 0 (gcc + extension) and 1 are likely intentional ("struct + hack"). Note that max_index is array dimension - 1. */ + && compare_tree_int (max_index, 1) >= 0) + { + warning (OPT_Warray_bounds, + "array subscript is above array bounds"); + return true; + } + else + { + const_tree min_index; + + min_index = TYPE_MIN_VALUE (TYPE_DOMAIN (TREE_TYPE (array))); + if (min_index && TREE_CODE (min_index) == INTEGER_CST + && tree_int_cst_lt (index, min_index)) + { + warning (OPT_Warray_bounds, + compare_tree_int (min_index, 0) == 0 + ? "array subscript is negative" + : "array subscript is below array bounds"); + return true; + } + } + } + + return false; +} + /* Implement -Wparentheses for the unexpected C precedence rules, to cover cases like x + y << z which readers are likely to misinterpret. We have seen an expression in which CODE is a binary diff --git a/gcc/c-common.h b/gcc/c-common.h index c403bee6343..4f4a414c6d3 100644 --- a/gcc/c-common.h +++ b/gcc/c-common.h @@ -893,6 +893,7 @@ extern int complete_array_type (tree *, tree, bool); extern tree builtin_type_for_size (int, bool); extern void warn_array_subscript_with_type_char (tree); +extern bool warn_array_subscript_range (const_tree, const_tree); extern void warn_about_parentheses (enum tree_code, enum tree_code, enum tree_code); extern void warn_for_unused_label (tree label); diff --git a/gcc/c-typeck.c b/gcc/c-typeck.c index 2f8428210d7..dc4cb2b95af 100644 --- a/gcc/c-typeck.c +++ b/gcc/c-typeck.c @@ -2086,7 +2086,12 @@ build_array_ref (tree array, tree index) if (TREE_CODE (TREE_TYPE (array)) == ARRAY_TYPE) { - tree rval, type; + tree rval, type, ref; + bool has_warned_on_bounds_check = false; + + /* Warn about any obvious array bounds errors for fixed size arrays that + are indexed by a constant. */ + has_warned_on_bounds_check = warn_array_subscript_range (array, index); /* An array that is indexed by a non-constant cannot be stored in a register; we must be able to do @@ -2139,7 +2144,12 @@ build_array_ref (tree array, tree index) in an inline function. Hope it doesn't break something else. */ | TREE_THIS_VOLATILE (array)); - return require_complete_type (fold (rval)); + ref = require_complete_type (fold (rval)); + + /* Suppress bounds warning in tree-vrp.c if already warned here. */ + if (has_warned_on_bounds_check) + TREE_NO_WARNING (ref) = 1; + return ref; } else { diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 52892226720..29b70d02ad4 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,7 @@ +2008-05-01 Simon Baldwin + + * typeck.c (build_array_ref): Call warn_array_subscript_range. + 2008-04-30 Jakub Jelinek PR c++/35986 diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index bf264ad2cc7..1447182ed01 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -2556,7 +2556,8 @@ build_array_ref (tree array, tree idx) if (TREE_CODE (TREE_TYPE (array)) == ARRAY_TYPE) { - tree rval, type; + bool has_warned_on_bounds_check = false; + tree rval, type, ref; warn_array_subscript_with_type_char (idx); @@ -2573,6 +2574,10 @@ build_array_ref (tree array, tree idx) pointer arithmetic.) */ idx = perform_integral_promotions (idx); + /* Warn about any obvious array bounds errors for fixed size arrays that + are indexed by a constant. */ + has_warned_on_bounds_check = warn_array_subscript_range (array, idx); + /* An array that is indexed by a non-constant cannot be stored in a register; we must be able to do address arithmetic on its address. @@ -2623,7 +2628,12 @@ build_array_ref (tree array, tree idx) |= (CP_TYPE_VOLATILE_P (type) | TREE_SIDE_EFFECTS (array)); TREE_THIS_VOLATILE (rval) |= (CP_TYPE_VOLATILE_P (type) | TREE_THIS_VOLATILE (array)); - return require_complete_type (fold_if_not_in_template (rval)); + ref = require_complete_type (fold_if_not_in_template (rval)); + + /* Suppress bounds warning in tree-vrp.c if already warned here. */ + if (has_warned_on_bounds_check) + TREE_NO_WARNING (ref) = 1; + return ref; } { diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index dfd22afab6c..c147219ad1b 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -2679,7 +2679,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}. @option{-Wall} turns on the following warning flags: @gccoptlist{-Waddress @gol --Warray-bounds @r{(only with} @option{-O2}@r{)} @gol +-Warray-bounds @r{(some checks, but more complete with} @option{-O2}@r{)} @gol -Wc++0x-compat @gol -Wchar-subscripts @gol -Wimplicit-int @gol @@ -3382,9 +3382,10 @@ false positives. @item -Warray-bounds @opindex Wno-array-bounds @opindex Warray-bounds -This option is only active when @option{-ftree-vrp} is active -(default for -O2 and above). It warns about subscripts to arrays -that are always out of bounds. This warning is enabled by @option{-Wall}. +This option detects some cases of out-of-bounds accesses in unoptimized +compilations. More cases are detected when @option{-ftree-vrp} is enabled. +(The @option{-ftree-vrp} option is enabled automatically when compiling with +@option{-O2} or higher optimization options.) @item -Wno-div-by-zero @opindex Wno-div-by-zero diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 36ebbf96f3d..c4ef1364b47 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,11 @@ +2008-05-01 Simon Baldwin + + * testsuite/gcc.dg/Warray-bounds.c: Updated for frontend warnings, + additional tests for arrays of size 0 and size 1. + * testsuite/g++.dg/warn/Warray-bounds.c: Ditto. + * testsuite/gcc.dg/Warray-bounds-noopt.c: New testcase. + * testsuite/g++.dg/warn/Warray-bounds-noopt.c: Ditto. + 2008-05-01 Richard Guenther PR middle-end/36093 diff --git a/gcc/testsuite/g++.dg/warn/Warray-bounds-noopt.C b/gcc/testsuite/g++.dg/warn/Warray-bounds-noopt.C new file mode 100644 index 00000000000..650f2690e5c --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Warray-bounds-noopt.C @@ -0,0 +1,123 @@ +/* { dg-do compile } */ +/* { dg-options "-O0 -Warray-bounds" } */ + +int a[10]; + +static inline int n(void) { + __SIZE_TYPE__ strlen(const char *s); + return strlen("12345"); +} + +void g(int *p); +void h(int p); + +int* f(void) { + int b[10]; + int i; + struct { + int c[10]; + } c; + int p[0], q[1], r[2], s[3], t[4]; + + a[-1] = 0; /* { dg-warning "array subscript" } */ + a[ 0] = 0; + a[ 1] = 0; + + + a[ 9] = 0; + a[10] = 0; + a[11] = 0; /* { dg-warning "array subscript" } */ + a[2 * n() - 11] = 0; + a[2 * n() - 10] = 0; + a[2 * n() - 1] = 0; + a[2 * n() - 0] = 0; + + b[-1] = 0; /* { dg-warning "array subscript" } */ + b[ 0] = 0; + b[ 1] = 0; + b[ 9] = 0; + b[10] = 0; + b[11] = 0; /* { dg-warning "array subscript" } */ + b[2 * n() - 11] = 0; + b[2 * n() - 10] = 0; + b[2 * n() - 1] = 0; + b[2 * n() - 0] = 0; + + c.c[-1] = 0; /* { dg-warning "array subscript" } */ + c.c[ 0] = 0; + c.c[ 1] = 0; + c.c[ 9] = 0; + c.c[10] = 0; + c.c[11] = 0; /* { dg-warning "array subscript" } */ + c.c[2 * n() - 11] = 0; + c.c[2 * n() - 10] = 0; + c.c[2 * n() - 1] = 0; + c.c[2 * n() - 0] = 0; + + g(&a[8]); + g(&a[9]); + g(&a[10]); + g(&a[11]); /* { dg-warning "array subscript" } */ + g(&a[-30]+10); /* { dg-warning "array subscript" } */ + g(&a[-30]+30); /* { dg-warning "array subscript" } */ + + g(&b[10]); + g(&c.c[10]); + g(&b[11]); /* { dg-warning "array subscript" } */ + g(&c.c[11]); /* { dg-warning "array subscript" } */ + + g(&a[0]); + g(&b[0]); + g(&c.c[0]); + + g(&a[-1]); /* { dg-warning "array subscript" } */ + g(&b[-1]); /* { dg-warning "array subscript" } */ + h(sizeof a[-1]); + h(sizeof a[10]); + h(sizeof b[-1]); + h(sizeof b[10]); + h(sizeof c.c[-1]); + h(sizeof c.c[10]); + + p[-1] = 0; /* { dg-warning "array subscript" } */ + p[0] = 0; + p[1] = 0; + + q[-1] = 0; /* { dg-warning "array subscript" } */ + q[0] = 0; + q[1] = 0; + q[2] = 0; + + r[-1] = 0; /* { dg-warning "array subscript" } */ + r[0] = 0; + r[1] = 0; + r[2] = 0; + r[3] = 0; /* { dg-warning "array subscript" } */ + + s[-1] = 0; /* { dg-warning "array subscript" } */ + s[0] = 0; + s[1] = 0; + s[2] = 0; + s[3] = 0; + s[4] = 0; /* { dg-warning "array subscript" } */ + + t[-1] = 0; /* { dg-warning "array subscript" } */ + t[0] = 0; + t[1] = 0; + t[2] = 0; + t[3] = 0; + t[4] = 0; + t[5] = 0; /* { dg-warning "array subscript" } */ + + if (10 < 10) + a[10] = 0; + if (10 < 10) + b[10] = 0; + if (-1 >= 0) + c.c[-1] = 0; /* { dg-warning "array subscript" } */ + + for (i = 20; i < 30; ++i) + a[i] = 1; + + return a; +} diff --git a/gcc/testsuite/g++.dg/warn/Warray-bounds.C b/gcc/testsuite/g++.dg/warn/Warray-bounds.C index 0385516abd4..02fd8171869 100644 --- a/gcc/testsuite/g++.dg/warn/Warray-bounds.C +++ b/gcc/testsuite/g++.dg/warn/Warray-bounds.C @@ -17,6 +17,7 @@ int* f(void) { struct { int c[10]; } c; + int p[0], q[1], r[2], s[3], t[4]; a[-1] = 0; /* { dg-warning "array subscript" } */ a[ 0] = 0; @@ -57,12 +58,11 @@ int* f(void) { g(&a[9]); g(&a[10]); g(&a[11]); /* { dg-warning "array subscript" } */ - g(&a[-30]+10); /* { dg-warning "array subscript" } */ - g(&a[-30]+30); + g(&a[-30]+10); /* { dg-warning "array subscript" } */ + g(&a[-30]+30); /* { dg-warning "array subscript" } */ g(&b[10]); g(&c.c[10]); - g(&a[11]); /* { dg-warning "array subscript" } */ g(&b[11]); /* { dg-warning "array subscript" } */ g(&c.c[11]); /* { dg-warning "array subscript" } */ @@ -79,13 +79,45 @@ int* f(void) { h(sizeof c.c[-1]); h(sizeof c.c[10]); + p[-1] = 0; /* { dg-warning "array subscript" } */ + p[0] = 0; + p[1] = 0; + + q[-1] = 0; /* { dg-warning "array subscript" } */ + q[0] = 0; + q[1] = 0; + q[2] = 0; + + r[-1] = 0; /* { dg-warning "array subscript" } */ + r[0] = 0; + r[1] = 0; + r[2] = 0; + r[3] = 0; /* { dg-warning "array subscript" } */ + + s[-1] = 0; /* { dg-warning "array subscript" } */ + s[0] = 0; + s[1] = 0; + s[2] = 0; + s[3] = 0; + s[4] = 0; /* { dg-warning "array subscript" } */ + + t[-1] = 0; /* { dg-warning "array subscript" } */ + t[0] = 0; + t[1] = 0; + t[2] = 0; + t[3] = 0; + t[4] = 0; + t[5] = 0; /* { dg-warning "array subscript" } */ + if (10 < 10) a[10] = 0; if (10 < 10) b[10] = 0; if (-1 >= 0) - c.c[-1] = 0; + c.c[-1] = 0; /* { dg-warning "array subscript" } */ + + for (i = 20; i < 30; ++i) + a[i] = 1; /* { dg-warning "array subscript" } */ return a; } - diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-noopt.c b/gcc/testsuite/gcc.dg/Warray-bounds-noopt.c new file mode 100644 index 00000000000..650f2690e5c --- /dev/null +++ b/gcc/testsuite/gcc.dg/Warray-bounds-noopt.c @@ -0,0 +1,123 @@ +/* { dg-do compile } */ +/* { dg-options "-O0 -Warray-bounds" } */ + +int a[10]; + +static inline int n(void) { + __SIZE_TYPE__ strlen(const char *s); + return strlen("12345"); +} + +void g(int *p); +void h(int p); + +int* f(void) { + int b[10]; + int i; + struct { + int c[10]; + } c; + int p[0], q[1], r[2], s[3], t[4]; + + a[-1] = 0; /* { dg-warning "array subscript" } */ + a[ 0] = 0; + a[ 1] = 0; + + + a[ 9] = 0; + a[10] = 0; + a[11] = 0; /* { dg-warning "array subscript" } */ + a[2 * n() - 11] = 0; + a[2 * n() - 10] = 0; + a[2 * n() - 1] = 0; + a[2 * n() - 0] = 0; + + b[-1] = 0; /* { dg-warning "array subscript" } */ + b[ 0] = 0; + b[ 1] = 0; + b[ 9] = 0; + b[10] = 0; + b[11] = 0; /* { dg-warning "array subscript" } */ + b[2 * n() - 11] = 0; + b[2 * n() - 10] = 0; + b[2 * n() - 1] = 0; + b[2 * n() - 0] = 0; + + c.c[-1] = 0; /* { dg-warning "array subscript" } */ + c.c[ 0] = 0; + c.c[ 1] = 0; + c.c[ 9] = 0; + c.c[10] = 0; + c.c[11] = 0; /* { dg-warning "array subscript" } */ + c.c[2 * n() - 11] = 0; + c.c[2 * n() - 10] = 0; + c.c[2 * n() - 1] = 0; + c.c[2 * n() - 0] = 0; + + g(&a[8]); + g(&a[9]); + g(&a[10]); + g(&a[11]); /* { dg-warning "array subscript" } */ + g(&a[-30]+10); /* { dg-warning "array subscript" } */ + g(&a[-30]+30); /* { dg-warning "array subscript" } */ + + g(&b[10]); + g(&c.c[10]); + g(&b[11]); /* { dg-warning "array subscript" } */ + g(&c.c[11]); /* { dg-warning "array subscript" } */ + + g(&a[0]); + g(&b[0]); + g(&c.c[0]); + + g(&a[-1]); /* { dg-warning "array subscript" } */ + g(&b[-1]); /* { dg-warning "array subscript" } */ + h(sizeof a[-1]); + h(sizeof a[10]); + h(sizeof b[-1]); + h(sizeof b[10]); + h(sizeof c.c[-1]); + h(sizeof c.c[10]); + + p[-1] = 0; /* { dg-warning "array subscript" } */ + p[0] = 0; + p[1] = 0; + + q[-1] = 0; /* { dg-warning "array subscript" } */ + q[0] = 0; + q[1] = 0; + q[2] = 0; + + r[-1] = 0; /* { dg-warning "array subscript" } */ + r[0] = 0; + r[1] = 0; + r[2] = 0; + r[3] = 0; /* { dg-warning "array subscript" } */ + + s[-1] = 0; /* { dg-warning "array subscript" } */ + s[0] = 0; + s[1] = 0; + s[2] = 0; + s[3] = 0; + s[4] = 0; /* { dg-warning "array subscript" } */ + + t[-1] = 0; /* { dg-warning "array subscript" } */ + t[0] = 0; + t[1] = 0; + t[2] = 0; + t[3] = 0; + t[4] = 0; + t[5] = 0; /* { dg-warning "array subscript" } */ + + if (10 < 10) + a[10] = 0; + if (10 < 10) + b[10] = 0; + if (-1 >= 0) + c.c[-1] = 0; /* { dg-warning "array subscript" } */ + + for (i = 20; i < 30; ++i) + a[i] = 1; + + return a; +} diff --git a/gcc/testsuite/gcc.dg/Warray-bounds.c b/gcc/testsuite/gcc.dg/Warray-bounds.c index bbb5bea65e1..02fd8171869 100644 --- a/gcc/testsuite/gcc.dg/Warray-bounds.c +++ b/gcc/testsuite/gcc.dg/Warray-bounds.c @@ -17,6 +17,7 @@ int* f(void) { struct { int c[10]; } c; + int p[0], q[1], r[2], s[3], t[4]; a[-1] = 0; /* { dg-warning "array subscript" } */ a[ 0] = 0; @@ -56,13 +57,13 @@ int* f(void) { g(&a[8]); g(&a[9]); g(&a[10]); - g(&a[11]); /* { dg-warning "array subscript" "" { xfail *-*-* } } */ - g(&a[-30]+10); /* { dg-warning "array subscript" } */ - g(&a[-30]+30); + g(&a[11]); /* { dg-warning "array subscript" } */ + g(&a[-30]+10); /* { dg-warning "array subscript" } */ + g(&a[-30]+30); /* { dg-warning "array subscript" } */ g(&b[10]); g(&c.c[10]); - g(&b[11]); /* { dg-warning "array subscript" "" { xfail *-*-* } } */ + g(&b[11]); /* { dg-warning "array subscript" } */ g(&c.c[11]); /* { dg-warning "array subscript" } */ g(&a[0]); @@ -78,16 +79,45 @@ int* f(void) { h(sizeof c.c[-1]); h(sizeof c.c[10]); + p[-1] = 0; /* { dg-warning "array subscript" } */ + p[0] = 0; + p[1] = 0; + + q[-1] = 0; /* { dg-warning "array subscript" } */ + q[0] = 0; + q[1] = 0; + q[2] = 0; + + r[-1] = 0; /* { dg-warning "array subscript" } */ + r[0] = 0; + r[1] = 0; + r[2] = 0; + r[3] = 0; /* { dg-warning "array subscript" } */ + + s[-1] = 0; /* { dg-warning "array subscript" } */ + s[0] = 0; + s[1] = 0; + s[2] = 0; + s[3] = 0; + s[4] = 0; /* { dg-warning "array subscript" } */ + + t[-1] = 0; /* { dg-warning "array subscript" } */ + t[0] = 0; + t[1] = 0; + t[2] = 0; + t[3] = 0; + t[4] = 0; + t[5] = 0; /* { dg-warning "array subscript" } */ + if (10 < 10) a[10] = 0; if (10 < 10) b[10] = 0; if (-1 >= 0) - c.c[-1] = 0; + c.c[-1] = 0; /* { dg-warning "array subscript" } */ for (i = 20; i < 30; ++i) a[i] = 1; /* { dg-warning "array subscript" } */ return a; } - diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index 4b409e46aaf..61031996185 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -4645,8 +4645,8 @@ check_array_ref (tree ref, location_t* locus, bool ignore_off_by_one) && TYPE_MAX_VALUE (TYPE_DOMAIN (TREE_TYPE (ref))) == NULL_TREE) /* Accesses after the end of arrays of size 0 (gcc extension) and 1 are likely intentional ("struct - hack"). */ - || compare_tree_int (up_bound, 1) <= 0) + hack"). Note that up_bound is array dimension - 1. */ + || compare_tree_int (up_bound, 1) < 0) return; low_bound = array_ref_low_bound (ref); -- 2.30.2