On 2019-12-18 10:26 +0100, Jan Hubicka wrote:
> Hi,
> sorry I forgot to include cgraph and varpool changes in the patch.
>
> Index: varpool.c
> ===================================================================
> --- varpool.c (revision 279467)
> +++ varpool.c (working copy)
> @@ -539,8 +539,7 @@ varpool_node::assemble_aliases (void)
> {
> varpool_node *alias = dyn_cast <varpool_node *> (ref->referring);
> if (alias->symver)
> - do_assemble_symver (alias->decl,
> - DECL_ASSEMBLER_NAME (decl));
> + do_assemble_symver (alias->decl, decl);
> else if (!alias->transparent_alias)
> do_assemble_alias (alias->decl,
> DECL_ASSEMBLER_NAME (decl));
> Index: cgraphunit.c
> ===================================================================
> --- cgraphunit.c (revision 279467)
> +++ cgraphunit.c (working copy)
> @@ -2222,8 +2222,7 @@ cgraph_node::assemble_thunks_and_aliases
> of buffering it in same alias pairs. */
> TREE_ASM_WRITTEN (decl) = 1;
> if (alias->symver)
> - do_assemble_symver (alias->decl,
> - DECL_ASSEMBLER_NAME (decl));
> + do_assemble_symver (alias->decl, decl);
> else
> do_assemble_alias (alias->decl,
> DECL_ASSEMBLER_NAME (decl));
> Index: varasm.c
> ===================================================================
> --- varasm.c (revision 279467)
> +++ varasm.c (working copy)
> @@ -5970,9 +5970,47 @@ do_assemble_symver (tree decl, tree targ
> ultimate_transparent_alias_target (&id);
> ultimate_transparent_alias_target (&target);
ICE here.
lto1: internal compiler error: tree check: expected identifier_node, have
function_decl in ultimate_transparent_alias_target, at varasm.c:1308
0x6f9cfe tree_check_failed(tree_node const*, char const*, int, char const*, ...)
../../gcc/gcc/tree.c:9685
0x714541 tree_check(tree_node*, char const*, int, char const*, tree_code)
../../gcc/gcc/tree.h:3273
0x714541 ultimate_transparent_alias_target
../../gcc/gcc/varasm.c:1308
0x714541 do_assemble_symver(tree_node*, tree_node*)
../../gcc/gcc/varasm.c:5971
> #ifdef ASM_OUTPUT_SYMVER_DIRECTIVE
> - ASM_OUTPUT_SYMVER_DIRECTIVE (asm_out_file,
> - IDENTIFIER_POINTER (target),
> - IDENTIFIER_POINTER (id));
> + if (TREE_PUBLIC (target) && DECL_VISIBILITY (target) == VISIBILITY_DEFAULT)
> + ASM_OUTPUT_SYMVER_DIRECTIVE (asm_out_file,
> + IDENTIFIER_POINTER
> + (DECL_ASSEMBLER_NAME (target)),
> + IDENTIFIER_POINTER (id));
> + else
> + {
> + int nameend;
> + for (nameend = 0; IDENTIFIER_POINTER (id)[nameend] != '@'; nameend++)
> + ;
> + if (IDENTIFIER_POINTER (id)[nameend + 1] != '@'
> + || IDENTIFIER_POINTER (id)[nameend + 2] == '@')
> + {
> + sorry_at (DECL_SOURCE_LOCATION (target),
> + "can not produce %<symver%> of a symbol that is "
> + "not exported with default visibility");
> + return;
I think this does not make sense. Some library authors may export "foo@VER_1"
but not "foo_v1" to ensure the programmers using the library upgrade their code
to use new "correct" ABI, instead of an old one. This error makes it
impossible.
(Try to comment out "foo_v1" in version.map, in the testcase.)
> + }
> + tree tmpdecl = copy_node (decl);
> + char buf[256];
> + static int symver_labelno;
> + targetm.asm_out.generate_internal_label (buf,
> + "LSYMVER", symver_labelno++);
> + SET_DECL_ASSEMBLER_NAME (tmpdecl, get_identifier (buf));
> + globalize_decl (tmpdecl);
> +#ifdef ASM_OUTPUT_DEF_FROM_DECLS
> + ASM_OUTPUT_DEF_FROM_DECLS (asm_out_file, tmpdecl,
> + DECL_ASSEMBLER_NAME (target));
> +#else
> + ASM_OUTPUT_DEF (asm_out_file,
> + IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (tmpdecl)),
> + IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (target)));
> +#endif
> + memcpy (buf, IDENTIFIER_POINTER (id), nameend + 2);
> + buf[nameend + 2] = '@';
> + strcpy (buf + nameend + 3, IDENTIFIER_POINTER (id) + nameend + 2);
We can't replace a single "@" with "@@@". So I think producing .LSYMVERx is not
an option for "old" versions like "foo@VER_1".
> + ASM_OUTPUT_SYMVER_DIRECTIVE (asm_out_file,
> + IDENTIFIER_POINTER
> + (DECL_ASSEMBLER_NAME (tmpdecl)),
> + buf);
> + }
> #else
> error ("symver is only supported on ELF platforms");
> #endif
> Index: lto/lto-common.c
> ===================================================================
> --- lto/lto-common.c (revision 279467)
> +++ lto/lto-common.c (working copy)
> @@ -2818,6 +2818,10 @@ read_cgraph_and_symbols (unsigned nfiles
> IDENTIFIER_POINTER
> (DECL_ASSEMBLER_NAME (snode->decl)));
> }
> + /* Symbol versions are always used externally, but linker does not
> + report that correctly. */
> + else if (snode->symver && *res == LDPR_PREVAILING_DEF_IRONLY)
> + snode->resolution = LDPR_PREVAILING_DEF_IRONLY_EXP;
This is absolutely correct.
> else
> snode->resolution = *res;
> }
I still believe we should consider symver targets to be externally visible in
cgraph_externally_visible_p. There is a comment saying "if linker counts on us,
we must preserve the function". That's true in our case.
And, I think
.globl .LSYMVER0
.set .LSYMVER0, foo_v2
.symver .LSYMVER0, foo@@VERS_2
is exactly same as
.globl foo_v2
.symver foo_v2, foo@@VERS_2
except there is an unnecessary ".LSYMVER0". Adding ".globl foo_v2" or ".globl
foo_v1" won't cause them to be "global" in the final DSO because the linker will
hide them according to the version script.
So if it's safe we can force a ".globl foo_v2" before ".symver foo_v2,
foo@@VERS_2". But I can't prove it's safe so I think it's better to consider
this case in cgraph_externally_visible_p.
--
Xi Ruoyao <xry...@mengyan1223.wang>
School of Aerospace Science and Technology, Xidian University
