From dac5ddb482361cde11ac43e94c43acc94a3b78aa Mon Sep 17 00:00:00 2001 From: Georgii Rymar Date: Thu, 5 Dec 2019 17:59:34 +0300 Subject: [PATCH] [llvm-readelf/llvm-readobj] - Improved the error reporting in a few method related to versioning. I was investigating a change previously discussed that eliminates an excessive empty lines from the output when we report warnings and errors (https://reviews.llvm.org/D70826#inline-639055) and found that we need this refactoring or alike to achieve that. The problem is that some of our functions that finds symbol versions just fail instead of returning errors or printing warnings. Another problem is that they might print a warning on the same line with the regular output. In this patch I've splitted getting of the version information and dumping of it for GNU printVersionSymbolSection(). I had to change a few methods to return Error or Expected<> to do that properly. Differential revision: https://reviews.llvm.org/D71118 --- llvm/test/Object/invalid.test | 4 +- .../tools/llvm-readobj/ELF/verdef-invalid.test | 15 +-- .../tools/llvm-readobj/ELF/verneed-invalid.test | 27 ++++-- .../tools/llvm-readobj/ELF/versym-invalid.test | 44 +++++++++ llvm/tools/llvm-readobj/ELFDumper.cpp | 102 ++++++++++++--------- 5 files changed, 135 insertions(+), 57 deletions(-) diff --git a/llvm/test/Object/invalid.test b/llvm/test/Object/invalid.test index 91a93b0..b47ba6f 100644 --- a/llvm/test/Object/invalid.test +++ b/llvm/test/Object/invalid.test @@ -413,9 +413,9 @@ Symbols: ## Check llvm-readobj reports it. # RUN: yaml2obj %s --docnum=20 -o %t20 -# RUN: not llvm-readobj -dt %t20 2>&1 | FileCheck -DFILE=%t20 --check-prefix=INVALID-VERSION %s +# RUN: llvm-readobj -dt %t20 2>&1 | FileCheck -DFILE=%t20 --check-prefix=INVALID-VERSION %s -# INVALID-VERSION: error: '[[FILE]]': Invalid version entry +# INVALID-VERSION: warning: '[[FILE]]': SHT_GNU_versym section refers to a version index 255 which is missing --- !ELF FileHeader: diff --git a/llvm/test/tools/llvm-readobj/ELF/verdef-invalid.test b/llvm/test/tools/llvm-readobj/ELF/verdef-invalid.test index f80e938..e22ed6e 100644 --- a/llvm/test/tools/llvm-readobj/ELF/verdef-invalid.test +++ b/llvm/test/tools/llvm-readobj/ELF/verdef-invalid.test @@ -263,11 +263,12 @@ Sections: DynamicSymbols: - Name: foo -## Check we error out when trying to print version symbols, but SHT_GNU_verdef is invalid due to any reason. +## Check we report a warning when trying to print version symbols, but SHT_GNU_verdef +## is invalid due to any reason. # RUN: yaml2obj %s --docnum=10 -o %t10 -# RUN: not llvm-readobj -V %t10 2>&1 | FileCheck %s --check-prefix=INVALID-VERDEF-LLVM -DFILE=%t10 -# RUN: not llvm-readelf -V %t10 2>&1 | FileCheck %s --check-prefix=INVALID-VERDEF-GNU -DFILE=%t10 +# RUN: llvm-readobj -V %t10 2>&1 | FileCheck %s --check-prefix=INVALID-VERDEF-LLVM -DFILE=%t10 +# RUN: llvm-readelf -V %t10 2>&1 | FileCheck %s --check-prefix=INVALID-VERDEF-GNU -DFILE=%t10 # INVALID-VERDEF-LLVM: VersionSymbols [ # INVALID-VERDEF-LLVM-NEXT: Symbol { @@ -277,12 +278,14 @@ DynamicSymbols: # INVALID-VERDEF-LLVM-NEXT: Symbol { # INVALID-VERDEF-LLVM-NEXT: Version: 2 # INVALID-VERDEF-LLVM-EMPTY: -# INVALID-VERDEF-LLVM-NEXT: error: '[[FILE]]': invalid SHT_GNU_verdef section with index 2: version definition 1 goes past the end of the section +# INVALID-VERDEF-LLVM-NEXT: warning: '[[FILE]]': invalid SHT_GNU_verdef section with index 2: version definition 1 goes past the end of the section +# INVALID-VERDEF-LLVM-NEXT: Name: foo@ # INVALID-VERDEF-GNU: Version symbols section '.gnu.version' contains 2 entries: # INVALID-VERDEF-GNU-NEXT: Addr: 0000000000000000 Offset: 0x000040 Link: 5 (.dynsym) -# INVALID-VERDEF-GNU-NEXT: 000: 0 (*local*) -# INVALID-VERDEF-GNU-NEXT: error: '[[FILE]]': invalid SHT_GNU_verdef section with index 2: version definition 1 goes past the end of the section +# INVALID-VERDEF-GNU-EMPTY: +# INVALID-VERDEF-GNU-NEXT: warning: '[[FILE]]': unable to get a version for entry 1 of SHT_GNU_versym section with index 1: invalid SHT_GNU_verdef section with index 2: version definition 1 goes past the end of the section +# INVALID-VERDEF-GNU-NEXT: 000: 0 (*local*) 2 () --- !ELF FileHeader: diff --git a/llvm/test/tools/llvm-readobj/ELF/verneed-invalid.test b/llvm/test/tools/llvm-readobj/ELF/verneed-invalid.test index e8d75af..66c2d83 100644 --- a/llvm/test/tools/llvm-readobj/ELF/verneed-invalid.test +++ b/llvm/test/tools/llvm-readobj/ELF/verneed-invalid.test @@ -10,7 +10,7 @@ # GNU-VERNEED-NAME: Version symbols section '.gnu.version' contains 2 entries: # GNU-VERNEED-NAME-NEXT: Addr: 0000000000200210 Offset: 0x000040 Link: 5 (.dynsym) -# GNU-VERNEED-NAME-NEXT: 000: 0 (*local*) 2 (*invalid*) +# GNU-VERNEED-NAME-NEXT: 000: 0 (*local*) 2 () # GNU-VERNEED-NAME: Version needs section '.gnu.version_r' contains 1 entries: # GNU-VERNEED-NAME-NEXT: Addr: 0000000000000000 Offset: 0x000044 Link: 6 (.dynstr) @@ -86,9 +86,9 @@ DynamicSymbols: # GNU-NOLINK: Version symbols section '.gnu.version' contains 2 entries: # GNU-NOLINK-NEXT: Addr: 0000000000000000 Offset: 0x000040 Link: 5 (.dynsym) -# GNU-NOLINK-NEXT: 000: 0 (*local*) +# GNU-NOLINK-EMPTY: # GNU-NOLINK-NEXT: warning: '[[FILE]]': invalid string table linked to SHT_GNU_verneed section with index 2: invalid sh_type for string table section [index 0]: expected SHT_STRTAB, but got SHT_NULL -# GNU-NOLINK-NEXT: 2 () +# GNU-NOLINK-NEXT: 000: 0 (*local*) 2 () # GNU-NOLINK-EMPTY: # GNU-NOLINK: Version needs section '.gnu.version_r' contains 1 entries: # GNU-NOLINK-NEXT: Addr: 0000000000000000 Offset: 0x000044 Link: 0 () @@ -158,10 +158,23 @@ DynamicSymbols: ## We can't parse misaligned auxiliary version records. # RUN: yaml2obj --docnum=3 %s -o %t3 -# RUN: not llvm-readelf -V %t3 2>&1 | FileCheck %s -DFILE=%t3 --check-prefix=BROKEN-AUX -# RUN: not llvm-readobj -V %t3 2>&1 | FileCheck %s -DFILE=%t3 --check-prefix=BROKEN-AUX - -# BROKEN-AUX: error: '[[FILE]]': invalid SHT_GNU_verneed section with index 2: found a misaligned auxiliary entry at offset 0x11 +# RUN: llvm-readelf -V %t3 2>&1 | FileCheck %s -DFILE=%t3 --check-prefix=BROKEN-AUX-GNU +# RUN: llvm-readobj -V %t3 2>&1 | FileCheck %s -DFILE=%t3 --check-prefix=BROKEN-AUX-LLVM + +# BROKEN-AUX-GNU: Version symbols section '.gnu.version' contains 1 entries: +# BROKEN-AUX-GNU-NEXT: Addr: 0000000000000000 Offset: 0x000040 Link: 5 (.dynsym) +# BROKEN-AUX-GNU-EMPTY: +# BROKEN-AUX-GNU-NEXT: warning: '[[FILE]]': unable to get a version for entry 0 of SHT_GNU_versym section with index 1: invalid SHT_GNU_verneed section with index 2: found a misaligned auxiliary entry at offset 0x11 +# BROKEN-AUX-GNU-NEXT: 000: 2 () + +# BROKEN-AUX-LLVM: VersionSymbols [ +# BROKEN-AUX-LLVM-NEXT: Symbol { +# BROKEN-AUX-LLVM-NEXT: Version: 2 +# BROKEN-AUX-LLVM-EMPTY: +# BROKEN-AUX-LLVM-NEXT: warning: '[[FILE]]': invalid SHT_GNU_verneed section with index 2: found a misaligned auxiliary entry at offset 0x11 +# BROKEN-AUX-LLVM-NEXT: Name: @ +# BROKEN-AUX-LLVM-NEXT: } +# BROKEN-AUX-LLVM-NEXT: ] --- !ELF FileHeader: diff --git a/llvm/test/tools/llvm-readobj/ELF/versym-invalid.test b/llvm/test/tools/llvm-readobj/ELF/versym-invalid.test index a88318e..96c33de 100644 --- a/llvm/test/tools/llvm-readobj/ELF/versym-invalid.test +++ b/llvm/test/tools/llvm-readobj/ELF/versym-invalid.test @@ -218,3 +218,47 @@ Sections: DynamicSymbols: - Name: foo - Name: bar + +## Version index in a SHT_GNU_versym section overflows the version map. +## Check we report it when trying to dump dynamic symbols. + +# RUN: yaml2obj %s --docnum=8 -o %t8 +# RUN: llvm-readobj --dyn-syms %t8 2>&1 \ +# RUN: | FileCheck -DFILE=%t8 --implicit-check-not=warning --check-prefix=VERSION-OVERFLOW-LLVM %s +# RUN: llvm-readelf --dyn-syms %t8 2>&1 \ +# RUN: | FileCheck -DFILE=%t8 --implicit-check-not=warning --check-prefix=VERSION-OVERFLOW-GNU %s + +# VERSION-OVERFLOW-LLVM: DynamicSymbols [ +# VERSION-OVERFLOW-LLVM-EMPTY: +# VERSION-OVERFLOW-LLVM-NEXT: warning: '[[FILE]]': SHT_GNU_versym section refers to a version index 255 which is missing +# VERSION-OVERFLOW-LLVM-NEXT: Symbol { +# VERSION-OVERFLOW-LLVM-NEXT: Name: @ (0) +# VERSION-OVERFLOW-LLVM: warning: '[[FILE]]': SHT_GNU_versym section refers to a version index 254 which is missing +# VERSION-OVERFLOW-LLVM-NEXT: Symbol { +# VERSION-OVERFLOW-LLVM-NEXT: Name: foo@ (5) +# VERSION-OVERFLOW-LLVM: Symbol { +# VERSION-OVERFLOW-LLVM-NEXT: Name: bar@ (1) + +# VERSION-OVERFLOW-GNU: Symbol table '.dynsym' contains 3 entries: +# VERSION-OVERFLOW-GNU-NEXT: Num: Value Size Type Bind Vis Ndx Name +# VERSION-OVERFLOW-GNU-EMPTY: +# VERSION-OVERFLOW-GNU-NEXT: warning: '[[FILE]]': SHT_GNU_versym section refers to a version index 255 which is missing +# VERSION-OVERFLOW-GNU-NEXT: 0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND @ +# VERSION-OVERFLOW-GNU-EMPTY: +# VERSION-OVERFLOW-GNU-NEXT: warning: '[[FILE]]': SHT_GNU_versym section refers to a version index 254 which is missing +# VERSION-OVERFLOW-GNU-NEXT: 1: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND foo@ +# VERSION-OVERFLOW-GNU-NEXT: 2: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND bar@ *Obj); void parseDynamicTable(); - StringRef getSymbolVersion(const Elf_Sym *symb, bool &IsDefault) const; - void LoadVersionMap() const; + Expected getSymbolVersion(const Elf_Sym *symb, + bool &IsDefault) const; + Error LoadVersionMap() const; const object::ELFObjectFile *ObjF; DynRegionInfo DynRelRegion; @@ -320,8 +321,8 @@ public: unsigned SectionIndex) const; Expected getStaticSymbolName(uint32_t Index) const; std::string getDynamicString(uint64_t Value) const; - StringRef getSymbolVersionByIndex(uint32_t VersionSymbolIndex, - bool &IsDefault) const; + Expected getSymbolVersionByIndex(uint32_t VersionSymbolIndex, + bool &IsDefault) const; void printSymbolsHelper(bool IsDynamic) const; void printDynamicEntry(raw_ostream &OS, uint64_t Type, uint64_t Value) const; @@ -970,14 +971,14 @@ std::error_code createELFDumper(const object::ObjectFile *Obj, } // end namespace llvm -template void ELFDumper::LoadVersionMap() const { +template Error ELFDumper::LoadVersionMap() const { // If there is no dynamic symtab or version table, there is nothing to do. if (!DynSymRegion.Addr || !SymbolVersionSection) - return; + return Error::success(); // Has the VersionMap already been loaded? if (!VersionMap.empty()) - return; + return Error::success(); // The first two version indexes are reserved. // Index 0 is LOCAL, index 1 is GLOBAL. @@ -994,7 +995,7 @@ template void ELFDumper::LoadVersionMap() const { Expected> Defs = this->getVersionDefinitions(SymbolVersionDefSection); if (!Defs) - reportError(Defs.takeError(), ObjF->getFileName()); + return Defs.takeError(); for (const VerDef &Def : *Defs) InsertEntry(Def.Ndx & ELF::VERSYM_VERSION, Def.Name, true); } @@ -1003,16 +1004,18 @@ template void ELFDumper::LoadVersionMap() const { Expected> Deps = this->getVersionDependencies(SymbolVersionNeedSection); if (!Deps) - reportError(Deps.takeError(), ObjF->getFileName()); + return Deps.takeError(); for (const VerNeed &Dep : *Deps) for (const VernAux &Aux : Dep.AuxV) InsertEntry(Aux.Other & ELF::VERSYM_VERSION, Aux.Name, false); } + + return Error::success(); } template -StringRef ELFDumper::getSymbolVersion(const Elf_Sym *Sym, - bool &IsDefault) const { +Expected ELFDumper::getSymbolVersion(const Elf_Sym *Sym, + bool &IsDefault) const { // This is a dynamic symbol. Look in the GNU symbol version table. if (!SymbolVersionSection) { // No version table. @@ -1056,8 +1059,9 @@ ELFDumper::getStaticSymbolName(uint32_t Index) const { } template -StringRef ELFDumper::getSymbolVersionByIndex(uint32_t SymbolVersionIndex, - bool &IsDefault) const { +Expected +ELFDumper::getSymbolVersionByIndex(uint32_t SymbolVersionIndex, + bool &IsDefault) const { size_t VersionIndex = SymbolVersionIndex & VERSYM_VERSION; // Special markers for unversioned symbols. @@ -1067,9 +1071,11 @@ StringRef ELFDumper::getSymbolVersionByIndex(uint32_t SymbolVersionIndex, } // Lookup this symbol in the version table. - LoadVersionMap(); + if (Error E = LoadVersionMap()) + return std::move(E); if (VersionIndex >= VersionMap.size() || !VersionMap[VersionIndex]) - reportError(createError("Invalid version entry"), ObjF->getFileName()); + return createError("SHT_GNU_versym section refers to a version index " + + Twine(VersionIndex) + " which is missing"); const VersionEntry &Entry = *VersionMap[VersionIndex]; if (Entry.IsVerDef) @@ -1107,10 +1113,15 @@ std::string ELFDumper::getFullSymbolName(const Elf_Sym *Symbol, return SymbolName; bool IsDefault; - StringRef Version = getSymbolVersion(&*Symbol, IsDefault); - if (!Version.empty()) { + Expected VersionOrErr = getSymbolVersion(&*Symbol, IsDefault); + if (!VersionOrErr) { + ELFDumperStyle->reportUniqueWarning(VersionOrErr.takeError()); + return SymbolName + "@"; + } + + if (!VersionOrErr->empty()) { SymbolName += (IsDefault ? "@@" : "@"); - SymbolName += Version; + SymbolName += *VersionOrErr; } return SymbolName; } @@ -4110,34 +4121,41 @@ void GNUStyle::printVersionSymbolSection(const ELFFile *Obj, return; } + ArrayRef VerTable = *VerTableOrErr; + std::vector Versions; + for (size_t I = 0, E = VerTable.size(); I < E; ++I) { + unsigned Ndx = VerTable[I].vs_index; + if (Ndx == VER_NDX_LOCAL || Ndx == VER_NDX_GLOBAL) { + Versions.emplace_back(Ndx == VER_NDX_LOCAL ? "*local*" : "*global*"); + continue; + } + + bool IsDefault; + Expected NameOrErr = + this->dumper()->getSymbolVersionByIndex(Ndx, IsDefault); + if (!NameOrErr || NameOrErr->empty()) { + if (!NameOrErr) { + unsigned SecNdx = Sec - &cantFail(Obj->sections()).front(); + this->reportUniqueWarning(createError( + "unable to get a version for entry " + Twine(I) + + " of SHT_GNU_versym section with index " + Twine(SecNdx) + ": " + + toString(NameOrErr.takeError()))); + } + Versions.emplace_back(""); + continue; + } + Versions.emplace_back(*NameOrErr); + } + // readelf prints 4 entries per line. - uint64_t Entries = VerTableOrErr->size(); + uint64_t Entries = VerTable.size(); for (uint64_t VersymRow = 0; VersymRow < Entries; VersymRow += 4) { OS << " " << format_hex_no_prefix(VersymRow, 3) << ":"; - for (uint64_t I = 0; (I < 4) && (I + VersymRow) < Entries; ++I) { - unsigned Version = (*VerTableOrErr)[VersymRow + I].vs_index; - switch (Version) { - case 0: - OS << " 0 (*local*) "; - break; - case 1: - OS << " 1 (*global*) "; - break; - default: - bool IsDefault = true; - std::string VersionName = - this->dumper()->getSymbolVersionByIndex(Version, IsDefault); - - if (!VersionName.empty()) - VersionName = "(" + VersionName + ")"; - else - VersionName = "(*invalid*)"; - - OS << format("%4x%c", Version & VERSYM_VERSION, - Version & VERSYM_HIDDEN ? 'h' : ' '); - OS << left_justify(VersionName, 13); - } + unsigned Ndx = VerTable[VersymRow + I].vs_index; + OS << format("%4x%c", Ndx & VERSYM_VERSION, + Ndx & VERSYM_HIDDEN ? 'h' : ' '); + OS << left_justify("(" + std::string(Versions[VersymRow + I]) + ")", 13); } OS << '\n'; } -- 2.7.4