> 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  <xry...@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 <xry...@mengyan1223.wang>
> School of Aerospace Science and Technology, Xidian University
> 

Reply via email to