This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR40389
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
I understand correctly the issue only appears if a copy was elided.
I am currently bootstrapping the following more constrained fix.
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,3152 ****
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:;
}
--- 3153,3159 ----
switch (TREE_CODE (t))
{
case ADDR_EXPR:
! get_constraint_for_address_of (TREE_OPERAND (t, 0), results);
break;
default:;
}
*************** 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;
+ }
+