In standard AArch64 linking by the BFD linker, dynamic symbols in PIC code have their dynamic relocations created by elfNN_aarch64_finish_dynamic_symbol. Any required information in the relevant fragment is added by elfNN_aarch64_final_link_relocate.

Non-dynamic symbols that are supposed to go in the GOT have their RELATIVE relocations created in elfNN_aarch64_final_link_relocate next to the place where the fragment is populated.

The code in elfNN_aarch64_finish_dynamic_symbol was not updated when we ensured that RELATIVE relocations against function symbols were generated with the PCC base stored in their fragment and an addend defined to make up the difference so that the relocation pointed at the relevant function.

On top of this, elfNN_aarch64_final_link_relocate was never written to include the size and permission information in the GOT fragment for RELATIVE relocations that will be generated by elfNN_aarch64_finish_dynamic_symbol.

This patch resolves both issues by adding code to elfNN_aarch64_final_link_relocate to handle setting up the fragment of a RELATIVE relocation that elfNN_aarch64_finish_dynamic_symbol will create, and adding code in elfNN_aarch64_finish_dynamic_symbol to use the correct addend for the RELATIVE relocation that it generates.

Implementation choices:

The check in elfNN_aarch64_final_link_relocate for "cases where we would generate a RELATIVE relocation through elfNN_aarch64_finish_dynamic_symbol" is believed to handle undefined weak symbols by checking SYMBOL_REFERENCES_LOCAL on the belief that the latter would not return true if on undefined weak symbols. This is not as clearly correct as the rest of the condition, so seems reasonable to bring to the attention of anyone interested.

We add an assertion that this is the case so we get alerted if it is not, we could choose to include !UNDEFWEAK_NO_DYNAMIC_RELOC in the condition instead, but believe that would lead to confusion in the code (i.e. why check something that will always be false).

Similarly, when we check against SYMBOL_REFERENCES_LOCAL to decide whether to populate the fragment for this relocation this does not directly correspond to `h->dynindx == -1` (which would indicate that this symbol is not in the dynamic symbol table). This means that our clause catches symbols which would appear in the dynamic symbol table as long as SYMBOL_REFERENCES_LOCAL returns true. The only case in which we know this can happen is for PROTECTED visibility data when GNU_PROPERTY_NO_COPY_ON_PROTECTED is set. When this happens a RELATIVE relocation is generated (since this is an object we know will resove to the current binary) and the static linker provides the permissions and size of the associated object in the relevant fragment. This behaviour matches all other RELATIVE relocations and allows the dynamic loader to assume that all RELATIVE relocations should have their associated permissions and size provided. We mention this behaviour since the symbol for this object will appear in the dynamic symbol table and hence the dynamic loader *could* determine the size and permissions itself.

In our condition to decide whether to update this relocation we include a check that we `WILL_CALL_FINISH_DYNAMIC_SYMBOL`. This is not necessary, since the combination of conditions implies it, however it makes things much clearer as to what we're checking for.

Testsuite notes:

When testing our change here we check: 1) The addend and base of the RELATIVE relocation gives the required address of the hidden function. 2) The bounds of the RELATIVE relocation is non-zero. 3) The permissions of the RELATIVE relocation are executable. Lacking in this particular test is a check that the PCC bounds are calculated correctly, and that the base we define is the base of the PCC. We rely on existing tests to check our calculation of the PCC bounds.

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

diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c index c31e707a4fb8870522d2eb5a624e13fcaabf84bf..766d38c52c31e7f368747c89f2922de189b7c0c8 100644 --- a/bfd/elfnn-aarch64.c +++ b/bfd/elfnn-aarch64.c @@ -6633,6 +6633,7 @@ elfNN_aarch64_final_link_relocate (reloc_howto_type *howto, struct elf_aarch64_link_hash_table *globals; bfd_boolean weak_undef_p; bfd_boolean relative_reloc; + bfd_boolean c64_needs_frag_fixup; asection *base_got; bfd_vma orig_value = value; bfd_boolean resolved_to_zero; @@ -7229,10 +7230,13 @@ elfNN_aarch64_final_link_relocate (reloc_howto_type *howto, BFD_ASSERT (h != NULL);

relative_reloc = FALSE; + c64_needs_frag_fixup = FALSE; if (h != NULL) { 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. @@ -7251,7 +7255,37 @@ elfNN_aarch64_final_link_relocate (reloc_howto_type *howto, || (!bfd_link_pic (info) && bfd_link_executable (info) && c64_reloc)) && !symbol_got_offset_mark_p (input_bfd, h, r_symndx)) - relative_reloc = TRUE; + { + relative_reloc = TRUE; + c64_needs_frag_fixup = c64_reloc ? TRUE : FALSE; + } + /* If this is a dynamic symbol that binds locally then the generic + code and elfNN_aarch64_finish_dynamic_symbol will already handle + creating the RELATIVE reloc pointing into the GOT for this symbol. + That means that this function does not need to handle *creating* + such a relocation. This function does already handle setting the + base value as the fragment for that relocation, hence we should + ensure that we set the fragment correctly for C64 code (i.e. + including the required permissions and bounds). */ + else if (c64_reloc + && WILL_CALL_FINISH_DYNAMIC_SYMBOL (is_dynamic, + bfd_link_pic (info), h) + && bfd_link_pic (info) + && SYMBOL_REFERENCES_LOCAL (info, h)) + { + /* We believe that if `h` were undefined weak it would not have + SYMBOL_REFERENCES_LOCAL return true. However this is not 100% + clear based purely on the members that we check in the code. + The reason it matters is if we could have a + SYMBOL_REFERENCES_LOCAL symbol which is also + !UNDEFWEAK_NO_DYNAMIC_RELOC then the check above would + determine that we need to fix up the fragment for the RELATIVE + relocation that elfNN_aarch64_finish_dynamic_symbol will + create, but in actual fact elfNN_aarch64_finish_dynamic_symbol + would not create that relocation. */ + BFD_ASSERT (!UNDEFWEAK_NO_DYNAMIC_RELOC (info, h)); + c64_needs_frag_fixup = TRUE; + }

if (c64_reloc && c64_symbol_adjust (h, value, sym_sec, info, &frag_value)) @@ -7311,11 +7345,20 @@ elfNN_aarch64_final_link_relocate (reloc_howto_type *howto, linking stage. While for shared library, we need to update the content of GOT entry according to the shared object's runtime base address. So, we need to generate a R_AARCH64_RELATIVE reloc - for dynamic linker. */ + for dynamic linker. + + For any C64 binary we need to ensure there are RELATIVE + relocations to initialise the capabilities. The only case when + we would not want to emit such relocations is when the producing + a relocatable object file (since such files should not have + dynamic relocations). */ if (bfd_link_pic (info) || (!bfd_link_pic (info) && bfd_link_executable (info) && c64_reloc)) - relative_reloc = TRUE; + { + relative_reloc = TRUE; + c64_needs_frag_fixup = c64_reloc ? TRUE : FALSE; + }

symbol_got_offset_mark (input_bfd, h, r_symndx); } @@ -7332,6 +7375,22 @@ elfNN_aarch64_final_link_relocate (reloc_howto_type *howto, addend, weak_undef_p); }

+ if (c64_needs_frag_fixup) + { + BFD_ASSERT (c64_reloc); + /* For a C64 relative relocation, also add size and permissions into + the frag. */ + bfd_reloc_status_type ret; + + ret = c64_fixup_frag (input_bfd, info, bfd_r_type, sym, h, + sym_sec, globals->root.srelgot, + base_got->contents + off + 8, + orig_value, 0, off); + + if (ret != bfd_reloc_continue) + return ret; + } + if (relative_reloc) { asection *s; @@ -7341,19 +7400,8 @@ elfNN_aarch64_final_link_relocate (reloc_howto_type *howto,

s = globals->root.srelgot;

- /* For a C64 relative relocation, also add size and permissions into - the frag. */ if (c64_reloc) { - bfd_reloc_status_type ret; - - ret = c64_fixup_frag (input_bfd, info, bfd_r_type, sym, h, - sym_sec, s, base_got->contents + off + 8, - orig_value, 0, off); - - if (ret != bfd_reloc_continue) - return ret; - rtype = MORELLO_R (RELATIVE);

if (bfd_link_executable (info) && !bfd_link_pic (info)) @@ -11025,17 +11073,23 @@ elfNN_aarch64_finish_dynamic_symbol (bfd *output_bfd, return FALSE;

BFD_ASSERT ((h->got.offset & 1) != 0); + bfd_vma value = h->root.u.def.value + + h->root.u.def.section->output_section->vma + + h->root.u.def.section->output_offset; if (is_c64) { rela.r_info = ELFNN_R_INFO (0, MORELLO_R (RELATIVE)); - rela.r_addend = 0; + bfd_vma base_value = 0; + if (c64_symbol_adjust (h, value, h->root.u.def.section, info, + &base_value)) + rela.r_addend = (value | h->target_internal) - base_value; + else + rela.r_addend = 0; } else { rela.r_info = ELFNN_R_INFO (0, AARCH64_R (RELATIVE)); - rela.r_addend = (h->root.u.def.value - + h->root.u.def.section->output_section->vma - + h->root.u.def.section->output_offset); + rela.r_addend = value; } } else diff --git a/ld/testsuite/ld-aarch64/aarch64-elf.exp b/ld/testsuite/ld-aarch64/aarch64-elf.exp index adb0081720a48c984e246b838d6ccd0922fa1306..17ba4cfee2aa2214bb3e0c3150862d2aa722a395 100644 --- a/ld/testsuite/ld-aarch64/aarch64-elf.exp +++ b/ld/testsuite/ld-aarch64/aarch64-elf.exp @@ -78,6 +78,19 @@ proc run_dump_test_lp64 { testname } { [list ld [concat "-m " [aarch64_choose_lp64_emul]]]] }

+# Return the hexadecimal representation of the value we need to add to $base in +# order to return $result plus 1. Both $base and $result are assumed to be +# hexadecimal numbers without the leading 0x prefix. +# +# This procedure is especially useful for checking the addend of a RELATIVE +# relocation against a function in a testcase using the `#check:` directive. +# Dumping `objdump -DR -j .got -j .text` will give us the address we're trying +# to reach and base stored in the relocation fragment before reaching the +# addend we're using for this relocation. +proc aarch64_required_func_addend { base result } { + return [format %x [expr "0x$result + 1 - 0x$base"]]; +} + set eh-frame-merge-lp64 [list [list "EH Frame merge" \ [concat "-m " [aarch64_choose_lp64_emul] \ " -Ttext 0x8000"] \ @@ -251,6 +264,7 @@ run_dump_test_lp64 "emit-relocs-morello-6b" run_dump_test_lp64 "emit-relocs-morello-7" run_dump_test_lp64 "emit-relocs-morello-8" run_dump_test_lp64 "emit-relocs-morello-9" +run_dump_test_lp64 "emit-relocs-morello-hidden" run_dump_test_lp64 "emit-morello-reloc-markers-1" run_dump_test_lp64 "emit-morello-reloc-markers-2" run_dump_test_lp64 "emit-morello-reloc-markers-3" diff --git a/ld/testsuite/ld-aarch64/emit-relocs-morello-hidden.d b/ld/testsuite/ld-aarch64/emit-relocs-morello-hidden.d new file mode 100644 index 0000000000000000000000000000000000000000..9bac38de22516bc2f1679745df483a16a93c0728 --- /dev/null +++ b/ld/testsuite/ld-aarch64/emit-relocs-morello-hidden.d @@ -0,0 +1,33 @@ +#source: emit-relocs-morello-hidden.s +#as: -march=morello+c64 +#ld: -shared +#objdump: -DR -j .got -j .text + +.*: file format .* + + +Disassembly of section .text: + +#record: HIDDEN_ADDR +#... +([0-9a-f]*) <hidden_func>: + .*: 028043ff sub csp, csp, #0x10 + .*: b9000fe0 str w0, [csp, #12] + .*: b9400fe0 ldr w0, [csp, #12] + .*: 020043ff add csp, csp, #0x10 + .*: c2c253c0 ret c30 + +Disassembly of section .got: + +#... + ... +#record: RELOC_BASE + .*: ([0-9a-f]{8}) .* +#check: RELOC_ADDEND aarch64_required_func_addend $RELOC_BASE $HIDDEN_ADDR + .*: R_MORELLO_RELATIVE *ABS*+0xRELOC_ADDEND + .*: 00000000 .* +# Check that the bounds are anything except zero (rely on other tests to ensure +# that the PCC bounds calculation is correct). +! .*: 00000000 .* + .*: 04000000 .* + diff --git a/ld/testsuite/ld-aarch64/emit-relocs-morello-hidden.s b/ld/testsuite/ld-aarch64/emit-relocs-morello-hidden.s new file mode 100644 index 0000000000000000000000000000000000000000..24bb687456ea9de2fd16a2dfa311d770ee6580ac --- /dev/null +++ b/ld/testsuite/ld-aarch64/emit-relocs-morello-hidden.s @@ -0,0 +1,20 @@ + .text + .align 2 + .global foo + .type foo, %function +foo: + adrp c0, :got:hidden_func + ldr c0, [c0, #:got_lo12:hidden_func] + ret + .size foo, .-foo + .align 2 + .global hidden_func + .hidden hidden_func + .type hidden_func, %function +hidden_func: + sub csp, csp, #16 + str w0, [csp, 12] + ldr w0, [csp, 12] + add csp, csp, 16 + ret + .size hidden_func, .-hidden_func