[PATCH] D61767: [llvm-elfabi] Emit ELF header and string table section

[Jake Ehrlich via Phabricator via llvm-commits]

jakehehrlich added inline comments.


================
Comment at: llvm/tools/llvm-elfabi/ELFObjHandler.cpp:474
+
+class ELFStringTableBuilder : public StringTableBuilder {
+public:
----------------
amontanez wrote:
> I'm assuming this just exists to make templating easier. If so, add a quick one-line comment to mention that's why it exists.
Not exactly. I added a comment.


================
Comment at: llvm/tools/llvm-elfabi/ELFObjHandler.cpp:584
+  size_t GetSize() const {
+    return ElfHeader->e_shoff + ElfHeader->e_shnum * sizeof(Elf_Shdr);
+  }
----------------
amontanez wrote:
> This calculation doesn't look like it includes the string table.
That's not the right way to think about it. Our layout is the following

ELF header
[ Program Headers ]
[ Section Contents ]
[ Section Headers ]

ElfHeader->e_shoff always points to wherever the section headers were laid out and ElfHeader->e_shnum tells us the total size.

Now that's all we have to think about and we don't and shouldn't think about anything else. To answer your question however it transitively depends on the size of the string table because the string table is a part of the Section Content fragment of our file and ElfHeader->e_shoff depends on the layout of that. The layout of that depends on the size of the string table which depends on the content of the string table.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61767