[llvm-readelf] - Split GNUStyle<ELFT>::printHashHistogram. NFC.

As was mentioned in review comments for D80204,
`printHashHistogram` has 2 lambdas that are probably too large
and deserves splitting into member functions.

This patch does it.

Differential revision: https://reviews.llvm.org/D80546

llvm/tools/llvm-readobj/ELFDumper.cpp

@@ -230,7 +230,7 @@ public:
 
   void printStackMap() const override;
 
-  void printHashHistogram() override;
+  void printHashHistograms() override;
 
   void printCGProfile() override;
   void printAddrsig() override;
@@ -742,7 +742,7 @@ public:
                                              const Elf_Shdr *Sec) = 0;
   virtual void printVersionDependencySection(const ELFFile<ELFT> *Obj,
                                              const Elf_Shdr *Sec) = 0;
-  virtual void printHashHistogram(const ELFFile<ELFT> *Obj) = 0;
+  virtual void printHashHistograms(const ELFFile<ELFT> *Obj) = 0;
   virtual void printCGProfile(const ELFFile<ELFT> *Obj) = 0;
   virtual void printAddrsig(const ELFFile<ELFT> *Obj) = 0;
   virtual void printNotes(const ELFFile<ELFT> *Obj) = 0;
@@ -811,7 +811,7 @@ public:
                                      const Elf_Shdr *Sec) override;
   void printVersionDependencySection(const ELFFile<ELFT> *Obj,
                                      const Elf_Shdr *Sec) override;
-  void printHashHistogram(const ELFFile<ELFT> *Obj) override;
+  void printHashHistograms(const ELFFile<ELFT> *Obj) override;
   void printCGProfile(const ELFFile<ELFT> *Obj) override;
   void printAddrsig(const ELFFile<ELFT> *Obj) override;
   void printNotes(const ELFFile<ELFT> *Obj) override;
@@ -823,6 +823,9 @@ public:
   void printMipsABIFlags(const ELFObjectFile<ELFT> *Obj) override;
 
 private:
+  void printHashHistogram(const Elf_Hash &HashTable);
+  void printGnuHashHistogram(const Elf_GnuHash &GnuHashTable);
+
   struct Field {
     std::string Str;
     unsigned Column;
@@ -932,7 +935,7 @@ public:
                                      const Elf_Shdr *Sec) override;
   void printVersionDependencySection(const ELFFile<ELFT> *Obj,
                                      const Elf_Shdr *Sec) override;
-  void printHashHistogram(const ELFFile<ELFT> *Obj) override;
+  void printHashHistograms(const ELFFile<ELFT> *Obj) override;
   void printCGProfile(const ELFFile<ELFT> *Obj) override;
   void printAddrsig(const ELFFile<ELFT> *Obj) override;
   void printNotes(const ELFFile<ELFT> *Obj) override;
@@ -2287,8 +2290,8 @@ template <class ELFT> void ELFDumper<ELFT>::printHashSymbols() {
   ELFDumperStyle->printHashSymbols(ObjF->getELFFile());
 }
 
-template <class ELFT> void ELFDumper<ELFT>::printHashHistogram() {
+template <class ELFT> void ELFDumper<ELFT>::printHashHistograms() {
-  ELFDumperStyle->printHashHistogram(ObjF->getELFFile());
+  ELFDumperStyle->printHashHistograms(ObjF->getELFFile());
 }
 
 template <class ELFT> void ELFDumper<ELFT>::printCGProfile() {
@@ -4556,124 +4559,125 @@ void GNUStyle<ELFT>::printVersionDependencySection(const ELFFile<ELFT> *Obj,
   OS << '\n';
 }
 
-// Hash histogram shows  statistics of how efficient the hash was for the
-// dynamic symbol table. The table shows number of hash buckets for different
-// lengths of chains as absolute number and percentage of the total buckets.
-// Additionally cumulative coverage of symbols for each set of buckets.
 template <class ELFT>
-void GNUStyle<ELFT>::printHashHistogram(const ELFFile<ELFT> *Obj) {
+void GNUStyle<ELFT>::printHashHistogram(const Elf_Hash &HashTable) {
-  auto PrintHashHist = [&](const Elf_Hash &HashTable) {
+  size_t NBucket = HashTable.nbucket;
-    size_t NBucket = HashTable.nbucket;
+  size_t NChain = HashTable.nchain;
-    size_t NChain = HashTable.nchain;
+  ArrayRef<Elf_Word> Buckets = HashTable.buckets();
-    ArrayRef<Elf_Word> Buckets = HashTable.buckets();
+  ArrayRef<Elf_Word> Chains = HashTable.chains();
-    ArrayRef<Elf_Word> Chains = HashTable.chains();
+  size_t TotalSyms = 0;
-    size_t TotalSyms = 0;
+  // If hash table is correct, we have at least chains with 0 length
-    // If hash table is correct, we have at least chains with 0 length
+  size_t MaxChain = 1;
-    size_t MaxChain = 1;
+  size_t CumulativeNonZero = 0;
-    size_t CumulativeNonZero = 0;
 
-    if (NChain == 0 || NBucket == 0)
+  if (NChain == 0 || NBucket == 0)
-      return;
+    return;
 
-    std::vector<size_t> ChainLen(NBucket, 0);
+  std::vector<size_t> ChainLen(NBucket, 0);
-    // Go over all buckets and and note chain lengths of each bucket (total
+  // Go over all buckets and and note chain lengths of each bucket (total
-    // unique chain lengths).
+  // unique chain lengths).
-    for (size_t B = 0; B < NBucket; B++) {
+  for (size_t B = 0; B < NBucket; B++) {
-      std::vector<bool> Visited(NChain);
+    std::vector<bool> Visited(NChain);
-      for (size_t C = Buckets[B]; C < NChain; C = Chains[C]) {
+    for (size_t C = Buckets[B]; C < NChain; C = Chains[C]) {
-        if (C == ELF::STN_UNDEF)
+      if (C == ELF::STN_UNDEF)
-          break;
+        break;
-        if (Visited[C]) {
+      if (Visited[C]) {
-          reportWarning(
+        reportWarning(createError(".hash section is invalid: bucket " +
-              createError(".hash section is invalid: bucket " + Twine(C) +
+                                  Twine(C) +
-                          ": a cycle was detected in the linked chain"),
+                                  ": a cycle was detected in the linked chain"),
-              this->FileName);
+                      this->FileName);
-          break;
+        break;
-        }
-        Visited[C] = true;
-        if (MaxChain <= ++ChainLen[B])
-          MaxChain++;
       }
-      TotalSyms += ChainLen[B];
+      Visited[C] = true;
+      if (MaxChain <= ++ChainLen[B])
+        MaxChain++;
     }
+    TotalSyms += ChainLen[B];
+  }
 
-    if (!TotalSyms)
+  if (!TotalSyms)
-      return;
+    return;
 
-    std::vector<size_t> Count(MaxChain, 0);
+  std::vector<size_t> Count(MaxChain, 0);
-    // Count how long is the chain for each bucket
+  // Count how long is the chain for each bucket
-    for (size_t B = 0; B < NBucket; B++)
+  for (size_t B = 0; B < NBucket; B++)
-      ++Count[ChainLen[B]];
+    ++Count[ChainLen[B]];
-    // Print Number of buckets with each chain lengths and their cumulative
+  // Print Number of buckets with each chain lengths and their cumulative
-    // coverage of the symbols
+  // coverage of the symbols
-    OS << "Histogram for bucket list length (total of " << NBucket
+  OS << "Histogram for bucket list length (total of " << NBucket
-       << " buckets)\n"
+     << " buckets)\n"
-       << " Length  Number     % of total  Coverage\n";
+     << " Length  Number     % of total  Coverage\n";
-    for (size_t I = 0; I < MaxChain; I++) {
+  for (size_t I = 0; I < MaxChain; I++) {
-      CumulativeNonZero += Count[I] * I;
+    CumulativeNonZero += Count[I] * I;
-      OS << format("%7lu  %-10lu (%5.1f%%)     %5.1f%%\n", I, Count[I],
+    OS << format("%7lu  %-10lu (%5.1f%%)     %5.1f%%\n", I, Count[I],
-                   (Count[I] * 100.0) / NBucket,
+                 (Count[I] * 100.0) / NBucket,
-                   (CumulativeNonZero * 100.0) / TotalSyms);
+                 (CumulativeNonZero * 100.0) / TotalSyms);
-    }
+  }
-  };
+}
 
-  auto PrintGnuHashHist = [&](const Elf_GnuHash &GnuHashTable) {
+template <class ELFT>
-    size_t NBucket = GnuHashTable.nbuckets;
+void GNUStyle<ELFT>::printGnuHashHistogram(const Elf_GnuHash &GnuHashTable) {
-    ArrayRef<Elf_Word> Buckets = GnuHashTable.buckets();
+  size_t NBucket = GnuHashTable.nbuckets;
-    unsigned NumSyms = this->dumper()->dynamic_symbols().size();
+  ArrayRef<Elf_Word> Buckets = GnuHashTable.buckets();
-    if (!NumSyms)
+  unsigned NumSyms = this->dumper()->dynamic_symbols().size();
-      return;
+  if (!NumSyms)
-    ArrayRef<Elf_Word> Chains = GnuHashTable.values(NumSyms);
+    return;
-    size_t Symndx = GnuHashTable.symndx;
+  ArrayRef<Elf_Word> Chains = GnuHashTable.values(NumSyms);
-    size_t TotalSyms = 0;
+  size_t Symndx = GnuHashTable.symndx;
-    size_t MaxChain = 1;
+  size_t TotalSyms = 0;
-    size_t CumulativeNonZero = 0;
+  size_t MaxChain = 1;
+  size_t CumulativeNonZero = 0;
 
-    if (Chains.empty() || NBucket == 0)
+  if (Chains.empty() || NBucket == 0)
-      return;
+    return;
 
-    std::vector<size_t> ChainLen(NBucket, 0);
+  std::vector<size_t> ChainLen(NBucket, 0);
+  for (size_t B = 0; B < NBucket; B++) {
+    if (!Buckets[B])
+      continue;
+    size_t Len = 1;
+    for (size_t C = Buckets[B] - Symndx;
+         C < Chains.size() && (Chains[C] & 1) == 0; C++)
+      if (MaxChain < ++Len)
+        MaxChain++;
+    ChainLen[B] = Len;
+    TotalSyms += Len;
+  }
+  MaxChain++;
 
-    for (size_t B = 0; B < NBucket; B++) {
+  if (!TotalSyms)
-      if (!Buckets[B])
+    return;
-        continue;
-      size_t Len = 1;
-      for (size_t C = Buckets[B] - Symndx;
-           C < Chains.size() && (Chains[C] & 1) == 0; C++)
-        if (MaxChain < ++Len)
-          MaxChain++;
-      ChainLen[B] = Len;
-      TotalSyms += Len;
-    }
-    MaxChain++;
 
-    if (!TotalSyms)
+  std::vector<size_t> Count(MaxChain, 0);
-      return;
+  for (size_t B = 0; B < NBucket; B++)
-
+    ++Count[ChainLen[B]];
-    std::vector<size_t> Count(MaxChain, 0);
+  // Print Number of buckets with each chain lengths and their cumulative
-    for (size_t B = 0; B < NBucket; B++)
+  // coverage of the symbols
-      ++Count[ChainLen[B]];
+  OS << "Histogram for `.gnu.hash' bucket list length (total of " << NBucket
-    // Print Number of buckets with each chain lengths and their cumulative
+     << " buckets)\n"
-    // coverage of the symbols
+     << " Length  Number     % of total  Coverage\n";
-    OS << "Histogram for `.gnu.hash' bucket list length (total of " << NBucket
+  for (size_t I = 0; I < MaxChain; I++) {
-       << " buckets)\n"
+    CumulativeNonZero += Count[I] * I;
-       << " Length  Number     % of total  Coverage\n";
+    OS << format("%7lu  %-10lu (%5.1f%%)     %5.1f%%\n", I, Count[I],
-    for (size_t I = 0; I < MaxChain; I++) {
+                 (Count[I] * 100.0) / NBucket,
-      CumulativeNonZero += Count[I] * I;
+                 (CumulativeNonZero * 100.0) / TotalSyms);
-      OS << format("%7lu  %-10lu (%5.1f%%)     %5.1f%%\n", I, Count[I],
+  }
-                   (Count[I] * 100.0) / NBucket,
+}
-                   (CumulativeNonZero * 100.0) / TotalSyms);
-    }
-  };
 
+// Hash histogram shows statistics of how efficient the hash was for the
+// dynamic symbol table. The table shows the number of hash buckets for
+// different lengths of chains as an absolute number and percentage of the total
+// buckets, and the cumulative coverage of symbols for each set of buckets.
+template <class ELFT>
+void GNUStyle<ELFT>::printHashHistograms(const ELFFile<ELFT> *Obj) {
   // Print histogram for the .hash section.
   if (const Elf_Hash *HashTable = this->dumper()->getHashTable())
     if (checkHashTable(Obj, HashTable, this->FileName))
-      PrintHashHist(*HashTable);
+      printHashHistogram(*HashTable);
 
   // Print histogram for the .gnu.hash section.
   if (const Elf_GnuHash *GnuHashTable = this->dumper()->getGnuHashTable())
-    PrintGnuHashHist(*GnuHashTable);
+    printGnuHashHistogram(*GnuHashTable);
 }
 
 template <class ELFT>
@@ -6417,7 +6421,7 @@ void LLVMStyle<ELFT>::printVersionDependencySection(const ELFFile<ELFT> *Obj,
 }
 
 template <class ELFT>
-void LLVMStyle<ELFT>::printHashHistogram(const ELFFile<ELFT> *Obj) {
+void LLVMStyle<ELFT>::printHashHistograms(const ELFFile<ELFT> *Obj) {
   W.startLine() << "Hash Histogram not implemented!\n";
 }
 

llvm/tools/llvm-readobj/ObjDumper.h

@@ -64,7 +64,7 @@ public:
   virtual void printLoadName() {}
   virtual void printVersionInfo() {}
   virtual void printGroupSections() {}
-  virtual void printHashHistogram() {}
+  virtual void printHashHistograms() {}
   virtual void printCGProfile() {}
   virtual void printAddrsig() {}
   virtual void printNotes() {}

llvm/tools/llvm-readobj/llvm-readobj.cpp

@@ -502,7 +502,7 @@ static void dumpObject(const ObjectFile *Obj, ScopedPrinter &Writer,
     if (opts::SectionGroups)
       Dumper->printGroupSections();
     if (opts::HashHistogram)
-      Dumper->printHashHistogram();
+      Dumper->printHashHistograms();
     if (opts::CGProfile)
       Dumper->printCGProfile();
     if (opts::Addrsig)