[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

161–162

186–194

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

[Esme]

Address comment.

[shchenz]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

163

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–6

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

40

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 | \
 # RUN:   FileCheck %s --strict-whitespace --match-full-lines --check-prefix=RELOCSEXP
-
-## TODO: Output of relocations not aligned.
+# RUN: llvm-readobj --relocs %t | \
+# RUN:   FileCheck %s --strict-whitespace --match-full-lines --check-prefix=RELOCS
 
 # RELOCSEXP:Relocations [
 # RELOCSEXP-NEXT:  Section (index: 1) .text {
-# RELOCSEXP-NEXT:  Relocation {
-# RELOCSEXP-NEXT:    Virtual Address: 0x80
-# RELOCSEXP-NEXT:    Symbol: foo (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: foo (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: bar (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: foo (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: foo (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: bar (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 foo(0) 0x15
+# RELOCS-NEXT:    0x100 R_REL foo(0) 0x14
+# RELOCS-NEXT:  }
+# RELOCS-NEXT:  Section (index: 2) .data {
+# RELOCS-NEXT:    0x200 R_TOC bar(1) 0x13
+# 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 printSectionHeaders(ArrayRef<T> Sections);
   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 RelTy> void printRelocation(RelTy Reloc);
   template <typename Shdr, typename RelTy>
   void printRelocations(ArrayRef<Shdr> Sections);
   void printAuxiliaryHeader(const XCOFFAuxiliaryHeader32 *AuxHeader);
   void printAuxiliaryHeader(const XCOFFAuxiliaryHeader64 *AuxHeader);
   const XCOFFObjectFile &Obj;
 };
 } // anonymous namespace
 
#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 << ") " << W.hex(Reloc.Info) << "\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())
       continue;
 
     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();
+
+    for (const RelTy Reloc : Relocations)
+      printRelocation(Reloc);
 
-      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));
-    }
     W.unindent();
     W.startLine() << "}\n";
   }
 }
 
 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
 };