]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commit - bfd/elfnn-aarch64.c
Predicate fixes around srelcaps and capability GOT relocations
authorMatthew Malcomson <matthew.malcomson@arm.com>
Thu, 28 Apr 2022 08:50:30 +0000 (09:50 +0100)
committerMatthew Malcomson <matthew.malcomson@arm.com>
Thu, 28 Apr 2022 08:52:45 +0000 (09:52 +0100)
commitd9f4f6fc896adcb8abec896598ac42154bafc3a5
tree8b1d35a7812dd041459d9bdbf088807453037a54
parentb2cb8a193229c81cd4a1248e011351cb8ecc6ea8
Predicate fixes around srelcaps and capability GOT relocations

This patch clears up some confusing checks around where to place
capability relocations initialising GOT entries.

Our handling of capability entries for the GOT had a common mistake in
the predicates that we used.  Statically linked executables need to have
all capability relocations contiguous in order to be able to mark their
start and end with __rela_dyn_{start,end} symbols.  These symbols are
used by the runtime to find dynamic capability relocations that must be
performed.  They are not needed when dynamically linking as then it is
the responsibility of the dynamic loader to perform these relocations.

We generally used `bfd_link_executable (info) && !bfd_link_pic (info)`
to check for statically linked executables.  This predicate includes
dynamically linked PDE's.  In most cases seen we do not want to include
dynamically linked PDE's.

This problem manifested in a few different ways.  When the srelcaps
section was non-empty we would generate the __rela_dyn_{start,end}
symbols -- which meant that these would be unnecessarily emitted for
dynamically linked PDE's.  In one case we erroneously increased the size
of this section on seeing non-capability relocations, and since no
relocations were actually added we would see a set of uninitialised
relocations.

Here we inspected all places in the code handling the srelcaps section
and identified 5 problems.  We add tests for those problems which can
be seen (some of the problems are only problems once others have been
fixed) and fix them all.

Below we describe what was happening for each of the problems in turn:

---
Avoid non-capability relocations during srelcaps sizing

elfNN_aarch64_allocate_dynrelocs increases the size for relocation
sections based on the number of dynamic symbol relocations.

When increasing the size of the section in which we store capability
relocations (recorded as srelcaps in the link hash table) our
conditional erroneously included non-capability relocations.  We were
hence allocating space in a section like .rela.dyn for relocations
populating the GOT with addresses of non-capability symbols in a
statically linked executable (for non-Morello compilation).

This change widens the original if clause so it should catch CAP
relocations that should go in srelgot, and tightens the fallback else if
clause in allocate_dynrelocs to only act on capability entries in the
GOT, since those are the only ones not already caught which still need
relocations to populate.

Implementation notes:
While not necessary, we also stop the fallback conditional checking
!bfd_link_pic and instead put an assertion that we only ever enter the
conditions block in the case of !bfd_link_pic && !dynamic.
This is done to emphasise that this condition is there to account for
all the capability GOT entries for the hash table which need relocations
and are not caught by the existing code.  The fact that this should only
happen when building static executables seems like an emergent property
rather than the thing we would want to check against.

This is tested with no-morello-syms-static.

---
size_dynamic_sections use srelcaps for statically linked executables
and srelgot for dynamically linked binaries.

When creating a statically linked executable the srelcaps section will
always be initialised and that is where we should put all capability
relocations.  When creating a dynamically linked executable the srelcaps
may or may not be initialised (depending on if we saw CAPINIT
relocations) and either way we should put GOT relocations into the
srelgot section.

Though there is no functional change to look for, this code path is
exercised with the morello-static-got test and
morello-dynamic-link-rela-dyn for statically linked and dynamically
linked PDE's respectively.

---
Capability GOT relocations go in .rela.got for dynamically linked PDEs

final_link_relocate generates GOT relocations for entries in the GOT
that are not handled by the generic ELF relocation code.  For Morello
we require relocations for any entry in the GOT that needs to be a
capability.

For *static* linking we keep track of a section specifically for
capability relocations.  This is done in order to be able to emit
__rela_dyn_{start,end} symbols at the start and end of an array of these
relocations (see commit 40bbb79e5a3 for when this was introduced and
commit 8d4edc5f8 for when we ensured that MORELLO_RELATIVE relocations
into the GOT were included in this section).

The clause in final_link_relocate that decides whether we should put
MORELLO_RELATIVE relocations for initialising capability GOT entries
into this special section currently includes dynamically linked PDE's.
This is unnecessary, since for dynamically linked binaries we do not
want to emit such __rela_dyn_{start,end} symbols.

While this behaviuor is in general harmless (especially since both
input sections srelcaps and srelgot have the same output section in the
default linker scripts), this commit changes it for clarity of the code.
We now only put these relocations initialising GOT entries into the
srelcaps section if we require it for some reason.  The only time we do
require this is when statically linking binaries and we need the
__rela_dyn_* symbols.  Otherwise we put these entries into the `srelgot`
section which exists for holding GOT entries together.

Since this diff is not about a functional change we do not include a
testcase.  However we do ensure that the testcase
morello-dynamic-link-rela-dyn is written so as to exercise the codepath
which has changed.

---
Only ensure that srelcaps is initialised when required

In commit 8d4edc5f8 we started to ensure that capability relocations for
initialising GOT entries were stored next to dynamic RELATIVE
relocations arising from CAPINIT static relocations.

This was done in order to ensure that all relocations creating a
capability were stored next to each other, allowing us to mark the range
of capability relocations with __rela_dyn_{start,end} symbols.

We only need to do this for statically linked executables, for
dynamically linked executables the __rela_dyn_{start,end} symbols are
unnecessary.

When doing this, and there were no CAPINIT relocations that initialised
the srelcaps section, we set that srelcaps section to the same section
as srelgot.  Despite what the comment above this clause claimed we
mistakenly did this action when dynamically linking a PDE (i.e. we did
not *just* do this for static non-PIE binaries).

With recent changes that ensure we do not put anything in this srelcaps
section when not statically linking this makes no difference, but
changing the clause to correctly check for static linking is a nice
cleanup to have.

Since there is no observable change expected this diff has no
testcase, but the code path is exercised with morello-dynamic-got.

---
Only emit __rela_dyn_* symbols for statically linked exes

The intention of the code to emit these symbols in size_dynamic_sections
was only to emit symbols for statically linked executables.  We recently
noticed that the condition that has been used for this also included
dynamically linked PDE's.

Here we adjust the condition so that we only emit these symbols for
statically linked executables.

This allows initailisation code in glibc to be written much simpler,
since it does not need to determine whether the relocations have been
handled by the dynamic loader or not -- if the __rela_dyn_* symbols
exist then this is definitely a statically linked executable and the
relocations have not been handled by the dynamic loader.

This is tested with morello-dynamic-link-rela-dyn.
12 files changed:
bfd/elfnn-aarch64.c
ld/testsuite/ld-aarch64/aarch64-elf.exp
ld/testsuite/ld-aarch64/morello-dynamic-got.d [new file with mode: 0644]
ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn.d [new file with mode: 0644]
ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn.s [new file with mode: 0644]
ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn2.d [new file with mode: 0644]
ld/testsuite/ld-aarch64/morello-dynamic-local-got.d [new file with mode: 0644]
ld/testsuite/ld-aarch64/morello-dynamic-local-got.s [new file with mode: 0644]
ld/testsuite/ld-aarch64/morello-dynamic-relocs-lib.s [new file with mode: 0644]
ld/testsuite/ld-aarch64/morello-static-got.d [new file with mode: 0644]
ld/testsuite/ld-aarch64/morello-static-got.s [new file with mode: 0644]
ld/testsuite/ld-aarch64/no-morello-syms-static.d [new file with mode: 0644]