[obj2yaml] - Support dumping objects that have multiple SHT_SYMTAB_SHNDX sections.

[grimar]

Summary

It is allowed to have multiple SHT_SYMTAB_SHNDX sections, though
we currently don't implement it.

The current implementation assumes that there is a maximum of one SHT_SYMTAB_SHNDX
section and that it is always linked with .symtab section.

This patch drops this limitations.

[jhenderson]

I actually filed a bug about this a while back: https://bugs.llvm.org/show_bug.cgi?id=43991

llvm/tools/obj2yaml/elf2yaml.cpp

318

It might be worth a comment why this is a cantFail, since it seems quite possible sh_link could be invalid.

326

Is not_supported correct here? I'm asuming the error is when the data isn't a valid getSHNDXTable which sounds more like parse_failed or illegal_argument.

[grimar]

In D92644#2436573, @jhenderson wrote:

I actually filed a bug about this a while back: https://bugs.llvm.org/show_bug.cgi?id=43991

Yeah, I know. But it is about llvm-readelf, this patch is for obj2yaml, though the issue is indeed the same.
I am investigating that bug, btw, too.

[jhenderson]

In D92644#2436576, @grimar wrote:

In D92644#2436573, @jhenderson wrote:

I actually filed a bug about this a while back: https://bugs.llvm.org/show_bug.cgi?id=43991

Yeah, I know. But it is about llvm-readelf, this patch is for obj2yaml, though the issue is indeed the same.
I am investigating that bug, btw, too.

Oops, I misread, thanks.

[grimar]

• Addressed review comments.

llvm/tools/obj2yaml/elf2yaml.cpp

326

Right. Done.

[jhenderson]

LGTM.

llvm/test/tools/obj2yaml/ELF/sht-symtab-shndx.yaml

 
 ## Check that yaml2obj is unable to dump an object, which has symbols with
 ## section index == SHN_XINDEX, but SHT_SYMTAB_SHNDX table contains more
 ## entries than the number of symbols in .symtab.
 
 # RUN: yaml2obj --docnum=4 %s -o %t4
 # RUN: not obj2yaml %t4 2>&1 | FileCheck %s -DFILE=%t4 --check-prefix=CASE4
 
-## CASE4: Error reading file: [[FILE]]: SHT_SYMTAB_SHNDX has 3 entries, but the symbol table associated has 2
+## CASE4: Error reading file: [[FILE]]: unable to read extended section indexes: SHT_SYMTAB_SHNDX has 3 entries, but the symbol table associated has 2
 
 --- !ELF
 FileHeader:
   Class: ELFCLASS64
   Data:  ELFDATA2LSB
   Type:  ET_REL
 Sections:
   - Name: bar
     Type: SHT_PROGBITS
   - Name:    .symtab_shndx
     Type:    SHT_SYMTAB_SHNDX
     Entries: [ 0, 1, 2 ]
     Link:    .symtab
 Symbols:
   - Type:  STT_SECTION
     Index: SHN_XINDEX
 
 ## ELF gABI allows having multiple SHT_SYMTAB_SHNDX sections.
-## We only support having one associated with .symtab now.
+## Only one is allowed to be linked with a corresponding symbol table section though.
+## Check we report an error when multiple SHT_SYMTAB_SHNDX sections are linked
+## to the same symbol table.
 
 # RUN: yaml2obj --docnum=5 %s -o %t5
 # RUN: not obj2yaml %t5 2>&1 | FileCheck %s -DFILE=%t5 --check-prefix=CASE5
 
-# CASE5: Error reading file: [[FILE]]: multiple SHT_SYMTAB_SHNDX sections are not supported
+# CASE5: Error reading file: [[FILE]]: multiple SHT_SYMTAB_SHNDX sections are linked to the same symbol table with index 5
 
 --- !ELF
 FileHeader:
   Class: ELFCLASS64
   Data:  ELFDATA2LSB
   Type:  ET_REL
 Sections:
   - Name:    .symtab_shndx1
     Type:    SHT_SYMTAB_SHNDX
-    Entries: [ 0 ]
+    Entries: [ 0, 1 ]
     EntSize: 4
     Link:    .symtab
   - Name:    .symtab_shndx2
     Type:    SHT_SYMTAB_SHNDX
-    Entries: [ 0 ]
-    Link:    .symtab
-Symbols: []
+    Entries: [ 0, 2 ]
+    Link:    [[LINK=.symtab]]
+Symbols:
+  - Type:  STT_SECTION
+    Index: SHN_XINDEX
+DynamicSymbols:
+  - Type:  STT_SECTION
+    Index: SHN_XINDEX
+
+## Check it is possible to dump an object that has multiple SHT_SYMTAB_SHNDX sections.
+## Check that yaml2obj can dump the object and dynamic symbols properly when
+## the SHT_SYMTAB_SHNDX section is associated with a SHT_DYNSYM section.
+
+# RUN: yaml2obj --docnum=5 -DLINK=.dynsym %s -o %t5.multiple
+# RUN: obj2yaml %t5.multiple | FileCheck %s --check-prefix=MULTIPLE-SYMTAB
+
+# MULTIPLE-SYMTAB:      - Name: .symtab_shndx1
+# MULTIPLE-SYMTAB-NEXT:   Type: SHT_SYMTAB_SHNDX
+# MULTIPLE-SYMTAB-NEXT:   Link: .symtab
+# MULTIPLE-SYMTAB:      - Name: .symtab_shndx2
+# MULTIPLE-SYMTAB-NEXT:   Type: SHT_SYMTAB_SHNDX
+# MULTIPLE-SYMTAB-NEXT:   Link: .dynsym
+# MULTIPLE-SYMTAB:      Symbols:
+# MULTIPLE-SYMTAB-NEXT:   - Name:  .symtab_shndx1
+# MULTIPLE-SYMTAB-NEXT:     Type:  STT_SECTION
+# MULTIPLE-SYMTAB-NEXT:     Index: SHN_XINDEX
+# MULTIPLE-SYMTAB-NEXT: DynamicSymbols:
+# MULTIPLE-SYMTAB-NEXT:   - Name:   .symtab_shndx2
+# MULTIPLE-SYMTAB-NEXT:     Type:   STT_SECTION
+# MULTIPLE-SYMTAB-NEXT:     Index:  SHN_XINDEX
 
 ## Check that yaml2obj can't dump the object if SHT_SYMTAB_SHNDX is
 ## not associated with a SHT_SYMTAB section (this case is illegal).
 
 # RUN: yaml2obj --docnum=6 %s -o %t6
 # RUN: not obj2yaml %t6 2>&1 | FileCheck %s -DFILE=%t6 --check-prefix=CASE6
 
-# CASE6: Error reading file: [[FILE]]: only SHT_SYMTAB_SHNDX associated with SHT_SYMTAB are supported
+# CASE6: Error reading file: [[FILE]]: unable to read extended section indexes: SHT_SYMTAB_SHNDX section is linked with SHT_PROGBITS section (expected SHT_SYMTAB/SHT_DYNSYM)
 
 --- !ELF
 FileHeader:
   Class: ELFCLASS64
   Data:  ELFDATA2LSB
   Type:  ET_REL
 Sections:
   - Name:    .symtab_shndx
     Type:    SHT_SYMTAB_SHNDX
     Entries: [ 0 ]
     Link:    .foo
   - Name:    .foo
     Type:    SHT_PROGBITS
-
-## Check that yaml2obj can't dump the object if SHT_SYMTAB_SHNDX is
-## associated with a SHT_DYNSYM section (not implemented yet).
-
-# RUN: yaml2obj --docnum=7 %s -o %t7
-# RUN: not obj2yaml %t7 2>&1 | FileCheck %s -DFILE=%t7 --check-prefix=CASE7
-
-# CASE7: Error reading file: [[FILE]]: only SHT_SYMTAB_SHNDX associated with SHT_SYMTAB are supported
-
---- !ELF
-FileHeader:
-  Class: ELFCLASS64
-  Data:  ELFDATA2LSB
-  Type:  ET_REL
-Sections:
-  - Name:    .symtab_shndx
-    Type:    SHT_SYMTAB_SHNDX
-    Entries: [ 0, 1 ]
-    Link:    .dynsym
-DynamicSymbols:
-  - Type:  STT_SECTION
-    Index: SHN_XINDEX

llvm/tools/obj2yaml/elf2yaml.cpp

class ELFDumper {
   Expected<StringRef> getUniquedSectionName(const Elf_Shdr *Sec);
   Expected<StringRef> getUniquedSymbolName(const Elf_Sym *Sym,
                                            StringRef StrTable,
                                            const Elf_Shdr *SymTab);
   Expected<StringRef> getSymbolName(uint32_t SymtabNdx, uint32_t SymbolNdx);
 
   const object::ELFFile<ELFT> &Obj;
   std::unique_ptr<DWARFContext> DWARFCtx;
-  ArrayRef<Elf_Word> ShndxTable;
+
+  DenseMap<const Elf_Shdr *, ArrayRef<Elf_Word>> ShndxTables;
 
   Expected<std::vector<ELFYAML::ProgramHeader>>
   dumpProgramHeaders(ArrayRef<std::unique_ptr<ELFYAML::Chunk>> Sections);
 
   Optional<DWARFYAML::Data>
   dumpDWARFSections(std::vector<std::unique_ptr<ELFYAML::Chunk>> &Sections);
 
   Error dumpSymbols(const Elf_Shdr *Symtab,

 Expected<StringRef>
 ELFDumper<ELFT>::getUniquedSymbolName(const Elf_Sym *Sym, StringRef StrTable,
                                       const Elf_Shdr *SymTab) {
   Expected<StringRef> SymbolNameOrErr = Sym->getName(StrTable);
   if (!SymbolNameOrErr)
     return SymbolNameOrErr;
   StringRef Name = *SymbolNameOrErr;
   if (Name.empty() && Sym->getType() == ELF::STT_SECTION) {
-    auto ShdrOrErr = Obj.getSection(*Sym, SymTab, ShndxTable);
+    auto ShdrOrErr = Obj.getSection(*Sym, SymTab, ShndxTables.lookup(SymTab));
     if (!ShdrOrErr)
       return ShdrOrErr.takeError();
     return getUniquedSectionName(*ShdrOrErr);
   }
 
   // Symbols in .symtab can have duplicate names. For example, it is a common
   // situation for local symbols in a relocatable object. Here we assign unique
   // suffixes for such symbols so that we can differentiate them.
template <class ELFT> Expected<ELFYAML::Object *> ELFDumper<ELFT>::dump() {
   // case the real number of entries is held in the sh_size member of the
   // initial entry. We have a section header table when `e_shoff` is not 0.
   if (Obj.getHeader().e_shoff != 0 && Obj.getHeader().e_shnum == 0)
     Y->Header.EShNum = 0;
 
   // Dump symbols. We need to do this early because other sections might want
   // to access the deduplicated symbol names that we also create here.
   const Elf_Shdr *SymTab = nullptr;
-  const Elf_Shdr *SymTabShndx = nullptr;
   const Elf_Shdr *DynSymTab = nullptr;
 
   for (const Elf_Shdr &Sec : Sections) {
     if (Sec.sh_type == ELF::SHT_SYMTAB) {
       SymTab = &Sec;
     } else if (Sec.sh_type == ELF::SHT_DYNSYM) {
       DynSymTab = &Sec;
     } else if (Sec.sh_type == ELF::SHT_SYMTAB_SHNDX) {
-      // ABI allows us to have one SHT_SYMTAB_SHNDX for each symbol table.
-      // We only support having the SHT_SYMTAB_SHNDX for SHT_SYMTAB now.
-      if (SymTabShndx)
+      // We need to locate SHT_SYMTAB_SHNDX sections early, because they
+      // might be needed for dumping symbols.
+      if (Expected<ArrayRef<Elf_Word>> TableOrErr = Obj.getSHNDXTable(Sec)) {
+        // The `getSHNDXTable` calls the `getSection` internally when validates
+        // the symbol table section linked to the SHT_SYMTAB_SHNDX section.
+        const Elf_Shdr *LinkedSymTab = cantFail(Obj.getSection(Sec.sh_link));
+        if (!ShndxTables.insert({LinkedSymTab, *TableOrErr}).second)
           return createStringError(
-            errc::not_supported,
-            "multiple SHT_SYMTAB_SHNDX sections are not supported");
-      SymTabShndx = &Sec;
+              errc::invalid_argument,
+              "multiple SHT_SYMTAB_SHNDX sections are "
+              "linked to the same symbol table with index " +
+                  Twine(Sec.sh_link));
+      } else {
+        return createStringError(errc::invalid_argument,
+                                 "unable to read extended section indexes: " +
+                                     toString(TableOrErr.takeError()));
       }
     }
-
-  // We need to locate the SHT_SYMTAB_SHNDX section early, because it might be
-  // needed for dumping symbols.
-  if (SymTabShndx) {
-    if (!SymTab ||
-        SymTabShndx->sh_link != (unsigned)(SymTab - Sections.begin()))
-      return createStringError(
-          errc::not_supported,
-          "only SHT_SYMTAB_SHNDX associated with SHT_SYMTAB are supported");
-
-    auto TableOrErr = Obj.getSHNDXTable(*SymTabShndx);
-    if (!TableOrErr)
-      return TableOrErr.takeError();
-    ShndxTable = *TableOrErr;
   }
 
   if (SymTab) {
     Y->Symbols.emplace();
     if (Error E = dumpSymbols(SymTab, *Y->Symbols))
       return std::move(E);
   }
 
if (!SymbolNameOrErr)
     return SymbolNameOrErr.takeError();
   S.Name = SymbolNameOrErr.get();
 
   if (Sym->st_shndx >= ELF::SHN_LORESERVE) {
     S.Index = (ELFYAML::ELF_SHN)Sym->st_shndx;
     return Error::success();
   }
 
-  auto ShdrOrErr = Obj.getSection(*Sym, SymTab, ShndxTable);
+  auto ShdrOrErr = Obj.getSection(*Sym, SymTab, ShndxTables.lookup(SymTab));
   if (!ShdrOrErr)
     return ShdrOrErr.takeError();
   const Elf_Shdr *Shdr = *ShdrOrErr;
   if (!Shdr)
     return Error::success();
 
   auto NameOrErr = getUniquedSectionName(Shdr);
   if (!NameOrErr)