Created attachment 15430 [details] contains c file and linker script This bug was found while building a image for os How to reproduce : Take the attached c file and linker-sript and follow through steps 1> clang reprod.c -ffreestanding -c -o reprod.img 2> ld.lld --defsym VMA_START=0xffffffff80100000 --defsym LMA_START=0x4800000 -T l.ld reprod.img -o reprod.img 3> objcopy --change-section-lma .*-0xffffffff80000000 reprod.img 4> objcopy --adjust-start -0xffffffff80000000 reprod.img Tool version's: using clang-17 and binutils 2.40 on x86_64 but issue seems target agnostic. we get the following issues First issue we encounter. objcopy: stq0GBq2: section `.data' can't be allocated in segment 0 LOAD: .tbss .data .bss .sdata objcopy: stq0GBq2: section `.bss' can't be allocated in segment 0 LOAD: .tbss .data .bss .sdata second issue the image sizes are overblown du -sh a.out 34G a.out Note that the bug seems to only be reproducible with lld Preliminary observations: lld TLS 0x0000000000002058 0xffffffff80100060 (vma) 0xffffffff80100060 (lma) 0x0000000000000000 0x0000000000000004 R 0x4 03 .tbss Ld Linker TLS 0x0000000000100098 0xffffffff801000a0 (vma) 0xffffffffc81000a0 (lma) 0x0000000000000000 0x0000000000000004 R 0x4 ELF_SECTION_IN_SEGMENT_1 (include/elf/internal.h) is used to check if section belongs into a segment (based on VMA) but the heuristics fail because tbss is considered part of LOAD when it shouldn't be considered as one this becomes an issue when assigning file offsets assign_file_positions_for_load_sections (bfd/elf.c) sorts segments based on lma and this is when things seem to go for a toss. Previous discussion thread: https://lists.gnu.org/archive/html/bug-binutils/2024-03/msg00098.html
I have a patch suggestion for this issue added check to copy_private_bfd_data so that sections with inconsistent relation between vma and lma go to re-write im not sure how this will affect other cases maybe linker scripts? waiting on experts opinion on this change. alternative way would be to exclude tbss from PT_LOAD by adding checks in releavent places ( ELF_SECTION_IN_SEGMENT_1, make_mapping ). diff --git a/bfd/elf.c b/bfd/elf.c index c305b40e..b45e1eee 100644 --- a/bfd/elf.c +++ b/bfd/elf.c @@ -8394,7 +8394,7 @@ copy_private_bfd_data (bfd *ibfd, bfd *obfd) /* Check to see if any sections in the input BFD covered by ELF program header have changed. */ Elf_Internal_Phdr *segment; - asection *section, *osec; + asection *section, *osec, *prev; unsigned int i, num_segments; Elf_Internal_Shdr *this_hdr; const struct elf_backend_data *bed; @@ -8425,7 +8425,7 @@ copy_private_bfd_data (bfd *ibfd, bfd *obfd) || segment->p_type == PT_DYNAMIC)) goto rewrite; - for (section = ibfd->sections; + for (section = ibfd->sections,prev=section; section != NULL; section = section->next) { /* We mark the output section so that we know it comes @@ -8440,16 +8440,21 @@ copy_private_bfd_data (bfd *ibfd, bfd *obfd) { /* FIXME: Check if its output section is changed or removed. What else do we need to check? */ + /* make sure this sections vma and lma relation is + same as previous section otherwise it needs a + rewrite */ if (osec == NULL || section->flags != osec->flags || section->lma != osec->lma || section->vma != osec->vma || section->size != osec->size || section->rawsize != osec->rawsize - || section->alignment_power != osec->alignment_power) + || section->alignment_power != osec->alignment_power + || section->lma - section->vma != prev->lma - prev->vma) goto rewrite; } } + prev = section; } /* Check to see if any output section do not come from the
(In reply to vijay Shankar from comment #2) > I have a patch suggestion for this issue I am running the patch through my test farm and so far it is looking good. Can you confirm that the patch fixes the problem you initially reported ? Cheers Nick
> Can you confirm that the patch fixes the problem you initially reported ? Yes, can confirm that the patch does fix the issue Here's section to segment mapping with and without patch without change ------SEGMAP------ LOAD: .eh_frame ------SEGMAP------ LOAD: .text ------SEGMAP------ LOAD: .data .bss .tbss .sdata ------SEGMAP------ TLS: .tbss ------SEGMAP------ STACK: ./objcopy: stVzY0CU: section `.data' can't be allocated in segment 0 LOAD: .tbss .data .bss .sdata ./objcopy: stVzY0CU: section `.bss' can't be allocated in segment 0 LOAD: .tbss .data .bss .sdata with change ------SEGMAP------ LOAD: .eh_frame ------SEGMAP------ LOAD: .text ------SEGMAP------ LOAD: .data .bss .sdata ------SEGMAP------ TLS: .tbss ------SEGMAP------ STACK:
The master branch has been updated by Nick Clifton <nickc@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=ccbf42ec88f3e8bbb74dbdc1d6c9da3a9d805cff commit ccbf42ec88f3e8bbb74dbdc1d6c9da3a9d805cff Author: Vijay Shankar <shank.vijay@yandex.com> Date: Mon Apr 15 16:27:21 2024 +0100 When mapping sections to segments ensure that we do not add sections whose VMA->LMA relationship does not match the relationship of earlier sections in the segment. PR 31540
Right - well the patch looks good to me, and did not trigger any problems, so I have gone ahead and checked it in.
(In reply to Nick Clifton from comment #6) > Right - well the patch looks good to me, and did not trigger any problems, > so I have gone ahead and checked it in. Thanks Nick.
Nick, your change to binutils/testsuite/binutils-all/pr25662.ld results in arc-elf +FAIL: objcopy executable (pr25662) arc-linux-uclibc +FAIL: objcopy executable (pr25662) bfin-linux-uclibc +FAIL: objcopy executable (pr25662) frv-linux-gnu +FAIL: objcopy executable (pr25662) spu-elf +XPASS: objcopy executable (pr25662)
Hi Alan, > https://sourceware.org/bugzilla/show_bug.cgi?id=31540 > > --- Comment #8 from Alan Modra <amodra at gmail dot com> --- > Nick, your change to binutils/testsuite/binutils-all/pr25662.ld results in > arc-elf +FAIL: objcopy executable (pr25662) > arc-linux-uclibc +FAIL: objcopy executable (pr25662) > bfin-linux-uclibc +FAIL: objcopy executable (pr25662) > frv-linux-gnu +FAIL: objcopy executable (pr25662) > spu-elf +XPASS: objcopy executable (pr25662) Doh! That change was not even meant to go in. It was a potential patch for a completely different issue. I will revert my commit. Sorry about the mis-commmit! Cheers Nick