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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D75342#1919197 <https://reviews.llvm.org/D75342#1919197>, @grimar wrote:

> - Reimplemented, added test cases.
>
> In D75342#1903392 <https://reviews.llvm.org/D75342#1903392>, @MaskRay wrote:
>
> > @jhenderson listed many good test cases. It requires some work. I've left some notes.
>
>
> Thank you guys for these suggestions and comments, I used them in the update.
>
> I've added tests for all cases listed except the next two.
>
> >> Segments with gaps in them (file size != size of all sections), where gap is at start/middle/end of segment.
> > 
> > Type: Filler
>
> I think we might want having a separate test for this. Also it needs more code I think.
>
> >> Segments with memsize > file size.
> > 
> > Needs non-empty SHT_NOBITS.
>
> I'd like to support SHT_NOBITS case separatelly too for the same reasons.
>
> How does it sound?






================
Comment at: llvm/test/tools/obj2yaml/program-headers.yaml:113
+ProgramHeaders:
+## Check we can create a PT_LOAD with arbitrary (we used .hash, .gnu.hash)
+## and implicit sections (we used .dynsym, .dynstr).
----------------
used->use


================
Comment at: llvm/tools/obj2yaml/elf2yaml.cpp:195
+    for (const std::unique_ptr<ELFYAML::Chunk> &C : Chunks) {
+      ELFYAML::Section *S = cast<ELFYAML::Section>(C.get());
+      if (S->Flags && ((*S->Flags) & ELF::SHF_ALLOC))
----------------
`cast<ELFYAML::Section>(*C)` is better because you assert it to be non-null.


================
Comment at: llvm/tools/obj2yaml/elf2yaml.cpp:196
+      ELFYAML::Section *S = cast<ELFYAML::Section>(C.get());
+      if (S->Flags && ((*S->Flags) & ELF::SHF_ALLOC))
+        if (isInSegment<ELFT>(*S, Phdr))
----------------
The two `if` can be combined.


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

https://reviews.llvm.org/D75342