[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 ↗

(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 ↗

(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/yaml2obj/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
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+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

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_ABS || Sym->st_shndx == ELF::SHN_COMMON) {
+    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);