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] Fix PR41317, wrong code with folding conversions


We are currently folding (T *) ptr into all sorts of addresses of
component accesses of *ptr.  This is bogus as propagating these
addresses into dereferences can expose accesses that were not in
the original program and that violate strict aliasing rules.

Bootstrapped and tested on x86_64-unknown-linux-gnu, I'll install
this momentarily.

Richard.

2009-09-09  Richard Guenther  <rguenther@suse.de>

	PR middle-end/41317
	* tree-ssa-ccp.c (maybe_fold_offset_to_component_ref): Remove
	code dealing with plain pointer bases.
	(maybe_fold_offset_to_reference): Likewise.
	(maybe_fold_stmt_addition): Adjust.

	* gcc.c-torture/execute/pr41317.c: New testcase.
	* gcc.dg/tree-ssa/forwprop-11.c: XFAIL.
	* gcc.dg/tree-ssa/forwprop-12.c: Likewise.

Index: gcc/tree-ssa-ccp.c
===================================================================
*** gcc/tree-ssa-ccp.c.orig	2009-09-09 15:51:00.000000000 +0200
--- gcc/tree-ssa-ccp.c	2009-09-09 15:55:46.000000000 +0200
*************** maybe_fold_offset_to_array_ref (location
*** 1818,1825 ****
  
  static tree
  maybe_fold_offset_to_component_ref (location_t loc, tree record_type,
! 				    tree base, tree offset,
! 				    tree orig_type, bool base_is_ptr)
  {
    tree f, t, field_type, tail_array_field, field_offset;
    tree ret;
--- 1818,1824 ----
  
  static tree
  maybe_fold_offset_to_component_ref (location_t loc, tree record_type,
! 				    tree base, tree offset, tree orig_type)
  {
    tree f, t, field_type, tail_array_field, field_offset;
    tree ret;
*************** maybe_fold_offset_to_component_ref (loca
*** 1871,1878 ****
        if (cmp == 0
  	  && useless_type_conversion_p (orig_type, field_type))
  	{
- 	  if (base_is_ptr)
- 	    base = build1 (INDIRECT_REF, record_type, base);
  	  t = build3 (COMPONENT_REF, field_type, base, f, NULL_TREE);
  	  return t;
  	}
--- 1870,1875 ----
*************** maybe_fold_offset_to_component_ref (loca
*** 1897,1909 ****
  
        /* If we matched, then set offset to the displacement into
  	 this field.  */
!       if (base_is_ptr)
! 	new_base = build1 (INDIRECT_REF, record_type, base);
!       else
! 	new_base = base;
!       protected_set_expr_location (new_base, loc);
!       new_base = build3 (COMPONENT_REF, field_type, new_base, f, NULL_TREE);
!       protected_set_expr_location (new_base, loc);
  
        /* Recurse to possibly find the match.  */
        ret = maybe_fold_offset_to_array_ref (loc, new_base, t, orig_type,
--- 1894,1901 ----
  
        /* If we matched, then set offset to the displacement into
  	 this field.  */
!       new_base = build3 (COMPONENT_REF, field_type, base, f, NULL_TREE);
!       SET_EXPR_LOCATION (new_base, loc);
  
        /* Recurse to possibly find the match.  */
        ret = maybe_fold_offset_to_array_ref (loc, new_base, t, orig_type,
*************** maybe_fold_offset_to_component_ref (loca
*** 1911,1917 ****
        if (ret)
  	return ret;
        ret = maybe_fold_offset_to_component_ref (loc, field_type, new_base, t,
! 						orig_type, false);
        if (ret)
  	return ret;
      }
--- 1903,1909 ----
        if (ret)
  	return ret;
        ret = maybe_fold_offset_to_component_ref (loc, field_type, new_base, t,
! 						orig_type);
        if (ret)
  	return ret;
      }
*************** maybe_fold_offset_to_component_ref (loca
*** 1925,1935 ****
  
    /* If we get here, we've got an aggregate field, and a possibly 
       nonzero offset into them.  Recurse and hope for a valid match.  */
-   if (base_is_ptr)
-     {
-       base = build1 (INDIRECT_REF, record_type, base);
-       SET_EXPR_LOCATION (base, loc);
-     }
    base = build3 (COMPONENT_REF, field_type, base, f, NULL_TREE);
    SET_EXPR_LOCATION (base, loc);
  
--- 1917,1922 ----
*************** maybe_fold_offset_to_component_ref (loca
*** 1938,1944 ****
    if (t)
      return t;
    return maybe_fold_offset_to_component_ref (loc, field_type, base, offset,
! 					     orig_type, false);
  }
  
  /* Attempt to express (ORIG_TYPE)BASE+OFFSET as BASE->field_of_orig_type
--- 1925,1931 ----
    if (t)
      return t;
    return maybe_fold_offset_to_component_ref (loc, field_type, base, offset,
! 					     orig_type);
  }
  
  /* Attempt to express (ORIG_TYPE)BASE+OFFSET as BASE->field_of_orig_type
*************** maybe_fold_offset_to_reference (location
*** 1955,2015 ****
  {
    tree ret;
    tree type;
-   bool base_is_ptr = true;
  
    STRIP_NOPS (base);
!   if (TREE_CODE (base) == ADDR_EXPR)
!     {
!       base_is_ptr = false;
  
!       base = TREE_OPERAND (base, 0);
  
!       /* Handle case where existing COMPONENT_REF pick e.g. wrong field of union,
! 	 so it needs to be removed and new COMPONENT_REF constructed.
! 	 The wrong COMPONENT_REF are often constructed by folding the
! 	 (type *)&object within the expression (type *)&object+offset  */
!       if (handled_component_p (base))
  	{
!           HOST_WIDE_INT sub_offset, size, maxsize;
! 	  tree newbase;
! 	  newbase = get_ref_base_and_extent (base, &sub_offset,
! 					     &size, &maxsize);
! 	  gcc_assert (newbase);
! 	  if (size == maxsize
! 	      && size != -1
! 	      && !(sub_offset & (BITS_PER_UNIT - 1)))
! 	    {
! 	      base = newbase;
! 	      if (sub_offset)
! 		offset = int_const_binop (PLUS_EXPR, offset,
! 					  build_int_cst (TREE_TYPE (offset),
! 					  sub_offset / BITS_PER_UNIT), 1);
! 	    }
  	}
-       if (useless_type_conversion_p (orig_type, TREE_TYPE (base))
- 	  && integer_zerop (offset))
- 	return base;
-       type = TREE_TYPE (base);
-     }
-   else
-     {
-       base_is_ptr = true;
-       if (!POINTER_TYPE_P (TREE_TYPE (base)))
- 	return NULL_TREE;
-       type = TREE_TYPE (TREE_TYPE (base));
      }
!   ret = maybe_fold_offset_to_component_ref (loc, type, base, offset,
! 					    orig_type, base_is_ptr);
    if (!ret)
!     {
!       if (base_is_ptr)
! 	{
! 	  base = build1 (INDIRECT_REF, type, base);
! 	  SET_EXPR_LOCATION (base, loc);
! 	}
!       ret = maybe_fold_offset_to_array_ref (loc,
! 					    base, offset, orig_type, true);
!     }
    return ret;
  }
  
--- 1942,1985 ----
  {
    tree ret;
    tree type;
  
    STRIP_NOPS (base);
!   if (TREE_CODE (base) != ADDR_EXPR)
!     return NULL_TREE;
  
!   base = TREE_OPERAND (base, 0);
  
!   /* Handle case where existing COMPONENT_REF pick e.g. wrong field of union,
!      so it needs to be removed and new COMPONENT_REF constructed.
!      The wrong COMPONENT_REF are often constructed by folding the
!      (type *)&object within the expression (type *)&object+offset  */
!   if (handled_component_p (base))
!     {
!       HOST_WIDE_INT sub_offset, size, maxsize;
!       tree newbase;
!       newbase = get_ref_base_and_extent (base, &sub_offset,
! 					 &size, &maxsize);
!       gcc_assert (newbase);
!       if (size == maxsize
! 	  && size != -1
! 	  && !(sub_offset & (BITS_PER_UNIT - 1)))
  	{
! 	  base = newbase;
! 	  if (sub_offset)
! 	    offset = int_const_binop (PLUS_EXPR, offset,
! 				      build_int_cst (TREE_TYPE (offset),
! 						     sub_offset / BITS_PER_UNIT), 1);
  	}
      }
!   if (useless_type_conversion_p (orig_type, TREE_TYPE (base))
!       && integer_zerop (offset))
!     return base;
!   type = TREE_TYPE (base);
! 
!   ret = maybe_fold_offset_to_component_ref (loc, type, base, offset, orig_type);
    if (!ret)
!     ret = maybe_fold_offset_to_array_ref (loc, base, offset, orig_type, true);
! 
    return ret;
  }
  
*************** maybe_fold_stmt_addition (location_t loc
*** 2286,2292 ****
    t = maybe_fold_offset_to_array_ref (loc, op0, op1, ptd_type, true);
    if (!t)
      t = maybe_fold_offset_to_component_ref (loc, TREE_TYPE (op0), op0, op1,
! 					    ptd_type, false);
    if (t)
      {
        t = build1 (ADDR_EXPR, res_type, t);
--- 2256,2262 ----
    t = maybe_fold_offset_to_array_ref (loc, op0, op1, ptd_type, true);
    if (!t)
      t = maybe_fold_offset_to_component_ref (loc, TREE_TYPE (op0), op0, op1,
! 					    ptd_type);
    if (t)
      {
        t = build1 (ADDR_EXPR, res_type, t);
Index: gcc/testsuite/gcc.c-torture/execute/pr41317.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.c-torture/execute/pr41317.c	2009-09-09 15:51:11.000000000 +0200
***************
*** 0 ****
--- 1,28 ----
+ extern void abort (void);
+ 
+ struct A
+ {
+   int i;
+ };
+ struct B
+ {
+   struct A a;
+   int j;
+ };
+ 
+ static void
+ foo (struct B *p)
+ {
+   ((struct A *)p)->i = 1;
+ }
+ 
+ int main()
+ {
+   struct A a;
+   a.i = 0;
+   foo ((struct B *)&a);
+   if (a.i != 1)
+     abort ();
+   return 0;
+ }
+ 
Index: gcc/testsuite/gcc.dg/tree-ssa/forwprop-11.c
===================================================================
*** gcc/testsuite/gcc.dg/tree-ssa/forwprop-11.c.orig	2009-03-30 10:32:05.000000000 +0200
--- gcc/testsuite/gcc.dg/tree-ssa/forwprop-11.c	2009-09-09 16:14:09.000000000 +0200
*************** int g(int *p, int n)
*** 15,19 ****
    return q[-1];
  }
  
! /* { dg-final { scan-tree-dump-times "= \\\(\\\*a_..\\\)\\\[1\\\];" 2 "forwprop1" } } */
  /* { dg-final { cleanup-tree-dump "forwprop1" } } */
--- 15,19 ----
    return q[-1];
  }
  
! /* { dg-final { scan-tree-dump-times "= \\\(\\\*a_..\\\)\\\[1\\\];" 2 "forwprop1" { xfail *-*-* } } } */
  /* { dg-final { cleanup-tree-dump "forwprop1" } } */
Index: gcc/testsuite/gcc.dg/tree-ssa/forwprop-12.c
===================================================================
*** gcc/testsuite/gcc.dg/tree-ssa/forwprop-12.c.orig	2009-03-31 15:14:47.000000000 +0200
--- gcc/testsuite/gcc.dg/tree-ssa/forwprop-12.c	2009-09-09 16:14:26.000000000 +0200
*************** int bar(struct X *p, int i)
*** 18,22 ****
  /* We should have propagated the base array address through the
     address arithmetic into the memory access as an array access.  */
  
! /* { dg-final { scan-tree-dump-times "->a\\\[D\\\." 2 "forwprop1" } } */
  /* { dg-final { cleanup-tree-dump "forwprop1" } } */
--- 18,22 ----
  /* We should have propagated the base array address through the
     address arithmetic into the memory access as an array access.  */
  
! /* { dg-final { scan-tree-dump-times "->a\\\[D\\\." 2 "forwprop1" { xfail *-*-* } } } */
  /* { dg-final { cleanup-tree-dump "forwprop1" } } */


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