| Message ID | 4e112e2d2afb5f579fac32caec3741f7e140e556.camel@mengyan1223.wang |
|---|---|
| State | New |
| Headers | show |
| Series | Fix symver attribute with LTO | expand |
> Hi, > with Jan's patch commited in r278878 we can use symver attribute for functions > and variables. The symver attribute is designed for replacing toplevel asm > statements containing ".symver" which may be removed by LTO. Unfortunately, > a quick test shown GCC still generates buggy so file with LTO and new symver > attribute. Thanks for looking into this. It was on my TODo list to actually convert some packages, so it is great you did that. > > Two issues: > > 1. The symver node in symtab is marked as PREVAILING_DEF_IRONLY (no EXP) and > then removed by LTO. This is however wrong - linker should not mark it as PREVAILING_DEF_IRONLY if it is used externally. What linker do you use? On my testcases this was working with GNU ld (GNU Binutils) 2.31.51.20181222 I could easily imagine that some linkers get it wrong which should be reported to bintuils bugzilla but it is also easy to work around as done in your patch. > 2. The actual function body implementing the symver-ed function is also marked > as PREVAILING_DEF_IRONLY and then removed or marked as local. So no ".globl" > directive is outputed for it. Here is the symver-ed function exported from the DSO (or is it set to have hidden attribute)? Again this was working for me, so it would be good to understand this issue. I was thinking to extend the patch to also use name@@@nodename syntax in case the target node is static. This also needs bit extra work since during LTO partitioning we can not bring such symbol hidden and need to introduce additional attribute. I can look into that tomorrow. Honza > > Both issue cause symbols with symver missing in DSO (with LTO enabled). > > I modified fuse-3.9.0 code to use new symver attribute and tried to build it > with GCC trunk and LTO. The result is a buggy DSO. With this patch applied, > fuse-3.9.0 can be built with LTO enabled and no problem. > > I'll test symver patch and this patch with more packages. > > Bootstrapped/regtested x86_64-linux. I'm not a maintainer. > > gcc/ChangeLog: > > 2019-12-17 Xi Ruoyao <xry111@mengyan1223.wang> > > * cgraph.h (symtab_node::used_from_object_file_p): Symver nodes are > part of DSO ABI so always used by non-LTO object files. > * ipa-visibility.c (cgraph_externally_visible_p): Functions with symver > attributes should always be visible. > > Index: gcc/cgraph.h > =================================================================== > --- gcc/cgraph.h (revision 279452) > +++ gcc/cgraph.h (working copy) > @@ -2682,7 +2682,7 @@ symtab_node::used_from_object_file_p (vo > { > if (!TREE_PUBLIC (decl) || DECL_EXTERNAL (decl)) > return false; > - if (resolution_used_from_other_file_p (resolution)) > + if (symver || resolution_used_from_other_file_p (resolution)) > return true; > return false; > } > Index: gcc/ipa-visibility.c > =================================================================== > --- gcc/ipa-visibility.c (revision 279452) > +++ gcc/ipa-visibility.c (working copy) > @@ -216,6 +216,8 @@ cgraph_externally_visible_p (struct cgra > return true; > if (lookup_attribute ("noipa", DECL_ATTRIBUTES (node->decl))) > return true; > + if (lookup_attribute ("symver", DECL_ATTRIBUTES (node->decl))) > + return true; > if (TARGET_DLLIMPORT_DECL_ATTRIBUTES > && lookup_attribute ("dllexport", > DECL_ATTRIBUTES (node->decl))) > -- > Xi Ruoyao <xry111@mengyan1223.wang> > School of Aerospace Science and Technology, Xidian University >
On 2019-12-17 09:32 +0100, Jan Hubicka wrote: > > Hi, > > with Jan's patch commited in r278878 we can use symver attribute for > > functions > > and variables. The symver attribute is designed for replacing toplevel asm > > statements containing ".symver" which may be removed by LTO. Unfortunately, > > a quick test shown GCC still generates buggy so file with LTO and new symver > > attribute. > > Thanks for looking into this. It was on my TODo list to actually > convert some packages, so it is great you did that. > > Two issues: > > > > 1. The symver node in symtab is marked as PREVAILING_DEF_IRONLY (no EXP) and > > then removed by LTO. > > This is however wrong - linker should not mark it as > PREVAILING_DEF_IRONLY if it is used externally. What linker do you use? > On my testcases this was working with > GNU ld (GNU Binutils) 2.31.51.20181222 > I could easily imagine that some linkers get it wrong which should be > reported to bintuils bugzilla but it is also easy to work around as done > in your patch. Hi Jan, I'm using GNU ld 2.33.1. I'll attach a testcase simplified from fuse-3.9 code. "local: *;" in the versioning script triggers the issue. Without it there would be no problem. > > 2. The actual function body implementing the symver-ed function is also > > marked > > as PREVAILING_DEF_IRONLY and then removed or marked as local. So no > > ".globl" > > directive is outputed for it. > > Here is the symver-ed function exported from the DSO (or is it set > to have hidden attribute)? > Again this was working for me, so it would be good to understand this > issue. It's also triggered by "local: *;". Untar the attachment and use "make" to build it, then "make show-dynamic-syms" to dump the dynamic symbol table. I believe (with 99% chance) you'll see only foo (VERS_1) and foo_v1 (because foo_v1 is marked as global in the version script). And foo (VERS_2) would be missing. With this patch foo (VERS_2) would show up. We can't mark "foo_v2" to be "global" because it should not be a part of DSO ABI. The other 1% chance would be a regression in Binutils.
> Hi Jan, > > I'm using GNU ld 2.33.1. > > I'll attach a testcase simplified from fuse-3.9 code. "local: *;" in the > versioning script triggers the issue. Without it there would be no problem. Thanks. You are right that I did not play with local:. Now I wonder what is the intended behaviour here. In resolution file I see: 1 foo.o 4 205 dc8dc21a4ac8d072 PREVAILING_DEF_IRONLY_EXP foo_v1 207 dc8dc21a4ac8d072 PREVAILING_DEF_IRONLY foo@VERS_1 216 dc8dc21a4ac8d072 PREVAILING_DEF_IRONLY foo_v2 218 dc8dc21a4ac8d072 PREVAILING_DEF_IRONLY foo@@VERS_2 If I link the DSO w/o -flto I get with objdump -T: 0000000000001100 g DF .text 0000000000000006 (VERS_1) foo 0000000000001100 g DF .text 0000000000000006 VERS_2 foo_v1 0000000000000000 g DO *ABS* 0000000000000000 VERS_1 VERS_1 0000000000001110 g DF .text 0000000000000006 VERS_2 foo 0000000000000000 g DO *ABS* 0000000000000000 VERS_2 VERS_2 So I think linker is right here that foo_v1 is exported. I would have expected PREVAILING_DEF_IRONLY_EXP for foo@VERS_1 and foo@@VERS_2 since that symbols do get exported even though they land in special way in the DSO symbol table. So I think meaingful behaviour would be 1) make linker plugin interface to not annotate symbol version symbols with any resolution at all, since they are "special" 2) make them PREVAILING_DEF_IRONLY_EXP/PREVAILING_DEF since they always get exported to non-LTO land. We could workaround that on GCC side but if you agree with this understanding I would fill in binutils PR. Also since we use resolution info at many places, I would simply add logic working around this at a time we read resolution rahter than on one of places we use it. Comparing to objedump -T 0000000000001100 g DF .text 0000000000000006 VERS_2 foo_v1 0000000000000000 g DO *ABS* 0000000000000000 VERS_1 VERS_1 0000000000000000 g DO *ABS* 0000000000000000 VERS_2 VERS_2 Why we also miss 0000000000001100 g DF .text 0000000000000006 (VERS_1) foo and what does it mean? I am not too happy about forcing GCC to keep foo_v2 exported when it is not. For example, calls to foo_v2 will then not be optimized as they could if GCC knew it is not used by non-LTO world (except for fact that symver is bound to it). Would it be equivalent to: 1) output foo_v2 local 2) producing static alias with local name (.L1) 3) do .symver .L1,foo@@@VERS_2 That is somewhat more systematic and would not lead to false visibilities. Honza > > > > 2. The actual function body implementing the symver-ed function is also > > > marked > > > as PREVAILING_DEF_IRONLY and then removed or marked as local. So no > > > ".globl" > > > directive is outputed for it. > > > > Here is the symver-ed function exported from the DSO (or is it set > > to have hidden attribute)? > > Again this was working for me, so it would be good to understand this > > issue. > > It's also triggered by "local: *;". > > Untar the attachment and use "make" to build it, then "make show-dynamic-syms" > to dump the dynamic symbol table. I believe (with 99% chance) you'll see only > foo (VERS_1) and foo_v1 (because foo_v1 is marked as global in the version > script). And foo (VERS_2) would be missing. With this patch foo (VERS_2) would > show up. > > We can't mark "foo_v2" to be "global" because it should not be a part of DSO > ABI. > > The other 1% chance would be a regression in Binutils. > -- > Xi Ruoyao <xry111@mengyan1223.wang> > School of Aerospace Science and Technology, Xidian University
> Would it be equivalent to: > 1) output foo_v2 local > 2) producing static alias with local name (.L1) > 3) do .symver .L1,foo@@@VERS_2 > That is somewhat more systematic and would not lead to false > visibilities. I spent some time playing with this. An in order to 1) be able to handle foo_v2 according to the resolution info (so it behaves like a regular symbol and can be called dirrectly, localized and optimized) 2) get intended objdump -T relocations 3) do not polute global symbol tables I ended up with the following codegen: .type foo_v2, @function foo_v2: .LFB1: .cfi_startproc movl $2, %eax ret .cfi_endproc .LFE1: .size foo_v2, .-foo_v2 .globl .LSYMVER0 .set .LSYMVER0,foo_v2 .symver .LSYMVER0, foo@@@VERS_2 This uses @@@ symver version of gas which seems to have odd semantics of requiring to be passed global symbol name which it then tkes away and produces foo@@VERS_2. So the nm outoutp of the ltrans unit is: 0000000000000000 T foo_v1 0000000000000010 t foo_v2 0000000000000000 T foo@VERS_1 0000000000000010 T foo@@VERS_2 So the difference to your patch is that foo_v2 is static which enables normal optimizations. Since additional symbol alias is produced this would also make it possible to attach multiple symver attributes with @@ string. Does somehting like this make sense to you? Modulo the obvious buffer overflow issue? Honza Index: lto/lto-common.c =================================================================== --- lto/lto-common.c (revision 279178) +++ 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; else snode->resolution = *res; } Index: varasm.c =================================================================== --- varasm.c (revision 279178) +++ 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); #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; + } + 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); + ASM_OUTPUT_SYMVER_DIRECTIVE (asm_out_file, + IDENTIFIER_POINTER + (DECL_ASSEMBLER_NAME (tmpdecl)), + buf); + } #else error ("symver is only supported on ELF platforms"); #endif
On 2019-12-17 18:47 +0100, Jan Hubicka wrote: > > Would it be equivalent to: > > 1) output foo_v2 local > > 2) producing static alias with local name (.L1) > > 3) do .symver .L1,foo@@@VERS_2 > > That is somewhat more systematic and would not lead to false > > visibilities. > > I spent some time playing with this. An in order to > 1) be able to handle foo_v2 according to the resolution info > (so it behaves like a regular symbol and can be called dirrectly, > localized and optimized) > 2) get intended objdump -T relocations > 3) do not polute global symbol tables > > I ended up with the following codegen: > > .type foo_v2, @function > foo_v2: > .LFB1: > .cfi_startproc > movl $2, %eax > ret > .cfi_endproc > .LFE1: > .size foo_v2, .-foo_v2 > .globl .LSYMVER0 > .set .LSYMVER0,foo_v2 > .symver .LSYMVER0, foo@@@VERS_2 > > This uses @@@ symver version of gas which seems to have odd semantics of > requiring to be passed global symbol name which it then tkes away and > produces foo@@VERS_2. > > So the nm outoutp of the ltrans unit is: > 0000000000000000 T foo_v1 > 0000000000000010 t foo_v2 > 0000000000000000 T foo@VERS_1 > 0000000000000010 T foo@@VERS_2 > > So the difference to your patch is that foo_v2 is static which enables > normal optimizations. > > Since additional symbol alias is produced this would also make it > possible to attach multiple symver attributes with @@ string. > > Does somehting like this make sense to you? Modulo the obvious buffer > overflow issue? > Honza Unfortunately, I got an ICE with my testcase with the patch applied to trunk. lto1: internal compiler error: tree check: expected tree that contains ‘decl minimal’ structure, have ‘identifier_node’ in do_assemble_symver, at varasm.c:5986 0x6fa648 tree_contains_struct_check_failed(tree_node const*, tree_node_structure_enum, char const*, int, char const*) ../../gcc/gcc/tree.c:9859 0x71466e contains_struct_check(tree_node*, tree_node_structure_enum, char const*, int, char const*) ../../gcc/gcc/tree.h:3387 0x71466e do_assemble_symver(tree_node*, tree_node*) ../../gcc/gcc/varasm.c:5986 0x89e409 cgraph_node::assemble_thunks_and_aliases() ../../gcc/gcc/cgraphunit.c:2225 0x89e698 cgraph_node::expand() ../../gcc/gcc/cgraphunit.c:2351 0x89f62f expand_all_functions ../../gcc/gcc/cgraphunit.c:2456 0x89f62f symbol_table::compile() ../../gcc/gcc/cgraphunit.c:2806 0x7fb589 lto_main() ../../gcc/gcc/lto/lto.c:658 Please submit a full bug report, with preprocessed source if appropriate. Please include the complete backtrace with any bug report. See <https://gcc.gnu.org/bugs/> for instructions. lto-wrapper: fatal error: /home/xry111/gcc-test/bin/gcc returned 1 exit status compilation terminated. /usr/bin/ld: error: lto-wrapper failed collect2: error: ld returned 1 exit status make: *** [Makefile:4: obj/test.so] Error 1 The change to lto/lto-common.c makes sense. I tried it instead of my change to cgraph.h and everything is OK. I'll investigate the change to varasm.c a little.
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);
#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;
+ }
+ 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);
+ 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;
else
snode->resolution = *res;
}
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.
> 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 Interesting that it works for me, but indeed we can remove that call since there is no way to do weakref of symbol version. > > > #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.) The problem here is that we lie to the compiler (by pretending that foo_v2 is exported from DSO while it is not) and force user to do the same. We support two ways to hide symbol - either at compile time via attribute((visibility("hidden"))) or at link-time via map file. The first produces better code because compiler can do more optimizations knowing that the symbol can not be interposed. Generally we want users to use visiblity attribute or -fvisibility since it leads to better code. However now we tell users to use attribute((symver("..."))) to produce symbol version, but at the same time not use attribute((visibility("hidden"))). > > + 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". I wonder why gas implements the .symver this way at first place. Does the linker really need the global symbol foo_v1 to produce the version (in addition to foo@VER_1 that is in symbol table as well)? > > + /* 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. Good, I will go ahead with filling in binutils PR on the wrong LDPR value and apply the hack. > > > 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 I produce .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. The difference is that in first case compiler can fully control foo_v2 symbol (with LTO it will turn it into static symbol, it will inline calls to it and do other things), while in the second case we need to treat foo_v2 specially. > > 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. It sort of makes things work, but for example it will prevent gcc from inlining calls to foo_v2. I think we will still need to do something about -fvisibility=hidden. It is sad that we do not have way to produce symbol version without a corresponding symbol of global visiblity. If we had we could support multiple symver aliases from one symbol and avoid the need to explicitly hide the unnecesary symbols in the map files... Honza
On 2019-12-18 14:19 +0100, Jan Hubicka wrote: > > 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 > > Interesting that it works for me, but indeed we can remove that call > since there is no way to do weakref of symbol version. > > > #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.) > > The problem here is that we lie to the compiler (by pretending that > foo_v2 is exported from DSO while it is not) and force user to do the > same. > > We support two ways to hide symbol - either at compile time via > attribute((visibility("hidden"))) or at link-time via map file. The > first produces better code because compiler can do more optimizations > knowing that the symbol can not be interposed. > > Generally we want users to use visiblity attribute or -fvisibility since > it leads to better code. However now we tell users to use > attribute((symver("..."))) to produce symbol version, but at the same > time not use attribute((visibility("hidden"))). Could a symver symbol be interposed? I'll do some test to see. > > > + 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". > > I wonder why gas implements the .symver this way at first place. Does > the linker really need the global symbol foo_v1 to produce the > version (in addition to foo@VER_1 that is in symbol table as well)? I don't think the global symbol foo_v1 is necessary. But I can't find a way telling gas to make foo@VER_1 global and foo_v1 local. > > > + /* 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. > > Good, I will go ahead with filling in binutils PR on the wrong LDPR > value and apply the hack. > > > 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 > I produce > .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. > > The difference is that in first case compiler can fully control foo_v2 > symbol (with LTO it will turn it into static symbol, it will inline > calls to it and do other things), while in the second case we need to > treat foo_v2 specially. > > 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. > > It sort of makes things work, but for example it will prevent gcc from > inlining calls to foo_v2. I think we will still need to do something > about -fvisibility=hidden. > > It is sad that we do not have way to produce symbol version without a > corresponding symbol of global visiblity. If we had we could support > multiple symver aliases from one symbol and avoid the need to explicitly > hide the unnecesary symbols in the map files... Explicitly hiding the unnecessary symbols in map files is now how we handle this [with __asm__(".symver foo_v2, foo@@VERS_2")]. We can continue to do in this way and leave it as an enhancement.
On 2019-12-18 14:19 +0100, Jan Hubicka wrote: > The problem here is that we lie to the compiler (by pretending that > foo_v2 is exported from DSO while it is not) and force user to do the > same. > > We support two ways to hide symbol - either at compile time via > attribute((visibility("hidden"))) or at link-time via map file. The > first produces better code because compiler can do more optimizations > knowing that the symbol can not be interposed. I just get your point: if the library calls foo_v2 it won't be interposed. If it supposes a call to be interposed it should call foo() [foo@@VER_2] instead of foo_v2(). But it seems there is no way we can do this [even with traditional __asm__("symver foo, foo@@VER_2")]. For this purpose we should either: 1. Change GAS (introducing some new syntax like '@@@@' or '.symver_export') or 2. Add some mangled symbol name in GCC (like ".LSYMVERx" or "foo_v2.symver_export").
> On 2019-12-18 14:19 +0100, Jan Hubicka wrote: > > The problem here is that we lie to the compiler (by pretending that > > foo_v2 is exported from DSO while it is not) and force user to do the > > same. > > > > We support two ways to hide symbol - either at compile time via > > attribute((visibility("hidden"))) or at link-time via map file. The > > first produces better code because compiler can do more optimizations > > knowing that the symbol can not be interposed. > > I just get your point: if the library calls foo_v2 it won't be interposed. If > it supposes a call to be interposed it should call foo() [foo@@VER_2] instead of > foo_v2(). > > But it seems there is no way we can do this [even with traditional > __asm__("symver foo, foo@@VER_2")]. For this purpose we should either: > > 1. Change GAS (introducing some new syntax like '@@@@' or '.symver_export') > > or > > 2. Add some mangled symbol name in GCC (like ".LSYMVERx" or > "foo_v2.symver_export"). I agree. The problem with mangled symbol names is that we will require users to hide them from map files, so I think it is not best answer either. I have filled binutils https://sourceware.org/bugzilla/show_bug.cgi?id=25295 This is variant of your patch I comitted. It also adds verification so we get ICE rather then wrong code. In addition I moved the checks away rom used_from_object_file. This function is about non-LTO objects linked into the DSO and thus does not really fit for the check. Lastly we can not rely on symver attribute to still be present here. Regtested x86_64-linux and comitted. Honza * cgraph.c (cgraph_node_cannot_be_local_p_1): Prevent targets of symver attributes to be localized. * ipa-visibility.c (cgraph_externally_visible_p, varpool_node::externally_visible_p): Likewise. * symtab.c (symtab_node::verify_base): Check visibility of symbol versions. * lto-common.c (read_cgraph_and_symbols): Work around binutils PR25424 Index: cgraph.c =================================================================== --- cgraph.c (revision 279523) +++ cgraph.c (working copy) @@ -2226,6 +2226,9 @@ cgraph_node_cannot_be_local_p_1 (cgraph_ { return !(!node->force_output && !node->ifunc_resolver + /* Limitation of gas requires us to output targets of symver aliases + as global symbols. This is binutils PR 25295. */ + && !node->symver && ((DECL_COMDAT (node->decl) && !node->forced_by_abi && !node->used_from_object_file_p () Index: ipa-visibility.c =================================================================== --- ipa-visibility.c (revision 279523) +++ ipa-visibility.c (working copy) @@ -220,6 +220,14 @@ cgraph_externally_visible_p (struct cgra && lookup_attribute ("dllexport", DECL_ATTRIBUTES (node->decl))) return true; + + /* Limitation of gas requires us to output targets of symver aliases as + global symbols. This is binutils PR 25295. */ + ipa_ref *ref; + FOR_EACH_ALIAS (node, ref) + if (ref->referring->symver) + return true; + if (node->resolution == LDPR_PREVAILING_DEF_IRONLY) return false; /* When doing LTO or whole program, we can bring COMDAT functoins static. @@ -284,14 +292,13 @@ varpool_node::externally_visible_p (void DECL_ATTRIBUTES (decl))) return true; - /* See if we have linker information about symbol not being used or - if we need to make guess based on the declaration. + /* Limitation of gas requires us to output targets of symver aliases as + global symbols. This is binutils PR 25295. */ + ipa_ref *ref; + FOR_EACH_ALIAS (this, ref) + if (ref->referring->symver) + return true; - Even if the linker clams the symbol is unused, never bring internal - symbols that are declared by user as used or externally visible. - This is needed for i.e. references from asm statements. */ - if (used_from_object_file_p ()) - return true; if (resolution == LDPR_PREVAILING_DEF_IRONLY) return false; Index: lto/lto-common.c =================================================================== --- lto/lto-common.c (revision 279523) +++ lto/lto-common.c (working copy) @@ -2818,6 +2818,11 @@ 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. + This is binutils PR25924. */ + else if (snode->symver && *res == LDPR_PREVAILING_DEF_IRONLY) + snode->resolution = LDPR_PREVAILING_DEF_IRONLY_EXP; else snode->resolution = *res; } Index: symtab.c =================================================================== --- symtab.c (revision 279523) +++ symtab.c (working copy) @@ -1156,6 +1156,22 @@ symtab_node::verify_base (void) error ("node is symver but not alias"); error_found = true; } + /* Limitation of gas requires us to output targets of symver aliases as + global symbols. This is binutils PR 25295. */ + if (symver + && (!TREE_PUBLIC (get_alias_target ()->decl) + || DECL_VISIBILITY (get_alias_target ()->decl) != VISIBILITY_DEFAULT)) + { + error ("symver target is not exported with default visibility"); + error_found = true; + } + if (symver + && (!TREE_PUBLIC (decl) + || DECL_VISIBILITY (decl) != VISIBILITY_DEFAULT)) + { + error ("symver is not exported with default visibility"); + error_found = true; + } if (same_comdat_group) { symtab_node *n = same_comdat_group;
On 2019-12-19 11:06 +0100, Jan Hubicka wrote: > This is variant of your patch I comitted. It also adds verification so > we get ICE rather then wrong code. In addition I moved the checks away > rom used_from_object_file. This function is about non-LTO objects > linked into the DSO and thus does not really fit for the check. > Lastly we can not rely on symver attribute to still be present here. > > Regtested x86_64-linux and comitted. > Honza > * cgraph.c (cgraph_node_cannot_be_local_p_1): Prevent targets of > symver attributes to be localized. > * ipa-visibility.c (cgraph_externally_visible_p, > varpool_node::externally_visible_p): Likewise. > * symtab.c (symtab_node::verify_base): Check visibility of symbol > versions. > > * lto-common.c (read_cgraph_and_symbols): Work around binutils > PR25424 /* snip */ > Index: ipa-visibility.c > =================================================================== > --- ipa-visibility.c (revision 279523) > +++ ipa-visibility.c (working copy) > @@ -220,6 +220,14 @@ cgraph_externally_visible_p (struct cgra > && lookup_attribute ("dllexport", > DECL_ATTRIBUTES (node->decl))) > return true; > + > + /* Limitation of gas requires us to output targets of symver aliases as > + global symbols. This is binutils PR 25295. */ > + ipa_ref *ref; > + FOR_EACH_ALIAS (node, ref) > + if (ref->referring->symver) > + return true; > + > if (node->resolution == LDPR_PREVAILING_DEF_IRONLY) > return false; > /* When doing LTO or whole program, we can bring COMDAT functoins static. > @@ -284,14 +292,13 @@ varpool_node::externally_visible_p (void > DECL_ATTRIBUTES (decl))) > return true; > > - /* See if we have linker information about symbol not being used or > - if we need to make guess based on the declaration. > + /* Limitation of gas requires us to output targets of symver aliases as > + global symbols. This is binutils PR 25295. */ > + ipa_ref *ref; > + FOR_EACH_ALIAS (this, ref) > + if (ref->referring->symver) > + return true; > > - Even if the linker clams the symbol is unused, never bring internal > - symbols that are declared by user as used or externally visible. > - This is needed for i.e. references from asm statements. */ > - if (used_from_object_file_p ()) > - return true; Are these two lines removed intentionally? > if (resolution == LDPR_PREVAILING_DEF_IRONLY) > return false; > > Index: lto/lto-common.c > ===================================================================
On 2019-12-19 19:12 +0800, Xi Ruoyao wrote: > On 2019-12-19 11:06 +0100, Jan Hubicka wrote: > > - /* See if we have linker information about symbol not being used or > > - if we need to make guess based on the declaration. > > + /* Limitation of gas requires us to output targets of symver aliases as > > + global symbols. This is binutils PR 25295. */ > > + ipa_ref *ref; > > + FOR_EACH_ALIAS (this, ref) > > + if (ref->referring->symver) > > + return true; > > > > - Even if the linker clams the symbol is unused, never bring internal > > - symbols that are declared by user as used or externally visible. > > - This is needed for i.e. references from asm statements. */ > > - if (used_from_object_file_p ()) > > - return true; > > Are these two lines removed intentionally? Oh I see, it was a duplicated branch. Sorry for noise.
> On 2019-12-19 19:12 +0800, Xi Ruoyao wrote: > > On 2019-12-19 11:06 +0100, Jan Hubicka wrote: > > > - /* See if we have linker information about symbol not being used or > > > - if we need to make guess based on the declaration. > > > + /* Limitation of gas requires us to output targets of symver aliases as > > > + global symbols. This is binutils PR 25295. */ > > > + ipa_ref *ref; > > > + FOR_EACH_ALIAS (this, ref) > > > + if (ref->referring->symver) > > > + return true; > > > > > > - Even if the linker clams the symbol is unused, never bring internal > > > - symbols that are declared by user as used or externally visible. > > > - This is needed for i.e. references from asm statements. */ > > > - if (used_from_object_file_p ()) > > > - return true; > > > > Are these two lines removed intentionally? > > Oh I see, it was a duplicated branch. > > Sorry for noise. I should have mentioned that in the changelog. Indeed that function was checked twice. Looking into it more, we still have issue since call foo_v1 tranlsates as 5: e8 00 00 00 00 callq a <foo@VERS_1+0xa> while gcc believes that really foo_v1 is called. So after symver i guess foo_v1 symbol becomes completely unreachable from the current unit which is not what gcc expect. Since we do not allow any references to the symbol and we hacked gcc to believe that symvered symbol is externaly visible, I suppose things are more or less safe, but it is not working at it should. It would probably be best if gas supported something like .symver_set with similar semantics of .set but creating a symbol version alias which woud not add the translation of all references from symbol to its version. Honza
On Wed, 2019-12-18 at 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)); [ ... ] Do you plan on committing these bits? jeff >
> On Wed, 2019-12-18 at 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)); > [ ... ] > Do you plan on committing these bits? I think we got into agreement that gas extension is needed, since there is name@@@nodename that does what we need for name@@nodename but there is nothing that would do it for name@nodename. I filled in GAS PR for that. Honza > > jeff > > >
Index: gcc/cgraph.h =================================================================== --- gcc/cgraph.h (revision 279452) +++ gcc/cgraph.h (working copy) @@ -2682,7 +2682,7 @@ symtab_node::used_from_object_file_p (vo { if (!TREE_PUBLIC (decl) || DECL_EXTERNAL (decl)) return false; - if (resolution_used_from_other_file_p (resolution)) + if (symver || resolution_used_from_other_file_p (resolution)) return true; return false; } Index: gcc/ipa-visibility.c =================================================================== --- gcc/ipa-visibility.c (revision 279452) +++ gcc/ipa-visibility.c (working copy) @@ -216,6 +216,8 @@ cgraph_externally_visible_p (struct cgra return true; if (lookup_attribute ("noipa", DECL_ATTRIBUTES (node->decl))) return true; + if (lookup_attribute ("symver", DECL_ATTRIBUTES (node->decl))) + return true; if (TARGET_DLLIMPORT_DECL_ATTRIBUTES && lookup_attribute ("dllexport", DECL_ATTRIBUTES (node->decl)))