[prev in list] [next in list] [prev in thread] [next in thread]
List: gcc-patches
Subject: Re: [PATCH][RFC] Add a subset of -Warray-bounds warnings to C/C++ front ends
From: Simon Baldwin <simonb () google ! com>
Date: 2008-04-11 19:46:34
Message-ID: 47FFC01A.6050000 () google ! com
[Download RAW message or body]
Thanks for the feedback.
I've incorporated most comments, and a revised patch is attached below.
One thing I wasn't able to fathom was how to suppress warnings about
"sizeof(a[-1])" in the C and C++ frontends; if anyone's got any hints on
this, I'd love to hear them.
In testing, the new version passes bootstrap and gcc tests, and also the
3k C++ source files real world test.
When compiling libc 2.4 with it, two new warnings arise, in optimized
(tree-vrp) builds only, so from tree-vrp.c:check_array_ref():
digits_dots.c: In function '__nss_hostname_digits_dots':
digits_dots.c:159: warning: array subscript is above array bounds
digits_dots.c:291: warning: array subscript is above array bounds
Abstracting out the problematic parts of this gives:
void func(void) {
typedef unsigned char host_addr_t[16];
host_addr_t *host_addr;
typedef char *host_addr_list_t[2];
host_addr_list_t *h_addr_ptrs;
char **h_alias_ptr;
host_addr = (host_addr_t *) 0;
h_addr_ptrs = (host_addr_list_t *) ((char *) host_addr + sizeof
(*host_addr));
h_alias_ptr = (char **) ((char *) h_addr_ptrs + sizeof (*h_addr_ptrs));
h_alias_ptr[0] = ((void *)0); // Warning here
}
This doesn't show up with un-patched gcc because h_alias_ptr appears in
tree-vrp as having dimension 2, and prior to this patch
check_array_ref() let a[2], as well as a[1] and a[0], slide by
unchecked. I'm wondering if check_array_ref() doesn't actually have a
bona-fide complaint here.
["bounds.patch" (text/x-patch)]
This is a modest patch to provide a subset of the -Warray-bounds warnings
from tree-vrp.c in the C and C++ front ends. This permits the compiler to
warn about egregious array bounds violations in unoptimized compilations or
compilations that may use -fno-tree-vrp. At present, array bounds checking
is only done on optimized compilations.
A side effect of copying these warnings up into the language frontends is
that warnings are now printed even if the array access is in dead or
inaccessible code.
The current array bounds tests are modified to account for this new checking,
and additionally there are two new tests for warnings from -O0 compilations,
one for C and one for C++.
Bootstrapped, and regression tested on i686 Linux for gcc and g++.
:ADDPATCH diagnostic:
gcc/ChangeLog
2008-04-09 Simon Baldwin <simonb@google.com>
* 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.
gcc/cp/ChangeLog
2008-04-09 Simon Baldwin <simonb@google.com>
* typeck.c (build_array_ref): Call warn_array_subscript_range.
gcc/testsuite/ChangeLog
2008-04-09 Simon Baldwin <simonb@google.com>
* 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.
Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c (revision 133772)
+++ gcc/tree-vrp.c (working copy)
@@ -4540,8 +4540,8 @@
&& 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);
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi (revision 133772)
+++ gcc/doc/invoke.texi (working copy)
@@ -2658,7 +2658,7 @@
@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
@@ -3361,7 +3361,8 @@
@item -Warray-bounds
@opindex Wno-array-bounds
@opindex Warray-bounds
-This option is only active when @option{-ftree-vrp} is active
+This option performs a subset of checks in unoptimized compilations, and
+stricter checking 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}.
Index: gcc/testsuite/gcc.dg/Warray-bounds.c
===================================================================
--- gcc/testsuite/gcc.dg/Warray-bounds.c (revision 133772)
+++ gcc/testsuite/gcc.dg/Warray-bounds.c (working copy)
@@ -17,6 +17,7 @@
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 @@
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]);
@@ -71,23 +72,52 @@
g(&a[-1]); /* { dg-warning "array subscript" } */
g(&b[-1]); /* { dg-warning "array subscript" } */
- h(sizeof a[-1]);
+ h(sizeof a[-1]); /* { dg-warning "array subscript" } */
h(sizeof a[10]);
- h(sizeof b[-1]);
+ h(sizeof b[-1]); /* { dg-warning "array subscript" } */
h(sizeof b[10]);
- h(sizeof c.c[-1]);
+ h(sizeof c.c[-1]); /* { dg-warning "array subscript" } */
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;
}
-
Index: gcc/testsuite/g++.dg/warn/Warray-bounds.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Warray-bounds.C (revision 133772)
+++ gcc/testsuite/g++.dg/warn/Warray-bounds.C (working copy)
@@ -17,6 +17,7 @@
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 @@
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" } */
@@ -72,20 +72,52 @@
g(&a[-1]); /* { dg-warning "array subscript" } */
g(&b[-1]); /* { dg-warning "array subscript" } */
- h(sizeof a[-1]);
+ h(sizeof a[-1]); /* { dg-warning "array subscript" } */
h(sizeof a[10]);
- h(sizeof b[-1]);
+ h(sizeof b[-1]); /* { dg-warning "array subscript" } */
h(sizeof b[10]);
- h(sizeof c.c[-1]);
+ h(sizeof c.c[-1]); /* { dg-warning "array subscript" } */
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;
}
-
Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c (revision 133772)
+++ gcc/cp/typeck.c (working copy)
@@ -2554,7 +2554,8 @@
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);
@@ -2571,6 +2572,10 @@
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.
@@ -2621,7 +2626,12 @@
|= (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;
}
{
Index: gcc/c-typeck.c
===================================================================
--- gcc/c-typeck.c (revision 133772)
+++ gcc/c-typeck.c (working copy)
@@ -2086,7 +2086,12 @@
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 @@
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
{
Index: gcc/c-common.c
===================================================================
--- gcc/c-common.c (revision 133772)
+++ gcc/c-common.c (working copy)
@@ -7281,6 +7281,59 @@
warning (OPT_Wchar_subscripts, "array subscript has type %<char%>");
}
+/* 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 (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
Index: gcc/c-common.h
===================================================================
--- gcc/c-common.h (revision 133772)
+++ gcc/c-common.h (working copy)
@@ -884,6 +884,7 @@
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);
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic