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.
############### Attachment also inlined for ease of reply ###############
diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c index f33388629164b59e07cdbac7bb4bce82e2f62b8b..f3f058934ea203cb735b9b4baf77ba8e8f854066 100644 --- a/bfd/elfnn-aarch64.c +++ b/bfd/elfnn-aarch64.c @@ -5040,7 +5040,7 @@ elfNN_c64_resize_sections (bfd *output_bfd, struct bfd_link_info *info, if (queue->sec->alignment_power < align) queue->sec->alignment_power = align;
- padding = high - queue->sec->vma - queue->sec->size; + padding = high - low - queue->sec->size;
if (queue->sec != pcc_high_sec) { @@ -5066,7 +5066,10 @@ elfNN_c64_resize_sections (bfd *output_bfd, struct bfd_link_info *info, if (pcc_low_sec->alignment_power < align) pcc_low_sec->alignment_power = align;
- padding = pcc_high - pcc_high_sec->vma - pcc_high_sec->size; + bfd_vma current_length = + (pcc_high_sec->vma + pcc_high_sec->size) - pcc_low_sec->vma; + bfd_vma desired_length = (pcc_high - pcc_low); + padding = desired_length - current_length; c64_pad_section (pcc_high_sec, padding); } } diff --git a/ld/emultempl/aarch64elf.em b/ld/emultempl/aarch64elf.em index 95cee025e3dc7f0f573ed170db3a0f050b375e6f..8a123106e3df3a0236cf818051430b5ef27eca8e 100644 --- a/ld/emultempl/aarch64elf.em +++ b/ld/emultempl/aarch64elf.em @@ -216,7 +216,10 @@ elf64_c64_pad_section (asection *osec, bfd_vma padding) lang_output_section_statement_type *os = lang_output_section_get (osec);
lang_list_init (&list); - lang_add_newdot (&list, osec->vma + osec->size + padding); + lang_add_assignment_internal (&list, + exp_assign (".", + exp_binop ('+', exp_nameop (NAME, "."), exp_intop (padding)), + FALSE));
if (list.head == NULL) { @@ -224,8 +227,8 @@ elf64_c64_pad_section (asection *osec, bfd_vma padding) return; }
- *(list.tail) = os->header.next; - os->header.next = list.head; + *(list.tail) = NULL; + *(os->children.tail) = list.head; } }
diff --git a/ld/ldlang.h b/ld/ldlang.h index 6074e2cb0129f60d7efbd44c8d9a14050b44f7c2..89ee85e557363811c63ac168ddc7aca9271e8d21 100644 --- a/ld/ldlang.h +++ b/ld/ldlang.h @@ -561,6 +561,8 @@ extern lang_assignment_statement_type *lang_add_assignment (union etree_union *); extern void lang_add_attribute (enum statement_enum); +extern void lang_add_assignment_internal + (lang_statement_list_type *, etree_type *); extern void lang_add_newdot (lang_statement_list_type *, bfd_vma); extern void lang_startup diff --git a/ld/ldlang.c b/ld/ldlang.c index fb9340603c2aa2f0dbf038ea975b192d0e95349f..09feb5b7e1fc63d0d1cde59754fc991fc3bf2dce 100644 --- a/ld/ldlang.c +++ b/ld/ldlang.c @@ -8370,6 +8370,15 @@ lang_add_assignment (etree_type *exp) return new_stmt; }
+void +lang_add_assignment_internal (lang_statement_list_type *ptr, etree_type *exp) +{ + lang_assignment_statement_type *new_stmt; + + new_stmt = new_stat (lang_assignment_statement, ptr); + new_stmt->exp = exp; +} + void lang_add_newdot (lang_statement_list_type *ptr, bfd_vma newdot) {