[PATCH] D69093: [llvm-objcopy] --add-symbol: fix crash if SHT_SYMTAB does not exist

Fri Oct 18 09:06:33 PDT 2019

abrachet marked an inline comment as done.
abrachet added inline comments.

================
Comment at: llvm/tools/llvm-objcopy/ELF/Object.cpp:1550-1552
+        // Prefer .strtab to .shstrtab.
+        if (Obj.SectionNames != &Sec)
+          break;
----------------
rupprecht wrote:
> MaskRay wrote:
> > rupprecht wrote:
> > > Should `.dynstr` also be avoided? Is it better to just check that the name is literally `.strtab`?
> > .dynstr is avoided by `!(Sec.Flags & SHF_ALLOC)`. `.strtab` works as well but `Obj.SectionNames != &Sec` is more general. There are multiple ways to accomplish the samething. I just wanted to make the code less magical.
> From: http://www.sco.com/developers/gabi/latest/ch4.sheader.html#special_sections
> > .strtab
> > This section holds strings, most commonly the strings that represent the names associated with symbol table entries. If the file has a loadable segment that includes the symbol string table, the section's attributes will include the SHF_ALLOC bit; otherwise, that bit will be off.
> 
> So, `.strtab` can also be `SHF_ALLOC`.
For reference, we used to have a file in test/tools/llvm-objcopy/ELF/Inputs/ called alloc-symtab which had an SHF_ALLOC .strtab. It was removed in D65278. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69093