[PATCH] D91152: [obj2yaml] - Dump section offsets in some cases.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D91152#2388533 <https://reviews.llvm.org/D91152#2388533>, @grimar wrote:

> In D91152#2388491 <https://reviews.llvm.org/D91152#2388491>, @jhenderson wrote:
>
>> Doesn't the behaviour of a SHT_NOBITS section depend on whether it is a non-last member of a segment? IIRC, SHT_NOBITS sections in a segment which are last do not need file space, but to ensure correct addresses for sections appearing after them in a segment, they are allocated filespace by a linker if non-NOBITS appear after them in the same segment. (The behaviour you're doing here looks fine for when such sections are not inside segments, e.g. for ET_REL objects).
>
> True. But how much necessary to support this case now? The problem is that we at first dump sections and then dump program headers.
> To implement the precise behavior, I think we need to delay assigning offsets to be able to call `shouldAllocateFileSpace`, which does the job:

In `dumpProgramHeaders` we call `isInSegment` to know the parent segment of a section. If we move the logic earlier before `setOffsets`, we can refine the computation of the expected sh_offset.
With a containing PT_LOAD, sh_offset should equal its address modulo p_align.

>   static bool shouldAllocateFileSpace(ArrayRef<ELFYAML::ProgramHeader> Phdrs,
>                                       const ELFYAML::NoBitsSection &S) {
>     for (const ELFYAML::ProgramHeader &PH : Phdrs) {
>       auto It = llvm::find_if(
>           PH.Chunks, [&](ELFYAML::Chunk *C) { return C->Name == S.Name; });
>       if (std::any_of(It, PH.Chunks.end(), [](ELFYAML::Chunk *C) {
>             return (isa<ELFYAML::Fill>(C) ||
>                     cast<ELFYAML::Section>(C)->Type != ELF::SHT_NOBITS);
>           }))
>         return true;
>     }
>     return false;
>   }
>
> I wonder - do we want adding this complexity? I thought that nobits sections are usually placed after progbits anyways.




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91152/new/

https://reviews.llvm.org/D91152