> 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

Reply via email to