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 PR36666



This fixes PR36666 which runs into the asser asserting that a PTA constraint assigning two struct cannot happen. While the fix is to remove this assert, this bug uncovers a problem with field-sensitive PTA in the way we handle parameters and returns of functions.

With const functions that return aggregates with multiple pointers
any of these pointer may point to anything that a parameter to this
const function points to (in addition to any global variable).

To model this the fix includes a change to the get_constraint_for
function to return all the constraint expressions the argument
references, not only the first one.  Thus, get_constraint_for gets
split and its helper get_constraint_for_1 takes an additional
parameter indicating whether the result will be taken its address from.

This fixes the associated wrong-PTA bug where for a const call and
field-sensitive PTA we didn't account for all of the pointed-to
memory of the return value.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.  I'll
commit this tomorrow if testing succeeds.

Thanks,
Richard.

2008-06-30 Richard Guenther <rguenther@suse.de>

	PR tree-optimization/36666
	* tree-ssa-structalias.c (get_constraint_for_1): Declare.
	(get_constraint_exp_from_ssa_var): Split into ...
	(get_constraint_exp_for_temp): ... this ...
	(get_constraint_for_ssa_var): ... and that.
	Return constraint expressions for all touched sub-fields
	if the results address is not taken.
	(process_constraint): Remove assertion that aggregate
	assignments do not happen at this place.
	(get_constraint_for_component_ref): Add address_p argument.
	Return constraint expressions for all touched sub-fields
	if the results address is not taken.
	(do_deref): Use get_constraint_exp_for_temp.
	(get_constraint_for_1): Rename from ...
	(get_constraint_for): ... this.  Add the old function as
	wrapper.
	(do_structure_copy): Use get_constraint_for_1.

* gcc.c-torture/compile/pr36666.c: New testcase.

Index: testsuite/gcc.c-torture/compile/pr36666.c
===================================================================
*** testsuite/gcc.c-torture/compile/pr36666.c (revision 0)
--- testsuite/gcc.c-torture/compile/pr36666.c (revision 0)
***************
*** 0 ****
--- 1,22 ----
+ struct Foo {
+ int *p;
+ struct X {
+ int a,b,c,d,e,*f;
+ } x;
+ } *init, *init2;
+ + struct X __attribute__((const)) foo(struct X);
+ struct Foo __attribute__((const)) foo2(struct Foo);
+ + void bar1 (void)
+ {
+ init->x = foo (init2->x);
+ }
+ void bar2 (void)
+ {
+ init->x = foo (init->x);
+ }
+ void bar3 (void)
+ {
+ *init = foo2 (*init2);
+ }
Index: tree-ssa-structalias.c
===================================================================
*** tree-ssa-structalias.c (revision 137271)
--- tree-ssa-structalias.c (working copy)
*************** struct constraint_expr
*** 412,417 ****
--- 412,418 ----
typedef struct constraint_expr ce_s;
DEF_VEC_O(ce_s);
DEF_VEC_ALLOC_O(ce_s, heap);
+ static void get_constraint_for_1 (tree, VEC(ce_s, heap) **, bool);
static void get_constraint_for (tree, VEC(ce_s, heap) **);
static void do_deref (VEC (ce_s, heap) **);


*************** get_vi_for_tree (tree t)
*** 2495,2507 ****
    return (varinfo_t) *slot;
  }

! /* Get a constraint expression from an SSA_VAR_P node. */

  static struct constraint_expr
! get_constraint_exp_from_ssa_var (tree t)
  {
    struct constraint_expr cexpr;

gcc_assert (SSA_VAR_P (t) || DECL_P (t));

    /* For parameters, get at the points-to set for the actual parm
--- 2496,2527 ----
    return (varinfo_t) *slot;
  }

! /* Get a constraint expression for a new temporary variable. */

  static struct constraint_expr
! get_constraint_exp_for_temp (tree t)
  {
    struct constraint_expr cexpr;

+ gcc_assert (SSA_VAR_P (t));
+ + cexpr.type = SCALAR;
+ cexpr.var = get_vi_for_tree (t)->id;
+ cexpr.offset = 0;
+ + return cexpr;
+ }
+ + /* Get a constraint expression vector from an SSA_VAR_P node.
+ If address_p is true, the result will be taken its address of. */
+ + static void
+ get_constraint_for_ssa_var (tree t, VEC(ce_s, heap) **results, bool address_p)
+ {
+ struct constraint_expr cexpr;
+ varinfo_t vi;
+ + /* We allow FUNCTION_DECLs here even though it doesn't make much sense. */
gcc_assert (SSA_VAR_P (t) || DECL_P (t));


    /* For parameters, get at the points-to set for the actual parm
*************** get_constraint_exp_from_ssa_var (tree t)
*** 2509,2529 ****
    if (TREE_CODE (t) == SSA_NAME
        && TREE_CODE (SSA_NAME_VAR (t)) == PARM_DECL
        && SSA_NAME_IS_DEFAULT_DEF (t))
!     return get_constraint_exp_from_ssa_var (SSA_NAME_VAR (t));

cexpr.type = SCALAR;
! ! cexpr.var = get_vi_for_tree (t)->id;
/* If we determine the result is "anything", and we know this is readonly,
say it points to readonly memory instead. */
if (cexpr.var == anything_id && TREE_READONLY (t))
{
cexpr.type = ADDRESSOF;
cexpr.var = readonly_id;
}


!   cexpr.offset = 0;
!   return cexpr;
  }

  /* Process constraint T, performing various simplifications and then
--- 2529,2565 ----
    if (TREE_CODE (t) == SSA_NAME
        && TREE_CODE (SSA_NAME_VAR (t)) == PARM_DECL
        && SSA_NAME_IS_DEFAULT_DEF (t))
!     {
!       get_constraint_for_ssa_var (SSA_NAME_VAR (t), results, address_p);
!       return;
!     }

+   vi = get_vi_for_tree (t);
+   cexpr.var = vi->id;
    cexpr.type = SCALAR;
!   cexpr.offset = 0;
    /* If we determine the result is "anything", and we know this is readonly,
       say it points to readonly memory instead.  */
    if (cexpr.var == anything_id && TREE_READONLY (t))
      {
+       gcc_unreachable ();
        cexpr.type = ADDRESSOF;
        cexpr.var = readonly_id;
      }

! /* If we are not taking the address of the constraint expr, add all
! sub-fiels of the variable as well. */
! if (!address_p)
! {
! for (; vi; vi = vi->next)
! {
! cexpr.var = vi->id;
! VEC_safe_push (ce_s, heap, *results, &cexpr);
! }
! return;
! }
! ! VEC_safe_push (ce_s, heap, *results, &cexpr);
}


/* Process constraint T, performing various simplifications and then
*************** process_constraint (constraint_t t)
*** 2564,2576 ****
tree pointertype = TREE_TYPE (rhsdecl);
tree pointedtotype = TREE_TYPE (pointertype);
tree tmpvar = create_tmp_var_raw (pointedtotype, "doubledereftmp");
! struct constraint_expr tmplhs = get_constraint_exp_from_ssa_var (tmpvar);
! ! /* If this is an aggregate of known size, we should have passed
! this off to do_structure_copy, and it should have broken it
! up. */
! gcc_assert (!AGGREGATE_TYPE_P (pointedtotype)
! || get_varinfo (rhs.var)->is_unknown_size_var);


        process_constraint (new_constraint (tmplhs, rhs));
        process_constraint (new_constraint (lhs, tmplhs));
--- 2600,2606 ----
        tree pointertype = TREE_TYPE (rhsdecl);
        tree pointedtotype = TREE_TYPE (pointertype);
        tree tmpvar = create_tmp_var_raw (pointedtotype, "doubledereftmp");
!       struct constraint_expr tmplhs = get_constraint_exp_for_temp (tmpvar);

        process_constraint (new_constraint (tmplhs, rhs));
        process_constraint (new_constraint (lhs, tmplhs));
*************** process_constraint (constraint_t t)
*** 2581,2587 ****
        tree rhsdecl = get_varinfo (rhs.var)->decl;
        tree pointertype = TREE_TYPE (rhsdecl);
        tree tmpvar = create_tmp_var_raw (pointertype, "derefaddrtmp");
!       struct constraint_expr tmplhs = get_constraint_exp_from_ssa_var (tmpvar);

        process_constraint (new_constraint (tmplhs, rhs));
        process_constraint (new_constraint (lhs, tmplhs));
--- 2611,2617 ----
        tree rhsdecl = get_varinfo (rhs.var)->decl;
        tree pointertype = TREE_TYPE (rhsdecl);
        tree tmpvar = create_tmp_var_raw (pointertype, "derefaddrtmp");
!       struct constraint_expr tmplhs = get_constraint_exp_for_temp (tmpvar);

        process_constraint (new_constraint (tmplhs, rhs));
        process_constraint (new_constraint (lhs, tmplhs));
*************** bitpos_of_field (const tree fdecl)
*** 2625,2634 ****
  }


! /* Given a COMPONENT_REF T, return the constraint_expr for it. */


  static void
! get_constraint_for_component_ref (tree t, VEC(ce_s, heap) **results)
  {
    tree orig_t = t;
    HOST_WIDE_INT bitsize = -1;
--- 2655,2666 ----
  }


! /* Given a COMPONENT_REF T, return the constraint_expr vector for it. ! If address_p is true the result will be taken its address of. */

  static void
! get_constraint_for_component_ref (tree t, VEC(ce_s, heap) **results,
! 				  bool address_p)
  {
    tree orig_t = t;
    HOST_WIDE_INT bitsize = -1;
*************** get_constraint_for_component_ref (tree t
*** 2636,2642 ****
    HOST_WIDE_INT bitpos;
    tree forzero;
    struct constraint_expr *result;
-   unsigned int beforelength = VEC_length (ce_s, *results);

    /* Some people like to do cute things like take the address of
       &0->a.b */
--- 2668,2673 ----
*************** get_constraint_for_component_ref (tree t
*** 2657,2667 ****

t = get_ref_base_and_extent (t, &bitpos, &bitsize, &bitmaxsize);

!   get_constraint_for (t, results);
    result = VEC_last (ce_s, *results);
-   result->offset = bitpos;

! gcc_assert (beforelength + 1 == VEC_length (ce_s, *results));

    /* This can also happen due to weird offsetof type macros.  */
    if (TREE_CODE (t) != ADDR_EXPR && result->type == ADDRESSOF)
--- 2688,2699 ----

t = get_ref_base_and_extent (t, &bitpos, &bitsize, &bitmaxsize);

!   /* Pretend to take the address of the base, we'll take care of
!      adding the required subset of sub-fields below.  */
!   get_constraint_for_1 (t, results, true);
    result = VEC_last (ce_s, *results);

! gcc_assert (VEC_length (ce_s, *results) == 1);

    /* This can also happen due to weird offsetof type macros.  */
    if (TREE_CODE (t) != ADDR_EXPR && result->type == ADDRESSOF)
*************** get_constraint_for_component_ref (tree t
*** 2674,2701 ****
  	 ignore this constraint. When we handle pointer subtraction,
  	 we may have to do something cute here.  */

!       if (result->offset < get_varinfo (result->var)->fullsize
  	  && bitmaxsize != 0)
  	{
  	  /* It's also not true that the constraint will actually start at the
  	     right offset, it may start in some padding.  We only care about
  	     setting the constraint to the first actual field it touches, so
  	     walk to find it.  */
  	  varinfo_t curr;
! 	  for (curr = get_varinfo (result->var); curr; curr = curr->next)
  	    {
  	      if (ranges_overlap_p (curr->offset, curr->size,
! 				    result->offset, bitmaxsize))
  		{
! 		  result->var = curr->id;
! 		  break;
  		}
  	    }
  	  /* assert that we found *some* field there. The user couldn't be
  	     accessing *only* padding.  */
  	  /* Still the user could access one past the end of an array
  	     embedded in a struct resulting in accessing *only* padding.  */
! 	  gcc_assert (curr || ref_contains_array_ref (orig_t));
  	}
        else if (bitmaxsize == 0)
  	{
--- 2706,2739 ----
  	 ignore this constraint. When we handle pointer subtraction,
  	 we may have to do something cute here.  */

! if ((unsigned HOST_WIDE_INT)bitpos < get_varinfo (result->var)->fullsize
&& bitmaxsize != 0)
{
/* It's also not true that the constraint will actually start at the
right offset, it may start in some padding. We only care about
setting the constraint to the first actual field it touches, so
walk to find it. */
+ struct constraint_expr cexpr = *result;
varinfo_t curr;
! VEC_pop (ce_s, *results);
! cexpr.offset = 0;
! for (curr = get_varinfo (cexpr.var); curr; curr = curr->next)
{
if (ranges_overlap_p (curr->offset, curr->size,
! bitpos, bitmaxsize))
{
! cexpr.var = curr->id;
! VEC_safe_push (ce_s, heap, *results, &cexpr);
! if (address_p)
! break;
}
}
/* assert that we found *some* field there. The user couldn't be
accessing *only* padding. */
/* Still the user could access one past the end of an array
embedded in a struct resulting in accessing *only* padding. */
! gcc_assert (VEC_length (ce_s, *results) >= 1
! || ref_contains_array_ref (orig_t));
}
else if (bitmaxsize == 0)
{
*************** get_constraint_for_component_ref (tree t
*** 2706,2713 ****
else
if (dump_file && (dump_flags & TDF_DETAILS))
fprintf (dump_file, "Access to past the end of variable, ignoring\n");
- - result->offset = 0;
}
else if (bitmaxsize == -1)
{
--- 2744,2749 ----
*************** get_constraint_for_component_ref (tree t
*** 2716,2721 ****
--- 2752,2759 ----
result->var = anything_id;
result->offset = 0;
}
+ else
+ result->offset = bitpos;
}



*************** do_deref (VEC (ce_s, heap) **constraints *** 2740,2746 **** else if (c->type == DEREF) { tree tmpvar = create_tmp_var_raw (ptr_type_node, "dereftmp"); ! struct constraint_expr tmplhs = get_constraint_exp_from_ssa_var (tmpvar); process_constraint (new_constraint (tmplhs, *c)); c->var = tmplhs.var; } --- 2778,2784 ---- else if (c->type == DEREF) { tree tmpvar = create_tmp_var_raw (ptr_type_node, "dereftmp"); ! struct constraint_expr tmplhs = get_constraint_exp_for_temp (tmpvar); process_constraint (new_constraint (tmplhs, *c)); c->var = tmplhs.var; } *************** do_deref (VEC (ce_s, heap) **constraints *** 2752,2758 **** /* Given a tree T, return the constraint expression for it. */

  static void
! get_constraint_for (tree t, VEC (ce_s, heap) **results)
  {
    struct constraint_expr temp;

--- 2790,2796 ----
  /* Given a tree T, return the constraint expression for it.  */

  static void
! get_constraint_for_1 (tree t, VEC (ce_s, heap) **results, bool address_p)
  {
    struct constraint_expr temp;

*************** get_constraint_for (tree t, VEC (ce_s, h
*** 2796,2827 ****
  	      struct constraint_expr *c;
  	      unsigned int i;
  	      tree exp = TREE_OPERAND (t, 0);
- 	      tree pttype = TREE_TYPE (TREE_TYPE (t));

! get_constraint_for (exp, results);
! ! ! /* Complex types are special. Taking the address of one
! allows you to access either part of it through that
! pointer. */
! if (VEC_length (ce_s, *results) == 1 &&
! TREE_CODE (pttype) == COMPLEX_TYPE)
! {
! struct constraint_expr *origrhs;
! varinfo_t origvar;
! struct constraint_expr tmp;
! ! gcc_assert (VEC_length (ce_s, *results) == 1);
! origrhs = VEC_last (ce_s, *results);
! tmp = *origrhs;
! VEC_pop (ce_s, *results);
! origvar = get_varinfo (origrhs->var);
! for (; origvar; origvar = origvar->next)
! {
! tmp.var = origvar->id;
! VEC_safe_push (ce_s, heap, *results, &tmp);
! }
! }


  	      for (i = 0; VEC_iterate (ce_s, *results, i, c); i++)
  		{
--- 2834,2841 ----
  	      struct constraint_expr *c;
  	      unsigned int i;
  	      tree exp = TREE_OPERAND (t, 0);

! get_constraint_for_1 (exp, results, true);

  	      for (i = 0; VEC_iterate (ce_s, *results, i, c); i++)
  		{
*************** get_constraint_for (tree t, VEC (ce_s, h
*** 2888,2901 ****
  	  {
  	  case INDIRECT_REF:
  	    {
! 	      get_constraint_for (TREE_OPERAND (t, 0), results);
  	      do_deref (results);
  	      return;
  	    }
  	  case ARRAY_REF:
  	  case ARRAY_RANGE_REF:
  	  case COMPONENT_REF:
! 	    get_constraint_for_component_ref (t, results);
  	    return;
  	  default:
  	    {
--- 2902,2915 ----
  	  {
  	  case INDIRECT_REF:
  	    {
! 	      get_constraint_for_1 (TREE_OPERAND (t, 0), results, address_p);
  	      do_deref (results);
  	      return;
  	    }
  	  case ARRAY_REF:
  	  case ARRAY_RANGE_REF:
  	  case COMPONENT_REF:
! 	    get_constraint_for_component_ref (t, results, address_p);
  	    return;
  	  default:
  	    {
*************** get_constraint_for (tree t, VEC (ce_s, h
*** 2920,2926 ****
  	      if (!(POINTER_TYPE_P (TREE_TYPE (t))
  		    && ! POINTER_TYPE_P (TREE_TYPE (op))))
  		{
! 		  get_constraint_for (op, results);
  		  return;
  		}

--- 2934,2940 ----
  	      if (!(POINTER_TYPE_P (TREE_TYPE (t))
  		    && ! POINTER_TYPE_P (TREE_TYPE (op))))
  		{
! 		  get_constraint_for_1 (op, results, address_p);
  		  return;
  		}

*************** get_constraint_for (tree t, VEC (ce_s, h
*** 2942,2956 ****
  	  {
  	  case PHI_NODE:
  	    {
! 	      get_constraint_for (PHI_RESULT (t), results);
  	      return;
  	    }
  	    break;
  	  case SSA_NAME:
  	    {
! 	      struct constraint_expr temp;
! 	      temp = get_constraint_exp_from_ssa_var (t);
! 	      VEC_safe_push (ce_s, heap, *results, &temp);
  	      return;
  	    }
  	    break;
--- 2956,2968 ----
  	  {
  	  case PHI_NODE:
  	    {
! 	      get_constraint_for_1 (PHI_RESULT (t), results, address_p);
  	      return;
  	    }
  	    break;
  	  case SSA_NAME:
  	    {
! 	      get_constraint_for_ssa_var (t, results, address_p);
  	      return;
  	    }
  	    break;
*************** get_constraint_for (tree t, VEC (ce_s, h
*** 2966,2974 ****
        }
      case tcc_declaration:
        {
! 	struct constraint_expr temp;
! 	temp = get_constraint_exp_from_ssa_var (t);
! 	VEC_safe_push (ce_s, heap, *results, &temp);
  	return;
        }
      default:
--- 2978,2984 ----
        }
      case tcc_declaration:
        {
! 	get_constraint_for_ssa_var (t, results, address_p);
  	return;
        }
      default:
*************** get_constraint_for (tree t, VEC (ce_s, h
*** 2982,2987 ****
--- 2992,3006 ----
      }
  }

+ /* Given a gimple tree T, return the constraint expression vector for it. */
+ + static void
+ get_constraint_for (tree t, VEC (ce_s, heap) **results)
+ {
+ gcc_assert (VEC_length (ce_s, *results) == 0);
+ + get_constraint_for_1 (t, results, false);
+ }


  /* Handle the structure copy case where we have a simple structure copy
     between LHS and RHS that is of SIZE (in bits)
*************** do_structure_copy (tree lhsop, tree rhso
*** 3140,3147 ****
    unsigned HOST_WIDE_INT lhssize;
    unsigned HOST_WIDE_INT rhssize;

!   get_constraint_for (lhsop, &lhsc);
!   get_constraint_for (rhsop, &rhsc);
    gcc_assert (VEC_length (ce_s, lhsc) == 1);
    gcc_assert (VEC_length (ce_s, rhsc) == 1);
    lhs = *(VEC_last (ce_s, lhsc));
--- 3159,3168 ----
    unsigned HOST_WIDE_INT lhssize;
    unsigned HOST_WIDE_INT rhssize;

!   /* Pretend we are taking the address of the constraint exprs.
!      We deal with walking the sub-fields ourselves.  */
!   get_constraint_for_1 (lhsop, &lhsc, true);
!   get_constraint_for_1 (rhsop, &rhsc, true);
    gcc_assert (VEC_length (ce_s, lhsc) == 1);
    gcc_assert (VEC_length (ce_s, rhsc) == 1);
    lhs = *(VEC_last (ce_s, lhsc));


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