[XCOFF] [llvm-readobj] improve the relocation output.

[Esme]

Summary

1. implemented the unexpanded relocations output.
2. modified the expanded output format to align.

[Higuoxing]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

149–150

173–184

I think we can simplify it by using index to iterate the relocation entries.

[Esme]

Address comment.

[shchenz]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

151

Unexpanded relocation entries should have 4 fields: vaddr/symndx(SymbolName)/rsize/rtype.

Expanded relocation entries should have 6fields: vaddr/symndx(SymbolName)/sign/fixup/len/type.

Maybe we should also contain rsize(which contains info for sign/fixup/len) in the unexpanded relocation info?

[Esme]

Dump Info for unexpended relocations.

[Higuoxing]

llvm/test/tools/llvm-readobj/XCOFF/relocations.test

4–5

Since you're fixing the indention issue, let's add --strict-whitespace to FileCheck

[Esme]

Rebase and address comments.

[jhenderson]

Generally looks fine to me, but best wait for someone with XCOFF knowledge to approve.

llvm/test/tools/llvm-readobj/XCOFF/relocations.test

38

See my comment in D111699 for explanation (you could fix the above check similarly in this patch, if you wanted to match).

[shchenz]

The relocation info looks good to me. I am going to approve this since @jhenderson now has a minor comment related to the test checks.

llvm/test/tools/llvm-readobj/XCOFF/relocations.test

 ## Test how relocations are dumped.
 
 # RUN: yaml2obj %s -o %t
 # RUN: llvm-readobj --relocs --expand-relocs %t | FileCheck %s --check-prefix=RELOCSEXP
-
-## TODO: Output of relocations not aligned.
+# RUN: llvm-readobj --relocs %t | FileCheck %s --check-prefix=RELOCS
 
 # RELOCSEXP:      Relocations [
 # RELOCSEXP-NEXT:   Section (index: 1) .text {
-# RELOCSEXP-NEXT:   Relocation {
-# RELOCSEXP-NEXT:     Virtual Address: 0x80
-# RELOCSEXP-NEXT:     Symbol: .text (0)
-# RELOCSEXP-NEXT:     IsSigned: No
-# RELOCSEXP-NEXT:     FixupBitValue: 0
-# RELOCSEXP-NEXT:     Length: 22
-# RELOCSEXP-NEXT:     Type: R_POS (0x0)
-# RELOCSEXP-NEXT:   }
-# RELOCSEXP-NEXT:   Relocation {
-# RELOCSEXP-NEXT:     Virtual Address: 0x100
-# RELOCSEXP-NEXT:     Symbol: .text (0)
-# RELOCSEXP-NEXT:     IsSigned: No
-# RELOCSEXP-NEXT:     FixupBitValue: 0
-# RELOCSEXP-NEXT:     Length: 21
-# RELOCSEXP-NEXT:     Type: R_REL (0x2)
-# RELOCSEXP-NEXT:   }
-# RELOCSEXP-NEXT: }
-# RELOCSEXP-NEXT: Section (index: 2) .data {
-# RELOCSEXP-NEXT: Relocation {
-# RELOCSEXP-NEXT:   Virtual Address: 0x200
-# RELOCSEXP-NEXT:   Symbol: .data (1)
-# RELOCSEXP-NEXT:   IsSigned: No
-# RELOCSEXP-NEXT:   FixupBitValue: 0
-# RELOCSEXP-NEXT:   Length: 20
-# RELOCSEXP-NEXT:   Type: R_TOC (0x3)
-# RELOCSEXP-NEXT: }
-# RELOCSEXP-NEXT: }
+# RELOCSEXP-NEXT:     Relocation {
+# RELOCSEXP-NEXT:       Virtual Address: 0x80
+# RELOCSEXP-NEXT:       Symbol: .text (0)
+# RELOCSEXP-NEXT:       IsSigned: No
+# RELOCSEXP-NEXT:       FixupBitValue: 0
+# RELOCSEXP-NEXT:       Length: 22
+# RELOCSEXP-NEXT:       Type: R_POS (0x0)
+# RELOCSEXP-NEXT:     }
+# RELOCSEXP-NEXT:     Relocation {
+# RELOCSEXP-NEXT:       Virtual Address: 0x100
+# RELOCSEXP-NEXT:       Symbol: .text (0)
+# RELOCSEXP-NEXT:       IsSigned: No
+# RELOCSEXP-NEXT:       FixupBitValue: 0
+# RELOCSEXP-NEXT:       Length: 21
+# RELOCSEXP-NEXT:       Type: R_REL (0x2)
+# RELOCSEXP-NEXT:     }
+# RELOCSEXP-NEXT:   }
+# RELOCSEXP-NEXT:   Section (index: 2) .data {
+# RELOCSEXP-NEXT:     Relocation {
+# RELOCSEXP-NEXT:       Virtual Address: 0x200
+# RELOCSEXP-NEXT:       Symbol: .data (1)
+# RELOCSEXP-NEXT:       IsSigned: No
+# RELOCSEXP-NEXT:       FixupBitValue: 0
+# RELOCSEXP-NEXT:       Length: 20
+# RELOCSEXP-NEXT:       Type: R_TOC (0x3)
+# RELOCSEXP-NEXT:     }
+# RELOCSEXP-NEXT:   }
 # RELOCSEXP-NEXT: ]
 
+# RELOCS:      Relocations [
+# RELOCS-NEXT:   Section (index: 1) .text {
+# RELOCS-NEXT:     0x80 R_POS .text(0)
+# RELOCS-NEXT:     0x100 R_REL .text(0)
+# RELOCS-NEXT:   }
+# RELOCS-NEXT:   Section (index: 2) .data {
+# RELOCS-NEXT:     0x200 R_TOC .data(1)
+# RELOCS-NEXT:   }
+# RELOCS-NEXT: ]
+
 --- !XCOFF
 FileHeader:
   MagicNumber: 0x01DF
 Sections:
   - Name: .text
     Flags: [ STYP_TEXT ]
     Relocations:
       - Address: 0x80

llvm/tools/llvm-readobj/XCOFFDumper.cpp

private:
   template <typename T> void printGenericSectionHeader(T &Sec) const;
   template <typename T> void printOverflowSectionHeader(T &Sec) const;
   void printFileAuxEnt(const XCOFFFileAuxEnt *AuxEntPtr);
   void printCsectAuxEnt(XCOFFCsectAuxRef AuxEntRef);
   void printSectAuxEntForStat(const XCOFFSectAuxEntForStat *AuxEntPtr);
   void printSymbol(const SymbolRef &);
   template <typename Shdr, typename RelTy>
   void printRelocations(ArrayRef<Shdr> Sections);
+  template <typename RelTy> void printRelocation(RelTy Reloc);
   const XCOFFObjectFile &Obj;
 };
 } // anonymous namespace
 
 void XCOFFDumper::printFileHeaders() {
   DictScope DS(W, "FileHeader");
   W.printHex("Magic", Obj.getMagic());
   W.printNumber("NumberOfSections", Obj.getNumberOfSections());
#define ECase(X)                                                               \
     ECase(R_REL),    ECase(R_TOC),    ECase(R_TRL),    ECase(R_TRLA),
     ECase(R_GL),     ECase(R_TCL),    ECase(R_REF),    ECase(R_BA),
     ECase(R_BR),     ECase(R_RBA),    ECase(R_RBR),    ECase(R_TLS),
     ECase(R_TLS_IE), ECase(R_TLS_LD), ECase(R_TLS_LE), ECase(R_TLSM),
     ECase(R_TLSML),  ECase(R_TOCU),   ECase(R_TOCL)
 #undef ECase
 };
 
+template <typename RelTy> void XCOFFDumper::printRelocation(RelTy Reloc) {
+  Expected<StringRef> ErrOrSymbolName =
+      Obj.getSymbolNameByIndex(Reloc.SymbolIndex);
+  if (Error E = ErrOrSymbolName.takeError()) {
+    reportUniqueWarning(std::move(E));
+    return;
+  }
+  StringRef SymbolName = *ErrOrSymbolName;
+  StringRef RelocName = XCOFF::getRelocationTypeString(Reloc.Type);
+  if (opts::ExpandRelocs) {
+    DictScope Group(W, "Relocation");
+    W.printHex("Virtual Address", Reloc.VirtualAddress);
+    W.printNumber("Symbol", SymbolName, Reloc.SymbolIndex);
+    W.printString("IsSigned", Reloc.isRelocationSigned() ? "Yes" : "No");
+    W.printNumber("FixupBitValue", Reloc.isFixupIndicated() ? 1 : 0);
+    W.printNumber("Length", Reloc.getRelocatedLength());
+    W.printEnum("Type", (uint8_t)Reloc.Type,
+                makeArrayRef(RelocationTypeNameclass));
+  } else {
+    raw_ostream &OS = W.startLine();
+    OS << W.hex(Reloc.VirtualAddress) << " " << RelocName << " " << SymbolName
+       << "(" << Reloc.SymbolIndex << ")\n";
+  }
+}
+
 template <typename Shdr, typename RelTy>
 void XCOFFDumper::printRelocations(ArrayRef<Shdr> Sections) {
-  if (!opts::ExpandRelocs)
-    report_fatal_error("Unexpanded relocation output not implemented.");
-
   ListScope LS(W, "Relocations");
   uint16_t Index = 0;
   for (const Shdr &Sec : Sections) {
     ++Index;
     // Only the .text, .data, .tdata, and STYP_DWARF sections have relocation.
     if (Sec.Flags != XCOFF::STYP_TEXT && Sec.Flags != XCOFF::STYP_DATA &&
         Sec.Flags != XCOFF::STYP_TDATA && Sec.Flags != XCOFF::STYP_DWARF)
       continue;
     Expected<ArrayRef<RelTy>> ErrOrRelocations = Obj.relocations<Shdr, RelTy>(Sec);
     if (Error E = ErrOrRelocations.takeError()) {
       reportUniqueWarning(std::move(E));
       continue;
     }
 
     const ArrayRef<RelTy> Relocations = *ErrOrRelocations;
-    if (Relocations.empty())
+    size_t RelocCount = Relocations.size();
+    if (!RelocCount)
       continue;
 
+    for (size_t I = 0; I < RelocCount; ++I) {
+      if (I == 0) {
         W.startLine() << "Section (index: " << Index << ") " << Sec.getName()
                       << " {\n";
-    for (const RelTy Reloc : Relocations) {
-      Expected<StringRef> ErrOrSymbolName =
-          Obj.getSymbolNameByIndex(Reloc.SymbolIndex);
-      if (Error E = ErrOrSymbolName.takeError()) {
-        reportUniqueWarning(std::move(E));
-        continue;
+        W.indent();
       }
-
-      StringRef SymbolName = *ErrOrSymbolName;
-      DictScope RelocScope(W, "Relocation");
-      W.printHex("Virtual Address", Reloc.VirtualAddress);
-      W.printNumber("Symbol", SymbolName, Reloc.SymbolIndex);
-      W.printString("IsSigned", Reloc.isRelocationSigned() ? "Yes" : "No");
-      W.printNumber("FixupBitValue", Reloc.isFixupIndicated() ? 1 : 0);
-      W.printNumber("Length", Reloc.getRelocatedLength());
-      W.printEnum("Type", (uint8_t)Reloc.Type,
-                  makeArrayRef(RelocationTypeNameclass));
+      printRelocation(Relocations[I]);
     }
     W.unindent();
     W.startLine() << "}\n";
   }
 }
 
 static const EnumEntry<XCOFF::CFileStringType> FileStringType[] = {
 #define ECase(X)                                                               \
   { #X, XCOFF::X }
     ECase(XFT_FN), ECase(XFT_CT), ECase(XFT_CV), ECase(XFT_CD)
 #undef ECase
 };