[llvm-readobj/elf] - Stop reporting invalid extended indexes in warnings for unnamed section symbols.

[grimar]

Summary

We have an issue with getFullSymbolName: it assumes that the symbol passed is
always in the .symtab, what is wrong. We might calculate and report a wrong index currently.
I've added a test case revealing that.

This patch adds the "symbol index" argument to getFullSymbolName signature,
what fixes the issue.

[jhenderson]

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

365–370

Do we need all these checks for the test purpose? If so, why do we need comparatively fewer in the GNU version?

llvm/tools/llvm-readobj/ELFDumper.cpp

5846

Maybe just get the Elf_Sym here, and pass that in? I also think you should do [0] as it's more explicit than .data().

6392

For the same reason.

6779–6780

Ditto.

[grimar]

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

365–370

GNU version shows the index of symbol (Num: 2). All I wanted is to demostrate that the warning is reported for symbol with index 2.

[grimar]

• Addressed review comments.

[MaskRay]

All the passing of FirstSym is for &Sym - &FirstSym in ELF.h getExtendedSymbolTableIndex. If we only care about the index, passing around the index rather than the first symbol probably makes more sense?

[jhenderson]

In D87899#2284145, @MaskRay wrote:

All the passing of FirstSym is for &Sym - &FirstSym in ELF.h getExtendedSymbolTableIndex. If we only care about the index, passing around the index rather than the first symbol probably makes more sense?

+1 - this makes sense to me too. Everything else looks good.

[grimar]

In D87899#2284675, @jhenderson wrote:

In D87899#2284145, @MaskRay wrote:

All the passing of FirstSym is for &Sym - &FirstSym in ELF.h getExtendedSymbolTableIndex. If we only care about the index, passing around the index rather than the first symbol probably makes more sense?

+1 - this makes sense to me too. Everything else looks good.

I've prepared an independent NFCI patch for getExtendedSymbolTableIndex and other API: D88016

[grimar]

• Addressed review comment.

[jhenderson]

Looks good.

[MaskRay]

Looks great!

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

 # VERSIONED-SEC-SYM-GNU:     Symbol table '.dynsym' contains 4 entries:
 # VERSIONED-SEC-SYM-GNU:        Num: {{.*}} Ndx Name
 # VERSIONED-SEC-SYM-GNU:          1: {{.*}} UND foo
 # VERSIONED-SEC-SYM-GNU:      warning: '[[FILE]]': unable to get section index for symbol with st_shndx = 0xfff1 (SHN_ABS)
 # VERSIONED-SEC-SYM-GNU-NEXT:     2: {{.*}} ABS <?>
 # VERSIONED-SEC-SYM-GNU-NEXT: warning: '[[FILE]]': unable to get section index for symbol with st_shndx = 0x0 (SHN_UNDEF)
 # VERSIONED-SEC-SYM-GNU-NEXT:     3: {{.*}} UND <?>
 
+## Check we print a proper warning when an unnamed versioned section symbol has st_shndx = SHN_XINDEX, but there
+## is no SHT_SYMTAB_SHNDX section in the object.
+# RUN: yaml2obj %s -DTYPE=STT_SECTION -DNAME="''" -DINDEX=SHN_XINDEX --docnum=6 -o %t6.sec.xindex.sym
+# RUN: llvm-readobj -V --dyn-symbols %t6.sec.xindex.sym 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t6.sec.xindex.sym --check-prefix=VERSIONED-SEC-SYM-XINDEX-LLVM
+# RUN: llvm-readelf -V --dyn-symbols %t6.sec.xindex.sym 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t6.sec.xindex.sym --check-prefix=VERSIONED-SEC-SYM-XINDEX-GNU
+
+# VERSIONED-SEC-SYM-XINDEX-LLVM:       Name: (0)
+# VERSIONED-SEC-SYM-XINDEX-LLVM:       Name: foo (12)
+# VERSIONED-SEC-SYM-XINDEX-LLVM:      warning: '[[FILE]]': extended symbol index (2) is past the end of the SHT_SYMTAB_SHNDX section of size 0
+# VERSIONED-SEC-SYM-XINDEX-LLVM-NEXT: Symbol {
+# VERSIONED-SEC-SYM-XINDEX-LLVM-NEXT:  Name: <?> (0)
+# VERSIONED-SEC-SYM-XINDEX-LLVM:       Name: <?> (0)
+
+# VERSIONED-SEC-SYM-XINDEX-GNU:      Symbol table '.dynsym' contains 4 entries:
+# VERSIONED-SEC-SYM-XINDEX-GNU:         Num: {{.*}} Ndx Name
+# VERSIONED-SEC-SYM-XINDEX-GNU:      warning: '[[FILE]]': extended symbol index (2) is past the end of the SHT_SYMTAB_SHNDX section of size 0
+# VERSIONED-SEC-SYM-XINDEX-GNU-NEXT:      2: {{.*}} RSV[0xffff] <?>
+
 ## Case 8: Check what we print when:
 ## a) The dynamic symbol table does not exist.
 # RUN: yaml2obj %s --docnum=7 -o %t7
 # RUN: llvm-readobj --dyn-symbols %t7 | FileCheck %s --check-prefix=NO-DYNSYM-LLVM
 # RUN: llvm-readelf --dyn-symbols %t7 | count 0
 ## b) The dynamic symbol table has a size of 0.
 # RUN: yaml2obj %s --docnum=8 -o %t8
 # RUN: llvm-readobj --dyn-symbols %t8 | FileCheck %s --check-prefix=NO-DYNSYM-LLVM

llvm/tools/llvm-readobj/ELFDumper.cpp

Elf_Sym_Range dynamic_symbols() const {
     if (!DynSymRegion)
       return Elf_Sym_Range();
     return DynSymRegion->getAsArrayRef<Elf_Sym>();
   }
 
   Elf_Rel_Range dyn_rels() const;
   Elf_Rela_Range dyn_relas() const;
   Elf_Relr_Range dyn_relrs() const;
-  std::string getFullSymbolName(const Elf_Sym *Symbol,
+  std::string getFullSymbolName(const Elf_Sym *Symbol, const Elf_Sym *FirstSym,
                                 Optional<StringRef> StrTable,
                                 bool IsDynamic) const;
   Expected<unsigned> getSymbolSectionIndex(const Elf_Sym *Symbol,
                                            const Elf_Sym *FirstSym) const;
   Expected<StringRef> getSymbolSectionName(const Elf_Sym *Symbol,
                                            unsigned SectionIndex) const;
   std::string getStaticSymbolName(uint32_t Index) const;
   StringRef getDynamicString(uint64_t Value) const;
if (!NameOrErr)
       return NameOrErr.takeError();
     return RelSymbol<ELFT>(Sym, NameOrErr->str());
   }
 
   Expected<StringRef> StrTableOrErr = Obj.getStringTableForSymtab(*SymTab);
   if (!StrTableOrErr)
     return StrTableOrErr.takeError();
 
-  std::string SymbolName =
-      getFullSymbolName(Sym, *StrTableOrErr, SymTab->sh_type == SHT_DYNSYM);
+  const Elf_Sym *FirstSym =
+      cantFail(Obj.template getEntry<Elf_Sym>(*SymTab, 0));
+  std::string SymbolName = getFullSymbolName(Sym, FirstSym, *StrTableOrErr,
+                                             SymTab->sh_type == SHT_DYNSYM);
   return RelSymbol<ELFT>(Sym, SymbolName);
 }
 
 static std::string maybeDemangle(StringRef Name) {
   return opts::Demangle ? demangle(std::string(Name)) : Name.str();
 }
 
 template <typename ELFT>
if (Entry.IsVerDef)
     IsDefault = !(SymbolVersionIndex & VERSYM_HIDDEN);
   else
     IsDefault = false;
   return Entry.Name.c_str();
 }
 
 template <typename ELFT>
 std::string ELFDumper<ELFT>::getFullSymbolName(const Elf_Sym *Symbol,
+                                               const Elf_Sym *FirstSym,
                                                Optional<StringRef> StrTable,
                                                bool IsDynamic) const {
   if (!StrTable)
     return "<?>";
 
   std::string SymbolName;
   if (Expected<StringRef> NameOrErr = Symbol->getName(*StrTable)) {
     SymbolName = maybeDemangle(*NameOrErr);
   } else {
     reportUniqueWarning(NameOrErr.takeError());
     return "<?>";
   }
 
   if (SymbolName.empty() && Symbol->getType() == ELF::STT_SECTION) {
-    Elf_Sym_Range Syms = unwrapOrError(
-        ObjF->getFileName(), ObjF->getELFFile()->symbols(DotSymtabSec));
-    Expected<unsigned> SectionIndex =
-        getSymbolSectionIndex(Symbol, Syms.begin());
+    Expected<unsigned> SectionIndex = getSymbolSectionIndex(Symbol, FirstSym);
     if (!SectionIndex) {
       reportUniqueWarning(SectionIndex.takeError());
       return "<?>";
     }
     Expected<StringRef> NameOrErr = getSymbolSectionName(Symbol, *SectionIndex);
     if (!NameOrErr) {
       reportUniqueWarning(NameOrErr.takeError());
       return ("<section " + Twine(*SectionIndex) + ">").str();
public:
   Entries getLocalEntries() const;
   Entries getGlobalEntries() const;
   Entries getOtherEntries() const;
   Entries getPltEntries() const;
 
   uint64_t getGotAddress(const Entry * E) const;
   int64_t getGotOffset(const Entry * E) const;
   const Elf_Sym *getGotSym(const Entry *E) const;
+  Elf_Sym_Range getGotDynSyms() const { return GotDynSyms; }
 
   uint64_t getPltAddress(const Entry * E) const;
   const Elf_Sym *getPltSym(const Entry *E) const;
 
   StringRef getPltStrTable() const { return PltStrTable; }
+  const Elf_Shdr *getPltSymTable() const { return PltSymTable; }
 
 private:
   const Elf_Shdr *GotSec;
   size_t LocalNum;
   size_t GlobalNum;
 
   const Elf_Shdr *PltSec;
   const Elf_Shdr *PltRelSec;
void GNUStyle<ELFT>::printSymbol(const Elf_Sym *Symbol, const Elf_Sym *FirstSym,
   if (Symbol->st_other & ~0x3)
     Fields[5].Str +=
         " [<other: " + to_string(format_hex(Symbol->st_other, 2)) + ">]";
 
   Fields[6].Column += NonVisibilityBitsUsed ? 13 : 0;
   Fields[6].Str = getSymbolSectionNdx(Symbol, FirstSym);
 
   Fields[7].Str =
-      this->dumper()->getFullSymbolName(Symbol, StrTable, IsDynamic);
+      this->dumper()->getFullSymbolName(Symbol, FirstSym, StrTable, IsDynamic);
   for (auto &Entry : Fields)
     printField(Entry);
   OS << "\n";
 }
 
 template <class ELFT>
 void GNUStyle<ELFT>::printHashedSymbol(const Elf_Sym *FirstSym, uint32_t Sym,
                                        StringRef StrTable, uint32_t Bucket) {
void GNUStyle<ELFT>::printHashedSymbol(const Elf_Sym *FirstSym, uint32_t Sym,
   else
     Fields[4].Str = printEnum(SymbolType, makeArrayRef(ElfSymbolTypes));
 
   Fields[5].Str =
       printEnum(Symbol->getBinding(), makeArrayRef(ElfSymbolBindings));
   Fields[6].Str =
       printEnum(Symbol->getVisibility(), makeArrayRef(ElfSymbolVisibilities));
   Fields[7].Str = getSymbolSectionNdx(Symbol, FirstSym);
-  Fields[8].Str = this->dumper()->getFullSymbolName(Symbol, StrTable, true);
+  Fields[8].Str =
+      this->dumper()->getFullSymbolName(Symbol, FirstSym, StrTable, true);
 
   for (auto &Entry : Fields)
     printField(Entry);
   OS << "\n";
 }
 
 template <class ELFT>
 void GNUStyle<ELFT>::printSymbols(bool PrintSymbols, bool PrintDynamicSymbols) {
if (!Parser.getGlobalEntries().empty()) {
     OS << " Global entries:\n";
     if (ELFT::Is64Bits)
       OS << "           Address     Access          Initial         Sym.Val."
          << " Type    Ndx Name\n";
     else
       OS << "   Address     Access  Initial Sym.Val. Type    Ndx Name\n";
     for (auto &E : Parser.getGlobalEntries()) {
       const Elf_Sym *Sym = Parser.getGotSym(&E);
+      const Elf_Sym *FirstSym = &this->dumper()->dynamic_symbols()[0];
       std::string SymName = this->dumper()->getFullSymbolName(
-          Sym, this->dumper()->getDynamicStringTable(), false);
+          Sym, FirstSym, this->dumper()->getDynamicStringTable(), false);
 
       OS.PadToColumn(2);
       OS << to_string(format_hex_no_prefix(Parser.getGotAddress(&E), 8 + Bias));
       OS.PadToColumn(11 + Bias);
       OS << to_string(format_decimal(Parser.getGotOffset(&E), 6)) + "(gp)";
       OS.PadToColumn(22 + Bias);
       OS << to_string(format_hex_no_prefix(E, 8 + Bias));
       OS.PadToColumn(31 + 2 * Bias);
if (Parser.getPltModulePointer())
     PrintEntry(Parser.getPltModulePointer(), "Module pointer");
 
   if (!Parser.getPltEntries().empty()) {
     OS << "\n";
     OS << " Entries:\n";
     OS << "   Address  Initial Sym.Val. Type    Ndx Name\n";
     for (auto &E : Parser.getPltEntries()) {
       const Elf_Sym *Sym = Parser.getPltSym(&E);
+      const Elf_Sym *FirstSym =
+          cantFail(this->Obj.template getEntry<const Elf_Sym>(
+              *Parser.getPltSymTable(), 0));
       std::string SymName = this->dumper()->getFullSymbolName(
-          Sym, this->dumper()->getDynamicStringTable(), false);
+          Sym, FirstSym, this->dumper()->getDynamicStringTable(), false);
 
       OS.PadToColumn(2);
       OS << to_string(format_hex_no_prefix(Parser.getPltAddress(&E), 8 + Bias));
       OS.PadToColumn(11 + Bias);
       OS << to_string(format_hex_no_prefix(E, 8 + Bias));
       OS.PadToColumn(20 + 2 * Bias);
       OS << to_string(format_hex_no_prefix(Sym->st_value, 8 + Bias));
       OS.PadToColumn(29 + 3 * Bias);
void LLVMStyle<ELFT>::printSymbolSection(const Elf_Sym *Symbol,
   }
 }
 
 template <class ELFT>
 void LLVMStyle<ELFT>::printSymbol(const Elf_Sym *Symbol, const Elf_Sym *First,
                                   Optional<StringRef> StrTable, bool IsDynamic,
                                   bool /*NonVisibilityBitsUsed*/) {
   std::string FullSymbolName =
-      this->dumper()->getFullSymbolName(Symbol, StrTable, IsDynamic);
+      this->dumper()->getFullSymbolName(Symbol, First, StrTable, IsDynamic);
   unsigned char SymbolType = Symbol->getType();
 
   DictScope D(W, "Symbol");
   W.printNumber("Name", FullSymbolName, Symbol->st_name);
   W.printHex("Value", Symbol->st_value);
   W.printNumber("Size", Symbol->st_size);
   W.printEnum("Binding", Symbol->getBinding(), makeArrayRef(ElfSymbolBindings));
   if (this->Obj.getHeader().e_machine == ELF::EM_AMDGPU &&
void LLVMStyle<ELFT>::printVersionSymbolSection(const Elf_Shdr *Sec) {
   }
 
   if (StrTable.empty() || Syms.empty() || Syms.size() != VerTableOrErr->size())
     return;
 
   for (size_t I = 0, E = Syms.size(); I < E; ++I) {
     DictScope S(W, "Symbol");
     W.printNumber("Version", (*VerTableOrErr)[I].vs_index & VERSYM_VERSION);
-    W.printString("Name", this->dumper()->getFullSymbolName(&Syms[I], StrTable,
-                                                           /*IsDynamic=*/true));
+    W.printString(
+        "Name", this->dumper()->getFullSymbolName(&Syms[I], &Syms[0], StrTable,
   }
 }
 
 static const EnumEntry<unsigned> SymVersionFlags[] = {
     {"Base", "BASE", VER_FLG_BASE},
     {"Weak", "WEAK", VER_FLG_WEAK},
     {"Info", "INFO", VER_FLG_INFO}};
 
for (auto &E : Parser.getGlobalEntries()) {
       PrintEntry(&E);
 
       const Elf_Sym *Sym = Parser.getGotSym(&E);
       W.printHex("Value", Sym->st_value);
       W.printEnum("Type", Sym->getType(), makeArrayRef(ElfSymbolTypes));
       printSymbolSection(Sym, this->dumper()->dynamic_symbols().begin());
 
       std::string SymName = this->dumper()->getFullSymbolName(
-          Sym, this->dumper()->getDynamicStringTable(), true);
+          Sym, &Parser.getGotDynSyms()[0],
+          this->dumper()->getDynamicStringTable(), true);
       W.printNumber("Name", SymName, Sym->st_name);
     }
   }
 
   W.printNumber("Number of TLS and multi-GOT entries",
                 uint64_t(Parser.getOtherEntries().size()));
 }
 
for (auto &E : Parser.getPltEntries()) {
       DictScope D(W, "Entry");
       PrintEntry(&E);
 
       const Elf_Sym *Sym = Parser.getPltSym(&E);
       W.printHex("Value", Sym->st_value);
       W.printEnum("Type", Sym->getType(), makeArrayRef(ElfSymbolTypes));
       printSymbolSection(Sym, this->dumper()->dynamic_symbols().begin());
 
-      std::string SymName =
-          this->dumper()->getFullSymbolName(Sym, Parser.getPltStrTable(), true);
+      const Elf_Sym *FirstSym =
+          cantFail(this->Obj.template getEntry<const Elf_Sym>(
+              *Parser.getPltSymTable(), 0));
+      std::string SymName = this->dumper()->getFullSymbolName(
+          Sym, FirstSym, Parser.getPltStrTable(), true);
       W.printNumber("Name", SymName, Sym->st_name);
     }
   }
 }
 
 template <class ELFT>
 void LLVMStyle<ELFT>::printMipsABIFlags(const ELFObjectFile<ELFT> *ObjF) {
   const Elf_Mips_ABIFlags<ELFT> *Flags;