[llvm] r368260 - [llvm-readobj/libObject] - Introduce a custom warning handler for `ELFFile<ELFT>` methods.
George Rimar via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 8 00:17:35 PDT 2019
Author: grimar
Date: Thu Aug 8 00:17:35 2019
New Revision: 368260
URL: http://llvm.org/viewvc/llvm-project?rev=368260&view=rev
Log:
[llvm-readobj/libObject] - Introduce a custom warning handler for `ELFFile<ELFT>` methods.
Currently, we have a code duplication in llvm-readobj which was introduced in D63266.
The duplication was introduced to allow llvm-readobj to dump the partially
broken object. Methods in ELFFile<ELFT> perform a strict validation of the inputs,
what is itself good, but not for dumper tools, that might want to dump the information,
even if some pieces are broken/unexpected.
This patch introduces a warning handler which can be passed to ELFFile<ELFT> methods
and can allow skipping the non-critical errors when needed/possible.
For demonstration, I removed the duplication from llvm-readobj and implemented a warning using
the new custom warning handler. It also deduplicates the strings printed, making the output less verbose.
Differential revision: https://reviews.llvm.org/D65515
Modified:
llvm/trunk/include/llvm/Object/ELF.h
llvm/trunk/test/Object/invalid.test
llvm/trunk/test/tools/llvm-readobj/elf-wrong-shstrtab-type.test
llvm/trunk/tools/llvm-readobj/ELFDumper.cpp
llvm/trunk/tools/llvm-readobj/llvm-readobj.cpp
llvm/trunk/tools/llvm-readobj/llvm-readobj.h
Modified: llvm/trunk/include/llvm/Object/ELF.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/ELF.h?rev=368260&r1=368259&r2=368260&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Object/ELF.h (original)
+++ llvm/trunk/include/llvm/Object/ELF.h Thu Aug 8 00:17:35 2019
@@ -64,6 +64,8 @@ std::string getSecIndexForError(const EL
return "[unknown index]";
}
+static Error defaultWarningHandler(const Twine &Msg) { return createError(Msg); }
+
template <class ELFT>
class ELFFile {
public:
@@ -95,6 +97,13 @@ public:
using Elf_Relr_Range = typename ELFT::RelrRange;
using Elf_Phdr_Range = typename ELFT::PhdrRange;
+ // This is a callback that can be passed to a number of functions.
+ // It can be used to ignore non-critical errors (warnings), which is
+ // useful for dumpers, like llvm-readobj.
+ // It accepts a warning message string and returns a success
+ // when the warning should be ignored or an error otherwise.
+ using WarningHandler = llvm::function_ref<Error(const Twine &Msg)>;
+
const uint8_t *base() const { return Buf.bytes_begin(); }
size_t getBufSize() const { return Buf.size(); }
@@ -114,7 +123,9 @@ public:
template <typename T>
Expected<const T *> getEntry(const Elf_Shdr *Section, uint32_t Entry) const;
- Expected<StringRef> getStringTable(const Elf_Shdr *Section) const;
+ Expected<StringRef>
+ getStringTable(const Elf_Shdr *Section,
+ WarningHandler WarnHandler = &defaultWarningHandler) const;
Expected<StringRef> getStringTableForSymtab(const Elf_Shdr &Section) const;
Expected<StringRef> getStringTableForSymtab(const Elf_Shdr &Section,
Elf_Shdr_Range Sections) const;
@@ -261,7 +272,9 @@ public:
return make_range(notes_begin(Shdr, Err), notes_end());
}
- Expected<StringRef> getSectionStringTable(Elf_Shdr_Range Sections) const;
+ Expected<StringRef> getSectionStringTable(
+ Elf_Shdr_Range Sections,
+ WarningHandler WarnHandler = &defaultWarningHandler) const;
Expected<uint32_t> getSectionIndex(const Elf_Sym *Sym, Elf_Sym_Range Syms,
ArrayRef<Elf_Word> ShndxTable) const;
Expected<const Elf_Shdr *> getSection(const Elf_Sym *Sym,
@@ -275,7 +288,9 @@ public:
Expected<const Elf_Sym *> getSymbol(const Elf_Shdr *Sec,
uint32_t Index) const;
- Expected<StringRef> getSectionName(const Elf_Shdr *Section) const;
+ Expected<StringRef>
+ getSectionName(const Elf_Shdr *Section,
+ WarningHandler WarnHandler = &defaultWarningHandler) const;
Expected<StringRef> getSectionName(const Elf_Shdr *Section,
StringRef DotShstrtab) const;
template <typename T>
@@ -458,7 +473,8 @@ ELFFile<ELFT>::getRelocationSymbol(const
template <class ELFT>
Expected<StringRef>
-ELFFile<ELFT>::getSectionStringTable(Elf_Shdr_Range Sections) const {
+ELFFile<ELFT>::getSectionStringTable(Elf_Shdr_Range Sections,
+ WarningHandler WarnHandler) const {
uint32_t Index = getHeader()->e_shstrndx;
if (Index == ELF::SHN_XINDEX)
Index = Sections[0].sh_link;
@@ -468,7 +484,7 @@ ELFFile<ELFT>::getSectionStringTable(Elf
if (Index >= Sections.size())
return createError("section header string table index " + Twine(Index) +
" does not exist");
- return getStringTable(&Sections[Index]);
+ return getStringTable(&Sections[Index], WarnHandler);
}
template <class ELFT> ELFFile<ELFT>::ELFFile(StringRef Object) : Buf(Object) {}
@@ -569,13 +585,16 @@ ELFFile<ELFT>::getSection(uint32_t Index
template <class ELFT>
Expected<StringRef>
-ELFFile<ELFT>::getStringTable(const Elf_Shdr *Section) const {
+ELFFile<ELFT>::getStringTable(const Elf_Shdr *Section,
+ WarningHandler WarnHandler) const {
if (Section->sh_type != ELF::SHT_STRTAB)
- return createError("invalid sh_type for string table section " +
- getSecIndexForError(this, Section) +
- ": expected SHT_STRTAB, but got " +
- object::getELFSectionTypeName(getHeader()->e_machine,
- Section->sh_type));
+ if (Error E = WarnHandler("invalid sh_type for string table section " +
+ getSecIndexForError(this, Section) +
+ ": expected SHT_STRTAB, but got " +
+ object::getELFSectionTypeName(
+ getHeader()->e_machine, Section->sh_type)))
+ return std::move(E);
+
auto V = getSectionContentsAsArray<char>(Section);
if (!V)
return V.takeError();
@@ -650,11 +669,12 @@ ELFFile<ELFT>::getStringTableForSymtab(c
template <class ELFT>
Expected<StringRef>
-ELFFile<ELFT>::getSectionName(const Elf_Shdr *Section) const {
+ELFFile<ELFT>::getSectionName(const Elf_Shdr *Section,
+ WarningHandler WarnHandler) const {
auto SectionsOrErr = sections();
if (!SectionsOrErr)
return SectionsOrErr.takeError();
- auto Table = getSectionStringTable(*SectionsOrErr);
+ auto Table = getSectionStringTable(*SectionsOrErr, WarnHandler);
if (!Table)
return Table.takeError();
return getSectionName(Section, *Table);
Modified: llvm/trunk/test/Object/invalid.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Object/invalid.test?rev=368260&r1=368259&r2=368260&view=diff
==============================================================================
--- llvm/trunk/test/Object/invalid.test (original)
+++ llvm/trunk/test/Object/invalid.test Thu Aug 8 00:17:35 2019
@@ -344,9 +344,9 @@ Sections:
## overflows the section name string table.
# RUN: yaml2obj %s --docnum=17 -o %t17
-# RUN: not llvm-readobj --sections %t17 2>&1 | FileCheck --check-prefix=BROKEN-SECNAME %s
+# RUN: not llvm-readobj --sections %t17 2>&1 | FileCheck -DFILE=%t17 --check-prefix=BROKEN-SECNAME %s
-## BROKEN-SECNAME: error: a section [index 1] has an invalid sh_name (0x1) offset which goes past the end of the section name string table
+## BROKEN-SECNAME: error: '[[FILE]]': a section [index 1] has an invalid sh_name (0x1) offset which goes past the end of the section name string table
--- !ELF
FileHeader:
Modified: llvm/trunk/test/tools/llvm-readobj/elf-wrong-shstrtab-type.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-readobj/elf-wrong-shstrtab-type.test?rev=368260&r1=368259&r2=368260&view=diff
==============================================================================
--- llvm/trunk/test/tools/llvm-readobj/elf-wrong-shstrtab-type.test (original)
+++ llvm/trunk/test/tools/llvm-readobj/elf-wrong-shstrtab-type.test Thu Aug 8 00:17:35 2019
@@ -1,15 +1,38 @@
## Check we do not fail to dump the section headers when
## a .shstrtab section does not have a SHT_STRTAB type.
+## Check we report only one warning for the issue for each input object.
+
# RUN: yaml2obj %s -o %t1
-# RUN: llvm-readobj -S %t1 | FileCheck %s --check-prefix LLVM
-# RUN: llvm-readelf -S %t1 | FileCheck %s --check-prefix GNU
+# RUN: llvm-readobj -S %t1 2>&1 | FileCheck %s -DFILE=%t1 --implicit-check-not warning --check-prefix LLVM
+# RUN: llvm-readelf -S %t1 2>&1 | FileCheck %s -DFILE=%t1 --implicit-check-not warning --check-prefix GNU
+
+# LLVM: warning: '[[FILE]]': invalid sh_type for string table section [index 1]: expected SHT_STRTAB, but got SHT_PROGBITS
+# LLVM: Section {
+# LLVM: Name: .shstrtab
+# LLVM: Type: SHT_PROGBITS
+
+# GNU: Section Headers:
+# GNU: [Nr] Name Type Address Off Size ES Flg Lk Inf Al
+# GNU: warning: '[[FILE]]': invalid sh_type for string table section [index 1]: expected SHT_STRTAB, but got SHT_PROGBITS
+# GNU: [ 1] .shstrtab PROGBITS 0000000000000000 000140 00001b 00 0 0 0
+
+## Test we report multiple identical warnings (one for each object) when dumping an archive.
+
+# RUN: rm -f %t.a
+# RUN: cp %t1 %t2
+# RUN: llvm-ar rc %t.a %t1 %t2 %t1
+# RUN: llvm-readobj -S %t.a 2>&1 | FileCheck %s --implicit-check-not warning --check-prefix WARNINGS
+# RUN: llvm-readelf -S %t.a 2>&1 | FileCheck %s --implicit-check-not warning --check-prefix WARNINGS
+
+# WARNINGS: warning: '{{.*}}1': invalid sh_type for string table section [index 1]: expected SHT_STRTAB, but got SHT_PROGBITS
+# WARNINGS: warning: '{{.*}}2': invalid sh_type for string table section [index 1]: expected SHT_STRTAB, but got SHT_PROGBITS
+# WARNINGS: warning: '{{.*}}1': invalid sh_type for string table section [index 1]: expected SHT_STRTAB, but got SHT_PROGBITS
-# LLVM: Name: .shstrtab
-# LLVM-NEXT: Type: SHT_PROGBITS
+## Test we report the warning for each input file specified on the command line.
-# GNU: [Nr] Name Type
-# GNU: [ 1] .shstrtab PROGBITS
+# RUN: llvm-readobj -S %t1 %t2 %t1 2>&1 | FileCheck %s --implicit-check-not warning --check-prefix WARNINGS
+# RUN: llvm-readelf -S %t1 %t2 %t1 2>&1 | FileCheck %s --implicit-check-not warning --check-prefix WARNINGS
--- !ELF
FileHeader:
Modified: llvm/trunk/tools/llvm-readobj/ELFDumper.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-readobj/ELFDumper.cpp?rev=368260&r1=368259&r2=368260&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-readobj/ELFDumper.cpp (original)
+++ llvm/trunk/tools/llvm-readobj/ELFDumper.cpp Thu Aug 8 00:17:35 2019
@@ -63,6 +63,7 @@
#include <memory>
#include <string>
#include <system_error>
+#include <unordered_set>
#include <vector>
using namespace llvm;
@@ -352,7 +353,17 @@ public:
using Elf_Sym = typename ELFT::Sym;
using Elf_Addr = typename ELFT::Addr;
- DumpStyle(ELFDumper<ELFT> *Dumper) : Dumper(Dumper) {}
+ DumpStyle(ELFDumper<ELFT> *Dumper) : Dumper(Dumper) {
+ // Dumper reports all non-critical errors as warnings.
+ // It does not print the same warning more than once.
+ WarningHandler = [this](const Twine &Msg) {
+ StringRef FileName = this->Dumper->getElfObject()->getFileName();
+ if (Warnings.insert(Msg.str()).second)
+ reportWarning(FileName, createError(Msg));
+ return Error::success();
+ };
+ }
+
virtual ~DumpStyle() = default;
virtual void printFileHeaders(const ELFFile<ELFT> *Obj) = 0;
@@ -401,7 +412,11 @@ public:
virtual void printMipsPLT(const MipsGOTParser<ELFT> &Parser) = 0;
const ELFDumper<ELFT> *dumper() const { return Dumper; }
+protected:
+ std::function<Error(const Twine &Msg)> WarningHandler;
+
private:
+ std::unordered_set<std::string> Warnings;
const ELFDumper<ELFT> *Dumper;
};
@@ -3032,26 +3047,6 @@ static std::string getSectionTypeString(
}
template <class ELFT>
-static StringRef getSectionName(const typename ELFT::Shdr &Sec,
- const ELFObjectFile<ELFT> &ElfObj,
- ArrayRef<typename ELFT::Shdr> Sections) {
- const ELFFile<ELFT> &Obj = *ElfObj.getELFFile();
- uint32_t Index = Obj.getHeader()->e_shstrndx;
- if (Index == ELF::SHN_XINDEX)
- Index = Sections[0].sh_link;
- if (!Index) // no section string table.
- return "";
- // TODO: Test a case when the sh_link of the section with index 0 is broken.
- if (Index >= Sections.size())
- reportError(ElfObj.getFileName(),
- createError("section header string table index " +
- Twine(Index) + " does not exist"));
- StringRef Data = toStringRef(unwrapOrError(
- Obj.template getSectionContentsAsArray<uint8_t>(&Sections[Index])));
- return unwrapOrError(Obj.getSectionName(&Sec, Data));
-}
-
-template <class ELFT>
void GNUStyle<ELFT>::printSectionHeaders(const ELFO *Obj) {
unsigned Bias = ELFT::Is64Bits ? 0 : 8;
ArrayRef<Elf_Shdr> Sections = unwrapOrError(Obj->sections());
@@ -3072,7 +3067,8 @@ void GNUStyle<ELFT>::printSectionHeaders
size_t SectionIndex = 0;
for (const Elf_Shdr &Sec : Sections) {
Fields[0].Str = to_string(SectionIndex);
- Fields[1].Str = getSectionName(Sec, *ElfObj, Sections);
+ Fields[1].Str = unwrapOrError<StringRef>(
+ ElfObj->getFileName(), Obj->getSectionName(&Sec, this->WarningHandler));
Fields[2].Str =
getSectionTypeString(Obj->getHeader()->e_machine, Sec.sh_type);
Fields[3].Str =
@@ -4984,7 +4980,8 @@ void LLVMStyle<ELFT>::printSectionHeader
ArrayRef<Elf_Shdr> Sections = unwrapOrError(Obj->sections());
const ELFObjectFile<ELFT> *ElfObj = this->dumper()->getElfObject();
for (const Elf_Shdr &Sec : Sections) {
- StringRef Name = getSectionName(Sec, *ElfObj, Sections);
+ StringRef Name = unwrapOrError(
+ ElfObj->getFileName(), Obj->getSectionName(&Sec, this->WarningHandler));
DictScope SectionD(W, "Section");
W.printNumber("Index", ++SectionIndex);
W.printNumber("Name", Name, Sec.sh_name);
Modified: llvm/trunk/tools/llvm-readobj/llvm-readobj.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-readobj/llvm-readobj.cpp?rev=368260&r1=368259&r2=368260&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-readobj/llvm-readobj.cpp (original)
+++ llvm/trunk/tools/llvm-readobj/llvm-readobj.cpp Thu Aug 8 00:17:35 2019
@@ -394,6 +394,12 @@ void reportWarning(Twine Msg) {
WithColor::warning(errs()) << Msg << "\n";
}
+void reportWarning(StringRef Input, Error Err) {
+ if (Input == "-")
+ Input = "<stdin>";
+ warn(createFileError(Input, std::move(Err)));
+}
+
void warn(Error Err) {
handleAllErrors(std::move(Err), [&](const ErrorInfoBase &EI) {
reportWarning(EI.message());
Modified: llvm/trunk/tools/llvm-readobj/llvm-readobj.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-readobj/llvm-readobj.h?rev=368260&r1=368259&r2=368260&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-readobj/llvm-readobj.h (original)
+++ llvm/trunk/tools/llvm-readobj/llvm-readobj.h Thu Aug 8 00:17:35 2019
@@ -24,6 +24,7 @@ namespace llvm {
LLVM_ATTRIBUTE_NORETURN void reportError(Twine Msg);
void reportError(StringRef Input, Error Err);
void reportWarning(Twine Msg);
+ void reportWarning(StringRef Input, Error Err);
void warn(llvm::Error Err);
void error(std::error_code EC);
void error(llvm::Error EC);
@@ -37,6 +38,8 @@ namespace llvm {
return *EO;
reportError(EO.getError().message());
}
+
+ // TODO: This one is deprecated. Use one with a Input name below.
template <class T> T unwrapOrError(Expected<T> EO) {
if (EO)
return *EO;
@@ -46,6 +49,13 @@ namespace llvm {
OS.flush();
reportError(Buf);
}
+
+ template <class T> T unwrapOrError(StringRef Input, Expected<T> EO) {
+ if (EO)
+ return *EO;
+ reportError(Input, EO.takeError());
+ llvm_unreachable("reportError shouldn't return in this case");
+ }
} // namespace llvm
namespace opts {
More information about the llvm-commits
mailing list