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.

############### Attachment also inlined for ease of reply ###############

diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c index 49c489744e860ea36709d13b78dbb8bd3c617adf..4f4f4f7418808fb032b1b6657073d4c6527f0565 100644 --- a/bfd/elfnn-aarch64.c +++ b/bfd/elfnn-aarch64.c @@ -7211,6 +7211,7 @@ elfNN_aarch64_final_link_relocate (reloc_howto_type *howto, off = symbol_got_offset (input_bfd, h, r_symndx); base_got = globals->root.sgot;

+ bfd_boolean is_dynamic = elf_hash_table (info)->dynamic_sections_created; bfd_boolean c64_reloc = (bfd_r_type == BFD_RELOC_MORELLO_LD128_GOT_LO12_NC || bfd_r_type == BFD_RELOC_MORELLO_ADR_GOT_PAGE); @@ -7235,8 +7236,6 @@ elfNN_aarch64_final_link_relocate (reloc_howto_type *howto, { bfd_vma addend = 0; bfd_vma frag_value; - bfd_boolean is_dynamic - = elf_hash_table (info)->dynamic_sections_created;

/* If a symbol is not dynamic and is not undefined weak, bind it locally and generate a RELATIVE relocation under PIC mode. @@ -7415,7 +7414,12 @@ elfNN_aarch64_final_link_relocate (reloc_howto_type *howto, { rtype = MORELLO_R (RELATIVE);

- if (bfd_link_executable (info) && !bfd_link_pic (info)) + /* Ensure that Morello RELATIVE relocations for static non-PIE + binaries are all stored in the same input section. This is + done so that we can mark that section with + __rela_dyn_{start,end} symbols for the runtime to find and + initialise relocations with. */ + if (bfd_link_executable (info) && !is_dynamic) s = globals->srelcaps;

outrel.r_addend = signed_addend; @@ -9084,7 +9088,7 @@ aarch64_elf_init_got_section (bfd *abfd, struct bfd_link_info *info) }

/* Track capability initialisation for static non-PIE binaries. */ - if (bfd_link_executable (info) && !bfd_link_pic (info) + if (bfd_link_executable (info) && !globals->root.dynamic_sections_created && globals->srelcaps == NULL) globals->srelcaps = globals->root.srelgot;

@@ -10222,18 +10226,26 @@ elfNN_aarch64_allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf) { h->got.offset = htab->root.sgot->size; htab->root.sgot->size += GOT_ENTRY_SIZE (htab); - if ((ELF_ST_VISIBILITY (h->other) == STV_DEFAULT - || h->root.type != bfd_link_hash_undefweak) - && (bfd_link_pic (info) - || WILL_CALL_FINISH_DYNAMIC_SYMBOL (dyn, 0, h)) - /* Undefined weak symbol in static PIE resolves to 0 without - any dynamic relocations. */ - && !UNDEFWEAK_NO_DYNAMIC_RELOC (info, h)) + if (((ELF_ST_VISIBILITY (h->other) == STV_DEFAULT + || h->root.type != bfd_link_hash_undefweak) + && (bfd_link_pic (info) + || WILL_CALL_FINISH_DYNAMIC_SYMBOL (dyn, 0, h)) + /* Undefined weak symbol in static PIE resolves to 0 without + any dynamic relocations. */ + && !UNDEFWEAK_NO_DYNAMIC_RELOC (info, h)) + /* Any capability relocations required in a dynamic binary + should go in the srelgot. */ + || ((got_type == GOT_CAP) && dyn)) { htab->root.srelgot->size += RELOC_SIZE (htab); } - else if (bfd_link_executable (info) && !bfd_link_pic (info)) - htab->srelcaps->size += RELOC_SIZE (htab); + else if (bfd_link_executable (info) && (got_type == GOT_CAP)) + { + /* If we have a capability relocation that is not handled by the + case above then this must be a statically linked executable. */ + BFD_ASSERT (!bfd_link_pic (info) && !dyn); + htab->srelcaps->size += RELOC_SIZE (htab); + } } else { @@ -10579,8 +10591,13 @@ elfNN_aarch64_size_dynamic_sections (bfd *output_bfd, htab->root.srelgot->size += RELOC_SIZE (htab); } /* Static binary; put relocs into srelcaps. */ - else if (bfd_link_executable (info) && (got_type & GOT_CAP)) + else if (bfd_link_executable (info) + && !htab->root.dynamic_sections_created + && (got_type & GOT_CAP)) htab->srelcaps->size += RELOC_SIZE (htab); + /* Else capability relocation needs to go into srelgot. */ + else if (got_type & GOT_CAP) + htab->root.srelgot->size += RELOC_SIZE (htab); } else { @@ -10606,7 +10623,7 @@ elfNN_aarch64_size_dynamic_sections (bfd *output_bfd, info);

if (bfd_link_executable (info) - && !bfd_link_pic (info) + && !htab->root.dynamic_sections_created && htab->srelcaps && htab->srelcaps->size > 0) { diff --git a/ld/testsuite/ld-aarch64/aarch64-elf.exp b/ld/testsuite/ld-aarch64/aarch64-elf.exp index 3e9cf4147f3012a12a0535bee3f6f7bc4c109a12..26d96d79249a007a6e57b6c7b3ea06ccf522d5d8 100644 --- a/ld/testsuite/ld-aarch64/aarch64-elf.exp +++ b/ld/testsuite/ld-aarch64/aarch64-elf.exp @@ -276,11 +276,16 @@ run_dump_test_lp64 "c64-ehdr-sized-reloc"

# Test for morello dynamic relocs can not be written in the usual manner since # we need to specify different `ld` command lines for different objects. -if { [ld_assemble $as $srcdir/$subdir/morello-dynamic-relocs-lib.s tmpdir/morello-dynamic-relocs-lib.o] +if { [ld_assemble_flags $as -march=morello+c64 $srcdir/$subdir/morello-dynamic-relocs-lib.s tmpdir/morello-dynamic-relocs-lib.o] && [ld_link $ld tmpdir/morello-dynamic-relocs.so "--shared tmpdir/morello-dynamic-relocs-lib.o"] } { run_dump_test_lp64 "morello-dynamic-relocs" + run_dump_test_lp64 "morello-dynamic-link-rela-dyn" + run_dump_test_lp64 "morello-dynamic-link-rela-dyn2" + run_dump_test_lp64 "morello-dynamic-local-got" }

+run_dump_test_lp64 "morello-static-got" +run_dump_test_lp64 "morello-dynamic-got" run_dump_test_lp64 "morello-dt-init-fini"

run_dump_test_lp64 "morello-capinit" @@ -300,6 +305,8 @@ run_dump_test_lp64 "morello-tlsdesc" run_dump_test_lp64 "morello-tlsdesc-static" run_dump_test_lp64 "morello-tlsdesc-staticpie"

+run_dump_test "no-morello-syms-static" + run_dump_test "reloc-overflow-bad"

# test addend correctness when --emit-relocs specified for non-relocatable obj. diff --git a/ld/testsuite/ld-aarch64/morello-dynamic-got.d b/ld/testsuite/ld-aarch64/morello-dynamic-got.d new file mode 100644 index 0000000000000000000000000000000000000000..810eb1033ef7003420791eeeca147170f0f97477 --- /dev/null +++ b/ld/testsuite/ld-aarch64/morello-dynamic-got.d @@ -0,0 +1,12 @@ +# This testcase is just to exercise some code rather than to test for an +# observable. We may as well check that the __rela_dyn_start symbol does not +# exists. +#source: morello-static-got.s +#as: -march=morello+c64 +#ld: -shared +#readelf: --symbols + +#failif +#... +.* __rela_dyn_start +#... diff --git a/ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn.d b/ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn.d new file mode 100644 index 0000000000000000000000000000000000000000..ee25827e8aa7ec88b56fa112610cc30350729530 --- /dev/null +++ b/ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn.d @@ -0,0 +1,20 @@ +# Test here is to ensure that __rela_dyn_* symbols are not emitted for +# dynamically linked objects. The code that was acting when we noticed this +# problem was correctly avoiding the behaviour for shared objects, but not for +# dynamically linked PDE's. Hence that's what this testcase is added for. +# +# N.b. aarch64-elf.exp compiles a shared libary for this test under +# tmpdir/morello-dynamic-relocs.so. We use that shared library for the test in +# the `ld` command below. +# +# This testcase is written partly to ensure a particular code path is +# exercised. That is the purpose of the local `val` symbol that we have a GOT +# relocation to. +#as: -march=morello+c64 +#ld: tmpdir/morello-dynamic-relocs.so +#readelf: --symbols + +#failif +#... +.* __rela_dyn_start +#... diff --git a/ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn.s b/ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn.s new file mode 100644 index 0000000000000000000000000000000000000000..3cffb35a8b82e93aa45acc841f0f661a320f4849 --- /dev/null +++ b/ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn.s @@ -0,0 +1,41 @@ + .arch morello+crc+c64 + .text + .align 2 + .global _start + .type _start, %function +_start: +.LFB0: + .cfi_startproc purecap + adrp c0, :got:weakval + ldr c0, [c0, #:got_lo12:weakval] + adrp c0, :got:globval + ldr c0, [c0, #:got_lo12:globval] + adrp c0, :got:val + ldr c0, [c0, #:got_lo12:val] + adrp c0, :got:var + ldr c0, [c0, #:got_lo12:var] + str wzr, [c0] + ret + .cfi_endproc +.LFE0: + .size _start, .-_start +.data + .type val, %object +val: + .byte 0x42 + .byte 0x42 + .byte 0x42 + .size val, .-val + .chericap val + .global globval +globval: + .byte 0x1 + .byte 0x1 + .byte 0x1 + .size globval, .-globval + .weak weakval +weakval: + .byte 0x1 + .byte 0x1 + .byte 0x1 + .size weakval, .-weakval diff --git a/ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn2.d b/ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn2.d new file mode 100644 index 0000000000000000000000000000000000000000..9de0c5f4f362ba8fb2d6c83624f22b4feae839f2 --- /dev/null +++ b/ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn2.d @@ -0,0 +1,14 @@ +# This testcase ensures that we have the expected number and type of +# relocations in our resultant binary. +#source: morello-dynamic-link-rela-dyn.s +#as: -march=morello+c64 +#ld: tmpdir/morello-dynamic-relocs.so +#readelf: --relocs + +Relocation section '.rela.dyn' at offset .* contains 5 entries: + Offset Info Type Sym. Value Sym. Name + Addend +.* 00000000e803 R_MORELLO_RELATIV 0 +.* 00000000e803 R_MORELLO_RELATIV 0 +.* 00000000e803 R_MORELLO_RELATIV 0 +.* 00000000e803 R_MORELLO_RELATIV 0 +.* 00010000e801 R_MORELLO_GLOB_DA 0000000000000000 var + 0 diff --git a/ld/testsuite/ld-aarch64/morello-dynamic-local-got.d b/ld/testsuite/ld-aarch64/morello-dynamic-local-got.d new file mode 100644 index 0000000000000000000000000000000000000000..3b607c9c3255ee5a4b191db274d7a1c29d7720cb --- /dev/null +++ b/ld/testsuite/ld-aarch64/morello-dynamic-local-got.d @@ -0,0 +1,16 @@ +# This testcase is written mostly to ensure a particular code path is +# exercised. That is the purpose of the local `x` symbol that we have a GOT +# relocation to. +# +# There is no particular observable for this code path -- a relocation is put +# in one input section rather than another (though both input sections have the +# same output section). May as well check that the __rela_dyn_* symbols are +# not emitted, since this is a dynamic symbol. +#as: -march=morello+crc+c64 +#ld: tmpdir/morello-dynamic-relocs.so +#readelf: --symbols + +#failif +#... +.* __rela_dyn_start +#... diff --git a/ld/testsuite/ld-aarch64/morello-dynamic-local-got.s b/ld/testsuite/ld-aarch64/morello-dynamic-local-got.s new file mode 100644 index 0000000000000000000000000000000000000000..e846bb7422cf626d54868db07836cd19c5b112e5 --- /dev/null +++ b/ld/testsuite/ld-aarch64/morello-dynamic-local-got.s @@ -0,0 +1,23 @@ + .text + .global x + .bss + .align 2 + .type x, %object + .size x, 4 +x: + .zero 4 + .text + .align 2 + .global _start + .type _start, %function +_start: +.LFB0: + .cfi_startproc purecap + adrp c0, :got:x + ldr c0, [c0, #:got_lo12:x] + ldr w0, [c0] + ret + .cfi_endproc +.LFE0: + .size _start, .-_start + diff --git a/ld/testsuite/ld-aarch64/morello-dynamic-relocs-lib.s b/ld/testsuite/ld-aarch64/morello-dynamic-relocs-lib.s index e4cd890c18bd2f39510273c04d36eaca7ee8ac04..80e84e5bc847a99b21c1da515749a3362842a641 100644 --- a/ld/testsuite/ld-aarch64/morello-dynamic-relocs-lib.s +++ b/ld/testsuite/ld-aarch64/morello-dynamic-relocs-lib.s @@ -7,4 +7,3 @@ .size var, 4 var: .zero 4 - diff --git a/ld/testsuite/ld-aarch64/morello-dynamic-relocs.d b/ld/testsuite/ld-aarch64/morello-dynamic-relocs.d index ce8d7b39e5db1caacc1dede4265725ef37e813c6..c7cfbd91626c97a1429f6b4a16606031dac8e256 100644 --- a/ld/testsuite/ld-aarch64/morello-dynamic-relocs.d +++ b/ld/testsuite/ld-aarch64/morello-dynamic-relocs.d @@ -7,6 +7,7 @@ # N.b. aarch64-elf.exp compiles a shared libary for this test under # tmpdir/morello-dynamic-relocs.so. We use that shared library for the test in # the `ld` command below. +#as: -march=morello+c64 #ld: tmpdir/morello-dynamic-relocs.so #readelf: --relocs

diff --git a/ld/testsuite/ld-aarch64/morello-static-got.d b/ld/testsuite/ld-aarch64/morello-static-got.d new file mode 100644 index 0000000000000000000000000000000000000000..0e24d97f2d1afa92e0c339b25174cf566de1bd09 --- /dev/null +++ b/ld/testsuite/ld-aarch64/morello-static-got.d @@ -0,0 +1,13 @@ +# This testcase is just to exercise some code rather than to test for an +# observable. We may as well check that the __rela_dyn_start symbol exists. +#as: -march=morello+c64 +#ld: -static +#readelf: --symbols --relocs + +Relocation section '.rela.dyn' at offset .* contains 1 entry: + Offset Info Type Sym. Value Sym. Name + Addend +.* 00000000e803 R_MORELLO_RELATIV 0 + +#... +.* __rela_dyn_start +#... diff --git a/ld/testsuite/ld-aarch64/morello-static-got.s b/ld/testsuite/ld-aarch64/morello-static-got.s new file mode 100644 index 0000000000000000000000000000000000000000..2a4e410bb469503bb765d83a3406274c3488ef05 --- /dev/null +++ b/ld/testsuite/ld-aarch64/morello-static-got.s @@ -0,0 +1,12 @@ +.data +val: + .byte 0x42 + .byte 0x42 + .byte 0x42 + .size val, .-val + +.text +.global _start +_start: + ldr c0, [c0, :got_lo12:val] + diff --git a/ld/testsuite/ld-aarch64/no-morello-syms-static.d b/ld/testsuite/ld-aarch64/no-morello-syms-static.d new file mode 100644 index 0000000000000000000000000000000000000000..e1b495b34a4ee9c21ceb4c232b6e75f35ff6fc39 --- /dev/null +++ b/ld/testsuite/ld-aarch64/no-morello-syms-static.d @@ -0,0 +1,13 @@ +# The emit-relocs-28 test was creating space for unnecessary relocations and +# correspondingly adding the __rela_dyn_{start,end} symbols to span them. +# This was happening because of Morello changes which applied on non-Morello +# links by mistake. This testcase is to ensure that that does not happen. +#source: emit-relocs-28.s +#as: -mabi=ilp32 +#ld: -m [aarch64_choose_ilp32_emul] --defsym globala=0x11000 --defsym globalb=0x45000 --defsym globalc=0x1234 -e0 --emit-relocs +#readelf: --symbols + +#failif +#... +.* __rela_dyn_start +#...