[PATCH v3 1/2] [ld] Allow symbol visibility support for ld -r
Jan Beulich
jbeulich@suse.com
Fri Nov 7 11:29:08 GMT 2025
On 17.10.2025 19:50, Eyal Itkin wrote:
> Introduce the "--localize-hidden" CLI flag, which will
> be supported only for relocatables (ld -r).
>
> When provided, hidden symbols will be converted from
> STB_GLOBAL to STB_LOCAL when the relocatable is being
> built. This change aligns the ET_REL creation to support
> the same visibility semantics as that of the shared
> library (ET_DYN).
>
> This added support will enable users to use the
> "ld -r --localize-hidden" outputs, wrapped inside
> static (.a) archives, as an alternative format for the
> existing static libraries.
>
> The full discussion about the format is available in
> the generic-abi group that is responsible for the ELF
> standard. Specifically, the summarized architecture can
> be found here:
> https://groups.google.com/g/generic-abi/c/sT25-xfX9yc/m/J9SIthF4BgAJ
>
> Signed-off-by: Eyal Itkin <eyal.itkin@gmail.com>
> ---
> bfd/elf64-x86-64.c | 3 +-
> bfd/elflink.c | 42 ++++++++++++++++++-------
> include/bfdlink.h | 3 ++
> ld/ld.h | 3 ++
> ld/ldlex.h | 1 +
> ld/lexsup.c | 11 +++++++
> ld/testsuite/ld-elf/localize_hidden.map | 7 +++++
> ld/testsuite/ld-elf/localize_hidden.s | 19 +++++++++++
> ld/testsuite/ld-elf/localize_hidden1.d | 11 +++++++
> ld/testsuite/ld-elf/localize_hidden2.d | 11 +++++++
> ld/testsuite/ld-elf/localize_hidden3.d | 11 +++++++
> ld/testsuite/ld-elf/localize_hidden4.d | 3 ++
> 12 files changed, 112 insertions(+), 13 deletions(-)
> create mode 100644 ld/testsuite/ld-elf/localize_hidden.map
> create mode 100644 ld/testsuite/ld-elf/localize_hidden.s
> create mode 100644 ld/testsuite/ld-elf/localize_hidden1.d
> create mode 100644 ld/testsuite/ld-elf/localize_hidden2.d
> create mode 100644 ld/testsuite/ld-elf/localize_hidden3.d
> create mode 100644 ld/testsuite/ld-elf/localize_hidden4.d
>
> diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
> index 59b43149897..2c7e7c63357 100644
> --- a/bfd/elf64-x86-64.c
> +++ b/bfd/elf64-x86-64.c
> @@ -5409,7 +5409,8 @@ elf_x86_64_finish_dynamic_symbol (bfd *output_bfd,
> if (h->got.offset != (bfd_vma) -1
> && ! GOT_TLS_GD_ANY_P (elf_x86_hash_entry (h)->tls_type)
> && elf_x86_hash_entry (h)->tls_type != GOT_TLS_IE
> - && !local_undefweak)
> + && !local_undefweak
> + && !bfd_link_relocatable (info))
> {
> Elf_Internal_Rela rela;
> asection *relgot = htab->elf.srelgot;
Why would such a change be necessary, and only for x86-64?
> --- a/bfd/elflink.c
> +++ b/bfd/elflink.c
> @@ -803,9 +803,9 @@ bfd_elf_record_link_assignment (bfd *output_bfd,
> (*bed->elf_backend_hide_symbol) (info, h, true);
> }
>
> - /* STV_HIDDEN and STV_INTERNAL symbols must be STB_LOCAL in shared objects
> - and executables. */
> - if (!bfd_link_relocatable (info)
> + /* STV_HIDDEN and STV_INTERNAL symbols must be STB_LOCAL if hidden
> + symbols were marked for localization. */
> + if (info->localize_hidden
> && h->dynindx != -1
> && (ELF_ST_VISIBILITY (h->other) == STV_HIDDEN
> || ELF_ST_VISIBILITY (h->other) == STV_INTERNAL))
Where did the bfd_link_relocatable() check go? Similarly the old contents
of the comment wants amending, not replacing, as it looks.
> @@ -3151,7 +3151,10 @@ _bfd_elf_fix_symbol_flags (struct elf_link_hash_entry *h,
> h->def_regular = 1;
> }
>
> - /* Backend specific symbol fixup. */
> + /* Backend specific symbol fixup, only needed for dynamic case. */
> + if (elf_hash_table (eif->info)->dynobj == NULL)
> + return true;
> +
> bed = get_elf_backend_data (elf_hash_table (eif->info)->dynobj);
> if (bed->elf_backend_fixup_symbol
> && !(*bed->elf_backend_fixup_symbol) (eif->info, h))
How is "only needed for dynamic case" justified? Is it entirely excluded
that some arch may need to play with some of the symbols to satisfy the
"localize hidden" request?
> @@ -3197,7 +3200,7 @@ _bfd_elf_fix_symbol_flags (struct elf_link_hash_entry *h,
> visibility. If the symbol has hidden or internal visibility, we
> will force it local. */
> else if (h->needs_plt
> - && bfd_link_pic (eif->info)
> + && eif->info->localize_hidden
> && is_elf_hash_table (eif->info->hash)
> && (SYMBOLIC_BIND (eif->info, h)
> || ELF_ST_VISIBILITY (h->other) != STV_DEFAULT)
Again replacement of an earlier check without it becoming clear why that's
a correct thing to do. (More of this pattern further down, where I won't
repeat the comment.)
> @@ -5680,9 +5683,9 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
> goto error_free_vers;
> }
> }
> - else if (h->dynindx != -1)
> - /* If the symbol already has a dynamic index, but
> - visibility says it should not be visible, turn it into
> + else if (info->localize_hidden
> + && (h->dynindx != -1 || bfd_link_relocatable (info)))
> + /* If visibility says it should not be visible, turn it into
> a local symbol. */
> switch (ELF_ST_VISIBILITY (h->other))
> {
When ->localize_hidden only applies to relocatable links, how can this be
a correct change (i.e. not affecting final linking)?
> @@ -6925,6 +6928,7 @@ bfd_elf_size_dynamic_sections (bfd *output_bfd,
> {
> bfd *dynobj;
> const struct elf_backend_data *bed;
> + struct elf_info_failed asvinfo;
Please have this ...
> @@ -6948,10 +6952,22 @@ bfd_elf_size_dynamic_sections (bfd *output_bfd,
>
> dynobj = elf_hash_table (info)->dynobj;
>
> - if (dynobj != NULL && elf_hash_table (info)->dynamic_sections_created)
> + /* Symbol visibility should be applied in some non-dynamic cases. */
> + if (bfd_link_relocatable (info) && info->localize_hidden)
> + {
> + /* Attach all the symbols to their version information. */
> + asvinfo.info = info;
> + asvinfo.failed = false;
> +
> + elf_link_hash_traverse (elf_hash_table (info),
> + _bfd_elf_link_assign_sym_version,
> + &asvinfo);
> + if (asvinfo.failed)
> + return false;
> + }
... in just the scope it's needed in. The assignments then can become the
initializer of the variable (an initializer for .failed actually isn't needed
then).
> --- a/include/bfdlink.h
> +++ b/include/bfdlink.h
> @@ -486,6 +486,9 @@ struct bfd_link_info
> /* TRUE if multiple definitions should only warn. */
> unsigned int warn_multiple_definition: 1;
>
> + /* TRUE if supports localizing hidden symbols. */
> + unsigned int localize_hidden: 1;
I think it is unhelpful if two fields (this and ...
> --- a/ld/ld.h
> +++ b/ld/ld.h
> @@ -332,6 +332,9 @@ typedef struct
>
> /* Compress DWARF debug sections. */
> enum compressed_debug_section_type compress_debug;
> +
> + /* If set, localize hidden symbols. */
> + bool localize_hidden;
... this) have exactly the same name, but different purpose. Albeit - the
former being in struct bfd_link_info: Why does the comment say "supports"?
At the same time I can't spot any use of this latter field.
> --- a/ld/lexsup.c
> +++ b/ld/lexsup.c
> @@ -208,6 +208,8 @@ static const struct ld_option ld_options[] =
> 'q', NULL, "Generate relocations in final output", TWO_DASHES },
> { {"relocatable", no_argument, NULL, 'r'},
> 'r', NULL, N_("Generate relocatable output"), TWO_DASHES },
> + { {"localize-hidden", no_argument, NULL, OPTION_LOCALIZE_HIDDEN},
> + '\0', NULL, N_("Localize hidden relocatable symbols"), TWO_DASHES },
> { {NULL, no_argument, NULL, '\0'},
> 'i', NULL, NULL, ONE_DASH },
> { {"just-symbols", required_argument, NULL, 'R'},
> @@ -1237,6 +1239,7 @@ parse_args (unsigned argc, char **argv)
> bfd_link_dll (&link_info) ? "-shared" : "-pie");
>
> link_info.type = type_relocatable;
> + link_info.localize_hidden = false;
> config.build_constructors = false;
> config.magic_demand_paged = false;
> config.text_read_only = false;
> @@ -1347,6 +1350,7 @@ parse_args (unsigned argc, char **argv)
> "-shared");
>
> link_info.type = type_dll;
> + link_info.localize_hidden = true;
Why would this be? Didn't you say this is for relocatable linking only?
> @@ -1367,6 +1377,7 @@ parse_args (unsigned argc, char **argv)
> fatal (_("%P: -r and %s may not be used together\n"), "-pie");
>
> link_info.type = type_pie;
> + link_info.localize_hidden = true;
Same here, plus (if really you mean to set the flag for final linking as
well): What about non-PIE executables?
Jan
More information about the Binutils
mailing list