Do not lose track of resolution info due to tree merging

Jan Hubicka hubicka@ucw.cz
Mon Feb 5 14:49:00 GMT 2018


Hi,
in the testcase of PR81004 we produce wrong code (reference to optimized out
comdat symbol) due to strange sequence of events that is initiated by fact
that after streaming in a comdat group we sed one of resolution infos to
LDPR_UNKNOWN.

This is because of tree merging with earlier unit that streams the symbol for
debug info but does not use it and thus there is no symtab entry.

Currently resolutions are streamed in a funny way - they are indexed by decls
and passed through plugin to be read back as array and later attached into hash
to tree nodes to be later written into symtab node (where they belong).
This is quite crazy design but I do not see how to easy clean it up, so this
fixes the problem and I will add cleanup to my todo for next stage1.

patch also adds sanity checking so we error out on missing relocation info.
I have checked this works with non-plugin path.
I did not add testcase because it is very fragile (depends on what we stream)
and it already does not reproduce on mainline without Jakub's patch to clear
abstract origins in free_lang_data reverted.

Bootstrapped/regtested x86_64-linux, ltobootstrap in progress (passing stage2)
OK?

Honza
	PR lto/81004
	* lto.c (register_resolution): Merge resolutions in case trees was
	merged across units.
	(lto_maybe_register_decl): Break out from ...
	(lto_read_decls): ... here.
	(unify_scc): Also register decls here.
	(read_cgraph_and_symbols): Sanity check that all resolutions was
	read.
	* symtab.c (symtab_node::verify_base): Verify that externaly visible
	symbol has PUBLIC decl.
Index: lto/lto.c
===================================================================
--- lto/lto.c	(revision 257382)
+++ lto/lto.c	(working copy)
@@ -830,12 +830,20 @@ static void
 register_resolution (struct lto_file_decl_data *file_data, tree decl,
 		     enum ld_plugin_symbol_resolution resolution)
 {
+  bool existed;
   if (resolution == LDPR_UNKNOWN)
     return;
   if (!file_data->resolution_map)
     file_data->resolution_map
       = new hash_map<tree, ld_plugin_symbol_resolution>;
-  file_data->resolution_map->put (decl, resolution);
+  ld_plugin_symbol_resolution_t &res
+     = file_data->resolution_map->get_or_insert (decl, &existed);
+  gcc_assert (!existed || res == resolution);
+  if (!existed
+      || resolution == LDPR_PREVAILING_DEF_IRONLY
+      || resolution == LDPR_PREVAILING_DEF
+      || resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
+    res = resolution;
 }
 
 /* Register DECL with the global symbol table and change its
@@ -878,6 +886,18 @@ lto_register_function_decl_in_symtab (st
 			 decl, get_resolution (data_in, ix));
 }
 
+/* Check if T is a decl and needs register its resolution info.  */
+
+static void
+lto_maybe_register_decl (struct data_in *data_in, tree t, unsigned ix)
+{
+  if (TREE_CODE (t) == VAR_DECL)
+    lto_register_var_decl_in_symtab (data_in, t, ix);
+  else if (TREE_CODE (t) == FUNCTION_DECL
+	   && !DECL_BUILT_IN (t))
+    lto_register_function_decl_in_symtab (data_in, t, ix);
+}
+
 
 /* For the type T re-materialize it in the type variant list and
    the pointer/reference-to chains.  */
@@ -1617,7 +1637,11 @@ unify_scc (struct data_in *data_in, unsi
 	  /* Fixup the streamer cache with the prevailing nodes according
 	     to the tree node mapping computed by compare_tree_sccs.  */
 	  if (len == 1)
-	    streamer_tree_cache_replace_tree (cache, pscc->entries[0], from);
+	    {
+	      if (!flag_ltrans)
+		lto_maybe_register_decl (data_in, pscc->entries[0], from);
+	       streamer_tree_cache_replace_tree (cache, pscc->entries[0], from);
+	    }
 	  else
 	    {
 	      tree *map2 = XALLOCAVEC (tree, 2 * len);
@@ -1625,6 +1649,8 @@ unify_scc (struct data_in *data_in, unsi
 		{
 		  map2[i*2] = (tree)(uintptr_t)(from + i);
 		  map2[i*2+1] = scc->entries[i];
+		  if (!flag_ltrans)
+		    lto_maybe_register_decl (data_in, scc->entries[i], from + i);
 		}
 	      qsort (map2, len, 2 * sizeof (tree), cmp_tree);
 	      qsort (map, len, 2 * sizeof (tree), cmp_tree);
@@ -1761,13 +1787,7 @@ lto_read_decls (struct lto_file_decl_dat
 		cache_integer_cst (t);
 	      if (!flag_ltrans)
 		{
-		  /* Register variables and functions with the
-		     symbol table.  */
-		  if (TREE_CODE (t) == VAR_DECL)
-		    lto_register_var_decl_in_symtab (data_in, t, from + i);
-		  else if (TREE_CODE (t) == FUNCTION_DECL
-			   && !DECL_BUILT_IN (t))
-		    lto_register_function_decl_in_symtab (data_in, t, from + i);
+		  lto_maybe_register_decl (data_in, t, from + i);
 		  /* Scan the tree for references to global functions or
 		     variables and record those for later fixup.  */
 		  if (mentions_vars_p (t))
@@ -2873,13 +2893,19 @@ read_cgraph_and_symbols (unsigned nfiles
 
   /* Store resolutions into the symbol table.  */
 
-  ld_plugin_symbol_resolution_t *res;
   FOR_EACH_SYMBOL (snode)
-    if (snode->real_symbol_p ()
-	&& snode->lto_file_data
-	&& snode->lto_file_data->resolution_map
-	&& (res = snode->lto_file_data->resolution_map->get (snode->decl)))
-      snode->resolution = *res;
+    if (snode->externally_visible && snode->real_symbol_p ()
+	&& snode->lto_file_data && snode->lto_file_data->resolution_map)
+      {
+	ld_plugin_symbol_resolution_t *res;
+
+	res = snode->lto_file_data->resolution_map->get (snode->decl);
+	if (!res || *res == LDPR_UNKNOWN)
+	  fatal_error (input_location, "missing resolution data for %s",
+		       IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (snode->decl)));
+	else
+          snode->resolution = *res;
+      }
   for (i = 0; all_file_decl_data[i]; i++)
     if (all_file_decl_data[i]->resolution_map)
       {
Index: symtab.c
===================================================================
--- symtab.c	(revision 257382)
+++ symtab.c	(working copy)
@@ -1056,6 +1056,11 @@ symtab_node::verify_base (void)
       error ("node has body_removed but is definition");
       error_found = true;
     }
+  if (externally_visible && !TREE_PUBLIC (decl))
+    {
+      error ("node is externally visible but decl has no PUBLIC flag");
+      error_found = true;
+    }
   if (analyzed && !definition)
     {
       error ("node is analyzed but it is not a definition");



More information about the Gcc-patches mailing list