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

["H.J. Lu" <hjl.tools@gmail.com>]

On Mon, Dec 20, 2021 at 8:36 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Fangrui Song:
>
> > On 2021-12-15, H.J. Lu via Binutils wrote:
> >>Currently, on 32-bit and 64-bit ARM, it seems that ld generates p_align
> >>values of 0x10000 even if no section alignment is greater than 0x1000.
> >>The issue is more general and probably affects other targets with
> >>multiple common page sizes.
> >>
> >>While file layout absolutely must take 64K page size into account, that
> >>does not have to be reflected in the p_align value.  If running on a 64K
> >>kernel, the file will be loaded at a 64K page boundary by necessity. On
> >>a 4K kernel, 64K alignment is not needed.
> >>
> >>The glibc loader has been fixed to honor p_align:
> >
> > Maybe it's just me who is very careful on the words: aligning to p_align
> > is a new feature, not a bug, as no ABI requires it. No ld.so I know
> > (FreeBSD, musl, bionic) does this.
>
> The expectation seems to be fairly clear that p_align should reflect
> segment alignment.  It's true that the ELF specification does not
> explicitly say that segment alignment of virtual addresses also carries
> over to the process image, but that part seems so obvious that it
> perhaps wasn't stated explicitly.
>
> Current linkers probably should have used the reserved values 0 and 1 if
> they want to convey that alignment does not matter.
>
> >>https://sourceware.org/bugzilla/show_bug.cgi?id=28676
> >>
> >>similar to kernel:
> >>
> >>commit ce81bb256a224259ab686742a6284930cbe4f1fa
> >>Author: Chris Kennelly <ckennelly@google.com>
> >>Date:   Thu Oct 15 20:12:32 2020 -0700
> >>
> >>    fs/binfmt_elf: use PT_LOAD p_align values for suitable start address
> >
> > This kernel patch has no cost. It just picks a load bias, while the
> > glibc's .so loading patch has some costs because there is no alignment
> > parameter to mmap... So now, every
> >
> > * (Linux x86-64) -z noseparate-code (default max-page-size=2MiB) .so incurs some munmap overhead
> > * arm/aarch64/powerpc (default max-page-size=65536) .so incurs some munmap overhead...
> >
> > If I were to do this, I would fix objcopy first, then adjust ld's
> > p_align, finally tune glibc's .so loading.
>
> Maybe we need to add some markup that the p_align value is actually
> real.
>

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.

-- 
H.J.