[PATCH] Fix PR/8268: implement compile time array subscript checking

[PATCH] Fix PR/8268: implement compile time array subscript checking

[Dirk Mueller]

Hi, 

The patch below implements PR/8268, which seems to be the one major diagnostic 
we're missing compared to icc (at least one openSUSE contributor regularly 
rebuilds all of the openSUSE distribution with icc just to report those 
warnings as bugs to us ;) ). 

bootstrapped and regtested with no additional failures many times on 
i686-suse-linux. 

Ok? Do we need the extra -Warray-bounds?


:ADDPATCH diagnostic:

2006-12-06  Dirk Mueller  <dmueller@suse.de>
	    Richard Guenther <rguenther@suse.de>

	PR diagnostic/8268
	* doc/invoke.texi (Warray-bounds): Document -Warray-bounds.
	* common.opt (Warray-bounds): Add new warning option.
	* c-opts.c (c_common_handle_option): Define -Warray-bounds
	if -Wall is given.
	* tree-vrp.c (vrp_finalize): Call check_array_refs if -Warray-bounds
	is enabled.
	(check_array_refs, check_array_bounds, check_array_ref): New.

        * testsuite/gcc.dg/Warray-bounds.c: New testcase.

--- doc/invoke.texi     (revision 119391)
+++ doc/invoke.texi     (working copy)
@@ -244,7 +244,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wreturn-type  -Wsequence-point  -Wshadow @gol
 -Wsign-compare  -Wstack-protector @gol
 -Wstrict-aliasing -Wstrict-aliasing=2 @gol
--Wstring-literal-comparison @gol
+-Wstring-literal-comparison -Warray-bounds @gol
 -Wswitch  -Wswitch-default  -Wswitch-enum @gol
 -Wsystem-headers  -Wtrigraphs  -Wundef  -Wuninitialized @gol
 -Wunknown-pragmas  -Wno-pragmas -Wunreachable-code @gol
@@ -2829,6 +2828,11 @@ compiler is using for optimization.  Thi
 @option{-Wstrict-aliasing}, but it will also give a warning for some 
ambiguous
 cases that are safe.

+@item -Warray-bounds
+@opindex Warray-bounds
+This option is only active when @option{-O1} or higher is active. It warns
+about constant subscripts in array accesses that are out of bounds.
+
 @item -Wall
 @opindex Wall
 All of the above @samp{-W} options combined.  This enables all the

--- common.opt	(revision 119391)
+++ common.opt	(working copy)
@@ -61,6 +61,10 @@ Walways-true
 Common Var(warn_always_true)
 Warn about comparisons that always evaluate to true
 
+Warray-bounds
+Common Var(warn_array_bounds)
+Warn if an array is accessed out of bounds
+
 Wattributes
 Common Var(warn_attributes) Init(1)
 Warn about inappropriate attribute usage
Index: c-opts.c
===================================================================
--- c-opts.c	(revision 119391)
+++ c-opts.c	(working copy)
@@ -396,6 +396,7 @@ c_common_handle_option (size_t scode, co
       warn_strict_aliasing = value;
       warn_string_literal_comparison = value;
       warn_always_true = value;
+      warn_array_bounds = value;
 
       /* Only warn about unknown pragmas that are not in system
 	 headers.  */
Index: tree-vrp.c
===================================================================
--- tree-vrp.c	(revision 119391)
+++ tree-vrp.c	(working copy)
@@ -32,6 +32,7 @@ Boston, MA 02110-1301, USA.  */
 #include "tree-dump.h"
 #include "timevar.h"
 #include "diagnostic.h"
+#include "toplev.h"
 #include "cfgloop.h"
 #include "tree-scalar-evolution.h"
 #include "tree-ssa-propagate.h"
@@ -658,7 +659,6 @@ value_ranges_intersect_p (value_range_t 
 	  || value_inside_range (vr0->max, vr1) == 1);
 }
 
-
 /* Return true if VR includes the value zero, false otherwise.  FIXME,
    currently this will return false for an anti-range like ~[-4, 3].
    This will be wrong when the semantics of value_inside_range are
@@ -3372,6 +3372,129 @@ insert_range_assertions (void)
   BITMAP_FREE (need_assert_for);
 }
 
+/*  Checks one ARRAY_REF. Ignores arrays that are flexible or
+    likely to be a struct hack. If VRP can determine that the
+    array subscript is a contant, check if it is outside valid
+    range. If the array subscript is a RANGE, warn if it is
+    non-overlapping with valid range.  */
+
+static void
+check_array_ref (tree ref, location_t* locus, bool ignore_off_by_one)
+{
+  value_range_t* vr = NULL;
+  int rl, ru;
+  tree ls = TREE_OPERAND (ref, 1);
+  tree us = TREE_OPERAND (ref, 1);
+  tree lb = array_ref_low_bound(ref);
+  tree ub = array_ref_up_bound(ref);
+
+  if (!ub || !locus || TREE_NO_WARNING (ref)
+      || TREE_CODE (ub) != INTEGER_CST
+      /* Can not check flexible arrays.  */
+      || (TYPE_SIZE (TREE_TYPE (ref)) == NULL_TREE
+          && TYPE_DOMAIN (TREE_TYPE (ref)) != NULL_TREE
+          && 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 (ub, 1) <= 0)
+    return;
+
+  ub = fold_build2 (PLUS_EXPR, TREE_TYPE (ub), ub, integer_one_node);
+  if (TREE_CODE (ls) == SSA_NAME)
+    {
+      vr = get_value_range (ls);
+      if (vr->type == VR_RANGE || vr->type == VR_ANTI_RANGE)
+        {
+          ls = vr->type == VR_RANGE ? vr->max : vr->min;
+          us = vr->type == VR_RANGE ? vr->min : vr->max;
+        }
+    }
+
+  ru = TREE_CODE (us) == INTEGER_CST ? tree_int_cst_compare (ub, us) : -2;
+  rl = TREE_CODE (ls) == INTEGER_CST ? tree_int_cst_compare (ls, lb) : -2;
+  TREE_NO_WARNING (ref ) = 1;
+
+  if (vr && vr->type == VR_ANTI_RANGE)
+    {
+      if (ru == -1 && rl == -1)
+        warning (OPT_Warray_bounds,
+                 "%Harray subscript is outside array bounds", locus);
+      else
+        TREE_NO_WARNING (ref) = 0;
+    }
+  else if (ru == -1 || (!ignore_off_by_one && ru == 0))
+    warning (OPT_Warray_bounds, "%Harray subscript is above array bounds",
+             locus);
+  else if (rl == -1)
+    warning (OPT_Warray_bounds, "%Harray subscript is below array bounds",
+             locus);
+  else
+    TREE_NO_WARNING (ref) = 0;
+}
+
+static bool array_bounds_already_done;
+
+/* Checks if this is an ARRAY_REF inside an ADDR_EXPR (in which an array
+   subscript one outside the valid range is allowed) or not. Call
+   check_array_ref for each ARRAY_REF.  */
+
+static tree
+check_array_bounds (tree *tp, int *walk_subtree, void *data)
+{
+ tree t = *tp;
+
+ *walk_subtree = TRUE;
+
+  if (TREE_CODE (t) == ARRAY_REF)
+    check_array_ref (t, (location_t*) data, false /*ignore_off_by_one*/);
+  else if (TREE_CODE (t) == ADDR_EXPR)
+    {
+       t = TREE_OPERAND (t, 0);
+       while (!array_bounds_already_done && handled_component_p (t))
+         {
+           if (TREE_CODE (t) == ARRAY_REF)
+             check_array_ref (t, (location_t*) data, 
true /*ignore_off_by_one*/);
+           t = TREE_OPERAND (t, 0);
+         }
+       *walk_subtree = FALSE;
+    }
+
+  return NULL_TREE;
+}
+
+/* Walk over all statements of all reachable BBs and call check_array_bounds
+   on them.  */
+
+static void
+check_all_array_refs (void)
+{
+  basic_block bb;
+  block_stmt_iterator si;
+
+  FOR_EACH_BB (bb)
+    {
+      /* Skip bb's that are clearly unreachable.  */
+      if (single_pred_p (bb))
+      {
+	basic_block pred_bb = EDGE_PRED (bb, 0)->src;
+	tree ls = NULL_TREE;
+
+	if (!bsi_end_p (bsi_last (pred_bb)))
+	  ls = bsi_stmt (bsi_last (pred_bb));
+
+	if (ls && TREE_CODE (ls) == COND_EXPR
+	    && ((COND_EXPR_COND (ls) == boolean_false_node
+		 && (EDGE_PRED (bb, 0)->flags & EDGE_TRUE_VALUE))
+		|| (COND_EXPR_COND (ls) == boolean_true_node
+		    && (EDGE_PRED (bb, 0)->flags & EDGE_FALSE_VALUE))))
+	  continue;
+      }
+      for (si = bsi_start (bb); !bsi_end_p (si); bsi_next (&si))
+	walk_tree_without_duplicates (bsi_stmt_ptr (si), check_array_bounds,
+				      EXPR_LOCUS (*bsi_stmt_ptr (si)));
+    }
+}
 
 /* Convert range assertion expressions into the implied copies and
    copy propagate away the copies.  Doing the trivial copy propagation
@@ -3577,7 +3700,7 @@ vrp_visit_assignment (tree stmt, tree *o
 
       return SSA_PROP_NOT_INTERESTING;
     }
-  
+
   /* Every other statement produces no useful ranges.  */
   FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_DEF)
     set_value_range_to_varying (get_value_range (def));
@@ -4684,6 +4807,12 @@ vrp_finalize (void)
 
   substitute_and_fold (single_val_range, true);
 
+  if (warn_array_bounds)
+      check_all_array_refs();
+
+  /* workaround for PR/26726.  */
+  array_bounds_already_done = true;
+
   /* We must identify jump threading opportunities before we release
      the datastructures built by VRP.  */
   identify_jump_threads ();
Index: testsuite/gcc.dg/Warray-bounds.c
===================================================================
--- testsuite/gcc.dg/Warray-bounds.c	(revision 0)
+++ testsuite/gcc.dg/Warray-bounds.c	(revision 0)
@@ -0,0 +1,92 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -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;
+
+    a[-1] = 0;             /* { dg-warning "array subscript" } */
+    a[ 0] = 0;
+    a[ 1] = 0;
+
+
+    a[ 9] = 0;
+    a[10] = 0;             /* { dg-warning "array subscript" } */
+    a[11] = 0;             /* { dg-warning "array subscript" } */
+    a[2 * n() - 11] = 0;    /* { dg-warning "array subscript" } */
+    a[2 * n() - 10] = 0;
+    a[2 * n() -  1] = 0;
+    a[2 * n() -  0] = 0;    /* { dg-warning "array subscript" } */
+
+    b[-1] = 0;             /* { dg-warning "array subscript" } */
+    b[ 0] = 0;
+    b[ 1] = 0;
+    b[ 9] = 0;
+    b[10] = 0;             /* { dg-warning "array subscript" } */
+    b[11] = 0;             /* { dg-warning "array subscript" } */
+    b[2 * n() - 11] = 0;    /* { dg-warning "array subscript" } */
+    b[2 * n() - 10] = 0;
+    b[2 * n() -  1] = 0;
+    b[2 * n() -  0] = 0;    /* { dg-warning "array subscript" } */
+
+    c.c[-1] = 0;           /* { dg-warning "array subscript" } */
+    c.c[ 0] = 0;
+    c.c[ 1] = 0;
+    c.c[ 9] = 0;
+    c.c[10] = 0;           /* { dg-warning "array subscript" } */
+    c.c[11] = 0;           /* { dg-warning "array subscript" } */
+    c.c[2 * n() - 11] = 0;  /* { dg-warning "array subscript" } */
+    c.c[2 * n() - 10] = 0;
+    c.c[2 * n() -  1] = 0;
+    c.c[2 * n() -  0] = 0;  /* { dg-warning "array subscript" } */
+
+    g(&a[8]);
+    g(&a[9]);
+    g(&a[10]);
+    g(&a[11]);             /* { 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" } */
+
+    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]);
+
+    if (10 < 10)
+       a[10] = 0;
+    if (10 < 10)
+       b[10] = 0;
+    if (-1 >= 0)
+       c.c[-1] = 0;
+
+    for (i = 20; i < 30; ++i)
+             a[i] = 1;       /* { dg-warning "array subscript" } */
+
+    return a;
+}
+

Re: [PATCH] Fix PR/8268: implement compile time array subscript checking

[Andrew Pinski]

> 
> 
> Hi, 
> 
> The patch below implements PR/8268, which seems to be the one major diagnostic 
> we're missing compared to icc (at least one openSUSE contributor regularly 
> rebuilds all of the openSUSE distribution with icc just to report those 
> warnings as bugs to us ;) ). 
> 
> bootstrapped and regtested with no additional failures many times on 
> i686-suse-linux. 
> 
> Ok? Do we need the extra -Warray-bounds?

Your documentation for the warning is incorrect as
you say:

> +This option is only active when @option{-O1} or higher is active. It warns
> +about constant subscripts in array accesses that are out of bounds.

But that is wrong as it is actived only with -O1 -ftree-vrp or -O2 and higher
with VRP still turned on.

Second I would not just set it for the C family of languages:
> --- c-opts.c	(revision 119391)
> +++ c-opts.c	(working copy)
> @@ -396,6 +396,7 @@ c_common_handle_option (size_t scode, co
>        warn_strict_aliasing = value;
>        warn_string_literal_comparison = value;
>        warn_always_true = value;
> +      warn_array_bounds = value;

Third how well does the current VRP implentation handle:

int v[10]={0};
void f(void)
{
  int n = 99;
  int i;
  if (n <= 0)
    return;
  if (n > 0)
    {
      i = 1;
      do {
       /* _Bool t = i <= 0;
        _Bool t1 = i > n;
        _Bool t2 = t || t1;
        if (t2)  __builtin_abort (); */

        v[i] = i*i;

        i++;
      } while (i != n);
    }
}

As far as I know VRP current does not get the above correctly,
that is change n to a nonconstant and uncomment the bounds checking
code.

Thanks,
Andrew Pinski

Re: [PATCH] Fix PR/8268: implement compile time array subscript checking

[Dirk Müller]

Am Donnerstag, 7. Dezember 2006 00:35 schrieb Andrew Pinski:

> Your documentation for the warning is incorrect as
> you say:

You're right, I forgot to update the documentation. 

> But that is wrong as it is actived only with -O1 -ftree-vrp or -O2 and
> higher with VRP still turned on.


+@item -Warray-bounds
+@opindex Warray-bounds
+This option is only active when @option{-O2} or higher is active. It warns
+about subscripts to arrays that are always out of bounds.
+

Ok? Or should I mention the -ftree-vrp dependency explicitely ?

I can also make the warning work without -O2 for constant subscripts, so the 
documentation doesn't have to be that precise. Updating the patch for that is 
easy, but the regtest will take another day or two. 

> Second I would not just set it for the C family of languages:

Which language do you have in mind? I'm not very familiar with non-C-style 
languages, but afaik in Java out of bound accesses are legal (throw an 
exception) and fortran already has a warning to handle this (just guessed 
this from some patches I saw on this mailing list). Ada seems to ignore -Wall 
completely. 


> As far as I know VRP current does not get the above correctly,
> that is change n to a nonconstant and uncomment the bounds checking
> code.

VRP seems to try to be correct as far as "the actual range is within the range 
that is determined". So in your testcase it determines a range of [+1,+INF], 
which is okay, just not precise. 

This is particularly the reason why I only warn if the access is 
non-overlapping with the vrp determined range. E.g. if you change the i = 1; 
in your testcase to i = 10; it will warn, otherwise it will not. This is 
because I like to use -Werror=array-bounds and therefore do not like false 
positives. 


Dirk

Re: [PATCH] Fix PR/8268: implement compile time array subscript checking

[Dirk Müller]

Am Donnerstag, 7. Dezember 2006 10:51 schrieb Dirk Müller:

> non-overlapping with the vrp determined range. E.g. if you change the i =
> 1; in your testcase to i = 10; it will warn, otherwise it will not. This is
> because I like to use -Werror=array-bounds and therefore do not like false
> positives.

one possible alternative would be to unconditionally emit the current warning 
with -Wall, and make -Warray-bounds also emit when an array subscript "might" 
go out of range. I do not consider it useful because it will warn essentially 
on any loop that does something non-trivial with array-indices. 


Dirk

Re: [PATCH] Fix PR/8268: implement compile time array subscript checking

[Andrew Pinski]

On Thu, 2006-12-07 at 10:51 +0100, Dirk Müller wrote:
> +@item -Warray-bounds
> +@opindex Warray-bounds
> +This option is only active when @option{-O2} or higher is active. It
> warns
> +about subscripts to arrays that are always out of bounds.
> +
> 
> Ok? Or should I mention the -ftree-vrp dependency explicitely ?

I still think it should mention the -ftree-vrp dependency explicitly
because people can use -O1 -ftree-vrp and still get the warnings and
then it will look funny as the documentation says -O2 and above.

Thanks,
Andrew Pinski

Re: [PATCH] Fix PR/8268: implement compile time array subscript checking

[Richard Guenther]

On 12/6/06, Dirk Mueller <dmuell@gmx.net> wrote:
>
> Hi,
>
> The patch below implements PR/8268, which seems to be the one major diagnostic
> we're missing compared to icc (at least one openSUSE contributor regularly
> rebuilds all of the openSUSE distribution with icc just to report those
> warnings as bugs to us ;) ).
>
> bootstrapped and regtested with no additional failures many times on
> i686-suse-linux.
>
> Ok? Do we need the extra -Warray-bounds?

Yes, we need the extra -Warray-bounds for languages that don't include it
in -Wall and to turn the warning off.

>
> :ADDPATCH diagnostic:
>
> 2006-12-06  Dirk Mueller  <dmueller@suse.de>
>             Richard Guenther <rguenther@suse.de>
>
>         PR diagnostic/8268
>         * doc/invoke.texi (Warray-bounds): Document -Warray-bounds.
>         * common.opt (Warray-bounds): Add new warning option.
>         * c-opts.c (c_common_handle_option): Define -Warray-bounds
>         if -Wall is given.
>         * tree-vrp.c (vrp_finalize): Call check_array_refs if -Warray-bounds
>         is enabled.
>         (check_array_refs, check_array_bounds, check_array_ref): New.
>
>         * testsuite/gcc.dg/Warray-bounds.c: New testcase.
>
> --- doc/invoke.texi     (revision 119391)
> +++ doc/invoke.texi     (working copy)
> @@ -244,7 +244,7 @@ Objective-C and Objective-C++ Dialects}.
>  -Wreturn-type  -Wsequence-point  -Wshadow @gol
>  -Wsign-compare  -Wstack-protector @gol
>  -Wstrict-aliasing -Wstrict-aliasing=2 @gol
> --Wstring-literal-comparison @gol
> +-Wstring-literal-comparison -Warray-bounds @gol
>  -Wswitch  -Wswitch-default  -Wswitch-enum @gol
>  -Wsystem-headers  -Wtrigraphs  -Wundef  -Wuninitialized @gol
>  -Wunknown-pragmas  -Wno-pragmas -Wunreachable-code @gol
> @@ -2829,6 +2828,11 @@ compiler is using for optimization.  Thi
>  @option{-Wstrict-aliasing}, but it will also give a warning for some
> ambiguous
>  cases that are safe.
>
> +@item -Warray-bounds
> +@opindex Warray-bounds
> +This option is only active when @option{-O1} or higher is active. It warns
> +about constant subscripts in array accesses that are out of bounds.
> +
>  @item -Wall
>  @opindex Wall
>  All of the above @samp{-W} options combined.  This enables all the
>
> --- common.opt  (revision 119391)
> +++ common.opt  (working copy)
> @@ -61,6 +61,10 @@ Walways-true
>  Common Var(warn_always_true)
>  Warn about comparisons that always evaluate to true
>
> +Warray-bounds
> +Common Var(warn_array_bounds)
> +Warn if an array is accessed out of bounds
> +
>  Wattributes
>  Common Var(warn_attributes) Init(1)
>  Warn about inappropriate attribute usage
> Index: c-opts.c
> ===================================================================
> --- c-opts.c    (revision 119391)
> +++ c-opts.c    (working copy)
> @@ -396,6 +396,7 @@ c_common_handle_option (size_t scode, co
>        warn_strict_aliasing = value;
>        warn_string_literal_comparison = value;
>        warn_always_true = value;
> +      warn_array_bounds = value;
>
>        /* Only warn about unknown pragmas that are not in system
>          headers.  */
> Index: tree-vrp.c
> ===================================================================
> --- tree-vrp.c  (revision 119391)
> +++ tree-vrp.c  (working copy)
> @@ -32,6 +32,7 @@ Boston, MA 02110-1301, USA.  */
>  #include "tree-dump.h"
>  #include "timevar.h"
>  #include "diagnostic.h"
> +#include "toplev.h"
>  #include "cfgloop.h"
>  #include "tree-scalar-evolution.h"
>  #include "tree-ssa-propagate.h"
> @@ -658,7 +659,6 @@ value_ranges_intersect_p (value_range_t
>           || value_inside_range (vr0->max, vr1) == 1);
>  }
>
> -
>  /* Return true if VR includes the value zero, false otherwise.  FIXME,
>     currently this will return false for an anti-range like ~[-4, 3].
>     This will be wrong when the semantics of value_inside_range are
> @@ -3372,6 +3372,129 @@ insert_range_assertions (void)
>    BITMAP_FREE (need_assert_for);
>  }
>
> +/*  Checks one ARRAY_REF. Ignores arrays that are flexible or
> +    likely to be a struct hack. If VRP can determine that the
> +    array subscript is a contant, check if it is outside valid
> +    range. If the array subscript is a RANGE, warn if it is
> +    non-overlapping with valid range.  */

You need two spaces after '.' but not after /*, also the parameters
need documentation.

> +static void
> +check_array_ref (tree ref, location_t* locus, bool ignore_off_by_one)
> +{
> +  value_range_t* vr = NULL;
> +  int rl, ru;
> +  tree ls = TREE_OPERAND (ref, 1);
> +  tree us = TREE_OPERAND (ref, 1);
> +  tree lb = array_ref_low_bound(ref);
> +  tree ub = array_ref_up_bound(ref);
> +
> +  if (!ub || !locus || TREE_NO_WARNING (ref)
> +      || TREE_CODE (ub) != INTEGER_CST
> +      /* Can not check flexible arrays.  */
> +      || (TYPE_SIZE (TREE_TYPE (ref)) == NULL_TREE
> +          && TYPE_DOMAIN (TREE_TYPE (ref)) != NULL_TREE
> +          && 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 (ub, 1) <= 0)
> +    return;

The "struct hack" needs to include all arrays that are the last member
in a structure.
See tree-dfa.c:get_ref_base_and_extent - you can feed it the array_ref
and check if
*pmax_size is -1 (unbound or unknown).  This function should also deal with
flexible arrays (i.e. report them as unknown extent).

> +
> +  ub = fold_build2 (PLUS_EXPR, TREE_TYPE (ub), ub, integer_one_node);

You should use int_const_binop here.

> +  if (TREE_CODE (ls) == SSA_NAME)
> +    {
> +      vr = get_value_range (ls);
> +      if (vr->type == VR_RANGE || vr->type == VR_ANTI_RANGE)
> +        {
> +          ls = vr->type == VR_RANGE ? vr->max : vr->min;
> +          us = vr->type == VR_RANGE ? vr->min : vr->max;
> +        }
> +    }
> +
> +  ru = TREE_CODE (us) == INTEGER_CST ? tree_int_cst_compare (ub, us) : -2;
> +  rl = TREE_CODE (ls) == INTEGER_CST ? tree_int_cst_compare (ls, lb) : -2;

It looks like you can avoid adding 1 to ub by adjusting the checks
below to ru <= 0.

> +  TREE_NO_WARNING (ref ) = 1;
> +
> +  if (vr && vr->type == VR_ANTI_RANGE)
> +    {
> +      if (ru == -1 && rl == -1)
> +        warning (OPT_Warray_bounds,
> +                 "%Harray subscript is outside array bounds", locus);
> +      else
> +        TREE_NO_WARNING (ref) = 0;
> +    }
> +  else if (ru == -1 || (!ignore_off_by_one && ru == 0))
> +    warning (OPT_Warray_bounds, "%Harray subscript is above array bounds",
> +             locus);
> +  else if (rl == -1)
> +    warning (OPT_Warray_bounds, "%Harray subscript is below array bounds",
> +             locus);
> +  else
> +    TREE_NO_WARNING (ref) = 0;
> +}
> +
> +static bool array_bounds_already_done;
> +
> +/* Checks if this is an ARRAY_REF inside an ADDR_EXPR (in which an array
> +   subscript one outside the valid range is allowed) or not. Call
> +   check_array_ref for each ARRAY_REF.  */

Parameters need documenting.

> +static tree
> +check_array_bounds (tree *tp, int *walk_subtree, void *data)
> +{
> + tree t = *tp;
> +
> + *walk_subtree = TRUE;
> +
> +  if (TREE_CODE (t) == ARRAY_REF)
> +    check_array_ref (t, (location_t*) data, false /*ignore_off_by_one*/);
> +  else if (TREE_CODE (t) == ADDR_EXPR)
> +    {
> +       t = TREE_OPERAND (t, 0);
> +       while (!array_bounds_already_done && handled_component_p (t))
> +         {
> +           if (TREE_CODE (t) == ARRAY_REF)
> +             check_array_ref (t, (location_t*) data,
> true /*ignore_off_by_one*/);
> +           t = TREE_OPERAND (t, 0);
> +         }
> +       *walk_subtree = FALSE;
> +    }
> +
> +  return NULL_TREE;
> +}
> +
> +/* Walk over all statements of all reachable BBs and call check_array_bounds
> +   on them.  */
> +
> +static void
> +check_all_array_refs (void)
> +{
> +  basic_block bb;
> +  block_stmt_iterator si;
> +
> +  FOR_EACH_BB (bb)
> +    {
> +      /* Skip bb's that are clearly unreachable.  */
> +      if (single_pred_p (bb))
> +      {
> +       basic_block pred_bb = EDGE_PRED (bb, 0)->src;
> +       tree ls = NULL_TREE;
> +
> +       if (!bsi_end_p (bsi_last (pred_bb)))
> +         ls = bsi_stmt (bsi_last (pred_bb));
> +
> +       if (ls && TREE_CODE (ls) == COND_EXPR
> +           && ((COND_EXPR_COND (ls) == boolean_false_node
> +                && (EDGE_PRED (bb, 0)->flags & EDGE_TRUE_VALUE))
> +               || (COND_EXPR_COND (ls) == boolean_true_node
> +                   && (EDGE_PRED (bb, 0)->flags & EDGE_FALSE_VALUE))))
> +         continue;
> +      }
> +      for (si = bsi_start (bb); !bsi_end_p (si); bsi_next (&si))
> +       walk_tree_without_duplicates (bsi_stmt_ptr (si), check_array_bounds,
> +                                     EXPR_LOCUS (*bsi_stmt_ptr (si)));
> +    }
> +}
>
>  /* Convert range assertion expressions into the implied copies and
>     copy propagate away the copies.  Doing the trivial copy propagation
> @@ -3577,7 +3700,7 @@ vrp_visit_assignment (tree stmt, tree *o
>
>        return SSA_PROP_NOT_INTERESTING;
>      }
> -
> +
>    /* Every other statement produces no useful ranges.  */
>    FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_DEF)
>      set_value_range_to_varying (get_value_range (def));
> @@ -4684,6 +4807,12 @@ vrp_finalize (void)
>
>    substitute_and_fold (single_val_range, true);
>
> +  if (warn_array_bounds)
> +      check_all_array_refs();
> +
> +  /* workaround for PR/26726.  */
> +  array_bounds_already_done = true;

This comment should be more elaborate.

>    /* We must identify jump threading opportunities before we release
>       the datastructures built by VRP.  */
>    identify_jump_threads ();
> Index: testsuite/gcc.dg/Warray-bounds.c
> ===================================================================
> --- testsuite/gcc.dg/Warray-bounds.c    (revision 0)
> +++ testsuite/gcc.dg/Warray-bounds.c    (revision 0)
> @@ -0,0 +1,92 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -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;
> +
> +    a[-1] = 0;             /* { dg-warning "array subscript" } */
> +    a[ 0] = 0;
> +    a[ 1] = 0;
> +
> +
> +    a[ 9] = 0;
> +    a[10] = 0;             /* { dg-warning "array subscript" } */
> +    a[11] = 0;             /* { dg-warning "array subscript" } */
> +    a[2 * n() - 11] = 0;    /* { dg-warning "array subscript" } */
> +    a[2 * n() - 10] = 0;
> +    a[2 * n() -  1] = 0;
> +    a[2 * n() -  0] = 0;    /* { dg-warning "array subscript" } */
> +
> +    b[-1] = 0;             /* { dg-warning "array subscript" } */
> +    b[ 0] = 0;
> +    b[ 1] = 0;
> +    b[ 9] = 0;
> +    b[10] = 0;             /* { dg-warning "array subscript" } */
> +    b[11] = 0;             /* { dg-warning "array subscript" } */
> +    b[2 * n() - 11] = 0;    /* { dg-warning "array subscript" } */
> +    b[2 * n() - 10] = 0;
> +    b[2 * n() -  1] = 0;
> +    b[2 * n() -  0] = 0;    /* { dg-warning "array subscript" } */
> +
> +    c.c[-1] = 0;           /* { dg-warning "array subscript" } */
> +    c.c[ 0] = 0;
> +    c.c[ 1] = 0;
> +    c.c[ 9] = 0;
> +    c.c[10] = 0;           /* { dg-warning "array subscript" } */
> +    c.c[11] = 0;           /* { dg-warning "array subscript" } */
> +    c.c[2 * n() - 11] = 0;  /* { dg-warning "array subscript" } */
> +    c.c[2 * n() - 10] = 0;
> +    c.c[2 * n() -  1] = 0;
> +    c.c[2 * n() -  0] = 0;  /* { dg-warning "array subscript" } */
> +
> +    g(&a[8]);
> +    g(&a[9]);
> +    g(&a[10]);
> +    g(&a[11]);             /* { 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" } */
> +
> +    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]);
> +
> +    if (10 < 10)
> +       a[10] = 0;
> +    if (10 < 10)
> +       b[10] = 0;
> +    if (-1 >= 0)
> +       c.c[-1] = 0;
> +
> +    for (i = 20; i < 30; ++i)
> +             a[i] = 1;       /* { dg-warning "array subscript" } */
> +
> +    return a;
> +}
> +

Otherwise this looks reasonable.  Please incorporate the documentation changes
suggested by Andrew.  I think C family languages are ok for now, others can be
added as a followup with approrpiate testcases.

Thanks,
Richard.

Re: [PATCH] Fix PR/8268: implement compile time array subscript checking

[Dirk Mueller]

On Tuesday, 12. December 2006 13:51, Richard Guenther wrote:

> > Ok? Do we need the extra -Warray-bounds?
> Yes, we need the extra -Warray-bounds for languages that don't include it
> in -Wall and to turn the warning off.

I don't really think it is a good idea to turn off the warning ;) The main 
reason I'm asking is because we'd need an extra flag for "maybe it overflows 
the array here" kind of warnings, as in my case I'd imagine that I want to 
Werror-out on the "always overflows" and only warn on the "maybe overflows" 
case. 

> > NULL_TREE) +      /* Accesses after the end of arrays of size 0 (gcc
> > +         extension) and 1 are likely intentional ("struct
> > +         hack").  */
> > +      || compare_tree_int (ub, 1) <= 0)
> > +    return;
> The "struct hack" needs to include all arrays that are the last member
> in a structure.
> See tree-dfa.c:get_ref_base_and_extent - you can feed it the array_ref
> and check if
> *pmax_size is -1 (unbound or unknown).  This function should also deal with
> flexible arrays (i.e. report them as unknown extent).

Hmm, it does that when it can determine that the array subscript is accessing 
the array beyond a memory location that is inside the struct the array ref 
was declared. It doesn't make sense to disable an array overflow warning for 
the case that it overflows an array more than the size of the struct, does 
it? There might be many legal cases suppressed this way. Do you know any real 
life code that would trigger false positives here? Or do you only want the 
struct hack to be enabled if the array is at the end of a structure?

> > +  ru = TREE_CODE (us) == INTEGER_CST ? tree_int_cst_compare (ub, us) :
> > -2; +  rl = TREE_CODE (ls) == INTEGER_CST ? tree_int_cst_compare (ls, lb)
> > : -2;
> It looks like you can avoid adding 1 to ub by adjusting the checks
> below to ru <= 0.
> > +  else if (ru == -1 || (!ignore_off_by_one && ru == 0))
> > +    warning (OPT_Warray_bounds, "%Harray subscript is above array

No, because of the case quoted above. If the ARRAY_REF is inside an ADDR_EXPR, 
taking the address of the first element after the end of the array is 
welldefined. 

if you're saying that int_const_binop is computationally more expensive than 
two tree_int_cst_compare calls.. then I see your point. Ok, I've done some 
profiling and the change below seems to be in your spirit, and is about 
factor 2.5 faster. The total runtime overhead for insn-recog.c, which has more 
than 30000 array subscripts, is now about 0,1%. Much of the speedup however 
comes from compare_tree_int() which I had to change to be cheaper. 

Bootstrapped, regtested against trunk on i686-suse-linux. 

Thanks,
Dirk

2006-12-21  Dirk Mueller  <dmueller@suse.de>
	    Richard Guenther <rguenther@suse.de>

	PR diagnostic/8268
	* doc/invoke.texi (Warray-bounds): Document -Warray-bounds.
	* common.opt (Warray-bounds): Add new warning option.
	* c-opts.c (c_common_handle_option): Define -Warray-bounds
	if -Wall is given.
	* tree-vrp.c (vrp_finalize): Call check_array_refs if -Warray-bounds
	is enabled.
	* tree.c (simple_cst_equal): Manually inline tree_int_cst_sgn.
	(check_array_refs, check_array_bounds, check_array_ref): New.

	* testsuite/gcc.dg/Warray-bounds.c: New testcase.


--- doc/invoke.texi	(revision 119972)
+++ doc/invoke.texi	(working copy)
@@ -245,7 +245,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wreturn-type  -Wsequence-point  -Wshadow @gol
 -Wsign-compare  -Wstack-protector @gol
 -Wstrict-aliasing -Wstrict-aliasing=2 @gol
--Wstring-literal-comparison @gol
+-Wstring-literal-comparison -Warray-bounds @gol
 -Wswitch  -Wswitch-default  -Wswitch-enum @gol
 -Wsystem-headers  -Wtrigraphs  -Wundef  -Wuninitialized @gol
 -Wunknown-pragmas  -Wno-pragmas -Wunreachable-code @gol
@@ -2831,6 +2831,12 @@ compiler is using for optimization.  Thi
 @option{-Wstrict-aliasing}, but it will also give a warning for some 
ambiguous
 cases that are safe.
 
+@item -Warray-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.
+
 @item -Wall
 @opindex Wall
 All of the above @samp{-W} options combined.  This enables all the
--- common.opt	(revision 119972)
+++ common.opt	(working copy)
@@ -61,6 +61,10 @@ Walways-true
 Common Var(warn_always_true)
 Warn about comparisons that always evaluate to true
 
+Warray-bounds
+Common Var(warn_array_bounds)
+Warn if an array is accessed out of bounds
+
 Wattributes
 Common Var(warn_attributes) Init(1)
 Warn about inappropriate attribute usage
--- tree.c	(revision 119972)
+++ tree.c	(working copy)
@@ -5003,7 +5003,7 @@ simple_cst_equal (tree t1, tree t2)
 int
 compare_tree_int (tree t, unsigned HOST_WIDE_INT u)
 {
-  if (tree_int_cst_sgn (t) < 0)
+  if (!TYPE_UNSIGNED (TREE_TYPE (t)) && TREE_INT_CST_HIGH (t) < 0)
     return -1;
   else if (TREE_INT_CST_HIGH (t) != 0)
     return 1;
--- tree-vrp.c	(revision 119972)
+++ tree-vrp.c	(working copy)
@@ -32,6 +32,7 @@ Boston, MA 02110-1301, USA.  */
 #include "tree-dump.h"
 #include "timevar.h"
 #include "diagnostic.h"
+#include "toplev.h"
 #include "cfgloop.h"
 #include "tree-scalar-evolution.h"
 #include "tree-ssa-propagate.h"
@@ -3420,6 +3420,142 @@ insert_range_assertions (void)
   BITMAP_FREE (need_assert_for);
 }
 
+/* Checks one ARRAY_REF in REF, located at LOCUS. Ignores flexible arrays
+   and "struct" hacks. If VRP can determine that the
+   array subscript is a contant, check if it is outside valid
+   range. If the array subscript is a RANGE, warn if it is
+   non-overlapping with valid range.
+   IGNORE_OFF_BY_ONE is true if the ARRAY_REF is inside a ADDR_EXPR.  */
+
+static void
+check_array_ref (tree ref, location_t* locus, bool ignore_off_by_one)
+{
+  value_range_t* vr = NULL;
+  tree low_sub, up_sub;
+  tree low_bound, up_bound = array_ref_up_bound(ref);
+
+  low_sub = up_sub = TREE_OPERAND (ref, 1);
+
+  if (!up_bound || !locus || TREE_NO_WARNING (ref)
+      || TREE_CODE (up_bound) != INTEGER_CST
+      /* Can not check flexible arrays.  */
+      || (TYPE_SIZE (TREE_TYPE (ref)) == NULL_TREE
+          && TYPE_DOMAIN (TREE_TYPE (ref)) != NULL_TREE
+          && 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)
+    return;
+
+  low_bound = array_ref_low_bound(ref);
+
+  if (TREE_CODE (low_sub) == SSA_NAME)
+    {
+      vr = get_value_range (low_sub);
+      if (vr->type == VR_RANGE || vr->type == VR_ANTI_RANGE)
+        {
+          low_sub = vr->type == VR_RANGE ? vr->max : vr->min;
+          up_sub = vr->type == VR_RANGE ? vr->min : vr->max;
+        }
+    }
+
+  TREE_NO_WARNING (ref) = 1;
+
+  if (vr && vr->type == VR_ANTI_RANGE)
+    {
+      if (TREE_CODE (up_sub) == INTEGER_CST
+          && tree_int_cst_lt (up_bound, up_sub)
+          && TREE_CODE (low_sub) == INTEGER_CST
+          && tree_int_cst_lt (low_sub, low_bound))
+        warning (OPT_Warray_bounds,
+                 "%Harray subscript is outside array bounds", locus);
+      else
+        TREE_NO_WARNING (ref) = 0;
+    }
+  else if (TREE_CODE (up_sub) == INTEGER_CST
+           && tree_int_cst_lt (up_bound, up_sub)
+           && !tree_int_cst_equal (up_bound, up_sub)
+           && (!ignore_off_by_one
+               || !tree_int_cst_equal (int_const_binop (PLUS_EXPR,
+                                                        up_bound,
+                                                        integer_one_node,
+                                                        0),
+                                       up_sub)))
+    warning (OPT_Warray_bounds, "%Harray subscript is above array bounds",
+             locus);
+  else if (TREE_CODE (low_sub) == INTEGER_CST
+           && tree_int_cst_lt (low_sub, low_bound))
+    warning (OPT_Warray_bounds, "%Harray subscript is below array bounds",
+             locus);
+  else
+    TREE_NO_WARNING (ref) = 0;
+}
+
+static bool array_bounds_already_done;
+
+/* walk_tree() callback that checks if *TP is
+   an ARRAY_REF inside an ADDR_EXPR (in which an array
+   subscript one outside the valid range is allowed). Call
+   check_array_ref for each ARRAY_REF found. The location is 
+   passed in DATA.  */
+
+static tree
+check_array_bounds (tree *tp, int *walk_subtree, void *data)
+{
+ tree t = *tp;
+
+ *walk_subtree = TRUE;
+
+  if (TREE_CODE (t) == ARRAY_REF)
+    check_array_ref (t, (location_t*) data, false /*ignore_off_by_one*/);
+  else if (TREE_CODE (t) == ADDR_EXPR)
+    {
+       t = TREE_OPERAND (t, 0);
+       while (!array_bounds_already_done && handled_component_p (t))
+         {
+           if (TREE_CODE (t) == ARRAY_REF)
+             check_array_ref (t, (location_t*) data, 
true /*ignore_off_by_one*/);
+           t = TREE_OPERAND (t, 0);
+         }
+       *walk_subtree = FALSE;
+    }
+
+  return NULL_TREE;
+}
+
+/* Walk over all statements of all reachable BBs and call check_array_bounds
+   on them.  */
+
+static void
+check_all_array_refs (void)
+{
+  basic_block bb;
+  block_stmt_iterator si;
+
+  FOR_EACH_BB (bb)
+    {
+      /* Skip bb's that are clearly unreachable.  */
+      if (single_pred_p (bb))
+      {
+	basic_block pred_bb = EDGE_PRED (bb, 0)->src;
+	tree ls = NULL_TREE;
+
+	if (!bsi_end_p (bsi_last (pred_bb)))
+	  ls = bsi_stmt (bsi_last (pred_bb));
+
+	if (ls && TREE_CODE (ls) == COND_EXPR
+	    && ((COND_EXPR_COND (ls) == boolean_false_node
+		 && (EDGE_PRED (bb, 0)->flags & EDGE_TRUE_VALUE))
+		|| (COND_EXPR_COND (ls) == boolean_true_node
+		    && (EDGE_PRED (bb, 0)->flags & EDGE_FALSE_VALUE))))
+	  continue;
+      }
+      for (si = bsi_start (bb); !bsi_end_p (si); bsi_next (&si))
+	walk_tree (bsi_stmt_ptr (si), check_array_bounds,
+		   EXPR_LOCUS (*bsi_stmt_ptr (si)), NULL);
+    }
+}
 
 /* Convert range assertion expressions into the implied copies and
    copy propagate away the copies.  Doing the trivial copy propagation
@@ -4733,6 +4869,19 @@ vrp_finalize (void)
 
   substitute_and_fold (single_val_range, true);
 
+  if (warn_array_bounds)
+      check_all_array_refs();
+
+  /* workaround for PR/26726. The problem here is that -fivopts
+     sometimes shifts offsets so that arrays are accessed apparently
+     out of bounds, but actually are not. therefore we do not warn
+     about ARRAY_REF's inside ADDR_EXPR's anymore after the first run
+     (which is before ivopts).
+   */
+
+  array_bounds_already_done = true;
+
+
   /* We must identify jump threading opportunities before we release
      the datastructures built by VRP.  */
   identify_jump_threads ();
Index: testsuite/gcc.dg/Warray-bounds.c
===================================================================
--- testsuite/gcc.dg/Warray-bounds.c	(revision 0)
+++ testsuite/gcc.dg/Warray-bounds.c	(revision 0)
@@ -0,0 +1,92 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -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;
+
+    a[-1] = 0;             /* { dg-warning "array subscript" } */
+    a[ 0] = 0;
+    a[ 1] = 0;
+
+
+    a[ 9] = 0;
+    a[10] = 0;             /* { dg-warning "array subscript" } */
+    a[11] = 0;             /* { dg-warning "array subscript" } */
+    a[2 * n() - 11] = 0;    /* { dg-warning "array subscript" } */
+    a[2 * n() - 10] = 0;
+    a[2 * n() -  1] = 0;
+    a[2 * n() -  0] = 0;    /* { dg-warning "array subscript" } */
+
+    b[-1] = 0;             /* { dg-warning "array subscript" } */
+    b[ 0] = 0;
+    b[ 1] = 0;
+    b[ 9] = 0;
+    b[10] = 0;             /* { dg-warning "array subscript" } */
+    b[11] = 0;             /* { dg-warning "array subscript" } */
+    b[2 * n() - 11] = 0;    /* { dg-warning "array subscript" } */
+    b[2 * n() - 10] = 0;
+    b[2 * n() -  1] = 0;
+    b[2 * n() -  0] = 0;    /* { dg-warning "array subscript" } */
+
+    c.c[-1] = 0;           /* { dg-warning "array subscript" } */
+    c.c[ 0] = 0;
+    c.c[ 1] = 0;
+    c.c[ 9] = 0;
+    c.c[10] = 0;           /* { dg-warning "array subscript" } */
+    c.c[11] = 0;           /* { dg-warning "array subscript" } */
+    c.c[2 * n() - 11] = 0;  /* { dg-warning "array subscript" } */
+    c.c[2 * n() - 10] = 0;
+    c.c[2 * n() -  1] = 0;
+    c.c[2 * n() -  0] = 0;  /* { dg-warning "array subscript" } */
+
+    g(&a[8]);
+    g(&a[9]);
+    g(&a[10]);
+    g(&a[11]);             /* { 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" } */
+
+    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]);
+
+    if (10 < 10)
+       a[10] = 0;
+    if (10 < 10)
+       b[10] = 0;
+    if (-1 >= 0)
+       c.c[-1] = 0;
+
+    for (i = 20; i < 30; ++i)
+             a[i] = 1;       /* { dg-warning "array subscript" } */
+
+    return a;
+}
+

Re: [PATCH] Fix PR/8268: implement compile time array subscript checking

[Richard Guenther]

On 12/21/06, Dirk Mueller <dmuell@gmx.net> wrote:
> On Tuesday, 12. December 2006 13:51, Richard Guenther wrote:
>
> > > Ok? Do we need the extra -Warray-bounds?
> > Yes, we need the extra -Warray-bounds for languages that don't include it
> > in -Wall and to turn the warning off.
>
> I don't really think it is a good idea to turn off the warning ;) The main
> reason I'm asking is because we'd need an extra flag for "maybe it overflows
> the array here" kind of warnings, as in my case I'd imagine that I want to
> Werror-out on the "always overflows" and only warn on the "maybe overflows"
> case.

I see.  We still should keep the flag.

> > > NULL_TREE) +      /* Accesses after the end of arrays of size 0 (gcc
> > > +         extension) and 1 are likely intentional ("struct
> > > +         hack").  */
> > > +      || compare_tree_int (ub, 1) <= 0)
> > > +    return;
> > The "struct hack" needs to include all arrays that are the last member
> > in a structure.
> > See tree-dfa.c:get_ref_base_and_extent - you can feed it the array_ref
> > and check if
> > *pmax_size is -1 (unbound or unknown).  This function should also deal with
> > flexible arrays (i.e. report them as unknown extent).
>
> Hmm, it does that when it can determine that the array subscript is accessing
> the array beyond a memory location that is inside the struct the array ref
> was declared. It doesn't make sense to disable an array overflow warning for
> the case that it overflows an array more than the size of the struct, does
> it? There might be many legal cases suppressed this way. Do you know any real
> life code that would trigger false positives here? Or do you only want the
> struct hack to be enabled if the array is at the end of a structure?

Yes, I only want the struct hack to be enabled if the array is at the
end of a structure.
You can find all of struct s { ... int c[]; }, { ... int c[0]; }, {
... int c[1]; } and even
{ ... char c[4]; } (use all padding explicitly).  The standard allows
treating all those
as flexible array members if they are last in the structure.

> > > +  ru = TREE_CODE (us) == INTEGER_CST ? tree_int_cst_compare (ub, us) :
> > > -2; +  rl = TREE_CODE (ls) == INTEGER_CST ? tree_int_cst_compare (ls, lb)
> > > : -2;
> > It looks like you can avoid adding 1 to ub by adjusting the checks
> > below to ru <= 0.
> > > +  else if (ru == -1 || (!ignore_off_by_one && ru == 0))
> > > +    warning (OPT_Warray_bounds, "%Harray subscript is above array
>
> No, because of the case quoted above. If the ARRAY_REF is inside an ADDR_EXPR,
> taking the address of the first element after the end of the array is
> welldefined.

Yes, but I was refering to the tree building (now you're using
int_const_binop which
also creates a new tree - at least now less frequently).  Looking
again there doesn't
seem a way to avoid it.

> if you're saying that int_const_binop is computationally more expensive than
> two tree_int_cst_compare calls.. then I see your point. Ok, I've done some
> profiling and the change below seems to be in your spirit, and is about
> factor 2.5 faster. The total runtime overhead for insn-recog.c, which has more
> than 30000 array subscripts, is now about 0,1%. Much of the speedup however
> comes from compare_tree_int() which I had to change to be cheaper.
>
> Bootstrapped, regtested against trunk on i686-suse-linux.
>
> Thanks,
> Dirk
>
> 2006-12-21  Dirk Mueller  <dmueller@suse.de>
>             Richard Guenther <rguenther@suse.de>
>
>         PR diagnostic/8268
>         * doc/invoke.texi (Warray-bounds): Document -Warray-bounds.
>         * common.opt (Warray-bounds): Add new warning option.
>         * c-opts.c (c_common_handle_option): Define -Warray-bounds
>         if -Wall is given.
>         * tree-vrp.c (vrp_finalize): Call check_array_refs if -Warray-bounds
>         is enabled.
>         * tree.c (simple_cst_equal): Manually inline tree_int_cst_sgn.

You changed compare_tree_int, not simple_cst_equal.

Ok with that change.

Thanks,
Richard.

>         (check_array_refs, check_array_bounds, check_array_ref): New.
>
>         * testsuite/gcc.dg/Warray-bounds.c: New testcase.
>
>
> --- doc/invoke.texi     (revision 119972)
> +++ doc/invoke.texi     (working copy)
> @@ -245,7 +245,7 @@ Objective-C and Objective-C++ Dialects}.
>  -Wreturn-type  -Wsequence-point  -Wshadow @gol
>  -Wsign-compare  -Wstack-protector @gol
>  -Wstrict-aliasing -Wstrict-aliasing=2 @gol
> --Wstring-literal-comparison @gol
> +-Wstring-literal-comparison -Warray-bounds @gol
>  -Wswitch  -Wswitch-default  -Wswitch-enum @gol
>  -Wsystem-headers  -Wtrigraphs  -Wundef  -Wuninitialized @gol
>  -Wunknown-pragmas  -Wno-pragmas -Wunreachable-code @gol
> @@ -2831,6 +2831,12 @@ compiler is using for optimization.  Thi
>  @option{-Wstrict-aliasing}, but it will also give a warning for some
> ambiguous
>  cases that are safe.
>
> +@item -Warray-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.
> +
>  @item -Wall
>  @opindex Wall
>  All of the above @samp{-W} options combined.  This enables all the
> --- common.opt  (revision 119972)
> +++ common.opt  (working copy)
> @@ -61,6 +61,10 @@ Walways-true
>  Common Var(warn_always_true)
>  Warn about comparisons that always evaluate to true
>
> +Warray-bounds
> +Common Var(warn_array_bounds)
> +Warn if an array is accessed out of bounds
> +
>  Wattributes
>  Common Var(warn_attributes) Init(1)
>  Warn about inappropriate attribute usage
> --- tree.c      (revision 119972)
> +++ tree.c      (working copy)
> @@ -5003,7 +5003,7 @@ simple_cst_equal (tree t1, tree t2)
>  int
>  compare_tree_int (tree t, unsigned HOST_WIDE_INT u)
>  {
> -  if (tree_int_cst_sgn (t) < 0)
> +  if (!TYPE_UNSIGNED (TREE_TYPE (t)) && TREE_INT_CST_HIGH (t) < 0)
>      return -1;
>    else if (TREE_INT_CST_HIGH (t) != 0)
>      return 1;
> --- tree-vrp.c  (revision 119972)
> +++ tree-vrp.c  (working copy)
> @@ -32,6 +32,7 @@ Boston, MA 02110-1301, USA.  */
>  #include "tree-dump.h"
>  #include "timevar.h"
>  #include "diagnostic.h"
> +#include "toplev.h"
>  #include "cfgloop.h"
>  #include "tree-scalar-evolution.h"
>  #include "tree-ssa-propagate.h"
> @@ -3420,6 +3420,142 @@ insert_range_assertions (void)
>    BITMAP_FREE (need_assert_for);
>  }
>
> +/* Checks one ARRAY_REF in REF, located at LOCUS. Ignores flexible arrays
> +   and "struct" hacks. If VRP can determine that the
> +   array subscript is a contant, check if it is outside valid
> +   range. If the array subscript is a RANGE, warn if it is
> +   non-overlapping with valid range.
> +   IGNORE_OFF_BY_ONE is true if the ARRAY_REF is inside a ADDR_EXPR.  */
> +
> +static void
> +check_array_ref (tree ref, location_t* locus, bool ignore_off_by_one)
> +{
> +  value_range_t* vr = NULL;
> +  tree low_sub, up_sub;
> +  tree low_bound, up_bound = array_ref_up_bound(ref);
> +
> +  low_sub = up_sub = TREE_OPERAND (ref, 1);
> +
> +  if (!up_bound || !locus || TREE_NO_WARNING (ref)
> +      || TREE_CODE (up_bound) != INTEGER_CST
> +      /* Can not check flexible arrays.  */
> +      || (TYPE_SIZE (TREE_TYPE (ref)) == NULL_TREE
> +          && TYPE_DOMAIN (TREE_TYPE (ref)) != NULL_TREE
> +          && 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)
> +    return;
> +
> +  low_bound = array_ref_low_bound(ref);
> +
> +  if (TREE_CODE (low_sub) == SSA_NAME)
> +    {
> +      vr = get_value_range (low_sub);
> +      if (vr->type == VR_RANGE || vr->type == VR_ANTI_RANGE)
> +        {
> +          low_sub = vr->type == VR_RANGE ? vr->max : vr->min;
> +          up_sub = vr->type == VR_RANGE ? vr->min : vr->max;
> +        }
> +    }
> +
> +  TREE_NO_WARNING (ref) = 1;
> +
> +  if (vr && vr->type == VR_ANTI_RANGE)
> +    {
> +      if (TREE_CODE (up_sub) == INTEGER_CST
> +          && tree_int_cst_lt (up_bound, up_sub)
> +          && TREE_CODE (low_sub) == INTEGER_CST
> +          && tree_int_cst_lt (low_sub, low_bound))
> +        warning (OPT_Warray_bounds,
> +                 "%Harray subscript is outside array bounds", locus);
> +      else
> +        TREE_NO_WARNING (ref) = 0;
> +    }
> +  else if (TREE_CODE (up_sub) == INTEGER_CST
> +           && tree_int_cst_lt (up_bound, up_sub)
> +           && !tree_int_cst_equal (up_bound, up_sub)
> +           && (!ignore_off_by_one
> +               || !tree_int_cst_equal (int_const_binop (PLUS_EXPR,
> +                                                        up_bound,
> +                                                        integer_one_node,
> +                                                        0),
> +                                       up_sub)))
> +    warning (OPT_Warray_bounds, "%Harray subscript is above array bounds",
> +             locus);
> +  else if (TREE_CODE (low_sub) == INTEGER_CST
> +           && tree_int_cst_lt (low_sub, low_bound))
> +    warning (OPT_Warray_bounds, "%Harray subscript is below array bounds",
> +             locus);
> +  else
> +    TREE_NO_WARNING (ref) = 0;
> +}
> +
> +static bool array_bounds_already_done;
> +
> +/* walk_tree() callback that checks if *TP is
> +   an ARRAY_REF inside an ADDR_EXPR (in which an array
> +   subscript one outside the valid range is allowed). Call
> +   check_array_ref for each ARRAY_REF found. The location is
> +   passed in DATA.  */
> +
> +static tree
> +check_array_bounds (tree *tp, int *walk_subtree, void *data)
> +{
> + tree t = *tp;
> +
> + *walk_subtree = TRUE;
> +
> +  if (TREE_CODE (t) == ARRAY_REF)
> +    check_array_ref (t, (location_t*) data, false /*ignore_off_by_one*/);
> +  else if (TREE_CODE (t) == ADDR_EXPR)
> +    {
> +       t = TREE_OPERAND (t, 0);
> +       while (!array_bounds_already_done && handled_component_p (t))
> +         {
> +           if (TREE_CODE (t) == ARRAY_REF)
> +             check_array_ref (t, (location_t*) data,
> true /*ignore_off_by_one*/);
> +           t = TREE_OPERAND (t, 0);
> +         }
> +       *walk_subtree = FALSE;
> +    }
> +
> +  return NULL_TREE;
> +}
> +
> +/* Walk over all statements of all reachable BBs and call check_array_bounds
> +   on them.  */
> +
> +static void
> +check_all_array_refs (void)
> +{
> +  basic_block bb;
> +  block_stmt_iterator si;
> +
> +  FOR_EACH_BB (bb)
> +    {
> +      /* Skip bb's that are clearly unreachable.  */
> +      if (single_pred_p (bb))
> +      {
> +       basic_block pred_bb = EDGE_PRED (bb, 0)->src;
> +       tree ls = NULL_TREE;
> +
> +       if (!bsi_end_p (bsi_last (pred_bb)))
> +         ls = bsi_stmt (bsi_last (pred_bb));
> +
> +       if (ls && TREE_CODE (ls) == COND_EXPR
> +           && ((COND_EXPR_COND (ls) == boolean_false_node
> +                && (EDGE_PRED (bb, 0)->flags & EDGE_TRUE_VALUE))
> +               || (COND_EXPR_COND (ls) == boolean_true_node
> +                   && (EDGE_PRED (bb, 0)->flags & EDGE_FALSE_VALUE))))
> +         continue;
> +      }
> +      for (si = bsi_start (bb); !bsi_end_p (si); bsi_next (&si))
> +       walk_tree (bsi_stmt_ptr (si), check_array_bounds,
> +                  EXPR_LOCUS (*bsi_stmt_ptr (si)), NULL);
> +    }
> +}
>
>  /* Convert range assertion expressions into the implied copies and
>     copy propagate away the copies.  Doing the trivial copy propagation
> @@ -4733,6 +4869,19 @@ vrp_finalize (void)
>
>    substitute_and_fold (single_val_range, true);
>
> +  if (warn_array_bounds)
> +      check_all_array_refs();
> +
> +  /* workaround for PR/26726. The problem here is that -fivopts
> +     sometimes shifts offsets so that arrays are accessed apparently
> +     out of bounds, but actually are not. therefore we do not warn
> +     about ARRAY_REF's inside ADDR_EXPR's anymore after the first run
> +     (which is before ivopts).
> +   */
> +
> +  array_bounds_already_done = true;
> +
> +
>    /* We must identify jump threading opportunities before we release
>       the datastructures built by VRP.  */
>    identify_jump_threads ();
> Index: testsuite/gcc.dg/Warray-bounds.c
> ===================================================================
> --- testsuite/gcc.dg/Warray-bounds.c    (revision 0)
> +++ testsuite/gcc.dg/Warray-bounds.c    (revision 0)
> @@ -0,0 +1,92 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -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;
> +
> +    a[-1] = 0;             /* { dg-warning "array subscript" } */
> +    a[ 0] = 0;
> +    a[ 1] = 0;
> +
> +
> +    a[ 9] = 0;
> +    a[10] = 0;             /* { dg-warning "array subscript" } */
> +    a[11] = 0;             /* { dg-warning "array subscript" } */
> +    a[2 * n() - 11] = 0;    /* { dg-warning "array subscript" } */
> +    a[2 * n() - 10] = 0;
> +    a[2 * n() -  1] = 0;
> +    a[2 * n() -  0] = 0;    /* { dg-warning "array subscript" } */
> +
> +    b[-1] = 0;             /* { dg-warning "array subscript" } */
> +    b[ 0] = 0;
> +    b[ 1] = 0;
> +    b[ 9] = 0;
> +    b[10] = 0;             /* { dg-warning "array subscript" } */
> +    b[11] = 0;             /* { dg-warning "array subscript" } */
> +    b[2 * n() - 11] = 0;    /* { dg-warning "array subscript" } */
> +    b[2 * n() - 10] = 0;
> +    b[2 * n() -  1] = 0;
> +    b[2 * n() -  0] = 0;    /* { dg-warning "array subscript" } */
> +
> +    c.c[-1] = 0;           /* { dg-warning "array subscript" } */
> +    c.c[ 0] = 0;
> +    c.c[ 1] = 0;
> +    c.c[ 9] = 0;
> +    c.c[10] = 0;           /* { dg-warning "array subscript" } */
> +    c.c[11] = 0;           /* { dg-warning "array subscript" } */
> +    c.c[2 * n() - 11] = 0;  /* { dg-warning "array subscript" } */
> +    c.c[2 * n() - 10] = 0;
> +    c.c[2 * n() -  1] = 0;
> +    c.c[2 * n() -  0] = 0;  /* { dg-warning "array subscript" } */
> +
> +    g(&a[8]);
> +    g(&a[9]);
> +    g(&a[10]);
> +    g(&a[11]);             /* { 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" } */
> +
> +    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]);
> +
> +    if (10 < 10)
> +       a[10] = 0;
> +    if (10 < 10)
> +       b[10] = 0;
> +    if (-1 >= 0)
> +       c.c[-1] = 0;
> +
> +    for (i = 20; i < 30; ++i)
> +             a[i] = 1;       /* { dg-warning "array subscript" } */
> +
> +    return a;
> +}
> +
>
>

Re: [PATCH] Fix PR/8268: implement compile time array subscript checking

[Roger Sayle]

Hi Dirk,

Given you haven't committed this patch yet, I thought I'd comment on a few
minor style issues missed by Richard's review.

On Thu, 21 Dec 2006, Dirk Mueller wrote:
> 	* tree-vrp.c (vrp_finalize): Call check_array_refs if -Warray-bounds
> 	is enabled.
> 	* tree.c (simple_cst_equal): Manually inline tree_int_cst_sgn.
> 	(check_array_refs, check_array_bounds, check_array_ref): New.

The new check_array_refs, check_array_bounds and check_array_ref are
actually in tree-vrp.c not tree.c.  Looks like the lines of your ChangeLog
entry have become scrambled.

> +  tree low_bound, up_bound = array_ref_up_bound(ref);

Space needed between "array_ref_up_bound" and "(ref);"

> +  low_bound = array_ref_low_bound(ref);

Likewise.

> +static bool array_bounds_already_done;

New global variables require a comment above them, describing their
semantics.  Particularly important is their scope.  It looks like this
variable is never reset, so you may only be reporting array bounds
problems (or some class of array bound problems) for the first function
in a file (or the first function to reach VRP).

> +/* walk_tree() callback that checks if *TP is
> +   an ARRAY_REF inside an ADDR_EXPR (in which an array
> +   subscript one outside the valid range is allowed). Call
> +   check_array_ref for each ARRAY_REF found. The location is
> +   passed in DATA.  */
> +
> +static tree
> +check_array_bounds (tree *tp, int *walk_subtree, void *data)
> +{
> +
> + tree t = *tp;
> +
> + *walk_subtree = TRUE;

Looks like you have indentation issues in this function.  The usual
GNU style indentation is two spaces, and multiples thereof.

> +  /* workaround for PR/26726. The problem here is that -fivopts
> +     sometimes shifts offsets so that arrays are accessed apparently
> +     out of bounds, but actually are not. therefore we do not warn
> +     about ARRAY_REF's inside ADDR_EXPR's anymore after the first run
> +     (which is before ivopts).
> +   */

Comments should begin with a capital letter.  The closing "*/" needs
to be on the previous line.


As a very minor style nit, though this is just a personal preference,
I find the idiom that you use in check_array_ref a little awkward and
potentially inefficient.  This sequence:

    TREE_NO_WARNING (ref) = 1;
    if (condition)
      warning (...);
    else
      TREE_NO_WARNING (ref) = 0;

initially unconditionally sets the warning issued indicator, then decides
whether or not to issue the warning and ultimately may reset it back to
zero in the common case.  Good style is to avoid writing to memory if
that's not necessary, especially for tree nodes and other datastructures
which it would be nice to keep immutable, share, store in read-only
memory or clean VM pages.  Much preferred is something like:

    if (condition)
      {
        TREE_NO_WARNING (ref) = 1;
        warning (...);
      }

where the common path now has no writes to memory.


It's true that partial dead code elimination, or lazy code motion
or other optimizations should eventually be able to clean this idiom
up for us (once implemented), but it's good practice to write
efficient/readable code to start with.

I hope this helps.

Roger
--

Re: [PATCH] Fix PR/8268: implement compile time array subscript checking

[Dirk Mueller]

[-- Attachment #1: Type: text/plain, Size: 1452 bytes --]

On Saturday, 13. January 2007 18:35, Roger Sayle wrote:

I've addressed your coding style comments. 

> > +static bool array_bounds_already_done;
> New global variables require a comment above them, describing their
> semantics.  Particularly important is their scope.  It looks like this
> variable is never reset, so you may only be reporting array bounds
> problems (or some class of array bound problems) for the first function
> in a file (or the first function to reach VRP).

Hmm, indeed, this is a major oversight which I forgot about :-( It sucks when 
you put aside a half-finished patch for almost a year and then forget about 
all the unfinished todo's. Many thanks for catching this. I've reworked the 
patch by removing this static variable and adding a workaround to avoid 
warning on such statements as generated by the optimizing passes.

> As a very minor style nit, though this is just a personal preference,
> initially unconditionally sets the warning issued indicator, then decides
> whether or not to issue the warning and ultimately may reset it back to
> zero in the common case. 

Correct, which made the patch quite some lines shorter :)

> Much preferred is something like: 

Ok, I don't mind much, so I've changed it that way now. 

Many thanks for looking at the patch. 

Patch below is bootstrapped and regtested against not entirely recent trunk 
(because of the current bootstrap failure) with no new failures. 


Dirk

[-- Attachment #2: array_bounds_vrp_4.patch --]
[-- Type: text/x-diff, Size: 11107 bytes --]

2007-01-15  Dirk Mueller  <dmueller@suse.de>
	    Richard Guenther <rguenther@suse.de>

	PR diagnostic/8268
	* doc/invoke.texi (Warray-bounds): Document -Warray-bounds.
	* common.opt (Warray-bounds): Add new warning option.
	* c-opts.c (c_common_handle_option): Define -Warray-bounds
	if -Wall is given.
	* tree-vrp.c (vrp_finalize): Call check_array_refs if -Warray-bounds
	is enabled.
	(check_array_refs, check_array_bounds, check_array_ref): New.

	* testsuite/gcc.dg/Warray-bounds.c: New testcase.


--- doc/invoke.texi	(revision 120757)
+++ doc/invoke.texi	(working copy)
@@ -247,7 +247,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wreturn-type  -Wsequence-point  -Wshadow @gol
 -Wsign-compare  -Wstack-protector @gol
 -Wstrict-aliasing -Wstrict-aliasing=2 @gol
--Wstring-literal-comparison @gol
+-Wstring-literal-comparison -Warray-bounds @gol
 -Wswitch  -Wswitch-default  -Wswitch-enum @gol
 -Wsystem-headers  -Wtrigraphs  -Wundef  -Wuninitialized @gol
 -Wunknown-pragmas  -Wno-pragmas -Wunreachable-code @gol
@@ -2842,6 +2843,12 @@ compiler is using for optimization.  Thi
 @option{-Wstrict-aliasing}, but it will also give a warning for some ambiguous
 cases that are safe.
 
+@item -Warray-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.
+
 @item -Wall
 @opindex Wall
 All of the above @samp{-W} options combined.  This enables all the
--- common.opt	(revision 120757)
+++ common.opt	(working copy)
@@ -61,6 +61,10 @@ Walways-true
 Common Var(warn_always_true)
 Warn about comparisons that always evaluate to true
 
+Warray-bounds
+Common Var(warn_array_bounds)
+Warn if an array is accessed out of bounds
+
 Wattributes
 Common Var(warn_attributes) Init(1)
 Warn about inappropriate attribute usage
--- Makefile.in	(revision 120757)
+++ Makefile.in	(working copy)
@@ -1862,7 +1862,7 @@ tree-vn.o : tree-vn.c $(CONFIG_H) $(SYST
 tree-vrp.o : tree-vrp.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) \
    $(TREE_FLOW_H) tree-pass.h $(TREE_DUMP_H) $(DIAGNOSTIC_H) $(GGC_H) \
    $(BASIC_BLOCK_H) tree-ssa-propagate.h $(FLAGS_H) $(TREE_DUMP_H) \
-   $(CFGLOOP_H) $(SCEV_H) tree-chrec.h $(TIMEVAR_H)
+   $(CFGLOOP_H) $(SCEV_H) tree-chrec.h $(TIMEVAR_H) toplev.h
 tree-cfg.o : tree-cfg.c $(TREE_FLOW_H) $(CONFIG_H) $(SYSTEM_H) \
    $(RTL_H) $(TREE_H) $(TM_P_H) $(EXPR_H) $(GGC_H) $(FLAGS_H) output.h \
    $(DIAGNOSTIC_H) $(FUNCTION_H) $(TIMEVAR_H) $(TM_H) coretypes.h \
--- tree-vrp.c	(revision 120757)
+++ tree-vrp.c	(working copy)
@@ -32,6 +32,7 @@ Boston, MA 02110-1301, USA.  */
 #include "tree-dump.h"
 #include "timevar.h"
 #include "diagnostic.h"
+#include "toplev.h"
 #include "cfgloop.h"
 #include "tree-scalar-evolution.h"
 #include "tree-ssa-propagate.h"
@@ -3478,6 +3478,154 @@ insert_range_assertions (void)
   BITMAP_FREE (need_assert_for);
 }
 
+/* Checks one ARRAY_REF in REF, located at LOCUS. Ignores flexible arrays
+   and "struct" hacks. If VRP can determine that the
+   array subscript is a contant, check if it is outside valid
+   range. If the array subscript is a RANGE, warn if it is
+   non-overlapping with valid range.
+   IGNORE_OFF_BY_ONE is true if the ARRAY_REF is inside a ADDR_EXPR.  */
+
+static void
+check_array_ref (tree ref, location_t* locus, bool ignore_off_by_one)
+{
+  value_range_t* vr = NULL;
+  tree low_sub, up_sub;
+  tree low_bound, up_bound = array_ref_up_bound (ref);
+
+  low_sub = up_sub = TREE_OPERAND (ref, 1);
+
+  if (!up_bound || !locus || TREE_NO_WARNING (ref)
+      || TREE_CODE (up_bound) != INTEGER_CST
+      /* Can not check flexible arrays.  */
+      || (TYPE_SIZE (TREE_TYPE (ref)) == NULL_TREE
+          && TYPE_DOMAIN (TREE_TYPE (ref)) != NULL_TREE
+          && 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)
+    return;
+
+  low_bound = array_ref_low_bound (ref);
+
+  if (TREE_CODE (low_sub) == SSA_NAME)
+    {
+      vr = get_value_range (low_sub);
+      if (vr->type == VR_RANGE || vr->type == VR_ANTI_RANGE)
+        {
+          low_sub = vr->type == VR_RANGE ? vr->max : vr->min;
+          up_sub = vr->type == VR_RANGE ? vr->min : vr->max;
+        }
+    }
+
+  if (vr && vr->type == VR_ANTI_RANGE)
+    {
+      if (TREE_CODE (up_sub) == INTEGER_CST
+          && tree_int_cst_lt (up_bound, up_sub)
+          && TREE_CODE (low_sub) == INTEGER_CST
+          && tree_int_cst_lt (low_sub, low_bound))
+        {
+          warning (OPT_Warray_bounds,
+                   "%Harray subscript is outside array bounds", locus);
+          TREE_NO_WARNING (ref) = 1;
+        }
+    }
+  else if (TREE_CODE (up_sub) == INTEGER_CST
+           && tree_int_cst_lt (up_bound, up_sub)
+           && !tree_int_cst_equal (up_bound, up_sub)
+           && (!ignore_off_by_one
+               || !tree_int_cst_equal (int_const_binop (PLUS_EXPR,
+                                                        up_bound,
+                                                        integer_one_node,
+                                                        0),
+                                       up_sub)))
+    {
+      warning (OPT_Warray_bounds, "%Harray subscript is above array bounds",
+               locus);
+      TREE_NO_WARNING (ref) = 1;
+    }
+  else if (TREE_CODE (low_sub) == INTEGER_CST
+           && tree_int_cst_lt (low_sub, low_bound))
+    {
+      warning (OPT_Warray_bounds, "%Harray subscript is below array bounds",
+               locus);
+      TREE_NO_WARNING (ref) = 1;
+    }
+}
+
+/* walk_tree() callback that checks if *TP is
+   an ARRAY_REF inside an ADDR_EXPR (in which an array
+   subscript one outside the valid range is allowed). Call
+   check_array_ref for each ARRAY_REF found. The location is 
+   passed in DATA.  */
+
+static tree
+check_array_bounds (tree *tp, int *walk_subtree, void *data)
+{
+  tree t = *tp;
+  location_t *location = EXPR_LOCUS ((tree) data);
+
+  *walk_subtree = TRUE;
+
+  if (TREE_CODE (t) == ARRAY_REF)
+    check_array_ref (t, location, false /*ignore_off_by_one*/);
+  else if (TREE_CODE (t) == ADDR_EXPR)
+    {
+       t = TREE_OPERAND (t, 0);
+
+       /* Don't warn on statements like
+          ssa_name = 500 + &array[-200] which are sometimes
+          produced by various optimizing passes.  */
+       if (TREE_CODE ((tree)data) == GIMPLE_MODIFY_STMT
+           && BINARY_CLASS_P (GIMPLE_STMT_OPERAND ((tree)data, 1)))
+         {
+           *walk_subtree = FALSE;
+           return NULL_TREE;
+         }
+       while (handled_component_p (t))
+         {
+           if (TREE_CODE (t) == ARRAY_REF)
+             check_array_ref (t, location, true /*ignore_off_by_one*/);
+           t = TREE_OPERAND (t, 0);
+         }
+       *walk_subtree = FALSE;
+    }
+
+  return NULL_TREE;
+}
+
+/* Walk over all statements of all reachable BBs and call check_array_bounds
+   on them.  */
+
+static void
+check_all_array_refs (void)
+{
+  basic_block bb;
+  block_stmt_iterator si;
+
+  FOR_EACH_BB (bb)
+    {
+      /* Skip bb's that are clearly unreachable.  */
+      if (single_pred_p (bb))
+      {
+	basic_block pred_bb = EDGE_PRED (bb, 0)->src;
+	tree ls = NULL_TREE;
+
+	if (!bsi_end_p (bsi_last (pred_bb)))
+	  ls = bsi_stmt (bsi_last (pred_bb));
+
+	if (ls && TREE_CODE (ls) == COND_EXPR
+	    && ((COND_EXPR_COND (ls) == boolean_false_node
+		 && (EDGE_PRED (bb, 0)->flags & EDGE_TRUE_VALUE))
+		|| (COND_EXPR_COND (ls) == boolean_true_node
+		    && (EDGE_PRED (bb, 0)->flags & EDGE_FALSE_VALUE))))
+	  continue;
+      }
+      for (si = bsi_start (bb); !bsi_end_p (si); bsi_next (&si))
+	walk_tree (bsi_stmt_ptr (si), check_array_bounds,
+		   bsi_stmt (si), NULL);
+    }
+}
 
 /* Convert range assertion expressions into the implied copies and
    copy propagate away the copies.  Doing the trivial copy propagation
@@ -4797,6 +4945,9 @@ vrp_finalize (void)
 
   substitute_and_fold (single_val_range, true);
 
+  if (warn_array_bounds)
+      check_all_array_refs();
+
   /* We must identify jump threading opportunities before we release
      the datastructures built by VRP.  */
   identify_jump_threads ();
--- testsuite/gcc.dg/Warray-bounds.c	(revision 0)
+++ testsuite/gcc.dg/Warray-bounds.c	(revision 0)
@@ -0,0 +1,94 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -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;
+
+    a[-1] = 0;             /* { dg-warning "array subscript" } */
+    a[ 0] = 0;
+    a[ 1] = 0;
+
+
+    a[ 9] = 0;
+    a[10] = 0;             /* { dg-warning "array subscript" } */
+    a[11] = 0;             /* { dg-warning "array subscript" } */
+    a[2 * n() - 11] = 0;    /* { dg-warning "array subscript" } */
+    a[2 * n() - 10] = 0;
+    a[2 * n() -  1] = 0;
+    a[2 * n() -  0] = 0;    /* { dg-warning "array subscript" } */
+
+    b[-1] = 0;             /* { dg-warning "array subscript" } */
+    b[ 0] = 0;
+    b[ 1] = 0;
+    b[ 9] = 0;
+    b[10] = 0;             /* { dg-warning "array subscript" } */
+    b[11] = 0;             /* { dg-warning "array subscript" } */
+    b[2 * n() - 11] = 0;    /* { dg-warning "array subscript" } */
+    b[2 * n() - 10] = 0;
+    b[2 * n() -  1] = 0;
+    b[2 * n() -  0] = 0;    /* { dg-warning "array subscript" } */
+
+    c.c[-1] = 0;           /* { dg-warning "array subscript" } */
+    c.c[ 0] = 0;
+    c.c[ 1] = 0;
+    c.c[ 9] = 0;
+    c.c[10] = 0;           /* { dg-warning "array subscript" } */
+    c.c[11] = 0;           /* { dg-warning "array subscript" } */
+    c.c[2 * n() - 11] = 0;  /* { dg-warning "array subscript" } */
+    c.c[2 * n() - 10] = 0;
+    c.c[2 * n() -  1] = 0;
+    c.c[2 * n() -  0] = 0;  /* { dg-warning "array subscript" } */
+
+    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);
+
+    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" } */
+
+    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]);
+
+    if (10 < 10)
+       a[10] = 0;
+    if (10 < 10)
+       b[10] = 0;
+    if (-1 >= 0)
+       c.c[-1] = 0;
+
+    for (i = 20; i < 30; ++i)
+             a[i] = 1;       /* { dg-warning "array subscript" } */
+
+    return a;
+}
+

Re: [PATCH] Fix PR/8268: implement compile time array subscript checking

[Manuel López-Ibáñez]

On 16/01/07, Dirk Mueller <dmuell@gmx.net> wrote:
>
> Patch below is bootstrapped and regtested against not entirely recent trunk
> (because of the current bootstrap failure) with no new failures.
>
>
+@item -Warray-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.
+
 @item -Wall
 @opindex Wall
 All of the above @samp{-W} options combined.  This enables all the

I think we should start adding the negative form in opindex as we have
discussed elsewhere:
+@opindex Wno-array-bounds

Also, the placemente is far from ideal, the option is not enabled by
Wall (despite of what your Changelog says), so it would be best to
place it after the sentence "All of the above  @samp{-W}  options
combined".

This also applies for the list of options:
 -Wstrict-aliasing -Wstrict-aliasing=2 @gol
--Wstring-literal-comparison @gol
+-Wstring-literal-comparison -Warray-bounds @gol
 -Wswitch  -Wswitch-default  -Wswitch-enum @gol

Why don't you put the option on its place in alphabetical order?

Re: [PATCH] Fix PR/8268: implement compile time array subscript checking

[Dirk Mueller]

[-- Attachment #1: Type: text/plain, Size: 354 bytes --]

On Tuesday, 16. January 2007 15:16, Manuel López-Ibáñez wrote:

> Also, the placemente is far from ideal, the option is not enabled by
> Wall (despite of what your Changelog says)

I forgot this particular hunk in the patch I mailed. I've resorted the invoke 
entry to be in alphabetical order and added the -Wno-array-bounds opindex. 

Thanks,

Dirk

[-- Attachment #2: array_bounds_vrp_5.patch --]
[-- Type: text/x-diff, Size: 11734 bytes --]

2007-01-15  Dirk Mueller  <dmueller@suse.de>
	    Richard Guenther <rguenther@suse.de>

	PR diagnostic/8268
	* doc/invoke.texi (Warray-bounds): Document -Warray-bounds.
	* common.opt (Warray-bounds): Add new warning option.
	* c-opts.c (c_common_handle_option): Define -Warray-bounds
	if -Wall is given.
        * Makefile.in: make tree-vrp.o depend on toplev.h
	* tree-vrp.c (vrp_finalize): Call check_array_refs if -Warray-bounds
	is enabled.
	(check_array_refs, check_array_bounds, check_array_ref): New.

	* testsuite/gcc.dg/Warray-bounds.c: New testcase.

--- doc/invoke.texi	(revision 120757)
+++ doc/invoke.texi	(working copy)
@@ -222,8 +222,8 @@ Objective-C and Objective-C++ Dialects}.
 @item Warning Options
 @xref{Warning Options,,Options to Request or Suppress Warnings}.
 @gccoptlist{-fsyntax-only  -pedantic  -pedantic-errors @gol
--w  -Wextra  -Wall  -Waggregate-return -Walways-true -Wno-attributes @gol
--Wc++-compat -Wcast-align  -Wcast-qual  -Wchar-subscripts @gol
+-w  -Wextra  -Wall  -Waggregate-return -Walways-true -Warray-bounds @gol
+-Wno-attributes -Wc++-compat -Wcast-align  -Wcast-qual  -Wchar-subscripts @gol
 -Wclobbered  -Wcomment @gol
 -Wconversion  -Wno-deprecated-declarations @gol
 -Wdisabled-optimization  -Wno-div-by-zero  @gol
@@ -2842,6 +2843,13 @@ compiler is using for optimization.  Thi
 @option{-Wstrict-aliasing}, but it will also give a warning for some ambiguous
 cases that are safe.
 
+@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.
+
 @item -Wall
 @opindex Wall
 All of the above @samp{-W} options combined.  This enables all the
--- common.opt	(revision 120757)
+++ common.opt	(working copy)
@@ -61,6 +61,10 @@ Walways-true
 Common Var(warn_always_true)
 Warn about comparisons that always evaluate to true
 
+Warray-bounds
+Common Var(warn_array_bounds)
+Warn if an array is accessed out of bounds
+
 Wattributes
 Common Var(warn_attributes) Init(1)
 Warn about inappropriate attribute usage
--- c-opts.c	(revision 120757)
+++ c-opts.c	(working copy)
@@ -396,6 +396,7 @@ c_common_handle_option (size_t scode, co
       warn_strict_aliasing = value;
       warn_string_literal_comparison = value;
       warn_always_true = value;
+      warn_array_bounds = value;
 
       /* Only warn about unknown pragmas that are not in system
 	 headers.  */
--- Makefile.in	(revision 120757)
+++ Makefile.in	(working copy)
@@ -1862,7 +1862,7 @@ tree-vn.o : tree-vn.c $(CONFIG_H) $(SYST
 tree-vrp.o : tree-vrp.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) \
    $(TREE_FLOW_H) tree-pass.h $(TREE_DUMP_H) $(DIAGNOSTIC_H) $(GGC_H) \
    $(BASIC_BLOCK_H) tree-ssa-propagate.h $(FLAGS_H) $(TREE_DUMP_H) \
-   $(CFGLOOP_H) $(SCEV_H) tree-chrec.h $(TIMEVAR_H)
+   $(CFGLOOP_H) $(SCEV_H) tree-chrec.h $(TIMEVAR_H) toplev.h
 tree-cfg.o : tree-cfg.c $(TREE_FLOW_H) $(CONFIG_H) $(SYSTEM_H) \
    $(RTL_H) $(TREE_H) $(TM_P_H) $(EXPR_H) $(GGC_H) $(FLAGS_H) output.h \
    $(DIAGNOSTIC_H) $(FUNCTION_H) $(TIMEVAR_H) $(TM_H) coretypes.h \
--- tree-vrp.c	(revision 120757)
+++ tree-vrp.c	(working copy)
@@ -32,6 +32,7 @@ Boston, MA 02110-1301, USA.  */
 #include "tree-dump.h"
 #include "timevar.h"
 #include "diagnostic.h"
+#include "toplev.h"
 #include "cfgloop.h"
 #include "tree-scalar-evolution.h"
 #include "tree-ssa-propagate.h"
@@ -3478,6 +3478,154 @@ insert_range_assertions (void)
   BITMAP_FREE (need_assert_for);
 }
 
+/* Checks one ARRAY_REF in REF, located at LOCUS. Ignores flexible arrays
+   and "struct" hacks. If VRP can determine that the
+   array subscript is a contant, check if it is outside valid
+   range. If the array subscript is a RANGE, warn if it is
+   non-overlapping with valid range.
+   IGNORE_OFF_BY_ONE is true if the ARRAY_REF is inside a ADDR_EXPR.  */
+
+static void
+check_array_ref (tree ref, location_t* locus, bool ignore_off_by_one)
+{
+  value_range_t* vr = NULL;
+  tree low_sub, up_sub;
+  tree low_bound, up_bound = array_ref_up_bound (ref);
+
+  low_sub = up_sub = TREE_OPERAND (ref, 1);
+
+  if (!up_bound || !locus || TREE_NO_WARNING (ref)
+      || TREE_CODE (up_bound) != INTEGER_CST
+      /* Can not check flexible arrays.  */
+      || (TYPE_SIZE (TREE_TYPE (ref)) == NULL_TREE
+          && TYPE_DOMAIN (TREE_TYPE (ref)) != NULL_TREE
+          && 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)
+    return;
+
+  low_bound = array_ref_low_bound (ref);
+
+  if (TREE_CODE (low_sub) == SSA_NAME)
+    {
+      vr = get_value_range (low_sub);
+      if (vr->type == VR_RANGE || vr->type == VR_ANTI_RANGE)
+        {
+          low_sub = vr->type == VR_RANGE ? vr->max : vr->min;
+          up_sub = vr->type == VR_RANGE ? vr->min : vr->max;
+        }
+    }
+
+  if (vr && vr->type == VR_ANTI_RANGE)
+    {
+      if (TREE_CODE (up_sub) == INTEGER_CST
+          && tree_int_cst_lt (up_bound, up_sub)
+          && TREE_CODE (low_sub) == INTEGER_CST
+          && tree_int_cst_lt (low_sub, low_bound))
+        {
+          warning (OPT_Warray_bounds,
+                   "%Harray subscript is outside array bounds", locus);
+          TREE_NO_WARNING (ref) = 1;
+        }
+    }
+  else if (TREE_CODE (up_sub) == INTEGER_CST
+           && tree_int_cst_lt (up_bound, up_sub)
+           && !tree_int_cst_equal (up_bound, up_sub)
+           && (!ignore_off_by_one
+               || !tree_int_cst_equal (int_const_binop (PLUS_EXPR,
+                                                        up_bound,
+                                                        integer_one_node,
+                                                        0),
+                                       up_sub)))
+    {
+      warning (OPT_Warray_bounds, "%Harray subscript is above array bounds",
+               locus);
+      TREE_NO_WARNING (ref) = 1;
+    }
+  else if (TREE_CODE (low_sub) == INTEGER_CST
+           && tree_int_cst_lt (low_sub, low_bound))
+    {
+      warning (OPT_Warray_bounds, "%Harray subscript is below array bounds",
+               locus);
+      TREE_NO_WARNING (ref) = 1;
+    }
+}
+
+/* walk_tree() callback that checks if *TP is
+   an ARRAY_REF inside an ADDR_EXPR (in which an array
+   subscript one outside the valid range is allowed). Call
+   check_array_ref for each ARRAY_REF found. The location is 
+   passed in DATA.  */
+
+static tree
+check_array_bounds (tree *tp, int *walk_subtree, void *data)
+{
+  tree t = *tp;
+  location_t *location = EXPR_LOCUS ((tree) data);
+
+  *walk_subtree = TRUE;
+
+  if (TREE_CODE (t) == ARRAY_REF)
+    check_array_ref (t, location, false /*ignore_off_by_one*/);
+  else if (TREE_CODE (t) == ADDR_EXPR)
+    {
+       t = TREE_OPERAND (t, 0);
+
+       /* Don't warn on statements like
+          ssa_name = 500 + &array[-200] which are sometimes
+          produced by various optimizing passes.  */
+       if (TREE_CODE ((tree)data) == GIMPLE_MODIFY_STMT
+           && BINARY_CLASS_P (GIMPLE_STMT_OPERAND ((tree)data, 1)))
+         {
+           *walk_subtree = FALSE;
+           return NULL_TREE;
+         }
+       while (handled_component_p (t))
+         {
+           if (TREE_CODE (t) == ARRAY_REF)
+             check_array_ref (t, location, true /*ignore_off_by_one*/);
+           t = TREE_OPERAND (t, 0);
+         }
+       *walk_subtree = FALSE;
+    }
+
+  return NULL_TREE;
+}
+
+/* Walk over all statements of all reachable BBs and call check_array_bounds
+   on them.  */
+
+static void
+check_all_array_refs (void)
+{
+  basic_block bb;
+  block_stmt_iterator si;
+
+  FOR_EACH_BB (bb)
+    {
+      /* Skip bb's that are clearly unreachable.  */
+      if (single_pred_p (bb))
+      {
+	basic_block pred_bb = EDGE_PRED (bb, 0)->src;
+	tree ls = NULL_TREE;
+
+	if (!bsi_end_p (bsi_last (pred_bb)))
+	  ls = bsi_stmt (bsi_last (pred_bb));
+
+	if (ls && TREE_CODE (ls) == COND_EXPR
+	    && ((COND_EXPR_COND (ls) == boolean_false_node
+		 && (EDGE_PRED (bb, 0)->flags & EDGE_TRUE_VALUE))
+		|| (COND_EXPR_COND (ls) == boolean_true_node
+		    && (EDGE_PRED (bb, 0)->flags & EDGE_FALSE_VALUE))))
+	  continue;
+      }
+      for (si = bsi_start (bb); !bsi_end_p (si); bsi_next (&si))
+	walk_tree (bsi_stmt_ptr (si), check_array_bounds,
+		   bsi_stmt (si), NULL);
+    }
+}
 
 /* Convert range assertion expressions into the implied copies and
    copy propagate away the copies.  Doing the trivial copy propagation
@@ -4797,6 +4945,9 @@ vrp_finalize (void)
 
   substitute_and_fold (single_val_range, true);
 
+  if (warn_array_bounds)
+      check_all_array_refs();
+
   /* We must identify jump threading opportunities before we release
      the datastructures built by VRP.  */
   identify_jump_threads ();
--- testsuite/gcc.dg/Warray-bounds.c	(revision 0)
+++ testsuite/gcc.dg/Warray-bounds.c	(revision 0)
@@ -0,0 +1,94 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -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;
+
+    a[-1] = 0;             /* { dg-warning "array subscript" } */
+    a[ 0] = 0;
+    a[ 1] = 0;
+
+
+    a[ 9] = 0;
+    a[10] = 0;             /* { dg-warning "array subscript" } */
+    a[11] = 0;             /* { dg-warning "array subscript" } */
+    a[2 * n() - 11] = 0;    /* { dg-warning "array subscript" } */
+    a[2 * n() - 10] = 0;
+    a[2 * n() -  1] = 0;
+    a[2 * n() -  0] = 0;    /* { dg-warning "array subscript" } */
+
+    b[-1] = 0;             /* { dg-warning "array subscript" } */
+    b[ 0] = 0;
+    b[ 1] = 0;
+    b[ 9] = 0;
+    b[10] = 0;             /* { dg-warning "array subscript" } */
+    b[11] = 0;             /* { dg-warning "array subscript" } */
+    b[2 * n() - 11] = 0;    /* { dg-warning "array subscript" } */
+    b[2 * n() - 10] = 0;
+    b[2 * n() -  1] = 0;
+    b[2 * n() -  0] = 0;    /* { dg-warning "array subscript" } */
+
+    c.c[-1] = 0;           /* { dg-warning "array subscript" } */
+    c.c[ 0] = 0;
+    c.c[ 1] = 0;
+    c.c[ 9] = 0;
+    c.c[10] = 0;           /* { dg-warning "array subscript" } */
+    c.c[11] = 0;           /* { dg-warning "array subscript" } */
+    c.c[2 * n() - 11] = 0;  /* { dg-warning "array subscript" } */
+    c.c[2 * n() - 10] = 0;
+    c.c[2 * n() -  1] = 0;
+    c.c[2 * n() -  0] = 0;  /* { dg-warning "array subscript" } */
+
+    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);
+
+    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" } */
+
+    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]);
+
+    if (10 < 10)
+       a[10] = 0;
+    if (10 < 10)
+       b[10] = 0;
+    if (-1 >= 0)
+       c.c[-1] = 0;
+
+    for (i = 20; i < 30; ++i)
+             a[i] = 1;       /* { dg-warning "array subscript" } */
+
+    return a;
+}
+

Re: [PATCH] Fix PR/8268: implement compile time array subscript checking

[Manuel López-Ibáñez]

On 16/01/07, Dirk Mueller <dmuell@gmx.net> wrote:
> On Tuesday, 16. January 2007 15:16, Manuel López-Ibáñez wrote:
>
> > Also, the placemente is far from ideal, the option is not enabled by
> > Wall (despite of what your Changelog says)
>
> I forgot this particular hunk in the patch I mailed. I've resorted the invoke
> entry to be in alphabetical order and added the -Wno-array-bounds opindex.
>

Thanks! Then, it is typical to add the sentence  "This warning is
enabled by -Wall." after the description of each warning that is
enabled by -Wall. See -Wtrigraphs, -Wuninitialized, etc.

Also, I am not sure whether you need testcases for "testing that is
enabled by -Wall" and "testing that is disabled with -Wall
-Wno-array-bounds". Let's see what diagnostics maintainers say...

Cheers,

Manuel.

PS: I guess that the new option needs to be documented in
http://gcc.gnu.org/gcc-4.3/changes.html once you have finished with
it.

Re: [PATCH] Fix PR/8268: implement compile time array subscript checking

[Gabriel Dos Reis]

On Tue, 16 Jan 2007, Manuel López-Ibáñez wrote:

| Also, I am not sure whether you need testcases for "testing that is
| enabled by -Wall" and "testing that is disabled with -Wall
| -Wno-array-bounds".

you're free to add one.

-- Gaby

Re: [PATCH] Fix PR/8268: implement compile time array subscript checking

[Dirk Mueller]

[-- Attachment #1: Type: text/plain, Size: 428 bytes --]

On Tuesday, 16. January 2007 16:52, Gabriel Dos Reis wrote:

> | Also, I am not sure whether you need testcases for "testing that is
> | enabled by -Wall" and "testing that is disabled with -Wall
> | -Wno-array-bounds".
> you're free to add one.

Ok, I've added testcases that check that the functionality is enabled for C 
and for C++. 

Attached for reference, regtest in progress. Is the patch OK if it passes?

Thanks,
Dirk

[-- Attachment #2: array_bounds_vrp_6.patch --]
[-- Type: text/x-diff, Size: 15653 bytes --]

2007-01-15  Dirk Mueller  <dmueller@suse.de>
	    Richard Guenther <rguenther@suse.de>

	PR diagnostic/8268
	* doc/invoke.texi (Warray-bounds): Document -Warray-bounds.
	* common.opt (Warray-bounds): Add new warning option.
	* c-opts.c (c_common_handle_option): Define -Warray-bounds
	if -Wall is given.
        * Makefile.in: make tree-vrp.o depend on toplev.h
	* tree-vrp.c (vrp_finalize): Call check_array_refs if -Warray-bounds
	is enabled.
	(check_array_refs, check_array_bounds, check_array_ref): New.

	* gcc.dg/Warray-bounds.c: New testcase.
        * gcc.dg/Warray-bounds-2.c: New testcase.
        * g++.dg/warn/Warray-bounds.C: New testcase.
        * g++.dg/warn/Warray-bounds-2.C: New testcase.

--- doc/invoke.texi	(revision 120757)
+++ doc/invoke.texi	(working copy)
@@ -222,8 +222,8 @@ Objective-C and Objective-C++ Dialects}.
 @item Warning Options
 @xref{Warning Options,,Options to Request or Suppress Warnings}.
 @gccoptlist{-fsyntax-only  -pedantic  -pedantic-errors @gol
--w  -Wextra  -Wall  -Waggregate-return -Walways-true -Wno-attributes @gol
--Wc++-compat -Wcast-align  -Wcast-qual  -Wchar-subscripts @gol
+-w  -Wextra  -Wall  -Waggregate-return -Walways-true -Warray-bounds @gol
+-Wno-attributes -Wc++-compat -Wcast-align  -Wcast-qual  -Wchar-subscripts @gol
 -Wclobbered  -Wcomment @gol
 -Wconversion  -Wno-deprecated-declarations @gol
 -Wdisabled-optimization  -Wno-div-by-zero  @gol
@@ -2842,6 +2843,13 @@ compiler is using for optimization.  Thi
 @option{-Wstrict-aliasing}, but it will also give a warning for some ambiguous
 cases that are safe.

+@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}.
+
 @item -Wall
 @opindex Wall
 All of the above @samp{-W} options combined.  This enables all the
--- common.opt	(revision 120757)
+++ common.opt	(working copy)
@@ -61,6 +61,10 @@ Walways-true
 Common Var(warn_always_true)
 Warn about comparisons that always evaluate to true
 
+Warray-bounds
+Common Var(warn_array_bounds)
+Warn if an array is accessed out of bounds
+
 Wattributes
 Common Var(warn_attributes) Init(1)
 Warn about inappropriate attribute usage
--- c-opts.c	(revision 120757)
+++ c-opts.c	(working copy)
@@ -396,6 +396,7 @@ c_common_handle_option (size_t scode, co
       warn_strict_aliasing = value;
       warn_string_literal_comparison = value;
       warn_always_true = value;
+      warn_array_bounds = value;
 
       /* Only warn about unknown pragmas that are not in system
 	 headers.  */
--- Makefile.in	(revision 120757)
+++ Makefile.in	(working copy)
@@ -1862,7 +1862,7 @@ tree-vn.o : tree-vn.c $(CONFIG_H) $(SYST
 tree-vrp.o : tree-vrp.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) \
    $(TREE_FLOW_H) tree-pass.h $(TREE_DUMP_H) $(DIAGNOSTIC_H) $(GGC_H) \
    $(BASIC_BLOCK_H) tree-ssa-propagate.h $(FLAGS_H) $(TREE_DUMP_H) \
-   $(CFGLOOP_H) $(SCEV_H) tree-chrec.h $(TIMEVAR_H)
+   $(CFGLOOP_H) $(SCEV_H) tree-chrec.h $(TIMEVAR_H) toplev.h
 tree-cfg.o : tree-cfg.c $(TREE_FLOW_H) $(CONFIG_H) $(SYSTEM_H) \
    $(RTL_H) $(TREE_H) $(TM_P_H) $(EXPR_H) $(GGC_H) $(FLAGS_H) output.h \
    $(DIAGNOSTIC_H) $(FUNCTION_H) $(TIMEVAR_H) $(TM_H) coretypes.h \
--- tree-vrp.c	(revision 120757)
+++ tree-vrp.c	(working copy)
@@ -32,6 +32,7 @@ Boston, MA 02110-1301, USA.  */
 #include "tree-dump.h"
 #include "timevar.h"
 #include "diagnostic.h"
+#include "toplev.h"
 #include "cfgloop.h"
 #include "tree-scalar-evolution.h"
 #include "tree-ssa-propagate.h"
@@ -3478,6 +3478,154 @@ insert_range_assertions (void)
   BITMAP_FREE (need_assert_for);
 }
 
+/* Checks one ARRAY_REF in REF, located at LOCUS. Ignores flexible arrays
+   and "struct" hacks. If VRP can determine that the
+   array subscript is a contant, check if it is outside valid
+   range. If the array subscript is a RANGE, warn if it is
+   non-overlapping with valid range.
+   IGNORE_OFF_BY_ONE is true if the ARRAY_REF is inside a ADDR_EXPR.  */
+
+static void
+check_array_ref (tree ref, location_t* locus, bool ignore_off_by_one)
+{
+  value_range_t* vr = NULL;
+  tree low_sub, up_sub;
+  tree low_bound, up_bound = array_ref_up_bound (ref);
+
+  low_sub = up_sub = TREE_OPERAND (ref, 1);
+
+  if (!up_bound || !locus || TREE_NO_WARNING (ref)
+      || TREE_CODE (up_bound) != INTEGER_CST
+      /* Can not check flexible arrays.  */
+      || (TYPE_SIZE (TREE_TYPE (ref)) == NULL_TREE
+          && TYPE_DOMAIN (TREE_TYPE (ref)) != NULL_TREE
+          && 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)
+    return;
+
+  low_bound = array_ref_low_bound (ref);
+
+  if (TREE_CODE (low_sub) == SSA_NAME)
+    {
+      vr = get_value_range (low_sub);
+      if (vr->type == VR_RANGE || vr->type == VR_ANTI_RANGE)
+        {
+          low_sub = vr->type == VR_RANGE ? vr->max : vr->min;
+          up_sub = vr->type == VR_RANGE ? vr->min : vr->max;
+        }
+    }
+
+  if (vr && vr->type == VR_ANTI_RANGE)
+    {
+      if (TREE_CODE (up_sub) == INTEGER_CST
+          && tree_int_cst_lt (up_bound, up_sub)
+          && TREE_CODE (low_sub) == INTEGER_CST
+          && tree_int_cst_lt (low_sub, low_bound))
+        {
+          warning (OPT_Warray_bounds,
+                   "%Harray subscript is outside array bounds", locus);
+          TREE_NO_WARNING (ref) = 1;
+        }
+    }
+  else if (TREE_CODE (up_sub) == INTEGER_CST
+           && tree_int_cst_lt (up_bound, up_sub)
+           && !tree_int_cst_equal (up_bound, up_sub)
+           && (!ignore_off_by_one
+               || !tree_int_cst_equal (int_const_binop (PLUS_EXPR,
+                                                        up_bound,
+                                                        integer_one_node,
+                                                        0),
+                                       up_sub)))
+    {
+      warning (OPT_Warray_bounds, "%Harray subscript is above array bounds",
+               locus);
+      TREE_NO_WARNING (ref) = 1;
+    }
+  else if (TREE_CODE (low_sub) == INTEGER_CST
+           && tree_int_cst_lt (low_sub, low_bound))
+    {
+      warning (OPT_Warray_bounds, "%Harray subscript is below array bounds",
+               locus);
+      TREE_NO_WARNING (ref) = 1;
+    }
+}
+
+/* walk_tree() callback that checks if *TP is
+   an ARRAY_REF inside an ADDR_EXPR (in which an array
+   subscript one outside the valid range is allowed). Call
+   check_array_ref for each ARRAY_REF found. The location is 
+   passed in DATA.  */
+
+static tree
+check_array_bounds (tree *tp, int *walk_subtree, void *data)
+{
+  tree t = *tp;
+  location_t *location = EXPR_LOCUS ((tree) data);
+
+  *walk_subtree = TRUE;
+
+  if (TREE_CODE (t) == ARRAY_REF)
+    check_array_ref (t, location, false /*ignore_off_by_one*/);
+  else if (TREE_CODE (t) == ADDR_EXPR)
+    {
+       t = TREE_OPERAND (t, 0);
+
+       /* Don't warn on statements like
+          ssa_name = 500 + &array[-200] which are sometimes
+          produced by various optimizing passes.  */
+       if (TREE_CODE ((tree)data) == GIMPLE_MODIFY_STMT
+           && BINARY_CLASS_P (GIMPLE_STMT_OPERAND ((tree)data, 1)))
+         {
+           *walk_subtree = FALSE;
+           return NULL_TREE;
+         }
+       while (handled_component_p (t))
+         {
+           if (TREE_CODE (t) == ARRAY_REF)
+             check_array_ref (t, location, true /*ignore_off_by_one*/);
+           t = TREE_OPERAND (t, 0);
+         }
+       *walk_subtree = FALSE;
+    }
+
+  return NULL_TREE;
+}
+
+/* Walk over all statements of all reachable BBs and call check_array_bounds
+   on them.  */
+
+static void
+check_all_array_refs (void)
+{
+  basic_block bb;
+  block_stmt_iterator si;
+
+  FOR_EACH_BB (bb)
+    {
+      /* Skip bb's that are clearly unreachable.  */
+      if (single_pred_p (bb))
+      {
+	basic_block pred_bb = EDGE_PRED (bb, 0)->src;
+	tree ls = NULL_TREE;
+
+	if (!bsi_end_p (bsi_last (pred_bb)))
+	  ls = bsi_stmt (bsi_last (pred_bb));
+
+	if (ls && TREE_CODE (ls) == COND_EXPR
+	    && ((COND_EXPR_COND (ls) == boolean_false_node
+		 && (EDGE_PRED (bb, 0)->flags & EDGE_TRUE_VALUE))
+		|| (COND_EXPR_COND (ls) == boolean_true_node
+		    && (EDGE_PRED (bb, 0)->flags & EDGE_FALSE_VALUE))))
+	  continue;
+      }
+      for (si = bsi_start (bb); !bsi_end_p (si); bsi_next (&si))
+	walk_tree (bsi_stmt_ptr (si), check_array_bounds,
+		   bsi_stmt (si), NULL);
+    }
+}
 
 /* Convert range assertion expressions into the implied copies and
    copy propagate away the copies.  Doing the trivial copy propagation
@@ -4797,6 +4945,9 @@ vrp_finalize (void)
 
   substitute_and_fold (single_val_range, true);
 
+  if (warn_array_bounds)
+      check_all_array_refs();
+
   /* We must identify jump threading opportunities before we release
      the datastructures built by VRP.  */
   identify_jump_threads ();
--- testsuite/gcc.dg/Warray-bounds.c	(revision 0)
+++ testsuite/gcc.dg/Warray-bounds.c	(revision 0)
@@ -0,0 +1,94 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -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;
+
+    a[-1] = 0;             /* { dg-warning "array subscript" } */
+    a[ 0] = 0;
+    a[ 1] = 0;
+
+
+    a[ 9] = 0;
+    a[10] = 0;             /* { dg-warning "array subscript" } */
+    a[11] = 0;             /* { dg-warning "array subscript" } */
+    a[2 * n() - 11] = 0;    /* { dg-warning "array subscript" } */
+    a[2 * n() - 10] = 0;
+    a[2 * n() -  1] = 0;
+    a[2 * n() -  0] = 0;    /* { dg-warning "array subscript" } */
+
+    b[-1] = 0;             /* { dg-warning "array subscript" } */
+    b[ 0] = 0;
+    b[ 1] = 0;
+    b[ 9] = 0;
+    b[10] = 0;             /* { dg-warning "array subscript" } */
+    b[11] = 0;             /* { dg-warning "array subscript" } */
+    b[2 * n() - 11] = 0;    /* { dg-warning "array subscript" } */
+    b[2 * n() - 10] = 0;
+    b[2 * n() -  1] = 0;
+    b[2 * n() -  0] = 0;    /* { dg-warning "array subscript" } */
+
+    c.c[-1] = 0;           /* { dg-warning "array subscript" } */
+    c.c[ 0] = 0;
+    c.c[ 1] = 0;
+    c.c[ 9] = 0;
+    c.c[10] = 0;           /* { dg-warning "array subscript" } */
+    c.c[11] = 0;           /* { dg-warning "array subscript" } */
+    c.c[2 * n() - 11] = 0;  /* { dg-warning "array subscript" } */
+    c.c[2 * n() - 10] = 0;
+    c.c[2 * n() -  1] = 0;
+    c.c[2 * n() -  0] = 0;  /* { dg-warning "array subscript" } */
+
+    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);
+
+    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" } */
+
+    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]);
+
+    if (10 < 10)
+       a[10] = 0;
+    if (10 < 10)
+       b[10] = 0;
+    if (-1 >= 0)
+       c.c[-1] = 0;
+
+    for (i = 20; i < 30; ++i)
+             a[i] = 1;       /* { dg-warning "array subscript" } */
+
+    return a;
+}
+


Index: testsuite/gcc.dg/Warray-bounds-2.c
===================================================================
--- testsuite/gcc.dg/Warray-bounds-2.c	(revision 0)
+++ testsuite/gcc.dg/Warray-bounds-2.c	(revision 0)
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* Test that -Warray-bounds is enabled by -Wall */
+/* { dg-options "-O2 -Wall" } */
+
+int a[10];
+
+int* f(void) {
+
+    a[-1] = 0;             /* { dg-warning "array subscript" } */
+
+    return a;
+}
+
Index: testsuite/g++.dg/warn/Warray-bounds.C
===================================================================
--- testsuite/g++.dg/warn/Warray-bounds.C	(revision 0)
+++ testsuite/g++.dg/warn/Warray-bounds.C	(revision 0)
@@ -0,0 +1,91 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -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;
+
+    a[-1] = 0;             /* { dg-warning "array subscript" } */
+    a[ 0] = 0;
+    a[ 1] = 0;
+
+
+    a[ 9] = 0;
+    a[10] = 0;             /* { dg-warning "array subscript" } */
+    a[11] = 0;             /* { dg-warning "array subscript" } */
+    a[2 * n() - 11] = 0;    /* { dg-warning "array subscript" } */
+    a[2 * n() - 10] = 0;
+    a[2 * n() -  1] = 0;
+    a[2 * n() -  0] = 0;    /* { dg-warning "array subscript" } */
+
+    b[-1] = 0;             /* { dg-warning "array subscript" } */
+    b[ 0] = 0;
+    b[ 1] = 0;
+    b[ 9] = 0;
+    b[10] = 0;             /* { dg-warning "array subscript" } */
+    b[11] = 0;             /* { dg-warning "array subscript" } */
+    b[2 * n() - 11] = 0;    /* { dg-warning "array subscript" } */
+    b[2 * n() - 10] = 0;
+    b[2 * n() -  1] = 0;
+    b[2 * n() -  0] = 0;    /* { dg-warning "array subscript" } */
+
+    c.c[-1] = 0;           /* { dg-warning "array subscript" } */
+    c.c[ 0] = 0;
+    c.c[ 1] = 0;
+    c.c[ 9] = 0;
+    c.c[10] = 0;           /* { dg-warning "array subscript" } */
+    c.c[11] = 0;           /* { dg-warning "array subscript" } */
+    c.c[2 * n() - 11] = 0;  /* { dg-warning "array subscript" } */
+    c.c[2 * n() - 10] = 0;
+    c.c[2 * n() -  1] = 0;
+    c.c[2 * n() -  0] = 0;  /* { dg-warning "array subscript" } */
+
+    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);
+
+    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" } */
+
+    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]);
+
+    if (10 < 10)
+       a[10] = 0;
+    if (10 < 10)
+       b[10] = 0;
+    if (-1 >= 0)
+       c.c[-1] = 0;
+
+    return a;
+}
+
Index: testsuite/g++.dg/warn/Warray-bounds-2.C
===================================================================
--- testsuite/g++.dg/warn/Warray-bounds-2.C	(revision 0)
+++ testsuite/g++.dg/warn/Warray-bounds-2.C	(revision 0)
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* Test that -Warray-bounds is enabled by -Wall */
+/* { dg-options "-O2 -Wall" } */
+
+int a[10];
+
+int* f(void) {
+
+    a[-1] = 0;             /* { dg-warning "array subscript" } */
+
+    return a;
+}
+

Re: [PATCH] Fix PR/8268: implement compile time array subscript checking

[Manuel López-Ibáñez]

On 16/01/07, Gabriel Dos Reis <gdr@cs.tamu.edu> wrote:
> On Tue, 16 Jan 2007, Manuel López-Ibáñez wrote:
>
> | Also, I am not sure whether you need testcases for "testing that is
> | enabled by -Wall" and "testing that is disabled with -Wall
> | -Wno-array-bounds".
>
> you're free to add one.
>

Gabriel, do you mean Dirk or I ? Because I cannot understand why
should I implement testcases for someone else's patch.

If my comments are a problem rather than a contribution, I'll avoid
commenting patches in the future. Please, let me know so I won't
pester anyone else.

I was just pointing out what *you* have requested *me* to implement
for new -W options previously [*].

Sincerely,

Manuel.

[*] http://gcc.gnu.org/ml/gcc-patches/2006-12/msg01483.html

Re: [PATCH] Fix PR/8268: implement compile time array subscript checking

[Gabriel Dos Reis]

On Tue, 16 Jan 2007, Dirk Mueller wrote:

| On Tuesday, 16. January 2007 16:52, Gabriel Dos Reis wrote:
|
| > | Also, I am not sure whether you need testcases for "testing that is
| > | enabled by -Wall" and "testing that is disabled with -Wall
| > | -Wno-array-bounds".
| > you're free to add one.
|
| Ok, I've added testcases that check that the functionality is enabled for C
| and for C++.
|
| Attached for reference, regtest in progress. Is the patch OK if it passes?

That is ok with me; assume Roger is already happy with it.

-- Gaby

Re: [PATCH] Fix PR/8268: implement compile time array subscript checking

[Roger Sayle]

Hi Dirk,

On Tue, January 16, 2007 9:08 am, Dirk Mueller wrote:
> Attached for reference, regtest in progress. Is the patch OK if it passes?

Sure, this is OK for mainline.  Many thanks for addressing the issues, I
appreciate that it can be tedious jumping through all of the required
hoops.  If other recent warning patches are anything to go by, I suspect
there'll be enough feedback to decide whether we need to tweak the
conditions under which issue warnings (-Wall vs. default vs -O1 etc..)

Many thanks again to you and Richard for tackling this.

Roger
--

Re: [PATCH] Fix PR/8268: implement compile time array subscript checking

[Gabriel Dos Reis]

On Tue, 16 Jan 2007, Manuel López-Ibáñez wrote:

| On 16/01/07, Gabriel Dos Reis <gdr@cs.tamu.edu> wrote:
| > On Tue, 16 Jan 2007, Manuel López-Ibáñez wrote:
| >
| > | Also, I am not sure whether you need testcases for "testing that is
| > | enabled by -Wall" and "testing that is disabled with -Wall
| > | -Wno-array-bounds".
| >
| > you're free to add one.
| >
|
| Gabriel, do you mean Dirk or I ?

it was a neutral "you", but I'm sure Dirk would not mind help from
free hands.

-- Gaby