[PATCH] D49371: [ELF] - Do not produce broken output when amount of sections is > ~65k

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

This looks correct to me. I've got a few small suggestions.



================
Comment at: ELF/Writer.cpp:2267
+  //
+  // When ELF object has SHN_LORESERVE or more of sections, it's header's
+  // e_shnum field should be zero and sh_size of the section header at index 0
----------------
I suggest combining the two comments for SHN_LORESERVE as they are closely related. For example:


```
The ELF header can only store numbers up to SHN_LORESERVE in the e_shnum and e_shstrndx fields. When the value of one of these fields exceeds SHN_LORESERVE ELF requires us to put sentinel values in
the ELF header and use fields in the section header at index 0 to store the value. The sentinel values and fields are:
e_shnum = 0, SHdrs[0].sh_size = number of sections.
e_shstrndx = SHN_XINDEX, SHdrs[0].sh_link = strtab section index.
```



================
Comment at: ELF/Writer.cpp:2271
   auto *SHdrs = reinterpret_cast<Elf_Shdr *>(Buf + EHdr->e_shoff);
+  if (OutputSections.size() + 1 >= SHN_LORESERVE)
+    SHdrs->sh_size = OutputSections.size() + 1;
----------------
Is it worth something like NumElfSections = OutputSections.size() + 1; as we use it 3 times in a row, and it makes the +1 for the 0th Section Header a bit more obvious.


================
Comment at: ELF/Writer.cpp:2272
+  if (OutputSections.size() + 1 >= SHN_LORESERVE)
+    SHdrs->sh_size = OutputSections.size() + 1;
+  else
----------------
Should we explicitly set EHdr->e_shnum to 0 here as we've called it out in the comment. 


https://reviews.llvm.org/D49371