[PATCH] D43944: [ELF] - Refactoring of the elf/program headers allocation code.

[Rafael Avila de Espindola via llvm-commits]

George Rimar via Phabricator via llvm-commits
<llvm-commits at lists.llvm.org> writes:

> Index: ELF/LinkerScript.cpp
> ===================================================================
> --- ELF/LinkerScript.cpp
> +++ ELF/LinkerScript.cpp
> @@ -897,13 +897,27 @@
>    return nullptr;
>  }
>  
> -static uint64_t computeBase(uint64_t Min, bool AllocateHeaders) {
> +static bool hasExplicitHeaders(ArrayRef<PhdrsCommand> Phdrs) {
> +  return llvm::any_of(Phdrs, [](const PhdrsCommand &Cmd) {
> +    return Cmd.HasPhdrs || Cmd.HasFilehdr;
> +  });
> +}

I am not a big fan of calling this twice. The cost might be low, but it
gives the impression that you might get different results on each call.

> +static Optional<uint64_t> getHeadersVA(uint64_t MinVA, uint64_t HeaderSize,
> +                                       ArrayRef<PhdrsCommand> Phdrs) {
> +  // Unable to allocate headers when their size is greater than minimal VA.
> +  if (HeaderSize > MinVA)
> +    return None;

This function now has two checks that are effectively duplicates:

HeaderSize > MinVA which is !(HeaderSize <= MinVA)

and

HeaderSize <= MinVA - alignDown(MinVA, Config->MaxPageSize)

They are duplicates because alignDown(MinVA, Config->MaxPageSize) is
always >= 0. That is why I like a structure where we first compute the
lowest allowed address (computeBase, a better name would be welcome).

Cheers,
Rafael