[PATCH] D67221: [yaml2obj] Write the section header table after section contents

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

Generally the idea is OK I think.



================
Comment at: lib/ObjectYAML/ELFEmitter.cpp:233
   // Immediately following the ELF header and program headers.
-  Header.e_shoff =
-      Doc.Header.SHOffset
-          ? (typename ELFT::uint)(*Doc.Header.SHOffset)
-          : sizeof(Header) + sizeof(Elf_Phdr) * Doc.ProgramHeaders.size();
+  // e_shoff will be filled in in writeELF.
   Header.e_shnum =
----------------
Seems you can just call `initELFHeader` later instead?
I think placing it right before `OS.write((const char *)&Header, sizeof(Header));`  in ` ELFState<ELFT>::writeELF` should work.

It needs rewriting the assignment:
```
  const size_t SectionContentBeginOffset = Header.e_ehsize +
                                           Header.e_phentsize * Header.e_phnum +
                                           Header.e_shentsize * Header.e_shnum;
```

to something like

```
  const size_t SectionContentBeginOffset = sizeof(Elf_Ehdr) +
                                           sizeof(Elf_Phdr) * Doc.ProgramHeaders.size() + ... ;
```

but seems there are no more problems?


================
Comment at: lib/ObjectYAML/ELFEmitter.cpp:1030
 int ELFState<ELFT>::writeELF(raw_ostream &OS, ELFYAML::Object &Doc) {
   ELFState<ELFT> State(Doc);
 
----------------
typedef typename ELFT::uint uintX_t; ?


================
Comment at: lib/ObjectYAML/ELFEmitter.cpp:1063
+  uint64_t Offset;
+  (void)CBA.getOSAndAlignedOffset(Offset, sizeof(typename ELFT::uint));
+  Header.e_shoff =
----------------
You don't need `(void)` I think. We already have one more place where the result isn't used.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67221