[PATCH] D37393: [llvm-objcopy] Add support for special section indexes in symbol table greater than SHN_LORESERVE

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In addition to the already-added SHN_ABS test, could we please have a test for at least SHN_COMMON, and possibly the hexagon values as well.



================
Comment at: tools/llvm-objcopy/Object.h:119
   SectionBase *DefinedIn;
+  uint32_t SectionIndex;
   uint32_t Index;
----------------
mcgrathr wrote:
> Since this is currently not actually the section index per se, I'd prefer not to call it that.
> It's the st_shndx value, but it's not an index if it's a reserved value.
> In fact, arguably it should be ElfNN_Section type, i.e. uint16_t.
> And have a comment about the meaning.
> When you actually support 32-bit section indices, you can have a field called SectionIndex that's 32 bits.
> (You'll still need to separately represent when st_shndx is a reserved value, so as to distinguish it from when st_shndx is SHN_XINDEX and the 32-bit SHT_SYMTAB_SHNDX entry happens to be in the [SHN_LORESERVE,SHN_HIRESERVE] range.)
As the Symbol has a reference to the corresponding section via "DefinedIn", I don't think we need to have separate fields for a section index and the st_shndx value. Indeed, with exception of the special indexes, we don't need the section index here at all. Therefore, I think it might make more sense to have a little enum that can be converted from and to, which only supports the special values we support (plus a "normal symbol" value). It would then be the responsibility of finalize and the caller of addSymbol to map between the two values.


Repository:
  rL LLVM

https://reviews.llvm.org/D37393