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]

RFA: Fix ICE related to ADDR_EXPR forward propagation


Mainline currently segfaults while trying to build newlib's hash.c.
The problem seems to be caused by the recent patch to forward-propagate
ADDR_EXPRs.  The idea of those changes is to replace:

    t1 = &thing[0]
    t2 = index * scale
    t3 = (ptr_type) t2
    result = t1 + t3

with:

    result = &thing[index]

The problem is that the tree for "&thing[index]" starts life
as a copy of "&thing[0]" and is then modified in-place.
Thus if "&thing[0]" was a function invariant, the tree for
"&thing[index]" will end up with the TREE_INVARIANT bit set.

Obviously things have already gone wrong at this point, but to
finish the story: if the flag is set, get_expr_operands() will
not look inside the ADDR_EXPR when building up a list of uses.
This in turn will cause DCE to delete the assignment to "index"
if its only use was in "&thing[index]".  We then ICE in
verify_stmt() due to "index" being on the free list.
(FWIW, because the verify_stmt() walk looks at parent nodes
before child nodes, it actually segfaults before hitting the
"SSA_NAME on free list" check.)

The code that does the replacement is in tree-ssa-forwprop.c:
forward_propagate_addr_into_variable_array_index():

  /* Replace the pointer addition with array indexing.  */
  TREE_OPERAND (use_stmt, 1) = unshare_expr (TREE_OPERAND (stmt, 1));
  TREE_OPERAND (TREE_OPERAND (TREE_OPERAND (use_stmt, 1), 0), 1) = index;

I'm not too familiar with the rules about constructing trees, so I'm not
sure whether there's a rule that says that unshare_expr must be used
here (rather than calling buildN(), say).  If there is such a rule,
I guess the obvious fix is to simply clear the TREE_INVARIANT flag
after the fact, as per the patch below.  I'm not exactly 100%
confident this will be accepted as the right fix though. ;)
Still, let's be optimistic...

...tested by bootstrapping & regression testing on i686-pc-linux-gnu.
OK to install?

Richard


	* tree-ssa-forwprop.c
	(forward_propagate_addr_into_variable_array_index): Clear the
	TREE_INVARIANT flag in the new ADDR_EXPR.

testsuite/
	* gcc.c-torture/compile/20050520-1.c: New test.

Index: tree-ssa-forwprop.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-forwprop.c,v
retrieving revision 2.19
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r2.19 tree-ssa-forwprop.c
*** tree-ssa-forwprop.c	19 May 2005 03:05:40 -0000	2.19
--- tree-ssa-forwprop.c	20 May 2005 06:13:57 -0000
*************** forward_propagate_addr_into_variable_arr
*** 515,520 ****
--- 515,521 ----
    /* Replace the pointer addition with array indexing.  */
    TREE_OPERAND (use_stmt, 1) = unshare_expr (TREE_OPERAND (stmt, 1));
    TREE_OPERAND (TREE_OPERAND (TREE_OPERAND (use_stmt, 1), 0), 1) = index;
+   TREE_INVARIANT (TREE_OPERAND (use_stmt, 1)) = false;
  
    /* That should have created gimple, so there is no need to
       record information to undo the propagation.  */
diff -c /dev/null testsuite/gcc.c-torture/compile/20050520-1.c
*** /dev/null	2005-03-29 10:04:47.000000000 +0100
--- testsuite/gcc.c-torture/compile/20050520-1.c	2005-05-20 04:36:42.000000000 +0100
***************
*** 0 ****
--- 1,13 ----
+ struct s { int x[4]; };
+ struct s gs;
+ 
+ void
+ bar (void)
+ {
+   struct s *s;
+   int i;
+ 
+   s = &gs;
+   for (i = 0; i < 4; i++)
+     ((char*) (&s->x[i]))[0] = 0;
+ }


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