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]

Re: [PATCH] Fix PR40389


On Sat, 13 Jun 2009, Richard Guenther wrote:

> On Sat, 13 Jun 2009, Richard Guenther wrote:
> 
> > 
> > Thix fixes PR40389 - we have to assume that the return value has
> > its address taken when NRV is in effect (this may not be a 100%
> > complete patch, as we probably also have to assume that it escaped).
> > 
> > At least it fixes the miscompile.
> > 
> > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
> 
> Actually not.  When adding the PTA fix I see missed optimizations as
> the C++ FE unconditionally sets TREE_ADDRESSABLE on the LHS.  If

Simple oversight from me.

> I understand correctly the issue only appears if a copy was elided.
> 
> I am currently bootstrapping the following more constrained fix.

I'm installing the following, bootstrapped and tested on
x86_64-unknown-linux-gnu.

Richard.

2009-06-13  Richard Guenther  <rguenther@suse.de>

	PR middle-end/40389
	* gimple.c (walk_stmt_load_store_addr_ops): The LHS of a call
	has its address taken if NRV was applied and it is addressable.
	* tree-ssa-structalias.c (get_constraint_for_address_of): New
	function split out from ...
	(get_constraint_for_1): ... here.
	(handle_rhs_call): Use it to mark the return slot escaped if
	it is addressable and NRV was applied.

	* g++.dg/torture/pr40389.C: New testcase.

Index: gcc/gimple.c
===================================================================
*** gcc/gimple.c	(revision 148457)
--- gcc/gimple.c	(working copy)
*************** walk_stmt_load_store_addr_ops (gimple st
*** 3264,3269 ****
--- 3264,3274 ----
  	  && TREE_CODE (gimple_call_chain (stmt)) == ADDR_EXPR)
  	ret |= visit_addr (stmt, TREE_OPERAND (gimple_call_chain (stmt), 0),
  			   data);
+       if (visit_addr
+ 	  && gimple_call_return_slot_opt_p (stmt)
+ 	  && gimple_call_lhs (stmt) != NULL_TREE
+ 	  && TREE_ADDRESSABLE (gimple_call_lhs (stmt)))
+ 	ret |= visit_addr (stmt, gimple_call_lhs (stmt), data);
      }
    else if (gimple_code (stmt) == GIMPLE_ASM)
      {
Index: gcc/tree-ssa-structalias.c
===================================================================
*** gcc/tree-ssa-structalias.c	(revision 148457)
--- gcc/tree-ssa-structalias.c	(working copy)
*************** do_deref (VEC (ce_s, heap) **constraints
*** 3080,3085 ****
--- 3080,3107 ----
      }
  }
  
+ static void get_constraint_for_1 (tree, VEC (ce_s, heap) **, bool);
+ 
+ /* Given a tree T, return the constraint expression for taking the
+    address of it.  */
+ 
+ static void
+ get_constraint_for_address_of (tree t, VEC (ce_s, heap) **results)
+ {
+   struct constraint_expr *c;
+   unsigned int i;
+ 
+   get_constraint_for_1 (t, results, true);
+ 
+   for (i = 0; VEC_iterate (ce_s, *results, i, c); i++)
+     {
+       if (c->type == DEREF)
+ 	c->type = SCALAR;
+       else
+ 	c->type = ADDRESSOF;
+     }
+ }
+ 
  /* Given a tree T, return the constraint expression for it.  */
  
  static void
*************** get_constraint_for_1 (tree t, VEC (ce_s,
*** 3131,3153 ****
  	switch (TREE_CODE (t))
  	  {
  	  case ADDR_EXPR:
! 	    {
! 	      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++)
! 		{
! 		  if (c->type == DEREF)
! 		    c->type = SCALAR;
! 		  else
! 		    c->type = ADDRESSOF;
! 		}
! 	      return;
! 	    }
! 	    break;
  	  default:;
  	  }
  	break;
--- 3153,3160 ----
  	switch (TREE_CODE (t))
  	  {
  	  case ADDR_EXPR:
! 	    get_constraint_for_address_of (TREE_OPERAND (t, 0), results);
! 	    return;
  	  default:;
  	  }
  	break;
*************** handle_rhs_call (gimple stmt, VEC(ce_s, 
*** 3333,3338 ****
--- 3340,3361 ----
    if (gimple_call_chain (stmt))
      make_escape_constraint (gimple_call_chain (stmt));
  
+   /* And if we applied NRV the address of the return slot escapes as well.  */
+   if (gimple_call_return_slot_opt_p (stmt)
+       && gimple_call_lhs (stmt) != NULL_TREE
+       && TREE_ADDRESSABLE (gimple_call_lhs (stmt)))
+     {
+       VEC(ce_s, heap) *tmpc = NULL;
+       struct constraint_expr lhsc, *c;
+       get_constraint_for_address_of (gimple_call_lhs (stmt), &tmpc);
+       lhsc.var = escaped_id;
+       lhsc.offset = 0;
+       lhsc.type = SCALAR;
+       for (i = 0; VEC_iterate (ce_s, tmpc, i, c); ++i)
+ 	process_constraint (new_constraint (lhsc, *c));
+       VEC_free(ce_s, heap, tmpc);
+     }
+ 
    /* Regular functions return nonlocal memory.  */
    rhsc.var = nonlocal_id;
    rhsc.offset = 0;
Index: gcc/testsuite/g++.dg/torture/pr40389.C
===================================================================
*** gcc/testsuite/g++.dg/torture/pr40389.C	(revision 0)
--- gcc/testsuite/g++.dg/torture/pr40389.C	(revision 0)
***************
*** 0 ****
--- 1,84 ----
+ /* { dg-do run } */
+ 
+ template <typename V> struct S
+ {
+   V *f, *l;
+   __attribute__ ((noinline)) S (void) { f = 0, l = 0; }
+   void foo (V *x)
+   {
+     if (x->p != 0)
+       x->p->n = x->n;
+     else
+       f = x->n;
+     if (x->n != 0)
+       x->n->p = x->p;
+     else
+       l = x->p;
+   }
+   __attribute__ ((noinline)) void bar (V *x)
+   {
+     x->n = 0;
+     x->p = l;
+     if (l != 0)
+       l->n = x;
+     else
+       f = x;
+     l = x;
+   }
+ };
+ 
+ struct H;
+ 
+ struct A
+ {
+   S <H> k;
+ };
+ 
+ struct H
+ {
+   A *a;
+   H *p, *n;
+   __attribute__ ((noinline)) H (void) { p = 0, n = 0, a = 0; }
+   __attribute__ ((noinline)) H (A *b) : a (b)
+   {
+     p = 0;
+     n = 0;
+     if (a != 0)
+       a->k.bar (this);
+   }
+   __attribute__ ((noinline)) H (const H &h) : a (h.a)
+   {
+     p = 0;
+     n = 0;
+     if (a != 0)
+       a->k.bar (this);
+   }
+   ~H (void) { if (a != 0) a->k.foo (this); }
+   H &operator= (const H &o)
+   {
+     if (a != 0 || &o == this)
+       __builtin_abort ();
+     a = o.a;
+     if (a != 0)
+       a->k.bar (this);
+     return *this;
+   }
+ };
+ 
+ __attribute__ ((noinline))
+ H baz (void)
+ {
+   return H (new A);
+ }
+ 
+ H g;
+ 
+ int
+ main (void)
+ {
+   g = baz ();
+   if (g.a->k.f != &g)
+     __builtin_abort ();
+   return 0;
+ }
+ 


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