From 31eeac915a0a25c2690b956931c77684bc34da0b Mon Sep 17 00:00:00 2001 From: Georgii Rymar Date: Fri, 27 Nov 2020 13:34:30 +0300 Subject: [PATCH] [llvm-readelf/obj] - Move unique warning handling logic to the `ObjDumper`. This moves the `reportUniqueWarning` method to the base class. My motivation is the following: I've experimented with replacing `reportWarning` calls with `reportUniqueWarning` in ELF dumper. I've found that for example for removing them from `DynRegionInfo` helper class, it is worth to pass a dumper instance to it (to be able to call dumper()->reportUniqueWarning()). The problem was that `ELFDumper` is a template class. I had to make `DynRegionInfo` to be templated and do lots of minor changes everywhere what did not look reasonable/nice. At the same time I guess one day other dumpers like COFF/MachO/Wasm etc might want to start using `reportUniqueWarning` API too. Then it looks reasonable to move the logic to the base class. With that the problem of passing the dumper instance will be gone. Differential revision: https://reviews.llvm.org/D92218 --- llvm/tools/llvm-readobj/COFFDumper.cpp | 3 ++- llvm/tools/llvm-readobj/ELFDumper.cpp | 27 +++---------------------- llvm/tools/llvm-readobj/MachODumper.cpp | 2 +- llvm/tools/llvm-readobj/ObjDumper.cpp | 18 +++++++++++++++-- llvm/tools/llvm-readobj/ObjDumper.h | 9 ++++++++- llvm/tools/llvm-readobj/WasmDumper.cpp | 2 +- llvm/tools/llvm-readobj/XCOFFDumper.cpp | 2 +- 7 files changed, 32 insertions(+), 31 deletions(-) diff --git a/llvm/tools/llvm-readobj/COFFDumper.cpp b/llvm/tools/llvm-readobj/COFFDumper.cpp index b1ac1d9d0f39e..144ecf56d50a1 100644 --- a/llvm/tools/llvm-readobj/COFFDumper.cpp +++ b/llvm/tools/llvm-readobj/COFFDumper.cpp @@ -77,7 +77,8 @@ class COFFDumper : public ObjDumper { public: friend class COFFObjectDumpDelegate; COFFDumper(const llvm::object::COFFObjectFile *Obj, ScopedPrinter &Writer) - : ObjDumper(Writer), Obj(Obj), Writer(Writer), Types(100) {} + : ObjDumper(Writer, Obj->getFileName()), Obj(Obj), Writer(Writer), + Types(100) {} void printFileHeaders() override; void printSectionHeaders() override; diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp index 890bb2a21e82d..f89d88724a9a6 100644 --- a/llvm/tools/llvm-readobj/ELFDumper.cpp +++ b/llvm/tools/llvm-readobj/ELFDumper.cpp @@ -64,7 +64,6 @@ #include #include #include -#include #include using namespace llvm; @@ -355,8 +354,6 @@ template class ELFDumper : public ObjDumper { }; mutable SmallVector, 16> VersionMap; - std::unordered_set Warnings; - std::string describe(const Elf_Shdr &Sec) const; public: @@ -435,9 +432,6 @@ template class ELFDumper : public ObjDumper { Expected> getRelocationTarget(const Relocation &R, const Elf_Shdr *SymTab) const; - - std::function WarningHandler; - void reportUniqueWarning(Error Err) const; }; template @@ -956,14 +950,6 @@ template class GNUStyle : public DumpStyle { const Twine &Label, unsigned EntriesNum); }; -template -void ELFDumper::reportUniqueWarning(Error Err) const { - handleAllErrors(std::move(Err), [&](const ErrorInfoBase &EI) { - cantFail(WarningHandler(EI.message()), - "WarningHandler should always return ErrorSuccess"); - }); -} - template void DumpStyle::reportUniqueWarning(Error Err) const { this->dumper().reportUniqueWarning(std::move(Err)); @@ -2020,16 +2006,9 @@ void ELFDumper::loadDynamicTable() { template ELFDumper::ELFDumper(const object::ELFObjectFile &O, ScopedPrinter &Writer) - : ObjDumper(Writer), ObjF(O), Obj(*O.getELFFile()), DynRelRegion(O), - DynRelaRegion(O), DynRelrRegion(O), DynPLTRelRegion(O), DynamicTable(O) { - // 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), ObjF.getFileName()); - return Error::success(); - }; - + : ObjDumper(Writer, O.getFileName()), ObjF(O), Obj(*O.getELFFile()), + DynRelRegion(O), DynRelaRegion(O), DynRelrRegion(O), DynPLTRelRegion(O), + DynamicTable(O) { if (opts::Output == opts::GNU) ELFDumperStyle.reset(new GNUStyle(Writer, *this)); else diff --git a/llvm/tools/llvm-readobj/MachODumper.cpp b/llvm/tools/llvm-readobj/MachODumper.cpp index 5c4960804e8fa..c13b1f3bf2a0a 100644 --- a/llvm/tools/llvm-readobj/MachODumper.cpp +++ b/llvm/tools/llvm-readobj/MachODumper.cpp @@ -27,7 +27,7 @@ namespace { class MachODumper : public ObjDumper { public: MachODumper(const MachOObjectFile *Obj, ScopedPrinter &Writer) - : ObjDumper(Writer), Obj(Obj) {} + : ObjDumper(Writer, Obj->getFileName()), Obj(Obj) {} void printFileHeaders() override; void printSectionHeaders() override; diff --git a/llvm/tools/llvm-readobj/ObjDumper.cpp b/llvm/tools/llvm-readobj/ObjDumper.cpp index f46993b4f4285..b830628e7f930 100644 --- a/llvm/tools/llvm-readobj/ObjDumper.cpp +++ b/llvm/tools/llvm-readobj/ObjDumper.cpp @@ -26,9 +26,23 @@ static inline Error createError(const Twine &Msg) { return createStringError(object::object_error::parse_failed, Msg); } -ObjDumper::ObjDumper(ScopedPrinter &Writer) : W(Writer) {} +ObjDumper::ObjDumper(ScopedPrinter &Writer, StringRef ObjName) : W(Writer) { + // Dumper reports all non-critical errors as warnings. + // It does not print the same warning more than once. + WarningHandler = [=](const Twine &Msg) { + if (Warnings.insert(Msg.str()).second) + reportWarning(createError(Msg), ObjName); + return Error::success(); + }; +} + +ObjDumper::~ObjDumper() {} -ObjDumper::~ObjDumper() { +void ObjDumper::reportUniqueWarning(Error Err) const { + handleAllErrors(std::move(Err), [&](const ErrorInfoBase &EI) { + cantFail(WarningHandler(EI.message()), + "WarningHandler should always return ErrorSuccess"); + }); } static void printAsPrintable(raw_ostream &W, const uint8_t *Start, size_t Len) { diff --git a/llvm/tools/llvm-readobj/ObjDumper.h b/llvm/tools/llvm-readobj/ObjDumper.h index fa1767fe8902e..156c2d19c8f6f 100644 --- a/llvm/tools/llvm-readobj/ObjDumper.h +++ b/llvm/tools/llvm-readobj/ObjDumper.h @@ -16,6 +16,8 @@ #include "llvm/Object/ObjectFile.h" #include "llvm/Support/CommandLine.h" +#include + namespace llvm { namespace object { class COFFImportFile; @@ -32,7 +34,7 @@ class ScopedPrinter; class ObjDumper { public: - ObjDumper(ScopedPrinter &Writer); + ObjDumper(ScopedPrinter &Writer, StringRef ObjName); virtual ~ObjDumper(); virtual bool canDumpContent() { return true; } @@ -109,6 +111,9 @@ class ObjDumper { void printSectionsAsHex(const object::ObjectFile &Obj, ArrayRef Sections); + std::function WarningHandler; + void reportUniqueWarning(Error Err) const; + protected: ScopedPrinter &W; @@ -117,6 +122,8 @@ class ObjDumper { virtual void printDynamicSymbols() {} virtual void printProgramHeaders() {} virtual void printSectionMapping() {} + + std::unordered_set Warnings; }; std::unique_ptr createCOFFDumper(const object::COFFObjectFile &Obj, diff --git a/llvm/tools/llvm-readobj/WasmDumper.cpp b/llvm/tools/llvm-readobj/WasmDumper.cpp index bb07b9d5bd155..488c6faa26b80 100644 --- a/llvm/tools/llvm-readobj/WasmDumper.cpp +++ b/llvm/tools/llvm-readobj/WasmDumper.cpp @@ -57,7 +57,7 @@ static const EnumEntry WasmSymbolFlags[] = { class WasmDumper : public ObjDumper { public: WasmDumper(const WasmObjectFile *Obj, ScopedPrinter &Writer) - : ObjDumper(Writer), Obj(Obj) {} + : ObjDumper(Writer, Obj->getFileName()), Obj(Obj) {} void printFileHeaders() override; void printSectionHeaders() override; diff --git a/llvm/tools/llvm-readobj/XCOFFDumper.cpp b/llvm/tools/llvm-readobj/XCOFFDumper.cpp index 3e34b9e104ed1..8f0f18cedcebf 100644 --- a/llvm/tools/llvm-readobj/XCOFFDumper.cpp +++ b/llvm/tools/llvm-readobj/XCOFFDumper.cpp @@ -24,7 +24,7 @@ class XCOFFDumper : public ObjDumper { public: XCOFFDumper(const XCOFFObjectFile &Obj, ScopedPrinter &Writer) - : ObjDumper(Writer), Obj(Obj) {} + : ObjDumper(Writer, Obj.getFileName()), Obj(Obj) {} void printFileHeaders() override; void printSectionHeaders() override;