[PATCH 3/5] LoongArch: Make protected function symbols local for -shared

mengqinggang mengqinggang@loongson.cn
Tue Jun 25 02:39:48 GMT 2024


在 2024/6/22 下午6:03, Xi Ruoyao 写道:
> 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


Extreme code model function call use GOT not PLT.

   10:   1a00000d        pcalau12i       $t1, 0
                         10: R_LARCH_GOT_PC_HI20 f
   14:   02c0000c        li.d            $t0, 0
                         14: R_LARCH_GOT_PC_LO12 f
   18:   1600000c        lu32i.d         $t0, 0
                         18: R_LARCH_GOT64_PC_LO20       f
   1c:   0300018c        lu52i.d         $t0, $t0, 0
                         1c: R_LARCH_GOT64_PC_HI12       f
   20:   380c31ac        ldx.d           $t0, $t1, $t0
   24:   4c000181        jirl            $ra, $t0, 0

> +   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
> +# 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



More information about the Binutils mailing list