Patchwork PR lto/44246 (same_comdat_group messup)

login
register
mail settings
Submitter Jan Hubicka
Date Sept. 17, 2010, 1:20 p.m.
Message ID <20100917132041.GF6006@kam.mff.cuni.cz>
Download mbox | patch
Permalink /patch/65074/
State New
Headers show

Comments

Jan Hubicka - Sept. 17, 2010, 1:20 p.m.
Hi,
this patch fixes ICE where we screw up streaming in same_comdat_group pointer.
This is because it is treamed as encoder reference and translated later.  
Builtins have two decls one for the function

<function_decl 0x7ffff7f40000 strlen
    type <function_type 0x7ffff7f3db28
        type <integer_type 0x7ffff7ed0690 long unsigned int public unsigned DI
            size <integer_cst 0x7ffff7ebc7d0 constant 64>
            unit size <integer_cst 0x7ffff7ebc7f8 constant 8>
            align 64 symtab 0 alias set 4 canonical type 0x7ffff7ed0690
precision 64 min <integer_cst 0x7ffff7ebc820 0> max <integer_cst 0x7ffff7ebc7a8
18446744073709551615>>
        QI
        size <integer_cst 0x7ffff7ebc4d8 constant 8>
        unit size <integer_cst 0x7ffff7ebc500 constant 1>
        align 8 symtab 0 alias set -1 canonical type 0x7ffff7ee8888
        attributes <tree_list 0x7ffff7f3ac58
            purpose <identifier_node 0x7ffff7ecfc30 nonnull>>
        arg-types <tree_list 0x7ffff7ee6640 value <pointer_type 0x7ffff7ee3dc8>
            chain <tree_list 0x7ffff7ebceb0 value <void_type 0x7ffff7ed0e70
void>>>
        pointer_to_this <pointer_type 0x7ffff5aebb28>>
    addressable used nothrow public external built-in decl_2 decl_5 decl_6 QI
file <built-in> line 0 col 0
    align 8 built-in BUILT_IN_NORMAL:BUILT_IN_STRLEN attributes <tree_list

and other for the builtin:

<function_decl 0x7ffff7f3ef00 __builtin_strlen
    type <function_type 0x7ffff7f3db28
        type <integer_type 0x7ffff7ed0690 long unsigned int public unsigned DI
            size <integer_cst 0x7ffff7ebc7d0 constant 64>
            unit size <integer_cst 0x7ffff7ebc7f8 constant 8>
            align 64 symtab 0 alias set 4 canonical type 0x7ffff7ed0690
precision 64 min <integer_cst 0x7ffff7ebc820 0> max <integer_cst 0x7ffff7ebc7a8
18446744073709551615>>
        QI
        size <integer_cst 0x7ffff7ebc4d8 constant 8>
        unit size <integer_cst 0x7ffff7ebc500 constant 1>
        align 8 symtab 0 alias set -1 canonical type 0x7ffff7ee8888
        attributes <tree_list 0x7ffff7f3ac58
            purpose <identifier_node 0x7ffff7ecfc30 nonnull>>
        arg-types <tree_list 0x7ffff7ee6640 value <pointer_type 0x7ffff7ee3dc8>
            chain <tree_list 0x7ffff7ebceb0 value <void_type 0x7ffff7ed0e70
void>>>
        pointer_to_this <pointer_type 0x7ffff5aebb28>>
    nothrow public external built-in decl_6 QI file <built-in> line 0 col 0
    align 8 built-in BUILT_IN_NORMAL:BUILT_IN_STRLEN attributes <tree_list
0x7ffff7f3ac80> chain <function_decl 0x7ffff7f40000 strlen>>

This property is not preserved when streaming and thus the two cgraph node unify.
The unified nodes then get twice through translation.  First we translate -1
into NULL and second time we interperd NULL as reference to node 0 and get wrong
reply.

This patch avoids the double translation, but I am somewhat nervous about this
unification of nodes on the fly, because I think it will just eventually cause
troubles reading summaries, as summary code in general assume that before decl
merging happening later there is 1-1 correspondence in between cgraph nodes
streamed out and streamed in.

Anyway, bootstrapped/regtested this fix and will commit it as other
parts of lto-cgraph are already special casing builtins same way.

Honza

int main(int argc, char *argv[])
{
  strcat(argv[0], "X");
  return strlen(argv[0]);
}
	PR lto/44246
	* lto-cgraph.c (input_cgraph_1, input_varpool_1): Avoid
	processing same node twice.

Patch

Index: lto-cgraph.c
===================================================================
--- lto-cgraph.c	(revision 164344)
+++ lto-cgraph.c	(working copy)
@@ -1283,11 +1294,20 @@  input_cgraph_1 (struct lto_file_decl_dat
 
       len = lto_input_uleb128 (ib);
     }
-
+  /* AUX pointers should be all non-zero for nodes read from the stream.  */
+#ifdef ENABLE_CHECKING
+  FOR_EACH_VEC_ELT (cgraph_node_ptr, nodes, i, node)
+    gcc_assert (node->aux);
+#endif
   FOR_EACH_VEC_ELT (cgraph_node_ptr, nodes, i, node)
     {
       int ref = (int) (intptr_t) node->global.inlined_to;
 
+      /* We share declaration of builtins, so we may read same node twice.  */
+      if (!node->aux)
+	continue;
+      node->aux = NULL;
+
       /* Fixup inlined_to from reference to pointer.  */
       if (ref != LCC_NOT_FOUND)
 	node->global.inlined_to = VEC_index (cgraph_node_ptr, nodes, ref);
@@ -1302,6 +1322,8 @@  input_cgraph_1 (struct lto_file_decl_dat
       else
 	node->same_comdat_group = NULL;
     }
+  FOR_EACH_VEC_ELT (cgraph_node_ptr, nodes, i, node)
+    node->aux = (void *)1;
   return nodes;
 }
 
@@ -1323,9 +1345,17 @@  input_varpool_1 (struct lto_file_decl_da
 		     input_varpool_node (file_data, ib));
       len--;
     }
+#ifdef ENABLE_CHECKING
+  FOR_EACH_VEC_ELT (varpool_node_ptr, varpool, i, node)
+    gcc_assert (!node->aux);
+#endif
   FOR_EACH_VEC_ELT (varpool_node_ptr, varpool, i, node)
     {
       int ref = (int) (intptr_t) node->same_comdat_group;
+      /* We share declaration of builtins, so we may read same node twice.  */
+      if (node->aux)
+	continue;
+      node->aux = (void *)1;
 
       /* Fixup same_comdat_group from reference to pointer.  */
       if (ref != LCC_NOT_FOUND)
@@ -1333,6 +1363,8 @@  input_varpool_1 (struct lto_file_decl_da
       else
 	node->same_comdat_group = NULL;
     }
+  FOR_EACH_VEC_ELT (varpool_node_ptr, varpool, i, node)
+    node->aux = NULL;
   return varpool;
 }