For dynamic symbol GOT entries, the linker emits relocations for that entry in finish_dynamic_symbol.

Since Morello capabilities always need dynamic relocations to initialise GOT entries at runtime, we need to emit relocations for any capability GOT entries. Two examples which are not needed for non-Morello linking are static linking and for global symbols defined and referenced in a PDE.

In order to ensure we emit those relocations we catch them in the existing clause of final_link_relocate that looks for GOT entries that require relocations which are not handled by finish_dynamic_symbol. Before this patch, the clause under which those relocations were emitted would include dynamic GOT entries in dynamically linked position dependent executables. These symbols hence had RELATIVE relocations emitted to initialise them in the executables GOT by final_link_relocate, and GLOB_DAT relocations emitted to initialise them by finish_dynamic_symbol.

The RELATIVE relocation is incorrect to use, since the static linker does not know the value of this symbol at runtime (i.e. it does not know the location in memory that the the shared library will be loaded).

This patch ensures that the clause in final_link_relocate does not catch such dynamic GOT entries by ensuring that we only catch symbols when we would not otherwise call finish_dynamic_symbol.

N.b. we also add an assertion in the condition-guarded block, partly to catch similar problems earlier, but mainly to make it clear that `relative_reloc` should not be set when finish_dynamic_symbol will be called.

N.b.2 The bfd_link_pic check is a little awkward to understand. Due to the definition of WILL_CALL_FINISH_DYNAMIC_SYMBOL, the only time that `!bfd_link_pic (info) && !WILL_CALL_FINISH_DYNAMIC_SYMBOL` is false and `!WILL_CALL_FINISH_DYNAMIC_SYMBOL (is_dynamic, bfd_link_pic (info), h)` is true is when the below holds: is_dynamic && !h->forced_local && h->dynindx == -1

This clause is looking for local GOT relocations that are not in the dynamic symbol table, in a binary that will have dynamic sections.

This situation is the case that this clause was originally added to handle (before the Morello specific code was added). It is the case when we need a RELATIVE relocation because we have a PIC object, but finish_dynamic_symbol would not be called on the symbol.

Since all capability GOT entries need relocations to initialise them it would seem unnecessary to include the bfd_link_pic check in our Morello clause. However the existing clause handling these relocations for AArch64 specifically avoids adding a relocation for bfd_link_hash_undefweak symbols. By keeping the `!bfd_link_pic (info)` clause in the Morello part of this condition we ensure such undefweak symbols are still avoided.

I do not believe it is possible to trigger the above case that requires this `bfd_link_pic` clause (where we have a GOT relocation against a symbol satisfying): h->dynindx == -1 && !h->forced_local && h->root.type == bfd_link_hash_undefweak && bfd_link_pic (info) && bfd_link_executable (info) I believe this is because when creating an undefweak symbol that has a GOT reference we hit the clause in elfNN_aarch64_allocate_dynrelocs which ensures that such symbols are put in `.dynsym` (and hence have a `h->dynindx != -1`). A useful commit to reference for understanding this is ff07562f1e.

Hence there is no testcase for this part. We do add some code that exercises the relevant case (but does not exercise this particular clause) into the morello-dynamic-link-rela-dyn testcase.

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

diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c index 88573828da7461238a20013aefb538a296ced99e..76216f8f00125e8c22cacbe8b5610b4b1d105eb4 100644 --- a/bfd/elfnn-aarch64.c +++ b/bfd/elfnn-aarch64.c @@ -7244,17 +7244,28 @@ elfNN_aarch64_final_link_relocate (reloc_howto_type *howto, should only generate one RELATIVE relocation for that symbol. Therefore, check GOT offset mark first.

- NOTE2: Symbol references via GOT in C64 static binaries without - PIC should always have relative relocations, so we do that here - early. */ + NOTE2: Symbol references via GOT in C64 should always have + relocations of some kind. Here we try to catch any such GOT + reference which would not otherwise be caught by + finish_dynamic_symbol. */ if (((h->dynindx == -1 && !h->forced_local && h->root.type != bfd_link_hash_undefweak && bfd_link_pic (info)) - || (!bfd_link_pic (info) && bfd_link_executable (info) - && c64_reloc)) + || (!bfd_link_pic (info) + && !WILL_CALL_FINISH_DYNAMIC_SYMBOL + (is_dynamic, bfd_link_pic (info), h) + && bfd_link_executable (info) && c64_reloc)) && !symbol_got_offset_mark_p (input_bfd, h, r_symndx)) { + /* If we would call finish_dynamic_symbol for this symbol then we + should not be introducing a relocation for the GOT entry + (that function handles creating relocations for the GOT entry + in the usual case, this bit of code is to handle special + cases where the relocation would not otherwise be generated). + */ + BFD_ASSERT (!WILL_CALL_FINISH_DYNAMIC_SYMBOL + (is_dynamic, bfd_link_pic (info), h)); relative_reloc = TRUE; c64_needs_frag_fixup = c64_reloc ? TRUE : FALSE; } diff --git a/ld/testsuite/ld-aarch64/aarch64-elf.exp b/ld/testsuite/ld-aarch64/aarch64-elf.exp index ba0b65a8aba4dacbcb0dd353bc2f81743780f2c7..19070408e990abbe50050b1cba183a6e4bf24934 100644 --- a/ld/testsuite/ld-aarch64/aarch64-elf.exp +++ b/ld/testsuite/ld-aarch64/aarch64-elf.exp @@ -278,6 +278,7 @@ run_dump_test_lp64 "c64-ehdr-sized-reloc" # we need to specify different `ld` command lines for different objects. 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" diff --git a/ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn.s b/ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn.s index 3cffb35a8b82e93aa45acc841f0f661a320f4849..cf6f0bfea613926049595d225c4d685b46bc9ac5 100644 --- a/ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn.s +++ b/ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn.s @@ -6,6 +6,8 @@ _start: .LFB0: .cfi_startproc purecap + adrp c0, :got:undefweakval + ldr c0, [c0, #:got_lo12:undefweakval] adrp c0, :got:weakval ldr c0, [c0, #:got_lo12:weakval] adrp c0, :got:globval @@ -39,3 +41,4 @@ weakval: .byte 0x1 .byte 0x1 .size weakval, .-weakval + .weak undefweakval diff --git a/ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn2.d b/ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn2.d index 9de0c5f4f362ba8fb2d6c83624f22b4feae839f2..dcdd02606c38e6f1ad8c551a88577c2da2e9bbc1 100644 --- a/ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn2.d +++ b/ld/testsuite/ld-aarch64/morello-dynamic-link-rela-dyn2.d @@ -3,12 +3,20 @@ #source: morello-dynamic-link-rela-dyn.s #as: -march=morello+c64 #ld: tmpdir/morello-dynamic-relocs.so -#readelf: --relocs +#readelf: --relocs --dyn-sym

-Relocation section '.rela.dyn' at offset .* contains 5 entries: +Relocation section '.rela.dyn' at offset .* contains 6 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 +.* 00010000e801 R_MORELLO_GLOB_DA 0000000000000000 undefweakval + 0 +.* 00020000e801 R_MORELLO_GLOB_DA 0000000000000000 var + 0 + +Symbol table '.dynsym' contains [34] entries: + Num: Value Size Type Bind Vis Ndx Name + 0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND + 1: 0000000000000000 0 NOTYPE WEAK DEFAULT UND undefweakval + 2: 0000000000000000 0 OBJECT GLOBAL DEFAULT UND var +#pass diff --git a/ld/testsuite/ld-aarch64/morello-dynamic-relocs.d b/ld/testsuite/ld-aarch64/morello-dynamic-relocs.d new file mode 100644 index 0000000000000000000000000000000000000000..c7cfbd91626c97a1429f6b4a16606031dac8e256 --- /dev/null +++ b/ld/testsuite/ld-aarch64/morello-dynamic-relocs.d @@ -0,0 +1,16 @@ +# Test here is to ensure that we only emit one relocation into the GOT for the +# one use of a GOT entry. This is already checked by the fact that the last +# linker command would hit an assertion failure if it were not the case. +# We check that there is only one relocation in the resulting binary anway, +# since double-checking is always nice. +# +# 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 + +Relocation section '.rela.dyn' at offset 0x[0-9a-f]+ contains 1 entry: + Offset Info Type Sym. Value Sym. Name + Addend +[0-9a-f]+ [0-9a-f]+ R_MORELLO_GLOB_DA 0000000000000000 var + 0 diff --git a/ld/testsuite/ld-aarch64/morello-dynamic-relocs.s b/ld/testsuite/ld-aarch64/morello-dynamic-relocs.s new file mode 100644 index 0000000000000000000000000000000000000000..6a4e862471b63d3740171374aba0c885b6278762 --- /dev/null +++ b/ld/testsuite/ld-aarch64/morello-dynamic-relocs.s @@ -0,0 +1,15 @@ + .arch morello+crc+c64 + .text + .align 2 + .global _start + .type _start, %function +_start: +.LFB0: + .cfi_startproc purecap + adrp c0, :got:var + ldr c0, [c0, #:got_lo12:var] + str wzr, [c0] + ret + .cfi_endproc +.LFE0: + .size _start, .-_start