public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] ELF: Don't check relocations in non-loaded, non-alloced sections
@ 2020-06-02 20:38 H.J. Lu
  2020-06-03  0:05 ` Fangrui Song
       [not found] ` <MWHPR12MB14567E24AED5430AAE2497A8CB880@MWHPR12MB1456.namprd12.prod.outlook.com>
  0 siblings, 2 replies; 10+ messages in thread
From: H.J. Lu @ 2020-06-02 20:38 UTC (permalink / raw)
  To: binutils

Don't do anything special with non-loaded, non-alloced sections.
In particular, any relocs in such sections should not affect GOT
and PLT reference counting (ie. we don't allow them to create GOT
or PLT entries), there's no possibility or desire to optimize TLS
relocs, and there's not much point in propagating relocs to shared
libs that the dynamic linker won't relocate.

I will check it in if there are no objections.

H.J.
---
	* elf32-i386.c (elf_i386_check_relocs): Remove SEC_ALLOC check.
	* elf32-lm32.c (lm32_elf_check_relocs): Likewise.
	* elf32-m32r.c (m32r_elf_check_relocs): Likewise.
	* elf32-nds32.c (nds32_elf_check_relocs): Likewise.
	* elf32-or1k.c (or1k_elf_check_relocs): Likewise.
	* elf32-ppc.c (ppc_elf_check_relocs): Likewise.
	* elf32-sh.c (sh_elf_check_relocs): Likewise.
	* elf32-xtensa.c (elf_xtensa_check_relocs): Likewise.
	* elf64-alpha.c (elf64_alpha_check_relocs): Likewise.
	* elf64-ppc.c (ppc64_elf_check_relocs): Likewise.
	* elf64-x86-64.c (elf_x86_64_check_relocs): Likewise.
	* elflink.c (_bfd_elf_link_check_relocs): Skip non-loaded,
	non-alloced sections.
---
 bfd/elf32-i386.c   |  9 ---------
 bfd/elf32-lm32.c   |  9 ---------
 bfd/elf32-m32r.c   |  9 ---------
 bfd/elf32-nds32.c  |  9 ---------
 bfd/elf32-or1k.c   |  9 ---------
 bfd/elf32-ppc.c    |  9 ---------
 bfd/elf32-sh.c     |  9 ---------
 bfd/elf32-xtensa.c |  2 +-
 bfd/elf64-alpha.c  |  9 ---------
 bfd/elf64-ppc.c    |  9 ---------
 bfd/elf64-x86-64.c |  9 ---------
 bfd/elflink.c      | 12 ++++++++++--
 12 files changed, 11 insertions(+), 93 deletions(-)

diff --git a/bfd/elf32-i386.c b/bfd/elf32-i386.c
index 544b931552..f6f669957c 100644
--- a/bfd/elf32-i386.c
+++ b/bfd/elf32-i386.c
@@ -1487,15 +1487,6 @@ elf_i386_check_relocs (bfd *abfd,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   htab = elf_x86_hash_table (info, I386_ELF_DATA);
   if (htab == NULL)
     {
diff --git a/bfd/elf32-lm32.c b/bfd/elf32-lm32.c
index 5d09f2d350..acef37af5d 100644
--- a/bfd/elf32-lm32.c
+++ b/bfd/elf32-lm32.c
@@ -1128,15 +1128,6 @@ lm32_elf_check_relocs (bfd *abfd,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
   sym_hashes = elf_sym_hashes (abfd);
   sym_hashes_end = sym_hashes + symtab_hdr->sh_size/sizeof (Elf32_External_Sym);
diff --git a/bfd/elf32-m32r.c b/bfd/elf32-m32r.c
index afe0ee899c..9b8e5cd124 100644
--- a/bfd/elf32-m32r.c
+++ b/bfd/elf32-m32r.c
@@ -3450,15 +3450,6 @@ m32r_elf_check_relocs (bfd *abfd,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   sreloc = NULL;
   symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
   sym_hashes = elf_sym_hashes (abfd);
diff --git a/bfd/elf32-nds32.c b/bfd/elf32-nds32.c
index 4f7ea76469..3e094f6270 100644
--- a/bfd/elf32-nds32.c
+++ b/bfd/elf32-nds32.c
@@ -7081,15 +7081,6 @@ nds32_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
       return TRUE;
     }
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
   sym_hashes = elf_sym_hashes (abfd);
   sym_hashes_end =
diff --git a/bfd/elf32-or1k.c b/bfd/elf32-or1k.c
index b141b45886..3ed44ccceb 100644
--- a/bfd/elf32-or1k.c
+++ b/bfd/elf32-or1k.c
@@ -1880,15 +1880,6 @@ or1k_elf_check_relocs (bfd *abfd,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
   sym_hashes = elf_sym_hashes (abfd);
 
diff --git a/bfd/elf32-ppc.c b/bfd/elf32-ppc.c
index 588b79781d..62c6270329 100644
--- a/bfd/elf32-ppc.c
+++ b/bfd/elf32-ppc.c
@@ -2888,15 +2888,6 @@ ppc_elf_check_relocs (bfd *abfd,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
 #ifdef DEBUG
   _bfd_error_handler ("ppc_elf_check_relocs called for section %pA in %pB",
 		      sec, abfd);
diff --git a/bfd/elf32-sh.c b/bfd/elf32-sh.c
index 0428829757..7c9e695981 100644
--- a/bfd/elf32-sh.c
+++ b/bfd/elf32-sh.c
@@ -5376,15 +5376,6 @@ sh_elf_check_relocs (bfd *abfd, struct bfd_link_info *info, asection *sec,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   BFD_ASSERT (is_sh_elf (abfd));
 
   symtab_hdr = &elf_symtab_hdr (abfd);
diff --git a/bfd/elf32-xtensa.c b/bfd/elf32-xtensa.c
index 05c4f8430a..9dc815edbb 100644
--- a/bfd/elf32-xtensa.c
+++ b/bfd/elf32-xtensa.c
@@ -1039,7 +1039,7 @@ elf_xtensa_check_relocs (bfd *abfd,
   const Elf_Internal_Rela *rel;
   const Elf_Internal_Rela *rel_end;
 
-  if (bfd_link_relocatable (info) || (sec->flags & SEC_ALLOC) == 0)
+  if (bfd_link_relocatable (info))
     return TRUE;
 
   BFD_ASSERT (is_xtensa_elf (abfd));
diff --git a/bfd/elf64-alpha.c b/bfd/elf64-alpha.c
index 4e4efae0b1..0b31d450dc 100644
--- a/bfd/elf64-alpha.c
+++ b/bfd/elf64-alpha.c
@@ -1782,15 +1782,6 @@ elf64_alpha_check_relocs (bfd *abfd, struct bfd_link_info *info,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   BFD_ASSERT (is_alpha_elf (abfd));
 
   dynobj = elf_hash_table (info)->dynobj;
diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c
index 49fda96be7..3941addd57 100644
--- a/bfd/elf64-ppc.c
+++ b/bfd/elf64-ppc.c
@@ -4536,15 +4536,6 @@ ppc64_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   BFD_ASSERT (is_ppc64_elf (abfd));
 
   htab = ppc_hash_table (info);
diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
index 183c808346..eada0e53ed 100644
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -1862,15 +1862,6 @@ elf_x86_64_check_relocs (bfd *abfd, struct bfd_link_info *info,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   htab = elf_x86_hash_table (info, X86_64_ELF_DATA);
   if (htab == NULL)
     {
diff --git a/bfd/elflink.c b/bfd/elflink.c
index a2b40ccb04..7cee0afac0 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -3956,8 +3956,16 @@ _bfd_elf_link_check_relocs (bfd *abfd, struct bfd_link_info *info)
 	  Elf_Internal_Rela *internal_relocs;
 	  bfd_boolean ok;
 
-	  /* Don't check relocations in excluded sections.  */
-	  if ((o->flags & SEC_RELOC) == 0
+	  /* Don't check relocations in excluded sections.  Don't do
+	     anything special with non-loaded, non-alloced sections.
+	     In particular, any relocs in such sections should not
+	     affect GOT and PLT reference counting (ie.  we don't
+	     allow them to create GOT or PLT entries), there's no
+	     possibility or desire to optimize TLS relocs, and
+	     there's not much point in propagating relocs to shared
+	     libs that the dynamic linker won't relocate.  */
+	  if ((o->flags & SEC_ALLOC) == 0
+	      || (o->flags & SEC_RELOC) == 0
 	      || (o->flags & SEC_EXCLUDE) != 0
 	      || o->reloc_count == 0
 	      || ((info->strip == strip_all || info->strip == strip_debugger)
-- 
2.26.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ELF: Don't check relocations in non-loaded, non-alloced sections
  2020-06-02 20:38 [PATCH] ELF: Don't check relocations in non-loaded, non-alloced sections H.J. Lu
@ 2020-06-03  0:05 ` Fangrui Song
       [not found] ` <MWHPR12MB14567E24AED5430AAE2497A8CB880@MWHPR12MB1456.namprd12.prod.outlook.com>
  1 sibling, 0 replies; 10+ messages in thread
From: Fangrui Song @ 2020-06-03  0:05 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

I haven't really read the patch, but just wanted to express a point.
It is sometimes useful to check relocations even for a non-SHF_ALLOC
section. For example, a PC relative relocation type does not make
sense from a non-SHF_ALLOC section referencing a SHF_ALLOC section.

Conceptually, even if a non-SHF_ALLOC is loaded as part of the memory
image, the distance between it and a SHF_ALLOC section may not be a
constant, so the linker cannot reasonably resolve the relocation.

On Tue, Jun 2, 2020 at 1:38 PM H.J. Lu via Binutils
<binutils@sourceware.org> wrote:
>
> Don't do anything special with non-loaded, non-alloced sections.
> In particular, any relocs in such sections should not affect GOT
> and PLT reference counting (ie. we don't allow them to create GOT
> or PLT entries), there's no possibility or desire to optimize TLS
> relocs, and there's not much point in propagating relocs to shared
> libs that the dynamic linker won't relocate.
>
> I will check it in if there are no objections.
>
> H.J.
> ---
>         * elf32-i386.c (elf_i386_check_relocs): Remove SEC_ALLOC check.
>         * elf32-lm32.c (lm32_elf_check_relocs): Likewise.
>         * elf32-m32r.c (m32r_elf_check_relocs): Likewise.
>         * elf32-nds32.c (nds32_elf_check_relocs): Likewise.
>         * elf32-or1k.c (or1k_elf_check_relocs): Likewise.
>         * elf32-ppc.c (ppc_elf_check_relocs): Likewise.
>         * elf32-sh.c (sh_elf_check_relocs): Likewise.
>         * elf32-xtensa.c (elf_xtensa_check_relocs): Likewise.
>         * elf64-alpha.c (elf64_alpha_check_relocs): Likewise.
>         * elf64-ppc.c (ppc64_elf_check_relocs): Likewise.
>         * elf64-x86-64.c (elf_x86_64_check_relocs): Likewise.
>         * elflink.c (_bfd_elf_link_check_relocs): Skip non-loaded,
>         non-alloced sections.
> ---
>  bfd/elf32-i386.c   |  9 ---------
>  bfd/elf32-lm32.c   |  9 ---------
>  bfd/elf32-m32r.c   |  9 ---------
>  bfd/elf32-nds32.c  |  9 ---------
>  bfd/elf32-or1k.c   |  9 ---------
>  bfd/elf32-ppc.c    |  9 ---------
>  bfd/elf32-sh.c     |  9 ---------
>  bfd/elf32-xtensa.c |  2 +-
>  bfd/elf64-alpha.c  |  9 ---------
>  bfd/elf64-ppc.c    |  9 ---------
>  bfd/elf64-x86-64.c |  9 ---------
>  bfd/elflink.c      | 12 ++++++++++--
>  12 files changed, 11 insertions(+), 93 deletions(-)
>
> diff --git a/bfd/elf32-i386.c b/bfd/elf32-i386.c
> index 544b931552..f6f669957c 100644
> --- a/bfd/elf32-i386.c
> +++ b/bfd/elf32-i386.c
> @@ -1487,15 +1487,6 @@ elf_i386_check_relocs (bfd *abfd,
>    if (bfd_link_relocatable (info))
>      return TRUE;
>
> -  /* Don't do anything special with non-loaded, non-alloced sections.
> -     In particular, any relocs in such sections should not affect GOT
> -     and PLT reference counting (ie. we don't allow them to create GOT
> -     or PLT entries), there's no possibility or desire to optimize TLS
> -     relocs, and there's not much point in propagating relocs to shared
> -     libs that the dynamic linker won't relocate.  */
> -  if ((sec->flags & SEC_ALLOC) == 0)
> -    return TRUE;
> -
>    htab = elf_x86_hash_table (info, I386_ELF_DATA);
>    if (htab == NULL)
>      {
> diff --git a/bfd/elf32-lm32.c b/bfd/elf32-lm32.c
> index 5d09f2d350..acef37af5d 100644
> --- a/bfd/elf32-lm32.c
> +++ b/bfd/elf32-lm32.c
> @@ -1128,15 +1128,6 @@ lm32_elf_check_relocs (bfd *abfd,
>    if (bfd_link_relocatable (info))
>      return TRUE;
>
> -  /* Don't do anything special with non-loaded, non-alloced sections.
> -     In particular, any relocs in such sections should not affect GOT
> -     and PLT reference counting (ie. we don't allow them to create GOT
> -     or PLT entries), there's no possibility or desire to optimize TLS
> -     relocs, and there's not much point in propagating relocs to shared
> -     libs that the dynamic linker won't relocate.  */
> -  if ((sec->flags & SEC_ALLOC) == 0)
> -    return TRUE;
> -
>    symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
>    sym_hashes = elf_sym_hashes (abfd);
>    sym_hashes_end = sym_hashes + symtab_hdr->sh_size/sizeof (Elf32_External_Sym);
> diff --git a/bfd/elf32-m32r.c b/bfd/elf32-m32r.c
> index afe0ee899c..9b8e5cd124 100644
> --- a/bfd/elf32-m32r.c
> +++ b/bfd/elf32-m32r.c
> @@ -3450,15 +3450,6 @@ m32r_elf_check_relocs (bfd *abfd,
>    if (bfd_link_relocatable (info))
>      return TRUE;
>
> -  /* Don't do anything special with non-loaded, non-alloced sections.
> -     In particular, any relocs in such sections should not affect GOT
> -     and PLT reference counting (ie. we don't allow them to create GOT
> -     or PLT entries), there's no possibility or desire to optimize TLS
> -     relocs, and there's not much point in propagating relocs to shared
> -     libs that the dynamic linker won't relocate.  */
> -  if ((sec->flags & SEC_ALLOC) == 0)
> -    return TRUE;
> -
>    sreloc = NULL;
>    symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
>    sym_hashes = elf_sym_hashes (abfd);
> diff --git a/bfd/elf32-nds32.c b/bfd/elf32-nds32.c
> index 4f7ea76469..3e094f6270 100644
> --- a/bfd/elf32-nds32.c
> +++ b/bfd/elf32-nds32.c
> @@ -7081,15 +7081,6 @@ nds32_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
>        return TRUE;
>      }
>
> -  /* Don't do anything special with non-loaded, non-alloced sections.
> -     In particular, any relocs in such sections should not affect GOT
> -     and PLT reference counting (ie. we don't allow them to create GOT
> -     or PLT entries), there's no possibility or desire to optimize TLS
> -     relocs, and there's not much point in propagating relocs to shared
> -     libs that the dynamic linker won't relocate.  */
> -  if ((sec->flags & SEC_ALLOC) == 0)
> -    return TRUE;
> -
>    symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
>    sym_hashes = elf_sym_hashes (abfd);
>    sym_hashes_end =
> diff --git a/bfd/elf32-or1k.c b/bfd/elf32-or1k.c
> index b141b45886..3ed44ccceb 100644
> --- a/bfd/elf32-or1k.c
> +++ b/bfd/elf32-or1k.c
> @@ -1880,15 +1880,6 @@ or1k_elf_check_relocs (bfd *abfd,
>    if (bfd_link_relocatable (info))
>      return TRUE;
>
> -  /* Don't do anything special with non-loaded, non-alloced sections.
> -     In particular, any relocs in such sections should not affect GOT
> -     and PLT reference counting (ie. we don't allow them to create GOT
> -     or PLT entries), there's no possibility or desire to optimize TLS
> -     relocs, and there's not much point in propagating relocs to shared
> -     libs that the dynamic linker won't relocate.  */
> -  if ((sec->flags & SEC_ALLOC) == 0)
> -    return TRUE;
> -
>    symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
>    sym_hashes = elf_sym_hashes (abfd);
>
> diff --git a/bfd/elf32-ppc.c b/bfd/elf32-ppc.c
> index 588b79781d..62c6270329 100644
> --- a/bfd/elf32-ppc.c
> +++ b/bfd/elf32-ppc.c
> @@ -2888,15 +2888,6 @@ ppc_elf_check_relocs (bfd *abfd,
>    if (bfd_link_relocatable (info))
>      return TRUE;
>
> -  /* Don't do anything special with non-loaded, non-alloced sections.
> -     In particular, any relocs in such sections should not affect GOT
> -     and PLT reference counting (ie. we don't allow them to create GOT
> -     or PLT entries), there's no possibility or desire to optimize TLS
> -     relocs, and there's not much point in propagating relocs to shared
> -     libs that the dynamic linker won't relocate.  */
> -  if ((sec->flags & SEC_ALLOC) == 0)
> -    return TRUE;
> -
>  #ifdef DEBUG
>    _bfd_error_handler ("ppc_elf_check_relocs called for section %pA in %pB",
>                       sec, abfd);
> diff --git a/bfd/elf32-sh.c b/bfd/elf32-sh.c
> index 0428829757..7c9e695981 100644
> --- a/bfd/elf32-sh.c
> +++ b/bfd/elf32-sh.c
> @@ -5376,15 +5376,6 @@ sh_elf_check_relocs (bfd *abfd, struct bfd_link_info *info, asection *sec,
>    if (bfd_link_relocatable (info))
>      return TRUE;
>
> -  /* Don't do anything special with non-loaded, non-alloced sections.
> -     In particular, any relocs in such sections should not affect GOT
> -     and PLT reference counting (ie. we don't allow them to create GOT
> -     or PLT entries), there's no possibility or desire to optimize TLS
> -     relocs, and there's not much point in propagating relocs to shared
> -     libs that the dynamic linker won't relocate.  */
> -  if ((sec->flags & SEC_ALLOC) == 0)
> -    return TRUE;
> -
>    BFD_ASSERT (is_sh_elf (abfd));
>
>    symtab_hdr = &elf_symtab_hdr (abfd);
> diff --git a/bfd/elf32-xtensa.c b/bfd/elf32-xtensa.c
> index 05c4f8430a..9dc815edbb 100644
> --- a/bfd/elf32-xtensa.c
> +++ b/bfd/elf32-xtensa.c
> @@ -1039,7 +1039,7 @@ elf_xtensa_check_relocs (bfd *abfd,
>    const Elf_Internal_Rela *rel;
>    const Elf_Internal_Rela *rel_end;
>
> -  if (bfd_link_relocatable (info) || (sec->flags & SEC_ALLOC) == 0)
> +  if (bfd_link_relocatable (info))
>      return TRUE;
>
>    BFD_ASSERT (is_xtensa_elf (abfd));
> diff --git a/bfd/elf64-alpha.c b/bfd/elf64-alpha.c
> index 4e4efae0b1..0b31d450dc 100644
> --- a/bfd/elf64-alpha.c
> +++ b/bfd/elf64-alpha.c
> @@ -1782,15 +1782,6 @@ elf64_alpha_check_relocs (bfd *abfd, struct bfd_link_info *info,
>    if (bfd_link_relocatable (info))
>      return TRUE;
>
> -  /* Don't do anything special with non-loaded, non-alloced sections.
> -     In particular, any relocs in such sections should not affect GOT
> -     and PLT reference counting (ie. we don't allow them to create GOT
> -     or PLT entries), there's no possibility or desire to optimize TLS
> -     relocs, and there's not much point in propagating relocs to shared
> -     libs that the dynamic linker won't relocate.  */
> -  if ((sec->flags & SEC_ALLOC) == 0)
> -    return TRUE;
> -
>    BFD_ASSERT (is_alpha_elf (abfd));
>
>    dynobj = elf_hash_table (info)->dynobj;
> diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c
> index 49fda96be7..3941addd57 100644
> --- a/bfd/elf64-ppc.c
> +++ b/bfd/elf64-ppc.c
> @@ -4536,15 +4536,6 @@ ppc64_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
>    if (bfd_link_relocatable (info))
>      return TRUE;
>
> -  /* Don't do anything special with non-loaded, non-alloced sections.
> -     In particular, any relocs in such sections should not affect GOT
> -     and PLT reference counting (ie. we don't allow them to create GOT
> -     or PLT entries), there's no possibility or desire to optimize TLS
> -     relocs, and there's not much point in propagating relocs to shared
> -     libs that the dynamic linker won't relocate.  */
> -  if ((sec->flags & SEC_ALLOC) == 0)
> -    return TRUE;
> -
>    BFD_ASSERT (is_ppc64_elf (abfd));
>
>    htab = ppc_hash_table (info);
> diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
> index 183c808346..eada0e53ed 100644
> --- a/bfd/elf64-x86-64.c
> +++ b/bfd/elf64-x86-64.c
> @@ -1862,15 +1862,6 @@ elf_x86_64_check_relocs (bfd *abfd, struct bfd_link_info *info,
>    if (bfd_link_relocatable (info))
>      return TRUE;
>
> -  /* Don't do anything special with non-loaded, non-alloced sections.
> -     In particular, any relocs in such sections should not affect GOT
> -     and PLT reference counting (ie. we don't allow them to create GOT
> -     or PLT entries), there's no possibility or desire to optimize TLS
> -     relocs, and there's not much point in propagating relocs to shared
> -     libs that the dynamic linker won't relocate.  */
> -  if ((sec->flags & SEC_ALLOC) == 0)
> -    return TRUE;
> -
>    htab = elf_x86_hash_table (info, X86_64_ELF_DATA);
>    if (htab == NULL)
>      {
> diff --git a/bfd/elflink.c b/bfd/elflink.c
> index a2b40ccb04..7cee0afac0 100644
> --- a/bfd/elflink.c
> +++ b/bfd/elflink.c
> @@ -3956,8 +3956,16 @@ _bfd_elf_link_check_relocs (bfd *abfd, struct bfd_link_info *info)
>           Elf_Internal_Rela *internal_relocs;
>           bfd_boolean ok;
>
> -         /* Don't check relocations in excluded sections.  */
> -         if ((o->flags & SEC_RELOC) == 0
> +         /* Don't check relocations in excluded sections.  Don't do
> +            anything special with non-loaded, non-alloced sections.
> +            In particular, any relocs in such sections should not
> +            affect GOT and PLT reference counting (ie.  we don't
> +            allow them to create GOT or PLT entries), there's no
> +            possibility or desire to optimize TLS relocs, and
> +            there's not much point in propagating relocs to shared
> +            libs that the dynamic linker won't relocate.  */
> +         if ((o->flags & SEC_ALLOC) == 0
> +             || (o->flags & SEC_RELOC) == 0
>               || (o->flags & SEC_EXCLUDE) != 0
>               || o->reloc_count == 0
>               || ((info->strip == strip_all || info->strip == strip_debugger)
> --
> 2.26.2
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ELF: Don't check relocations in non-loaded, non-alloced sections
       [not found] ` <MWHPR12MB14567E24AED5430AAE2497A8CB880@MWHPR12MB1456.namprd12.prod.outlook.com>
@ 2020-06-03  1:29   ` H.J. Lu
  2020-06-03 16:48     ` V2 " H.J. Lu
  0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2020-06-03  1:29 UTC (permalink / raw)
  To: Fangrui Song; +Cc: Binutils

On Tue, Jun 2, 2020 at 5:06 PM Fangrui Song <i@maskray.me> wrote:
>
> I haven't really read the patch, but just wanted to express a point.
> It is sometimes useful to check relocations even for a non-SHF_ALLOC
> section. For example, a PC relative relocation type does not make
> sense from a non-SHF_ALLOC section referencing a SHF_ALLOC section.
>
> Conceptually, even if a non-SHF_ALLOC is loaded as part of the memory
> image, the distance between it and a SHF_ALLOC section may not be a
> constant, so the linker cannot reasonably resolve the relocation.
>

Since non-SHF_ALLOC sections have no impact on run-time behavior,
ld.so ignores dynamic relocations on non-SHF_ALLOC section.   In
checking phase, relocations on non-SHF_ALLOC section shouldn't
alter other relocatitons against the same symbol.  When resolving
such relocations, linker should ignore any relocation errors for such
relocations.


-- 
H.J.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* V2 [PATCH] ELF: Don't check relocations in non-loaded, non-alloced sections
  2020-06-03  1:29   ` H.J. Lu
@ 2020-06-03 16:48     ` H.J. Lu
  2020-06-03 18:55       ` V3 " H.J. Lu
  0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2020-06-03 16:48 UTC (permalink / raw)
  To: Nick Clifton, Alan Modra; +Cc: Fangrui Song, Binutils

On Tue, Jun 02, 2020 at 06:29:16PM -0700, H.J. Lu wrote:
> On Tue, Jun 2, 2020 at 5:06 PM Fangrui Song <i@maskray.me> wrote:
> >
> > I haven't really read the patch, but just wanted to express a point.
> > It is sometimes useful to check relocations even for a non-SHF_ALLOC
> > section. For example, a PC relative relocation type does not make
> > sense from a non-SHF_ALLOC section referencing a SHF_ALLOC section.
> >
> > Conceptually, even if a non-SHF_ALLOC is loaded as part of the memory
> > image, the distance between it and a SHF_ALLOC section may not be a
> > constant, so the linker cannot reasonably resolve the relocation.
> >
> 
> Since non-SHF_ALLOC sections have no impact on run-time behavior,
> ld.so ignores dynamic relocations on non-SHF_ALLOC section.   In
> checking phase, relocations on non-SHF_ALLOC section shouldn't
> alter other relocatitons against the same symbol.  When resolving
> such relocations, linker should ignore any relocation errors for such
> relocations.
> 
> 

Linker shouldn't ignore any relocation errors for such relocations.  But
linker should resolve them to 0:

https://sourceware.org/bugzilla/show_bug.cgi?id=26080

Here is the updated patch.  OK for master?

Thanks.

H.J.
---
From 0e27705c41db5200c248403c9b50f17c5533dd04 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 2 Jun 2020 06:53:34 -0700
Subject: [PATCH] ELF: Don't check relocations in non-loaded, non-alloced
 sections

Don't do anything special with non-loaded, non-alloced sections.
In particular, any relocs in such sections should not affect GOT
and PLT reference counting (ie. we don't allow them to create GOT
or PLT entries), there's no possibility or desire to optimize TLS
relocs, and there's not much point in propagating relocs to shared
libs that the dynamic linker won't relocate.  Resolve relocation
in debug section against symbol defined in shared library to 0.

bfd/

	PR ld/26080
	* elf-m10300.c (mn10300_elf_relocate_section): Resolve relocation
	in debug section against symbol defined in shared library to 0.
	* elf32-i386.c (elf_i386_check_relocs): Remove SEC_ALLOC check.
	* elf32-lm32.c (lm32_elf_check_relocs): Likewise.
	* elf32-m32r.c (m32r_elf_check_relocs): Likewise.
	* elf32-nds32.c (nds32_elf_check_relocs): Likewise.
	* elf32-nios2.c (nios2_elf32_check_relocs): Likewise.
	* elf32-or1k.c (or1k_elf_check_relocs): Likewise.
	* elf32-ppc.c (ppc_elf_check_relocs): Likewise.
	* elf32-sh.c (sh_elf_check_relocs): Likewise.
	* elf32-xtensa.c (elf_xtensa_check_relocs): Likewise.
	* elf64-alpha.c (elf64_alpha_check_relocs): Likewise.
	* elf64-ppc.c (ppc64_elf_check_relocs): Likewise.
	* elf64-x86-64.c (elf_x86_64_check_relocs): Likewise.
	* elf32-vax.c (elf_vax_check_relocs): Set non_got_ref for non-GOT
	reference.
	(elf_vax_adjust_dynamic_symbol): Generate a copy reloc only if
	there is non-GOT reference.
	* elflink.c (_bfd_elf_link_check_relocs): Skip non-loaded,
	non-alloced sections.

ld/

	PR ld/26080
	* testsuite/ld-elf/comm-data.exp: Remove copy_reloc.
	* testsuite/ld-elf/comm-data2r.rd: Removed.
	* testsuite/ld-elf/comm-data2r.sd: Likewise.
	* testsuite/ld-elf/comm-data2r.xd: Likewise.
---
 bfd/elf-m10300.c                   |  6 +++---
 bfd/elf32-i386.c                   |  9 ---------
 bfd/elf32-lm32.c                   |  9 ---------
 bfd/elf32-m32r.c                   |  9 ---------
 bfd/elf32-nds32.c                  |  9 ---------
 bfd/elf32-or1k.c                   |  9 ---------
 bfd/elf32-ppc.c                    |  9 ---------
 bfd/elf32-sh.c                     |  9 ---------
 bfd/elf32-vax.c                    | 10 ++++++++++
 bfd/elf32-xtensa.c                 |  2 +-
 bfd/elf64-alpha.c                  |  9 ---------
 bfd/elf64-ppc.c                    |  9 ---------
 bfd/elf64-x86-64.c                 |  9 ---------
 bfd/elflink.c                      | 12 ++++++++++--
 ld/testsuite/ld-elf/comm-data.exp  | 15 +++------------
 ld/testsuite/ld-elf/comm-data2r.rd |  3 ---
 ld/testsuite/ld-elf/comm-data2r.sd | 10 ----------
 ld/testsuite/ld-elf/comm-data2r.xd |  2 --
 18 files changed, 27 insertions(+), 123 deletions(-)
 delete mode 100644 ld/testsuite/ld-elf/comm-data2r.rd
 delete mode 100644 ld/testsuite/ld-elf/comm-data2r.sd
 delete mode 100644 ld/testsuite/ld-elf/comm-data2r.xd

diff --git a/bfd/elf-m10300.c b/bfd/elf-m10300.c
index 696514ab05..5a0bb9f005 100644
--- a/bfd/elf-m10300.c
+++ b/bfd/elf-m10300.c
@@ -2066,12 +2066,12 @@ mn10300_elf_relocate_section (bfd *output_bfd,
 		      && elf_hash_table (info)->dynamic_sections_created
 		      && !SYMBOL_REFERENCES_LOCAL (info, hh))
 		  || (r_type == R_MN10300_32
+		      && !SYMBOL_REFERENCES_LOCAL (info, hh)
 		      /* _32 relocs in executables force _COPY relocs,
 			 such that the address of the symbol ends up
 			 being local.  */
-		      && !bfd_link_executable (info)
-		      && !SYMBOL_REFERENCES_LOCAL (info, hh)
-		      && ((input_section->flags & SEC_ALLOC) != 0
+		      && (((input_section->flags & SEC_ALLOC) != 0
+			   && !bfd_link_executable (info))
 			  /* DWARF will emit R_MN10300_32 relocations
 			     in its sections against symbols defined
 			     externally in shared libraries.  We can't
diff --git a/bfd/elf32-i386.c b/bfd/elf32-i386.c
index 544b931552..f6f669957c 100644
--- a/bfd/elf32-i386.c
+++ b/bfd/elf32-i386.c
@@ -1487,15 +1487,6 @@ elf_i386_check_relocs (bfd *abfd,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   htab = elf_x86_hash_table (info, I386_ELF_DATA);
   if (htab == NULL)
     {
diff --git a/bfd/elf32-lm32.c b/bfd/elf32-lm32.c
index 3c31dd44c8..aba821ffd1 100644
--- a/bfd/elf32-lm32.c
+++ b/bfd/elf32-lm32.c
@@ -1128,15 +1128,6 @@ lm32_elf_check_relocs (bfd *abfd,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
   sym_hashes = elf_sym_hashes (abfd);
   sym_hashes_end = sym_hashes + symtab_hdr->sh_size/sizeof (Elf32_External_Sym);
diff --git a/bfd/elf32-m32r.c b/bfd/elf32-m32r.c
index 931e138b37..740be93382 100644
--- a/bfd/elf32-m32r.c
+++ b/bfd/elf32-m32r.c
@@ -3424,15 +3424,6 @@ m32r_elf_check_relocs (bfd *abfd,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   sreloc = NULL;
   symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
   sym_hashes = elf_sym_hashes (abfd);
diff --git a/bfd/elf32-nds32.c b/bfd/elf32-nds32.c
index 01ea277426..1d3a0f7526 100644
--- a/bfd/elf32-nds32.c
+++ b/bfd/elf32-nds32.c
@@ -7055,15 +7055,6 @@ nds32_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
       return TRUE;
     }
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
   sym_hashes = elf_sym_hashes (abfd);
   sym_hashes_end =
diff --git a/bfd/elf32-or1k.c b/bfd/elf32-or1k.c
index ac62d630ab..b25f96b42d 100644
--- a/bfd/elf32-or1k.c
+++ b/bfd/elf32-or1k.c
@@ -1880,15 +1880,6 @@ or1k_elf_check_relocs (bfd *abfd,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
   sym_hashes = elf_sym_hashes (abfd);
 
diff --git a/bfd/elf32-ppc.c b/bfd/elf32-ppc.c
index 0cb7f67504..995e1a95e2 100644
--- a/bfd/elf32-ppc.c
+++ b/bfd/elf32-ppc.c
@@ -2888,15 +2888,6 @@ ppc_elf_check_relocs (bfd *abfd,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
 #ifdef DEBUG
   _bfd_error_handler ("ppc_elf_check_relocs called for section %pA in %pB",
 		      sec, abfd);
diff --git a/bfd/elf32-sh.c b/bfd/elf32-sh.c
index 9ec745be19..dd670466c3 100644
--- a/bfd/elf32-sh.c
+++ b/bfd/elf32-sh.c
@@ -5350,15 +5350,6 @@ sh_elf_check_relocs (bfd *abfd, struct bfd_link_info *info, asection *sec,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   BFD_ASSERT (is_sh_elf (abfd));
 
   symtab_hdr = &elf_symtab_hdr (abfd);
diff --git a/bfd/elf32-vax.c b/bfd/elf32-vax.c
index fa84e0be19..b5c3d8943a 100644
--- a/bfd/elf32-vax.c
+++ b/bfd/elf32-vax.c
@@ -712,6 +712,11 @@ elf_vax_check_relocs (bfd *abfd, struct bfd_link_info *info, asection *sec,
 		h->plt.refcount++;
 	    }
 
+	  /* Non-GOT reference may need a copy reloc in executable or
+	     a dynamic reloc in shared library.  */
+	  if (h != NULL)
+	    h->non_got_ref = 1;
+
 	  /* If we are creating a shared library, we need to copy the
 	     reloc into the shared library.  */
 	  if (bfd_link_pic (info)
@@ -929,6 +934,11 @@ elf_vax_adjust_dynamic_symbol (struct bfd_link_info *info,
   if (bfd_link_pic (info))
     return TRUE;
 
+  /* If there are no references to this symbol that do not use the
+     GOT relocation, we don't need to generate a copy reloc.  */
+  if (!h->non_got_ref)
+    return TRUE;
+
   /* We must allocate the symbol in our .dynbss section, which will
      become part of the .bss section of the executable.  There will be
      an entry for this symbol in the .dynsym section.  The dynamic
diff --git a/bfd/elf32-xtensa.c b/bfd/elf32-xtensa.c
index 05c4f8430a..9dc815edbb 100644
--- a/bfd/elf32-xtensa.c
+++ b/bfd/elf32-xtensa.c
@@ -1039,7 +1039,7 @@ elf_xtensa_check_relocs (bfd *abfd,
   const Elf_Internal_Rela *rel;
   const Elf_Internal_Rela *rel_end;
 
-  if (bfd_link_relocatable (info) || (sec->flags & SEC_ALLOC) == 0)
+  if (bfd_link_relocatable (info))
     return TRUE;
 
   BFD_ASSERT (is_xtensa_elf (abfd));
diff --git a/bfd/elf64-alpha.c b/bfd/elf64-alpha.c
index 4e4efae0b1..0b31d450dc 100644
--- a/bfd/elf64-alpha.c
+++ b/bfd/elf64-alpha.c
@@ -1782,15 +1782,6 @@ elf64_alpha_check_relocs (bfd *abfd, struct bfd_link_info *info,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   BFD_ASSERT (is_alpha_elf (abfd));
 
   dynobj = elf_hash_table (info)->dynobj;
diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c
index 5f99d4344b..769afc5aa5 100644
--- a/bfd/elf64-ppc.c
+++ b/bfd/elf64-ppc.c
@@ -4536,15 +4536,6 @@ ppc64_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   BFD_ASSERT (is_ppc64_elf (abfd));
 
   htab = ppc_hash_table (info);
diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
index 183c808346..eada0e53ed 100644
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -1862,15 +1862,6 @@ elf_x86_64_check_relocs (bfd *abfd, struct bfd_link_info *info,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   htab = elf_x86_hash_table (info, X86_64_ELF_DATA);
   if (htab == NULL)
     {
diff --git a/bfd/elflink.c b/bfd/elflink.c
index ce6282a9dc..7e86adec5b 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -3956,8 +3956,16 @@ _bfd_elf_link_check_relocs (bfd *abfd, struct bfd_link_info *info)
 	  Elf_Internal_Rela *internal_relocs;
 	  bfd_boolean ok;
 
-	  /* Don't check relocations in excluded sections.  */
-	  if ((o->flags & SEC_RELOC) == 0
+	  /* Don't check relocations in excluded sections.  Don't do
+	     anything special with non-loaded, non-alloced sections.
+	     In particular, any relocs in such sections should not
+	     affect GOT and PLT reference counting (ie.  we don't
+	     allow them to create GOT or PLT entries), there's no
+	     possibility or desire to optimize TLS relocs, and
+	     there's not much point in propagating relocs to shared
+	     libs that the dynamic linker won't relocate.  */
+	  if ((o->flags & SEC_ALLOC) == 0
+	      || (o->flags & SEC_RELOC) == 0
 	      || (o->flags & SEC_EXCLUDE) != 0
 	      || o->reloc_count == 0
 	      || ((info->strip == strip_all || info->strip == strip_debugger)
diff --git a/ld/testsuite/ld-elf/comm-data.exp b/ld/testsuite/ld-elf/comm-data.exp
index a53a77123c..c735fe244c 100644
--- a/ld/testsuite/ld-elf/comm-data.exp
+++ b/ld/testsuite/ld-elf/comm-data.exp
@@ -79,12 +79,6 @@ setup_xfail "bfin-*-*"
 
 setup_xfail "arm*-*-*" "ld/13802"
 
-# List targets here that keep copy relocs rather than eliminating
-# them where possible in favour to dynamic relocs in the relevant
-# loadable sections; see also the "-z nocopyreloc" command-line
-# option and the ELIMINATE_COPY_RELOCS macro some backends use.
-set copy_reloc [expr [istarget mn10300-*-*] || [istarget vax-*-*]]
-
 # Verify that a common symbol has been converted to an undefined
 # reference to the global symbol of the same name defined above
 # and that the debug reference has been dropped.
@@ -95,12 +89,9 @@ run_ld_link_tests [list \
 	"$AFLAGS" \
 	{ comm-data2.s } \
 	[list \
-	    [list readelf -s \
-		[expr { $copy_reloc ? "comm-data2r.sd" : "comm-data2.sd"}]] \
-	    [list readelf -r \
-		[expr { $copy_reloc ? "comm-data2r.rd" : "comm-data2.rd"}]] \
-	    [list readelf "-x .debug_foo" \
-		[expr { $copy_reloc ? "comm-data2r.xd" : "comm-data2.xd"}]]] \
+	    [list readelf -s comm-data2.sd] \
+	    [list readelf -r comm-data2.rd] \
+	    [list readelf "-x .debug_foo" comm-data2.xd]] \
 	"comm-data" \
     ] \
     [list \
diff --git a/ld/testsuite/ld-elf/comm-data2r.rd b/ld/testsuite/ld-elf/comm-data2r.rd
deleted file mode 100644
index 64c0396d26..0000000000
--- a/ld/testsuite/ld-elf/comm-data2r.rd
+++ /dev/null
@@ -1,3 +0,0 @@
-Relocation section '\.rela\.dyn' at offset 0x[0-9a-f]+ contains 1 entry:
- +Offset +Info +Type +Sym\.Value +Sym\. Name \+ Addend
-0*12340000 +[0-9a-f]+ +R_.*_COPY +0*12340000 +foo \+ 0
diff --git a/ld/testsuite/ld-elf/comm-data2r.sd b/ld/testsuite/ld-elf/comm-data2r.sd
deleted file mode 100644
index 685b0befd1..0000000000
--- a/ld/testsuite/ld-elf/comm-data2r.sd
+++ /dev/null
@@ -1,10 +0,0 @@
-Symbol table '\.dynsym' contains [0-9]+ entries:
- +Num: +Value +Size +Type +Bind +Vis +Ndx +Name
-#...
- +[0-9]+: +0*12340000 +4 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo
-#...
-Symbol table '\.symtab' contains [0-9]+ entries:
- +Num: +Value +Size +Type +Bind +Vis +Ndx +Name
-#...
- +[0-9]+: +0*12340000 +4 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo
-#pass
diff --git a/ld/testsuite/ld-elf/comm-data2r.xd b/ld/testsuite/ld-elf/comm-data2r.xd
deleted file mode 100644
index 58f6f2a88f..0000000000
--- a/ld/testsuite/ld-elf/comm-data2r.xd
+++ /dev/null
@@ -1,2 +0,0 @@
-Hex dump of section '\.debug_foo':
- +0x0*76540000 (?:12340000 00000000|00003412 00000000|00000000 00003412) 00000000 00000000 .*
-- 
2.26.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* V3 [PATCH] ELF: Don't check relocations in non-loaded, non-alloced sections
  2020-06-03 16:48     ` V2 " H.J. Lu
@ 2020-06-03 18:55       ` H.J. Lu
  2020-06-04  7:47         ` Alan Modra
  0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2020-06-03 18:55 UTC (permalink / raw)
  To: Nick Clifton, Alan Modra, Maciej W. Rozycki; +Cc: Fangrui Song, Binutils

[-- Attachment #1: Type: text/plain, Size: 1452 bytes --]

On Wed, Jun 3, 2020 at 9:48 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jun 02, 2020 at 06:29:16PM -0700, H.J. Lu wrote:
> > On Tue, Jun 2, 2020 at 5:06 PM Fangrui Song <i@maskray.me> wrote:
> > >
> > > I haven't really read the patch, but just wanted to express a point.
> > > It is sometimes useful to check relocations even for a non-SHF_ALLOC
> > > section. For example, a PC relative relocation type does not make
> > > sense from a non-SHF_ALLOC section referencing a SHF_ALLOC section.
> > >
> > > Conceptually, even if a non-SHF_ALLOC is loaded as part of the memory
> > > image, the distance between it and a SHF_ALLOC section may not be a
> > > constant, so the linker cannot reasonably resolve the relocation.
> > >
> >
> > Since non-SHF_ALLOC sections have no impact on run-time behavior,
> > ld.so ignores dynamic relocations on non-SHF_ALLOC section.   In
> > checking phase, relocations on non-SHF_ALLOC section shouldn't
> > alter other relocatitons against the same symbol.  When resolving
> > such relocations, linker should ignore any relocation errors for such
> > relocations.
> >
> >
>
> Linker shouldn't ignore any relocation errors for such relocations.  But
> linker should resolve them to 0:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=26080
>
> Here is the updated patch.  OK for master?
>

Here is the updated patch to remove SEC_ALLOC check in
_bfd_mips_elf_check_relocs.

OK for master?

Thanks.

-- 
H.J.

[-- Attachment #2: 0001-ELF-Don-t-check-relocations-in-non-loaded-non-alloce.patch --]
[-- Type: text/x-patch, Size: 21707 bytes --]

From bb7ea9860f5b2818e8180100bf22e19a32b75325 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 2 Jun 2020 06:53:34 -0700
Subject: V3 [PATCH] ELF: Don't check relocations in non-loaded, non-alloced
 sections

Don't do anything special with non-loaded, non-alloced sections.
In particular, any relocs in such sections should not affect GOT
and PLT reference counting (ie. we don't allow them to create GOT
or PLT entries), there's no possibility or desire to optimize TLS
relocs, and there's not much point in propagating relocs to shared
libs that the dynamic linker won't relocate.

Since check_relocs is no longer called on non-loaded, non-alloced
sections, remove SEC_ALLOC check.  Resolve relocation in debug section
against symbol defined in shared library to 0.

bfd/

	PR ld/26080
	* elf-m10300.c (mn10300_elf_relocate_section): Resolve relocation
	in debug section against symbol defined in shared library to 0.
	* elf32-i386.c (elf_i386_check_relocs): Remove SEC_ALLOC check.
	* elf32-lm32.c (lm32_elf_check_relocs): Likewise.
	* elf32-m32r.c (m32r_elf_check_relocs): Likewise.
	* elf32-nds32.c (nds32_elf_check_relocs): Likewise.
	* elf32-nios2.c (nios2_elf32_check_relocs): Likewise.
	* elf32-or1k.c (or1k_elf_check_relocs): Likewise.
	* elf32-ppc.c (ppc_elf_check_relocs): Likewise.
	* elf32-sh.c (sh_elf_check_relocs): Likewise.
	* elf32-xtensa.c (elf_xtensa_check_relocs): Likewise.
	* elf64-alpha.c (elf64_alpha_check_relocs): Likewise.
	* elf64-ppc.c (ppc64_elf_check_relocs): Likewise.
	* elf64-x86-64.c (elf_x86_64_check_relocs): Likewise.
	* elfxx-mips.c (_bfd_mips_elf_check_relocs): Likewise.
	* elf32-vax.c (elf_vax_check_relocs): Set non_got_ref for non-GOT
	reference.
	(elf_vax_adjust_dynamic_symbol): Generate a copy reloc only if
	there is non-GOT reference.
	* elflink.c (_bfd_elf_link_check_relocs): Skip non-loaded,
	non-alloced sections.

ld/

	PR ld/26080
	* testsuite/ld-elf/comm-data.exp: Remove copy_reloc.
	* testsuite/ld-elf/comm-data2r.rd: Removed.
	* testsuite/ld-elf/comm-data2r.sd: Likewise.
	* testsuite/ld-elf/comm-data2r.xd: Likewise.
---
 bfd/elf-m10300.c                   |  6 ++--
 bfd/elf32-i386.c                   |  9 ------
 bfd/elf32-lm32.c                   |  9 ------
 bfd/elf32-m32r.c                   |  9 ------
 bfd/elf32-nds32.c                  |  9 ------
 bfd/elf32-nios2.c                  |  9 ------
 bfd/elf32-or1k.c                   |  9 ------
 bfd/elf32-ppc.c                    |  9 ------
 bfd/elf32-sh.c                     |  9 ------
 bfd/elf32-vax.c                    | 10 +++++++
 bfd/elf32-xtensa.c                 |  2 +-
 bfd/elf64-alpha.c                  |  9 ------
 bfd/elf64-ppc.c                    |  9 ------
 bfd/elf64-x86-64.c                 |  9 ------
 bfd/elflink.c                      | 12 ++++++--
 bfd/elfxx-mips.c                   | 45 ++++++++----------------------
 ld/testsuite/ld-elf/comm-data.exp  | 15 ++--------
 ld/testsuite/ld-elf/comm-data2r.rd |  3 --
 ld/testsuite/ld-elf/comm-data2r.sd | 10 -------
 ld/testsuite/ld-elf/comm-data2r.xd |  2 --
 20 files changed, 39 insertions(+), 165 deletions(-)
 delete mode 100644 ld/testsuite/ld-elf/comm-data2r.rd
 delete mode 100644 ld/testsuite/ld-elf/comm-data2r.sd
 delete mode 100644 ld/testsuite/ld-elf/comm-data2r.xd

diff --git a/bfd/elf-m10300.c b/bfd/elf-m10300.c
index 696514ab05..5a0bb9f005 100644
--- a/bfd/elf-m10300.c
+++ b/bfd/elf-m10300.c
@@ -2066,12 +2066,12 @@ mn10300_elf_relocate_section (bfd *output_bfd,
 		      && elf_hash_table (info)->dynamic_sections_created
 		      && !SYMBOL_REFERENCES_LOCAL (info, hh))
 		  || (r_type == R_MN10300_32
+		      && !SYMBOL_REFERENCES_LOCAL (info, hh)
 		      /* _32 relocs in executables force _COPY relocs,
 			 such that the address of the symbol ends up
 			 being local.  */
-		      && !bfd_link_executable (info)
-		      && !SYMBOL_REFERENCES_LOCAL (info, hh)
-		      && ((input_section->flags & SEC_ALLOC) != 0
+		      && (((input_section->flags & SEC_ALLOC) != 0
+			   && !bfd_link_executable (info))
 			  /* DWARF will emit R_MN10300_32 relocations
 			     in its sections against symbols defined
 			     externally in shared libraries.  We can't
diff --git a/bfd/elf32-i386.c b/bfd/elf32-i386.c
index 544b931552..f6f669957c 100644
--- a/bfd/elf32-i386.c
+++ b/bfd/elf32-i386.c
@@ -1487,15 +1487,6 @@ elf_i386_check_relocs (bfd *abfd,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   htab = elf_x86_hash_table (info, I386_ELF_DATA);
   if (htab == NULL)
     {
diff --git a/bfd/elf32-lm32.c b/bfd/elf32-lm32.c
index 3c31dd44c8..aba821ffd1 100644
--- a/bfd/elf32-lm32.c
+++ b/bfd/elf32-lm32.c
@@ -1128,15 +1128,6 @@ lm32_elf_check_relocs (bfd *abfd,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
   sym_hashes = elf_sym_hashes (abfd);
   sym_hashes_end = sym_hashes + symtab_hdr->sh_size/sizeof (Elf32_External_Sym);
diff --git a/bfd/elf32-m32r.c b/bfd/elf32-m32r.c
index 931e138b37..740be93382 100644
--- a/bfd/elf32-m32r.c
+++ b/bfd/elf32-m32r.c
@@ -3424,15 +3424,6 @@ m32r_elf_check_relocs (bfd *abfd,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   sreloc = NULL;
   symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
   sym_hashes = elf_sym_hashes (abfd);
diff --git a/bfd/elf32-nds32.c b/bfd/elf32-nds32.c
index 01ea277426..1d3a0f7526 100644
--- a/bfd/elf32-nds32.c
+++ b/bfd/elf32-nds32.c
@@ -7055,15 +7055,6 @@ nds32_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
       return TRUE;
     }
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
   sym_hashes = elf_sym_hashes (abfd);
   sym_hashes_end =
diff --git a/bfd/elf32-nios2.c b/bfd/elf32-nios2.c
index 71200da9b5..cdc11f9715 100644
--- a/bfd/elf32-nios2.c
+++ b/bfd/elf32-nios2.c
@@ -4689,15 +4689,6 @@ nios2_elf32_check_relocs (bfd *abfd, struct bfd_link_info *info,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
   sym_hashes = elf_sym_hashes (abfd);
   sym_hashes_end = (sym_hashes
diff --git a/bfd/elf32-or1k.c b/bfd/elf32-or1k.c
index ac62d630ab..b25f96b42d 100644
--- a/bfd/elf32-or1k.c
+++ b/bfd/elf32-or1k.c
@@ -1880,15 +1880,6 @@ or1k_elf_check_relocs (bfd *abfd,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
   sym_hashes = elf_sym_hashes (abfd);
 
diff --git a/bfd/elf32-ppc.c b/bfd/elf32-ppc.c
index 0cb7f67504..995e1a95e2 100644
--- a/bfd/elf32-ppc.c
+++ b/bfd/elf32-ppc.c
@@ -2888,15 +2888,6 @@ ppc_elf_check_relocs (bfd *abfd,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
 #ifdef DEBUG
   _bfd_error_handler ("ppc_elf_check_relocs called for section %pA in %pB",
 		      sec, abfd);
diff --git a/bfd/elf32-sh.c b/bfd/elf32-sh.c
index 9ec745be19..dd670466c3 100644
--- a/bfd/elf32-sh.c
+++ b/bfd/elf32-sh.c
@@ -5350,15 +5350,6 @@ sh_elf_check_relocs (bfd *abfd, struct bfd_link_info *info, asection *sec,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   BFD_ASSERT (is_sh_elf (abfd));
 
   symtab_hdr = &elf_symtab_hdr (abfd);
diff --git a/bfd/elf32-vax.c b/bfd/elf32-vax.c
index fa84e0be19..b5c3d8943a 100644
--- a/bfd/elf32-vax.c
+++ b/bfd/elf32-vax.c
@@ -712,6 +712,11 @@ elf_vax_check_relocs (bfd *abfd, struct bfd_link_info *info, asection *sec,
 		h->plt.refcount++;
 	    }
 
+	  /* Non-GOT reference may need a copy reloc in executable or
+	     a dynamic reloc in shared library.  */
+	  if (h != NULL)
+	    h->non_got_ref = 1;
+
 	  /* If we are creating a shared library, we need to copy the
 	     reloc into the shared library.  */
 	  if (bfd_link_pic (info)
@@ -929,6 +934,11 @@ elf_vax_adjust_dynamic_symbol (struct bfd_link_info *info,
   if (bfd_link_pic (info))
     return TRUE;
 
+  /* If there are no references to this symbol that do not use the
+     GOT relocation, we don't need to generate a copy reloc.  */
+  if (!h->non_got_ref)
+    return TRUE;
+
   /* We must allocate the symbol in our .dynbss section, which will
      become part of the .bss section of the executable.  There will be
      an entry for this symbol in the .dynsym section.  The dynamic
diff --git a/bfd/elf32-xtensa.c b/bfd/elf32-xtensa.c
index 05c4f8430a..9dc815edbb 100644
--- a/bfd/elf32-xtensa.c
+++ b/bfd/elf32-xtensa.c
@@ -1039,7 +1039,7 @@ elf_xtensa_check_relocs (bfd *abfd,
   const Elf_Internal_Rela *rel;
   const Elf_Internal_Rela *rel_end;
 
-  if (bfd_link_relocatable (info) || (sec->flags & SEC_ALLOC) == 0)
+  if (bfd_link_relocatable (info))
     return TRUE;
 
   BFD_ASSERT (is_xtensa_elf (abfd));
diff --git a/bfd/elf64-alpha.c b/bfd/elf64-alpha.c
index 4e4efae0b1..0b31d450dc 100644
--- a/bfd/elf64-alpha.c
+++ b/bfd/elf64-alpha.c
@@ -1782,15 +1782,6 @@ elf64_alpha_check_relocs (bfd *abfd, struct bfd_link_info *info,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   BFD_ASSERT (is_alpha_elf (abfd));
 
   dynobj = elf_hash_table (info)->dynobj;
diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c
index 5f99d4344b..769afc5aa5 100644
--- a/bfd/elf64-ppc.c
+++ b/bfd/elf64-ppc.c
@@ -4536,15 +4536,6 @@ ppc64_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   BFD_ASSERT (is_ppc64_elf (abfd));
 
   htab = ppc_hash_table (info);
diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
index 183c808346..eada0e53ed 100644
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -1862,15 +1862,6 @@ elf_x86_64_check_relocs (bfd *abfd, struct bfd_link_info *info,
   if (bfd_link_relocatable (info))
     return TRUE;
 
-  /* Don't do anything special with non-loaded, non-alloced sections.
-     In particular, any relocs in such sections should not affect GOT
-     and PLT reference counting (ie. we don't allow them to create GOT
-     or PLT entries), there's no possibility or desire to optimize TLS
-     relocs, and there's not much point in propagating relocs to shared
-     libs that the dynamic linker won't relocate.  */
-  if ((sec->flags & SEC_ALLOC) == 0)
-    return TRUE;
-
   htab = elf_x86_hash_table (info, X86_64_ELF_DATA);
   if (htab == NULL)
     {
diff --git a/bfd/elflink.c b/bfd/elflink.c
index ce6282a9dc..7e86adec5b 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -3956,8 +3956,16 @@ _bfd_elf_link_check_relocs (bfd *abfd, struct bfd_link_info *info)
 	  Elf_Internal_Rela *internal_relocs;
 	  bfd_boolean ok;
 
-	  /* Don't check relocations in excluded sections.  */
-	  if ((o->flags & SEC_RELOC) == 0
+	  /* Don't check relocations in excluded sections.  Don't do
+	     anything special with non-loaded, non-alloced sections.
+	     In particular, any relocs in such sections should not
+	     affect GOT and PLT reference counting (ie.  we don't
+	     allow them to create GOT or PLT entries), there's no
+	     possibility or desire to optimize TLS relocs, and
+	     there's not much point in propagating relocs to shared
+	     libs that the dynamic linker won't relocate.  */
+	  if ((o->flags & SEC_ALLOC) == 0
+	      || (o->flags & SEC_RELOC) == 0
 	      || (o->flags & SEC_EXCLUDE) != 0
 	      || o->reloc_count == 0
 	      || ((info->strip == strip_all || info->strip == strip_debugger)
diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c
index e563d56dbb..5b5864a259 100644
--- a/bfd/elfxx-mips.c
+++ b/bfd/elfxx-mips.c
@@ -8618,7 +8618,6 @@ _bfd_mips_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
       struct elf_link_hash_entry *h;
       bfd_boolean can_make_dynamic_p;
       bfd_boolean call_reloc_p;
-      bfd_boolean constrain_symbol_p;
 
       r_symndx = ELF_R_SYM (abfd, rel->r_info);
       r_type = ELF_R_TYPE (abfd, rel->r_info);
@@ -8653,12 +8652,6 @@ _bfd_mips_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
 	 and if pointer equality therefore doesn't matter.  */
       call_reloc_p = FALSE;
 
-      /* Set CONSTRAIN_SYMBOL_P if we need to take the relocation
-	 into account when deciding how to define the symbol.
-	 Relocations in nonallocatable sections such as .pdr and
-	 .debug* should have no effect.  */
-      constrain_symbol_p = ((sec->flags & SEC_ALLOC) != 0);
-
       switch (r_type)
 	{
 	case R_MIPS_CALL16:
@@ -8740,21 +8733,10 @@ _bfd_mips_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
 	case R_MIPS_NONE:
 	case R_MIPS_JALR:
 	case R_MICROMIPS_JALR:
-	  /* These relocations have empty fields and are purely there to
-	     provide link information.  The symbol value doesn't matter.  */
-	  constrain_symbol_p = FALSE;
-	  break;
-
 	case R_MIPS_GPREL16:
 	case R_MIPS_GPREL32:
 	case R_MIPS16_GPREL:
 	case R_MICROMIPS_GPREL16:
-	  /* GP-relative relocations always resolve to a definition in a
-	     regular input file, ignoring the one-definition rule.  This is
-	     important for the GP setup sequence in NewABI code, which
-	     always resolves to a local function even if other relocations
-	     against the symbol wouldn't.  */
-	  constrain_symbol_p = FALSE;
 	  break;
 
 	case R_MIPS_32:
@@ -8801,21 +8783,18 @@ _bfd_mips_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
 
       if (h)
 	{
-	  if (constrain_symbol_p)
-	    {
-	      if (!can_make_dynamic_p)
-		((struct mips_elf_link_hash_entry *) h)->has_static_relocs = 1;
-
-	      if (!call_reloc_p)
-		h->pointer_equality_needed = 1;
-
-	      /* We must not create a stub for a symbol that has
-		 relocations related to taking the function's address.
-		 This doesn't apply to VxWorks, where CALL relocs refer
-		 to a .got.plt entry instead of a normal .got entry.  */
-	      if (!htab->is_vxworks && (!can_make_dynamic_p || !call_reloc_p))
-		((struct mips_elf_link_hash_entry *) h)->no_fn_stub = TRUE;
-	    }
+	  if (!can_make_dynamic_p)
+	    ((struct mips_elf_link_hash_entry *) h)->has_static_relocs = 1;
+
+	  if (!call_reloc_p)
+	    h->pointer_equality_needed = 1;
+
+	  /* We must not create a stub for a symbol that has
+	     relocations related to taking the function's address.
+	     This doesn't apply to VxWorks, where CALL relocs refer
+	     to a .got.plt entry instead of a normal .got entry.  */
+	  if (!htab->is_vxworks && (!can_make_dynamic_p || !call_reloc_p))
+	    ((struct mips_elf_link_hash_entry *) h)->no_fn_stub = TRUE;
 
 	  /* Relocations against the special VxWorks __GOTT_BASE__ and
 	     __GOTT_INDEX__ symbols must be left to the loader.  Allocate
diff --git a/ld/testsuite/ld-elf/comm-data.exp b/ld/testsuite/ld-elf/comm-data.exp
index a53a77123c..c735fe244c 100644
--- a/ld/testsuite/ld-elf/comm-data.exp
+++ b/ld/testsuite/ld-elf/comm-data.exp
@@ -79,12 +79,6 @@ setup_xfail "bfin-*-*"
 
 setup_xfail "arm*-*-*" "ld/13802"
 
-# List targets here that keep copy relocs rather than eliminating
-# them where possible in favour to dynamic relocs in the relevant
-# loadable sections; see also the "-z nocopyreloc" command-line
-# option and the ELIMINATE_COPY_RELOCS macro some backends use.
-set copy_reloc [expr [istarget mn10300-*-*] || [istarget vax-*-*]]
-
 # Verify that a common symbol has been converted to an undefined
 # reference to the global symbol of the same name defined above
 # and that the debug reference has been dropped.
@@ -95,12 +89,9 @@ run_ld_link_tests [list \
 	"$AFLAGS" \
 	{ comm-data2.s } \
 	[list \
-	    [list readelf -s \
-		[expr { $copy_reloc ? "comm-data2r.sd" : "comm-data2.sd"}]] \
-	    [list readelf -r \
-		[expr { $copy_reloc ? "comm-data2r.rd" : "comm-data2.rd"}]] \
-	    [list readelf "-x .debug_foo" \
-		[expr { $copy_reloc ? "comm-data2r.xd" : "comm-data2.xd"}]]] \
+	    [list readelf -s comm-data2.sd] \
+	    [list readelf -r comm-data2.rd] \
+	    [list readelf "-x .debug_foo" comm-data2.xd]] \
 	"comm-data" \
     ] \
     [list \
diff --git a/ld/testsuite/ld-elf/comm-data2r.rd b/ld/testsuite/ld-elf/comm-data2r.rd
deleted file mode 100644
index 64c0396d26..0000000000
--- a/ld/testsuite/ld-elf/comm-data2r.rd
+++ /dev/null
@@ -1,3 +0,0 @@
-Relocation section '\.rela\.dyn' at offset 0x[0-9a-f]+ contains 1 entry:
- +Offset +Info +Type +Sym\.Value +Sym\. Name \+ Addend
-0*12340000 +[0-9a-f]+ +R_.*_COPY +0*12340000 +foo \+ 0
diff --git a/ld/testsuite/ld-elf/comm-data2r.sd b/ld/testsuite/ld-elf/comm-data2r.sd
deleted file mode 100644
index 685b0befd1..0000000000
--- a/ld/testsuite/ld-elf/comm-data2r.sd
+++ /dev/null
@@ -1,10 +0,0 @@
-Symbol table '\.dynsym' contains [0-9]+ entries:
- +Num: +Value +Size +Type +Bind +Vis +Ndx +Name
-#...
- +[0-9]+: +0*12340000 +4 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo
-#...
-Symbol table '\.symtab' contains [0-9]+ entries:
- +Num: +Value +Size +Type +Bind +Vis +Ndx +Name
-#...
- +[0-9]+: +0*12340000 +4 +OBJECT +GLOBAL +DEFAULT +[0-9]+ +foo
-#pass
diff --git a/ld/testsuite/ld-elf/comm-data2r.xd b/ld/testsuite/ld-elf/comm-data2r.xd
deleted file mode 100644
index 58f6f2a88f..0000000000
--- a/ld/testsuite/ld-elf/comm-data2r.xd
+++ /dev/null
@@ -1,2 +0,0 @@
-Hex dump of section '\.debug_foo':
- +0x0*76540000 (?:12340000 00000000|00003412 00000000|00000000 00003412) 00000000 00000000 .*
-- 
2.26.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: V3 [PATCH] ELF: Don't check relocations in non-loaded, non-alloced sections
  2020-06-03 18:55       ` V3 " H.J. Lu
@ 2020-06-04  7:47         ` Alan Modra
  2020-06-04 11:24           ` V4 " H.J. Lu
  0 siblings, 1 reply; 10+ messages in thread
From: Alan Modra @ 2020-06-04  7:47 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Nick Clifton, Maciej W. Rozycki, Fangrui Song, Binutils

On Wed, Jun 03, 2020 at 11:55:08AM -0700, H.J. Lu wrote:
> --- a/bfd/elfxx-mips.c
> +++ b/bfd/elfxx-mips.c
> @@ -8618,7 +8618,6 @@ _bfd_mips_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
>        struct elf_link_hash_entry *h;
>        bfd_boolean can_make_dynamic_p;
>        bfd_boolean call_reloc_p;
> -      bfd_boolean constrain_symbol_p;
>  
>        r_symndx = ELF_R_SYM (abfd, rel->r_info);
>        r_type = ELF_R_TYPE (abfd, rel->r_info);
> @@ -8653,12 +8652,6 @@ _bfd_mips_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
>  	 and if pointer equality therefore doesn't matter.  */
>        call_reloc_p = FALSE;
>  
> -      /* Set CONSTRAIN_SYMBOL_P if we need to take the relocation
> -	 into account when deciding how to define the symbol.
> -	 Relocations in nonallocatable sections such as .pdr and
> -	 .debug* should have no effect.  */
> -      constrain_symbol_p = ((sec->flags & SEC_ALLOC) != 0);
> -

So "normal" sections will result in constrain_symbol_p being true.

>        switch (r_type)
>  	{
>  	case R_MIPS_CALL16:
> @@ -8740,21 +8733,10 @@ _bfd_mips_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
>  	case R_MIPS_NONE:
>  	case R_MIPS_JALR:
>  	case R_MICROMIPS_JALR:
> -	  /* These relocations have empty fields and are purely there to
> -	     provide link information.  The symbol value doesn't matter.  */
> -	  constrain_symbol_p = FALSE;
> -	  break;
> -

But in for some special relocs it should be false.  I think removing
constrain_symbol_p is just wrong.  You should keep it but initialise
to true.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 10+ messages in thread

* V4 [PATCH] ELF: Don't check relocations in non-loaded, non-alloced sections
  2020-06-04  7:47         ` Alan Modra
@ 2020-06-04 11:24           ` H.J. Lu
  2020-06-04 12:41             ` Alan Modra
  0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2020-06-04 11:24 UTC (permalink / raw)
  To: Alan Modra; +Cc: Nick Clifton, Maciej W. Rozycki, Fangrui Song, Binutils

[-- Attachment #1: Type: text/plain, Size: 1828 bytes --]

On Thu, Jun 4, 2020 at 12:47 AM Alan Modra <amodra@gmail.com> wrote:
>
> On Wed, Jun 03, 2020 at 11:55:08AM -0700, H.J. Lu wrote:
> > --- a/bfd/elfxx-mips.c
> > +++ b/bfd/elfxx-mips.c
> > @@ -8618,7 +8618,6 @@ _bfd_mips_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
> >        struct elf_link_hash_entry *h;
> >        bfd_boolean can_make_dynamic_p;
> >        bfd_boolean call_reloc_p;
> > -      bfd_boolean constrain_symbol_p;
> >
> >        r_symndx = ELF_R_SYM (abfd, rel->r_info);
> >        r_type = ELF_R_TYPE (abfd, rel->r_info);
> > @@ -8653,12 +8652,6 @@ _bfd_mips_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
> >        and if pointer equality therefore doesn't matter.  */
> >        call_reloc_p = FALSE;
> >
> > -      /* Set CONSTRAIN_SYMBOL_P if we need to take the relocation
> > -      into account when deciding how to define the symbol.
> > -      Relocations in nonallocatable sections such as .pdr and
> > -      .debug* should have no effect.  */
> > -      constrain_symbol_p = ((sec->flags & SEC_ALLOC) != 0);
> > -
>
> So "normal" sections will result in constrain_symbol_p being true.
>
> >        switch (r_type)
> >       {
> >       case R_MIPS_CALL16:
> > @@ -8740,21 +8733,10 @@ _bfd_mips_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
> >       case R_MIPS_NONE:
> >       case R_MIPS_JALR:
> >       case R_MICROMIPS_JALR:
> > -       /* These relocations have empty fields and are purely there to
> > -          provide link information.  The symbol value doesn't matter.  */
> > -       constrain_symbol_p = FALSE;
> > -       break;
> > -
>
> But in for some special relocs it should be false.  I think removing
> constrain_symbol_p is just wrong.  You should keep it but initialise
> to true.

Done.

Here is the updated patch.   OK for master?

-- 
H.J.

[-- Attachment #2: 0001-ELF-Don-t-check-relocations-in-non-loaded-non-alloce.patch --]
[-- Type: application/x-patch, Size: 19234 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: V4 [PATCH] ELF: Don't check relocations in non-loaded, non-alloced sections
  2020-06-04 11:24           ` V4 " H.J. Lu
@ 2020-06-04 12:41             ` Alan Modra
  2020-06-05  3:07               ` Alan Modra
  0 siblings, 1 reply; 10+ messages in thread
From: Alan Modra @ 2020-06-04 12:41 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Nick Clifton, Maciej W. Rozycki, Fangrui Song, Binutils

On Thu, Jun 04, 2020 at 04:24:36AM -0700, H.J. Lu wrote:
> Here is the updated patch.   OK for master?

Looks OK to me.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: V4 [PATCH] ELF: Don't check relocations in non-loaded, non-alloced sections
  2020-06-04 12:41             ` Alan Modra
@ 2020-06-05  3:07               ` Alan Modra
  2020-06-05 12:32                 ` [PATCH] bfin: Skip non SEC_ALLOC section H.J. Lu
  0 siblings, 1 reply; 10+ messages in thread
From: Alan Modra @ 2020-06-05  3:07 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On Thu, Jun 04, 2020 at 10:11:20PM +0930, Alan Modra wrote:
> On Thu, Jun 04, 2020 at 04:24:36AM -0700, H.J. Lu wrote:
> > Here is the updated patch.   OK for master?
> 
> Looks OK to me.

This patch may have caused the following.  The first bfin fail is due
to bfinfdpic_relocs_info_for_local returning NULL since relocs_info is
NULL, which in turn is because check_relocs isn't called and thus
doesn't create a got section.  _bfin_create_got_section creates the
relocs_info hash table.

bfin-linux-uclibc  +FAIL: ld-elf/compress1a
bfin-linux-uclibc  +FAIL: ld-elf/compressed1a
bfin-linux-uclibc  +FAIL: ld-elf/pr12851
bfin-linux-uclibc  +FAIL: ld-elf/pr24526
bfin-linux-uclibc  +FAIL: ld-elf/pr25021
bfin-linux-uclibc  +FAIL: ld-elf/stab
bfin-linux-uclibc  +FAIL: --gc-sections with multiple debug sections for a function section
bfin-linux-uclibc  +FAIL: --gc-sections with relocations in debug section
bfin-linux-uclibc  +FAIL: Preserve default . = 0
bfin-linux-uclibc  +FAIL: Preserve explicit . = 0

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] bfin: Skip non SEC_ALLOC section
  2020-06-05  3:07               ` Alan Modra
@ 2020-06-05 12:32                 ` H.J. Lu
  0 siblings, 0 replies; 10+ messages in thread
From: H.J. Lu @ 2020-06-05 12:32 UTC (permalink / raw)
  To: Alan Modra; +Cc: Binutils

[-- Attachment #1: Type: text/plain, Size: 1158 bytes --]

On Thu, Jun 4, 2020 at 8:07 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Thu, Jun 04, 2020 at 10:11:20PM +0930, Alan Modra wrote:
> > On Thu, Jun 04, 2020 at 04:24:36AM -0700, H.J. Lu wrote:
> > > Here is the updated patch.   OK for master?
> >
> > Looks OK to me.
>
> This patch may have caused the following.  The first bfin fail is due
> to bfinfdpic_relocs_info_for_local returning NULL since relocs_info is
> NULL, which in turn is because check_relocs isn't called and thus
> doesn't create a got section.  _bfin_create_got_section creates the
> relocs_info hash table.
>
> bfin-linux-uclibc  +FAIL: ld-elf/compress1a
> bfin-linux-uclibc  +FAIL: ld-elf/compressed1a
> bfin-linux-uclibc  +FAIL: ld-elf/pr12851
> bfin-linux-uclibc  +FAIL: ld-elf/pr24526
> bfin-linux-uclibc  +FAIL: ld-elf/pr25021
> bfin-linux-uclibc  +FAIL: ld-elf/stab
> bfin-linux-uclibc  +FAIL: --gc-sections with multiple debug sections for a function section
> bfin-linux-uclibc  +FAIL: --gc-sections with relocations in debug section
> bfin-linux-uclibc  +FAIL: Preserve default . = 0
> bfin-linux-uclibc  +FAIL: Preserve explicit . = 0
>

I am checking in this patch.

-- 
H.J.

[-- Attachment #2: 0001-bfin-Skip-non-SEC_ALLOC-section.patch --]
[-- Type: text/x-patch, Size: 1229 bytes --]

From 981f151804e47290f4dcff507aeb530b3334ac17 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 5 Jun 2020 05:30:25 -0700
Subject: [PATCH] bfin: Skip non SEC_ALLOC section

	* elf32-bfin.c (bfinfdpic_relocate_section): Skip non SEC_ALLOC
	section.
---
 bfd/ChangeLog    | 5 +++++
 bfd/elf32-bfin.c | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 011a49d5ec..4971b87828 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,8 @@
+2020-06-05  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* elf32-bfin.c (bfinfdpic_relocate_section): Skip non SEC_ALLOC
+	section.
+
 2020-06-05  Nick Clifton  <nickc@redhat.com>
 
 	* pdp11.c (aout_link_add_symbols): Fix use before initialisation
diff --git a/bfd/elf32-bfin.c b/bfd/elf32-bfin.c
index b06daf507e..e067cdeaad 100644
--- a/bfd/elf32-bfin.c
+++ b/bfd/elf32-bfin.c
@@ -2614,6 +2614,9 @@ bfinfdpic_relocate_section (bfd * output_bfd,
 	case R_BFIN_FUNCDESC_GOTOFFLO:
 	case R_BFIN_FUNCDESC:
 	case R_BFIN_FUNCDESC_VALUE:
+	  if ((input_section->flags & SEC_ALLOC) == 0)
+	    break;
+
 	  if (h != NULL)
 	    picrel = bfinfdpic_relocs_info_for_global (bfinfdpic_relocs_info
 						       (info), input_bfd, h,
-- 
2.26.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-06-05 12:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-02 20:38 [PATCH] ELF: Don't check relocations in non-loaded, non-alloced sections H.J. Lu
2020-06-03  0:05 ` Fangrui Song
     [not found] ` <MWHPR12MB14567E24AED5430AAE2497A8CB880@MWHPR12MB1456.namprd12.prod.outlook.com>
2020-06-03  1:29   ` H.J. Lu
2020-06-03 16:48     ` V2 " H.J. Lu
2020-06-03 18:55       ` V3 " H.J. Lu
2020-06-04  7:47         ` Alan Modra
2020-06-04 11:24           ` V4 " H.J. Lu
2020-06-04 12:41             ` Alan Modra
2020-06-05  3:07               ` Alan Modra
2020-06-05 12:32                 ` [PATCH] bfin: Skip non SEC_ALLOC section H.J. Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).