Hi,
this patch fixes with weakrefs seen on building latest firefox. The problem is
that currently we handle weakrefs as external variables/functions that makes us
to merge them. In firefox there are two weakrefs with different aliases used
in different units. This is correct and well defined even if weird use.
This patch adds special cases for weakrefs into lto-symtab and lto-partition
to make them go through correctly. I also fixed two fallouts of my previous
change that reproduce on firefox (and the testcase addded).
For lto-partition the weakrefs with defined target are even bit more special
animals, since they needs to be duplicated into every unit that use them.
It is ugly to special case wekarefs all around. My plan is to cleanup whole
area, but it seems that the correctness issue deserve to be fixed first.
I have bootstrapped/regtested x86_64-linux and tested mozilla build. Will
commit it tonight if there are no complains.
Honza
PR lto/57038
PR lto/47375
* lto-symtab.c (lto_symtab_symbol_p): Add external symbol; weakrefs are
not external.
(lto_symtab_merge_decls): Fix thinko when dealing with non-lto_symtab
decls.
(lto_symtab_merge_cgraph_nodes): Use lto_symtab_symbol_p.
(lto_symtab_prevailing_decl): Get int sync with lto_symtab_symbol_p.
* varpool.c (dump_varpool_node): Dump more flags.
* lto-partition.c (get_symbol_class): Fix weakrefs.
(lto_balanced_map): Fix weakrefs.
(privatize_symbol_name): Remove unnecesary label.
(rename_statics): Handle weakrefs as statics.
* gcc.dg/lto/attr-weakref-1_0.c: New testcase.
* gcc.dg/lto/attr-weakref-1_1.c: New testcase.
* gcc.dg/lto/attr-weakref-1_2.c: New testcase.
Index: lto-symtab.c
===================================================================
*** lto-symtab.c (revision 198824)
--- lto-symtab.c (working copy)
*************** lto_symtab_resolve_replaceable_p (symtab
*** 227,239 ****
}
/* Return true, if the symbol E should be resolved by lto-symtab.
! Those are all real symbols that are not static (we handle renaming
! of static later in partitioning). */
static bool
lto_symtab_symbol_p (symtab_node e)
{
! if (!TREE_PUBLIC (e->symbol.decl))
return false;
return symtab_real_symbol_p (e);
}
--- 227,242 ----
}
/* Return true, if the symbol E should be resolved by lto-symtab.
! Those are all external symbols and all real symbols that are not static (we
! handle renaming of static later in partitioning). */
static bool
lto_symtab_symbol_p (symtab_node e)
{
! if (!TREE_PUBLIC (e->symbol.decl) && !DECL_EXTERNAL (e->symbol.decl))
! return false;
! /* weakrefs are really static variables that are made external by a hack.
*/
! if (lookup_attribute ("weakref", DECL_ATTRIBUTES (e->symbol.decl)))
return false;
return symtab_real_symbol_p (e);
}
*************** lto_symtab_merge_decls (void)
*** 528,537 ****
symtab_initialize_asm_name_hash ();
FOR_EACH_SYMBOL (node)
! if (TREE_PUBLIC (node->symbol.decl)
! && node->symbol.next_sharing_asm_name
! && !node->symbol.previous_sharing_asm_name)
! lto_symtab_merge_decls_1 (node);
}
/* Helper to process the decl chain for the symbol table entry *SLOT. */
--- 531,549 ----
symtab_initialize_asm_name_hash ();
FOR_EACH_SYMBOL (node)
! if (lto_symtab_symbol_p (node)
! && node->symbol.next_sharing_asm_name)
! {
! symtab_node n;
!
! /* To avoid duplicated work, see if this is first real symbol in the
! chain. */
! for (n = node->symbol.previous_sharing_asm_name;
! n && !lto_symtab_symbol_p (n); n =
n->symbol.previous_sharing_asm_name)
! ;
! if (!n)
! lto_symtab_merge_decls_1 (node);
! }
}
/* Helper to process the decl chain for the symbol table entry *SLOT. */
*************** lto_symtab_merge_cgraph_nodes (void)
*** 574,580 ****
if (!flag_ltrans)
FOR_EACH_SYMBOL (node)
! if (TREE_PUBLIC (node->symbol.decl)
&& node->symbol.next_sharing_asm_name
&& !node->symbol.previous_sharing_asm_name)
lto_symtab_merge_cgraph_nodes_1 (node);
--- 586,592 ----
if (!flag_ltrans)
FOR_EACH_SYMBOL (node)
! if (lto_symtab_symbol_p (node)
&& node->symbol.next_sharing_asm_name
&& !node->symbol.previous_sharing_asm_name)
lto_symtab_merge_cgraph_nodes_1 (node);
*************** lto_symtab_prevailing_decl (tree decl)
*** 602,608 ****
symtab_node ret;
/* Builtins and local symbols are their own prevailing decl. */
! if (!TREE_PUBLIC (decl) || is_builtin_fn (decl))
return decl;
/* DECL_ABSTRACTs are their own prevailng decl. */
--- 614,620 ----
symtab_node ret;
/* Builtins and local symbols are their own prevailing decl. */
! if ((!TREE_PUBLIC (decl) && !DECL_EXTERNAL (decl)) || is_builtin_fn (decl))
return decl;
/* DECL_ABSTRACTs are their own prevailng decl. */
*************** lto_symtab_prevailing_decl (tree decl)
*** 614,619 ****
--- 626,636 ----
if (TREE_CODE (decl) == FUNCTION_DECL && DECL_BUILT_IN (decl))
return decl;
+ /* As an anoying special cases weakrefs are really static variables with
+ EXTERNAL flag. */
+ if (lookup_attribute ("weakref", DECL_ATTRIBUTES (decl)))
+ return decl;
+
/* Ensure DECL_ASSEMBLER_NAME will not set assembler name. */
gcc_assert (DECL_ASSEMBLER_NAME_SET_P (decl));
Index: testsuite/gcc.dg/lto/attr-weakref-1_0.c
===================================================================
*** testsuite/gcc.dg/lto/attr-weakref-1_0.c (revision 0)
--- testsuite/gcc.dg/lto/attr-weakref-1_0.c (revision 0)
***************
*** 0 ****
--- 1,29 ----
+ /* { dg-lto-do run } */
+ int first = 0;
+ void abort (void);
+ int second = 0;
+ void callmealias (void)
+ {
+ if (!first || !second)
+ abort ();
+ }
+ void callmefirst (void)
+ {
+ if (first)
+ abort();
+ first = 1;
+ }
+ void callmesecond (void)
+ {
+ if (!first)
+ abort();
+ if (second)
+ abort();
+ second = 1;
+ }
+ main()
+ {
+ c();
+ b();
+ return 0;
+ }
Index: testsuite/gcc.dg/lto/attr-weakref-1_1.c
===================================================================
*** testsuite/gcc.dg/lto/attr-weakref-1_1.c (revision 0)
--- testsuite/gcc.dg/lto/attr-weakref-1_1.c (revision 0)
***************
*** 0 ****
--- 1,7 ----
+ extern void callmesecond();
+ static void callmealias() __attribute__((weakref ("callmesecond")));
+
+ b()
+ {
+ callmealias();
+ }
Index: testsuite/gcc.dg/lto/attr-weakref-1_2.c
===================================================================
*** testsuite/gcc.dg/lto/attr-weakref-1_2.c (revision 0)
--- testsuite/gcc.dg/lto/attr-weakref-1_2.c (revision 0)
***************
*** 0 ****
--- 1,7 ----
+ extern void callmefirst();
+ static void callmealias() __attribute__((weakref ("callmefirst")));
+
+ c()
+ {
+ callmealias();
+ }
Index: lto/lto-partition.c
===================================================================
*** lto/lto-partition.c (revision 198824)
--- lto/lto-partition.c (working copy)
*************** get_symbol_class (symtab_node node)
*** 59,64 ****
--- 59,68 ----
if (cnode && cnode->global.inlined_to)
return SYMBOL_DUPLICATE;
+ /* Weakref aliases are always duplicated. */
+ if (lookup_attribute ("weakref", DECL_ATTRIBUTES (node->symbol.decl)))
+ return SYMBOL_DUPLICATE;
+
/* External declarations are external. */
if (DECL_EXTERNAL (node->symbol.decl))
return SYMBOL_EXTERNAL;
*************** get_symbol_class (symtab_node node)
*** 79,88 ****
else if (!cgraph (node)->analyzed)
return SYMBOL_EXTERNAL;
- /* Weakref aliases are always duplicated. */
- if (lookup_attribute ("weakref", DECL_ATTRIBUTES (node->symbol.decl)))
- return SYMBOL_DUPLICATE;
-
/* Comdats are duplicated to every use unless they are keyed.
Those do not need duplication. */
if (DECL_COMDAT (node->symbol.decl)
--- 83,88 ----
*************** lto_balanced_map (void)
*** 561,567 ****
last_visited_node++;
! gcc_assert (node->analyzed);
/* Compute boundary cost of callgraph edges. */
for (edge = node->callees; edge; edge = edge->next_callee)
--- 561,568 ----
last_visited_node++;
! gcc_assert (node->analyzed
! || lookup_attribute ("weakref", DECL_ATTRIBUTES
(node->symbol.decl)));
/* Compute boundary cost of callgraph edges. */
for (edge = node->callees; edge; edge = edge->next_callee)
*************** privatize_symbol_name (symtab_node node)
*** 768,774 ****
{
tree decl = node->symbol.decl;
const char *name = IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl));
- char *label;
/* Our renaming machinery do not handle more than one change of assembler
name.
We should not need more than one anyway. */
--- 769,774 ----
*************** privatize_symbol_name (symtab_node node)
*** 793,799 ****
name);
return;
}
- ASM_FORMAT_PRIVATE_NAME (label, name, DECL_UID (decl));
change_decl_assembler_name (decl, clone_function_name (decl, "lto_priv"));
if (node->symbol.lto_file_data)
lto_record_renamed_decl (node->symbol.lto_file_data, name,
--- 793,798 ----
*************** rename_statics (lto_symtab_encoder_t enc
*** 869,875 ****
once this is fixed. */
|| DECL_EXTERNAL (node->symbol.decl)
|| !symtab_real_symbol_p (node))
! && !may_need_named_section_p (encoder, node))
return;
/* Now walk symbols sharing the same name and see if there are any
conflicts.
--- 868,875 ----
once this is fixed. */
|| DECL_EXTERNAL (node->symbol.decl)
|| !symtab_real_symbol_p (node))
! && !may_need_named_section_p (encoder, node)
! && !lookup_attribute ("weakref", DECL_ATTRIBUTES (node->symbol.decl)))
return;
/* Now walk symbols sharing the same name and see if there are any
conflicts.
*************** rename_statics (lto_symtab_encoder_t enc
*** 894,902 ****
/* Assign every symbol in the set that shares the same ASM name an unique
mangled name. */
for (s = symtab_node_for_asm (name); s;)
! if (!s->symbol.externally_visible
&& ((symtab_real_symbol_p (s)
! && !DECL_EXTERNAL (node->symbol.decl)
&& !TREE_PUBLIC (node->symbol.decl))
|| may_need_named_section_p (encoder, s))
&& (!encoder
--- 894,904 ----
/* Assign every symbol in the set that shares the same ASM name an unique
mangled name. */
for (s = symtab_node_for_asm (name); s;)
! if ((!s->symbol.externally_visible
! || lookup_attribute ("weakref", DECL_ATTRIBUTES (node->symbol.decl)))
&& ((symtab_real_symbol_p (s)
! && (!DECL_EXTERNAL (node->symbol.decl)
! || lookup_attribute ("weakref", DECL_ATTRIBUTES
(node->symbol.decl)))
&& !TREE_PUBLIC (node->symbol.decl))
|| may_need_named_section_p (encoder, s))
&& (!encoder
Index: varpool.c
===================================================================
*** varpool.c (revision 198824)
--- varpool.c (working copy)
*************** dump_varpool_node (FILE *f, struct varpo
*** 86,91 ****
--- 86,95 ----
fprintf (f, " finalized");
if (node->output)
fprintf (f, " output");
+ if (TREE_READONLY (node->symbol.decl))
+ fprintf (f, " read-only");
+ if (const_value_known_p (node->symbol.decl))
+ fprintf (f, " const-value-known");
fprintf (f, "\n");
}
