[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

279

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

279

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

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

10

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

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 ↗

(On Diff #187791)

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 ↗

(On Diff #187791)

Thanks :)

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

+# RUN: yaml2obj %s > %t
+# RUN: obj2yaml %t | FileCheck %s
+
+# CHECK:      Symbols:
+# CHECK-NEXT:   Global:
+# CHECK-NEXT:     - Name:    absolute1
+# CHECK-NEXT:       Index:   SHN_ABS
+# CHECK-NEXT:       Value:   0x0000000000001234
+# CHECK-NEXT:     - Name:    absolute2
+# CHECK-NEXT:       Index:   SHN_ABS
+# CHECK-NEXT:       Value:   0x0000000000004321
+# CHECK-NEXT:     - Name:    common1
+# CHECK-NEXT:       Index:   SHN_COMMON
+# CHECK-NEXT:     - Name:    common2
+# CHECK-NEXT:       Index:   SHN_COMMON
+# CHECK-NEXT:     - Name:    valid_index
+# CHECK-NEXT:       Section: .text
+# CHECK-NEXT:     - Name:    processor_specific_index1
+# CHECK-NEXT:       Index:   SHN_HEXAGON_SCOMMON_1
+# CHECK-NEXT:     - Name:    processor_specific_index2
+# CHECK-NEXT:       Index:   SHN_HEXAGON_SCOMMON_1
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_HEXAGON
+Sections:
+  - Name: .text
+    Type: SHT_PROGBITS
+Symbols:
+  Global:
+    - Name:     absolute1
+      Index:    SHN_ABS
+      Value:    0x1234
+    - Name:     absolute2
+      Index:    0xfff1
+      Value:    0x4321
+    - Name:     common1
+      Index:    SHN_COMMON
+    - Name:     common2
+      Index:    0xfff2
+    - Name:     valid_index
+      Index:    0x1
+    - Name:     processor_specific_index1
+      Index:    0xff01
+    - Name:     processor_specific_index2
+      Index:    SHN_HEXAGON_SCOMMON_1
+
+## 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);