This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] Use offsets instead of addresses in ELF_SECTION_IN_SEGMENT
On Wed, May 23, 2018 at 09:53:06AM +0000, Alan Hayward wrote:
> (swapping original email order around)
> Thanks for the review!
>
> > On 22 May 2018, at 15:04, Alan Modra <amodra@gmail.com> wrote:
> >
> >> diff --git a/include/elf/internal.h b/include/elf/internal.h
> >> index 05f9fab89c..5d93e0070e 100644
> >> --- a/include/elf/internal.h
> >> +++ b/include/elf/internal.h
> >> @@ -347,9 +347,9 @@ struct elf_segment_map
> >> || ((sec_hdr)->sh_flags & SHF_ALLOC) == 0 \
> >> || ((sec_hdr)->sh_addr >= (segment)->p_vaddr \
> >> && (!(strict) \
> >> - || ((sec_hdr)->sh_addr - (segment)->p_vaddr \
> >> + || ((sec_hdr)->sh_offset - (segment)->p_offset \
> >> <= (segment)->p_memsz - 1)) \
> >> - && (((sec_hdr)->sh_addr - (segment)->p_vaddr \
> >> + && (((sec_hdr)->sh_offset - (segment)->p_offset \
> >> + ELF_SECTION_SIZE(sec_hdr, segment)) \
> >> <= (segment)->p_memsz))) \
> >> /* No zero size sections at start or end of PT_DYNAMIC. */ \
> >
> > We test sh_offset already, just above the code you are patching. So I
> > think this VMA test should continue to test sh_addr.
> >
> > Now there are reasons you might want to disable the VMA checks
> > entirely, for example when overlays are present. To do that, use
> > ELF_SECTION_IN_SEGMENT_1 with check_vma false. See the example in
> > elf.c assign_files_positions_for_load_sections.
> >
>
> Hmm, yes I somehow hadn’t spotted that.
>
> My issue with ELF_SECTION_IN_SEGMENT was it’s use in
> _bfd_elf_make_section_from_shdr(). If I remove my patch and replace it
> with the following then everything “works” for me:
>
> diff --git a/bfd/elf.c b/bfd/elf.c
> index 6c66bbca02..1cf3131b2a 100644
> --- a/bfd/elf.c
> +++ b/bfd/elf.c
> @@ -1140,7 +1140,7 @@ _bfd_elf_make_section_from_shdr (bfd *abfd,
> if (((phdr->p_type == PT_LOAD
> && (hdr->sh_flags & SHF_TLS) == 0)
> || phdr->p_type == PT_TLS)
> - && ELF_SECTION_IN_SEGMENT (hdr, phdr))
> + && ELF_SECTION_IN_SEGMENT_1 (hdr, phdr, 0, 0))
> {
> if ((flags & SEC_LOAD) == 0)
> newsect->lma = (phdr->p_paddr
>
> Whilst this logically gives the same result as my patch, it feels
> less like the correct solution.
>
>
> > On Tue, May 22, 2018 at 11:52:15AM +0100, Alan Hayward wrote:
> >> Use sh_offset and p_offset instead of sh_addr and p_vaddr when calculating if a section fits in a segment. Both methods are valid when using the GNU linker.
> >>
> >> This change makes ELF_SECTION_IN_SEGMENT consistent with BFD _bfd_elf_make_section_from_shdr(), which has used offsets to calculate LMA since 2008.
> >
> > LMA is not VMA. The code you referenced doesn't really support the
> > change you propose, nor did you say exactly why the change is needed.
>
>
> I was hoping to not need to go into too much detail (my original version
> of the patch email did have a description of the Armlinker, which I removed
> due to it being a large off putting info dump).
Thanks for the explanation.
> The Armlinker in Arm Compiler uses a feature of scatter-loading, in the
> sense that the linker has generated an image whose first task on startup is
> to copy pieces of itself from their load addresses to their execution
> addresses. Specifically, the entire image is packed into a single contiguous
> blob running from addresses 0x0 – 0x49d4 (you imagine this being the target
> embedded device's ROM / PROM / Flash / whatever), and the first thing that
> happens is that the function __scatterload copies part of that range to
> 0x20000000 (the device's actual RAM, where the RW data will live during the
> program's run time) and also zeroes out another range in the 0x20000000 area
> (the bss / ZI data section).
>
> (Other scatter loading features include data compression: in some situations
> the version of a section's data packed into the initial ROM image will not even
> match its runtime contents byte for byte, because instead it will be compressed,
> and the scatter loading code will call a decompressor instead of a memcpy-type
> function.)
>
> In images of this type, armlink's policy – since more or less forever – has
> been to use section addresses (i.e. sh_addr in the section header table entries)
> to indicate where a given section is expected to end up after the initial scatter
> loading pass has run; and the segment addresses (p_vaddr and p_paddr – armlink
> draws no distinction between the two, and I think in fact it always sets them
> identically) indicate where and how the image expects to have been installed in
> memory at the point where it starts running, i.e. before scatter loading. So in
> this kind of case where the image starts off packed into a single chunk of ROM
> and scatters itself out into RAM at the beginning of its run, the segment
> addresses will describe the starting chunk of ROM, and the section addresses
> show where everything ends up at the point where the interesting part of the
> runtime begins.
>
> The idea is that this gives a debugger everything it needs to know to both load
> the image into memory in the first place (you use the segment addresses for that),
> and to map memory addresses back to code / data / symbols during the main
> runtime of the program (you use the section addresses, on the assumption that by
> the time you're concerned with an address in any given section, that section has
> already gone through scatter loading).
>
> For example, here is the readelf output of a small binary I compiled. Note the
> address for ER_DATA with PROGBITS. In the existing code this will use sh_addr,
> incorrectly loading it to 0xead4. What we want instead is to use (p_vaddr +
> sh_offset - p_offset), which is (0x8000 + 0x2b08 - 0x34), which is 0xaad4.
>
>
> Section Headers:
> [Nr] Name Type Addr Off Size ES Flg Lk Inf Al
> [ 0] NULL 00000000 000000 000000 00 0 0 0
> [ 1] ER_CODE PROGBITS 00008000 000034 002ad4 00 AX 0 0 4
> [ 2] ER_DATA PROGBITS 0000ead4 002b08 000020 00 WA 0 0 4
> [ 3] ER_DATA NOBITS 0000eaf4 002b28 00015c 00 WA 0 0 4
> [ 4] ARM_LIB_STACKHEAP NOBITS 00040000 002b28 001000 00 WA 0 0 1
> [ 5] .debug_abbrev PROGBITS 00000000 002b28 000065 00 0 0 1
> [ 6] .debug_frame PROGBITS 00000000 002b8d 000d9c 00 0 0 1
> [ 7] .debug_info PROGBITS 00000000 003929 00006e 00 0 0 1
> [ 8] .debug_line PROGBITS 00000000 003997 00003c 00 0 0 1
> [ 9] .debug_pubnames PROGBITS 00000000 0039d3 000029 00 0 0 1
> [10] .debug_pubtypes PROGBITS 00000000 0039fc 000023 00 0 0 1
> [11] .debug_str PROGBITS 00000000 003a1f 00008b 00 0 0 1
> [12] .symtab SYMTAB 00000000 003aac 002d60 10 13 498 4
> [13] .strtab STRTAB 00000000 00680c 001f54 00 0 0 1
> [14] .note NOTE 00000000 008760 000020 00 0 0 4
> [15] .comment PROGBITS 00000000 008780 0003e0 00 0 0 1
> [16] .shstrtab STRTAB 00000000 008b60 0000ac 00 0 0 1
>
> Program Headers:
> Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
> LOAD 0x000034 0x00008000 0x00008000 0x02af4 0x03c50 RWE 0x4
>
>
> All of this is still within the elf spec, but clearly different from the GNU
> linker - the idea was to get a patch that allowed us the safely work for both
> without any horrible “if gnu ... else if armlinker ...” code.
I think you may need to detect these armlinker segments. The problem
with disabling the sh_addr comparisons is that sh_offset for
SHT_NOBITS sections doesn't really have a physical meaning. You don't
load SHT_NOBITS from file after all. Your armlinker binaries might
have the convention that sh_offset is given values as if a segment's
bss part actually existed on disk, but not all ELF binaries will
follow that convention. What's more, the convention breaks down in
the presence of multiple tightly packed PT_LOAD segments where any but
the last has p_memsz > p_filesz. That situation would lead to
SHT_NOBITS sections having sh_offset values corresponding to the
PT_LOAD segment past the one to which they actually belong.
If the armlinker only ever creates one PT_LOAD segment, it might be
possible to accommodate it by disabling check_vma whenever there is
just one PT_LOAD segment.
--
Alan Modra
Australia Development Lab, IBM