[obj2yaml] - Do not miss section index for special symbols.

[grimar]

Summary

This fixes https://bugs.llvm.org/show_bug.cgi?id=40786
("obj2yaml symbol output missing section index for SHN_ABS and SHN_COMMON symbols")

Since SHN_ABS and SHN_COMMON symbols are special, we should preserve
the st_shndx for them. The patch does this for them and the other special symbols.

The test case is based on the test provided by James Henderson at the bug page!

[jhenderson]

tools/obj2yaml/elf2yaml.cpp

287

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.

[grimar]

• Addressed review comment.

tools/obj2yaml/elf2yaml.cpp

287

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.

[jhenderson]

test/tools/obj2yaml/abs-common-symbols.yaml

1 ↗

(On Diff #187773)

The test needs renaming to show its more general purpose now. Also maybe worth a comment to describe the test ;-)

10 ↗

(On Diff #187773)

The point of the original test was to show that both SHN_ABS and 0xfff1 can be handled. I don't think it matters too much whether both are tested or not, but there's not much point in having duplicates.

21 ↗

(On Diff #187773)

How about testing another arbitrary value in the reserved range that doesn't necessarily correspond to any known value?

[grimar]

• Addressed review comments.

[jhenderson]

test/tools/obj2yaml/special-symbols-indices.yaml

1

Nit: test name should be special-symbol-indices.test (you do not need to make "symbol" plural).

[jhenderson]

LGTM.

[grimar]

test/tools/obj2yaml/special-symbols-indices.yaml

1

Thanks :)

test/tools/obj2yaml/special-symbols-indices.yaml

+# RUN: yaml2obj %s > %t
+# RUN: obj2yaml %t | FileCheck %s
+
+## Test checks that we are able to handle symbols with special/reserved indices.
+
+# CHECK:      Symbols:
+# CHECK-NEXT:   Global:
+# CHECK-NEXT:     - Name:    absolute
+# CHECK-NEXT:       Index:   SHN_ABS
+# CHECK-NEXT:       Value:   0x0000000000001234
+# CHECK-NEXT:     - Name:    common
+# CHECK-NEXT:       Index:   SHN_COMMON
+# CHECK-NEXT:     - Name:    valid_index
+# CHECK-NEXT:       Section: .text
+# CHECK-NEXT:     - Name:    processor_specific_index
+# CHECK-NEXT:       Index:   SHN_HEXAGON_SCOMMON_1
+# CHECK-NEXT:     - Name:    unknown_index
+# CHECK-NEXT:       Index:   0x0000FFFE
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_HEXAGON
+Sections:
+  - Name: .text
+    Type: SHT_PROGBITS
+Symbols:
+  Global:
+    - Name:     absolute
+      Index:    SHN_ABS
+      Value:    0x1234
+    - Name:     common
+      Index:    SHN_COMMON
+    - Name:     valid_index
+      Index:    0x1
+    - Name:     processor_specific_index
+      Index:    SHN_HEXAGON_SCOMMON_1
+    - Name:     unknown_index
+      Index:    0xfffe
+
+## shn_xindex.o contains a symbol with st_shndx == SHN_XINDEX.
+## We do not support it at this moment.
+# RUN: not obj2yaml %S/Inputs/shn_xindex.o 2>&1 | FileCheck %s --check-prefix=ERR
+# ERR: Error reading file: {{.*}}shn_xindex.o: Feature not yet implemented.

tools/obj2yaml/elf2yaml.cpp

ELFDumper<ELFT>::dumpSymbol(const Elf_Sym *Sym, const Elf_Shdr *SymTab,
   S.Size = Sym->st_size;
   S.Other = Sym->st_other;
 
   Expected<StringRef> SymbolNameOrErr = getSymbolName(Sym, StrTable, SymTab);
   if (!SymbolNameOrErr)
     return errorToErrorCode(SymbolNameOrErr.takeError());
   S.Name = SymbolNameOrErr.get();
 
+  if (Sym->st_shndx >= ELF::SHN_LORESERVE) {
+    if (Sym->st_shndx == ELF::SHN_XINDEX)
+      return obj2yaml_error::not_implemented;
+    S.Index = (ELFYAML::ELF_SHN)Sym->st_shndx;
+    return obj2yaml_error::success;
+  }
+
   auto ShdrOrErr = Obj.getSection(Sym, SymTab, ShndxTable);
   if (!ShdrOrErr)
     return errorToErrorCode(ShdrOrErr.takeError());
   const Elf_Shdr *Shdr = *ShdrOrErr;
   if (!Shdr)
     return obj2yaml_error::success;
 
   auto NameOrErr = getUniquedSectionName(Shdr);