[PATCH] D37391: [yaml2obj][ELF] Add support for symbol indexes greater than SHN_LORESERVE

[Roland McGrath via Phabricator via llvm-commits]

mcgrathr added inline comments.


================
Comment at: lib/ObjectYAML/ELFYAML.cpp:491
+  ECase(SHN_COMMON);
+  ECase(SHN_XINDEX);
+  ECase(SHN_HIRESERVE);
----------------
jakehehrlich wrote:
> mcgrathr wrote:
> > SHN_XINDEX is a special animal unlike other special cases.  It means the real shndx value is elsewhere.  See SHN_XINDEX handling elsewhere in the code base (or the ELF spec).
> > 
> > SHN_UNDEF, SHN_ABS, and SHN_COMMON are standard special cases.  Those just go through as they are.
> > (Note that SHN_UNDEF is not in the [SHN_LORESERVE,SHN_HIRESERVE] range like all the others here are.)
> > 
> > SHN_{LO,HI}* are not actual values, but markers for the ranges used for machine-specific and OS-specific values.
> > It doesn't make sense to match them.
> > 
> > I think you need to handle each such value specially, because the correct way to treat it will depend on the specific machine/OS-specific definition.
> > All the ones I've seen are basically variants of SHN_COMMON and are handled the same way (just pass it through).  But I think the only safe thing is to go case by case for handling specific ones we understand, and error out if you encounter one we don't know how to handle.
> This is just code that translates strings of t he form " SHN_* " to the appropriate value for use anywhere in yaml2elf. It doesn't handle what happens in these cases any further than that. I think there's an argument that SHN_{LO,HI}* shouldn't be here since you would never want to use those names when building an ELF object. The rest of the file however includes such values however (see SHT_LOOS and SHT_LOPROC in the equivliant code for section types) so I followed suit. 
> 
> In general ELF_SHN should be the type of values with names SHN_* in this code base; even the ones which are not logically values you would want to use in a yaml file to produce an ELF.
Understood.  I think you still need to handle SHN_XINDEX specially though, because it has a structural meaning.
It probably makes more sense for yaml files to list large literal section numbers and have yaml2obj do the SHN_XINDEX encoding for them automagically.  But maybe yaml2elf is meant to test that encoding logic too, in which case you'd have to spell out SHN_XINDEX in symtab entries and then spell out the SHT_SYMTAB_SHNDX section with its contents explicitly.

You should certainly handle all the reserved-range SHN_* names in include/llvm/BinaryFormat/ELF.h (there are more that binutils groks).


Repository:
  rL LLVM

https://reviews.llvm.org/D37391