xtensa: don't emit dynamic relocation for weak undefined symbol

Message ID 20180705160445.19834-1-jcmvbkbc@gmail.com
State New
Headers show
Series
  • xtensa: don't emit dynamic relocation for weak undefined symbol
Related show

Commit Message

Max Filippov July 5, 2018, 4:04 p.m.
Resolved reference to a weak undefined symbol in PIE must not have
a dynamic relative relocation against itself, otherwise the value of a
reference will be changed from 0 to the base of executable, breaking
code like the following:

  void weak_function (void);
  if (weak_function)
    weak_function ();

This fixes tests for PR ld/22269 and a number of PIE tests in xtensa gcc
testsuite.

bfd/
2018-07-05  Max Filippov  <jcmvbkbc@gmail.com>

	* elf32-xtensa.c (elf_xtensa_allocate_dynrelocs): Don't allocate
	space for dynamic relocation for undefined weak symbol.
	(elf_xtensa_relocate_section): Don't emit R_XTENSA_RELATIVE
	relocation for undefined weak symbols.
	(shrink_dynamic_reloc_sections): Don't shrink dynamic relocation
	section for relocations against undefined weak symbols.
---
 bfd/elf32-xtensa.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

-- 
2.11.0

Comments

augustine.sterling@gmail.com July 6, 2018, 7:09 p.m. | #1
On Thu, Jul 5, 2018 at 9:04 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> bfd/

> 2018-07-05  Max Filippov  <jcmvbkbc@gmail.com>

>

>         * elf32-xtensa.c (elf_xtensa_allocate_dynrelocs): Don't allocate

>         space for dynamic relocation for undefined weak symbol.

>         (elf_xtensa_relocate_section): Don't emit R_XTENSA_RELATIVE

>         relocation for undefined weak symbols.

>         (shrink_dynamic_reloc_sections): Don't shrink dynamic relocation

>         section for relocations against undefined weak symbols.


Approved. Please apply.
Max Filippov July 6, 2018, 11:08 p.m. | #2
On Fri, Jul 6, 2018 at 12:09 PM, augustine.sterling@gmail.com
<augustine.sterling@gmail.com> wrote:
> On Thu, Jul 5, 2018 at 9:04 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:

>> bfd/

>> 2018-07-05  Max Filippov  <jcmvbkbc@gmail.com>

>>

>>         * elf32-xtensa.c (elf_xtensa_allocate_dynrelocs): Don't allocate

>>         space for dynamic relocation for undefined weak symbol.

>>         (elf_xtensa_relocate_section): Don't emit R_XTENSA_RELATIVE

>>         relocation for undefined weak symbols.

>>         (shrink_dynamic_reloc_sections): Don't shrink dynamic relocation

>>         section for relocations against undefined weak symbols.

>

> Approved. Please apply.


Applied to master. Thanks!

-- Max
Max Filippov July 10, 2018, 4:52 p.m. | #3
Hi Nick,

I'd like to backport the following patch to the binutils-2_31-branch, is it OK?

On Thu, Jul 5, 2018 at 9:04 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> Resolved reference to a weak undefined symbol in PIE must not have

> a dynamic relative relocation against itself, otherwise the value of a

> reference will be changed from 0 to the base of executable, breaking

> code like the following:

>

>   void weak_function (void);

>   if (weak_function)

>     weak_function ();

>

> This fixes tests for PR ld/22269 and a number of PIE tests in xtensa gcc

> testsuite.

>

> bfd/

> 2018-07-05  Max Filippov  <jcmvbkbc@gmail.com>

>

>         * elf32-xtensa.c (elf_xtensa_allocate_dynrelocs): Don't allocate

>         space for dynamic relocation for undefined weak symbol.

>         (elf_xtensa_relocate_section): Don't emit R_XTENSA_RELATIVE

>         relocation for undefined weak symbols.

>         (shrink_dynamic_reloc_sections): Don't shrink dynamic relocation

>         section for relocations against undefined weak symbols.

> ---

>  bfd/elf32-xtensa.c | 13 +++++++++++--

>  1 file changed, 11 insertions(+), 2 deletions(-)

>

> diff --git a/bfd/elf32-xtensa.c b/bfd/elf32-xtensa.c

> index cf7e12797e17..e012f5e29d8d 100644

> --- a/bfd/elf32-xtensa.c

> +++ b/bfd/elf32-xtensa.c

> @@ -1444,6 +1444,10 @@ elf_xtensa_allocate_dynrelocs (struct elf_link_hash_entry *h, void *arg)

>    if (! elf_xtensa_dynamic_symbol_p (h, info))

>      elf_xtensa_make_sym_local (info, h);

>

> +  if (! elf_xtensa_dynamic_symbol_p (h, info)

> +      && h->root.type == bfd_link_hash_undefweak)

> +    return TRUE;

> +

>    if (h->plt.refcount > 0)

>      htab->elf.srelplt->size += (h->plt.refcount * sizeof (Elf32_External_Rela));

>

> @@ -2784,12 +2788,16 @@ elf_xtensa_relocate_section (bfd *output_bfd,

>                         }

>                       unresolved_reloc = FALSE;

>                     }

> -                 else

> +                 else if (!is_weak_undef)

>                     {

>                       /* Generate a RELATIVE relocation.  */

>                       outrel.r_info = ELF32_R_INFO (0, R_XTENSA_RELATIVE);

>                       outrel.r_addend = 0;

>                     }

> +                 else

> +                   {

> +                     continue;

> +                   }

>                 }

>

>               loc = (srel->contents

> @@ -10028,7 +10036,8 @@ shrink_dynamic_reloc_sections (struct bfd_link_info *info,

>

>    if ((r_type == R_XTENSA_32 || r_type == R_XTENSA_PLT)

>        && (input_section->flags & SEC_ALLOC) != 0

> -      && (dynamic_symbol || bfd_link_pic (info)))

> +      && (dynamic_symbol || bfd_link_pic (info))

> +      && (!h || h->root.type != bfd_link_hash_undefweak))

>      {

>        asection *srel;

>        bfd_boolean is_plt = FALSE;

> --

> 2.11.0

>


-- 
Thanks.
-- Max
Nick Clifton July 11, 2018, 4:25 p.m. | #4
Hi Max,

> I'd like to backport the following patch to the binutils-2_31-branch, is it OK?


Yes - this is fine - please go ahead.

Cheers
  Nick
Max Filippov July 11, 2018, 4:43 p.m. | #5
On Wed, Jul 11, 2018 at 9:25 AM, Nick Clifton <nickc@redhat.com> wrote:
>> I'd like to backport the following patch to the binutils-2_31-branch, is it OK?

>

> Yes - this is fine - please go ahead.


Thanks, applied.

-- Max

Patch

diff --git a/bfd/elf32-xtensa.c b/bfd/elf32-xtensa.c
index cf7e12797e17..e012f5e29d8d 100644
--- a/bfd/elf32-xtensa.c
+++ b/bfd/elf32-xtensa.c
@@ -1444,6 +1444,10 @@  elf_xtensa_allocate_dynrelocs (struct elf_link_hash_entry *h, void *arg)
   if (! elf_xtensa_dynamic_symbol_p (h, info))
     elf_xtensa_make_sym_local (info, h);
 
+  if (! elf_xtensa_dynamic_symbol_p (h, info)
+      && h->root.type == bfd_link_hash_undefweak)
+    return TRUE;
+
   if (h->plt.refcount > 0)
     htab->elf.srelplt->size += (h->plt.refcount * sizeof (Elf32_External_Rela));
 
@@ -2784,12 +2788,16 @@  elf_xtensa_relocate_section (bfd *output_bfd,
 			}
 		      unresolved_reloc = FALSE;
 		    }
-		  else
+		  else if (!is_weak_undef)
 		    {
 		      /* Generate a RELATIVE relocation.  */
 		      outrel.r_info = ELF32_R_INFO (0, R_XTENSA_RELATIVE);
 		      outrel.r_addend = 0;
 		    }
+		  else
+		    {
+		      continue;
+		    }
 		}
 
 	      loc = (srel->contents
@@ -10028,7 +10036,8 @@  shrink_dynamic_reloc_sections (struct bfd_link_info *info,
 
   if ((r_type == R_XTENSA_32 || r_type == R_XTENSA_PLT)
       && (input_section->flags & SEC_ALLOC) != 0
-      && (dynamic_symbol || bfd_link_pic (info)))
+      && (dynamic_symbol || bfd_link_pic (info))
+      && (!h || h->root.type != bfd_link_hash_undefweak))
     {
       asection *srel;
       bfd_boolean is_plt = FALSE;