[PR,ld/22263,PR,ld/25694] RISC-V: Avoid dynamic TLS relocs in PIE.
Checks
Commit Message
From: Nelson Chu <nelson@nelson.ba.rivosinc.com>
Lots of targets already fixed the TEXTREL problem for TLS in PIE.
* For PR ld/25694,
In the check_reloc, refer to spare and loongarch, they don't need to reserve
any local dynamic reloc for TLS LE in pie/pde, and similar to other targets.
So it seems like riscv was too conservative to estimate the TLS LE before.
Just break and don't goto static_reloc for TLS LE in pie/pde can fix the
TEXTREL problem.
* For PR ld/22263,
The risc-v code for TLS GD/IE in the relocate_section seems same as MIPS port.
So similar to MIPS, pr22570, commits 9143e72c6d4d and 1cb83cac9a89, it seems
also the right way to do the same thing for risc-v.
On risc-v, fixes
FAIL: Build pr22263-1
RISC-V haven't supported the TLS transitions, so will need the same fix (use
bfd_link_dll) in the future.
bfd/
PR ld/22263
PR ld/25694
* elfnn-riscv.c (riscv_elf_check_relocs): Replace bfd_link_pic with
bfd_link_dll for TLS IE. Don't need to reserve the local dynamic
relocation for TLS LE in pie/pde, and report error in pic just like
before.
(riscv_elf_relocate_section): For TLS GD/IE, use bfd_link_dll rather
than !bfd_link_pic in determining the dynamic symbol index. Avoid
the index of -1.
---
bfd/elfnn-riscv.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
Comments
Committed since it should fix the pr22263-1 test.
There is still a problem mentioned by Andreas Schwab in the pr25694, that
ld still generate the unexpected R_RISCV_NONE for the pr22263-1 test after
applying this patch. Thanks for the tips from Alan that we should have the
same condition checked for this in the relocate_section and
allocate_dynrelocs.
So here is the following fix for pr22263,
https://sourceware.org/pipermail/binutils/2023-May/127653.html
After applying the fix, it seems ld no longer generates the R_RISCV_NONE
reloc for TLS GD/IE with -pie in pr22263-1 test.
However, passing the riscv-gnu-toolchain regressions, so looks well so far.
Thanks
Nelson
On Thu, May 4, 2023 at 5:08 PM Nelson Chu <nelson@rivosinc.com> wrote:
> From: Nelson Chu <nelson@nelson.ba.rivosinc.com>
>
> Lots of targets already fixed the TEXTREL problem for TLS in PIE.
>
> * For PR ld/25694,
> In the check_reloc, refer to spare and loongarch, they don't need to
> reserve
> any local dynamic reloc for TLS LE in pie/pde, and similar to other
> targets.
> So it seems like riscv was too conservative to estimate the TLS LE before.
> Just break and don't goto static_reloc for TLS LE in pie/pde can fix the
> TEXTREL problem.
>
> * For PR ld/22263,
> The risc-v code for TLS GD/IE in the relocate_section seems same as MIPS
> port.
> So similar to MIPS, pr22570, commits 9143e72c6d4d and 1cb83cac9a89, it
> seems
> also the right way to do the same thing for risc-v.
>
> On risc-v, fixes
> FAIL: Build pr22263-1
>
> RISC-V haven't supported the TLS transitions, so will need the same fix
> (use
> bfd_link_dll) in the future.
>
> bfd/
> PR ld/22263
> PR ld/25694
> * elfnn-riscv.c (riscv_elf_check_relocs): Replace bfd_link_pic with
> bfd_link_dll for TLS IE. Don't need to reserve the local dynamic
> relocation for TLS LE in pie/pde, and report error in pic just like
> before.
> (riscv_elf_relocate_section): For TLS GD/IE, use bfd_link_dll
> rather
> than !bfd_link_pic in determining the dynamic symbol index. Avoid
> the index of -1.
> ---
> bfd/elfnn-riscv.c | 25 +++++++++++--------------
> 1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> index a23b91ac15c..52dbf55a9f7 100644
> --- a/bfd/elfnn-riscv.c
> +++ b/bfd/elfnn-riscv.c
> @@ -824,7 +824,7 @@ riscv_elf_check_relocs (bfd *abfd, struct
> bfd_link_info *info,
> break;
>
> case R_RISCV_TLS_GOT_HI20:
> - if (bfd_link_pic (info))
> + if (bfd_link_dll (info))
> info->flags |= DF_STATIC_TLS;
> if (!riscv_elf_record_got_reference (abfd, info, h, r_symndx)
> || !riscv_elf_record_tls_type (abfd, h, r_symndx,
> GOT_TLS_IE))
> @@ -920,11 +920,12 @@ riscv_elf_check_relocs (bfd *abfd, struct
> bfd_link_info *info,
> goto static_reloc;
>
> case R_RISCV_TPREL_HI20:
> + /* This is not allowed in the pic, but okay in pie. */
> if (!bfd_link_executable (info))
> return bad_static_reloc (abfd, r_type, h);
> if (h != NULL)
> riscv_elf_record_tls_type (abfd, h, r_symndx, GOT_TLS_LE);
> - goto static_reloc;
> + break;
>
> case R_RISCV_HI20:
> if (bfd_link_pic (info))
> @@ -2797,24 +2798,20 @@ riscv_elf_relocate_section (bfd *output_bfd,
> if (htab->elf.srelgot == NULL)
> abort ();
>
> - if (h != NULL)
> - {
> - bool dyn, pic;
> - dyn = htab->elf.dynamic_sections_created;
> - pic = bfd_link_pic (info);
> -
> - if (WILL_CALL_FINISH_DYNAMIC_SYMBOL (dyn, pic, h)
> - && (!pic || !SYMBOL_REFERENCES_LOCAL (info, h)))
> - indx = h->dynindx;
> - }
> + bool dyn = elf_hash_table (info)->dynamic_sections_created;
> + if (h != NULL
> + && h->dynindx != -1
> + && WILL_CALL_FINISH_DYNAMIC_SYMBOL (dyn, bfd_link_pic
> (info), h)
> + && (bfd_link_dll (info) || !SYMBOL_REFERENCES_LOCAL
> (info, h)))
> + indx = h->dynindx;
>
> /* The GOT entries have not been initialized yet. Do it
> now, and emit any relocations. */
> - if ((bfd_link_pic (info) || indx != 0)
> + if ((bfd_link_dll (info) || indx != 0)
> && (h == NULL
> || ELF_ST_VISIBILITY (h->other) == STV_DEFAULT
> || h->root.type != bfd_link_hash_undefweak))
> - need_relocs = true;
> + need_relocs = true;
>
> if (tls_type & GOT_TLS_GD)
> {
> --
> 2.39.2 (Apple Git-143)
>
>
@@ -824,7 +824,7 @@ riscv_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
break;
case R_RISCV_TLS_GOT_HI20:
- if (bfd_link_pic (info))
+ if (bfd_link_dll (info))
info->flags |= DF_STATIC_TLS;
if (!riscv_elf_record_got_reference (abfd, info, h, r_symndx)
|| !riscv_elf_record_tls_type (abfd, h, r_symndx, GOT_TLS_IE))
@@ -920,11 +920,12 @@ riscv_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
goto static_reloc;
case R_RISCV_TPREL_HI20:
+ /* This is not allowed in the pic, but okay in pie. */
if (!bfd_link_executable (info))
return bad_static_reloc (abfd, r_type, h);
if (h != NULL)
riscv_elf_record_tls_type (abfd, h, r_symndx, GOT_TLS_LE);
- goto static_reloc;
+ break;
case R_RISCV_HI20:
if (bfd_link_pic (info))
@@ -2797,24 +2798,20 @@ riscv_elf_relocate_section (bfd *output_bfd,
if (htab->elf.srelgot == NULL)
abort ();
- if (h != NULL)
- {
- bool dyn, pic;
- dyn = htab->elf.dynamic_sections_created;
- pic = bfd_link_pic (info);
-
- if (WILL_CALL_FINISH_DYNAMIC_SYMBOL (dyn, pic, h)
- && (!pic || !SYMBOL_REFERENCES_LOCAL (info, h)))
- indx = h->dynindx;
- }
+ bool dyn = elf_hash_table (info)->dynamic_sections_created;
+ if (h != NULL
+ && h->dynindx != -1
+ && WILL_CALL_FINISH_DYNAMIC_SYMBOL (dyn, bfd_link_pic (info), h)
+ && (bfd_link_dll (info) || !SYMBOL_REFERENCES_LOCAL (info, h)))
+ indx = h->dynindx;
/* The GOT entries have not been initialized yet. Do it
now, and emit any relocations. */
- if ((bfd_link_pic (info) || indx != 0)
+ if ((bfd_link_dll (info) || indx != 0)
&& (h == NULL
|| ELF_ST_VISIBILITY (h->other) == STV_DEFAULT
|| h->root.type != bfd_link_hash_undefweak))
- need_relocs = true;
+ need_relocs = true;
if (tls_type & GOT_TLS_GD)
{