[llvm-readobj][XCOFF]implement parsing relocation information for 32-bit xcoff objectfile

[sfertile]

llvm/include/llvm/BinaryFormat/XCOFF.h

158

This is 0x4 in the XCOFF Object Format Docs, do they need to be updated or is the patch wrong?

165

Minor nit: IIUC R_RL and 'R_RLA` are handled the same as R_POS, maybe we should keep them together.

170

Missing R_BR here.

176

Missing R_TLSML.

llvm/include/llvm/Object/XCOFFObjectFile.h

173

I imagine this (and getRelocatedLength) was templated to handle both XCOFFRelocation32 and XCOFFRelocation64 eventually? Lets just take an argument of XCOFFRelocation32 for now, and template them once we land 64-bit object file relocation support.

llvm/lib/Object/XCOFFObjectFile.cpp

499

>=

llvm/tools/llvm-readobj/XCOFFDumper.cpp

53

Minor nit: Take the concrete type, and we can change to a templated function when we land 64-bit relocation support.

[DiggerLin]

llvm/include/llvm/BinaryFormat/XCOFF.h

158

I think it is Doc wrong, The value in the patch come from aix OS header file . I opened ticket to confirm it.

165

the enum is keep in value order, I am prefer to keep them as now.

170

good catch, added it , thanks

176

good catch, thanks

llvm/include/llvm/Object/XCOFFObjectFile.h

173

yes, all template here will be used to support XCOFFRelocation32 and XCOFFRelocation64. we support 32bits now, we will support 64bits later.

llvm/lib/Object/XCOFFObjectFile.cpp

499

yes, you are correct, symbol index are from zero.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

53

we will have a 64bits support later, I am prefer to keep the templated function here. if you strong need to use a concrete type now, I will do it.

[sfertile]

llvm/include/llvm/BinaryFormat/XCOFF.h

158

Thanks.

165

There are already relocations grouped together by functionality as opposed to value order: eg R_TOC', 'R_TRL','R_TRLA or 'R_BA`,'R_RBA','R_RBR' . Why is important to keep these grouped in value order as opposed these others that are grouped by functionality?

[hubert.reinterpretcast]

llvm/include/llvm/BinaryFormat/XCOFF.h

170

Please be consistent on whether to use lowercase hex digits or uppercase ones.

llvm/include/llvm/Object/XCOFFObjectFile.h

162

s/instuction/instruction/;

171

Add a blank line before the comment.

327

Add a hyphen: "Relocation-related".

llvm/lib/Object/XCOFFObjectFile.cpp

499

Is this value verified elsewhere against the offset of the symbol table and the buffer size? If not, then this check is not a substitute for bounds checking on the pointer arithmetic below.

655

getWithOffset does not do any bounds checking. Please ensure bounds checking.

660

This should use the logical number of relocations, not the raw field value.

llvm/test/tools/llvm-readobj/xcoff-basic.test

92

I would prefer:

Section (index: 1) .text {

llvm/tools/llvm-readobj/XCOFFDumper.cpp

119

s/Reloation/Relocation/;

122

The number of ECases does not match the number of enumerators in the enumeration.

[sfertile]

llvm/include/llvm/BinaryFormat/XCOFF.h

152

We should be documenting what each of these relocation types represents.

llvm/include/llvm/Object/XCOFFObjectFile.h

173

The problem is that we are now reviewing these functions in a context that doesn't exist yet: ie for both 32-bit relocation representation (in this patch) and 64-bit relocation representation (which is yet to be added). I think we can make an educated guess what the 64-bit relocation struct will look like, and review in that context but it wont be so obvious to other readers of the file in the meantime (unitl 64-bit support lands). To be pedantic we should either add 64-bit support along side the 32-bit support (which would mean landing 64-bit symbol table support first), or stick to the existing concrete type. (Ditto on XCOFFDumper::printRelocations).

llvm/lib/Object/XCOFFObjectFile.cpp

655

report_fatal_error() for 64-bit object files.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

140

Your section index will get out of sync, since we continue (to the next section) but don't increment the index.

[hubert.reinterpretcast]

llvm/test/tools/llvm-readobj/xcoff-basic.test

128

This reduction is odd. Two kinds of TLS-related relocations are represented by the .o file, but we don't check for them. Please also add in the relocation referencing the unnamed [RO] csect.

[DiggerLin]

llvm/include/llvm/BinaryFormat/XCOFF.h

152

added

165

changed as suggestion

170

fixed

llvm/include/llvm/Object/XCOFFObjectFile.h

162

changed , thanks

171

added

327

added

llvm/lib/Object/XCOFFObjectFile.cpp

499

yes, We already check in XCOFFObjectFile::create as

// Parse symbol table.
CurOffset = Obj->fileHeader32()->SymbolTableOffset;
uint64_t SymbolTableSize = (uint64_t)(sizeof(XCOFFSymbolEntry)) *
                           Obj->getLogicalNumberOfSymbolTableEntries32();
auto SymTableOrErr =
    getObject<XCOFFSymbolEntry>(Data, Base + CurOffset, SymbolTableSize);
if (Error E = SymTableOrErr.takeError())
  return std::move(E);
Obj->SymbolTblPtr = SymTableOrErr.get();
CurOffset += SymbolTableSize;

655

I will do as suggestion

660

there is no logical number of relocation , it defined as
support::ubig16_t NumberOfRelocations; in the struct XCOFFSectionHeader32 {

llvm/test/tools/llvm-readobj/xcoff-basic.test

92

added

128

changed as suggestion.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

140

index has been increased before W.unindent();

[DiggerLin]

changed code based on comment

[hubert.reinterpretcast]

llvm/lib/Object/XCOFFObjectFile.cpp

660

"In an XCOFF32 file, if more than 65,534 relocation entries are required, the field value will be 65535, and an STYP_OVRFLO section header will contain the actual count of relocation entries in the s_paddr field."

[DiggerLin]

llvm/lib/Object/XCOFFObjectFile.cpp

660

for the relocation entry or line number entry overflow. I think we need to add a new patch for it. for the interpret of fields of Overflow section also be changed. We need to display Overflow section as separation(other than use current display source code).
s_nreloc (Overflow section)
Specifies the file section number of the section header that overflowed; that is, the section header containing a value of 65535 in its s_nreloc and s_nlnno fields. This value provides a reference to the primary section header. This field must have the same value as the s_nlnno field.

s_paddr (Overflow section)
Specifies the number of relocation entries actually required. This field is used instead of the s_nreloc field of the section header that overflowed.
s_vaddr (Overflow section)
Specifies the number of line-number entries actually required. This field is used instead of the s_nlnno field of the section header that overflowed.

I added a TODO: in the code.

[DiggerLin]

address comment of relocation entries overflow.

[DiggerLin]

delete the template code.

llvm/include/llvm/Object/XCOFFObjectFile.h

173

changed without template

[sfertile]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

140

index has been increased before W.unindent();

Yes, and that line of code will be skipped by the continue for any section that doesn't contain relocations. Any sections following a section with no relocations will be printed with the wrong section index.

[DiggerLin]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

140

thanks for pointout

[DiggerLin]

address comment

[sfertile]

llvm/include/llvm/BinaryFormat/XCOFF.h

152

Thanks for commenting these Digger. I see you used the descriptions from reloc.h. Those work. In some cases I think we can do a bit better on the descriptions if we crib from here. I can write up my suggestions and then run them by you before we attempt to update any though.

llvm/lib/Object/XCOFFObjectFile.cpp

660

Would it be worth while to loop over the section header table after parsing it and emit a fatal error if any sections have overflowed (either their relocation or their line number infos). I agree any handling of overflow sections must be added in a separate patch. The error I am suggesting would also need to be landed in a separate patch from this, but it should be a very quick patch to review and commit.

[DiggerLin]

llvm/include/llvm/BinaryFormat/XCOFF.h

152

OK, thanks please share with suggestions.

llvm/lib/Object/XCOFFObjectFile.cpp

660

if a relocation is overflow , I think we still can print out part of the relocation information which is under 65535 (even if we do not know exactly number of the relocation entry at this moment)

[hubert.reinterpretcast]

llvm/lib/Object/XCOFFObjectFile.cpp

660

We don't know the number, and it is not immediately obvious (because the relocation might not be the field whose logical value didn't fit) that the value would be at least 65535, so the patch Sean suggested makes sense.

llvm/test/tools/llvm-readobj/xcoff-basic.test

128

To be more clear: There are two TLS-related relocation kinds represented in the object file, and neither are checked in this test. Please check for them.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

140

https://llvm.org/docs/CodingStandards.html#prefer-preincrement: ++Index.

[DiggerLin]

llvm/include/llvm/BinaryFormat/XCOFF.h

158

I have confirmed from the aix OS developer, the value in the doc is wrong , they will fix it.

[DiggerLin]

llvm/lib/Object/XCOFFObjectFile.cpp

660

I have added some new code to deal with the relocation entries overflow.

llvm/test/tools/llvm-readobj/xcoff-basic.test

128

added, thanks

llvm/tools/llvm-readobj/XCOFFDumper.cpp

140

changed as suggestion

[DiggerLin]

added a new function getLogicalNumberOfRelocationEntries() to deal with the relocation entries overflow. and change comments of the Relocation Type.

[hubert.reinterpretcast]

llvm/include/llvm/Object/XCOFFObjectFile.h

168

Do these need to be declared in the header? Are they called only in one .cpp file? If so, they can be made static in the .cpp file. Otherwise, it seems odd that these aren't const member functions of XCOFFRelocation32.

llvm/lib/Object/XCOFFObjectFile.cpp

540

Please add a blank line after the function body here.

541

We can reduce the amount of background for the comment to what is necessary to understand the code here:
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.

546

No need to pass in the index. The index can be retrieved from within this function.

547

RELOC_OVERFLOW . USHRT_MAX does not need to be 65535.

549

No need for else here. Just place the else logic at the same level as the if.

556

This is not an internal-error situation. A different error handling mechanism is needed.

562

Why a is64Bit check in a file that needs an XCOFFSectionHeader32 in order to call it?

569

Write the cast out as reinterpret_cast.

570

This also needs to use the logical number of relocation entries.

[hubert.reinterpretcast]

llvm/include/llvm/BinaryFormat/XCOFF.h

160

s/dispacement/displacement/;

173

s/it/It/;

185

Add "(such as code used for dynamic initialization of non-local statics)" before "for which".

188

s/non modifiable/non-modifiable/;

191

s/refrenced/referenced/g;

192

s/ / /;
s/non modifiable/non-modifiable/;

193

s/refrences/references/;

195

s/Simialr/Similar/;
s/R_BR/the R_BR/;

llvm/include/llvm/Object/XCOFFObjectFile.h

158

Insert

//

between the group description and the specific description.

161

Insert

//

before the next specific description to extend the group while maintaining spacing.

164

Same here.

272

Leave the blank line that was there before this patch.

327

Insert a blank line above the comment.

llvm/lib/Object/XCOFFObjectFile.cpp

545

@sfertile, suggested that this be a separate patch. Could we land that first (with an update to how STYP_OVRFLO section headers are printed)?

[DiggerLin]

added a new relocation overflow test case and changed code based on comment

[DiggerLin]

llvm/include/llvm/Object/XCOFFObjectFile.h

164

changed .

168

the llvm-readobj is using those function and obj2yaml will use them too.

272

changed

327

added

llvm/lib/Object/XCOFFObjectFile.cpp

541

added.

546

good idea.

547

changed.

549

thanks

[sfertile]

llvm/lib/Object/XCOFFObjectFile.cpp

545

Yes Please.

llvm/test/tools/llvm-readobj/reloc_overflow.ll

1 ↗

(On Diff #222860)

The .ll suffix implies the test is written in LLVM IR. Use .test instead.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

140

Is this specified in the docs? I wasn't able to find it specified anywhere. What about the exception section? I don't know anything about the exception implementation on AIX so I could be wrong, but I suspect it might contain relocations.

I did find the specification of the special relocations in the loader table, and that they are a different format from the 'normal' relocations implemented in this patch. Does the loader section use the relocation pointer and relocation count in the section header table for these different relocations, or do we find them through fields defined in the loader section itself?

[DiggerLin]

llvm/lib/Object/XCOFFObjectFile.cpp

545

I have created a new patch for it. https://reviews.llvm.org/D68575 . implement parsing overflow section header.

llvm/test/tools/llvm-readobj/reloc_overflow.ll

1 ↗

(On Diff #222860)

I will change the name

llvm/tools/llvm-readobj/XCOFFDumper.cpp

140

from the xcoff document.
s_relptr Recognized for the .text, .data, .tdata, and STYP_DWARF sections only.

[hubert.reinterpretcast]

llvm/include/llvm/BinaryFormat/XCOFF.h

192

The typo, "refrenced", is still here.

193

Still missing the hyphen for "non-modifiable".

194

Either remove the "the" for this line or add "relocation" after "R_BA".

llvm/include/llvm/Object/XCOFFObjectFile.h

168

It is still odd to me that these aren't const non-static member functions of XCOFFRelocation32.

llvm/lib/Object/XCOFFObjectFile.cpp

541

I am not seeing the change.

565

Suggestion: NumRelocEntriesOrErr

569

Suggestion: NumRelocEntries

[sfertile]

llvm/include/llvm/Object/XCOFFObjectFile.h

168

I think were these originally templated to work with both 32-bit and 64-bit relocations, which explains why they aren't member functions.

llvm/test/tools/llvm-readobj/reloc_overflow.ll

1 ↗

(On Diff #222860)

I still see the .ll suffix.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

140

Thanks.

[hubert.reinterpretcast]

llvm/include/llvm/Object/XCOFFObjectFile.h

168

Would using CRTP with a base class template work for that case?

[DiggerLin]

llvm/include/llvm/Object/XCOFFObjectFile.h

168

as Sean's comment, for we only implement 32 bits relocation, we do not use any template for the relocation implement this moment.

[hubert.reinterpretcast]

llvm/include/llvm/Object/XCOFFObjectFile.h

168

All the more reason why these should be non-static member functions in the context of this patch.

[DiggerLin]

llvm/include/llvm/Object/XCOFFObjectFile.h

168

changed to member functions

[hubert.reinterpretcast]

I've marked comments (all minor) that have not yet been addressed.

llvm/include/llvm/BinaryFormat/XCOFF.h

192

Still not seeing the change: https://reviews.llvm.org/D67008?id=222860#inline-616695

193

Still not seeing the change: https://reviews.llvm.org/D67008?id=222860#inline-616694

194

Still not seeing the change: https://reviews.llvm.org/D67008?id=222860#inline-616692

llvm/include/llvm/Object/XCOFFObjectFile.h

185

Blank line between the class definitions please.

llvm/lib/Object/XCOFFObjectFile.cpp

541

Still not seeing the change: https://reviews.llvm.org/D67008?id=222237#inline-613037

565

Still not seeing the change: https://reviews.llvm.org/D67008?id=222860#inline-616702

569

Still not seeing the change: https://reviews.llvm.org/D67008?id=222860#inline-616704

[DiggerLin]

llvm/include/llvm/BinaryFormat/XCOFF.h

193

changed

194

changed

llvm/include/llvm/Object/XCOFFObjectFile.h

185

added

llvm/lib/Object/XCOFFObjectFile.cpp

569

changed

[DiggerLin]

llvm/lib/Object/XCOFFObjectFile.cpp

541

changed the comment as suggestion.

565

changed

[hubert.reinterpretcast]

Thanks @DiggerLin. I think this is almost ready.

llvm/include/llvm/Object/XCOFFObjectFile.h

151

Move the masks to the start of this class. Separate the nested types/constants from the fields using an access specifier label (e.g., public) or a comment.

156

Separate the fields from the methods using an access specified label or a comment.

158

Remove the comment and the blank line before it once the mask constants are defined in the class.

331

I would prefer a blank line between multi-line function declarations.

[DiggerLin]

llvm/include/llvm/Object/XCOFFObjectFile.h

151

changed a suggestion.

156

changed as suggestion

158

changed as suggestion

331

added

[hubert.reinterpretcast]

LGTM. Thanks.

llvm/include/llvm/BinaryFormat/XCOFF.h

 enum SymbolType {
   XTY_ER = 0, ///< External reference.
   XTY_SD = 1, ///< Csect definition for initialized storage.
   XTY_LD = 2, ///< Label definition.
               ///< Defines an entry point to an initialized csect.
   XTY_CM = 3  ///< Common csect definition. For uninitialized storage.
 };
 
+// Relocation types, defined in `/usr/include/reloc.h`.
+enum RelocationType : uint8_t {
+  R_POS = 0x00, // A(sym) Positive Relocation.
+  R_RL = 0x0c,  // A(sym) Pos indirect load. modifiable instruction.
+  R_RLA = 0x0d, // A(sym) Pos Load Address. modifiable instruction.
+
+  R_NEG = 0x01, // -A(sym) Negative Relocation.
+  R_REL = 0x02, // A(sym-*) Relative to self.
+
+  R_TOC = 0x03, // A(sym-TOC) Relative to TOC.
+  R_TRL = 0x12, // A(sym-TOC) TOC Relative indirect load.
+
+  R_TRLA = 0x13, // A(sym-TOC) TOC Rel load address. modifiable inst.
+
+  R_GL = 0x05,  // A(external TOC of sym) Global Linkage.
+  R_TCL = 0x06, // A(local TOC of sym) Local object TOC address.
+
+  R_REF = 0x0f, // AL0(sym) Non relocating ref. No garbage collec.
+
+  R_BA = 0x08,  // A(sym) Branch absolute. Cannot modify instruction.
+  R_BR = 0x0a,  // A(sym-*) Branch rel to self. non modifiable.
+  R_RBA = 0x18, // A(sym) Branch absolute. modifiable instruction.
+  R_RBR = 0x1a, // A(sym-*) Branch rel to self. modifiable instr.
+
+  R_TLS = 0x20,    // General-dynamic reference to TLS symbol.
+  R_TLS_IE = 0x21, // Initial-exec reference to TLS symbol.
+  R_TLS_LD = 0x22, // Local-dynamic reference to TLS symbol.
+  R_TLS_LE = 0x23, // Local-exec reference to TLS symbol.
+  R_TLSM = 0x24,   // Module reference to TLS symbol.
+  R_TLSML = 0x25,  // Module reference to local (own) module.
+
+  R_TOCU = 0x30, // Relative to TOC - high order bits.
+  R_TOCL = 0x31  // Relative to TOC - low order bits.
+};
+
 struct FileHeader32 {
   uint16_t Magic;
   uint16_t NumberOfSections;
   int32_t TimeStamp;
   uint32_t SymbolTableFileOffset;
   int32_t NumberOfSymbolTableEntries;
   uint16_t AuxiliaryHeaderSize;
   uint16_t Flags;
 };
 
 struct SectionHeader32 {
   char Name[XCOFF::NameSize];
   uint32_t PhysicalAddress;
   uint32_t VirtualAddress;
   uint32_t Size;
   uint32_t FileOffsetToData;
   uint32_t FileOffsetToRelocations;
   uint32_t FileOffsetToLineNumbers;

llvm/include/llvm/Object/XCOFFObjectFile.h

 
 struct XCOFFSectAuxEntForStat {
   support::ubig32_t SectionLength;
   support::ubig16_t NumberOfRelocEnt;
   support::ubig16_t NumberOfLineNum;
   uint8_t Pad[10];
 };
 
+struct XCOFFRelocation32 {
+  support::ubig32_t VirtualAddress;
+  support::ubig32_t SymbolIndex;
+
+  // Packed field, see XR_* masks for details of packing.
+  uint8_t Info;
+
+  XCOFF::RelocationType Type;
+};
+
+// 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;
+
+static bool isRelocationSigned(XCOFFRelocation32 &Reloc) {
+  return Reloc.Info & XR_SIGN_INDICATOR_MASK;
+}
+
+// If the Fixup bit is set, it indicates that the linker has modified
+// the instruction the relocation refers to.
+static bool isFixupIndicated(XCOFFRelocation32 &Reloc) {
+  return Reloc.Info & XR_FIXUP_INDICATOR_MASK;
+}
+
+// Returns the number of bits being relocated.
+static uint8_t getRelocatedLength(XCOFFRelocation32 &Reloc) {
+  // The relocation encodes the bit length being relocated minus 1. Add back
+  // the 1 to get the actual length being relocated.
+  return (Reloc.Info & XR_BIASED_LENGTH_MASK) + 1;
+}
+
 class XCOFFObjectFile : public ObjectFile {
 private:
   const void *FileHeader = nullptr;
   const void *SectionHeaderTable = nullptr;
 
   const XCOFFSymbolEntry *SymbolTblPtr = nullptr;
   XCOFFStringTable StringTable = {0, nullptr};
 
   const XCOFFFileHeader32 *fileHeader32() const;
public:
   relocation_iterator section_rel_end(DataRefImpl Sec) const override;
 
   void moveRelocationNext(DataRefImpl &Rel) const override;
   uint64_t getRelocationOffset(DataRefImpl Rel) const override;
   symbol_iterator getRelocationSymbol(DataRefImpl Rel) const override;
   uint64_t getRelocationType(DataRefImpl Rel) const override;
   void getRelocationTypeName(DataRefImpl Rel,
                              SmallVectorImpl<char> &Result) const override;
-
   section_iterator section_begin() const override;
   section_iterator section_end() const override;
   uint8_t getBytesInAddress() const override;
   StringRef getFileFormatName() const override;
   Triple::ArchType getArch() const override;
   SubtargetFeatures getFeatures() const override;
   Expected<uint64_t> getStartAddress() const override;
   bool isRelocatableObject() const override;
 
public:
   // values are reserved for future use.
   int32_t getRawNumberOfSymbolTableEntries32() const;
 
   // The sanitized value appropriate to use as an index into the symbol table.
   uint32_t getLogicalNumberOfSymbolTableEntries32() const;
 
   uint32_t getNumberOfSymbolTableEntries64() const;
   uint32_t getSymbolIndex(uintptr_t SymEntPtr) const;
+  Expected<StringRef> getSymbolNameByIndex(uint32_t SymbolTableIndex) const;
 
   Expected<StringRef> getCFileName(const XCOFFFileAuxEnt *CFileEntPtr) const;
   uint16_t getOptionalHeaderSize() const;
   uint16_t getFlags() const;
 
   // Section header table related interfaces.
   ArrayRef<XCOFFSectionHeader32> sections32() const;
   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.
+  Expected<ArrayRef<XCOFFRelocation32>>
+  relocations(const XCOFFSectionHeader32 &) const;
 }; // XCOFFObjectFile
 
 class XCOFFSymbolRef {
   const DataRefImpl SymEntDataRef;
   const XCOFFObjectFile *const OwningObjectPtr;
 
 public:
   XCOFFSymbolRef(DataRefImpl SymEntDataRef,
                  const XCOFFObjectFile *OwningObjectPtr)
       : SymEntDataRef(SymEntDataRef), OwningObjectPtr(OwningObjectPtr){};

llvm/lib/Object/XCOFFObjectFile.cpp

 
 #include "llvm/Object/XCOFFObjectFile.h"
 #include <cstddef>
 #include <cstring>
 
 namespace llvm {
 namespace object {
 
-enum {
-  FUNCTION_SYM = 0x20,
-  SYM_TYPE_MASK = 0x07
-};
+enum { FUNCTION_SYM = 0x20, SYM_TYPE_MASK = 0x07, RELOC_OVERFLOW = 65535 };
 
 // Checks that [Ptr, Ptr + Size) bytes fall inside the memory buffer
 // 'M'. Returns a pointer to the underlying object on success.
 template <typename T>
 static Expected<const T *> getObject(MemoryBufferRef M, const void *Ptr,
                                      const uint64_t Size = sizeof(T)) {
   uintptr_t Addr = uintptr_t(Ptr);
   if (std::error_code EC = Binary::checkOffset(M, Addr, Size))

 }
 
 uint32_t XCOFFObjectFile::getSymbolIndex(uintptr_t SymbolEntPtr) const {
   return (reinterpret_cast<const char *>(SymbolEntPtr) -
           reinterpret_cast<const char *>(SymbolTblPtr)) /
          XCOFF::SymbolTableEntrySize;
 }
 
+Expected<StringRef>
+XCOFFObjectFile::getSymbolNameByIndex(uint32_t Index) const {
+  if (is64Bit())
+    report_fatal_error("64-bit symbol table support not implemented yet.");
+
+  if (Index >= getLogicalNumberOfSymbolTableEntries32())
+    return errorCodeToError(object_error::invalid_symbol_index);
+
+  DataRefImpl SymDRI;
+  SymDRI.p = reinterpret_cast<uintptr_t>(getPointerToSymbolTable() + Index);
+  return getSymbolName(SymDRI);
+}
+
 uint16_t XCOFFObjectFile::getFlags() const {
   return is64Bit() ? fileHeader64()->Flags : fileHeader32()->Flags;
 }
 
 const char *XCOFFObjectFile::getSectionNameInternal(DataRefImpl Sec) const {
   return is64Bit() ? toSection64(Sec)->Name : toSection32(Sec)->Name;
 }
 
return ArrayRef<XCOFFSectionHeader64>(TablePtr,
                                         TablePtr + getNumberOfSections());
 }
 
 ArrayRef<XCOFFSectionHeader32> XCOFFObjectFile::sections32() const {
   assert(!is64Bit() && "32-bit interface called for non 32-bit file.");
   const XCOFFSectionHeader32 *TablePtr = sectionHeaderTable32();
   return ArrayRef<XCOFFSectionHeader32>(TablePtr,
                                         TablePtr + getNumberOfSections());
 }
 
 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 (auto EC = Binary::checkOffset(
           Obj->Data, reinterpret_cast<uintptr_t>(Obj->base() + Offset), 4))
     return XCOFFStringTable{0, nullptr};
 
   // Read the size out of the buffer.
   uint32_t Size = support::endian::read32be(Obj->base() + Offset);
 
   // If the size is less then 4, then the string table is just a size and no
   // string data.
   if (Size <= 4)
     return XCOFFStringTable{4, nullptr};
 
   auto StringTableOrErr =
       getObject<char>(Obj->Data, Obj->base() + Offset, Size);
   if (Error E = StringTableOrErr.takeError())
     return std::move(E);
 
   const char *StringTablePtr = StringTableOrErr.get();
   if (StringTablePtr[Size - 1] != '\0')
     return errorCodeToError(object_error::string_table_non_null_end);
 
   return XCOFFStringTable{Size, StringTablePtr};
 }
 
 Expected<std::unique_ptr<XCOFFObjectFile>>
 XCOFFObjectFile::create(unsigned Type, MemoryBufferRef MBR) {
   // Can't use std::make_unique because of the private constructor.
   std::unique_ptr<XCOFFObjectFile> Obj;
   Obj.reset(new XCOFFObjectFile(Type, MBR));
 
   uint64_t CurOffset = 0;
   const auto *Base = Obj->base();
   MemoryBufferRef Data = Obj->Data;

 XCOFF::StorageClass XCOFFSymbolRef::getStorageClass() const {
   return OwningObjectPtr->toSymbolEntry(SymEntDataRef)->StorageClass;
 }
 
 uint8_t XCOFFSymbolRef::getNumberOfAuxEntries() const {
   return OwningObjectPtr->toSymbolEntry(SymEntDataRef)->NumberOfAuxEntries;
 }
 
+Expected<ArrayRef<XCOFFRelocation32>>
+XCOFFObjectFile::relocations(const XCOFFSectionHeader32 &Sec) const {
+  if (is64Bit())
+    report_fatal_error("64-bit relocation support not implemented yet.");
+
+  uintptr_t RelocAddr = getWithOffset(reinterpret_cast<uintptr_t>(FileHeader),
+                                      Sec.FileOffsetToRelocationInfo);
+  // In an XCOFF32 file, if more than 65,534 relocation entries are required,
+  // the field value will be 65535, and an STYP_OVRFLO section header will
+  // contain the actual count of relocation entries in the s_paddr field. Refer
+  // to the discussion of overflow headers in "Sections and Section Headers".
+  // TODO: Support relocation entries overflow.
+  auto RelocationOrErr = getObject<XCOFFRelocation32>(
+      Data, (void *)RelocAddr,
+      Sec.NumberOfRelocations * sizeof(XCOFFRelocation32));
+  if (Error E = RelocationOrErr.takeError())
+    return std::move(E);
+
+  const XCOFFRelocation32 *StartReloc = RelocationOrErr.get();
+
+  return ArrayRef<XCOFFRelocation32>(StartReloc,
+                                     StartReloc + Sec.NumberOfRelocations);
+}
+
 const XCOFFCsectAuxEnt32 *XCOFFSymbolRef::getXCOFFCsectAuxEnt32() const {
   assert(!OwningObjectPtr->is64Bit() &&
          "32-bit interface called on 64-bit object file.");
   assert(hasCsectAuxEnt() && "No Csect Auxiliary Entry is found.");
 
   // In XCOFF32, the csect auxilliary entry is always the last auxiliary
   // entry for the symbol.
   uintptr_t AuxAddr = getWithOffset(

llvm/test/tools/llvm-readobj/xcoff-basic.test

 # RUN: llvm-readobj --file-header %p/Inputs/xcoff-basic.o | \
 # RUN: FileCheck --check-prefix=FILEHEADER %s
 #
 # RUN: llvm-readobj --file-header %p/Inputs/xcoff-basic-64.o | \
 # RUN: FileCheck --check-prefix=FILEHEADER64 %s
 
 # RUN: llvm-readobj --file-header %p/Inputs/xcoff-basic-neg-time.o | \
 # RUN: FileCheck --check-prefix=NEGTIME %s
 
 # RUN: llvm-readobj --file-header %p/Inputs/xcoff-basic-neg-sym-count.o | \
 # RUN: FileCheck --check-prefix=NEGSYMCOUNT %s
 
+# RUN: llvm-readobj --relocs --expand-relocs %p/Inputs/xcoff-basic.o | \
+# RUN: FileCheck --check-prefix=RELOCSEXP %s
+
 # FILEHEADER:     File: {{.*}}xcoff-basic.o
 # FILEHEADER-NEXT: Format: aixcoff-rs6000
 # FILEHEADER-NEXT: Arch: powerpc
 # FILEHEADER-NEXT: AddressSize: 32bit
 # FILEHEADER-NEXT: FileHeader {
 # FILEHEADER-NEXT:   Magic: 0x1DF
 # FILEHEADER-NEXT:   NumberOfSections: 6
 # FILEHEADER-NEXT:   TimeStamp: 2019-03-12T14:04:43Z (0x5C87BC7B)

 # int B() { return b; }
 # int J() { return j; }
 # double D() { return d; }
 #
 # xcoff-basic-neg-time.o was manually edited to include a negative time stamp.
 # xcoff-basic-neg-sym-count.o was stripped using the 'strip' utility, and
 # manually edited to have a negative symbol table entry count.
 
+# RELOCSEXP: File: {{.*}}xcoff-basic.o
+# RELOCSEXP-NEXT: Format: aixcoff-rs6000
+# RELOCSEXP-NEXT: Arch: powerpc
+# RELOCSEXP-NEXT: AddressSize: 32bit
+# RELOCSEXP-NEXT: Relocations [
+# RELOCSEXP-NEXT:  Section (index: 1) .text {
+# RELOCSEXP-NEXT:    Relocation {
+# RELOCSEXP-NEXT:      Virtual Address: 0x2
+# RELOCSEXP-NEXT:      Symbol: a (85) 
+# RELOCSEXP-NEXT:      IsSigned: Yes
+# RELOCSEXP-NEXT:      FixupBitValue: 0
+# RELOCSEXP-NEXT:      Length: 16
+# RELOCSEXP-NEXT:      Type: R_TOC (0x3)
+# RELOCSEXP-NEXT:    }
+
+# RELOCSEXP:           Virtual Address: 0x90
+# RELOCSEXP-NEXT:      Symbol: .__tls_get_addr (118)
+# RELOCSEXP-NEXT:      IsSigned: Yes
+# RELOCSEXP-NEXT:      FixupBitValue: 0
+# RELOCSEXP-NEXT:      Length: 26
+# RELOCSEXP-NEXT:      Type: R_RBA (0x18)
+# RELOCSEXP-NEXT:    }
+# RELOCSEXP-NEXT:  }
+# RELOCSEXP-NEXT:  Section (index: 2) .data {
+# RELOCSEXP-NEXT:    Relocation {
+# RELOCSEXP-NEXT:      Virtual Address: 0x100
+# RELOCSEXP-NEXT:      Symbol: A (78)
+# RELOCSEXP-NEXT:      IsSigned: No
+# RELOCSEXP-NEXT:      FixupBitValue: 0
+# RELOCSEXP-NEXT:      Length: 32
+# RELOCSEXP-NEXT:      Type: R_POS (0x0)
+# RELOCSEXP-NEXT:    }
+
+# RELOCSEXP:           Virtual Address: 0x110
+# RELOCSEXP-NEXT:      Symbol: J (96)
+# RELOCSEXP-NEXT:      IsSigned: No
+# RELOCSEXP-NEXT:      FixupBitValue: 0
+# RELOCSEXP-NEXT:      Length: 32
+# RELOCSEXP-NEXT:      Type: R_POS (0x0)
+# RELOCSEXP-NEXT:    }
+
+# RELOCSEXP:           Virtual Address: 0x128
+# RELOCSEXP-NEXT:      Symbol: (76)
+# RELOCSEXP-NEXT:      IsSigned: No
+# RELOCSEXP-NEXT:      FixupBitValue: 0
+# RELOCSEXP-NEXT:      Length: 32
+# RELOCSEXP-NEXT:      Type: R_POS (0x0)
+# RELOCSEXP-NEXT:    }
+
+# RELOCSEXP:           Virtual Address: 0x154
+# RELOCSEXP-NEXT:      Symbol: TOC (72)
+# RELOCSEXP-NEXT:      IsSigned: No
+# RELOCSEXP-NEXT:      FixupBitValue: 0
+# RELOCSEXP-NEXT:      Length: 32
+# RELOCSEXP-NEXT:      Type: R_POS (0x0)
+# RELOCSEXP-NEXT:    }
+# RELOCSEXP-NEXT:  }
+# RELOCSEXP-NEXT:]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

private:
   template <typename T> void printSectionHeaders(ArrayRef<T> Sections);
   void printFileAuxEnt(const XCOFFFileAuxEnt *AuxEntPtr);
   void printCsectAuxEnt32(const XCOFFCsectAuxEnt32 *AuxEntPtr);
   void printSectAuxEntForStat(const XCOFFSectAuxEntForStat *AuxEntPtr);
   void printSymbol(const SymbolRef &);
 
   // Least significant 3 bits are reserved.
   static constexpr unsigned SectionFlagsReservedMask = 0x7;
+  void printRelocations(ArrayRef<XCOFFSectionHeader32> 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() {
-  llvm_unreachable("Unimplemented functionality for XCOFFDumper");
+  if (Obj.is64Bit())
+    llvm_unreachable("64-bit relocation output not implemented!");
+  else
+    printRelocations(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) {
+  if (!opts::ExpandRelocs)
+    report_fatal_error("Unexpanded relocation output not implemented.");
+
+  ListScope LS(W, "Relocations");
+  uint16_t Index = 0;
+  for (const auto &Sec : Sections) {
+    auto Relocations = unwrapOrError(Obj.getFileName(), Obj.relocations(Sec));
+
+    Index++;
+    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));
+
+      DictScope RelocScope(W, "Relocation");
+      W.printHex("Virtual Address", Reloc.VirtualAddress);
+      W.printNumber("Symbol", SymbolName, Reloc.SymbolIndex);
+      W.printString("IsSigned", isRelocationSigned(Reloc) ? "Yes" : "No");
+      W.printNumber("FixupBitValue", isFixupIndicated(Reloc) ? 1 : 0);
+      W.printNumber("Length", getRelocatedLength(Reloc));
+      W.printEnum("Type", (uint8_t)Reloc.Type,
+                  makeArrayRef(RelocationTypeNameclass));
+    }
+    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
 };