[AIX][XCOFF] 64-bit relocation reading support

[MaryamBen]

Summary

Support XCOFFDumper relocation reading support
This patch is part of D103696 partition

[jhenderson]

For a patch advertised as llvm-readobj relocation dumping support, I wouldn't expect to see test changes in the CodeGen directory.

If you need the llvm-readobj changes to test some other functionality in other locations, you should make the llvm-readobj patch first, and then add testing for the other part in a separate patch.

I've said this elsewhere on XCOFF patches, but I really don't like the sort of duplication that the 32-bit/64-bit implementations in the ObjectFile code have. It seems to me like you should be able to avoid it by some additional abstraction. Take a look at the equivalent ELF code - there we use templates to avoid needing to duplicate most code, and/or common objects that hide the details from the user.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

136

These changes to use warnings are good, but should be their own separate patch, along with appropriate testing in tools/llvm-readobj.

142

It's not clear to me what the type of Relocations is here, and similarly the ErrOrRelocations above. Don't use auto unless the type is obvious from the RHS (or for some iterator cases). See the Coding Standards for details.

148–154

Ditto.

[MaryamBen]

I kept one test from llvm-objdump. Is it okey, or I should move it to another patch?

[jhenderson]

llvm/lib/Object/XCOFFObjectFile.cpp

359

I believe this will cause an assertion in some configurations because you don't do anything with a failing error. You should either report it, or more likely in this case, use consumeError() with a TODO saying the error should be propagated up the stack, to be reported elsewhere.

You also need to make sure you've got a test case that triggers this error. Whether it is a lit test or a gtest unit test, I don't mind, although I suspect the latter may be easier.

728–729

Perhaps name these types more descriptively, e.g. Shdr and Reloc.

743

It would be nice if this static assert included a message that the compiler can properly report.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

125–126

As elsewhere, consider renaming these types to be more descriptive.

[jhenderson]

In D104646#2840670, @MaryamBen wrote:

I kept one test from llvm-objdump. Is it okey, or I should move it to another patch?

The problem is that you label this patch as "llvm-readobj 64-bit relocation reading support", which suggests this is specific to llvm-readobj. You shouldn't conflate llvm-readobj and llvm-objdump. If the changes in the patch mean that llvm-objdump starts to just work, then consider updating the patch summary.

[jhenderson]

llvm/lib/Object/XCOFFObjectFile.cpp

334

I'd add a TODO to say to report the error up the stack. Same below.

In general, consumeError calls should be accompanied with a comment saying why it's okay to throw away the error. In this case, it's just because the interface doesn't currently support reporting the error, hence the TODO to improve the situation.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

148–152

Or something similar - "Rel" could easily be confused with the variable name. I'd probably change the other instanecs in a similar manner too.

[MaryamBen]

Thank you for your valuable comments

[jhenderson]

llvm/lib/Object/XCOFFObjectFile.cpp

334

Nit: The common pattern is // TODO: <some text>. (note the colon, also it's a comment, so should have a trailing '.').

Same below.

[MaryamBen]

Any news regarding the patch?

[jhenderson]

Apologies, this patch somehow dropped off my radar. I've still got one outstanding comment (move the error to warning switch to a prerequisite patch). Beyond that, this should probably get another pair of eyes on it, ideally from someone with XCOFF experience. I don't feel comfortable approving it myself.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

136

It looks like this comment has been ignored? Or does this patch need rebasing on a different patch that addresses this point?

[MaryamBen]

I created another patch to replace unwrapOrError with reportUniqueWarning.
Could you suggest someone who can take a look on it ?
Thank you

[jhenderson]

In D104646#2903848, @MaryamBen wrote:

I created another patch to replace unwrapOrError with reportUniqueWarning.
Could you suggest someone who can take a look on it ?

I assume you mean this patch (I'll take a look at the unwrapOrError one)? Take a look at who has made changes to the XCOFF code up to now, and add them as reviewers.

[jhenderson]

Spotted one more case that needs a test.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

108

This needs a test.

[MaryamBen]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

108

It's checked in D104913, since you have asked to put the tests from the CodeGen directory in a different patch.

[Helflym]

Ok for the XCOFF part

[daltenty]

nit: the existing relocation test llvm/test/tools/llvm-objdump/XCOFF/print-reloc.test could probably have the 64-bit cases added now, but that can easily be handled in a follow on patch if required

[daltenty]

LGTM, apart from minor nits (and possibly follow on tests). I can assist with committing this once you're ready.

llvm/lib/Object/XCOFFObjectFile.cpp

714

nit: maybe we'd benefit from an assert or some other way to make sure the type T and the bitmode agree?

[sfertile]

llvm/lib/Object/XCOFFObjectFile.cpp

714

This is a good suggestion. As an alternative (and it can come as a post commit NFC patch if need be) would it make sense to specialize this for both valid section header types since the implementation is not the same for 32-bit vs 64-bit?

[daltenty]

We'll follow up with the rest of the minor suggestions here in a NFC follow-on, since the author will be unavailable after today and these are minor cleanups.

[MaryamBen]

LGTM, apart from minor nits (and possibly follow on tests). I can assist with committing this once you're ready.

I'm sorry, I just saw your comment.

We'll follow up with the rest of the minor suggestions here in a NFC follow-on, since the author will be unavailable after today and these are minor cleanups.

Thank you

llvm/include/llvm/Object/XCOFFObjectFile.h

 
 struct XCOFFSectAuxEntForStat {
   support::ubig32_t SectionLength;
   support::ubig16_t NumberOfRelocEnt;
   support::ubig16_t NumberOfLineNum;
   uint8_t Pad[10];
 }; // 32-bit XCOFF file only.
 
-struct XCOFFRelocation32 {
+template <typename AddressType> struct XCOFFRelocation {
   // Masks for packing/unpacking the r_rsize field of relocations.
 
   // The msb is used to indicate if the bits being relocated are signed or
   // unsigned.
   static constexpr uint8_t XR_SIGN_INDICATOR_MASK = 0x80;
 
   // The 2nd msb is used to indicate that the binder has replaced/modified the
   // original instruction.
   static constexpr uint8_t XR_FIXUP_INDICATOR_MASK = 0x40;
 
   // The remaining bits specify the bit length of the relocatable reference
   // minus one.
   static constexpr uint8_t XR_BIASED_LENGTH_MASK = 0x3f;
 
 public:
-  support::ubig32_t VirtualAddress;
+  AddressType VirtualAddress;
   support::ubig32_t SymbolIndex;
 
   // Packed field, see XR_* masks for details of packing.
   uint8_t Info;
 
   XCOFF::RelocationType Type;
 
 public:
   bool isRelocationSigned() const;
   bool isFixupIndicated() const;
 
   // Returns the number of bits being relocated.
   uint8_t getRelocatedLength() const;
 };
 
+extern template struct XCOFFRelocation<llvm::support::ubig32_t>;
+extern template struct XCOFFRelocation<llvm::support::ubig64_t>;
+
+struct XCOFFRelocation32 : XCOFFRelocation<llvm::support::ubig32_t> {};
+struct XCOFFRelocation64 : XCOFFRelocation<llvm::support::ubig64_t> {};
+
 class XCOFFSymbolRef;
 
 class XCOFFObjectFile : public ObjectFile {
 private:
   const void *FileHeader = nullptr;
   const void *SectionHeaderTable = nullptr;
 
   const void *SymbolTblPtr = nullptr;
   XCOFFStringTable StringTable = {0, nullptr};
 
   const XCOFFFileHeader32 *fileHeader32() const;
   const XCOFFFileHeader64 *fileHeader64() const;
 
   const XCOFFSectionHeader32 *sectionHeaderTable32() const;
   const XCOFFSectionHeader64 *sectionHeaderTable64() const;
+  template <typename T> const T *sectionHeaderTable() const;
 
   size_t getFileHeaderSize() const;
   size_t getSectionHeaderSize() const;
 
   const XCOFFSectionHeader32 *toSection32(DataRefImpl Ref) const;
   const XCOFFSectionHeader64 *toSection64(DataRefImpl Ref) const;
   uintptr_t getSectionHeaderTableAddress() const;
   uintptr_t getEndOfSymbolTableAddress() const;
public:
   ArrayRef<XCOFFSectionHeader64> sections64() const;
 
   int32_t getSectionFlags(DataRefImpl Sec) const;
   Expected<DataRefImpl> getSectionByNum(int16_t Num) const;
 
   void checkSymbolEntryPointer(uintptr_t SymbolEntPtr) const;
 
   // Relocation-related interfaces.
+  template <typename T>
   Expected<uint32_t>
-  getLogicalNumberOfRelocationEntries(const XCOFFSectionHeader32 &Sec) const;
+  getNumberOfRelocationEntries(const XCOFFSectionHeader<T> &Sec) const;
 
-  Expected<ArrayRef<XCOFFRelocation32>>
-  relocations(const XCOFFSectionHeader32 &) const;
+  template <typename Shdr, typename Reloc>
+  Expected<ArrayRef<Reloc>> relocations(const Shdr &Sec) const;
 
   // This function returns string table entry.
   Expected<StringRef> getStringTableEntry(uint32_t Offset) const;
 
   const XCOFF::SymbolAuxType *getSymbolAuxType(uintptr_t AuxEntryAddress) const;
 
   static uintptr_t getAdvancedSymbolEntryAddress(uintptr_t CurrentAddress,
                                                  uint32_t Distance);
class XCOFFTracebackTable {
   Optional<uint32_t> NumOfCtlAnchors;
   Optional<SmallVector<uint32_t, 8>> ControlledStorageInfoDisp;
   Optional<StringRef> FunctionName;
   Optional<uint8_t> AllocaRegister;
   Optional<TBVectorExt> VecExt;
   Optional<uint8_t> ExtensionTable;
 
   XCOFFTracebackTable(const uint8_t *Ptr, uint64_t &Size, Error &Err);
+
 public:
   /// Parse an XCOFF Traceback Table from \a Ptr with \a Size bytes.
   /// Returns an XCOFFTracebackTable upon successful parsing, otherwise an
   /// Error is returned.
   ///
   /// \param[in] Ptr
   ///   A pointer that points just past the initial 4 bytes of zeros at the
   ///   beginning of an XCOFF Traceback Table.

llvm/lib/Object/XCOFFObjectFile.cpp

template <typename T> uint16_t XCOFFSectionHeader<T>::getSectionType() const {
   return DerivedXCOFFSectionHeader.Flags & SectionFlagsTypeMask;
 }
 
 template <typename T>
 bool XCOFFSectionHeader<T>::isReservedSectionType() const {
   return getSectionType() & SectionFlagsReservedMask;
 }
 
-bool XCOFFRelocation32::isRelocationSigned() const {
+template <typename AddressType>
+bool XCOFFRelocation<AddressType>::isRelocationSigned() const {
   return Info & XR_SIGN_INDICATOR_MASK;
 }
 
-bool XCOFFRelocation32::isFixupIndicated() const {
+template <typename AddressType>
+bool XCOFFRelocation<AddressType>::isFixupIndicated() const {
   return Info & XR_FIXUP_INDICATOR_MASK;
 }
 
-uint8_t XCOFFRelocation32::getRelocatedLength() const {
+template <typename AddressType>
+uint8_t XCOFFRelocation<AddressType>::getRelocatedLength() const {
   // The relocation encodes the bit length being relocated minus 1. Add back
   // the 1 to get the actual length being relocated.
   return (Info & XR_BIASED_LENGTH_MASK) + 1;
 }
 
 uintptr_t
 XCOFFObjectFile::getAdvancedSymbolEntryAddress(uintptr_t CurrentAddress,
                                                uint32_t Distance) {
const XCOFFFileHeader32 *XCOFFObjectFile::fileHeader32() const {
   return static_cast<const XCOFFFileHeader32 *>(FileHeader);
 }
 
 const XCOFFFileHeader64 *XCOFFObjectFile::fileHeader64() const {
   assert(is64Bit() && "64-bit interface called on a 32-bit object file.");
   return static_cast<const XCOFFFileHeader64 *>(FileHeader);
 }
 
+template <typename T> const T *XCOFFObjectFile::sectionHeaderTable() const {
+  return static_cast<const T *>(SectionHeaderTable);
+}
+
 const XCOFFSectionHeader32 *
 XCOFFObjectFile::sectionHeaderTable32() const {
   assert(!is64Bit() && "32-bit interface called on 64-bit object file.");
   return static_cast<const XCOFFSectionHeader32 *>(SectionHeaderTable);
 }
 
 const XCOFFSectionHeader64 *
 XCOFFObjectFile::sectionHeaderTable64() const {

 }
 
 bool XCOFFObjectFile::isSectionVirtual(DataRefImpl Sec) const {
   return is64Bit() ? toSection64(Sec)->FileOffsetToRawData == 0
                    : toSection32(Sec)->FileOffsetToRawData == 0;
 }
 
 relocation_iterator XCOFFObjectFile::section_rel_begin(DataRefImpl Sec) const {
-  if (is64Bit())
-    report_fatal_error("64-bit support not implemented yet");
+  DataRefImpl Ret;
+  if (is64Bit()) {
+    const XCOFFSectionHeader64 *SectionEntPtr = toSection64(Sec);
+    auto RelocationsOrErr =
+        relocations<XCOFFSectionHeader64, XCOFFRelocation64>(*SectionEntPtr);
+    if (Error E = RelocationsOrErr.takeError()) {
+      // TODO report the error up the stack
+      consumeError(std::move(E));
+      return relocation_iterator(RelocationRef());
+    }
+    Ret.p = reinterpret_cast<uintptr_t>(&*RelocationsOrErr.get().begin());
+  } else {
     const XCOFFSectionHeader32 *SectionEntPtr = toSection32(Sec);
-  auto RelocationsOrErr = relocations(*SectionEntPtr);
-  if (Error E = RelocationsOrErr.takeError())
+    auto RelocationsOrErr =
+        relocations<XCOFFSectionHeader32, XCOFFRelocation32>(*SectionEntPtr);
+    if (Error E = RelocationsOrErr.takeError()) {
+      // TODO report the error up the stack
+      consumeError(std::move(E));
       return relocation_iterator(RelocationRef());
-  DataRefImpl Ret;
+    }
     Ret.p = reinterpret_cast<uintptr_t>(&*RelocationsOrErr.get().begin());
+  }
   return relocation_iterator(RelocationRef(Ret, this));
 }
 
 relocation_iterator XCOFFObjectFile::section_rel_end(DataRefImpl Sec) const {
-  if (is64Bit())
-    report_fatal_error("64-bit support not implemented yet");
+  DataRefImpl Ret;
+  if (is64Bit()) {
+    const XCOFFSectionHeader64 *SectionEntPtr = toSection64(Sec);
+    auto RelocationsOrErr =
+        relocations<XCOFFSectionHeader64, XCOFFRelocation64>(*SectionEntPtr);
+    if (Error E = RelocationsOrErr.takeError()) {
+      // TODO report the error up the stack
+      consumeError(std::move(E));
+      return relocation_iterator(RelocationRef());
+    }
+    Ret.p = reinterpret_cast<uintptr_t>(&*RelocationsOrErr.get().end());
+  } else {
     const XCOFFSectionHeader32 *SectionEntPtr = toSection32(Sec);
-  auto RelocationsOrErr = relocations(*SectionEntPtr);
-  if (Error E = RelocationsOrErr.takeError())
+    auto RelocationsOrErr =
+        relocations<XCOFFSectionHeader32, XCOFFRelocation32>(*SectionEntPtr);
+    if (Error E = RelocationsOrErr.takeError()) {
+      // TODO report the error up the stack
+      consumeError(std::move(E));
       return relocation_iterator(RelocationRef());
-  DataRefImpl Ret;
+    }
     Ret.p = reinterpret_cast<uintptr_t>(&*RelocationsOrErr.get().end());
+  }
   return relocation_iterator(RelocationRef(Ret, this));
 }
 
 void XCOFFObjectFile::moveRelocationNext(DataRefImpl &Rel) const {
+  if (is64Bit())
+    Rel.p = reinterpret_cast<uintptr_t>(viewAs<XCOFFRelocation64>(Rel.p) + 1);
+  else
     Rel.p = reinterpret_cast<uintptr_t>(viewAs<XCOFFRelocation32>(Rel.p) + 1);
 }
 
 uint64_t XCOFFObjectFile::getRelocationOffset(DataRefImpl Rel) const {
-  if (is64Bit())
-    report_fatal_error("64-bit support not implemented yet");
+  if (is64Bit()) {
+    const XCOFFRelocation64 *Reloc = viewAs<XCOFFRelocation64>(Rel.p);
+    const XCOFFSectionHeader64 *Sec64 = sectionHeaderTable64();
+    const uint64_t RelocAddress = Reloc->VirtualAddress;
+    const uint16_t NumberOfSections = getNumberOfSections();
+    for (uint16_t I = 0; I < NumberOfSections; ++I) {
+      // Find which section this relocation belongs to, and get the
+      // relocation offset relative to the start of the section.
+      if (Sec64->VirtualAddress <= RelocAddress &&
+          RelocAddress < Sec64->VirtualAddress + Sec64->SectionSize) {
+        return RelocAddress - Sec64->VirtualAddress;
+      }
+      ++Sec64;
+    }
+  } else {
     const XCOFFRelocation32 *Reloc = viewAs<XCOFFRelocation32>(Rel.p);
     const XCOFFSectionHeader32 *Sec32 = sectionHeaderTable32();
     const uint32_t RelocAddress = Reloc->VirtualAddress;
     const uint16_t NumberOfSections = getNumberOfSections();
-  for (uint16_t i = 0; i < NumberOfSections; ++i) {
-    // Find which section this relocation is belonging to, and get the
+    for (uint16_t I = 0; I < NumberOfSections; ++I) {
+      // Find which section this relocation belongs to, and get the
       // relocation offset relative to the start of the section.
       if (Sec32->VirtualAddress <= RelocAddress &&
           RelocAddress < Sec32->VirtualAddress + Sec32->SectionSize) {
         return RelocAddress - Sec32->VirtualAddress;
       }
       ++Sec32;
     }
+  }
   return InvalidRelocOffset;
 }
 
 symbol_iterator XCOFFObjectFile::getRelocationSymbol(DataRefImpl Rel) const {
-  if (is64Bit())
-    report_fatal_error("64-bit support not implemented yet");
+  uint32_t Index;
+  if (is64Bit()) {
+    const XCOFFRelocation64 *Reloc = viewAs<XCOFFRelocation64>(Rel.p);
+    Index = Reloc->SymbolIndex;
+
+    if (Index >= getNumberOfSymbolTableEntries64())
+      return symbol_end();
+  } else {
     const XCOFFRelocation32 *Reloc = viewAs<XCOFFRelocation32>(Rel.p);
-  const uint32_t Index = Reloc->SymbolIndex;
+    Index = Reloc->SymbolIndex;
 
     if (Index >= getLogicalNumberOfSymbolTableEntries32())
       return symbol_end();
-
+  }
   DataRefImpl SymDRI;
   SymDRI.p = getSymbolEntryAddressByIndex(Index);
   return symbol_iterator(SymbolRef(SymDRI, this));
 }
 
 uint64_t XCOFFObjectFile::getRelocationType(DataRefImpl Rel) const {
   if (is64Bit())
-    report_fatal_error("64-bit support not implemented yet");
+    return viewAs<XCOFFRelocation64>(Rel.p)->Type;
   return viewAs<XCOFFRelocation32>(Rel.p)->Type;
 }
 
 void XCOFFObjectFile::getRelocationTypeName(
     DataRefImpl Rel, SmallVectorImpl<char> &Result) const {
-  if (is64Bit())
-    report_fatal_error("64-bit support not implemented yet");
+  StringRef Res;
+  if (is64Bit()) {
+    const XCOFFRelocation64 *Reloc = viewAs<XCOFFRelocation64>(Rel.p);
+    Res = XCOFF::getRelocationTypeString(Reloc->Type);
+  } else {
     const XCOFFRelocation32 *Reloc = viewAs<XCOFFRelocation32>(Rel.p);
-  StringRef Res = XCOFF::getRelocationTypeString(Reloc->Type);
+    Res = XCOFF::getRelocationTypeString(Reloc->Type);
+  }
   Result.append(Res.begin(), Res.end());
 }
 
 Expected<uint32_t> XCOFFObjectFile::getSymbolFlags(DataRefImpl Symb) const {
   uint32_t Result = 0;
   llvm_unreachable("Not yet implemented!");
   return Result;
 }

 }
 
 SubtargetFeatures XCOFFObjectFile::getFeatures() const {
   return SubtargetFeatures();
 }
 
 bool XCOFFObjectFile::isRelocatableObject() const {
   if (is64Bit())
-    report_fatal_error("64-bit support not implemented yet");
+    return !(fileHeader64()->Flags & NoRelMask);
   return !(fileHeader32()->Flags & NoRelMask);
 }
 
 Expected<uint64_t> XCOFFObjectFile::getStartAddress() const {
   // TODO FIXME Should get from auxiliary_header->o_entry when support for the
   // auxiliary_header is added.
   return 0;
 }
ArrayRef<XCOFFSectionHeader32> XCOFFObjectFile::sections32() const {
   return ArrayRef<XCOFFSectionHeader32>(TablePtr,
                                         TablePtr + getNumberOfSections());
 }
 
 // In an XCOFF32 file, when the field value is 65535, then an STYP_OVRFLO
 // section header contains the actual count of relocation entries in the s_paddr
 // field. STYP_OVRFLO headers contain the section index of their corresponding
 // sections as their raw "NumberOfRelocations" field value.
-Expected<uint32_t> XCOFFObjectFile::getLogicalNumberOfRelocationEntries(
-    const XCOFFSectionHeader32 &Sec) const {
-
-  uint16_t SectionIndex = &Sec - sectionHeaderTable32() + 1;
+template <typename T>
+Expected<uint32_t> XCOFFObjectFile::getNumberOfRelocationEntries(
+    const XCOFFSectionHeader<T> &Sec) const {
+  const T &Section = static_cast<const T &>(Sec);
+  if (is64Bit())
+    return Section.NumberOfRelocations;
 
-  if (Sec.NumberOfRelocations < XCOFF::RelocOverflow)
-    return Sec.NumberOfRelocations;
+  uint16_t SectionIndex = &Section - sectionHeaderTable<T>() + 1;
+  if (Section.NumberOfRelocations < XCOFF::RelocOverflow)
+    return Section.NumberOfRelocations;
   for (const auto &Sec : sections32()) {
     if (Sec.Flags == XCOFF::STYP_OVRFLO &&
         Sec.NumberOfRelocations == SectionIndex)
       return Sec.PhysicalAddress;
   }
   return errorCodeToError(object_error::parse_failed);
 }
 
-Expected<ArrayRef<XCOFFRelocation32>>
-XCOFFObjectFile::relocations(const XCOFFSectionHeader32 &Sec) const {
+template <typename Shdr, typename Reloc>
+Expected<ArrayRef<Reloc>> XCOFFObjectFile::relocations(const Shdr &Sec) const {
   uintptr_t RelocAddr = getWithOffset(reinterpret_cast<uintptr_t>(FileHeader),
                                       Sec.FileOffsetToRelocationInfo);
-  auto NumRelocEntriesOrErr = getLogicalNumberOfRelocationEntries(Sec);
+  auto NumRelocEntriesOrErr = getNumberOfRelocationEntries(Sec);
   if (Error E = NumRelocEntriesOrErr.takeError())
     return std::move(E);
 
   uint32_t NumRelocEntries = NumRelocEntriesOrErr.get();
-
-  static_assert(
-      sizeof(XCOFFRelocation32) == XCOFF::RelocationSerializationSize32, "");
+  static_assert((sizeof(Reloc) == XCOFF::RelocationSerializationSize64 ||
+                 sizeof(Reloc) == XCOFF::RelocationSerializationSize32),
+                "Relocation structure is incorrect");
   auto RelocationOrErr =
-      getObject<XCOFFRelocation32>(Data, reinterpret_cast<void *>(RelocAddr),
-                                   NumRelocEntries * sizeof(XCOFFRelocation32));
+      getObject<Reloc>(Data, reinterpret_cast<void *>(RelocAddr),
+                       NumRelocEntries * sizeof(Reloc));
   if (Error E = RelocationOrErr.takeError())
     return std::move(E);
 
-  const XCOFFRelocation32 *StartReloc = RelocationOrErr.get();
+  const Reloc *StartReloc = RelocationOrErr.get();
 
-  return ArrayRef<XCOFFRelocation32>(StartReloc, StartReloc + NumRelocEntries);
+  return ArrayRef<Reloc>(StartReloc, StartReloc + NumRelocEntries);
 }
 
 Expected<XCOFFStringTable>
 XCOFFObjectFile::parseStringTable(const XCOFFObjectFile *Obj, uint64_t Offset) {
   // If there is a string table, then the buffer must contain at least 4 bytes
   // for the string table's size. Not having a string table is not an error.
   if (Error E = Binary::checkOffset(
           Obj->Data, reinterpret_cast<uintptr_t>(Obj->base() + Offset), 4)) {
Expected<StringRef> XCOFFSymbolRef::getName() const {
 
   return OwningObjectPtr->getStringTableEntry(Entry64->Offset);
 }
 
 // Explictly instantiate template classes.
 template struct XCOFFSectionHeader<XCOFFSectionHeader32>;
 template struct XCOFFSectionHeader<XCOFFSectionHeader64>;
 
+template struct XCOFFRelocation<llvm::support::ubig32_t>;
+template struct XCOFFRelocation<llvm::support::ubig64_t>;
+
+template llvm::Expected<llvm::ArrayRef<llvm::object::XCOFFRelocation64>>
+llvm::object::XCOFFObjectFile::relocations<llvm::object::XCOFFSectionHeader64,
+                                           llvm::object::XCOFFRelocation64>(
+    llvm::object::XCOFFSectionHeader64 const &) const;
+template llvm::Expected<llvm::ArrayRef<llvm::object::XCOFFRelocation32>>
+llvm::object::XCOFFObjectFile::relocations<llvm::object::XCOFFSectionHeader32,
+                                           llvm::object::XCOFFRelocation32>(
+    llvm::object::XCOFFSectionHeader32 const &) const;
+
 bool doesXCOFFTracebackTableBegin(ArrayRef<uint8_t> Bytes) {
   if (Bytes.size() < 4)
     return false;
 
   return support::endian::read32be(Bytes.data()) == 0;
 }
 
 TBVectorExt::TBVectorExt(StringRef TBvectorStrRef) {

llvm/test/tools/llvm-objdump/XCOFF/disassemble-symbol-description64.test

 # REQUIRES: powerpc-registered-target
 
 # RUN: llvm-objdump -D %p/Inputs/xcoff-section-headers64.o | \
 # RUN:   FileCheck --check-prefixes=COMMON,PLAIN %s
 
 # RUN: llvm-objdump -D --symbol-description %p/Inputs/xcoff-section-headers64.o | \
 # RUN:   FileCheck --check-prefixes=COMMON,DESC %s
 
-# RUN: not --crash llvm-objdump -D -r --symbol-description %p/Inputs/xcoff-section-headers64.o 2>&1 | \
-# RUN:   FileCheck --check-prefix=ERROR %s
-# ERROR: 64-bit support not implemented yet
+# RUN: llvm-objdump -D -r --symbol-description %p/Inputs/xcoff-section-headers64.o | \
+# RUN:   FileCheck --check-prefixes=COMMON,DESC,RELOC,COMMON-RELOC %s
 
 ## xcoff-section-headers64.o Compiled with IBM XL C/C++ for AIX, V16.1.0
 ## compiler command: xlc -q64 -qtls -o xcoff-section-headers64.o -c test.c
 
 ## test.c:
 ## int a;
 ## int b = 12345;
 ## __thread int c;
 ## __thread double d = 3.14159;
 ##
 ## int func(void)  {
 ##   return a;
 ## }
 
 COMMON:       Inputs/xcoff-section-headers64.o: file format aix5coff64-rs6000
 COMMON:       Disassembly of section .text:
 COMMON-EMPTY:
 PLAIN:        0000000000000000 <.func>:
 DESC:         0000000000000000 (idx: 6) .func:
 COMMON-NEXT:         0: e8 62 00 08   ld 3, 8(2)
+RELOC-NEXT:                           00000002:  R_TOC        (idx: 16) a[TC]
 COMMON-NEXT:         4: e8 63 00 02   lwa 3, 0(3)
 COMMON-NEXT:         8: 4e 80 00 20   blr
 COMMON-NEXT:         c: 00 00 00 00   <unknown>
 COMMON-NEXT:        10: 00 00 20 40   <unknown>
 COMMON-NEXT:        14: 00 00 00 01   <unknown>
 COMMON-NEXT:        18: 00 00 00 0c   <unknown>
 COMMON-NEXT:        1c: 00 04 66 75   <unknown>
 COMMON-NEXT:        20: 6e 63 00 00   xoris 3, 19, 0
 COMMON-NEXT:                  ...
 COMMON-EMPTY:
 COMMON-NEXT:  Disassembly of section .data:
 COMMON-EMPTY:
 PLAIN:        0000000000000080 <func>:
 DESC:         0000000000000080 (idx: 12) func[TC]:
 COMMON-NEXT:        80: 00 00 00 00   <unknown>
+RELOC-NEXT:                           00000080:  R_POS       (idx: 10) func[DS]
 COMMON-NEXT:        84: 00 00 00 a8   <unknown>
 COMMON-EMPTY:
 PLAIN:        0000000000000088 <a>:
 DESC:         0000000000000088 (idx: 16) a[TC]:
 COMMON-NEXT:        88: 00 00 00 00   <unknown>
+RELOC-NEXT:                            00000088:  R_POS       (idx: 14) a[RW]
 COMMON-NEXT:        8c: 00 00 00 c8   <unknown>
 COMMON-EMPTY:
 PLAIN:        0000000000000090 <b>:
 DESC:         0000000000000090 (idx: 20) b[TC]:
 COMMON-NEXT:        90: 00 00 00 00   <unknown>
+RELOC-NEXT:                            00000090:  R_POS       (idx: 18) b[RW]
 COMMON-NEXT:        94: 00 00 00 c0   <unknown>
 COMMON-EMPTY:
 PLAIN:        0000000000000098 <c>:
 DESC:         0000000000000098 (idx: 24) c[TC]:
 COMMON-NEXT:        98: 00 00 00 00   <unknown>
+RELOC-NEXT:                            00000098:  R_TLS       (idx: 22) c[UL]
 COMMON-NEXT:        9c: 00 00 00 08   <unknown>
 COMMON-EMPTY:
 PLAIN:        00000000000000a0 <d>:
 DESC:         00000000000000a0 (idx: 28) d[TC]:
-COMMON-NEXT:                  ...
-COMMON-EMPTY:
+COMMON-RELOC-NEXT:  a0: 00 00 00 00   <unknown>
+RELOC-NEXT:                           000000a0:  R_TLS        (idx: 26) d[TL]
+COMMON-RELOC-NEXT:  a4: 00 00 00 00   <unknown>
 PLAIN:        00000000000000a8 <func>:
 DESC:         00000000000000a8 (idx: 10) func[DS]:
-COMMON-NEXT:                  ...
-COMMON-NEXT:        b4: 00 00 00 80   <unknown>
+COMMON-RELOC-NEXT:  a8: 00 00 00 00   <unknown>
+RELOC-NEXT:                           000000a8:  R_POS        (idx: 6) .func
+COMMON-RELOC-NEXT:  ac: 00 00 00 00   <unknown>
+COMMON-RELOC-NEXT:  b0: 00 00 00 00   <unknown>
+RELOC-NEXT:                           000000b0:  R_POS        (idx: 8) TOC[TC0]
+COMMON:             b4: 00 00 00 80   <unknown>
 COMMON-NEXT:                  ...
 COMMON-EMPTY:
 PLAIN:        00000000000000c0 <b>:
 DESC:         00000000000000c0 (idx: 18) b[RW]:
 COMMON-NEXT:        c0: 00 00 30 39   <unknown>
 COMMON-NEXT:        c4: 00 00 00 00   <unknown>
 COMMON-EMPTY:
 COMMON-NEXT:  Disassembly of section .bss:

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 &);
-  void printRelocations(ArrayRef<XCOFFSectionHeader32> Sections);
+  template <typename Shdr, typename RelTy>
+  void printRelocations(ArrayRef<Shdr> Sections);
   const XCOFFObjectFile &Obj;
 };
 } // anonymous namespace
 
 void XCOFFDumper::printFileHeaders() {
   DictScope DS(W, "FileHeader");
   W.printHex("Magic", Obj.getMagic());
   W.printNumber("NumberOfSections", Obj.getNumberOfSections());
void XCOFFDumper::printSectionHeaders() {
   if (Obj.is64Bit())
     printSectionHeaders(Obj.sections64());
   else
     printSectionHeaders(Obj.sections32());
 }
 
 void XCOFFDumper::printRelocations() {
   if (Obj.is64Bit())
-    llvm_unreachable("64-bit relocation output not implemented!");
+    printRelocations<XCOFFSectionHeader64, XCOFFRelocation64>(Obj.sections64());
   else
-    printRelocations(Obj.sections32());
+    printRelocations<XCOFFSectionHeader32, XCOFFRelocation32>(Obj.sections32());
 }
 
 static const EnumEntry<XCOFF::RelocationType> RelocationTypeNameclass[] = {
 #define ECase(X)                                                               \
   { #X, XCOFF::X }
     ECase(R_POS),    ECase(R_RL),     ECase(R_RLA),    ECase(R_NEG),
     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
 };
 
-void XCOFFDumper::printRelocations(ArrayRef<XCOFFSectionHeader32> Sections) {
+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 auto &Sec : Sections) {
+  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;
-    auto Relocations = unwrapOrError(Obj.getFileName(), Obj.relocations(Sec));
+    Expected<ArrayRef<RelTy>> ErrOrRelocations = Obj.relocations<Shdr, RelTy>(Sec);
+    if (Error E = ErrOrRelocations.takeError())
+      reportUniqueWarning(std::move(E));
+
+    const ArrayRef<RelTy> Relocations = *ErrOrRelocations;
     if (Relocations.empty())
       continue;
 
     W.startLine() << "Section (index: " << Index << ") " << Sec.getName()
                   << " {\n";
-    for (auto Reloc : Relocations) {
-      StringRef SymbolName = unwrapOrError(
-          Obj.getFileName(), Obj.getSymbolNameByIndex(Reloc.SymbolIndex));
+    for (const RelTy &Reloc : Relocations) {
+      Expected<StringRef> ErrOrSymbolName =
+          Obj.getSymbolNameByIndex(Reloc.SymbolIndex);
+      if (Error E = ErrOrSymbolName.takeError())
+        reportUniqueWarning(std::move(E));
 
+      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));