This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] ld: Add _bfd_elf_link_hide_sym_by_version
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Binutils <binutils at sourceware dot org>, Alan Modra <amodra at gmail dot com>, Nick Clifton <nickc at redhat dot com>
- Date: Thu, 24 May 2018 14:13:42 -0700
- Subject: Re: [PATCH] ld: Add _bfd_elf_link_hide_sym_by_version
- References: <20180521150606.GA13319@intel.com>
On Mon, May 21, 2018 at 8:06 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> bfd_hide_sym_by_version can't be used to check if a versioned symbol is
> hidden. This patch adds _bfd_elf_link_hide_sym_by_version to support
> both versioned and unversioned symbols by extracting versioned symbol
> check from _bfd_elf_link_assign_sym_version.
>
> OK for master?
>
> H.J.
> ---
> bfd/
>
> PR ld/23194
> * elf-bfd.h (_bfd_elf_link_hide_sym_by_version): New.
> * elflink.c (_bfd_elf_link_hide_versioned_symbol): New function.
> Extracted from _bfd_elf_link_assign_sym_version.
> (_bfd_elf_link_hide_sym_by_version): New function.
> (_bfd_elf_link_assign_sym_version): Use
> _bfd_elf_link_hide_versioned_symbol.
> * elfxx-x86.c (_bfd_x86_elf_link_symbol_references_local): Call
> _bfd_elf_link_hide_sym_by_version instead of
> bfd_hide_sym_by_version. Don't check unversioned symbol.
>
> ld/
>
> PR ld/23194
> * testsuite/ld-i386/pr23194.d: Expect only R_386_GLOB_DAT
> against foobar.
> * testsuite/ld-i386/pr23194.map: Add foobar.
> * testsuite/ld-x86-64/pr23194.map: Likewise.
> * testsuite/ld-i386/pr23194.s: Add a common foobar symbol.
> * testsuite/ld-x86-64/pr23194.s: Likewise.
> * testsuite/ld-x86-64/pr23194.d: Expect only R_X86_64_GLOB_DAT
> against foobar.
> ---
> bfd/elf-bfd.h | 3 +
> bfd/elflink.c | 164 +++++++++++++++++++++--------
> bfd/elfxx-x86.c | 11 +-
> ld/testsuite/ld-i386/pr23194.d | 2 +-
> ld/testsuite/ld-i386/pr23194.map | 1 +
> ld/testsuite/ld-i386/pr23194.s | 3 +-
> ld/testsuite/ld-x86-64/pr23194.d | 2 +-
> ld/testsuite/ld-x86-64/pr23194.map | 1 +
> ld/testsuite/ld-x86-64/pr23194.s | 3 +-
> 9 files changed, 134 insertions(+), 56 deletions(-)
>
> diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
> index 7b746900ba..ad958f3c86 100644
> --- a/bfd/elf-bfd.h
> +++ b/bfd/elf-bfd.h
> @@ -2184,6 +2184,9 @@ extern const struct bfd_elf_special_section *_bfd_elf_get_special_section
> extern const struct bfd_elf_special_section *_bfd_elf_get_sec_type_attr
> (bfd *, asection *);
>
> +extern bfd_boolean _bfd_elf_link_hide_sym_by_version
> + (struct bfd_link_info *, struct elf_link_hash_entry *);
> +
> /* If the target doesn't have reloc handling written yet: */
> extern bfd_boolean _bfd_elf_no_info_to_howto
> (bfd *, arelent *, Elf_Internal_Rela *);
> diff --git a/bfd/elflink.c b/bfd/elflink.c
> index ce3765e45b..734f8f88ad 100644
> --- a/bfd/elflink.c
> +++ b/bfd/elflink.c
> @@ -2221,6 +2221,115 @@ _bfd_elf_link_find_version_dependencies (struct elf_link_hash_entry *h,
> return TRUE;
> }
>
> +/* Return TRUE and set *HIDE to TRUE if the versioned symbol is
> + hidden. Set *T_P to NULL if there is no match. */
> +
> +static bfd_boolean
> +_bfd_elf_link_hide_versioned_symbol (struct bfd_link_info *info,
> + struct elf_link_hash_entry *h,
> + const char *version_p,
> + struct bfd_elf_version_tree **t_p,
> + bfd_boolean *hide)
> +{
> + struct bfd_elf_version_tree *t;
> +
> + /* Look for the version. If we find it, it is no longer weak. */
> + for (t = info->version_info; t != NULL; t = t->next)
> + {
> + if (strcmp (t->name, version_p) == 0)
> + {
> + size_t len;
> + char *alc;
> + struct bfd_elf_version_expr *d;
> +
> + len = version_p - h->root.root.string;
> + alc = (char *) bfd_malloc (len);
> + if (alc == NULL)
> + return FALSE;
> + memcpy (alc, h->root.root.string, len - 1);
> + alc[len - 1] = '\0';
> + if (alc[len - 2] == ELF_VER_CHR)
> + alc[len - 2] = '\0';
> +
> + h->verinfo.vertree = t;
> + t->used = TRUE;
> + d = NULL;
> +
> + if (t->globals.list != NULL)
> + d = (*t->match) (&t->globals, NULL, alc);
> +
> + /* See if there is anything to force this symbol to
> + local scope. */
> + if (d == NULL && t->locals.list != NULL)
> + {
> + d = (*t->match) (&t->locals, NULL, alc);
> + if (d != NULL
> + && h->dynindx != -1
> + && ! info->export_dynamic)
> + *hide = TRUE;
> + }
> +
> + free (alc);
> + break;
> + }
> + }
> +
> + *t_p = t;
> +
> + return TRUE;
> +}
> +
> +/* Return TRUE if the symbol H is hidden by version script. */
> +
> +bfd_boolean
> +_bfd_elf_link_hide_sym_by_version (struct bfd_link_info *info,
> + struct elf_link_hash_entry *h)
> +{
> + const char *p;
> + bfd_boolean hide = FALSE;
> + const struct elf_backend_data *bed
> + = get_elf_backend_data (info->output_bfd);
> +
> + /* Version script only hides symbols defined in regular objects. */
> + if (!h->def_regular && !ELF_COMMON_DEF_P (h))
> + return TRUE;
> +
> + p = strchr (h->root.root.string, ELF_VER_CHR);
> + if (p != NULL && h->verinfo.vertree == NULL)
> + {
> + struct bfd_elf_version_tree *t;
> +
> + ++p;
> + if (*p == ELF_VER_CHR)
> + ++p;
> +
> + if (*p != '\0'
> + && _bfd_elf_link_hide_versioned_symbol (info, h, p, &t, &hide)
> + && hide)
> + {
> + if (hide)
> + (*bed->elf_backend_hide_symbol) (info, h, TRUE);
> + return TRUE;
> + }
> + }
> +
> + /* If we don't have a version for this symbol, see if we can find
> + something. */
> + if (h->verinfo.vertree == NULL && info->version_info != NULL)
> + {
> + h->verinfo.vertree
> + = bfd_find_version_for_sym (info->version_info,
> + h->root.root.string, &hide);
> + if (h->verinfo.vertree != NULL && hide)
> + {
> + (*bed->elf_backend_hide_symbol) (info, h, TRUE);
> + return TRUE;
> + }
> + }
> +
> + return FALSE;
> +}
> +
> /* Figure out appropriate versions for all the symbols. We may not
> have the version number script until we have read all of the input
> files, so until that point we don't know which symbols should be
> @@ -2234,6 +2343,7 @@ _bfd_elf_link_assign_sym_version (struct elf_link_hash_entry *h, void *data)
> const struct elf_backend_data *bed;
> struct elf_info_failed eif;
> char *p;
> + bfd_boolean hide;
>
> sinfo = (struct elf_info_failed *) data;
> info = sinfo->info;
> @@ -2253,6 +2363,7 @@ _bfd_elf_link_assign_sym_version (struct elf_link_hash_entry *h, void *data)
> if (!h->def_regular)
> return TRUE;
>
> + hide = FALSE;
> bed = get_elf_backend_data (info->output_bfd);
> p = strchr (h->root.root.string, ELF_VER_CHR);
> if (p != NULL && h->verinfo.vertree == NULL)
> @@ -2267,50 +2378,15 @@ _bfd_elf_link_assign_sym_version (struct elf_link_hash_entry *h, void *data)
> if (*p == '\0')
> return TRUE;
>
> - /* Look for the version. If we find it, it is no longer weak. */
> - for (t = sinfo->info->version_info; t != NULL; t = t->next)
> + if (!_bfd_elf_link_hide_versioned_symbol (info, h, p, &t, &hide))
> {
> - if (strcmp (t->name, p) == 0)
> - {
> - size_t len;
> - char *alc;
> - struct bfd_elf_version_expr *d;
> -
> - len = p - h->root.root.string;
> - alc = (char *) bfd_malloc (len);
> - if (alc == NULL)
> - {
> - sinfo->failed = TRUE;
> - return FALSE;
> - }
> - memcpy (alc, h->root.root.string, len - 1);
> - alc[len - 1] = '\0';
> - if (alc[len - 2] == ELF_VER_CHR)
> - alc[len - 2] = '\0';
> -
> - h->verinfo.vertree = t;
> - t->used = TRUE;
> - d = NULL;
> -
> - if (t->globals.list != NULL)
> - d = (*t->match) (&t->globals, NULL, alc);
> -
> - /* See if there is anything to force this symbol to
> - local scope. */
> - if (d == NULL && t->locals.list != NULL)
> - {
> - d = (*t->match) (&t->locals, NULL, alc);
> - if (d != NULL
> - && h->dynindx != -1
> - && ! info->export_dynamic)
> - (*bed->elf_backend_hide_symbol) (info, h, TRUE);
> - }
> -
> - free (alc);
> - break;
> - }
> + sinfo->failed = TRUE;
> + return FALSE;
> }
>
> + if (hide)
> + (*bed->elf_backend_hide_symbol) (info, h, TRUE);
> +
> /* If we are building an application, we need to create a
> version node for this version. */
> if (t == NULL && bfd_link_executable (info))
> @@ -2366,10 +2442,10 @@ _bfd_elf_link_assign_sym_version (struct elf_link_hash_entry *h, void *data)
>
> /* If we don't have a version for this symbol, see if we can find
> something. */
> - if (h->verinfo.vertree == NULL && sinfo->info->version_info != NULL)
> + if (!hide
> + && h->verinfo.vertree == NULL
> + && sinfo->info->version_info != NULL)
> {
> - bfd_boolean hide;
> -
> h->verinfo.vertree
> = bfd_find_version_for_sym (sinfo->info->version_info,
> h->root.root.string, &hide);
> diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c
> index 29d92d288a..cb2487eacd 100644
> --- a/bfd/elfxx-x86.c
> +++ b/bfd/elfxx-x86.c
> @@ -2038,21 +2038,16 @@ _bfd_x86_elf_link_symbol_references_local (struct bfd_link_info *info,
> && htab->interp == NULL)
> || info->dynamic_undefined_weak == 0))
> || ((h->def_regular || ELF_COMMON_DEF_P (h))
> - && h->versioned == unversioned
> && info->version_info != NULL
> - && bfd_hide_sym_by_version (info->version_info,
> - h->root.root.string)))
> + && _bfd_elf_link_hide_sym_by_version (info, h)))
> {
> eh->local_ref = 2;
> return TRUE;
> }
>
> /* Symbols created by HIDDEN and PROVIDE_HIDDEN assignments in linker
> - script aren't forced local here yet. bfd_hide_sym_by_version
> - can't be used to check if a versioned symbol is hidden. It has to
> - be syncd with _bfd_elf_link_assign_sym_version to get the correct
> - answer. */
> - if (!h->root.ldscript_def && h->versioned == unversioned)
> + script aren't forced local here yet. */
> + if (!h->root.ldscript_def)
> eh->local_ref = 1;
>
> return FALSE;
Any comments? I need _bfd_elf_link_hide_sym_by_version to check if
both versioned
and unversioned symbols are hidden by version script. If we don't
want it in elflink.c,
I can put it in elfxx-x86.c.
Thanks.
--
H.J.