[llvm-readelf/obj] - Move unique warning handling logic to the `ObjDumper`.

[grimar]

Summary

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.

[jhenderson]

LGTM. The inline suggestion probably deserves a separate patch. Might want to wait and give others an opportunity to chime in, especially as it's Thanksgiving weekend in the US.

llvm/tools/llvm-readobj/ObjDumper.cpp

43–44

Maybe we could simplify this by having WarningHandler not return an Error at all? What do you think?

[grimar]

llvm/tools/llvm-readobj/ObjDumper.cpp

43–44

I'll take a look.

[grimar]

llvm/tools/llvm-readobj/ObjDumper.cpp

43–44

I think we can't do this, because this WarningHandler is used as argument to the API from ELF.h:

// 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)>;
....
template <class ELFT>
Expected<StringRef>
ELFFile<ELFT>::getSectionName(const Elf_Shdr &Section,
                              WarningHandler WarnHandler) const {
  auto SectionsOrErr = sections();
  if (!SectionsOrErr)
    return SectionsOrErr.takeError();
  auto Table = getSectionStringTable(*SectionsOrErr, WarnHandler);
  if (!Table)
    return Table.takeError();
  return getSectionName(Section, *Table);
}

llvm/tools/llvm-readobj/COFFDumper.cpp

struct LoadConfigTables {
   uint64_t GuardLJmpTableVA = 0;
   uint64_t GuardLJmpTableCount = 0;
 };
 
 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;
   void printRelocations() override;
   void printUnwindInfo() override;
 
   void printNeededLibraries() override;
 

llvm/tools/llvm-readobj/ELFDumper.cpp

 #include <cinttypes>
 #include <cstddef>
 #include <cstdint>
 #include <cstdlib>
 #include <iterator>
 #include <memory>
 #include <string>
 #include <system_error>
-#include <unordered_set>
 #include <vector>
 
 using namespace llvm;
 using namespace llvm::object;
 using namespace ELF;
 
 #define LLVM_READOBJ_ENUM_CASE(ns, enum)                                       \
   case ns::enum:                                                               \
private:
   const Elf_Shdr *SymbolVersionDefSection = nullptr; // .gnu.version_d
 
   struct VersionEntry {
     std::string Name;
     bool IsVerDef;
   };
   mutable SmallVector<Optional<VersionEntry>, 16> VersionMap;
 
-  std::unordered_set<std::string> Warnings;
-
   std::string describe(const Elf_Shdr &Sec) const;
 
 public:
   unsigned getHashTableEntSize() const {
     // EM_S390 and ELF::EM_ALPHA platforms use 8-bytes entries in SHT_HASH
     // sections. This violates the ELF specification.
     if (Obj.getHeader().e_machine == ELF::EM_S390 ||
         Obj.getHeader().e_machine == ELF::EM_ALPHA)
Expected<ArrayRef<Elf_Versym>> getVersionTable(const Elf_Shdr &Sec,
                                                  StringRef *StrTab) const;
   Expected<std::vector<VerDef>>
   getVersionDefinitions(const Elf_Shdr &Sec) const;
   Expected<std::vector<VerNeed>>
   getVersionDependencies(const Elf_Shdr &Sec) const;
 
   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>
 static std::string describe(const ELFFile<ELFT> &Obj,
                             const typename ELFT::Shdr &Sec) {
   unsigned SecNdx = &Sec - &cantFail(Obj.sections()).front();
   return (object::getELFSectionTypeName(Obj.getHeader().e_machine,
                                         Sec.sh_type) +
private:
   std::string getSymbolSectionNdx(const Elf_Sym &Symbol, unsigned SymIndex);
   void printProgramHeaders();
   void printSectionMapping();
   void printGNUVersionSectionProlog(const typename ELFT::Shdr &Sec,
                                     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));
 }
 
 template <typename ELFT> class LLVMStyle : public DumpStyle<ELFT> {
 public:
   TYPEDEF_ELF_TYPES(ELFT)
 
void ELFDumper<ELFT>::loadDynamicTable() {
   }
 
   parseDynamicTable();
 }
 
 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
     ELFDumperStyle.reset(new LLVMStyle<ELFT>(Writer, *this));
 
   if (!O.IsContentValid())
     return;
 

llvm/tools/llvm-readobj/MachODumper.cpp

 using namespace llvm;
 using namespace object;
 
 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;
   void printRelocations() override;
   void printUnwindInfo() override;
   void printStackMap() const override;
 
   void printNeededLibraries() override;

llvm/tools/llvm-readobj/ObjDumper.h

 
 #include <memory>
 #include <system_error>
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/CommandLine.h"
 
+#include <unordered_set>
+
 namespace llvm {
 namespace object {
 class COFFImportFile;
 class ObjectFile;
 class XCOFFObjectFile;
 class ELFObjectFileBase;
 }
 namespace codeview {
 class GlobalTypeTableBuilder;
 class MergingTypeTableBuilder;
 } // namespace codeview
 
 class ScopedPrinter;
 
 class ObjDumper {
 public:
-  ObjDumper(ScopedPrinter &Writer);
+  ObjDumper(ScopedPrinter &Writer, StringRef ObjName);
   virtual ~ObjDumper();
 
   virtual bool canDumpContent() { return true; }
 
   virtual void printFileHeaders() = 0;
   virtual void printSectionHeaders() = 0;
   virtual void printRelocations() = 0;
   virtual void printSymbols(bool PrintSymbols, bool PrintDynamicSymbols) {
public:
 
   virtual void printStackMap() const = 0;
 
   void printSectionsAsString(const object::ObjectFile &Obj,
                              ArrayRef<std::string> Sections);
   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;
 
 private:
   virtual void printSymbols() {}
   virtual void printDynamicSymbols() {}
   virtual void printProgramHeaders() {}
   virtual void printSectionMapping() {}
+
+  std::unordered_set<std::string> Warnings;
 };
 
 std::unique_ptr<ObjDumper> createCOFFDumper(const object::COFFObjectFile &Obj,
                                             ScopedPrinter &Writer);
 
 std::unique_ptr<ObjDumper> createELFDumper(const object::ELFObjectFileBase &Obj,
                                            ScopedPrinter &Writer);
 

llvm/tools/llvm-readobj/ObjDumper.cpp

 #include <map>
 
 namespace llvm {
 
 static inline Error createError(const Twine &Msg) {
   return createStringError(object::object_error::parse_failed, Msg);
 }
 
-ObjDumper::ObjDumper(ScopedPrinter &Writer) : W(Writer) {}
-
-ObjDumper::~ObjDumper() {
+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() {}
+
+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) {
   for (size_t i = 0; i < Len; i++)
     W << (isPrint(Start[i]) ? static_cast<char>(Start[i]) : '.');
 }
 
 static std::vector<object::SectionRef>

llvm/tools/llvm-readobj/WasmDumper.cpp

#define ENUM_ENTRY(X)                                                          \
   ENUM_ENTRY(EXPLICIT_NAME),
   ENUM_ENTRY(NO_STRIP),
 #undef ENUM_ENTRY
 };
 
 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;
   void printRelocations() override;
   void printUnwindInfo() override { llvm_unreachable("unimplemented"); }
   void printStackMap() const override { llvm_unreachable("unimplemented"); }
 
 protected:

llvm/tools/llvm-readobj/XCOFFDumper.cpp

 using namespace object;
 
 namespace {
 
 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;
   void printRelocations() override;
   void printSymbols() override;
   void printDynamicSymbols() override;
   void printUnwindInfo() override;
   void printStackMap() const override;