[PATCH] D75342: [obj2yaml] - Teach tool to dump program headers.

Thu Mar 19 04:17:15 PDT 2020

jhenderson added inline comments.

================
Comment at: llvm/tools/obj2yaml/elf2yaml.cpp:196
+      ELFYAML::Section &S = cast<ELFYAML::Section>(*C);
+      if ((S.Flags.getValueOr(ELFYAML::ELF_SHF(0)) & ELF::SHF_ALLOC) &&
+          isInSegment<ELFT>(S, Phdr))
----------------
grimar wrote:
> grimar wrote:
> > jhenderson wrote:
> > > As noted in the test, this SHF_ALLOC check is incorrect. It doesn't faithfully recreate what was in the object, and doesn't allow for non-loadable segments. Please remove it.
> > The approach used by this patch won't work then.     
> > 
> > We do not create `Chunk`s for non-allocatable `SHT_STRTAB`, `SHT_SYMTAB` and `SHT_DYNSYM`,
> > so iterating over `Chunks` does not work for non-allocatable case in general.
> > 
> When I experimented with how to implement it, I had a prototype that had more complex code, but had no this limitation.
> I'll try to return to what I had.
FWIW, our use case doesn't have the static symbol table or string table inside a segment, although I don't know of any strict reason why this shouldn't be allowed.

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

https://reviews.llvm.org/D75342