[PATCH 3/5] LoongArch: Make protected function symbols local for -shared
Fangrui Song
i@maskray.me
Sat Jun 22 17:37:02 GMT 2024
On Sat, Jun 22, 2024 at 3:03 AM Xi Ruoyao <xry111@xry111.site> wrote:
>
> On LoongArch there is no reason to treat STV_PROTECTED STT_FUNC symbols
> as preemptable. See the comment above LARCH_REF_LOCAL for detailed
> explanation.
>
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
> bfd/elfnn-loongarch.c | 78 ++++++++++++++-----
> ld/testsuite/ld-loongarch-elf/ifunc-reloc.d | 2 +-
> .../ld-loongarch-elf/ld-loongarch-elf.exp | 1 +
> .../ld-loongarch-elf/protected-func.d | 6 ++
> .../ld-loongarch-elf/protected-func.s | 17 ++++
> 5 files changed, 83 insertions(+), 21 deletions(-)
> create mode 100644 ld/testsuite/ld-loongarch-elf/protected-func.d
> create mode 100644 ld/testsuite/ld-loongarch-elf/protected-func.s
>
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index e0b3e9f5ee6..2acb5c7fa95 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -181,6 +181,46 @@ struct loongarch_elf_link_hash_table
> } \
> while (0)
>
> +/* TL;DR always use it in this file instead when you want to type
> + SYMBOL_REFERENCES_LOCAL.
> +
> + It's like SYMBOL_REFERENCES_LOCAL, but it returns true for local
> + protected functions. It happens to be same as SYMBOL_CALLS_LOCAL but
> + let's not reuse SYMBOL_CALLS_LOCAL or "CALLS" may puzzle people.
> +
> + We do generate the so-called "canonical PLT entry" when someone attempts
> + to la.pcrel an external function. But this is only an unwanted side-
> + effect from using R_LARCH_PCALA_{HI20,LO12} for medium and extreme
> + code model function call: we have the same problem as i386 where
> + R_386_PC32 is used for both call and lea (for medium code model new code
> + should use R_LARCH_CALL36 instead), but unlike i386 we never really
> + implemented "R_LARCH_COPY" thus attempting to la.pcrel an external
> + symbol is always considered a programming error unless it's a part of
> + a extreme code model function call, and pointer equality will be broken
> + even with a STV_DEFAULT function:
> +
> + $ cat t.c
> + #include <assert.h>
> + void check(void *p) {assert(p == check);}
> + $ cat main.c
> + extern void check(void *);
> + int main(void) { check(check); }
> + $ cc t.c -fPIC -shared -o t.so
> + $ cc main.c -mdirect-extern-access t.so -Wl,-rpath=.
> + $ ./a.out
> + a.out: t.c:2: check: Assertion `p == check' failed.
> + Aborted
> +
> + Thus handling STV_PROTECTED function specially just fixes nothing:
> + adding -fvisibility=protected compiling t.c will not magically fix
> + the inequality. The only possible and correct fix is not to use
> + -mdirect-extern-access.
> +
> + So we should remove this special handling, because it's only an
> + unsuccessful workaround for invalid code and it's penalizing valid
> + code. */
> +#define LARCH_REF_LOCAL(info, h) \
> + (_bfd_elf_symbol_refs_local_p ((h), (info), true))
>
> /* Generate a PLT header. */
>
> @@ -712,7 +752,7 @@ loongarch_tls_transition_without_check (struct bfd_link_info *info,
> struct elf_link_hash_entry *h)
> {
> bool local_exec = bfd_link_executable (info)
> - && SYMBOL_REFERENCES_LOCAL (info, h);
> + && LARCH_REF_LOCAL (info, h);
>
> switch (r_type)
> {
> @@ -1196,7 +1236,7 @@ loongarch_elf_adjust_dynamic_symbol (struct bfd_link_info *info,
> {
> if (h->plt.refcount <= 0
> || (h->type != STT_GNU_IFUNC
> - && (SYMBOL_REFERENCES_LOCAL (info, h)
> + && (LARCH_REF_LOCAL (info, h)
> || (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT
> && h->root.type == bfd_link_hash_undefweak))))
> {
> @@ -1714,14 +1754,14 @@ elfNN_allocate_ifunc_dynrelocs (struct elf_link_hash_entry *h,
> here if it is defined and referenced in a non-shared object. */
> if (h->type == STT_GNU_IFUNC && h->def_regular)
> {
> - if (ref_local && SYMBOL_REFERENCES_LOCAL (info, h))
> + if (ref_local && LARCH_REF_LOCAL (info, h))
> return local_allocate_ifunc_dyn_relocs (info, h,
> &h->dyn_relocs,
> PLT_ENTRY_SIZE,
> PLT_HEADER_SIZE,
> GOT_ENTRY_SIZE,
> false);
> - else if (!ref_local && !SYMBOL_REFERENCES_LOCAL (info, h))
> + else if (!ref_local && !LARCH_REF_LOCAL (info, h))
> return _bfd_elf_allocate_ifunc_dyn_relocs (info, h,
> &h->dyn_relocs,
> PLT_ENTRY_SIZE,
> @@ -1749,7 +1789,6 @@ elfNN_allocate_ifunc_dynrelocs_ref_global (struct elf_link_hash_entry *h,
> false);
> }
>
> -
> /* Allocate space in .plt, .got and associated reloc sections for
> ifunc dynamic relocs. */
>
> @@ -2662,7 +2701,6 @@ tlsoff (struct bfd_link_info *info, bfd_vma addr)
> return addr - elf_hash_table (info)->tls_sec->vma;
> }
>
> -
> static int
> loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
> bfd *input_bfd, asection *input_section,
> @@ -2788,7 +2826,7 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
> {
> defined_local = !unresolved_reloc && !ignored;
> resolved_local =
> - defined_local && SYMBOL_REFERENCES_LOCAL (info, h);
> + defined_local && LARCH_REF_LOCAL (info, h);
> resolved_dynly = !resolved_local;
> resolved_to_const = !resolved_local && !resolved_dynly;
> }
> @@ -2877,7 +2915,7 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
> outrel.r_addend = 0;
> }
>
> - if (SYMBOL_REFERENCES_LOCAL (info, h))
> + if (LARCH_REF_LOCAL (info, h))
> {
>
> if (htab->elf.splt != NULL)
> @@ -3246,7 +3284,7 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
> if (!WILL_CALL_FINISH_DYNAMIC_SYMBOL (is_dyn,
> bfd_link_pic (info), h)
> && ((bfd_link_pic (info)
> - && SYMBOL_REFERENCES_LOCAL (info, h))))
> + && LARCH_REF_LOCAL (info, h))))
> {
> /* This is actually a static link, or it is a
> -Bsymbolic link and the symbol is defined
> @@ -3391,7 +3429,7 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
> asection *srel = htab->elf.srelgot;
> bfd_vma tls_block_off = 0;
>
> - if (SYMBOL_REFERENCES_LOCAL (info, h))
> + if (LARCH_REF_LOCAL (info, h))
> {
> BFD_ASSERT (elf_hash_table (info)->tls_sec);
> tls_block_off = relocation
> @@ -3402,7 +3440,7 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
> {
> rela.r_offset = sec_addr (got) + got_off;
> rela.r_addend = 0;
> - if (SYMBOL_REFERENCES_LOCAL (info, h))
> + if (LARCH_REF_LOCAL (info, h))
> {
> /* Local sym, used in exec, set module id 1. */
> if (bfd_link_executable (info))
> @@ -3435,7 +3473,7 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
> if (tls_type & GOT_TLS_IE)
> {
> rela.r_offset = sec_addr (got) + got_off + ie_off;
> - if (SYMBOL_REFERENCES_LOCAL (info, h))
> + if (LARCH_REF_LOCAL (info, h))
> {
> /* Local sym, used in exec, set module id 1. */
> if (!bfd_link_executable (info))
> @@ -3637,7 +3675,7 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
> bfd_link_pic (info),
> h)
> && bfd_link_pic (info)
> - && SYMBOL_REFERENCES_LOCAL (info, h))
> + && LARCH_REF_LOCAL (info, h))
> {
> Elf_Internal_Rela rela;
> rela.r_offset = sec_addr (got) + got_off;
> @@ -4178,7 +4216,7 @@ loongarch_tls_perform_trans (bfd *abfd, asection *sec,
> {
> unsigned long insn;
> bool local_exec = bfd_link_executable (info)
> - && SYMBOL_REFERENCES_LOCAL (info, h);
> + && LARCH_REF_LOCAL (info, h);
> bfd_byte *contents = elf_section_data (sec)->this_hdr.contents;
> unsigned long r_type = ELFNN_R_TYPE (rel->r_info);
> unsigned long r_symndx = ELFNN_R_SYM (rel->r_info);
> @@ -4890,7 +4928,7 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
> else
> continue;
>
> - if (h && SYMBOL_REFERENCES_LOCAL (info, h))
> + if (h && LARCH_REF_LOCAL (info, h))
> local_got = true;
> symtype = h->type;
> }
> @@ -5027,12 +5065,12 @@ loongarch_elf_finish_dynamic_symbol (bfd *output_bfd,
> if (htab->elf.splt)
> {
> BFD_ASSERT ((h->type == STT_GNU_IFUNC
> - && SYMBOL_REFERENCES_LOCAL (info, h))
> + && LARCH_REF_LOCAL (info, h))
> || h->dynindx != -1);
>
> plt = htab->elf.splt;
> gotplt = htab->elf.sgotplt;
> - if (h->type == STT_GNU_IFUNC && SYMBOL_REFERENCES_LOCAL (info, h))
> + if (h->type == STT_GNU_IFUNC && LARCH_REF_LOCAL (info, h))
> relplt = htab->elf.srelgot;
> else
> relplt = htab->elf.srelplt;
> @@ -5043,7 +5081,7 @@ loongarch_elf_finish_dynamic_symbol (bfd *output_bfd,
> else /* if (htab->elf.iplt) */
> {
> BFD_ASSERT (h->type == STT_GNU_IFUNC
> - && SYMBOL_REFERENCES_LOCAL (info, h));
> + && LARCH_REF_LOCAL (info, h));
>
> plt = htab->elf.iplt;
> gotplt = htab->elf.igotplt;
> @@ -5131,7 +5169,7 @@ loongarch_elf_finish_dynamic_symbol (bfd *output_bfd,
> if (htab->elf.splt == NULL)
> srela = htab->elf.irelplt;
>
> - if (SYMBOL_REFERENCES_LOCAL (info, h))
> + if (LARCH_REF_LOCAL (info, h))
> {
> asection *sec = h->root.u.def.section;
> rela.r_info = ELFNN_R_INFO (0, R_LARCH_IRELATIVE);
> @@ -5168,7 +5206,7 @@ loongarch_elf_finish_dynamic_symbol (bfd *output_bfd,
> return true;
> }
> }
> - else if (bfd_link_pic (info) && SYMBOL_REFERENCES_LOCAL (info, h))
> + else if (bfd_link_pic (info) && LARCH_REF_LOCAL (info, h))
> {
> asection *sec = h->root.u.def.section;
> rela.r_info = ELFNN_R_INFO (0, R_LARCH_RELATIVE);
> diff --git a/ld/testsuite/ld-loongarch-elf/ifunc-reloc.d b/ld/testsuite/ld-loongarch-elf/ifunc-reloc.d
> index cb592874b1e..968e7564b49 100644
> --- a/ld/testsuite/ld-loongarch-elf/ifunc-reloc.d
> +++ b/ld/testsuite/ld-loongarch-elf/ifunc-reloc.d
> @@ -8,6 +8,7 @@
> .* R_LARCH_IRELATIVE .*
> .* R_LARCH_IRELATIVE .*
> .* R_LARCH_IRELATIVE .*
> +.* R_LARCH_IRELATIVE .*
> #...
> .*'\.rela\.plt'.*
> #...
> @@ -16,4 +17,3 @@
> .* R_LARCH_JUMP_SLOT .*
> .* R_LARCH_JUMP_SLOT .*
> .* R_LARCH_JUMP_SLOT .*
> -.* R_LARCH_JUMP_SLOT .*
> diff --git a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
> index b8721f0eaff..3172a0ab4b5 100644
> --- a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
> +++ b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
> @@ -141,6 +141,7 @@ if [istarget "loongarch64-*-*"] {
> run_dump_test "reloc_abs_with_shared"
> run_dump_test "r_larch_32_elf64"
> run_dump_test "ifunc-reloc"
> + run_dump_test "protected-func"
> }
>
> if [check_pie_support] {
> diff --git a/ld/testsuite/ld-loongarch-elf/protected-func.d b/ld/testsuite/ld-loongarch-elf/protected-func.d
> new file mode 100644
> index 00000000000..501c7cb5f8c
> --- /dev/null
> +++ b/ld/testsuite/ld-loongarch-elf/protected-func.d
> @@ -0,0 +1,6 @@
> +#ld: -shared
> +#readelf: -Wr
> +
> +#...
> +.* R_LARCH_RELATIVE .*
> +.* R_LARCH_RELATIVE .*
> diff --git a/ld/testsuite/ld-loongarch-elf/protected-func.s b/ld/testsuite/ld-loongarch-elf/protected-func.s
> new file mode 100644
> index 00000000000..347e371a777
> --- /dev/null
> +++ b/ld/testsuite/ld-loongarch-elf/protected-func.s
> @@ -0,0 +1,17 @@
> +# protected function should be unpreemptable and relocated with
LGTM.
unpreemptable => non-preemptible.
Regarding "preemptible" vs "preemptable": the former is used much more
than the latter.
> +# R_LARCH_RELATIVE in shared library, for both GOT and pointer data
> +
> +.globl x
> +.protected x
> +.type x, @function
> +x:
> + ret
> +
> +.globl _start
> +_start:
> + la.got $a0, x
> + ret
> +
> +.data
> +p:
> + .quad x
> --
> 2.45.2
>
More information about the Binutils
mailing list