Skip to content

Commit

Permalink
[NFC][llvm-readobj] Refactor unique warning handler
Browse files Browse the repository at this point in the history 
The unique warning handler was previously a property of the dump style,
but it is commonly used in the dumper too. Since the two ELF output
styles have no impact on the way warnings are printed, this patch moves
the handler and related functions into the dumper class, instead of the
dump style class.

Reviewed by: MaskRay, grimar

Differential Revision: https://reviews.llvm.org/D76777
  • Loading branch information
@jh7370
jh7370 committed Mar 26, 2020
1 parent 9086db7 commit 3110ac1
Showing 1 changed file with 30 additions and 23 deletions.
53 changes: 30 additions & 23 deletions llvm/tools/llvm-readobj/ELFDumper.cpp
View file
Expand Up @@ -305,6 +305,8 @@ template <typename ELFT> class ELFDumper : public ObjDumper {
};
mutable SmallVector<Optional<VersionEntry>, 16> VersionMap;

std::unordered_set<std::string> Warnings;

public:
Elf_Dyn_Range dynamic_table() const {
// A valid .dynamic section contains an array of entries terminated
Expand Down Expand Up @@ -367,6 +369,9 @@ template <typename ELFT> class ELFDumper : public ObjDumper {

Expected<std::pair<const Elf_Sym *, std::string>>
getRelocationTarget(const Elf_Shdr *SymTab, const Elf_Rela &R) const;

std::function<Error(const Twine &Msg)> WarningHandler;
void reportUniqueWarning(Error Err) const;
};

template <class ELFT>
Expand Down Expand Up @@ -459,12 +464,12 @@ ELFDumper<ELFT>::getVersionTable(const Elf_Shdr *Sec, ArrayRef<Elf_Sym> *SymTab,
Expected<std::pair<ArrayRef<Elf_Sym>, StringRef>> SymTabOrErr =
getLinkAsSymtab(Obj, Sec, SecNdx, SHT_DYNSYM);
if (!SymTabOrErr) {
ELFDumperStyle->reportUniqueWarning(SymTabOrErr.takeError());
reportUniqueWarning(SymTabOrErr.takeError());
return *VersionsOrErr;
}

if (SymTabOrErr->first.size() != VersionsOrErr->size())
ELFDumperStyle->reportUniqueWarning(
reportUniqueWarning(
createError("SHT_GNU_versym section with index " + Twine(SecNdx) +
": the number of entries (" + Twine(VersionsOrErr->size()) +
") does not match the number of symbols (" +
Expand Down Expand Up @@ -578,7 +583,7 @@ ELFDumper<ELFT>::getVersionDependencies(const Elf_Shdr *Sec) const {
StringRef StrTab;
Expected<StringRef> StrTabOrErr = getLinkAsStrtab(Obj, Sec, SecNdx);
if (!StrTabOrErr)
ELFDumperStyle->reportUniqueWarning(StrTabOrErr.takeError());
reportUniqueWarning(StrTabOrErr.takeError());
else
StrTab = *StrTabOrErr;

Expand Down Expand Up @@ -707,14 +712,6 @@ template <typename ELFT> class DumpStyle {

DumpStyle(ELFDumper<ELFT> *Dumper) : Dumper(Dumper) {
FileName = this->Dumper->getElfObject()->getFileName();

// Dumper reports all non-critical errors as warnings.
// It does not print the same warning more than once.
WarningHandler = [this](const Twine &Msg) {
if (Warnings.insert(Msg.str()).second)
reportWarning(createError(Msg), FileName);
return Error::success();
};
}

virtual ~DumpStyle() = default;
Expand Down Expand Up @@ -767,14 +764,11 @@ template <typename ELFT> class DumpStyle {
virtual void printMipsABIFlags(const ELFObjectFile<ELFT> *Obj) = 0;
const ELFDumper<ELFT> *dumper() const { return Dumper; }

void reportUniqueWarning(Error Err) const;

protected:
std::function<Error(const Twine &Msg)> WarningHandler;
void reportUniqueWarning(Error Err) const;
StringRef FileName;

private:
std::unordered_set<std::string> Warnings;
const ELFDumper<ELFT> *Dumper;
};

Expand Down Expand Up @@ -898,13 +892,18 @@ template <typename ELFT> class GNUStyle : public DumpStyle<ELFT> {
};

template <class ELFT>
void DumpStyle<ELFT>::reportUniqueWarning(Error Err) const {
void ELFDumper<ELFT>::reportUniqueWarning(Error Err) const {
handleAllErrors(std::move(Err), [&](const ErrorInfoBase &EI) {
cantFail(WarningHandler(EI.message()),
"WarningHandler should always return ErrorSuccess");
});
}

template <class ELFT>
void DumpStyle<ELFT>::reportUniqueWarning(Error Err) const {
this->dumper()->reportUniqueWarning(std::move(Err));
}

template <typename ELFT> class LLVMStyle : public DumpStyle<ELFT> {
public:
TYPEDEF_ELF_TYPES(ELFT)
Expand Down Expand Up @@ -1156,12 +1155,12 @@ std::string ELFDumper<ELFT>::getFullSymbolName(const Elf_Sym *Symbol,
Expected<unsigned> SectionIndex =
getSymbolSectionIndex(Symbol, Syms.begin());
if (!SectionIndex) {
ELFDumperStyle->reportUniqueWarning(SectionIndex.takeError());
reportUniqueWarning(SectionIndex.takeError());
return "<?>";
}
Expected<StringRef> NameOrErr = getSymbolSectionName(Symbol, *SectionIndex);
if (!NameOrErr) {
ELFDumperStyle->reportUniqueWarning(NameOrErr.takeError());
reportUniqueWarning(NameOrErr.takeError());
return ("<section " + Twine(*SectionIndex) + ">").str();
}
return std::string(*NameOrErr);
Expand All @@ -1173,7 +1172,7 @@ std::string ELFDumper<ELFT>::getFullSymbolName(const Elf_Sym *Symbol,
bool IsDefault;
Expected<StringRef> VersionOrErr = getSymbolVersion(&*Symbol, IsDefault);
if (!VersionOrErr) {
ELFDumperStyle->reportUniqueWarning(VersionOrErr.takeError());
reportUniqueWarning(VersionOrErr.takeError());
return SymbolName + "@<corrupt>";
}

Expand Down Expand Up @@ -1984,6 +1983,14 @@ ELFDumper<ELFT>::ELFDumper(const object::ELFObjectFile<ELFT> *ObjF,
: ObjDumper(Writer), ObjF(ObjF), DynRelRegion(ObjF->getFileName()),
DynRelaRegion(ObjF->getFileName()), DynRelrRegion(ObjF->getFileName()),
DynPLTRelRegion(ObjF->getFileName()), DynamicTable(ObjF->getFileName()) {
// Dumper reports all non-critical errors as warnings.
// It does not print the same warning more than once.
WarningHandler = [this](const Twine &Msg) {
if (Warnings.insert(Msg.str()).second)
reportWarning(createError(Msg), this->ObjF->getFileName());
return Error::success();
};

if (opts::Output == opts::GNU)
ELFDumperStyle.reset(new GNUStyle<ELFT>(Writer, this));
else
Expand Down Expand Up @@ -2174,7 +2181,7 @@ void ELFDumper<ELFT>::parseDynamicTable(const ELFFile<ELFT> *Obj) {
// locate .dynsym at runtime. The location we find in the section header
// and the location we find here should match.
if (DynSymFromTable && DynSymFromTable->Addr != DynSymRegion->Addr)
ELFDumperStyle->reportUniqueWarning(
reportUniqueWarning(
createError("SHT_DYNSYM section header and DT_SYMTAB disagree about "
"the location of the dynamic symbol table"));

Expand All @@ -2184,7 +2191,7 @@ void ELFDumper<ELFT>::parseDynamicTable(const ELFFile<ELFT> *Obj) {
// according to the section header.
if (HashTable &&
HashTable->nchain != DynSymRegion->Size / DynSymRegion->EntSize)
ELFDumperStyle->reportUniqueWarning(createError(
reportUniqueWarning(createError(
"hash table nchain (" + Twine(HashTable->nchain) +
") differs from symbol count derived from SHT_DYNSYM section "
"header (" +
Expand Down Expand Up @@ -3677,7 +3684,7 @@ void GNUStyle<ELFT>::printSectionHeaders(const ELFO *Obj) {
const ELFObjectFile<ELFT> *ElfObj = this->dumper()->getElfObject();
StringRef SecStrTable = unwrapOrError<StringRef>(
ElfObj->getFileName(),
Obj->getSectionStringTable(Sections, this->WarningHandler));
Obj->getSectionStringTable(Sections, this->dumper()->WarningHandler));
size_t SectionIndex = 0;
for (const Elf_Shdr &Sec : Sections) {
Fields[0].Str = to_string(SectionIndex);
Expand Down Expand Up @@ -5845,7 +5852,7 @@ void LLVMStyle<ELFT>::printSectionHeaders(const ELFO *Obj) {
for (const Elf_Shdr &Sec : Sections) {
StringRef Name = "<?>";
if (Expected<StringRef> SecNameOrErr =
Obj->getSectionName(&Sec, this->WarningHandler))
Obj->getSectionName(&Sec, this->dumper()->WarningHandler))
Name = *SecNameOrErr;
else
this->reportUniqueWarning(SecNameOrErr.takeError());
Expand Down

0 comments on commit 3110ac1

Please sign in to comment.