Skip to content

Commit

Permalink
[llvm-readelf/obj] - Move unique warning handling logic to the `ObjDu…
Browse files Browse the repository at this point in the history
…mper`.

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<ELFT>` 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
  • Loading branch information
Georgii Rymar committed Dec 1, 2020
1 parent e785379 commit 31eeac9
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 31 deletions.
3 changes: 2 additions & 1 deletion llvm/tools/llvm-readobj/COFFDumper.cpp
Expand Up @@ -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;
Expand Down
27 changes: 3 additions & 24 deletions llvm/tools/llvm-readobj/ELFDumper.cpp
Expand Up @@ -64,7 +64,6 @@
#include <memory>
#include <string>
#include <system_error>
#include <unordered_set>
#include <vector>

using namespace llvm;
Expand Down Expand Up @@ -355,8 +354,6 @@ template <typename ELFT> class ELFDumper : public ObjDumper {
};
mutable SmallVector<Optional<VersionEntry>, 16> VersionMap;

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

std::string describe(const Elf_Shdr &Sec) const;

public:
Expand Down Expand Up @@ -435,9 +432,6 @@ template <typename ELFT> class ELFDumper : public ObjDumper {

Expected<RelSymbol<ELFT>> getRelocationTarget(const Relocation<ELFT> &R,
const Elf_Shdr *SymTab) const;

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

template <class ELFT>
Expand Down Expand Up @@ -956,14 +950,6 @@ template <typename ELFT> class GNUStyle : public DumpStyle<ELFT> {
const Twine &Label, unsigned EntriesNum);
};

template <class ELFT>
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));
Expand Down Expand Up @@ -2020,16 +2006,9 @@ void ELFDumper<ELFT>::loadDynamicTable() {
template <typename ELFT>
ELFDumper<ELFT>::ELFDumper(const object::ELFObjectFile<ELFT> &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<ELFT>(Writer, *this));
else
Expand Down
2 changes: 1 addition & 1 deletion llvm/tools/llvm-readobj/MachODumper.cpp
Expand Up @@ -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;
Expand Down
18 changes: 16 additions & 2 deletions llvm/tools/llvm-readobj/ObjDumper.cpp
Expand Up @@ -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) {
Expand Down
9 changes: 8 additions & 1 deletion llvm/tools/llvm-readobj/ObjDumper.h
Expand Up @@ -16,6 +16,8 @@
#include "llvm/Object/ObjectFile.h"
#include "llvm/Support/CommandLine.h"

#include <unordered_set>

namespace llvm {
namespace object {
class COFFImportFile;
Expand All @@ -32,7 +34,7 @@ class ScopedPrinter;

class ObjDumper {
public:
ObjDumper(ScopedPrinter &Writer);
ObjDumper(ScopedPrinter &Writer, StringRef ObjName);
virtual ~ObjDumper();

virtual bool canDumpContent() { return true; }
Expand Down Expand Up @@ -109,6 +111,9 @@ class ObjDumper {
void printSectionsAsHex(const object::ObjectFile &Obj,
ArrayRef<std::string> Sections);

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

protected:
ScopedPrinter &W;

Expand All @@ -117,6 +122,8 @@ class ObjDumper {
virtual void printDynamicSymbols() {}
virtual void printProgramHeaders() {}
virtual void printSectionMapping() {}

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

std::unique_ptr<ObjDumper> createCOFFDumper(const object::COFFObjectFile &Obj,
Expand Down
2 changes: 1 addition & 1 deletion llvm/tools/llvm-readobj/WasmDumper.cpp
Expand Up @@ -57,7 +57,7 @@ static const EnumEntry<unsigned> 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;
Expand Down
2 changes: 1 addition & 1 deletion llvm/tools/llvm-readobj/XCOFFDumper.cpp
Expand Up @@ -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;
Expand Down

0 comments on commit 31eeac9

Please sign in to comment.