Elf Builder fails with non-homogenous PT_LOAD alignments

[JaciBrunning]

When calling object::build::elf::Builder::read32(...) on an ELF file that contains different alignments in its load sections (e.g. the below), the following error is raised: "Unsupported alignments for PT_LOAD segments"

From objdump -x myelf:

Program Header:
    LOAD off    0x00000114 vaddr 0x08000000 paddr 0x08000000 align 2**2
         filesz 0x0000020d memsz 0x0000020d flags r--
    LOAD off    0x00000324 vaddr 0x08000250 paddr 0x08000250 align 2**2
         filesz 0x00004260 memsz 0x00004260 flags r-x
    LOAD off    0x00004584 vaddr 0x080044b0 paddr 0x080044b0 align 2**2
         filesz 0x000003fc memsz 0x000003fc flags r--
    LOAD off    0x00004980 vaddr 0x20000100 paddr 0x20000100 align 2**3
         filesz 0x00000000 memsz 0x00001070 flags rw-
   STACK off    0x00000000 vaddr 0x00000000 paddr 0x00000000 align 2**0
         filesz 0x00000000 memsz 0x00000000 flags rw-
private flags = 0x5000400: [Version5 EABI] [hard-float ABI]

I may be missing something, but I don't see a reason for this restriction, and causes the elf builder to fail to load a subset of files that are valid. In my case, this prevents us from being able to read ELF files that perform floating point trigonometric methods on a 32-bit platform with hard-float support.

The issue seems to originate in src/build/elf.rs:146 and onwards, where the alignment of the first section with PT_LOAD is latched through builder.load_align.

            if segment.p_type(endian) == elf::PT_LOAD {
                let p_align = segment.p_align(endian).into();
                if builder.load_align != 0 && builder.load_align != p_align {
                    return Err(Error::new("Unsupported alignments for PT_LOAD segments"));
                }
                builder.load_align = p_align;
            }

The alignments are stored per-segment in the lines following, so it seems odd to be storing it at the builder level when I can only find references to builder.load_align in the rewrite crate.

            builder.segments.push(Segment {
                id,
                delete: false,
                p_type: segment.p_type(endian),
                p_flags: segment.p_flags(endian),
                p_offset: segment.p_offset(endian).into(),
                p_vaddr: segment.p_vaddr(endian).into(),
                p_paddr: segment.p_paddr(endian).into(),
                p_filesz: segment.p_filesz(endian).into(),
                p_memsz: segment.p_memsz(endian).into(),
                p_align: segment.p_align(endian).into(),
                sections: Vec::new(),
                marker: PhantomData,
            });

Is there a reason that I'm missing for the PT_LOAD segments to be homogenous in alignment as a requirement? If not, I'm happy to open a PR to fix this issue.

Thanks!

[philipc]

My assumption was that all PT_LOAD segment were page aligned, but that's clearly wrong. load_align would be better named page_size, and the requirement for all PT_LOAD segments to use it should be changed. We'll need a new way of determining the page size for cases where PT_LOAD segments don't use it as their alignment.

It seems that the program headers you gave don't satisfy the requirement that "loadable process segments must have congruent values for p_vaddr and p_offset, modulo the page size", so I guess there are some processors for which this is not a requirement?

I don't have much experience in this area, so if I have further wrong assumptions here then please correct me.

[philipc]

To elaborate, the congruence requirement comes from https://docs.oracle.com/cd/E23824_01/html/819-0690/chapter6-83432.html, but it does seem to be talking about something that is likely processor specific. We currently implement that requirement in Segments::add_load_segment, and the alignment passed in there is expected to be builder.load_align, as is done in the rewrite crate, so whatever fix we do will need to still handle this requirement for cases where it is needed.

[JaciBrunning]

I think you're right in that it's processor-dependent. This is specifically for the Cortex M4F (thumbv7em-none-eabihf), as found on the STM32G4 lineup. I've got something workable for now but it breaks the rewrite crate, so I'll have a look and see if I can fix whatever's required on the other side to make that work, too

[philipc]

See if #659 meets your needs.