Do not lose track of resolution info due to tree merging

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.

On February 5, 2018 3:49:25 PM GMT+01:00, Jan Hubicka <hubicka@ucw.cz> wrote:
>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?

I think unify_scc is only ever called from WPA so you can elide the flag_ltrans checks. 

Otherwise OK. 

Thanks, 
Richard. 

>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");

> 
> I think unify_scc is only ever called from WPA so you can elide the flag_ltrans checks. 
> 
> Otherwise OK. 

Thanks, I have updated this and also dropped the sanity check from symtab.c
because, well, it finds another bugs in target versioning I will handle
incrementally.  I also noticed I need to rule out the check for register
variables and builtins because we do not stream those into the lto symtab.  I
would say that real_symbol_p should return false for register variable and we
should stream bulitins but that is for independent change, too.

I suppose we should backport this to all release branches.

Honza

	PR lto/81004
	* lto.c: Include builtins.h
	(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.
Index: lto.c
===================================================================
--- lto.c	(revision 257382)
+++ lto.c	(working copy)
@@ -54,6 +54,7 @@ along with GCC; see the file COPYING3.
 #include "stringpool.h"
 #include "fold-const.h"
 #include "attribs.h"
+#include "builtins.h"
 
 
 /* Number of parallel tasks to run, -1 if we want to use GNU Make jobserver.  */
@@ -830,12 +831,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 +887,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 +1638,10 @@ 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);
+	    {
+	      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,7 @@ unify_scc (struct data_in *data_in, unsi
 		{
 		  map2[i*2] = (tree)(uintptr_t)(from + i);
 		  map2[i*2+1] = scc->entries[i];
+		  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 +1786,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 +2892,21 @@ 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
+	&& !is_builtin_fn (snode->decl)
+	&& !(VAR_P (snode->decl) && DECL_HARD_REGISTER (snode->decl)))
+      {
+	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)
       {

On February 6, 2018 2:29:42 PM GMT+01:00, Jan Hubicka <hubicka@ucw.cz> wrote:
>> 
>> I think unify_scc is only ever called from WPA so you can elide the
>flag_ltrans checks. 
>> 
>> Otherwise OK. 
>
>Thanks, I have updated this and also dropped the sanity check from
>symtab.c
>because, well, it finds another bugs in target versioning I will handle
>incrementally.  I also noticed I need to rule out the check for
>register
>variables and builtins because we do not stream those into the lto
>symtab.  I
>would say that real_symbol_p should return false for register variable
>and we
>should stream bulitins but that is for independent change, too.
>
>I suppose we should backport this to all release branches.

After some soaking, yes.

>Honza
>
>	PR lto/81004
>	* lto.c: Include builtins.h
>	(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.
>Index: lto.c
>===================================================================
>--- lto.c	(revision 257382)
>+++ lto.c	(working copy)
>@@ -54,6 +54,7 @@ along with GCC; see the file COPYING3.
> #include "stringpool.h"
> #include "fold-const.h"
> #include "attribs.h"
>+#include "builtins.h"
> 
> 
>/* Number of parallel tasks to run, -1 if we want to use GNU Make
>jobserver.  */
>@@ -830,12 +831,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 +887,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 +1638,10 @@ 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);
>+	    {
>+	      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,7 @@ unify_scc (struct data_in *data_in, unsi
> 		{
> 		  map2[i*2] = (tree)(uintptr_t)(from + i);
> 		  map2[i*2+1] = scc->entries[i];
>+		  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 +1786,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 +2892,21 @@ 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
>+	&& !is_builtin_fn (snode->decl)
>+	&& !(VAR_P (snode->decl) && DECL_HARD_REGISTER (snode->decl)))
>+      {
>+	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)
>       {

Hi,
it turns out that I have hit bit of can of worms here, so I spent most of
yesterday looking into minimal fix.  There are multiple issues remaining - we
forget to output symbols that have only DECL_EXTERNAL flag set; we behave funny
to builtins (in some cases it makes sense in others it does not); we do not
handle well references from extern inlines and constructors.

While preparing previous patch I misread unify_scc and it registers in some
cases prevailed decl instead of prevailing resulting in memory corruption.
This eventually was triggered by assert in get_resolution.

There is other bug that the sanity check in read_cgraph_and_symbols triggers
because we do not output some symbols into the lto symbol table.  This is partly
intended even though these are hacks from early LTO days. To silence false
posities I moved all the (sloppy) logic deciding whether to output symbol
into symtab_node::output_to_lto_symbol_table_p and use consistent check
at stream out time and stream in.

We still may possibly get errors on duplicated decls but I hope we do not have
those in practice anymore. (Those are bugs too) So we may need to end up
disabling this sanity check for release but I hope it won't be needed
and I would like to see if now the mechanizm works reliably, so I have decided
to keep it for now.

Patch survives bootstrap/regtest and lto-bootstrap.  I am giving it bit extra
checking and plan to commit later today unless there are complains.
I will see if I can reproduce the other issues and open separate bugs for them.

Honza

	PR ipa/81360
	* cgraph.h (symtab_node::output_to_lto_symbol_table_p): Declare
	* symtab.c: Include builtins.h
	(symtab_node::output_to_lto_symbol_table_p): Move here
	from lto-streamer-out.c:output_symbol_p.
	* lto-streamer-out.c (write_symbol): Turn early exit to assert.
	(output_symbol_p): Move all logic to symtab.c
	(produce_symtab): Update.

	* lto.c (unify_scc): Register prevailing trees, not trees to be freed.
	(read_cgraph_and_symbols): Use
	symtab_node::output_to_lto_symbol_table_p.
	
Index: cgraph.h
===================================================================
--- cgraph.h	(revision 257412)
+++ cgraph.h	(working copy)
@@ -328,6 +328,9 @@ public:
      or abstract function kept for debug info purposes only.  */
   bool real_symbol_p (void);
 
+  /* Return true when the symbol needs to be output to the LTO symbol table.  */
+  bool output_to_lto_symbol_table_p (void);
+
   /* Determine if symbol declaration is needed.  That is, visible to something
      either outside this translation unit, something magic in the system
      configury. This function is used just during symbol creation.  */
Index: lto/lto.c
===================================================================
--- lto/lto.c	(revision 257442)
+++ lto/lto.c	(working copy)
@@ -1648,13 +1648,16 @@ unify_scc (struct data_in *data_in, unsi
 		{
 		  map2[i*2] = (tree)(uintptr_t)(from + i);
 		  map2[i*2+1] = scc->entries[i];
-		  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);
 	      for (unsigned i = 0; i < len; ++i)
-		streamer_tree_cache_replace_tree (cache, map[2*i],
-						  (uintptr_t)map2[2*i]);
+		{
+		  lto_maybe_register_decl (data_in, map[2*i],
+					   (uintptr_t)map2[2*i]);
+		  streamer_tree_cache_replace_tree (cache, map[2*i],
+						    (uintptr_t)map2[2*i]);
+		}
 	    }
 
 	  /* Free the tree nodes from the read SCC.  */
@@ -2901,8 +2904,12 @@ read_cgraph_and_symbols (unsigned nfiles
 
 	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)));
+	  {
+	    if (snode->output_to_lto_symbol_table_p ())
+	      fatal_error (input_location, "missing resolution data for %s",
+		           IDENTIFIER_POINTER
+			     (DECL_ASSEMBLER_NAME (snode->decl)));
+	  }
 	else
           snode->resolution = *res;
       }
Index: lto-streamer-out.c
===================================================================
--- lto-streamer-out.c	(revision 257412)
+++ lto-streamer-out.c	(working copy)
@@ -2598,13 +2598,10 @@ write_symbol (struct streamer_tree_cache
   const char *comdat;
   unsigned char c;
 
-  /* None of the following kinds of symbols are needed in the
-     symbol table.  */
-  if (!TREE_PUBLIC (t)
-      || is_builtin_fn (t)
-      || DECL_ABSTRACT_P (t)
-      || (VAR_P (t) && DECL_HARD_REGISTER (t)))
-    return;
+  gcc_checking_assert (TREE_PUBLIC (t)
+		       && !is_builtin_fn (t)
+		       && !DECL_ABSTRACT_P (t)
+		       && (!VAR_P (t) || !DECL_HARD_REGISTER (t)));
 
   gcc_assert (VAR_OR_FUNCTION_DECL_P (t));
 
@@ -2692,45 +2689,6 @@ write_symbol (struct streamer_tree_cache
   lto_write_data (&slot_num, 4);
 }
 
-/* Return true if NODE should appear in the plugin symbol table.  */
-
-bool
-output_symbol_p (symtab_node *node)
-{
-  struct cgraph_node *cnode;
-  if (!node->real_symbol_p ())
-    return false;
-  /* We keep external functions in symtab for sake of inlining
-     and devirtualization.  We do not want to see them in symbol table as
-     references unless they are really used.  */
-  cnode = dyn_cast <cgraph_node *> (node);
-  if (cnode && (!node->definition || DECL_EXTERNAL (cnode->decl))
-      && cnode->callers)
-    return true;
-
- /* Ignore all references from external vars initializers - they are not really
-    part of the compilation unit until they are used by folding.  Some symbols,
-    like references to external construction vtables can not be referred to at all.
-    We decide this at can_refer_decl_in_current_unit_p.  */
- if (!node->definition || DECL_EXTERNAL (node->decl))
-    {
-      int i;
-      struct ipa_ref *ref;
-      for (i = 0; node->iterate_referring (i, ref); i++)
-	{
-	  if (ref->use == IPA_REF_ALIAS)
-	    continue;
-          if (is_a <cgraph_node *> (ref->referring))
-	    return true;
-	  if (!DECL_EXTERNAL (ref->referring->decl))
-	    return true;
-	}
-      return false;
-    }
-  return true;
-}
-
-
 /* Write an IL symbol table to OB.
    SET and VSET are cgraph/varpool node sets we are outputting.  */
 
@@ -2755,7 +2713,7 @@ produce_symtab (struct output_block *ob)
     {
       symtab_node *node = lsei_node (lsei);
 
-      if (!output_symbol_p (node) || DECL_EXTERNAL (node->decl))
+      if (DECL_EXTERNAL (node->decl) || !node->output_to_lto_symbol_table_p ())
 	continue;
       write_symbol (cache, node->decl, &seen, false);
     }
@@ -2764,7 +2722,7 @@ produce_symtab (struct output_block *ob)
     {
       symtab_node *node = lsei_node (lsei);
 
-      if (!output_symbol_p (node) || !DECL_EXTERNAL (node->decl))
+      if (!DECL_EXTERNAL (node->decl) || !node->output_to_lto_symbol_table_p ())
 	continue;
       write_symbol (cache, node->decl, &seen, false);
     }
Index: symtab.c
===================================================================
--- symtab.c	(revision 257412)
+++ symtab.c	(working copy)
@@ -37,6 +37,7 @@ along with GCC; see the file COPYING3.
 #include "calls.h"
 #include "stringpool.h"
 #include "attribs.h"
+#include "builtins.h"
 
 static const char *ipa_ref_use_name[] = {"read","write","addr","alias","chkp"};
 
@@ -2292,3 +2298,58 @@ symtab_node::binds_to_current_def_p (sym
 
   return false;
 }
+
+/* Return true if symbol should be output to the symbol table.  */
+
+bool
+symtab_node::output_to_lto_symbol_table_p (void)
+{
+  /* Only externally visible symbols matter.  */
+  if (!TREE_PUBLIC (decl))
+    return false;
+  if (!real_symbol_p ())
+    return false;
+  /* FIXME: variables probably should not be considered as real symbols at
+     first place.  */
+  if (VAR_P (decl) && DECL_HARD_REGISTER (decl))
+    return false;
+  /* FIXME: Builtins corresponding to real functions probably should have
+     symbol table entries.  */
+  if (is_builtin_fn (decl))
+    return false;
+
+  /* We have real symbol that should be in symbol table.  However try to trim
+     down the refernces to libraries bit more because linker will otherwise
+     bring unnecesary object files into the final link.
+     FIXME: The following checks can easily be confused i.e. by self recursive
+     function or self-referring variable.  */
+
+  /* We keep external functions in symtab for sake of inlining
+     and devirtualization.  We do not want to see them in symbol table as
+     references unless they are really used.  */
+  cgraph_node *cnode = dyn_cast <cgraph_node *> (this);
+  if (cnode && (!definition || DECL_EXTERNAL (decl))
+      && cnode->callers)
+    return true;
+
+ /* Ignore all references from external vars initializers - they are not really
+    part of the compilation unit until they are used by folding.  Some symbols,
+    like references to external construction vtables can not be referred to at
+    all.  We decide this at can_refer_decl_in_current_unit_p.  */
+ if (!definition || DECL_EXTERNAL (decl))
+    {
+      int i;
+      struct ipa_ref *ref;
+      for (i = 0; iterate_referring (i, ref); i++)
+	{
+	  if (ref->use == IPA_REF_ALIAS)
+	    continue;
+          if (is_a <cgraph_node *> (ref->referring))
+	    return true;
+	  if (!DECL_EXTERNAL (ref->referring->decl))
+	    return true;
+	}
+      return false;
+    }
+  return true;
+}

Hi,
this is patch I comitted yesterday (but failed to send email) which removes
forgotten sanity check from original fix. The check tries to catch cases where
we do merge definitions and declarations to see if resolution merging logic is
live.  It is.

Honza
	* lto.c (register_resolution): Remove forgotten sanity check.

Index: lto.c
===================================================================
--- lto.c	(revision 257412)
+++ lto.c	(working copy)
@@ -839,7 +839,6 @@ register_resolution (struct lto_file_dec
       = new hash_map<tree, ld_plugin_symbol_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