On February 5, 2018 3:49:25 PM GMT+01:00, Jan Hubicka <[email protected]> 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");
