This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][RFC] Add a subset of -Warray-bounds warnings to C/C++ front ends


Thanks to a well targeted pointer from Tom Tromey, I've now added the extra logic to suppress warnings about "sizeof(a[-1])" in the C and C++ frontends. Attached is the revised version.

So far I've been unable to reproduce the false warning from the current (unpatched) tree-vrp.c implementation, reported in gcc PR 35903. I'll keep trying.

In the meantime, go ahead and commit, leave in limbo, or consign to history? Thanks.


This is version 3 of a patch to provide a subset of -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++.

The patch also updates the current array bounds checking logic in tree-vrp.c
to agree with the code comments.

Bootstrapped, and regression tested on i686 Linux for gcc and g++.


:ADDPATCH diagnostic:

gcc/ChangeLog
2008-04-25  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-25  Simon Baldwin <simonb@google.com>

	* typeck.c (build_array_ref): Call warn_array_subscript_range.

gcc/testsuite/ChangeLog
2008-04-25  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]);
@@ -78,16 +79,45 @@
     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;
 }
-
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" } */
 
@@ -79,13 +79,45 @@
     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;
 }
-
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,60 @@
     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 (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
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);

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]