[PATCH] D39130: [yaml2obj][ELF] Add support for setting alignment in program headers and fix default alignment

[Rafael Avila de Espindola via llvm-commits]

Jake Ehrlich via Phabricator via llvm-commits
<llvm-commits at lists.llvm.org> writes:

> jakehehrlich created this revision.
>
> Sometimes program headers have larger alignments than any of the sections they contain. Currently yaml2obj can't produce such files. A bug recently appeared in llvm-objcopy that failed in such a case. I'd like to be able to add tests to llvm-objcopy for such cases.
>
> This change adds an optional alignment parameter to program headers that will be used instead of calculating the alignment. This change also fixes the default alignment in program headers to be the maximum of all sections in the segment.  I was confused about what the exact alignment constraint was on program headers and this caused me to argue that the incorrect default choice should be made for segment alignment (and also caused the bug in llvm-objcopy).
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D39130
>
> Files:
>   include/llvm/ObjectYAML/ELFYAML.h
>   lib/ObjectYAML/ELFYAML.cpp
>   test/tools/yaml2obj/program-header-align.yaml
>   tools/yaml2obj/yaml2elf.cpp
>
> Index: tools/yaml2obj/yaml2elf.cpp
> ===================================================================
> --- tools/yaml2obj/yaml2elf.cpp
> +++ tools/yaml2obj/yaml2elf.cpp
> @@ -378,15 +378,18 @@
>      }
>  
>      // Set the alignment of the segment to be the same as the maximum alignment
> -    // of the the sections with the same offset so that by default the segment
> +    // of the the sections in the segment so that by default the segment
>      // has a valid and sensible alignment.
> -    PHeader.p_align = 1;
> -    for (auto SecName : YamlPhdr.Sections) {
> -      uint32_t Index = 0;
> -      SN2I.lookup(SecName.Section, Index);
> -      const auto &SHeader = SHeaders[Index];
> -      if (SHeader.sh_offset == PHeader.p_offset)

The removal of this if is missing a test, no?

In fact, would you mind splitting this patch in two? The first one would
just fix the existing computation. The second one would add the optional
explicit value.

Cheers,
Rafael