[PATCH] D58498: [obj2yaml] - Do not miss section index for SHN_ABS and SHN_COMMON symbols.

Thu Feb 21 05:37:41 PST 2019

grimar added inline comments.

================
Comment at: tools/obj2yaml/elf2yaml.cpp:279

+  if (Sym->st_shndx == ELF::SHN_ABS || Sym->st_shndx == ELF::SHN_COMMON) {
+    S.Index = (ELFYAML::ELF_SHN)Sym->st_shndx;
----------------
jhenderson wrote:
> I'm thinking that we should probably actually preserve all values >= SHN_LORESERVE, since these aren't normal section indexes. What do you think?
> 
> SHN_XINDEX is possibly a special case though. Not sure about that one, but it's possible it just works.
Yes, I think you're right. I used 0xff01 value which maps to SHN_HEXAGON_SCOMMON_1 atm.
We actually map/print this value for EM_X86_64 yamls too, and it does not seem right to me,
I would expect to see just a hex value in output for that case.
So I switched the test to EM_HEXAGON, that allows to test it and makes the test case to be correct.

About SHN_XINDEX: it is a bit more complicated.
We do not support them atm:
https://github.com/llvm-mirror/llvm/blob/master/lib/ObjectYAML/ELFYAML.cpp#L833
So yaml2obj reports "Large indexes are not supported".

I created a little binary using debugger magic to test it. Btw, --no-validate would help here, so I would perhaps
do a patch for it after landing this one. It should allow getting rid of this binary as proof of usefulness.

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

https://reviews.llvm.org/D58498