[PATCH] D90458: [yaml2obj] - ProgramHeaders: introduce FirstSec/LastSec instead of Sections list.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/tools/obj2yaml/elf2yaml.cpp:392-399
     for (const std::unique_ptr<ELFYAML::Chunk> &C : Chunks) {
       ELFYAML::Section &S = cast<ELFYAML::Section>(*C.get());
-      if (isInSegment<ELFT>(S, Sections[S.OriginalSecNdx], Phdr))
-        PH.Sections.push_back({S.Name});
+      if (isInSegment<ELFT>(S, Sections[S.OriginalSecNdx], Phdr)) {
+        if (!PH.FirstSec)
+          PH.FirstSec = S.Name;
+        PH.LastSec = S.Name;
+      }
----------------
grimar wrote:
> jhenderson wrote:
> > Does this loop work if section headers are not in section offset order? Or are they guaranteed to be by some sorting earlier on?
> I have no short answer, we have a specific behavior for cases where sections are not in the
> section offset order currently, let me describe it.
> 
> Imagine we have 3 sections:
> .foo at offset 0x100, .bar at offset 0x200 and .zed at offset 0x300.
> And lets have a section header table where they are in a different order:
> .zed (0x300), .foo (0x100) and .bar (0x200).
> 
> And lets have a segment [.foo, .zed]. The YAML for this case is shown below:
> 
> ```
> --- !ELF
> FileHeader:
>   Class: ELFCLASS64
>   Data:  ELFDATA2LSB
>   Type:  ET_EXEC
> Sections:
>   - Name:    .foo
>     Type:    SHT_PROGBITS
>     Flags:   [ SHF_ALLOC ]
>     Size:    0x1
>     Offset:  0x100
>   - Name:    .bar
>     Type:    SHT_PROGBITS
>     Flags:   [ SHF_ALLOC ]
>     Size:    0x2
>     Offset:  0x200
>   - Name:    .zed
>     Type:    SHT_PROGBITS
>     Flags:   [ SHF_ALLOC ]
>     Size:    0x3
>     Offset:  0x300
> SectionHeaderTable:
>   Sections:
>     - Name: .zed
>     - Name: .foo
>     - Name: .bar
>     - Name: .strtab
>     - Name: .shstrtab
> ProgramHeaders:
>   - Type:     PT_LOAD
>     Flags:    [ PF_W, PF_R ]
>     VAddr:    0x1000
>     FirstSec: .foo
>     LastSec:  .zed
> 
> ```
> 
> after applying obj2yaml we have:
> 
> 
> ```
> --- !ELF
> FileHeader:
>   Class:           ELFCLASS64
>   Data:            ELFDATA2LSB
>   Type:            ET_EXEC
> ProgramHeaders:
>   - Type:            PT_LOAD
>     Flags:           [ PF_W, PF_R ]
>     FirstSec:        .zed
>     LastSec:         .bar
>     VAddr:           0x0000000000001000
> Sections:
>   - Name:            .zed
>     Type:            SHT_PROGBITS
>     Flags:           [ SHF_ALLOC ]
>     Address:         0x0000000000000003
>     Content:         '000000'
>   - Name:            .foo
>     Type:            SHT_PROGBITS
>     Flags:           [ SHF_ALLOC ]
>     Content:         '00'
>   - Name:            .bar
>     Type:            SHT_PROGBITS
>     Flags:           [ SHF_ALLOC ]
>     Address:         0x0000000000000001
>     Content:         '0000'
> ...
> ```
> 
> What is noticable:
> 1) We lost the information about the section header order. I.e. now
>     the order is not **.foo .bar. zed**, but **.zed .foo .bar**. Because we dump sections
>     by section header table order only and don't try to preserve section offsets.
> 2) `FirstSec`/`LastSec` were changed, but they match the YAML produced.
> 
My opinion is that we don't need to preserve in the YAML the section header table order, as long as the semantic meaning is identical. Section offset order likely needs maintaining if the sections are inside segments, or any hard-coded addresses will end up breaking, right? It sounds like that might be an issue already though?


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

https://reviews.llvm.org/D90458