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


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

[PATCH] Improve array bound warnings, fix PR43270


This fixes flexible array member detection in VRP (well, somewhat).
Mainly it avoids it for non-indirect reference bases and fixes
the detection for zero-sized arrays.

That causes some fallout during bootstrap - we warn for dead code
as in the added gcc.dg/Warray-bounds-7.c, because VN doesn't properly
handle reads from constant strings.  I fixed that.

Then we come along expmed.c:init_expmed ... ugh.  Completely bogus stuff.
So I added -Wno-error.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Richard.

2010-04-07  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/43270
	* tree-vrp.c (check_array_ref): Fix flexible array member
	detection.
	* tree-ssa-sccvn.h (fully_constant_vn_reference_p): Declare.
	* tree-ssa-pre.c (phi_translate_1): Adjust.
	(fully_constant_expression): Split out vn_reference handling to ...
	* tree-ssa-sccvn.c (fully_constant_vn_reference_p): ... here.
	Fold reads from constant strings.
	(vn_reference_lookup): Handle fully constant references.
	(vn_reference_lookup_pieces): Likewise.
	* Makefile.in (expmed.o-warn): Add -Wno-error.

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

Index: gcc/tree-vrp.c
===================================================================
*** gcc/tree-vrp.c.orig	2010-04-07 11:18:11.000000000 +0200
--- gcc/tree-vrp.c	2010-04-07 12:58:56.000000000 +0200
*************** check_array_ref (location_t location, tr
*** 4998,5020 ****
  {
    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 || 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)
      {
--- 4998,5043 ----
  {
    value_range_t* vr = NULL;
    tree low_sub, up_sub;
!   tree low_bound, up_bound, up_bound_p1;
!   tree base;
! 
!   if (TREE_NO_WARNING (ref))
!     return;
  
    low_sub = up_sub = TREE_OPERAND (ref, 1);
+   up_bound = array_ref_up_bound (ref);
  
!   /* Can not check flexible arrays.  */
!   if (!up_bound
!       || TREE_CODE (up_bound) != INTEGER_CST)
      return;
  
+   /* Accesses to trailing arrays via pointers may access storage
+      beyond the types array bounds.  */
+   base = get_base_address (ref);
+   if (base
+       && INDIRECT_REF_P (base))
+     {
+       tree cref, next = NULL_TREE;
+ 
+       if (TREE_CODE (TREE_OPERAND (ref, 0)) != COMPONENT_REF)
+ 	return;
+ 
+       cref = TREE_OPERAND (ref, 0);
+       if (TREE_CODE (TREE_TYPE (TREE_OPERAND (cref, 0))) == RECORD_TYPE)
+ 	for (next = TREE_CHAIN (TREE_OPERAND (cref, 1));
+ 	     next && TREE_CODE (next) != FIELD_DECL;
+ 	     next = TREE_CHAIN (next))
+ 	  ;
+ 
+       /* If this is the last field in a struct type or a field in a
+ 	 union type do not warn.  */
+       if (!next)
+ 	return;
+     }
+ 
    low_bound = array_ref_low_bound (ref);
+   up_bound_p1 = int_const_binop (PLUS_EXPR, up_bound, integer_one_node, 0);
  
    if (TREE_CODE (low_sub) == SSA_NAME)
      {
*************** check_array_ref (location_t location, tr
*** 5039,5052 ****
          }
      }
    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_at (location, OPT_Warray_bounds,
  		  "array subscript is above array bounds");
--- 5062,5072 ----
          }
      }
    else if (TREE_CODE (up_sub) == INTEGER_CST
! 	   && (ignore_off_by_one
! 	       ? (tree_int_cst_lt (up_bound, up_sub)
! 		  && !tree_int_cst_equal (up_bound_p1, up_sub))
! 	       : (tree_int_cst_lt (up_bound, up_sub)
! 		  || tree_int_cst_equal (up_bound_p1, up_sub))))
      {
        warning_at (location, OPT_Warray_bounds,
  		  "array subscript is above array bounds");
Index: gcc/testsuite/g++.dg/warn/Warray-bounds-4.C
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/g++.dg/warn/Warray-bounds-4.C	2010-04-07 12:58:56.000000000 +0200
***************
*** 0 ****
--- 1,37 ----
+ // { dg-do compile }
+ // { dg-options "-O2 -Warray-bounds" }
+ 
+ class String
+ {
+ public:
+   virtual unsigned long length() const = 0;
+   virtual char get(unsigned long index) const = 0;
+   virtual void set(unsigned long index, char value) = 0;
+   virtual char& operator[] (unsigned long value) = 0;
+   virtual ~String() {};
+ };
+ 
+ template<unsigned long size> class FixedString : public String
+ {
+ private:
+   char contents[size];
+ 
+ public:
+   virtual unsigned long length() const { return size; }
+   virtual char get(unsigned long index) const { return contents[index]; }
+   virtual void set(unsigned long index, char value) { contents[index] = value; }
+   virtual char& operator[] (unsigned long index) { return contents[index]; }
+ 
+   FixedString() { contents[0] = '\0'; } // { dg-warning "above array bounds" }
+ };
+ 
+ void print_length (const String& string);
+ 
+ int main()
+ {
+   const FixedString<0> empty;
+ 
+   print_length(empty);
+ 
+   return 0;
+ }
Index: gcc/tree-ssa-sccvn.h
===================================================================
*** gcc/tree-ssa-sccvn.h.orig	2010-04-07 11:18:11.000000000 +0200
--- gcc/tree-ssa-sccvn.h	2010-04-07 12:58:56.000000000 +0200
*************** unsigned int get_next_value_id (void);
*** 204,207 ****
--- 204,208 ----
  unsigned int get_constant_value_id (tree);
  unsigned int get_or_alloc_constant_value_id (tree);
  bool value_id_constant_p (unsigned int);
+ tree fully_constant_vn_reference_p (vn_reference_t);
  #endif /* TREE_SSA_SCCVN_H  */
Index: gcc/tree-ssa-pre.c
===================================================================
*** gcc/tree-ssa-pre.c.orig	2010-04-07 11:18:11.000000000 +0200
--- gcc/tree-ssa-pre.c	2010-04-07 13:11:57.000000000 +0200
*************** do_unary:
*** 1231,1279 ****
      case REFERENCE:
        {
  	vn_reference_t ref = PRE_EXPR_REFERENCE (e);
! 	VEC (vn_reference_op_s, heap) *operands = ref->operands;
! 	vn_reference_op_t op;
! 
! 	/* Try to simplify the translated expression if it is
! 	   a call to a builtin function with at most two arguments.  */
! 	op = VEC_index (vn_reference_op_s, operands, 0);
! 	if (op->opcode == CALL_EXPR
! 	    && TREE_CODE (op->op0) == ADDR_EXPR
! 	    && TREE_CODE (TREE_OPERAND (op->op0, 0)) == FUNCTION_DECL
! 	    && DECL_BUILT_IN (TREE_OPERAND (op->op0, 0))
! 	    && VEC_length (vn_reference_op_s, operands) >= 2
! 	    && VEC_length (vn_reference_op_s, operands) <= 3)
! 	  {
! 	    vn_reference_op_t arg0, arg1 = NULL;
! 	    bool anyconst = false;
! 	    arg0 = VEC_index (vn_reference_op_s, operands, 1);
! 	    if (VEC_length (vn_reference_op_s, operands) > 2)
! 	      arg1 = VEC_index (vn_reference_op_s, operands, 2);
! 	    if (TREE_CODE_CLASS (arg0->opcode) == tcc_constant
! 		|| (arg0->opcode == ADDR_EXPR
! 		    && is_gimple_min_invariant (arg0->op0)))
! 	      anyconst = true;
! 	    if (arg1
! 		&& (TREE_CODE_CLASS (arg1->opcode) == tcc_constant
! 		    || (arg1->opcode == ADDR_EXPR
! 			&& is_gimple_min_invariant (arg1->op0))))
! 	      anyconst = true;
! 	    if (anyconst)
! 	      {
! 		tree folded = build_call_expr (TREE_OPERAND (op->op0, 0),
! 					       arg1 ? 2 : 1,
! 					       arg0->op0,
! 					       arg1 ? arg1->op0 : NULL);
! 		if (folded
! 		    && TREE_CODE (folded) == NOP_EXPR)
! 		  folded = TREE_OPERAND (folded, 0);
! 		if (folded
! 		    && is_gimple_min_invariant (folded))
! 		  return get_or_alloc_expr_for_constant (folded);
! 	      }
! 	  }
! 	  return e;
! 	}
      default:
        return e;
      }
--- 1231,1241 ----
      case REFERENCE:
        {
  	vn_reference_t ref = PRE_EXPR_REFERENCE (e);
! 	tree folded;
! 	if ((folded = fully_constant_vn_reference_p (ref)))
! 	  return get_or_alloc_expr_for_constant (folded);
! 	return e;
!       }
      default:
        return e;
      }
*************** phi_translate_1 (pre_expr expr, bitmap_s
*** 1702,1708 ****
  						      ref->type,
  						      newoperands,
  						      &newref, true);
! 	    if (newref)
  	      VEC_free (vn_reference_op_s, heap, newoperands);
  
  	    if (result && is_gimple_min_invariant (result))
--- 1664,1670 ----
  						      ref->type,
  						      newoperands,
  						      &newref, true);
! 	    if (result)
  	      VEC_free (vn_reference_op_s, heap, newoperands);
  
  	    if (result && is_gimple_min_invariant (result))
Index: gcc/tree-ssa-sccvn.c
===================================================================
*** gcc/tree-ssa-sccvn.c.orig	2010-04-07 11:18:11.000000000 +0200
--- gcc/tree-ssa-sccvn.c	2010-04-07 13:08:04.000000000 +0200
*************** vn_reference_fold_indirect (VEC (vn_refe
*** 889,894 ****
--- 889,964 ----
    *i_p = i;
  }
  
+ /* Optimize the reference REF to a constant if possible or return
+    NULL_TREE if not.  */
+ 
+ tree
+ fully_constant_vn_reference_p (vn_reference_t ref)
+ {
+   VEC (vn_reference_op_s, heap) *operands = ref->operands;
+   vn_reference_op_t op;
+ 
+   /* Try to simplify the translated expression if it is
+      a call to a builtin function with at most two arguments.  */
+   op = VEC_index (vn_reference_op_s, operands, 0);
+   if (op->opcode == CALL_EXPR
+       && TREE_CODE (op->op0) == ADDR_EXPR
+       && TREE_CODE (TREE_OPERAND (op->op0, 0)) == FUNCTION_DECL
+       && DECL_BUILT_IN (TREE_OPERAND (op->op0, 0))
+       && VEC_length (vn_reference_op_s, operands) >= 2
+       && VEC_length (vn_reference_op_s, operands) <= 3)
+     {
+       vn_reference_op_t arg0, arg1 = NULL;
+       bool anyconst = false;
+       arg0 = VEC_index (vn_reference_op_s, operands, 1);
+       if (VEC_length (vn_reference_op_s, operands) > 2)
+ 	arg1 = VEC_index (vn_reference_op_s, operands, 2);
+       if (TREE_CODE_CLASS (arg0->opcode) == tcc_constant
+ 	  || (arg0->opcode == ADDR_EXPR
+ 	      && is_gimple_min_invariant (arg0->op0)))
+ 	anyconst = true;
+       if (arg1
+ 	  && (TREE_CODE_CLASS (arg1->opcode) == tcc_constant
+ 	      || (arg1->opcode == ADDR_EXPR
+ 		  && is_gimple_min_invariant (arg1->op0))))
+ 	anyconst = true;
+       if (anyconst)
+ 	{
+ 	  tree folded = build_call_expr (TREE_OPERAND (op->op0, 0),
+ 					 arg1 ? 2 : 1,
+ 					 arg0->op0,
+ 					 arg1 ? arg1->op0 : NULL);
+ 	  if (folded
+ 	      && TREE_CODE (folded) == NOP_EXPR)
+ 	    folded = TREE_OPERAND (folded, 0);
+ 	  if (folded
+ 	      && is_gimple_min_invariant (folded))
+ 	    return folded;
+ 	}
+     }
+ 
+   /* Simplify reads from constant strings.  */
+   else if (op->opcode == ARRAY_REF
+ 	   && TREE_CODE (op->op0) == INTEGER_CST
+ 	   && integer_zerop (op->op1)
+ 	   && VEC_length (vn_reference_op_s, operands) == 2)
+     {
+       vn_reference_op_t arg0;
+       arg0 = VEC_index (vn_reference_op_s, operands, 1);
+       if (arg0->opcode == STRING_CST
+ 	  && (TYPE_MODE (op->type)
+ 	      == TYPE_MODE (TREE_TYPE (TREE_TYPE (arg0->op0))))
+ 	  && GET_MODE_CLASS (TYPE_MODE (op->type)) == MODE_INT
+ 	  && GET_MODE_SIZE (TYPE_MODE (op->type)) == 1
+ 	  && compare_tree_int (op->op0, TREE_STRING_LENGTH (arg0->op0)) < 0)
+ 	return build_int_cst_type (op->type,
+ 				   (TREE_STRING_POINTER (arg0->op0)
+ 				    [TREE_INT_CST_LOW (op->op0)]));
+     }
+ 
+   return NULL_TREE;
+ }
+ 
  /* Transform any SSA_NAME's in a vector of vn_reference_op_s
     structures into their value numbers.  This is done in-place, and
     the vector passed in is returned.  */
*************** vn_reference_lookup_pieces (tree vuse, a
*** 1194,1199 ****
--- 1264,1270 ----
  {
    struct vn_reference_s vr1;
    vn_reference_t tmp;
+   tree cst;
  
    if (!vnresult)
      vnresult = &tmp;
*************** vn_reference_lookup_pieces (tree vuse, a
*** 1212,1219 ****
    vr1.type = type;
    vr1.set = set;
    vr1.hashcode = vn_reference_compute_hash (&vr1);
!   vn_reference_lookup_1 (&vr1, vnresult);
  
    if (!*vnresult
        && maywalk
        && vr1.vuse)
--- 1283,1292 ----
    vr1.type = type;
    vr1.set = set;
    vr1.hashcode = vn_reference_compute_hash (&vr1);
!   if ((cst = fully_constant_vn_reference_p (&vr1)))
!     return cst;
  
+   vn_reference_lookup_1 (&vr1, vnresult);
    if (!*vnresult
        && maywalk
        && vr1.vuse)
*************** vn_reference_lookup (tree op, tree vuse,
*** 1246,1251 ****
--- 1319,1325 ----
  {
    VEC (vn_reference_op_s, heap) *operands;
    struct vn_reference_s vr1;
+   tree cst;
  
    if (vnresult)
      *vnresult = NULL;
*************** vn_reference_lookup (tree op, tree vuse,
*** 1255,1260 ****
--- 1329,1336 ----
    vr1.type = TREE_TYPE (op);
    vr1.set = get_alias_set (op);
    vr1.hashcode = vn_reference_compute_hash (&vr1);
+   if ((cst = fully_constant_vn_reference_p (&vr1)))
+     return cst;
  
    if (maywalk
        && vr1.vuse)
Index: gcc/testsuite/gcc.dg/Warray-bounds-7.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.dg/Warray-bounds-7.c	2010-04-07 12:58:56.000000000 +0200
***************
*** 0 ****
--- 1,13 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O2 -Warray-bounds" } */
+ 
+ char *p;
+ 
+ int main()
+ {
+   p = "";
+   if (p[0] == 0
+       || (p[0] == '_' && p[1] == 0))  /* { dg-bogus "array bounds" "" } */
+     return 0;
+   return 1;
+ }
Index: gcc/Makefile.in
===================================================================
*** gcc/Makefile.in.orig	2010-04-07 11:18:11.000000000 +0200
--- gcc/Makefile.in	2010-04-07 12:58:56.000000000 +0200
*************** GCC_WARN_CXXFLAGS = $(LOOSE_WARN) $($(@D
*** 183,188 ****
--- 183,189 ----
  build/gengtype-lex.o-warn = -Wno-error
  # mips-tfile.c contains -Wcast-qual warnings.
  mips-tfile.o-warn = -Wno-error
+ expmed.o-warn = -Wno-error
  
  # All warnings have to be shut off in stage1 if the compiler used then
  # isn't gcc; configure determines that.  WARN_CFLAGS will be either


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