[binutils-gdb] [PR ld/22263][PR ld/24676] RISC-V: Avoid spurious R_RISCV_NONE for TLS GD/IE.

[binutils-gdb] [PR ld/22263][PR ld/24676] RISC-V: Avoid spurious R_RISCV_NONE for TLS GD/IE.

[Nelson Chu]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=20ef84ed2abb990da08d90e1c978f96d29f40606

commit 20ef84ed2abb990da08d90e1c978f96d29f40606
Author: Nelson Chu <nelson@rivosinc.com>
Date:   Fri May 26 18:05:34 2023 +0800

    [PR ld/22263][PR ld/24676] RISC-V: Avoid spurious R_RISCV_NONE for TLS GD/IE.
    
    For TLS GD/IE, add the same condition with the relocate_section in the
    allocate_dynrelocs, to make sure we won't reserve redundant spaces
    for dynamic relocations since the conservative estimatation.
    
    After applying this patch, ld seems no longer generate the spurious
    R_RISCV_NONE for pr22263-1 test, and the test in pr24676.
    
    bfd/
            PR ld/22263
            PR ld/24676
            * elfnn-riscv.c (RISCV_TLS_GD_IE_NEED_DYN_RELOC): New defined.
            Set NEED_RELOC to true if TLS GD/IE needs dynamic relocations,
            and INDX will be the dynamic index.
            (allocate_dynrelocs): Don't reserve extra spaces in the rela.got
            if RISCV_TLS_GD_IE_NEED_DYN_RELOC set need_reloc to false.  This
            condition needs to be same as relocate_section.
            (relocate_section): Likewise, use the same condition as
            allocate_dynrelocs.

Diff:
---
 bfd/elfnn-riscv.c | 41 ++++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index 762ea231c0b..30d2faa405d 100644
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -111,6 +111,25 @@
    || (bfd_link_pic (INFO) \
        && SYMBOL_REFERENCES_LOCAL ((INFO), (H))))
 
+/* Set NEED_RELOC to true if TLS GD/IE needs dynamic relocations, and INDX will
+   be the dynamic index.  PR22263, use the same check in allocate_dynrelocs and
+   riscv_elf_relocate_section for TLS GD/IE.  */
+#define RISCV_TLS_GD_IE_NEED_DYN_RELOC(INFO, DYN, H, INDX, NEED_RELOC) \
+  do \
+    { \
+      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; \
+      if ((bfd_link_dll (INFO) || (INDX) != 0) \
+	  && ((H) == NULL \
+	      || ELF_ST_VISIBILITY ((H)->other) == STV_DEFAULT \
+	      || (H)->root.type != bfd_link_hash_undefweak)) \
+	(NEED_RELOC) = true; \
+    } \
+  while (0)
+
 /* Internal relocations used exclusively by the relaxation pass.  */
 #define R_RISCV_DELETE (R_RISCV_max + 1)
 
@@ -1297,18 +1316,24 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
       dyn = htab->elf.dynamic_sections_created;
       if (tls_type & (GOT_TLS_GD | GOT_TLS_IE))
 	{
+	  int indx = 0;
+	  bool need_reloc = false;
+	  RISCV_TLS_GD_IE_NEED_DYN_RELOC(info, dyn, h, indx, need_reloc);
+
 	  /* TLS_GD needs two dynamic relocs and two GOT slots.  */
 	  if (tls_type & GOT_TLS_GD)
 	    {
 	      s->size += 2 * RISCV_ELF_WORD_BYTES;
-	      htab->elf.srelgot->size += 2 * sizeof (ElfNN_External_Rela);
+	      if (need_reloc)
+		htab->elf.srelgot->size += 2 * sizeof (ElfNN_External_Rela);
 	    }
 
 	  /* TLS_IE needs one dynamic reloc and one GOT slot.  */
 	  if (tls_type & GOT_TLS_IE)
 	    {
 	      s->size += RISCV_ELF_WORD_BYTES;
-	      htab->elf.srelgot->size += sizeof (ElfNN_External_Rela);
+	      if (need_reloc)
+		htab->elf.srelgot->size += sizeof (ElfNN_External_Rela);
 	    }
 	}
       else
@@ -2882,20 +2907,10 @@ riscv_elf_relocate_section (bfd *output_bfd,
 		abort ();
 
 	      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;
+	      RISCV_TLS_GD_IE_NEED_DYN_RELOC (info, dyn, h, indx, need_relocs);
 
 	      /* The GOT entries have not been initialized yet.  Do it
 		 now, and emit any relocations.  */
-	      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;
-
 	      if (tls_type & GOT_TLS_GD)
 		{
 		  if (need_relocs)