This project is mirrored from https://sourceware.org/git/binutils-gdb.git.
Pull mirroring updated .
- 28 Apr, 2022 4 commits
-
-
Matthew Malcomson authored
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 ff07562f. 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.
-
Matthew Malcomson authored
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 40bbb79e for when this was introduced and commit 8d4edc5f 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 8d4edc5f 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.
-
Matthew Malcomson authored
When DT_INIT and/or DT_FINI point to C64 functions they should have their LSB set. I.e. these entries should contain the address of the relevant functions and not a slight variation on them. This is already done by Morello clang, and we want GNU ld updated to match. Here we account for these LSB's for Morello in the same way as the Arm backend accounts for the Thumb LSB. This is done in the finish_dynamic_sections hook by checking the two dynamic section entries, looking up the relevant functions, and adding that LSB onto the entry value. In our testcase we simply check that the INIT and FINI section entries have the same address as the _init and _fini symbols.
-
Matthew Malcomson authored
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.
-
- 07 Apr, 2022 1 commit
-
-
Richard Sandiford authored
The alt-base loads and stores allow WZR and XZR to be specified as the register being loaded or stored. We were accepting the XZR forms but not the WZR ones. The easiest fix is to drop the separate Wt operand type. Most other instructions handle the W/X distinction using the qualifiers instead, and all instructions that used Wt already specified W qualifiers.
-
- 30 Mar, 2022 1 commit
-
-
Richard Sandiford authored
Many load instructions have two forms: LDR<x> that takes either: - a register index or - an unsigned scaled immediate offset and LDUR<x> that takes: - a signed unscaled immediate offset in the range [-256, 255] The assembler usually maps out-of-range LDR<x> offsets to LDUR<x> where possible. GAS does this using matching OP_* codes; see try_to_encode_as_unscaled_ldst in gas/config/tc-aarch64.c. Some alternative-base Morello instructions also come in these LDR/LDUR pairs, so we can use the same approach for them. However, the alternative-base forms of LDRS[BHW] only support a register index. They do not have a register+unsigned scaled form. There is therefore no OP_* pair linking alternative-base LDRS[BHW] and LDURS[BHW] instructions. This patch therefore treats immediate LDRS[BHW] as a straight alias of LDURS[BHW]. Following existing practice, LDURS[BHW] is still the preferred disassembly, so the patch uses F_P1 to force LDURS[BHW] to be chosen ahead of LDRS[BHW]. Following the general preference for using immediate forms where possible: ldrsb x0, [c0] is treated as: ldursb x0, [c0, #0] rather than: ldrsb x0, [c0, xzr] Co-Authored-By:
Stam Markianos-Wright <stam.markianos-wright@arm.com>
-
- 17 Mar, 2022 1 commit
-
-
Matthew Malcomson authored
This symbol is defined in a binary when there is a segment which contains both the file header and the program header. The symbol points at the file header. The point of this symbol is to allow the program to robustly examine its own output. Glibc uses this symbol. This symbol is currently not marked as a linker or linker script defined symbol, and hence does not get its bounds adjusted. The symbol is given zero size, and consequently any capability initialised as a relocation to this symbol is given zero bounds. In order to allow access to read the headers this symbol points at this patch adds a size to the symbol. We do not believe that the size of this symbol is used for anything other than CHERI bounds, so we believe that this is a safe change to make. Setting the size of the symbol means that c64_fixup_frag uses that size as the bounds to apply to a capability relocation pointing at that symbol. This allows access to the file and program headers loaded into memory. An alternative approach would be to *not* set the size of the symbol, but only change the bounds of the relocation generated. This would be done by checking for the `__ehdr_start' name in c64_fixup_frag and setting the size according to the `sizeof_ehdr' and `elf_program_header_size' values stored on the output BFD object. We chose the approach to set the size on the symbol for code-aesthetic reasons under the belief that having this size on the symbol in the final binary is a slight benefit in readability for a user and causes no downside. I do not believe that Morello lld sets the bounds of a capability to this symbol correctly. That issue has been raised separately.
-
- 07 Mar, 2022 6 commits
-
-
Matthew Malcomson authored
This was newly failing because it was checking for a value *without* the LSB set. In a recent commit we have fixed the bug which lost the LSB, and that caused this test to fail. Here we use the new testsuite implementation to test for "one plus the location" rather than "one of the values A, B, C, ...", which is a better representation of what we're trying to check.
-
Matthew Malcomson authored
There is special handling to ensure that symbols which look like they are supposed to point at the start of a section are given a size to span that entire section. GNU ld has special `start_stop` symbols which are automatically provided by the linker for sections where the output section and input section share a name and that name is representable as a C identifier. (see commit cbd0eecf) These special symbols represent the start and end address of the output section. These special symbols are used in much the same way in source code as section-start symbols provided by the linker script. Glibc uses these for the __libc_atexit section containing pointers for functions to run at exit. This change accounts for these `start_stop` symbols by giving them the size of the "remaining" range of the output section in the same way as linker script defined symbols. This means that the `start` symbols get section-spanning bounds and the `stop` symbols get bounds of zero. N.b. We will have to also account for these symbols in the `resize_sections` function, but that's not done yet.
-
Matthew Malcomson authored
Really don't like that we use hard-coded addresses. There are examples in the existing testsuite that use options of hard-coded addresses, but I want something more general that we can actually test the things we need to test with. Here we add an initial implementation to do such a thing. This initial implementation has quite a lot of problems, but it adds a lot in the fact that we can write testcases which should work across different setups. Hopefully we can work out the problems with use (or maybe identify that the problems don't actually matter very much in practice) and eventually upstream something better. To document the problems: - The implementation means that recording something actually puts that into the regexp_diff namespace which could shadow existing variables. - We don't have a way to say "the previous value", but always have to write some TCL procedure to return that previous value.
-
Matthew Malcomson authored
The LSB on STT_FUNC symbols was missed in a few different places. 1) Absolute relocations coming from .xword, .word, and .hword directives and the lowest bit MOVW relocations did not account for the LSB at all. 2) Relocations for the ADR instruction only added the LSB on local symbols. Here we account for these by adding the LSB in each clause in elfNN_aarch64_final_link_relocate. The change under the BFD_RELOC_AARCH64_NN clause handles absolute 64 bit relocations, the change for BFD_RELOC_AARCH64_ADR_LO21_PCREL handles the relocation on ADR instructions, and the extra relocations checked against in the clause including BFD_RELOC_AARCH64_ADD_LO12 ore the remaining items. N.b. we noticed the MOVW relocation problem because glibc's start.S was using these direct MOV relocations to access the value of `main`. Since `main` is a function we need to include the LSB in the resulting relocation value. These relocations did not include the LSB from STT_FUNC symbols. Others were found from inspection of each relocation in turn.
-
Matthew Malcomson authored
The previous code was not actually using the size of a symbol when the symbol was in the hash table. This meant that our TLS relaxations created an instruction sequence with bounds of zero so that the GCC TLS instruction sequence eventually ended up giving a length-zero capability. Also handle extra size of pointers in TCB for c64. For purecap we have 16 byte pointers. Hence the TCB is 32 bytes. This was not yet handled in our relaxations. Here we determine whether to use a 32 or 16 byte TCB based on the flags of the current BFD (i.e. whether this is a purecap binary that we're creating). Testcases are updated to account for the fact that the length of the capability to the symbol itself is now sometimes non-zero and for the different offset required into the TLS block for modules loaded at startup time.
-
Matthew Malcomson authored
The handling is done by putting the value that we want in a buffer and using that as the entry_symbol.name which lang_end picks up. Another option would be to find the entry symbol *after* lang_end has finished (e.g. in elfNN_aarch64_init_file_header) and add the LSB to it if that symbol is a C64 symbol. This approach was mainly chosen in order to match more closely what Thumb has done. N.b. we set the LSB based on the LSB of the entry point symbol. If the entry point symbol is in c64 code but is not an STT_FUNC (e.g. it is an STT_NOTYPE) then the LSB will not be set. This matches Morello clang behaviour.
-
- 21 Feb, 2022 7 commits
-
-
Matthew Malcomson authored
Before this change we would ensure the ability for precise bounds on any section which had a linker defined symbol pointing in it *unless* that linker defined symbol looked like a section starting symbol. In that case we would adjust the *next* section if the current section had no padding between this one and the next. I believe this was a mistake. The testcase we add here is a simple case of having a `__data_relro_start` symbol in the .data.rel.ro section, and that would not ensure that the .data.rel.ro section were precisely padded. The change we make here is to perform padding for precise bounds on all sections with linker defined symbols in them *and* the next section if there is no padding between this and the next section. This is a huge overfit and we do it for the reason described in the existing comment (that we have no information on the offset of this symbol within the output section). In the future we may want to remove this padding for linker script defined symbols which do not look like section start symbols. We would do this in conjunction with changing the bounds we put on such linker script defined symbols. This would be for another patch.
-
Matthew Malcomson authored
Unfortunately writing a test for many of these changes was difficult since we needed to check that thing A pointed to place B or that thing A had a length that spanned places B and C. Hence quite a few of the added testcases check for literal values. These are problematic for a few different reasons, both that they may not work with different configurations and because unrelated updates need to update the tests. We need to figure out a better way to test this. We believe the testsuite doesn't have facilities for checking this. For now we're leaving making the tests robust for later. Here we add all the tests which the recent patch series changes and include updates to the existing tests. Updated existing tests: emit-relocs-morello-2.d needed updating since the new alignment meant the relocation fragment pointed to a different place. emit-relocs-morello-7.d Needed updating because we added some padding, and re-laid the sections. This just ended up with a layout with the .text section at a different location and with a different range. emit-relocs-morello-8.d Just needed updating because the extra padding moved the .data.rel.ro section a bit. The position of the relocation was not part of the test so we replaced that with an "any number" regex. emit-morello-reloc-markers-{2,3}.d Needed updating because the PCC bounds required extra alignment and that was put on the first section in the PCC bounds. That happened to be the section we were looking at. morello-capinit.d Needed updating because this change actually fixed a problem with our calculation of the pcc bounds. Now we calculate the pcc_low and pcc_high *after* having made all our adjustments, the bounds we add on the capability start at the first section we want to contain. morello-stubs.d Updated just becase the location of our functions was changed due to the text section change. morello-stubs-static.d Updated because the location of the .text_low section changed along with all the code in it. Hence our stubs that needed to point to these functions also changed. c64-ifunc-{2,2-local,3a}.d Updated because the .text section moved and hence the address of the foo function we wanted to point to was adjusted. morello-sizeless-{got,global,local}-syms.d Again, .data .bss and .got sections ended up moved so addresses of the capabilities we wanted to point at all changed.
-
Matthew Malcomson authored
Now instead of iterating through relocations recording changes to make and sorting those changes according to the section VMA before going on to make the changes in section VMA order, we instead modify the sections as we iterate through relocations. This change can be done now that the alignment is always done, even if the VMA of the start and end was good for precise bounds and now that padding is added via an expression rather than setting the point to a specific location. Having these two things means that changing the layout of earlier sections does not affect the precise bounds property of later sections. It also makes things easier that we keep the padding of a section inside that section, so can tell whether a section has the correct size just from the size of the section rather recording in an out-of-bounds manner which sections have had their padding assigned. This patch makes no functional change, but simply changes the code to be more readable.
-
Matthew Malcomson authored
Before this change individual sections would not be padded or aligned if there was no C64 code or if there were no capability GOT relocations in the binary. This meant that if we had a data-only PURECAP shared library with a CAPINIT relocation in it pointing at something, then the section that relocation pointed into would not be padded accordingly. This patch changes this so that we look for sections which may need individual padding if we see the PURECAP elf header flag, and if there is a `srelcaps` section (i.e. the RELATIVE capability relocations). We keep the behaviour that we do not adjust the size of sections unless there is a static relocation pointing at a zero-sized symbol at that section. That is, we do not make any adjustment to try and handle section padding in the case where other binaries would dynamically link against such symbols. We do this since the "symbol pointing to section start implies spanning entire section" decision is a hack to enable some linker script uses, and we don't want to extend it without a known motivating example. Finally, this patch ignores padding PCC bounds on PURECAP binaries if there is no C64 code in this binary, but ensures that the PCC bounds are made precise even if there are no static relocations in the file. We could still get the current PCC and offset it using `adr` if there are no static relocations, but without there being any C64 code there will be no PCC to bound and hence we don't need the bounds on a hypothetical PCC to be precise.
-
Matthew Malcomson authored
The mechanism by which we were ensuring the PCC bounds were made precise for Morello happened to only work if there were some sections which needed to be made precisely representable for Morello individually. This was because we included the PCC bounds calculation in the `queue` iteration that only iterated over adjustments to individual sections. Here we move the PCC bounds calculation to after the `queue` iteration so that we always perform this operation. We suspect the original implementation was chosen to ensure that padding was added in sequential section ordering. This ordering seems to have been in order to ensure that padding at one position would not adjust sections that had already been adjusted (because padding one section changes the location of the sections after it). We have already found in previous patches that this approach was not sufficient to ensure an adjustment being permanent. The alignment change to the first section that the PCC should span can change the location of all sections after it, or the linker can simply have extra space before .text that it removes on a call to layout_sections_again. In the patches to fix those problems, we have adjusted the code here to represent the padding in a way that stays stable across changes. That has meant that the iteration in VMA order is no longer necessary, and that means that our movement of the PCC bounds calculation to outside of the `queue` iteration loop can be performed. One interesting part of this adjustment is that given a set of sections, the length of memory that they span can change if the first sections alignment is adjusted. For example, if we have the below: sectionA VMA 0xf size 0x1 alignment 0x1 sectionB VMA 0x10 size 0x10 alignment 0x10 Then aligning sectionA to 0x10 gives the below: sectionA VMA 0x10 size 0x1 alignment 0x10 sectionB VMA 0x20 size 0x10 alignment 0x10 The total range of the first case is [0xf -> 0x20] for a size of 0x11 and of the second case it is [0x10 -> 0x30] for a size of 0x20. This means that we should handle the alignment adjustment for the PCC bounds first, and must handle it in a loop to ensure that we handle the case that this change in length requires an extra alignment. Only then do we know the size that we want to add into the last section in the range so that the entire bounds are correct.
-
Matthew Malcomson authored
Before this patch we would only change the alignment of a section if it did not have a start and end address that was aligned properly. This meant that there was nothing stopping the alignment of this section degrading in the future. On first glance this looks like it would not be a problem since this function only adjusts sections in order of increasing VMA (hence it would seem that the alignment of the current section can not be reduced). However, in some cases layout_sections_again can be seen to reduce the alignment of sections if there was some initial space before the .text section that it shrinks for some reason. This led to a degredation of the alignment of all sections after that point (until another highly aligned section). The testcase added for this change (in the final "testsuite" commit of this patch series) is a good example of this, on first entry to the elfNN_c64_resize_sections function .text happened to have a start address of 0xb0 (which meant that .data.rel.ro was also aligned to such a boundary and the function did not believe there was a need to align .data.rel.ro to a 16 byte boundary). However after the first call to layout_sections_again this changed to 0x78, reducing the alignment of .data.rel.ro in the process.
-
Matthew Malcomson authored
When adding padding to ensure section bounds do not overlap we were implementing the padding using `lang_add_newdot`. This interacts with what is essentially an in-memory linker script that the linker will use at the very end to emit its sections according to those rules that have been built up. `lang_add_newdot` is essentially the same as defining the position to be the given address in a linker script. This means that all sections after this point in the linker script will be at an address starting from this known address. I.e. the method by which we add padding is essentially changing the description of how we will lay a binary out from: <sections before the padded one> <section to be padded> <sections after the padded one> to the description: <sections before the padded one> <section to be padded> <current position must be 0x[number calculated now]> <sections after the padded one> This works fine in most cases. The address we calculate is a known-good value and sections after this "point" are moved to after the known-good value. However, the fact that we choose a specific value when we call `c64_pad_section` means that adjusting sections which occur *before* the current point will not change anything that occurs after it. I.e. a description of <sections before the padded one> <section to be padded> <current position must be 0x[number calculated now]> <sections after the padded one> being changed to a description of <sections before section X> <New padding> <sections before padded one> <section to be padded> <current position must be 0x[number calculated now]> <sections after the padded one> leaves the `<sections after the padded one>` with the same address. This can lead to the padded section and the section after it overlapping. This rarely happens, because our padding always happens after a section and we iterate over sections in memory order. However, when we align the very start of the PCC range in order to produce precise bounds across this range that can change the start position of the first section that should be spanned by the PCC range. Since it can change the start position we can hit the problem described above. This happens when attempting to build glibc. It causes an error message like the one below. section .got LMA [000000000053c0c0,000000000053cfff] overlaps section .data.rel.ro LMA [0000000000525fe0,000000000053d08f] This patch solves this problem by adding an entry into this in-memory linker script that describes padding without specifying a given address. I.e. the outline of the script we produce becomes <sections before the padded one> <section to be padded> <current position goes from P to P+0x[padding calculated now]> <sections after the padded one> This is safe w.r.t. adjustments occuring before the padding we have inserted, and it avoids the warning we noticed when trying to build glibc. We also fix up some other bugs in this area around double-padding sections. First, the calculation of the padding required was based on the output section VMA and size. The calculation was done by taking the current start and end VMA then finding the resulting start and end VMA that we want using c64_valid_cap_range. Then we calculated the padding we wanted by finding the difference between the current and requested end VMA's. This ignored the fact that the output section was also getting aligned, which would change the start VMA -- hence the resulting end VMA would not end up where we wanted. Here we do the calculation of how much padding to add based on the size we want rather than based on the ending VMA we want. Second, the reported size of the output section was not changing after adding our padding. This meant that the second time around this loop (if for example a relocation into a given section was used in more than one place and hence this section was enqueued twice) we would again find that the section size was not padded and try again. We fix this by introducing the padding statement to the output section statement children rather than to the main statement list. This means that the padding will be accounted for in the output section size and hence the loop will avoid padding this section again. Just to note: LLD does not report the sizes of sections including their padding. This is so that programs which read binary information (such as readelf and objdump) do not need to read the padded zeros in the file. We choose to include this padding in the section size information on the premise that it is usually quite small and that the output from these programs is then more readable. The bug that we fixed by including this padding in the size of the output section could be fixed in another way.
-
- 10 Feb, 2022 2 commits
-
-
Matthew Malcomson authored
Before this they would span sections which are SEC_CODE or some specific known sections like the GOT and PLT. This is not enough, since the compiler can want to access .rodata via relative offsets to PCC. Hence we need to include READONLY sections. Similarly, we want to include .data.rel.ro sections in the PCC bounds so that they can be accessed via PCC -- this allows the capability indirection table to be accessed. We have not been noticing this until now because the default linker script happens to order sections such that the PCC being required to span .got and .text happens to end up including these problematic sections. RELRO sections are a bit interesting since the fact they are RELRO is not recorded anywhere on the section itself. Rather it is stored in the fact that the section is covered by the RELRO segment. This means that we need to check if the sections VMA is within the relevant range rather than just look at the section. This turns out to be pretty easy since we have a structure containing the RELRO range, however we do need to ensure that we don't mix up the uses of the section VMA and the RELRO start and end around calls of layout_sections_again since this call can change both.
-
Matthew Malcomson authored
We were specifying section alignment requirements based on the alignment that the section base happened to have. This sometimes resulted in very strange alignment requests that were much greater than actually required. That is not usually a problem, but it does give unnecessary padding upon re-adjustments due to changing the PCC bounds after individual sections have been padded. This patch adds an interface such that we return the alignment actually required for exact capability bounds from c64_valid_cap_range. We then use that alignment as our alignment requirement on the sections which have a section-sized symbol associated with them.
-
- 09 Feb, 2022 2 commits
-
-
Matthew Malcomson authored
The permissions that a capability to an object should end up with is based on the section it should point into. With symbols that point into SHN_ABS sections we have nothing to base the permissions on (since these sections don't have associated permission flags). For the moment we are making a default of choosing Read-Write permissions and warning the user about it. The permissions match what Morello LLD currently does (from observation). When Morello linkers use the symbol type to determine whether a capability should have executable permissions or not, this should end up being able to handle all uses (since STT_FUNC would get RX perms while everything else gets RW perms). In the only case we know of in the GNU team the symbol ends up with zero-size anyway, so the choice of Read-Write doesn't seem too lax. (Having zero-size is fine for the use-case we know of in glibc, since that use case simply checks if the address of the symbol is non-zero. Hence we have no need as yet to dereference the symbol). The use case we know about are the `_nl_current_<LANG>_used` symbols defined with `_NL_CURRENT_DEFINE` in the locale/lc-<lang>.c files in statically linked glibc. If any case that requires non-zero size or different permissions becomes important then something more will be required across the toolchain.
-
Matthew Malcomson authored
This commit is partly changing two existing (believed buggy) behaviours in elfNN_aarch64_merge_private_bfd_data and partly accounting for a capability-specific requirement. The existing behaviours in elfNN_aarch64_merge_private_bfd_data were: 1) It returned `TRUE` by default. This effectively ignored the ELF flags on the binaries, despite there being code looking at them. 2) We do not mark the output BFD as initialised until we see flags with non-default architecture and flags. This can't tell the difference between linking default objects to non-default objects if the default objects are given first on the command line. The capability-specific requirement is: - This function originally returned early if the object file getting merged into the existing output object file is not dynamic and has no code sections. The code reasoned that differing ELF flags did not matter in this case since there was no code that would be expecting it. For capabilities the binary compatibility is still important. Data sections now contain capabilities as pointers, got sections now have a different got element size. Hence we avoid this short-circuit if any of the flags we're checking are the CHERI_PURECAP flag.
-
- 07 Feb, 2022 2 commits
-
-
Matthew Malcomson authored
The reasoning behind only warning for symbols which have a size which cannot be precisely bounded is that there is nothing *requiring* precise bounds, GCC knowingly avoids changing the size of some symbols for precise bounds (TLS and symbols with user-specified alignment and user-specified section), and LLD only warns on imprecise bounds rather than erroring. N.b. the reasoning for GCC avoiding padding in these cases is explained in the commit message of b302420cb55 in the GCC branch vendors/ARM/heads/morello. All in all it's not something that we want in our toolchain as a requirement, and it's not something that other toolchains have as a requirement, so there doesn't seem to be much of a reason to include it. In order to make this warning a little nicer for anyone reading it, we add the name of the symbol to the warning. Update the testsuite to account for this. Co-Author: Alex Coplan <alex.coplan@arm.com>
-
Matthew Malcomson authored
It had two problems: 1) The linker was storing permission flags in the bottom byte and the size in the top 56 bits. Newlib was looking for the permission flags in the top byte and the length in the bottom 56 bits of a uint64_t stored as bytes 8:16 of the fragment. N.b. The ABI requires a given storage order between the size and permission flags (as opposed to requiring a given uint64_t value be stored in the relevant position). This means that our current implementation would not work for a hypothetical big-endian Morello. 2) The linker prioritised SEC_READONLY flags over SEC_CODE ones on the section, this meant that function symbols into the .text section (which has both flags on it) would be given read-only permissions rather than executable permissions. This patch also must update all tests to account for this change.
-
- 03 Feb, 2022 1 commit
-
-
Alex Coplan authored
This patch has two main goals: - Relax an existing diagnostic to permit the linker to accept capability relocations against symbols without size information. - Adjust the capability base and bounds for symbols which point into sections which may be accessed via the PCC. The Morello ABI accesses global data using ADR and ADRP, and has no special indirection to jump to other functions. Given this, the PCC must maintain its bounds and base so that during execution loading global data and jumping to other functions can be done without worrying about the current PCC permissions and bounds. To implement this, all capabilities that could be loaded into the PCC (via BLR or similar) must have a bounds and base according to the PCC. This must span all global data and text sections (i.e. .got, .text, .got.plt and the like). There is already code finding the range that the PCC should span, this patch records the information in a variable that we can query later. There are two places where we create a relocation requesting a capability to be initialised at runtime. When handling relocations which request a capability from the GOT, and when handling a CAPINIT relocation. This patch adjusts both. We can't tell from inspection which symbols would be loaded into the PCC, but we know that those symbols must point into a section which is executable. For now, we do this operation for all symbols which point into an executable section. Most RELATIVE relocations don't use the addend. Rather the VA and size we want are put in the relative fragment and the addend is zero. This is because the *base* of the capability usually matches the VA we want that capability initialised to. In these possibly-code symbols we want the base of the capability bounds to be the base of the PCC, and the VA to be something very different. Hence we make use of the addend in the RELA relocations to encode this offset. Note on implementation: c64_fixup_frag takes the base and size of a capability we want to request from the runtime and checks that these are exactly representable in a capability. This patch changes many of the capabilities we request from the runtime to have the same bounds (those of the PCC). We leave the check to look at the bounds requested by the symbol rather than to check the PCC bounds multiple times. That means that if a symbol that points into an executable section has incorrect bounds then this will trigger a linker error even though it will cause no security problem when this executes. This is a trade-off between getting extra checks that the compiler is handling object bounds sizes and erroring on non-problematic code. We have a compatibility hack that if a symbol is defined in the linker script to be directly after a given section but is *named* something like __.*_start or __start_.* then we treat it as if it is defined at the very start of the next section. The new behaviour introduced in this patch needs to take account of the above compatibility hack. This patch also updates the testsuite according to these changes. In some places the original test no longer checks what it wanted, since the base of all symbols pointing into executable sections are now the same. There we add extra symbols and things to check so we ensure that this behaviour of PCC bounds is seen and that the original behaviour is still seen on non-executable sections. This commit also includes a few tidy-ups: We adjust the base and limit that are checked in c64_fixup_frag. Originally this would calculate the base as value + addend. As discussed above the way we treat capabilities in Morello is such that the value determines the base and the addend determines the initial value pointing from that base. Hence the check that these capabilities had correct bounds was not correct. We add an extra assertion in final_link_relocate for robustness purposes. There is an existing bug in the assembler where GOT relocations against local symbols can be turned into relocations against the relevant section symbol plus an addend. This is problematic for multiple reasons, one being that the linker implementation does not have any way to associate different GOT entries with the same symbol but multiple offsets. In fact the linker ignores any offset. Here we simply add an assertion that this never happens. It turns a silent pre-existing error into a noisy one. 2022-02-03 Alex Coplan <alex.coplan@arm.com> Matthew Malcomson <matthew.malcomson@arm.com> bfd/ChangeLog: * elfnn-aarch64.c (pcc_low): New. (pcc_high): New. (elfNN_c64_resize_sections): Update new global variables pcc_{low,high} instead of local variables to track PCC span. (enum c64_section_perm_type): New. (c64_symbol_section_adjustment): New. (c64_fixup_frag): Rework to calculate size appropriately for symbols that need adjustment. (c64_symbol_adjust): New. Use it ... (elfNN_aarch64_final_link_relocate): ... here. ld/ChangeLog: * testsuite/ld-aarch64/aarch64-elf.exp: Add new tests. * testsuite/ld-aarch64/emit-relocs-morello-6.d: New test. * testsuite/ld-aarch64/emit-relocs-morello-6.s: Assembly. * testsuite/ld-aarch64/emit-relocs-morello-6b.d: New test. * testsuite/ld-aarch64/emit-relocs-morello-7.d: New test. * testsuite/ld-aarch64/emit-relocs-morello-7.ld: Linker script thereof. * testsuite/ld-aarch64/emit-relocs-morello-7.s: Assembly. * testsuite/ld-aarch64/morello-capinit.d: New test. * testsuite/ld-aarch64/morello-capinit.ld: Linker script. * testsuite/ld-aarch64/morello-capinit.s: Assembly. * testsuite/ld-aarch64/morello-sizeless-global-syms.d: New test. * testsuite/ld-aarch64/morello-sizeless-global-syms.s: Assembly. * testsuite/ld-aarch64/morello-sizeless-got-syms.d: New test. * testsuite/ld-aarch64/morello-sizeless-got-syms.s: Assembly. * testsuite/ld-aarch64/morello-sizeless-local-syms.d: New test. * testsuite/ld-aarch64/morello-sizeless-local-syms.s: Assembly. Co-authored-by:
Matthew Malcomson <matthew.malcomson@arm.com>
-
- 18 Jan, 2022 2 commits
-
-
Matthew Malcomson authored
Trying to link code against newlib with the current BFD Morello linker we get quite a lot of cases of the error below. "relocation truncated to fit: R_MORELLO_LD128_GOT_LO12_NC against symbol `<whatever>' defined in .text.<whatever> section in <filename>" This happens because the relocation gets transformed into a relocation pointing into the GOT in elfNN_aarch64_final_link_relocate, but the h->target_internal flag that indicates whether this is a C64 function symbol or not is then added to the *end* value rather than the value that is stored in the GOT. This then correctly falls foul of a check in _bfd_aarch64_elf_put_addend that ensures the value we get from this relocation is 8-byte aligned since it must be pointing to the start of a valid entry in the GOT. Here we ensure that this LSB is set on the value newly added into the GOT rather than on the offset pointing into the GOT. This both means that loading function symbols from the GOT will have the LSB correctly set (hence we stay in C64 mode when branching to this function as we should) and it means that the error about a misaligned GOT address is fixed. In this patch we also ensure that we add a dynamic relocation to initialise the correct GOT entry when we are resolving a MORELLO relocation that requires an entry in the GOT. This was already handled in the case of a global symbol, but had not been handled in the case of a local symbol. This is why we set `relative_reloc` to TRUE in if resolving a MORELLO GOT relocation against a static executable. In writing the testcase for this patch we found an existing bug to do with static relocations of this kind (of this kind meaning that are handled in this case statement). The assembler often chooses to create the relocation against the section symbol rather than the original symbol, and make up for that by giving the relocation an addend. The linker does not have any mechanism to create "symbol plus addend" entries in the GOT -- it indexes into the GOT based on the symbol only. Hence all relocations which are a section symbol plus addend end up pointing at one value in the GOT just containing the value of the symbol. We do not fix this existing bug, but just note it given that this is in the same area.
-
Matthew Malcomson authored
The name has been changed in LLVM, so we adjust it in binutils to match. We also move where these symbols are created. Previously they were created in elfNN_aarch64_always_size_sections, but we move this to elfNN_aarch64_size_dynamic_sections. We do the moving since these symbols are supposed to span all dynamic capability relocations stored in the .rela.dyn section for static executables. In the case of a static binary we place relocations for the GOT into this section as well as internal relocations. These relocations for the GOT are handled in elfNN_aarch64_size_dynamic_sections, which is called *after* elfNN_aarch64_always_size_sections. The size of this section is only fully known after those GOT relocations are managed, so the position these symbols should be placed in is only known at that point. Hence we only initialise the __rela_dyn* symbols at that point. 2021-10-06 Matthew Malcomson <matthew.malcomson@arm.com> ChangeLog: * bfd/elfnn-aarch64.c (elfNN_aarch64_always_size_sections): Move initialisation of __rela_dyn* symbols ... (elfNN_aarch64_size_dynamic_sections): ... to here. * ld/testsuite/ld-aarch64/aarch64-elf.exp: Run new tests. * ld/testsuite/ld-aarch64/emit-morello-reloc-markers-1.d: New test. * ld/testsuite/ld-aarch64/emit-morello-reloc-markers-1.s: New test. * ld/testsuite/ld-aarch64/emit-morello-reloc-markers-2.d: New test. * ld/testsuite/ld-aarch64/emit-morello-reloc-markers-2.s: New test. * ld/testsuite/ld-aarch64/emit-morello-reloc-markers-3.d: New test. * ld/testsuite/ld-aarch64/emit-morello-reloc-markers-3.s: New test.
-
- 17 Jan, 2022 1 commit
-
-
Alex Coplan authored
The behaviour of weak undef thread-local variables is not well defined. TLS relocations against weak undef symbols are not handled properly by the linker, and in some cases cause the linker to crash (notably when linking glibc for purecap Morello). This patch simply ignores these and emits a warning to that effect. This is a compromise to enable progress for Morello. bfd/ChangeLog: 2022-01-17 Alex Coplan <alex.coplan@arm.com> * elfnn-aarch64.c (elfNN_aarch64_relocate_section): Skip over TLS relocations against weak undef symbols. (elfNN_aarch64_check_relocs): Likewise, but also warn. ld/ChangeLog: 2022-01-17 Alex Coplan <alex.coplan@arm.com> * testsuite/ld-aarch64/aarch64-elf.exp: Add morello-weak-tls test. * testsuite/ld-aarch64/morello-weak-tls.d: New test. * testsuite/ld-aarch64/morello-weak-tls.s: New test. * testsuite/ld-aarch64/weak-tls.d: Update test wrt new behaviour.
-
- 14 Jan, 2022 8 commits
-
-
Luis Machado authored
Handle internal variable values when displaying capability types.
-
Luis Machado authored
Instead of actively setting the tagged field of struct value *, initialize the tagged field right when allocating a new value. This simplifies the management of this field.
-
Luis Machado authored
Fix a bug where Morello GDB wasn't setting the capability tag when calling a function by hand, and said function received a pointer/capability as parameter. This was observed when attempting to call strlen by hand and passing the argv[] entries.
-
Luis Machado authored
A small refactoring to reduce duplication of code.
-
Luis Machado authored
Enable assignment of capabilities to C registers while preserving the capability tag. This enables operations like the following: set $c0=$c1 set $c0=p, where "p" is a capability (pointer) in AARCH64-CAP Due to the X/C register overlap, we also force a re-read of register data after every register write. So any 'G' packets are immediately followed by a 'g' packet.
-
Luis Machado authored
Improve the messages a little so the errors are more clear. One case in particular is when we attempt to write a tagged capability to a shared mapping area that doesn't support tags. GDB and GDBserver share the same error messages now.
-
Luis Machado authored
Issue: #9 Improve error message when setting capability registers fails. Mention the name of the set (capability) explicitly so it is clear what register set we are talking about.
-
Luis Machado authored
For dynamically-linked binaries using the AAPCS64-CAP ABI, the PCC bounds for distinct DSO's can be different, and that needs to be taken into account. Since there isn't a good interface for GDB to fetch the precise bounds (the .plt.got could be used for this, but doesn't contain data for symbols not being used by the program), we use maximum bounds and the existing PCC permissions. This allows GDB to call functions by hand in AAPCS64-CAP mode. Also, revert a previous change to do partial writes (lower 64 bits) of the PCC, as this isn't needed anymore.
-
- 20 Dec, 2021 2 commits
-
-
Luis Machado authored
Introduce corresponding C pseudo registers that contain the following fields: type = struct __gdb_builtin_type_capability { uint64_t l; uint64_t u; bool t; } For each register of the C set, users can reference the pseudo registers by adding a 'p' prefix. For example, the pseudo register of c0 and pcc are pc0 and ppcc. Users can set the entire 129 bits of the capability this way, if the cheri.ptrace_forge_cap flag is enabled.
-
Luis Machado authored
Sanity check the existence of a type field before dereferencing it.
-