[llvm-readobj] Add -hex-dump (-x) option

[paulsemel]

Summary

This follows the same behavior as GNU objcopy --hex-dump option.

[dblaikie]

Could you add a test case?

[paulsemel]

In D48281#1135274, @dblaikie wrote:

Could you add a test case?

You're totally right. Plus, there is a little bug in the code, will fix it and upload the tests in the same time today !

[paulsemel]

Fixed overflow on section when section end is inbetween rows.
Added test.

[dblaikie]

Could/should this be format-agnostic? (looks like this change only implements the functionality for ELF - but I'd imagine the existence of sections and their contents might be general enough to allow for different object file formats to support this without each format having custom handling)

[paulsemel]

In D48281#1138290, @dblaikie wrote:

Could/should this be format-agnostic? (looks like this change only implements the functionality for ELF - but I'd imagine the existence of sections and their contents might be general enough to allow for different object file formats to support this without each format having custom handling)

Well, I think it is actually feasable. I'm not really familiar with all the formats handled by readobj, but I can give it a shot !

[paulsemel]

Added other binary formats handling.

[dblaikie]

include/llvm/Object/MachO.h

307–309 ↗

(On Diff #152467)

Is there really no suitable existing API for getting the contents of a section? (like the ones above that use DataRefImpl?)

& these two APIs using different schemes (one returning Expected<SectionRef> the other returning error_code and taking a SectionRef out parameter) seem unnecessarily inconsistent - can they use a matching approach (I'd favor the Expected<SectionRef> option)?

Maybe @echristo has some ideas/familiarity here.

tools/llvm-readobj/COFFDumper.cpp

665 ↗

(On Diff #152467)

Is there a need for bounds checking before casting that long down to an integer? (what's the intended behavior when the value is too large to fit in an integer? Is that tested?)

697 ↗

(On Diff #152467)

Typo 'Least' -> 'Last' (or Lastly)?

tools/llvm-readobj/ELFDumper.cpp

153

Does this need support in each format explicitly - or is there a way to generalize it over all formats in one place? (ie: could all formats support the ability to retrieve the section contents, and then the printing would be generalized over that)

Certainly looks like there's a lot of duplication in the different dumper function implementations - at least would be good to pull a lot of that out into a generalized helper even if there is some format-specific stuff to be handled (though I'm not sure if there should be any format-specific stuff to handle).

[echristo]

include/llvm/Object/MachO.h

307–309 ↗

(On Diff #152467)

In particular, I'd have thought we wanted getSectionContents here?

tools/llvm-readobj/ELFDumper.cpp

153

+1

[paulsemel]

include/llvm/Object/MachO.h

307–309 ↗

(On Diff #152467)

Well, there is an API for getting section content, but it requires to have to so called section.
Here I want to get the section either by index or by name, so I don't see any problem with the two functions. (this scheme was already present for the ELF and COFF files, just added it to MachO)

Yes I agree about the two different schemes, I'm gonna change it right now :)

tools/llvm-readobj/COFFDumper.cpp

665 ↗

(On Diff #152467)

Well, that's not a problem related to this patch, that's a more general problem about this function taking an integer..

697 ↗

(On Diff #152467)

Yes, sorry about it :/

tools/llvm-readobj/ELFDumper.cpp

153

I actually didn't see any generic helper in this project, so I duplicated. I definitely think that it is doable.
Do you have an idea where I can write generic functions with this architecture ?

[paulsemel]

Refactor code to create a generic function.
Applied David and Eric' suggestions.

[dblaikie]

Perhaps generalized further still - if every format has the ability to retrieve a section by name or by index and return a generic llvm::object::SectionRef - then the code can be written in general over that, query the SectionRef for the name and contents, etc?

[paulsemel]

In D48281#1150111, @dblaikie wrote:

Perhaps generalized further still - if every format has the ability to retrieve a section by name or by index and return a generic llvm::object::SectionRef - then the code can be written in general over that, query the SectionRef for the name and contents, etc?

Well, I thought about it, but the problem is that the interface are a bit different.. I tried to change the interface COFF (which is the only one that needs changes), but it would break too much things if I do so..

[paulsemel]

In D48281#1150535, @paulsemel wrote:

In D48281#1150111, @dblaikie wrote:

Perhaps generalized further still - if every format has the ability to retrieve a section by name or by index and return a generic llvm::object::SectionRef - then the code can be written in general over that, query the SectionRef for the name and contents, etc?

Well, I thought about it, but the problem is that the interface are a bit different.. I tried to change the interface COFF (which is the only one that needs changes), but it would break too much things if I do so..

Just to add one more argument to this. I think that getting the sections by Index/Name is not implemented as virtual method in ObjectFile. Thus, we might better do this way not to get confused on what can (for the moment) be generic and what can't be.
So what do you think ?

[paulsemel]

ping @dblaikie :)
I'd really like to go forward on this one, to be able to refact the -p option :)

[dblaikie]

OK

[paulsemel]

Ok I'm going to land this one, so that I can refactor the (-p) option, with the helpers created in this patch !

test/tools/llvm-readobj/print-hex.test

+RUN: llvm-readobj -x .strtab %p/Inputs/trivial.obj.elf-x86-64 \
+RUN:     | FileCheck %s
+
+CHECK: 0x00000000 00747269 7669616c 2e6c6c00 6d61696e .trivial.ll.main
+CHECK: 0x00000010 002e4c2e 73747200 70757473 00536f6d ..L.str.puts.Som
+CHECK: 0x00000020 654f7468 65724675 6e637469 6f6e005f eOtherFunction._
+CHECK: 0x00000030 474c4f42 414c5f4f 46465345 545f5441 GLOBAL_OFFSET_TA
+CHECK: 0x00000040 424c455f 00                         BLE_.

tools/llvm-readobj/ELFDumper.cpp

public:
   void printSymbols() override;
   void printDynamicSymbols() override;
   void printUnwindInfo() override;
 
   void printDynamicTable() override;
   void printNeededLibraries() override;
   void printProgramHeaders() override;
   void printSectionAsString(StringRef StringName) override;
+  void printSectionAsHex(StringRef StringName) override;
   void printHashTable() override;
   void printGnuHashTable() override;
   void printLoadName() override;
   void printVersionInfo() override;
   void printGroupSections() override;
 
   void printAttributes() override;
   void printMipsPLTGOT() override;
public:
   virtual void printSymtabMessage(const ELFFile<ELFT> *obj, StringRef Name,
                                   size_t Offset) {}
   virtual void printSymbol(const ELFFile<ELFT> *Obj, const Elf_Sym *Symbol,
                            const Elf_Sym *FirstSym, StringRef StrTable,
                            bool IsDynamic) = 0;
   virtual void printProgramHeaders(const ELFFile<ELFT> *Obj) = 0;
   virtual void printSectionAsString(const ELFFile<ELFT> *Obj,
                                    StringRef SectionName) = 0;
+  virtual void printSectionAsHex(const ELFFile<ELFT> *Obj,
+                                 StringRef SectionName) = 0;
   virtual void printHashHistogram(const ELFFile<ELFT> *Obj) = 0;
   virtual void printCGProfile(const ELFFile<ELFT> *Obj) = 0;
   virtual void printNotes(const ELFFile<ELFT> *Obj) = 0;
   virtual void printELFLinkerOptions(const ELFFile<ELFT> *Obj) = 0;
   virtual void printMipsGOT(const MipsGOTParser<ELFT> &Parser) = 0;
   virtual void printMipsPLT(const MipsGOTParser<ELFT> &Parser) = 0;
   const ELFDumper<ELFT> *dumper() const { return Dumper; }
 
public:
   void printSections(const ELFO *Obj) override;
   void printSymbols(const ELFO *Obj) override;
   void printDynamicSymbols(const ELFO *Obj) override;
   void printDynamicRelocations(const ELFO *Obj) override;
   void printSymtabMessage(const ELFO *Obj, StringRef Name,
                           size_t Offset) override;
   void printProgramHeaders(const ELFO *Obj) override;
   void printSectionAsString(const ELFO *Obj, StringRef SectionName) override;
+  void printSectionAsHex(const ELFO *Obj, StringRef SectionName) override;
   void printHashHistogram(const ELFFile<ELFT> *Obj) override;
   void printCGProfile(const ELFFile<ELFT> *Obj) override;
   void printNotes(const ELFFile<ELFT> *Obj) override;
   void printELFLinkerOptions(const ELFFile<ELFT> *Obj) override;
   void printMipsGOT(const MipsGOTParser<ELFT> &Parser) override;
   void printMipsPLT(const MipsGOTParser<ELFT> &Parser) override;
 
 private:
public:
   void printRelocations(const ELFO *Obj) override;
   void printRelocations(const Elf_Shdr *Sec, const ELFO *Obj);
   void printSections(const ELFO *Obj) override;
   void printSymbols(const ELFO *Obj) override;
   void printDynamicSymbols(const ELFO *Obj) override;
   void printDynamicRelocations(const ELFO *Obj) override;
   void printProgramHeaders(const ELFO *Obj) override;
   void printSectionAsString(const ELFO *Obj, StringRef SectionName) override;
+  void printSectionAsHex(const ELFO *Obj, StringRef SectionName) override;
   void printHashHistogram(const ELFFile<ELFT> *Obj) override;
   void printCGProfile(const ELFFile<ELFT> *Obj) override;
   void printNotes(const ELFFile<ELFT> *Obj) override;
   void printELFLinkerOptions(const ELFFile<ELFT> *Obj) override;
   void printMipsGOT(const MipsGOTParser<ELFT> &Parser) override;
   void printMipsPLT(const MipsGOTParser<ELFT> &Parser) override;
 
 private:
template <class ELFT> void ELFDumper<ELFT>::printProgramHeaders() {
   ELFDumperStyle->printProgramHeaders(Obj);
 }
 
 template <class ELFT>
 void ELFDumper<ELFT>::printSectionAsString(StringRef SectionName) {
   ELFDumperStyle->printSectionAsString(Obj, SectionName);
 }
 
+template <class ELFT>
+void ELFDumper<ELFT>::printSectionAsHex(StringRef SectionName) {
+  ELFDumperStyle->printSectionAsHex(Obj, SectionName);
+}
+
 template <class ELFT> void ELFDumper<ELFT>::printDynamicRelocations() {
   ELFDumperStyle->printDynamicRelocations(Obj);
 }
 
 template<class ELFT>
 void ELFDumper<ELFT>::printSymbols() {
   ELFDumperStyle->printSymbols(Obj);
 }
while (CurrentWord <= SecEnd) {
     OS << format("[%6tx]", CurrentWord - SecContent);
     OS << format(" %.*s\n", WordSize, CurrentWord);
     CurrentWord += WordSize + 1;
   }
   OS.flush();
 }
 
 template <class ELFT>
+void GNUStyle<ELFT>::printSectionAsHex(const ELFO *Obj, StringRef SectionName) {
+  char *StrPtr;
+  long SectionIndex = strtol(SectionName.data(), &StrPtr, 10);
+  const Elf_Shdr *Sec;
+  if (*StrPtr)
+    Sec = unwrapOrError(Obj->getSection(SectionName));
+  else
+    Sec = unwrapOrError(Obj->getSection((unsigned int)SectionIndex));
+
+  StringRef SecName = unwrapOrError(Obj->getSectionName(Sec));
+  OS << "Hex dump of section '" << SecName << "':\n";
+  const uint8_t *SecContent =
+      reinterpret_cast<const uint8_t *>(Obj->base() + Sec->sh_offset);
+  const uint8_t *SecEnd = SecContent + Sec->sh_size;
+
+  for (const uint8_t *SecPtr = SecContent; SecPtr < SecEnd; SecPtr += 16) {
+    const uint8_t *TmpSecPtr = SecPtr;
+    uint8_t i;
+    uint8_t k;
+
+    OS << format_hex(SecPtr - SecContent, 10);
+    OS << ' ';
+    for (i = 0; TmpSecPtr < SecEnd && i < 4; ++i) {
+      for (k = 0; TmpSecPtr < SecEnd && k < 4; k++, TmpSecPtr++) {
+        uint8_t Val = *(reinterpret_cast<const uint8_t *>(TmpSecPtr));
+        OS << format_hex_no_prefix(Val, 2);
+      }
+      OS << ' ';
+    }
+
+    // We need to print the correct amount of spaces to match the format.
+    // We are adding the (4 - i) last rows that are 8 characters each.
+    // Then, the (4 - i) spaces that are in between the rows.
+    // Least, if we cut in a middle of a row, we add the remaining characters,
+    // which is (8 - (k * 2))
+    if (i < 4)
+      OS << format("%*c", (4 - i) * 8 + (4 - i) + (8 - (k * 2)), ' ');
+
+    TmpSecPtr = SecPtr;
+    for (i = 0; TmpSecPtr + i < SecEnd && i < 16; ++i) {
+      if (isprint(TmpSecPtr[i]))
+        OS << TmpSecPtr[i];
+      else
+        OS << '.';
+    }
+
+    OS << '\n';
+  }
+  OS.flush();
+}
+
+template <class ELFT>
 void GNUStyle<ELFT>::printDynamicRelocation(const ELFO *Obj, Elf_Rela R,
                                             bool IsRela) {
   SmallString<32> RelocName;
   StringRef SymbolName;
   unsigned Width = ELFT::Is64Bits ? 16 : 8;
   unsigned Bias = ELFT::Is64Bits ? 8 : 0;
   // First two fields are bit width dependent. The rest of them are after are
   // fixed width.
W.startLine() << "["
                          format_hex_no_prefix((CurrentWord - SecContent), 6))
                   << "]";
     W.startLine() << format(" %.*s\n", WordSize, CurrentWord);
     CurrentWord += WordSize + 1;
   }
 }
 
 template <class ELFT>
+void LLVMStyle<ELFT>::printSectionAsHex(const ELFO *Obj,
+                                        StringRef SectionName) {
+  char *StrPtr;
+  long SectionIndex = strtol(SectionName.data(), &StrPtr, 10);
+  const Elf_Shdr *Sec;
+  if (*StrPtr)
+    Sec = unwrapOrError(Obj->getSection(SectionName));
+  else
+    Sec = unwrapOrError(Obj->getSection((unsigned int)SectionIndex));
+
+  StringRef SecName = unwrapOrError(Obj->getSectionName(Sec));
+  W.startLine() << "Hex dump of section '" << SecName << "':\n";
+  const uint8_t *SecContent =
+      reinterpret_cast<const uint8_t *>(Obj->base() + Sec->sh_offset);
+  const uint8_t *SecEnd = SecContent + Sec->sh_size;
+
+  for (const uint8_t *SecPtr = SecContent; SecPtr < SecEnd; SecPtr += 16) {
+    const uint8_t *TmpSecPtr = SecPtr;
+    uint8_t i;
+    uint8_t k;
+
+    W.startLine() << format_hex(SecPtr - SecContent, 10);
+    W.startLine() << ' ';
+    for (i = 0; TmpSecPtr < SecEnd && i < 4; ++i) {
+      for (k = 0; TmpSecPtr < SecEnd && k < 4; k++, TmpSecPtr++) {
+        uint8_t Val = *(reinterpret_cast<const uint8_t *>(TmpSecPtr));
+        W.startLine() << format_hex_no_prefix(Val, 2);
+      }
+      W.startLine() << ' ';
+    }
+
+    // We need to print the correct amount of spaces to match the format.
+    // We are adding the (4 - i) last rows that are 8 characters each.
+    // Then, the (4 - i) spaces that are in between the rows.
+    // Least, if we cut in a middle of a row, we add the remaining characters,
+    // which is (8 - (k * 2))
+    if (i < 4)
+      W.startLine() << format("%*c", (4 - i) * 8 + (4 - i) + (8 - (k * 2)),
+                              ' ');
+
+    TmpSecPtr = SecPtr;
+    for (i = 0; TmpSecPtr + i < SecEnd && i < 16; ++i) {
+      if (isprint(TmpSecPtr[i]))
+        W.startLine() << TmpSecPtr[i];
+      else
+        W.startLine() << '.';
+    }
+
+    W.startLine() << '\n';
+  }
+}
+
+template <class ELFT>
 void LLVMStyle<ELFT>::printHashHistogram(const ELFFile<ELFT> *Obj) {
   W.startLine() << "Hash Histogram not implemented!\n";
 }
 
 template <class ELFT>
 void LLVMStyle<ELFT>::printCGProfile(const ELFFile<ELFT> *Obj) {
   ListScope L(W, "CGProfile");
   if (!this->dumper()->getDotCGProfileSec())

tools/llvm-readobj/ObjDumper.h

public:
   virtual void printUnwindInfo() = 0;
 
   // Only implemented for ELF at this time.
   virtual void printDynamicRelocations() { }
   virtual void printDynamicTable() { }
   virtual void printNeededLibraries() { }
   virtual void printProgramHeaders() { }
   virtual void printSectionAsString(StringRef SectionName) {}
+  virtual void printSectionAsHex(StringRef SectionName) {}
   virtual void printHashTable() { }
   virtual void printGnuHashTable() { }
   virtual void printLoadName() {}
   virtual void printVersionInfo() {}
   virtual void printGroupSections() {}
   virtual void printHashHistogram() {}
   virtual void printCGProfile() {}
   virtual void printNotes() {}

tools/llvm-readobj/llvm-readobj.cpp

cl::alias ProgramHeadersShort("l", cl::desc("Alias for --program-headers"),
                                 cl::aliasopt(ProgramHeaders));
 
   // -string-dump
   cl::list<std::string> StringDump("string-dump", cl::desc("<number|name>"),
                                    cl::ZeroOrMore);
   cl::alias StringDumpShort("p", cl::desc("Alias for --string-dump"),
                             cl::aliasopt(StringDump));
 
+  // -hex-dump
+  cl::list<std::string> HexDump("hex-dump", cl::desc("<number|name>"),
+                                cl::ZeroOrMore);
+  cl::alias HexDumpShort("x", cl::desc("Alias for --hex-dump"),
+                         cl::aliasopt(HexDump));
+
   // -hash-table
   cl::opt<bool> HashTable("hash-table",
     cl::desc("Display ELF hash table"));
 
   // -gnu-hash-table
   cl::opt<bool> GnuHashTable("gnu-hash-table",
     cl::desc("Display ELF .gnu.hash section"));
 
static void dumpObject(const ObjectFile *Obj, ScopedPrinter &Writer) {
   if (opts::NeededLibraries)
     Dumper->printNeededLibraries();
   if (opts::ProgramHeaders)
     Dumper->printProgramHeaders();
   if (!opts::StringDump.empty())
     llvm::for_each(opts::StringDump, [&Dumper](StringRef SectionName) {
       Dumper->printSectionAsString(SectionName);
     });
+  if (!opts::HexDump.empty())
+    llvm::for_each(opts::HexDump, [&Dumper](StringRef SectionName) {
+      Dumper->printSectionAsHex(SectionName);
+    });
   if (opts::HashTable)
     Dumper->printHashTable();
   if (opts::GnuHashTable)
     Dumper->printGnuHashTable();
   if (opts::VersionInfo)
     Dumper->printVersionInfo();
   if (Obj->isELF()) {
     if (opts::ELFLinkerOptions)