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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
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 =
----------------
grimar wrote:
> 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?
Reordered.

```
  Elf_Ehdr Header;
  uint64_t SHOff;
  (void)CBA.getOSAndAlignedOffset(SHOff, sizeof(typename ELFT::uint));
  State.initELFHeader(Header, SHOff);
  OS.write((const char *)&Header, sizeof(Header));
```

is still needed.


================
Comment at: lib/ObjectYAML/ELFEmitter.cpp:1030
 int ELFState<ELFT>::writeELF(raw_ostream &OS, ELFYAML::Object &Doc) {
   ELFState<ELFT> State(Doc);
 
----------------
grimar wrote:
> typedef typename ELFT::uint uintX_t; ?
Moved one use of `typename ELFT::uint` into initELFHeader, so this function just uses `typename ELFT::uint` once. The typedef will be reundant, then.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67221