[llvm-readobj] Output valid JSON for GroupSections

[paulkirth]

Summary

The current implementation output the LLVM formatted heading for group
sections, which was not valid JSON. This patch provides two small
customization points that allow the heading to vary between the two
implementations, and another that allows the section members to be
output as valid JSON objects.

[jhenderson]

Please provide before and after examples, and test cases for the fix. Also, please fix the title to use "llvm-readobj" in the commit message/patch description, not simply "readobj".

[paulkirth]

Rebase

• avoid duplication in original patch
• add tests to check the JSON and LLVM differences

[paulkirth]

Rebase to fix patch application

[paulkirth]

Rebase

[paulkirth]

Rebase

[paulkirth]

Rebase.

[paulkirth]

Rebase.

[paulkirth]

Rebase.

[paulkirth]

Rebase.

[jhenderson]

Looks essentially good. Just a few nits and my ongoing comment about testing.

llvm/test/tools/llvm-readobj/ELF/llvm-vs-json-format.test

414 ↗

(On Diff #500342)

As in the other cases, I think you're better off moving this test case into the standard test case for printing group sections.

llvm/tools/llvm-readobj/ELFDumper.cpp

722–725

I would consider reordering the order these methods are in. Specifically, getGroupSectionHeaderName is used before the others, so it should be first, and printEmptyGroupMessage is used last, so it should be last in order.

Also applies to the definition order and other places where the function overrides are declared.

752–753

[paulkirth]

Rebase. I had accidentally merged this into an earlier revision in the stack.

[paulkirth]

Reorder methods.

[jhenderson]

LGTM, with nit.

llvm/tools/llvm-readobj/ELFDumper.cpp

7777

Nit: don't have an extra blank line at EOF (the file should end with precisely one \n).

[paulkirth]

Remove newline at EOF.

[paulkirth]

Fix formatting

[paulkirth]

Rebase.

• remove const qualifiers from StringRef

[paulkirth]

Fix Rebase

[paulkirth]

arc diff git merge-base HEAD origin --update D137095

[paulkirth]

Rebase

llvm/test/tools/llvm-readobj/ELF/groups.test

 ## Check that llvm-readelf and llvm-readobj can
 ## dump SHT_GROUP sections properly.
 
 # RUN: yaml2obj %s -o %t.o
 # RUN: llvm-readobj -g %t.o | FileCheck %s
 # RUN: llvm-readobj --elf-section-groups %t.o | FileCheck %s
+# RUN: llvm-readobj --elf-section-groups %t.o --elf-output-style=JSON --pretty-print | FileCheck %s --check-prefix=JSON
 # RUN: llvm-readelf -g %t.o | FileCheck --check-prefix=GNU %s
 
 # CHECK:      Groups {
 # CHECK-NEXT:   Group {
 # CHECK-NEXT:     Name: .group
 # CHECK-NEXT:     Index: 1
 # CHECK-NEXT:     Link: 7
 # CHECK-NEXT:     Info: 1

 # GNU-NEXT:  [Index]  Name
 # GNU-NEXT:  [    3] .text.foo
 # GNU-NEXT:  [    4] .rela.text.foo
 # GNU:      COMDAT group section [    2] `.group1' [bar] contains 2 sections:
 # GNU-NEXT:  [Index]  Name
 # GNU-NEXT:  [    5] .text.bar
 # GNU-NEXT:  [    6] .rela.text.bar
 
+# JSON:      "Groups": {
+# JSON-NEXT:   "Group": {
+# JSON-NEXT:     "Name": {
+# JSON-NEXT:       "Name": ".group",
+# JSON-NEXT:       "Value": 16
+# JSON-NEXT:     },
+# JSON-NEXT:     "Index": 1,
+# JSON-NEXT:     "Link": 7,
+# JSON-NEXT:     "Info": 1,
+# JSON-NEXT:     "Type": {
+# JSON-NEXT:       "Name": "COMDAT",
+# JSON-NEXT:       "Value": 1
+# JSON-NEXT:     },
+# JSON-NEXT:     "Signature": "foo",
+# JSON-NEXT:     "GroupSections": [
+# JSON-NEXT:       {
+# JSON-NEXT:         "Name": ".text.foo",
+# JSON-NEXT:         "Index": 3
+# JSON-NEXT:       },
+# JSON-NEXT:       {
+# JSON-NEXT:         "Name": ".rela.text.foo",
+# JSON-NEXT:         "Index": 4
+# JSON-NEXT:       }
+# JSON-NEXT:     ]
+# JSON-NEXT:   },
+# JSON-NEXT:   "Group": {
+# JSON-NEXT:     "Name": {
+# JSON-NEXT:       "Name": ".group1",
+# JSON-NEXT:       "Value": 64
+# JSON-NEXT:     },
+# JSON-NEXT:     "Index": 2,
+# JSON-NEXT:     "Link": 7,
+# JSON-NEXT:     "Info": 2,
+# JSON-NEXT:     "Type": {
+# JSON-NEXT:       "Name": "COMDAT",
+# JSON-NEXT:       "Value": 1
+# JSON-NEXT:     },
+# JSON-NEXT:     "Signature": "bar",
+# JSON-NEXT:     "GroupSections": [
+# JSON-NEXT:       {
+# JSON-NEXT:         "Name": ".text.bar",
+# JSON-NEXT:         "Index": 5
+# JSON-NEXT:       },
+# JSON-NEXT:       {
+# JSON-NEXT:         "Name": ".rela.text.bar",
+# JSON-NEXT:         "Index": 6
+# JSON-NEXT:       }
+# JSON-NEXT:     ]
+# JSON-NEXT:   }
+# JSON-NEXT: }
+
 --- !ELF
 FileHeader:
   Class: ELFCLASS64
   Data:  ELFDATA2LSB
   Type:  ET_REL
 Sections:
   - Name: .group
     Type: SHT_GROUP

llvm/tools/llvm-readobj/ELFDumper.cpp

void printStackSizeEntry(uint64_t Size,
                            ArrayRef<std::string> FuncNames) override;
 
   void printMipsGOT(const MipsGOTParser<ELFT> &Parser) override;
   void printMipsPLT(const MipsGOTParser<ELFT> &Parser) override;
   void printMipsABIFlags() override;
   virtual void printZeroSymbolOtherField(const Elf_Sym &Symbol) const;
 
 protected:
+  virtual std::string getGroupSectionHeaderName() const;
   void printSymbolOtherField(const Elf_Sym &Symbol) const;
   virtual void printExpandedRelRelaReloc(const Relocation<ELFT> &R,
                                          StringRef SymbolName,
                                          StringRef RelocName);
   virtual void printDefaultRelRelaReloc(const Relocation<ELFT> &R,
                                         StringRef SymbolName,
                                         StringRef RelocName);
   virtual void printRelocationSectionInfo(const Elf_Shdr &Sec, StringRef Name,
                                           const unsigned SecNdx);
+  virtual void printSectionGroupMembers(StringRef Name, uint64_t Idx) const;
+  virtual void printEmptyGroupMessage() const;
 
   ScopedPrinter &W;
 };
 
 // JSONELFDumper shares most of the same implementation as LLVMELFDumper except
 // it uses a JSONScopedPrinter.
 template <typename ELFT> class JSONELFDumper : public LLVMELFDumper<ELFT> {
 public:
   LLVM_ELF_IMPORT_TYPES_ELFT(ELFT)
 
   JSONELFDumper(const object::ELFObjectFile<ELFT> &ObjF, ScopedPrinter &Writer)
       : LLVMELFDumper<ELFT>(ObjF, Writer) {}
 
+  std::string getGroupSectionHeaderName() const override;
+
   void printFileSummary(StringRef FileStr, ObjectFile &Obj,
                         ArrayRef<std::string> InputFilenames,
                         const Archive *A) override;
   virtual void printZeroSymbolOtherField(const Elf_Sym &Symbol) const override;
 
   void printDefaultRelRelaReloc(const Relocation<ELFT> &R,
                                 StringRef SymbolName,
                                 StringRef RelocName) override;
 
   void printRelocationSectionInfo(const Elf_Shdr &Sec, StringRef Name,
                                   const unsigned SecNdx) override;
 
-  void printRelocationSectionInfo(const Elf_Shdr &Sec, const StringRef Name,
-                                  const unsigned SecNdx) override;
+  void printSectionGroupMembers(StringRef Name, uint64_t Idx) const override;
+
+  void printEmptyGroupMessage() const override;
 
 private:
   std::unique_ptr<DictScope> FileScope;
 };
 
 } // end anonymous namespace
 
 namespace llvm {
template <class ELFT> void LLVMELFDumper<ELFT>::printGroupSections() {
   DenseMap<uint64_t, const GroupSection *> Map = mapSectionsToGroups(V);
   for (const GroupSection &G : V) {
     DictScope D(W, "Group");
     W.printNumber("Name", G.Name, G.ShName);
     W.printNumber("Index", G.Index);
     W.printNumber("Link", G.Link);
     W.printNumber("Info", G.Info);
     W.printHex("Type", getGroupType(G.Type), G.Type);
-    W.startLine() << "Signature: " << G.Signature << "\n";
+    W.printString("Signature", G.Signature);
 
-    ListScope L(W, "Section(s) in group");
+    ListScope L(W, getGroupSectionHeaderName());
     for (const GroupMember &GM : G.Members) {
       const GroupSection *MainGroup = Map[GM.Index];
       if (MainGroup != &G)
         this->reportUniqueWarning(
             "section with index " + Twine(GM.Index) +
             ", included in the group section with index " +
             Twine(MainGroup->Index) +
             ", was also found in the group section with index " +
             Twine(G.Index));
-      W.startLine() << GM.Name << " (" << GM.Index << ")\n";
+      printSectionGroupMembers(GM.Name, GM.Index);
     }
   }
 
   if (V.empty())
-    W.startLine() << "There are no group sections in the file.\n";
+    printEmptyGroupMessage();
+}
+
+template <class ELFT>
+std::string LLVMELFDumper<ELFT>::getGroupSectionHeaderName() const {
+  return "Section(s) in group";
+}
+
+template <class ELFT>
+void LLVMELFDumper<ELFT>::printSectionGroupMembers(StringRef Name,
+                                                   uint64_t Idx) const {
+  W.startLine() << Name << " (" << Idx << ")\n";
 }
 
 template <class ELFT> void LLVMELFDumper<ELFT>::printRelocations() {
   ListScope D(W, "Relocations");
 
   for (const Elf_Shdr &Sec : cantFail(this->Obj.sections())) {
     if (!isRelocationSec<ELFT>(Sec))
       continue;

 template <class ELFT>
 void LLVMELFDumper<ELFT>::printRelocationSectionInfo(const Elf_Shdr &Sec,
                                                      StringRef Name,
                                                      const unsigned SecNdx) {
   DictScope D(W, (Twine("Section (") + Twine(SecNdx) + ") " + Name).str());
   this->printRelocationsHelper(Sec);
 }
 
+template <class ELFT> void LLVMELFDumper<ELFT>::printEmptyGroupMessage() const {
+  W.startLine() << "There are no group sections in the file.\n";
+}
+
 template <class ELFT>
 void LLVMELFDumper<ELFT>::printRelRelaReloc(const Relocation<ELFT> &R,
                                             const RelSymbol<ELFT> &RelSym) {
   StringRef SymbolName = RelSym.Name;
   SmallString<32> RelocName;
   this->Obj.getRelocationTypeName(R.Type, RelocName);
 
   if (opts::ExpandRelocs) {

 void JSONELFDumper<ELFT>::printRelocationSectionInfo(const Elf_Shdr &Sec,
                                                      StringRef Name,
                                                      const unsigned SecNdx) {
   DictScope Group(this->W);
   this->W.printNumber("SectionIndex", SecNdx);
   ListScope D(this->W, "Relocs");
   this->printRelocationsHelper(Sec);
 }
+
+template <class ELFT>
+std::string JSONELFDumper<ELFT>::getGroupSectionHeaderName() const {
+  return "GroupSections";
+}
+
+template <class ELFT>
+void JSONELFDumper<ELFT>::printSectionGroupMembers(StringRef Name,
+                                                   uint64_t Idx) const {
+  DictScope Grp(this->W);
+  this->W.printString("Name", Name);
+  this->W.printNumber("Index", Idx);
+}
+
+template <class ELFT> void JSONELFDumper<ELFT>::printEmptyGroupMessage() const {
+  // JSON output does not need to print anything for empty groups
+}