[prev in list] [next in list] [prev in thread] [next in thread]
List: gcc-patches
Subject: Re: [PATCH] Fix PR/8268: implement compile time array subscript checking
From: Dirk Mueller <dmuell () gmx ! net>
Date: 2007-01-16 16:08:19
Message-ID: 200701161708.19584.dmuell () gmx ! net
[Download RAW message or body]
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
["array_bounds_vrp_6.patch" (text/x-diff)]
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;
+}
+
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic