[PATCH] HPPA/PR16082 : Remove version information on hidden symbols

[PATCH] HPPA/PR16082 : Remove version information on hidden symbols

[Guy Martin]

[-- Attachment #1: Type: text/plain, Size: 413 bytes --]

Hi,


We've run into a bug on Gentoo/hppa when linking libdb.so :
https://bugs.gentoo.org/show_bug.cgi?id=466924

It turns out that the generated .gnu.version section is invalid
because the version information of _GLOBAL_OFFSET_TABLE_ from
libdb.so isn't removed when making the symbol local in
elf32_hppa_hide_symbol().

The attached patch fixes this. The check-ld test suite doesn't
show any regression.

   Guy

[-- Attachment #2: binutils-hppa_remove_version_when_local-v3.diff --]
[-- Type: text/plain, Size: 870 bytes --]

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 8977400..3923a7c 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,8 @@
+2013-10-24  Guy Martin <gmsoft@tuxicoman.be>
+
+	* elf32-hppa.c (elf32_hppa_hide_symbol): Remove old version
+	information when forcing a symbol to be local.
+
 2013-10-18  Hans-Peter Nilsson  <hp@axis.com>
 
 	* elf32-cris.c (cris_elf_check_relocs): Don't assume
diff --git a/bfd/elf32-hppa.c b/bfd/elf32-hppa.c
index dfffbcb..5da3b5a 100644
--- a/bfd/elf32-hppa.c
+++ b/bfd/elf32-hppa.c
@@ -1771,6 +1771,9 @@ elf32_hppa_hide_symbol (struct bfd_link_info *info,
 	  _bfd_elf_strtab_delref (elf_hash_table (info)->dynstr,
 				  eh->dynstr_index);
 	}
+      /* Remove version information from hidden symbol */
+      eh->verinfo.verdef = NULL;
+      eh->verinfo.vertree = NULL;
     }
 
   /* STT_GNU_IFUNC symbol must go through PLT.  */

Re: [PATCH] HPPA/PR16082 : Remove version information on hidden symbols

[Carlos O'Donell]

On 10/25/2013 03:54 AM, Guy Martin wrote:
> Hi,
> 
> 
> We've run into a bug on Gentoo/hppa when linking libdb.so :
> https://bugs.gentoo.org/show_bug.cgi?id=466924
> 
> It turns out that the generated .gnu.version section is invalid
> because the version information of _GLOBAL_OFFSET_TABLE_ from
> libdb.so isn't removed when making the symbol local in
> elf32_hppa_hide_symbol().
> 
> The attached patch fixes this. The check-ld test suite doesn't
> show any regression.

Is there an existing test in the testsuite that checks for this?

> diff --git a/bfd/ChangeLog b/bfd/ChangeLog
> index 8977400..3923a7c 100644
> --- a/bfd/ChangeLog
> +++ b/bfd/ChangeLog
> @@ -1,3 +1,8 @@
> +2013-10-24  Guy Martin <gmsoft@tuxicoman.be>
> +
> +	* elf32-hppa.c (elf32_hppa_hide_symbol): Remove old version
> +	information when forcing a symbol to be local.
> +

OK.

>  2013-10-18  Hans-Peter Nilsson  <hp@axis.com>
>  
>  	* elf32-cris.c (cris_elf_check_relocs): Don't assume
> diff --git a/bfd/elf32-hppa.c b/bfd/elf32-hppa.c
> index dfffbcb..5da3b5a 100644
> --- a/bfd/elf32-hppa.c
> +++ b/bfd/elf32-hppa.c
> @@ -1771,6 +1771,9 @@ elf32_hppa_hide_symbol (struct bfd_link_info *info,
>  	  _bfd_elf_strtab_delref (elf_hash_table (info)->dynstr,
>  				  eh->dynstr_index);
>  	}
> +      /* Remove version information from hidden symbol */
> +      eh->verinfo.verdef = NULL;
> +      eh->verinfo.vertree = NULL;

Why doesn't the generic elflink.c (_bfd_elf_link_hash_hide_symbol)
need to do this?

It could be that symbols we are about to hide never make it this
far in other targets and aren't even added to the list.

>      }
>  
>    /* STT_GNU_IFUNC symbol must go through PLT.  */

From a first principles perspective a hidden symbol will have
been converted to a local symbol and will never require version
information. I see no reason that verdef or vertree need be preserved.

There is quite a lot of code which uses verdef == NULL to validate
that the symbol has no version information so it seems valid to set
it to NULL here.

One is under the assumption here that whatever called
elf32_hppa_hide_symbol has already removed the version definition
from the appropriate hashes.

I would be OK with this patch if you can show that gcc (and a couple
of languages) and glibc all build without failure using the new
binutils.

Can you try using the new bintuils to build gcc and glibc?

Cheers,
Carlos.

Re: [PATCH] HPPA/PR16082 : Remove version information on hidden symbols

[Guy Martin]

Hi Carlos,

On Fri, 25 Oct 2013 10:15:37 -0400
"Carlos O'Donell" <carlos@redhat.com> wrote:
> > We've run into a bug on Gentoo/hppa when linking libdb.so :
> > https://bugs.gentoo.org/show_bug.cgi?id=466924
> > 
> > It turns out that the generated .gnu.version section is invalid
> > because the version information of _GLOBAL_OFFSET_TABLE_ from
> > libdb.so isn't removed when making the symbol local in
> > elf32_hppa_hide_symbol().
> > 
> > The attached patch fixes this. The check-ld test suite doesn't
> > show any regression.
> 
> Is there an existing test in the testsuite that checks for this?

I don't think so. It seems hppa is the only arch doing specific stuff
with the _GLOBAL_OFFSET_TABLE_.
> 
> Can you try using the new bintuils to build gcc and glibc?


I've rebuilt binutils itself, glibc-2.17 and gcc-4.6.3 as well as
gcc-4.8.1. There was no issue at all.

  Guy