[PATCH] elf: Set p_align to the common page size if possible

[Florian Weimer]

* H. J. Lu:

> On Mon, Dec 20, 2021 at 10:51 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * H. J. Lu:
>>
>> > We need to set larger p_align > sh_addralign for huge page executables.
>> > My current algorithm to decide if p_align should be used as the maximum
>> > page size for objcopy is
>> >
>> > static bool
>> > elf_is_p_align_valid (bfd *abfd)
>> > {
>> >   unsigned int i;
>> >   Elf_Internal_Phdr *segment;
>> >   unsigned int num_segments;
>> >   const struct elf_backend_data *bed = get_elf_backend_data (abfd);
>> >   bfd_size_type maxpagesize = bed->maxpagesize;
>> >   bfd_size_type commonpagesize = bed->commonpagesize;
>> >
>> >   if (commonpagesize == maxpagesize)
>> >     return true;
>> >
>> >   /* When the common page size != the maximum page size, p_align may
>> >      be set to the common page size while segments are aligned to
>> >      the maximum page size.  In this case, the input p_align will be
>> >      ignored and the maximum page size will be used to align the output
>> >      segments.  */
>> >   segment = elf_tdata (abfd)->phdr;
>> >   num_segments = elf_elfheader (abfd)->e_phnum;
>> >   for (i = 0; i < num_segments; i++, segment++)
>> >     if (segment->p_type == PT_LOAD
>> >         && (segment->p_align != commonpagesize
>> >             || vma_page_aligned_bias (segment->p_vaddr,
>> >                                       segment->p_offset,
>> >                                       maxpagesize) != 0))
>> >       return true;
>> >
>> >   return false;
>> > }
>> >
>> > It should cover all cases.
>>
>> Will this switch from 64K to 4K on POWER and AArch64?  I think this will
>
> It will set p_align to ELF_COMMONPAGESIZE.  It glibc doesn't support
> p_align == ELF_COMMONPAGESIZE, the ELF_COMMONPAGESIZE
> is wrong.

Hmm.

bfd/elf32-arm.c:#define ELF_COMMONPAGESIZE              0x1000
bfd/elf32-arm.c:#undef  ELF_COMMONPAGESIZE
bfd/elf32-arm.c:#undef  ELF_COMMONPAGESIZE
bfd/elf32-arm.c:#define ELF_COMMONPAGESIZE              0x1000
bfd/elf32-ppc.c:#define ELF_COMMONPAGESIZE      0x1000
bfd/elf64-ppc.c:#define ELF_COMMONPAGESIZE      0x1000
bfd/elfnn-aarch64.c:#define ELF_COMMONPAGESIZE          0x1000

This looks problematic.

We have a sed patch in binutils.spec to change ELF_COMMONPAGESIZE:

# On ppc64 and aarch64, we might use 64KiB pages
sed -i -e '/#define.*ELF_COMMONPAGESIZE/s/0x1000$/0x10000/' bfd/elf*ppc.c
sed -i -e '/#define.*ELF_COMMONPAGESIZE/s/0x1000$/0x10000/' bfd/elf*aarch64.c
sed -i -e '/common_pagesize/s/4 /64 /' gold/powerpc.cc
sed -i -e '/pagesize/s/0x1000,/0x10000,/' gold/aarch64.cc

So I guess we paper over the problem.  But we reject to run-time load
binaries without this patch due to bug 28688.

I think we need to fix glibc bug 28688 and drop our downstream
workaround from our binutils builds.  We tried to fix it in the wrong
place, it seems.

This lack of cross-distro interoperability is embarrassing. 8-(

Thanks,
Florian