[SPARC] Fix GOT relocation overflow

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[SPARC] Fix GOT relocation overflow

Eric Botcazou-3
Hi,

there are 2 failures left in the linker testsuite on SPARC64/Linux:

child killed: segmentation violation
FAIL: visibility (hidden_normal) (PIC main)

< main_visibility_checkweak () == 0
---
> main_visibility_checkweak () == 1
child process exited abnormally
FAIL: visibility (hidden_weak)

and they are caused by 2 different issues leading to the same end effect: the
overflow of the relocation section for the GOT, i.e. the linker generates too
many dynamic relocations for the GOT wrt the size of the relocation section,
leading to memory corruption and missing relocations in the final binary.

The first issue was introduced by:
  https://sourceware.org/ml/binutils/2017-06/msg00368.html
which makes the linker generate more R_SPARC_RELATIVE relocations for the GOT
without adjusting the size of the relocation section.  This is fixed by (1)
preventively adjusting this size in allocate_dynrelocs and (2) generating
R_SPARC_NONE relocations if needed when R_SPARC_GOTDATA_OP is relaxed.

The second issue is that we generate a GOT relocation for an undefined weak
symbol with non-default visibility in a PIC binary without accounting for that
in the size of the relocation section.  My understanding is that the address
of such an abomination should resolve to 0 at run time, which is achieved by
not generating the relocation at all, i.e. leaving the GOT entry zeroed.

The patch also contains minor tweaks and adjustments to comments, as well as
an assertion in sparc_elf_append_rela so as to abort the link on relocation
overflow instead of corrupting either the memory or the final binary.

Tested on SPARC64/Linux with a GCC bootstrap, OK for mainline and 2.30 branch?


2018-02-08  Eric Botcazou  <[hidden email]>

bfd/
        * elfxx-sparc.c (UNDEFINED_WEAK_RESOLVED_TO_ZERO): Reorder conditions.
        (sparc_elf_append_rela): Assert that there is enough room in the section.
        (_bfd_sparc_elf_copy_indirect_symbol): Fix formatting.
        (_bfd_sparc_elf_adjust_dynamic_symbol): Minor tweak.
        (allocate_dynrelocs): Remove outdated comments and reorder conditions.
        For a symbol subject to a GOT relocation, reserve a slot in the relocation
        section if the symbol isn't dynamic and we are in PIC mode.
        (_bfd_sparc_elf_relocate_section) <R_SPARC_GOTDATA_OP>: If the relocation
        is relaxed and a slot was reserved, generate a R_SPARC_NONE relocation.
        <R_SPARC_GOTDATA_OP_HIX22>: Adjust comments.
        <R_SPARC_PC10>: Reorder conditions.  Remove always-false assertion.
        (_bfd_sparc_elf_finish_dynamic_symbol): Rename local_undefweak into
        resolved_to_zero.  Do not generate a dynamic GOT relocation for an
        undefined weak symbol with non-default visibility.
        Remove superfluous 'else' and fix formatting.

--
Eric Botcazou

p.diff (10K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [SPARC] Fix GOT relocation overflow

Nick Clifton
Hi Eric,

> Tested on SPARC64/Linux with a GCC bootstrap, OK for mainline and 2.30 branch?

Approved for both - please apply.

> 2018-02-08  Eric Botcazou  <[hidden email]>
>
> bfd/
> * elfxx-sparc.c (UNDEFINED_WEAK_RESOLVED_TO_ZERO): Reorder conditions.
> (sparc_elf_append_rela): Assert that there is enough room in the section.
> (_bfd_sparc_elf_copy_indirect_symbol): Fix formatting.
> (_bfd_sparc_elf_adjust_dynamic_symbol): Minor tweak.
> (allocate_dynrelocs): Remove outdated comments and reorder conditions.
> For a symbol subject to a GOT relocation, reserve a slot in the relocation
> section if the symbol isn't dynamic and we are in PIC mode.
> (_bfd_sparc_elf_relocate_section) <R_SPARC_GOTDATA_OP>: If the relocation
> is relaxed and a slot was reserved, generate a R_SPARC_NONE relocation.
> <R_SPARC_GOTDATA_OP_HIX22>: Adjust comments.
> <R_SPARC_PC10>: Reorder conditions.  Remove always-false assertion.
> (_bfd_sparc_elf_finish_dynamic_symbol): Rename local_undefweak into
> resolved_to_zero.  Do not generate a dynamic GOT relocation for an
> undefined weak symbol with non-default visibility.
> Remove superfluous 'else' and fix formatting.
>

Cheers
  Nick