[PATCH] D93900: [llvm-readef/obj] - Change the design structure of ELF dumper. NFCI.

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

In D93900#2478721 <https://reviews.llvm.org/D93900#2478721>, @jhenderson wrote:

> Could you highlight the major changes you've had to make to achieve the refactor? This patch is quite large and tricky for me to review in depth.



1. I've introduced 2 dumper classes, derived from the `ELFDumper<ELFT>` and moved the implementation: `GNUStyle` -> `GNUStyleELFDumper`, `LLVMStyle` -> `LLVMStyleELFDumper`.
2. I've removed a bunch of methods from the `ELFDumper<ELFT>`. Like: `printSectionHeaders`, `printRelocations` and many others. They became unneeded, as they only did a redirection to `ELFDumperStyle`, e.g:

  template <class ELFT> void ELFDumper<ELFT>::printSectionHeaders() {
    ELFDumperStyle->printSectionHeaders();
  }



3. Since the `DumpStyle` class was eliminated, the common implementation was moved to the `ELFDumper<ELFT>`.

4. A lot of getters were eliminated:

E.g:

    const Elf_Shdr *getDotSymtabSec() const { return DotSymtabSec; }
    const Elf_Shdr *getDotCGProfileSec() const { return DotCGProfileSec; }
  ...
    const DynRegionInfo &getDynPLTRelRegion() const { return DynPLTRelRegion; }
    const DynRegionInfo &getDynamicTableRegion() const { return DynamicTable; }

because it became possible to use fields directly from the logic.

5. Some `const`s were added to make the new code to compile again.



================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:730
 
-template <class ELFT> class MipsGOTParser;
-
-template <typename ELFT> class DumpStyle {
-public:
-  LLVM_ELF_IMPORT_TYPES_ELFT(ELFT)
-
-  DumpStyle(const ELFDumper<ELFT> &Dumper)
-      : Obj(Dumper.getElfObject().getELFFile()), ElfObj(Dumper.getElfObject()),
-        Dumper(Dumper) {
-    FileName = ElfObj.getFileName();
-  }
-
-  virtual ~DumpStyle() = default;
-
-  virtual void printFileHeaders() = 0;
-  virtual void printGroupSections() = 0;
-  virtual void printRelocations() = 0;
-  virtual void printSectionHeaders() = 0;
-  virtual void printSymbols(bool PrintSymbols, bool PrintDynamicSymbols) = 0;
-  virtual void printHashSymbols() {}
-  virtual void printSectionDetails() {}
-  virtual void printDependentLibs() = 0;
-  virtual void printDynamic() {}
-  virtual void printDynamicRelocations() = 0;
-  virtual void printSymtabMessage(const Elf_Shdr *Symtab, size_t Offset,
-                                  bool NonVisibilityBitsUsed) {}
-  virtual void printSymbol(const Elf_Sym &Symbol, unsigned SymIndex,
-                           Optional<StringRef> StrTable, bool IsDynamic,
-                           bool NonVisibilityBitsUsed) = 0;
-  virtual void printProgramHeaders(bool PrintProgramHeaders,
-                                   cl::boolOrDefault PrintSectionMapping) = 0;
-  virtual void printVersionSymbolSection(const Elf_Shdr *Sec) = 0;
-  virtual void printVersionDefinitionSection(const Elf_Shdr *Sec) = 0;
-  virtual void printVersionDependencySection(const Elf_Shdr *Sec) = 0;
-  virtual void printHashHistograms() = 0;
-  virtual void printCGProfile() = 0;
-  virtual void printAddrsig() = 0;
-  virtual void printNotes() = 0;
-  virtual void printELFLinkerOptions() = 0;
-  virtual void printStackSizes() = 0;
-  void printNonRelocatableStackSizes(std::function<void()> PrintHeader);
-  void printRelocatableStackSizes(std::function<void()> PrintHeader);
-  bool printFunctionStackSize(uint64_t SymValue,
-                              Optional<const Elf_Shdr *> FunctionSec,
-                              const Elf_Shdr &StackSizeSec, DataExtractor Data,
-                              uint64_t *Offset);
-  void printStackSize(const Relocation<ELFT> &R, const Elf_Shdr &RelocSec,
-                      unsigned Ndx, const Elf_Shdr *SymTab,
-                      const Elf_Shdr *FunctionSec, const Elf_Shdr &StackSizeSec,
-                      const RelocationResolver &Resolver, DataExtractor Data);
-  virtual void printStackSizeEntry(uint64_t Size, StringRef FuncName) = 0;
-  virtual void printMipsGOT(const MipsGOTParser<ELFT> &Parser) = 0;
-  virtual void printMipsPLT(const MipsGOTParser<ELFT> &Parser) = 0;
-  virtual void printMipsABIFlags() = 0;
-  const ELFDumper<ELFT> &dumper() const { return Dumper; }
-  void reportUniqueWarning(Error Err) const;
-  void reportUniqueWarning(const Twine &Msg) const;
-
-protected:
-  std::vector<GroupSection> getGroups();
-
-  void printDependentLibsHelper(
-      function_ref<void(const Elf_Shdr &)> OnSectionStart,
-      function_ref<void(StringRef, uint64_t)> OnSectionEntry);
-
-  virtual void printReloc(const Relocation<ELFT> &R, unsigned RelIndex,
-                          const Elf_Shdr &Sec, const Elf_Shdr *SymTab) = 0;
-  virtual void printRelrReloc(const Elf_Relr &R) = 0;
-  virtual void printDynamicReloc(const Relocation<ELFT> &R) = 0;
-  void forEachRelocationDo(
-      const Elf_Shdr &Sec, bool RawRelr,
-      llvm::function_ref<void(const Relocation<ELFT> &, unsigned,
-                              const Elf_Shdr &, const Elf_Shdr *)>
-          RelRelaFn,
-      llvm::function_ref<void(const Elf_Relr &)> RelrFn);
-  void printRelocationsHelper(const Elf_Shdr &Sec);
-  void printDynamicRelocationsHelper();
-  virtual void printDynamicRelocHeader(unsigned Type, StringRef Name,
-                                       const DynRegionInfo &Reg){};
-
-  StringRef getPrintableSectionName(const Elf_Shdr &Sec) const;
-
-  StringRef FileName;
-  const ELFFile<ELFT> &Obj;
-  const ELFObjectFile<ELFT> &ElfObj;
-
-private:
-  const ELFDumper<ELFT> &Dumper;
-};
-
-template <typename ELFT> class GNUStyle : public DumpStyle<ELFT> {
+template <typename ELFT> class GNUStyleELFDumper : public ELFDumper<ELFT> {
   formatted_raw_ostream &OS;
----------------
jhenderson wrote:
> Maybe the name could be simplified slightly to `GNUELFDumper` (and the LLVM one to `LLVMELFDumper`)?
Done.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93900/new/

https://reviews.llvm.org/D93900