[llvm] dac5ddb - [llvm-readelf/llvm-readobj] - Improved the error reporting in a few method related to versioning.
Georgii Rymar via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 10 02:09:33 PST 2019
Author: Georgii Rymar
Date: 2019-12-10T13:08:18+03:00
New Revision: dac5ddb482361cde11ac43e94c43acc94a3b78aa
URL: https://github.com/llvm/llvm-project/commit/dac5ddb482361cde11ac43e94c43acc94a3b78aa
DIFF: https://github.com/llvm/llvm-project/commit/dac5ddb482361cde11ac43e94c43acc94a3b78aa.diff
LOG: [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
Added:
Modified:
llvm/test/Object/invalid.test
llvm/test/tools/llvm-readobj/ELF/verdef-invalid.test
llvm/test/tools/llvm-readobj/ELF/verneed-invalid.test
llvm/test/tools/llvm-readobj/ELF/versym-invalid.test
llvm/tools/llvm-readobj/ELFDumper.cpp
Removed:
################################################################################
diff --git a/llvm/test/Object/invalid.test b/llvm/test/Object/invalid.test
index 91a93b06b967..b47ba6f9facd 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 f80e9383f708..e22ed6ee7ecb 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@<corrupt>
# 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 (<corrupt>)
--- !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 e8d75afe1e6a..66c2d833a3a8 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 (<corrupt>)
# 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 (<corrupt>)
+# GNU-NOLINK-NEXT: 000: 0 (*local*) 2 (<corrupt>)
# 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 (<corrupt>)
+
+# 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: @<corrupt>
+# 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 a88318e5f492..96c33dee1a3b 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: @<corrupt> (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@<corrupt> (5)
+# VERSION-OVERFLOW-LLVM: Symbol {
+# VERSION-OVERFLOW-LLVM-NEXT: Name: bar@<corrupt> (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 @<corrupt>
+# 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@<corrupt>
+# VERSION-OVERFLOW-GNU-NEXT: 2: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND bar@<corrupt
+
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_DYN
+ Machine: EM_X86_64
+Sections:
+ - Name: .gnu.version
+ Type: SHT_GNU_versym
+ Entries: [ 0xFF, 0xFE, 0xFF ]
+DynamicSymbols:
+ - Name: foo
+ - Name: bar
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index 1bb2c505af6f..145c625bdd2b 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -259,8 +259,9 @@ template <typename ELFT> class ELFDumper : public ObjDumper {
void loadDynamicTable(const ELFFile<ELFT> *Obj);
void parseDynamicTable();
- StringRef getSymbolVersion(const Elf_Sym *symb, bool &IsDefault) const;
- void LoadVersionMap() const;
+ Expected<StringRef> getSymbolVersion(const Elf_Sym *symb,
+ bool &IsDefault) const;
+ Error LoadVersionMap() const;
const object::ELFObjectFile<ELFT> *ObjF;
DynRegionInfo DynRelRegion;
@@ -320,8 +321,8 @@ template <typename ELFT> class ELFDumper : public ObjDumper {
unsigned SectionIndex) const;
Expected<std::string> getStaticSymbolName(uint32_t Index) const;
std::string getDynamicString(uint64_t Value) const;
- StringRef getSymbolVersionByIndex(uint32_t VersionSymbolIndex,
- bool &IsDefault) const;
+ Expected<StringRef> 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 <class ELFT> void ELFDumper<ELFT>::LoadVersionMap() const {
+template <class ELFT> Error ELFDumper<ELFT>::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 <class ELFT> void ELFDumper<ELFT>::LoadVersionMap() const {
Expected<std::vector<VerDef>> 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 <class ELFT> void ELFDumper<ELFT>::LoadVersionMap() const {
Expected<std::vector<VerNeed>> 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 <typename ELFT>
-StringRef ELFDumper<ELFT>::getSymbolVersion(const Elf_Sym *Sym,
- bool &IsDefault) const {
+Expected<StringRef> ELFDumper<ELFT>::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<ELFT>::getStaticSymbolName(uint32_t Index) const {
}
template <typename ELFT>
-StringRef ELFDumper<ELFT>::getSymbolVersionByIndex(uint32_t SymbolVersionIndex,
- bool &IsDefault) const {
+Expected<StringRef>
+ELFDumper<ELFT>::getSymbolVersionByIndex(uint32_t SymbolVersionIndex,
+ bool &IsDefault) const {
size_t VersionIndex = SymbolVersionIndex & VERSYM_VERSION;
// Special markers for unversioned symbols.
@@ -1067,9 +1071,11 @@ StringRef ELFDumper<ELFT>::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<ELFT>::getFullSymbolName(const Elf_Sym *Symbol,
return SymbolName;
bool IsDefault;
- StringRef Version = getSymbolVersion(&*Symbol, IsDefault);
- if (!Version.empty()) {
+ Expected<StringRef> VersionOrErr = getSymbolVersion(&*Symbol, IsDefault);
+ if (!VersionOrErr) {
+ ELFDumperStyle->reportUniqueWarning(VersionOrErr.takeError());
+ return SymbolName + "@<corrupt>";
+ }
+
+ if (!VersionOrErr->empty()) {
SymbolName += (IsDefault ? "@@" : "@");
- SymbolName += Version;
+ SymbolName += *VersionOrErr;
}
return SymbolName;
}
@@ -4110,34 +4121,41 @@ void GNUStyle<ELFT>::printVersionSymbolSection(const ELFFile<ELFT> *Obj,
return;
}
+ ArrayRef<Elf_Versym> VerTable = *VerTableOrErr;
+ std::vector<StringRef> 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<StringRef> 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("<corrupt>");
+ 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';
}
More information about the llvm-commits
mailing list