implement of the parsing symbol table for xcoffobjfile and output as yaml format

[DiggerLin]

Summary

the implement of the parsing symbol table do not support parsing auxiliary entries of symbol. I will create a new issue to implement it.

the xcoff object file (aix_xcoff.o)which used in the test are comed from

-bash-4.2$ cat test.c
extern int i;
extern int TestforXcoff;
int main()
{
i++;
TestforXcoff--;
}

[sfertile]

llvm/include/llvm/Object/XCOFFObjectFile.h

156

Why is this needed?

167

Another blank line before this one as well please.

168

Add a blank line before this one please.

175

I would suggest rewording this, maybe
// The value as encoded in the object file.

I think that + the following line is enough to convey negative values *are* allowed/expected.

179

maybe: // Sanitized value, useable as an index into the symbol table. instead.

llvm/lib/Object/XCOFFObjectFile.cpp

98

I would get rid of this white space.

98–101

this is unneeded.

111

How do we know when the index is into the string table vs. into a .debug section? If we have a symbol where the name *is* in the debug section we still return as StringRef to the sting table which is a problem.

136

Why not just return toSymbolEntry(Symb)->Value;

337

Keep this with the other file header related accessors.

llvm/test/tools/obj2yaml/aix_xcoff.test

17

Is the intent to always keep dumping the type as a numeric value? For this tool I think that's reasonable but I'd like to clarify.

[MaskRay]

llvm/tools/obj2yaml/xcoff2yaml.cpp

50

You may want to use Expected<...> Error instead of std::error_code, then you can avoid conversion like errorToErrorCode()

[MaskRay]

llvm/include/llvm/Object/XCOFFObjectFile.h

124

You need to rebase. The signature is Expected<StringRef> getSectionName(DataRefImpl Sec) const override; now.

llvm/lib/Object/XCOFFObjectFile.cpp

402

The heavylifting parsing work should be done in createXCOFFObjectFile. The constructor should be private and be a very thin wrapper.

436

Use support::endian::read32be in llvm/Support/Endian.h

llvm/tools/obj2yaml/xcoff2yaml.cpp

22

For new code, prefer Error Expected<T> to std::error_code. The former enforces error checking and provides better error messages. Avoid errorToErrorCode if possible, it is very common to lost messages after converting to an Error.

69

redundant braces.

[sfertile]

llvm/lib/Object/XCOFFObjectFile.cpp

402

The constructor should be private

This makes sense to me.

The heavylifting parsing work should be done in createXCOFFObjectFile. ... be a very thin wrapper.

This seems to go against what most of the other file formats do. I think at this point it seems reasonable to move the parsing we have into createXCOFFObjectFile but we have a downstream patch under review locally for adding 64-bit support and I believe such a change would seriously complicate the parsing/error handling needed to support the different parts of the object format. If the parsing fails we propagate the error to the caller, so I'm not sure if the state of the object is that big if a concern. I'd like to know what benefits your looking to get by moving the parsing into the create function, maybe its worth the extra complication or maybe we can capture parts of the benefit without too drastic a change.

[MaskRay]

llvm/lib/Object/XCOFFObjectFile.cpp

402

In ELFObjectFile.cpp, the ctor is a thin wrapper.
In COFFObjectFile.cpp and MachOObjectFile.cpp, they pass an non-const reference of Error variable to carry the parsing error.
It seems to me the ELF approach is slightly better as we can just use the return type instead of passing another variable around.

// COFF
  std::unique_ptr<COFFObjectFile> Ret(new COFFObjectFile(Object, EC));
  if (EC)
//// the object is created then destroyed if there is an error.
    return errorCodeToError(EC);

// ELF
  auto Ret = ELFObjectFile<ELFT>::create(Object);
/// if there is a parse error, nothing gets created
  if (Error E = Ret.takeError())
    return std::move(E);
  return make_unique<ELFObjectFile<ELFT>>(std::move(*Ret));

I hope this wouldn't be too drastic a change to make, but if it is, feel free to keep the current shape and make improvement (if any) in the future :)

[sfertile]

llvm/tools/obj2yaml/xcoff2yaml.cpp

34

What gets returned when this is false?

[DiggerLin]

llvm/include/llvm/Object/XCOFFObjectFile.h

124

I rebased it, thanks

156

I added addition public to separate the functions which are inherited from objectfile class from the functions api which only in xcoffobjectfile. it more readable.

168

added a blank line as suggestion

175

changed comment as suggestion

179

fixed

llvm/lib/Object/XCOFFObjectFile.cpp

98

I always put blank line between a local variable define(declare) and function code, I think it maybe more readable.

111

I am agred with you. we do not support the debug related functionality this moment. I think I need to modify the function getSymbolName() when we support debug functionality. the symbol name of the debug symbol are in the dbx Stabstrings of debug section.

136

good idea, changed as suggestion.

337

moved

436

change as suggestion

llvm/test/tools/obj2yaml/aix_xcoff.test

17

for the N_ABS, N_DEBUG, N_UNDEF, which do not has section related to it, It not show section number here.

llvm/tools/obj2yaml/xcoff2yaml.cpp

22

if the dumpSymbols return a value other than std::error_code, I can try to use Expected<T>, what T should be in the this case?
same situation for the function std::error_code XCOFFDumper::dump() .

the even I have Expected<T> XCOFFDumper::dump() , the error information will still lose in function std::error_code xcoff2yaml(raw_ostream &Out,

const object::XCOFFObjectFile &Obj) {
and static std::error_code dumpObject(const ObjectFile &Obj) {

50

if the dumpSymbols return a value other than std::error_code, I can try to use Expected<T>, what T should be in the this case?
same situation for the function std::error_code XCOFFDumper::dump() .

the even I have Expected<T> XCOFFDumper::dump() , the error information will still lose in function std::error_code xcoff2yaml(raw_ostream &Out,

const object::XCOFFObjectFile &Obj) {

and static std::error_code dumpObject(const ObjectFile &Obj) {

69

deleted them

[DiggerLin]

changed code based on the comment

[DiggerLin]

changed code based on comment

[hubert.reinterpretcast]

llvm/include/llvm/Object/XCOFFObjectFile.h

175

s/Note that return/Returns/;

179

s/Return/Returns a/;

llvm/tools/obj2yaml/xcoff2yaml.cpp

64

Length mismatch.

llvm::yaml::Hex16 Value; // Symbol value; storage class-dependent.

support::ubig32_t Value; // Symbol value; storage class-dependent.

[hubert.reinterpretcast]

llvm/test/tools/obj2yaml/aix_xcoff.test

17

The question for this line is whether printing as a single type field is appropriate. This is a C_FILE entry where n_lang is C (0x00) and n_cputype is COM (0x03). That is, perhaps we should have inspected CFileLanguageIdAndTypeId?

[hubert.reinterpretcast]

llvm/lib/Object/XCOFFObjectFile.cpp

57

Use Size instead of size to match Name and also the corresponding parameter in checkSize.

107

Hardcoded 0 should be replaced with something that indicates the semantic: XCOFFSymbolEntry::NAME_IN_STR_TBL_MAGIC or at least 0x00000000.

111

By convention, a storage class value with the high-order bit on indicates that this field is a symbolic debugger stabstring.

The above means that the check should be for SectionNumber < 0.

[hubert.reinterpretcast]

llvm/lib/Object/XCOFFObjectFile.cpp

111

If an error recovery path is thought to be necessary, then llvm_unreachable is the wrong answer. Since the error recovery seems reasonable, use assert instead.

118

A byte offset value of 0 is a null or zero-length symbol name.

Is the "less than 4" an invention of this patch? A byte offset of 1, 2, or 3 points into the length field. It is well-formed as claimed by the comment in the patch, or is it erroneous?

123

Check that the Offset is within the string table.

125

It seems that there should be no period in the string:

llvm/lib/Object/WasmObjectFile.cpp:        make_error<StringError>("Bad magic number", object_error::parse_failed);

[DiggerLin]

changed the code based on comment

llvm/include/llvm/Object/XCOFFObjectFile.h

175

fixed. thanks

179

fixed , thanks

llvm/test/tools/obj2yaml/aix_xcoff.test

17

Thanks for Hubert let me I make a wrong answer.

1. for the question of " Is the intent to always keep dumping the type as a numeric value? For this tool I think that's reasonable but I'd like to clarify."

Answer:
in the xcoff document ,
n_type :
Bits 0-3
Symbol visibility. The SYM_V_MASK macro, with the value 0xF000, can be used to mask off bits in the n_type field that do not specify visibility. The following visibilities are defined:
0x1000 SYM_V_INTERNAL
0x2000 SYM_V_HIDDEN
0x3000 SYM_V_PROTECTED
0x4000 SYM_V_EXPORTED

Bit 10
Optionally set to 1 if the symbol is a function. Otherwise, it is set to 0.

I can not use enum to represent the value. I have to display as a value.

2. For the question "This is a C_FILE entry where n_lang is C (0x00) and n_cputype is COM (0x03). That is, perhaps we should have inspected CFileLanguageIdAndTypeId?"

Answer: I do not think yaml framework support showing different field name based on the value of the field.

The field name of yaml output are defined in the function, Once it is defined , we can not change the field name based on the value.

void MappingTraits<XCOFFYAML::Symbol>::mapping(IO &IO, XCOFFYAML::Symbol &S) {

IO.mapRequired("Name", S.SymbolName);
IO.mapRequired("Value", S.Value);
IO.mapRequired("Section", S.SectionName);
IO.mapRequired("Type", S.Type);
IO.mapRequired("StorageClass", S.StorageClass);
IO.mapRequired("NumberOfAuxEntries", S.NumberOfAuxEntries);

}

llvm/tools/obj2yaml/xcoff2yaml.cpp

64

thanks for point out. fixed

[hubert.reinterpretcast]

llvm/lib/Object/XCOFFObjectFile.cpp

98–101

There's still a stray return statement here.

442

Why the extra parentheses around StringTable.Size?

llvm/lib/ObjectYAML/XCOFFYAML.cpp

18

Both of the above includes are included directly by XCOFFYAML.h for which this is the implementation file.

llvm/test/tools/obj2yaml/aix_xcoff.test

17

What happens with something like:

llvm/lib/ObjectYAML/COFFYAML.cpp:  IO.mapOptional("SymbolTableIndex", Rel.SymbolTableIndex);

?

[DiggerLin]

changed code based the comment-May 15,2019

[DiggerLin]

changed code based on comment

llvm/lib/Object/XCOFFObjectFile.cpp

57

fixed , thanks

98–101

deleted

107

changed as suggestion

111

we do not support xcoff object file which generated by -g currently. we will support it later, and symbol name in the .debug section later. So I think use llvm_unreachable maybe more reasonable.

118

in the xcoff document, it describe as " A byte offset value of 0 is a null or zero-length symbol name."

A byte offset of 1, 2, or 3 points into the length field , the document do not talk about anything on it. we look it as "zero-length symbol name" . If we can treated the "A byte offset of 1, 2, or 3" as error format, and return as
make_error<GenericBinaryError>("Symbol Name offset error",object_error::parse_failed);
But I think looking "A byte offset of 1, 2, or 3" as "zero-length symbol name" will make obj2yaml more compatible。

123

Good idea, added offset check with the string table size. thanks

125

deleted the period.

442

deleted parentheses

llvm/lib/ObjectYAML/XCOFFYAML.cpp

18

good point, I deleted all redundant header files

llvm/test/tools/obj2yaml/aix_xcoff.test

17

The field name of yaml output are defined in the function, Once it is defined , we can not change the field name based on the value.

void MappingTraits<XCOFFYAML::Symbol>::mapping(IO &IO, XCOFFYAML::Symbol &S) {

IO.mapRequired("Name", S.SymbolName);
IO.mapRequired("Value", S.Value);
IO.mapRequired("Section", S.SectionName);
IO.mapRequired("Type", S.Type);
IO.mapRequired("StorageClass", S.StorageClass);
IO.mapRequired("NumberOfAuxEntries", S.NumberOfAuxEntries);
}

17

when you did the first comment on it , I think over to use IO.mapOptional, But I denied it in final, "n_type" is not optional field in the xcoff object file. if I use optional field for the CFileLanguageIdAndTypeId , it means that CFileLanguageIdAndTypeId is optional field of symbol entry , It may cause confuse, it actually only a different interpret of the n_type.

[DiggerLin]

changed code based on comment

[DiggerLin]

llvm/lib/Object/XCOFFObjectFile.cpp

111

changed to assert.

llvm/tools/obj2yaml/xcoff2yaml.cpp

34

if symbol name parse failed .
it return std::error_code which value is 3 . defined in
Error.h

enum class object_error {

// Error code 0 is absent. Use std::error_code() instead.
arch_not_found = 1,
invalid_file_type,
parse_failed,
unexpected_eof,
string_table_non_null_end,
invalid_section_index,
bitcode_section_not_found,
invalid_symbol_index,

};

[DiggerLin]

deleted a #include "llvm/Support/YAMLTraits.h" from llvm/lib/ObjectYAML/XCOFFYAML.cpp

[hubert.reinterpretcast]

llvm/lib/Object/XCOFFObjectFile.cpp

111

Suggestion: Access to symbolic debugger stabstrings from .debug section not yet implemented!
Also, reminder re: https://reviews.llvm.org/D61532?id=198073#inline-550220.

118

The comment should be clear as to what is part of the specification and what is part of the implementation. Suggestion:
A byte offset value of 0 is a null or zero-length symbol name. A byte offset in the range 1 to 3 (inclusive) points into the length field; as a soft-error recovery mechanism, we treat such cases as having an offset of 0.

125

I still see the period.

llvm/tools/obj2yaml/xcoff2yaml.cpp

34

Sean is pointing out that there is no return statement in the case where there is no error on the call to dumpSymbols.

[hubert.reinterpretcast]

llvm/lib/Object/XCOFFObjectFile.cpp

110

We still should not reach the next line in release mode.

[MaskRay]

llvm/tools/obj2yaml/xcoff2yaml.cpp

50

Return Error.

[DiggerLin]

modified code based on comment

llvm/lib/Object/XCOFFObjectFile.cpp

111

thanks for the information, using "By convention, a storage class value with the high-order bit on indicates that this field is a symbolic debugger stabstring." is more exact than assert(SymEntPtr->SectionNumber != XCOFF::N_DEBUG)

llvm/tools/obj2yaml/xcoff2yaml.cpp

34

thanks for clarify the question, fixed it.

50

thanks for the information. But I think I still keep std::error_code XCOFFDumper::dumpSymbols() as it now. I will schedule a new patch after this patch upstream.

the new patch will change the high level interface
static std::error_code dumpObject(const ObjectFile &Obj) {

if (Obj.isCOFF())
  return coff2yaml(outs(), cast<COFFObjectFile>(Obj));

if (Obj.isXCOFF())
  return xcoff2yaml(outs(), cast<XCOFFObjectFile>(Obj));

if (Obj.isELF())
  return elf2yaml(outs(), Obj);
if (Obj.isWasm())
  return wasm2yaml(outs(), cast<WasmObjectFile>(Obj));

return obj2yaml_error::unsupported_obj_file_format;

}

to

Expected<Error> dumpObject(const ObjectFile &Obj) as your suggestion.

if change to Expected<Error> dumpObject(const ObjectFile &Obj). I also need to modified the function
coff2yaml();
elf2yaml();
wasm2yaml();

[DiggerLin]

llvm/lib/Object/XCOFFObjectFile.cpp

111

fixed. thanks

118

changed the comment as suggestion

125

the period gone now

[hubert.reinterpretcast]

I think this is pretty much there. One last issue for this patch, and then I think we can leave further tweaks for later patches.

llvm/lib/Object/XCOFFObjectFile.cpp

115

I'm not sure what asserting on a asserts-enabled build but generating a hard-error on a release build buys us. If we are okay with a hard error, then we can just go with it for all builds. If we are not okay with a hard error, then we don't want the debug build to trip either (and a TODO comment with some recovery path is the right answer).

Note that llvm/test/tools/llvm-readobj/Inputs/xcoff-basic.o has such symbols.

[sfertile]

llvm/include/llvm/Object/XCOFFObjectFile.h

156

separate the functions which are inherited from objectfile class from the functions api which only in xcoffobjectfile.

Thats a good idea, adding a brief comment to that end would work.

llvm/lib/Object/XCOFFObjectFile.cpp

98

Ok, that's pretty reasonable.

115

Note that llvm/test/tools/llvm-readobj/Inputs/xcoff-basic.o has such symbols.

We need to commit to implementing the debug name handling either in the same patch that enables dumping symbols from readobj or before that. It would be useless if we have to tip-toe around object files with debug info. Until then I think the assert alone is fine.

118

Could we have a valid offset of less than 4 if the name is in the debug section? I'm not suggesting we need to change in this patch, just considering the future.

402

I definitely agree the ELF way is better. The way I extended the parsing for 64-bit support though relies on it being done in the constructor since I rely on the file header accessors and section header accessors to abstract over the structure differences. I'll have a closer look at the ELF handling to see if we can move in that direction.

llvm/tools/obj2yaml/xcoff2yaml.cpp

34

Sorry, I should have been clearer.

[hubert.reinterpretcast]

llvm/lib/Object/XCOFFObjectFile.cpp

115

Printing unrelated strings or reporting spurious errors in release is not fine in my book. The hard-error alone in fine (no need for an assert).

118

Great question.

In llvm/test/tools/llvm-readobj/Inputs/xcoff-basic.o:

0000620:                          0000 0000 0000
0000630: 0002 0000 0000 fffe 0000 8c00

The offset into the debug section is 2.

[daltenty]

llvm/include/llvm/Object/XCOFFObjectFile.h

71

Building with this section with -DLLVM_ENABLE_WERROR=ON results in "error: anonymous types declared in an anonymous union are an extension"

83

Same problem with anonymous types with this union

[DiggerLin]

fixed the code based on the comment

[hubert.reinterpretcast]

LGTM with additional small changes (including the changes requested by @daltenty).

llvm/lib/Object/XCOFFObjectFile.cpp

110

Add a comment:

// A storage class value with the high-order bit on indicates that the name is a
// symbolic debugger stabstring.

111

Missing "return".

[DiggerLin]

fixed the compiler error of
"anonymous types declared in an anonymous union are an extension

[-Wnested-anon-types] "

when compile with clang++

llvm/include/llvm/Object/XCOFFObjectFile.h

71

I have fixed the problem

83

I build success as
bash-4.2$ g++ -std=c++11 -I/scratch/zhijian/llvm/src/llvm/include -I/scratch/zhijian/llvm/build/include -c llvm/include/llvm/Object/XCOFFObjectFile.h

83

I have fixed the problem .

llvm/lib/Object/XCOFFObjectFile.cpp

115

1. I deleted the assert
2. print symbol in as "Unimplemented Debug Name" after I discussed with Sean

[daltenty]

LGTM (with addition of comment @hubert.reinterpretcast suggested above)

[hubert.reinterpretcast]

There are still unaddressed comments.

llvm/lib/Object/XCOFFObjectFile.cpp

111

Please fix this.

[DiggerLin]

llvm/lib/Object/XCOFFObjectFile.cpp

111

fixed , thanks a lot

[DiggerLin]

added comment based on the reviewed

llvm/include/llvm/BinaryFormat/XCOFF.h

 //===-- llvm/BinaryFormat/XCOFF.h - The XCOFF file format -------*- C++/-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines manifest constants for the XCOFF object file format.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_BINARYFORMAT_XCOFF_H
 #define LLVM_BINARYFORMAT_XCOFF_H
 
+#include <cstdint>
+
 namespace llvm {
 namespace XCOFF {
 
 // Constants used in the XCOFF definition.
-enum { SectionNameSize = 8 };
+enum { SectionNameSize = 8, SymbolNameSize = 8 };
+enum ReservedSectionNum { N_DEBUG = -2, N_ABS = -1, N_UNDEF = 0 };
 
 // Flags for defining the section type. Used for the s_flags field of
 // the section header structure. Defined in the system header `scnhdr.h`.
 enum SectionTypeFlags {
   STYP_PAD = 0x0008,
   STYP_DWARF = 0x0010,
   STYP_TEXT = 0x0020,
   STYP_DATA = 0x0040,
   STYP_BSS = 0x0080,
   STYP_EXCEPT = 0x0100,
   STYP_INFO = 0x0200,
   STYP_TDATA = 0x0400,
   STYP_TBSS = 0x0800,
   STYP_LOADER = 0x1000,
   STYP_DEBUG = 0x2000,
   STYP_TYPCHK = 0x4000,
   STYP_OVRFLO = 0x8000
 };
 
+// STORAGE CLASSES, n_sclass field of syment.
+// The values come from `storclass.h` and `dbxstclass.h`.
+enum StorageClass : uint8_t {
+  // Storage classes used for symbolic debugging symbols.
+  C_FILE = 103,  // File name
+  C_BINCL = 108, // Beginning of include file
+  C_EINCL = 109, // Ending of include file
+  C_GSYM = 128,  // Global variable
+  C_STSYM = 133, // Statically allocated symbol
+  C_BCOMM = 135, // Beginning of common block
+  C_ECOMM = 137, // End of common block
+  C_ENTRY = 141, // Alternate entry
+  C_BSTAT = 143, // Beginning of static block
+  C_ESTAT = 144, // End of static block
+  C_GTLS = 145,  // Global thread-local variable
+  C_STTLS = 146, // Static thread-local variable
+
+  // Storage classes used for DWARF symbols.
+  C_DWARF = 112, // DWARF section symbol
+
+  // Storage classes used for absolute symbols.
+  C_LSYM = 129,  // Automatic variable allocated on stack
+  C_PSYM = 130,  // Argument to subroutine allocated on stack
+  C_RSYM = 131,  // Register variable
+  C_RPSYM = 132, // Argument to function or procedure stored in register
+  C_ECOML = 136, // Local member of common block
+  C_FUN = 142,   // Function or procedure
+
+  // Storage classes used for undefined external symbols or
+  // symbols of general sections.
+  C_EXT = 2,       // External symbol
+  C_WEAKEXT = 111, // Weak external symbol
+
+  // Storage classes used for symbols of general sections.
+  C_NULL = 0,
+  C_STAT = 3,     // Static
+  C_BLOCK = 100,  // ".bb" or ".eb"
+  C_FCN = 101,    // ".bf" or ".ef"
+  C_HIDEXT = 107, // Un-named external symbol
+  C_INFO = 110,   // Comment string in .info section
+  C_DECL = 140,   // Declaration of object (type)
+
+  // Storage classes - Obsolete/Undocumented.
+  C_AUTO = 1,     // Automatic variable
+  C_REG = 4,      // Register variable
+  C_EXTDEF = 5,   // External definition
+  C_LABEL = 6,    // Label
+  C_ULABEL = 7,   // Undefined label
+  C_MOS = 8,      // Member of structure
+  C_ARG = 9,      // Function argument
+  C_STRTAG = 10,  // Structure tag
+  C_MOU = 11,     // Member of union
+  C_UNTAG = 12,   // Union tag
+  C_TPDEF = 13,   // Type definition
+  C_USTATIC = 14, // Undefined static
+  C_ENTAG = 15,   // Enumeration tag
+  C_MOE = 16,     // Member of enumeration
+  C_REGPARM = 17, // Register parameter
+  C_FIELD = 18,   // Bit field
+  C_EOS = 102,    // End of structure
+  C_LINE = 104,
+  C_ALIAS = 105,  // Duplicate tag
+  C_HIDDEN = 106, // Special storage class for external
+  C_EFCN = 255,   // Physical end of function
+
+  // Storage classes - reserved
+  C_TCSYM = 134 // Reserved
+};
+
 } // end namespace XCOFF
 } // end namespace llvm
 
 #endif

llvm/include/llvm/Object/XCOFFObjectFile.h

struct XCOFFSectionHeader {
   support::ubig32_t FileOffsetToRawData;
   support::ubig32_t FileOffsetToRelocationInfo;
   support::ubig32_t FileOffsetToLineNumberInfo;
   support::ubig16_t NumberOfRelocations;
   support::ubig16_t NumberOfLineNumbers;
   support::big32_t Flags;
 };
 
+struct XCOFFSymbolEntry {
+  enum { NAME_IN_STR_TBL_MAGIC = 0x0 };
+  union {
+    char SymbolName[XCOFF::SymbolNameSize];
+    struct {
+      support::big32_t Magic; // Zero indicates name in string table.
+      support::ubig32_t Offset;
+    } NameInStrTbl;
+  };
+
+  support::ubig32_t Value; // Symbol value; storage class-dependent.
+  support::big16_t SectionNumber;
+
+  union {
+    support::ubig16_t SymbolType;
+    struct {
+      uint8_t LanguageId;
+      uint8_t CpuTypeId;
+    } CFileLanguageIdAndTypeId;
+  };
+
+  XCOFF::StorageClass StorageClass;
+  uint8_t NumberOfAuxEntries;
+};
+
+struct XCOFFStringTable {
+  uint32_t Size;
+  const char *Data;
+};
+
 class XCOFFObjectFile : public ObjectFile {
 private:
   const XCOFFFileHeader *FileHdrPtr = nullptr;
   const XCOFFSectionHeader *SectionHdrTablePtr = nullptr;
+  const XCOFFSymbolEntry *SymbolTblPtr = nullptr;
+  XCOFFStringTable StringTable = {0, nullptr};
 
   size_t getFileHeaderSize() const;
   size_t getSectionHeaderSize() const;
 
   const XCOFFSectionHeader *toSection(DataRefImpl Ref) const;
+  static bool isReservedSectionNumber(int16_t SectionNumber);
+  std::error_code getSectionByNum(int16_t Num,
+                                  const XCOFFSectionHeader *&Result) const;
 
 
 public:
   void moveSymbolNext(DataRefImpl &Symb) const override;
   uint32_t getSymbolFlags(DataRefImpl Symb) const override;
   basic_symbol_iterator symbol_begin() const override;
   basic_symbol_iterator symbol_end() const override;
 
   Expected<StringRef> getSymbolName(DataRefImpl Symb) const override;
   Expected<uint64_t> getSymbolAddress(DataRefImpl Symb) const override;
   uint64_t getSymbolValueImpl(DataRefImpl Symb) const override;
   uint64_t getCommonSymbolSizeImpl(DataRefImpl Symb) const override;
   Expected<SymbolRef::Type> getSymbolType(DataRefImpl Symb) const override;
   Expected<section_iterator> getSymbolSection(DataRefImpl Symb) const override;
 
   void moveSectionNext(DataRefImpl &Sec) const override;
   Expected<StringRef> getSectionName(DataRefImpl Sec) const override;
   uint64_t getSectionAddress(DataRefImpl Sec) const override;
   uint64_t getSectionIndex(DataRefImpl Sec) const override;
   uint64_t getSectionSize(DataRefImpl Sec) const override;
   std::error_code getSectionContents(DataRefImpl Sec,
                                      StringRef &Res) const override;
   uint64_t getSectionAlignment(DataRefImpl Sec) const override;
   bool isSectionCompressed(DataRefImpl Sec) const override;
   bool isSectionText(DataRefImpl Sec) const override;
public:
   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;
 
   XCOFFObjectFile(MemoryBufferRef Object, std::error_code &EC);
 
   const XCOFFFileHeader *getFileHeader() const { return FileHdrPtr; }
+  const XCOFFSymbolEntry *getPointerToSymbolTable() const {
+    return SymbolTblPtr;
+  }
+
+  Expected<StringRef>
+  getSymbolSectionName(const XCOFFSymbolEntry *SymEntPtr) const;
 
+  const XCOFFSymbolEntry *toSymbolEntry(DataRefImpl Ref) const;
   uint16_t getMagic() const;
   uint16_t getNumberOfSections() const;
   int32_t  getTimeStamp() const;
   uint32_t  getSymbolTableOffset() const;
 
-  // Note that this value is signed and might return a negative value. Negative
-  // values are reserved for future use.
-  int32_t  getNumberOfSymbolTableEntries() const;
+  // Returns the value as encoded in the object file.
+  // Negative values are reserved for future use.
+  int32_t getRawNumberOfSymbolTableEntries() const;
   
+  // Returns a sanitized value, useable as an index into the symbol table.
+  uint32_t getLogicalNumberOfSymbolTableEntries() const;
   uint16_t getOptionalHeaderSize() const;
-  uint16_t getFlags() const;
+  uint16_t getFlags() const { return FileHdrPtr->Flags; };
 }; // XCOFFObjectFile
 
 } // namespace object
 } // namespace llvm
 
 #endif // LLVM_OBJECT_XCOFFOBJECTFILE_H

llvm/include/llvm/ObjectYAML/XCOFFYAML.h

 //===----- XCOFFYAML.h - XCOFF YAMLIO implementation ------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file declares classes for handling the YAML representation of XCOFF.
 //
 //===----------------------------------------------------------------------===//
 #ifndef LLVM_OBJECTYAML_XCOFFYAML_H
 #define LLVM_OBJECTYAML_XCOFFYAML_H
 
+#include "llvm/BinaryFormat/XCOFF.h"
 #include "llvm/ObjectYAML/YAML.h"
-#include <cstdint>
+#include <vector>
 
 namespace llvm {
 namespace XCOFFYAML {
 
 struct FileHeader {
   llvm::yaml::Hex16 Magic;
   uint16_t NumberOfSections;
   int32_t TimeStamp;
   llvm::yaml::Hex32 SymbolTableOffset; // File offset to symbol table.
   int32_t NumberOfSymTableEntries;
   uint16_t AuxHeaderSize;
   llvm::yaml::Hex16 Flags;
 };
 
+struct Symbol {
+  StringRef SymbolName;
+  llvm::yaml::Hex32 Value; // Symbol value; storage class-dependent.
+  StringRef SectionName;
+  llvm::yaml::Hex16 Type;
+  XCOFF::StorageClass StorageClass;
+  uint8_t NumberOfAuxEntries; // Number of auxiliary entries
+};
+
 struct Object {
   FileHeader Header;
+  std::vector<Symbol> Symbols;
   Object();
 };
 } // namespace XCOFFYAML
-
+} // namespace llvm
+LLVM_YAML_IS_SEQUENCE_VECTOR(XCOFFYAML::Symbol)
+namespace llvm {
 namespace yaml {
 
+template <> struct ScalarEnumerationTraits<XCOFF::StorageClass> {
+  static void enumeration(IO &IO, XCOFF::StorageClass &Value);
+};
+
 template <> struct MappingTraits<XCOFFYAML::FileHeader> {
   static void mapping(IO &IO, XCOFFYAML::FileHeader &H);
 };
 
 template <> struct MappingTraits<XCOFFYAML::Object> {
   static void mapping(IO &IO, XCOFFYAML::Object &Obj);
 };
 
+template <> struct MappingTraits<XCOFFYAML::Symbol> {
+  static void mapping(IO &IO, XCOFFYAML::Symbol &S);
+};
+
 } // namespace yaml
 } // namespace llvm
 
 #endif // LLVM_OBJECTYAML_XCOFFYAML_H

llvm/lib/Object/XCOFFObjectFile.cpp

 
 namespace llvm {
 namespace object {
 
 enum { XCOFF32FileHeaderSize = 20 };
 static_assert(sizeof(XCOFFFileHeader) == XCOFF32FileHeaderSize,
               "Wrong size for XCOFF file header.");
 
+// Sets EC and returns false if there is less than 'Size' bytes left in the
+// buffer at 'Offset'.
+static bool checkSize(MemoryBufferRef M, std::error_code &EC, uint64_t Offset,
+                      uint64_t Size) {
+  if (M.getBufferSize() < Offset + Size) {
+    EC = object_error::unexpected_eof;
+    return false;
+  }
+  return true;
+}
+
 // Sets Obj unless any bytes in [addr, addr + size) fall outsize of m.
 // Returns unexpected_eof on error.
 template <typename T>
 static std::error_code getObject(const T *&Obj, 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))
     return EC;
   Obj = reinterpret_cast<const T *>(Addr);
   return std::error_code();
 }
 
 template <typename T> static const T *viewAs(uintptr_t in) {
   return reinterpret_cast<const T *>(in);
 }
 
+static StringRef generateStringRef(const char *Name, uint64_t Size) {
+  auto NulCharPtr = static_cast<const char *>(memchr(Name, '\0', Size));
+  return NulCharPtr ? StringRef(Name, NulCharPtr - Name)
+                    : StringRef(Name, Size);
+}
+
 const XCOFFSectionHeader *XCOFFObjectFile::toSection(DataRefImpl Ref) const {
   auto Sec = viewAs<XCOFFSectionHeader>(Ref.p);
 #ifndef NDEBUG
   if (Sec < SectionHdrTablePtr ||
       Sec >= (SectionHdrTablePtr + getNumberOfSections()))
     report_fatal_error("Section header outside of section header table.");
 
   uintptr_t Offset = uintptr_t(Sec) - uintptr_t(SectionHdrTablePtr);
   if (Offset % getSectionHeaderSize() != 0)
     report_fatal_error(
         "Section header pointer does not point to a valid section header.");
 #endif
   return Sec;
 }
 
+const XCOFFSymbolEntry *XCOFFObjectFile::toSymbolEntry(DataRefImpl Ref) const {
+  assert(Ref.p != 0 && "Symbol table pointer can not be nullptr!");
+  auto SymEntPtr = viewAs<XCOFFSymbolEntry>(Ref.p);
+  return SymEntPtr;
+}
+
 // The next 2 functions are not exactly necessary yet, but they are useful to
 // abstract over the size difference between XCOFF32 and XCOFF64 structure
 // definitions.
 size_t XCOFFObjectFile::getFileHeaderSize() const {
   return sizeof(XCOFFFileHeader);
 }
 
 size_t XCOFFObjectFile::getSectionHeaderSize() const {
   return sizeof(XCOFFSectionHeader);
 }
 
+uint16_t XCOFFObjectFile::getMagic() const { return FileHdrPtr->Magic; }
+
 void XCOFFObjectFile::moveSymbolNext(DataRefImpl &Symb) const {
-  llvm_unreachable("Not yet implemented!");
-  return;
+  const XCOFFSymbolEntry *SymEntPtr = toSymbolEntry(Symb);
+
+  SymEntPtr += SymEntPtr->NumberOfAuxEntries + 1;
+  Symb.p = reinterpret_cast<uintptr_t>(SymEntPtr);
 }
 
 Expected<StringRef> XCOFFObjectFile::getSymbolName(DataRefImpl Symb) const {
-  StringRef Result;
+  const XCOFFSymbolEntry *SymEntPtr = toSymbolEntry(Symb);
+
+  if (SymEntPtr->NameInStrTbl.Magic != XCOFFSymbolEntry::NAME_IN_STR_TBL_MAGIC)
+    return generateStringRef(SymEntPtr->SymbolName, XCOFF::SymbolNameSize);
+
+  if (SymEntPtr->SectionNumber == XCOFF::N_DEBUG) {
     llvm_unreachable("Not yet implemented!");
-  return Result;
+    return StringRef(nullptr, 0);
+  }
+
+  uint32_t Offset = SymEntPtr->NameInStrTbl.Offset;
+  // The byte offset is relative to the start of the string table
+  // or .debug section.
+  // A byte offset value less than 4 is a null or zero-length symbol name.
+  if (Offset < 4)
+    return StringRef(nullptr, 0);
+
+  if (StringTable.Data != nullptr && StringTable.Size > Offset)
+    return (StringTable.Data + Offset);
+
+  return make_error<GenericBinaryError>("Symbol Name parse failed.",
+                                        object_error::parse_failed);
 }
 
 Expected<uint64_t> XCOFFObjectFile::getSymbolAddress(DataRefImpl Symb) const {
   uint64_t Result = 0;
   llvm_unreachable("Not yet implemented!");
   return Result;
 }
 
 uint64_t XCOFFObjectFile::getSymbolValueImpl(DataRefImpl Symb) const {
-  uint64_t Result = 0;
-  llvm_unreachable("Not yet implemented!");
-  return Result;
+  return toSymbolEntry(Symb)->Value;
 }
 
 uint64_t XCOFFObjectFile::getCommonSymbolSizeImpl(DataRefImpl Symb) const {
   uint64_t Result = 0;
   llvm_unreachable("Not yet implemented!");
   return Result;
 }
 
 Expected<SymbolRef::Type>
 XCOFFObjectFile::getSymbolType(DataRefImpl Symb) const {
   llvm_unreachable("Not yet implemented!");
   return SymbolRef::ST_Other;
 }
 
 Expected<section_iterator>
 XCOFFObjectFile::getSymbolSection(DataRefImpl Symb) const {
-  llvm_unreachable("Not yet implemented!");
-  return section_iterator(SectionRef());
+  const XCOFFSymbolEntry *SymEntPtr = toSymbolEntry(Symb);
+  int16_t SectNum = SymEntPtr->SectionNumber;
+
+  if (isReservedSectionNumber(SectNum))
+    return section_end();
+
+  const XCOFFSectionHeader *Sec;
+  if (std::error_code EC = getSectionByNum(SectNum, Sec))
+    return errorCodeToError(EC);
+
+  DataRefImpl SecDRI;
+  SecDRI.p = reinterpret_cast<uintptr_t>(Sec);
+
+  return section_iterator(SectionRef(SecDRI, this));
 }
 
 void XCOFFObjectFile::moveSectionNext(DataRefImpl &Sec) const {
   const char *Ptr = reinterpret_cast<const char *>(Sec.p);
   Sec.p = reinterpret_cast<uintptr_t>(Ptr + getSectionHeaderSize());
 }
 
 Expected<StringRef> XCOFFObjectFile::getSectionName(DataRefImpl Sec) const {

 
 uint32_t XCOFFObjectFile::getSymbolFlags(DataRefImpl Symb) const {
   uint32_t Result = 0;
   llvm_unreachable("Not yet implemented!");
   return Result;
 }
 
 basic_symbol_iterator XCOFFObjectFile::symbol_begin() const {
-  llvm_unreachable("Not yet implemented!");
-  return basic_symbol_iterator(SymbolRef());
+  DataRefImpl SymDRI;
+  SymDRI.p = reinterpret_cast<uintptr_t>(SymbolTblPtr);
+  return basic_symbol_iterator(SymbolRef(SymDRI, this));
 }
 
 basic_symbol_iterator XCOFFObjectFile::symbol_end() const {
-  llvm_unreachable("Not yet implemented!");
-  return basic_symbol_iterator(SymbolRef());
+  DataRefImpl SymDRI;
+  SymDRI.p = reinterpret_cast<uintptr_t>(
+      SymbolTblPtr + getLogicalNumberOfSymbolTableEntries());
+  return basic_symbol_iterator(SymbolRef(SymDRI, this));
 }
 
 section_iterator XCOFFObjectFile::section_begin() const {
   DataRefImpl DRI;
   DRI.p = reinterpret_cast<uintptr_t>(SectionHdrTablePtr);
   return section_iterator(SectionRef(DRI, this));
 }
 
 section_iterator XCOFFObjectFile::section_end() const {
   DataRefImpl DRI;
   DRI.p =
       reinterpret_cast<uintptr_t>(SectionHdrTablePtr + getNumberOfSections());
   return section_iterator(SectionRef(DRI, this));
 }
 
 uint8_t XCOFFObjectFile::getBytesInAddress() const {
   // Only support 32-bit object files for now ...
-  assert(getFileHeaderSize() ==  XCOFF32FileHeaderSize);
+  assert(getFileHeaderSize() == XCOFF32FileHeaderSize);
   return 4;
 }
 
 StringRef XCOFFObjectFile::getFileFormatName() const {
   assert(getFileHeaderSize() == XCOFF32FileHeaderSize);
   return "aixcoff-rs6000";
 }
 

 }
 
 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;
 }
 
+std::error_code
+XCOFFObjectFile::getSectionByNum(int16_t Num,
+                                 const XCOFFSectionHeader *&Result) const {
+  if (Num > 0 && static_cast<uint16_t>(Num) <= getNumberOfSections()) {
+    Result = SectionHdrTablePtr + (Num - 1);
+    return std::error_code();
+  }
+
+  return object_error::invalid_section_index;
+}
+
+Expected<StringRef>
+XCOFFObjectFile::getSymbolSectionName(const XCOFFSymbolEntry *SymEntPtr) const {
+  int16_t SectionNum = SymEntPtr->SectionNumber;
+
+  switch (SectionNum) {
+  case XCOFF::N_DEBUG:
+    return "N_DEBUG";
+  case XCOFF::N_ABS:
+    return "N_ABS";
+  case XCOFF::N_UNDEF:
+    return "N_UNDEF";
+  default: {
+    const XCOFFSectionHeader *SectHeaderPtr;
+    std::error_code EC;
+    if (EC = getSectionByNum(SectionNum, SectHeaderPtr))
+      return errorCodeToError(EC);
+    else
+      return generateStringRef(SectHeaderPtr->Name, XCOFF::SectionNameSize);
+  }
+  }
+}
+
+bool XCOFFObjectFile::isReservedSectionNumber(int16_t SectionNumber) {
+  return (SectionNumber <= 0 && SectionNumber >= -2);
+}
+
+uint16_t XCOFFObjectFile::getNumberOfSections() const {
+  return FileHdrPtr->NumberOfSections;
+}
+
+int32_t XCOFFObjectFile::getTimeStamp() const { return FileHdrPtr->TimeStamp; }
+
+uint32_t XCOFFObjectFile::getSymbolTableOffset() const {
+  return FileHdrPtr->SymbolTableOffset;
+}
+
+int32_t XCOFFObjectFile::getRawNumberOfSymbolTableEntries() const {
+  return FileHdrPtr->NumberOfSymTableEntries;
+}
+
+uint32_t XCOFFObjectFile::getLogicalNumberOfSymbolTableEntries() const {
+  return (FileHdrPtr->NumberOfSymTableEntries >= 0
+              ? FileHdrPtr->NumberOfSymTableEntries
+              : 0);
+}
+
+uint16_t XCOFFObjectFile::getOptionalHeaderSize() const {
+  return FileHdrPtr->AuxHeaderSize;
+}
+
 XCOFFObjectFile::XCOFFObjectFile(MemoryBufferRef Object, std::error_code &EC)
     : ObjectFile(Binary::ID_XCOFF32, Object) {
 
   // Current location within the file.
   uint64_t CurPtr = 0;
 
   if ((EC = getObject(FileHdrPtr, Data, base() + CurPtr)))
     return;
 
   CurPtr += getFileHeaderSize();
   // TODO FIXME we don't have support for an optional header yet, so just skip
   // past it.
   CurPtr += FileHdrPtr->AuxHeaderSize;
 
   if (getNumberOfSections() != 0) {
     if ((EC = getObject(SectionHdrTablePtr, Data, base() + CurPtr,
                         getNumberOfSections() * getSectionHeaderSize())))
       return;
   }
-}
 
-uint16_t XCOFFObjectFile::getMagic() const {
-  return FileHdrPtr->Magic;
-}
+  if (getLogicalNumberOfSymbolTableEntries() == 0)
+    return;
 
-uint16_t XCOFFObjectFile::getNumberOfSections() const {
-  return FileHdrPtr->NumberOfSections;
-}
+  // Get pointer to the symbol table.
+  CurPtr = FileHdrPtr->SymbolTableOffset;
+  uint64_t SymbolTableSize = (uint64_t)(sizeof(XCOFFSymbolEntry)) *
+                             getLogicalNumberOfSymbolTableEntries();
 
-int32_t XCOFFObjectFile::getTimeStamp() const {
-  return FileHdrPtr->TimeStamp;
-}
+  if ((EC = getObject(SymbolTblPtr, Data, base() + CurPtr, SymbolTableSize)))
+    return;
 
-uint32_t XCOFFObjectFile::getSymbolTableOffset() const {
-  return FileHdrPtr->SymbolTableOffset;
-}
+  // Move pointer to the string table.
+  CurPtr += SymbolTableSize;
 
-int32_t XCOFFObjectFile::getNumberOfSymbolTableEntries() const {
-  // As far as symbol table size is concerned, if this field is negative it is
-  // to be treated as a 0. However since this field is also used for printing we
-  // don't want to truncate any negative values.
-  return FileHdrPtr->NumberOfSymTableEntries;
-}
+  if (CurPtr + 4 > Data.getBufferSize())
+    return;
 
-uint16_t XCOFFObjectFile::getOptionalHeaderSize() const {
-  return FileHdrPtr->AuxHeaderSize;
-}
+  StringTable.Size = support::endian::read32be(base() + CurPtr);
+
+  if (StringTable.Size <= 4)
+    return;
+
+  // Check for whether the String table has the size indicated by length
+  // field
+  if (!checkSize(Data, EC, CurPtr, StringTable.Size))
+    return;
 
-uint16_t XCOFFObjectFile::getFlags() const {
-  return FileHdrPtr->Flags;
+  StringTable.Data = reinterpret_cast<const char *>(base() + CurPtr);
+  if (StringTable.Data[StringTable.Size - 1] != '\0') {
+    EC = object_error::string_table_non_null_end;
+    return;
+  }
 }
 
 Expected<std::unique_ptr<ObjectFile>>
 ObjectFile::createXCOFFObjectFile(MemoryBufferRef Object) {
   StringRef Data = Object.getBuffer();
   file_magic Type = identify_magic(Data);
   std::error_code EC;
   std::unique_ptr<ObjectFile> Ret;

llvm/lib/ObjectYAML/XCOFFYAML.cpp

 //===-- XCOFFYAML.cpp - XCOFF YAMLIO implementation -------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines classes for handling the YAML representation of XCOFF.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ObjectYAML/XCOFFYAML.h"
+#include "llvm/BinaryFormat/XCOFF.h"
+#include "llvm/Support/YAMLTraits.h"
 #include <string.h>
 
 namespace llvm {
 namespace XCOFFYAML {
 
 Object::Object() { memset(&Header, 0, sizeof(Header)); }
 
 } // namespace XCOFFYAML
 
 namespace yaml {
 
+void ScalarEnumerationTraits<XCOFF::StorageClass>::enumeration(
+    IO &IO, XCOFF::StorageClass &Value) {
+#define ECase(X) IO.enumCase(Value, #X, XCOFF::X)
+  ECase(C_NULL);
+  ECase(C_AUTO);
+  ECase(C_EXT);
+  ECase(C_STAT);
+  ECase(C_REG);
+  ECase(C_EXTDEF);
+  ECase(C_LABEL);
+  ECase(C_ULABEL);
+  ECase(C_MOS);
+  ECase(C_ARG);
+  ECase(C_STRTAG);
+  ECase(C_MOU);
+  ECase(C_UNTAG);
+  ECase(C_TPDEF);
+  ECase(C_USTATIC);
+  ECase(C_ENTAG);
+  ECase(C_MOE);
+  ECase(C_REGPARM);
+  ECase(C_FIELD);
+  ECase(C_BLOCK);
+  ECase(C_FCN);
+  ECase(C_EOS);
+  ECase(C_FILE);
+  ECase(C_LINE);
+  ECase(C_ALIAS);
+  ECase(C_HIDDEN);
+  ECase(C_HIDEXT);
+  ECase(C_BINCL);
+  ECase(C_EINCL);
+  ECase(C_INFO);
+  ECase(C_WEAKEXT);
+  ECase(C_DWARF);
+  ECase(C_GSYM);
+  ECase(C_LSYM);
+  ECase(C_PSYM);
+  ECase(C_RSYM);
+  ECase(C_RPSYM);
+  ECase(C_STSYM);
+  ECase(C_TCSYM);
+  ECase(C_BCOMM);
+  ECase(C_ECOML);
+  ECase(C_ECOMM);
+  ECase(C_DECL);
+  ECase(C_ENTRY);
+  ECase(C_FUN);
+  ECase(C_BSTAT);
+  ECase(C_ESTAT);
+  ECase(C_GTLS);
+  ECase(C_STTLS);
+  ECase(C_EFCN);
+#undef ECase
+}
+
 void MappingTraits<XCOFFYAML::FileHeader>::mapping(
     IO &IO, XCOFFYAML::FileHeader &FileHdr) {
   IO.mapRequired("MagicNumber", FileHdr.Magic);
   IO.mapRequired("NumberOfSections", FileHdr.NumberOfSections);
   IO.mapRequired("CreationTime", FileHdr.TimeStamp);
   IO.mapRequired("OffsetToSymbolTable", FileHdr.SymbolTableOffset);
   IO.mapRequired("EntriesInSymbolTable", FileHdr.NumberOfSymTableEntries);
   IO.mapRequired("AuxiliaryHeaderSize", FileHdr.AuxHeaderSize);
   IO.mapRequired("Flags", FileHdr.Flags);
 }
 
+void MappingTraits<XCOFFYAML::Symbol>::mapping(IO &IO, XCOFFYAML::Symbol &S) {
+  IO.mapRequired("Name", S.SymbolName);
+  IO.mapRequired("Value", S.Value);
+  IO.mapRequired("Section", S.SectionName);
+  IO.mapRequired("Type", S.Type);
+  IO.mapRequired("StorageClass", S.StorageClass);
+  IO.mapRequired("NumberOfAuxEntries", S.NumberOfAuxEntries);
+}
+
 void MappingTraits<XCOFFYAML::Object>::mapping(IO &IO, XCOFFYAML::Object &Obj) {
   IO.mapTag("!XCOFF", true);
   IO.mapRequired("FileHeader", Obj.Header);
+  IO.mapRequired("Symbols", Obj.Symbols);
 }
 
 } // namespace yaml
 } // namespace llvm

llvm/test/tools/obj2yaml/aix_xcoff.test

 # RUN: obj2yaml %S/Inputs/aix_xcoff.o | FileCheck %s
 # Test that we can parse the XCOFF object file correctly.
 # CHECK: --- !XCOFF
-# CHECK-NEXT: FileHeader:
-# CHECK-NEXT: MagicNumber:     0x01DF
-# CHECK-NEXT: NumberOfSections: 2
-# CHECK-NEXT: CreationTime:    1548692020
-# CHECK-NEXT: OffsetToSymbolTable: 0x00000108
-# CHECK-NEXT: EntriesInSymbolTable: 18
-# CHECK-NEXT: AuxiliaryHeaderSize: 0
-# CHECK-NEXT: Flags:           0x0000
+# CHECK-NEXT: FileHeader:      
+# CHECK-NEXT:   MagicNumber:     0x01DF
+# CHECK-NEXT:   NumberOfSections: 2
+# CHECK-NEXT:   CreationTime:    1552337792
+# CHECK-NEXT:   OffsetToSymbolTable: 0x0000013A
+# CHECK-NEXT:   EntriesInSymbolTable: 22
+# CHECK-NEXT:   AuxiliaryHeaderSize: 0
+# CHECK-NEXT:   Flags:           0x0000
+
+# CHECK: Symbols:         
+# CHECK-NEXT:   - Name:      .file
+# CHECK-NEXT:     Value:     0x00000000
+# CHECK-NEXT:     Section: N_DEBUG
+# CHECK-NEXT:     Type:      0x0003
+# CHECK-NEXT:     StorageClass:    C_FILE
+# CHECK-NEXT:     NumberOfAuxEntries: 1
+# CHECK-NEXT:   - Name:      i
+# CHECK-NEXT:     Value:     0x00000000
+# CHECK-NEXT:     Section: N_UNDEF
+# CHECK-NEXT:     Type:      0x0000
+# CHECK-NEXT:     StorageClass:    C_EXT
+# CHECK-NEXT:     NumberOfAuxEntries: 1
+# CHECK-NEXT:   - Name:      TestforXcoff
+# CHECK-NEXT:     Value:     0x00000000
+# CHECK-NEXT:     Section: N_UNDEF
+# CHECK-NEXT:     Type:      0x0000
+# CHECK-NEXT:     StorageClass:    C_EXT
+# CHECK-NEXT:     NumberOfAuxEntries: 1
+# CHECK-NEXT:   - Name:      .text
+# CHECK-NEXT:     Value:     0x00000000
+# CHECK-NEXT:     Section: .text
+# CHECK-NEXT:     Type:      0x0000
+# CHECK-NEXT:     StorageClass:    C_HIDEXT
+# CHECK-NEXT:     NumberOfAuxEntries: 1
+# CHECK-NEXT:   - Name:      .main
+# CHECK-NEXT:     Value:     0x00000000
+# CHECK-NEXT:     Section: .text
+# CHECK-NEXT:     Type:      0x0000
+# CHECK-NEXT:     StorageClass:    C_EXT
+# CHECK-NEXT:     NumberOfAuxEntries: 1
+# CHECK-NEXT:   - Name:      main
+# CHECK-NEXT:     Value:     0x00000060
+# CHECK-NEXT:     Section: .data
+# CHECK-NEXT:     Type:      0x0000
+# CHECK-NEXT:     StorageClass:    C_HIDEXT
+# CHECK-NEXT:     NumberOfAuxEntries: 1
+# CHECK-NEXT:   - Name:      main
+# CHECK-NEXT:     Value:     0x00000060
+# CHECK-NEXT:     Section: .data
+# CHECK-NEXT:     Type:      0x0000
+# CHECK-NEXT:     StorageClass:    C_EXT
+# CHECK-NEXT:     NumberOfAuxEntries: 1
+# CHECK-NEXT:   - Name:      .data
+# CHECK-NEXT:     Value:     0x00000070
+# CHECK-NEXT:     Section: .data
+# CHECK-NEXT:     Type:      0x0000
+# CHECK-NEXT:     StorageClass:    C_HIDEXT
+# CHECK-NEXT:     NumberOfAuxEntries: 1
+# CHECK-NEXT:   - Name:      TOC
+# CHECK-NEXT:     Value:     0x00000074
+# CHECK-NEXT:     Section: .data
+# CHECK-NEXT:     Type:      0x0000
+# CHECK-NEXT:     StorageClass:    C_HIDEXT
+# CHECK-NEXT:     NumberOfAuxEntries: 1
+# CHECK-NEXT:   - Name:      i
+# CHECK-NEXT:     Value:     0x00000074
+# CHECK-NEXT:     Section: .data
+# CHECK-NEXT:     Type:      0x0000
+# CHECK-NEXT:     StorageClass:    C_HIDEXT
+# CHECK-NEXT:     NumberOfAuxEntries: 1
+# CHECK-NEXT:   - Name:      TestforXcoff
+# CHECK-NEXT:     Value:     0x00000078
+# CHECK-NEXT:     Section: .data
+# CHECK-NEXT:     Type:      0x0000
+# CHECK-NEXT:     StorageClass:    C_HIDEXT
+# CHECK-NEXT:     NumberOfAuxEntries: 1
+# CHECK-NEXT: ...

llvm/tools/llvm-readobj/XCOFFDumper.cpp

if (TimeStamp > 0) {
     else
       W.printHex("Timestamp", TimeStamp);
   } else {
     W.printHex("TimeStamp", TimeStamp == 0 ? "None" : "Reserved Value",
                TimeStamp);
   }
 
   W.printHex("SymbolTableOffset", Obj.getSymbolTableOffset());
-  int32_t SymTabEntries = Obj.getNumberOfSymbolTableEntries();
+  int32_t SymTabEntries = Obj.getRawNumberOfSymbolTableEntries();
   if (SymTabEntries >= 0)
     W.printNumber("SymbolTableEntries", SymTabEntries);
   else
     W.printHex("SymbolTableEntries", "Reserved Value", SymTabEntries);
 
   W.printHex("OptionalHeaderSize", Obj.getOptionalHeaderSize());
   W.printHex("Flags", Obj.getFlags());
 

llvm/tools/obj2yaml/xcoff2yaml.cpp

 using namespace llvm;
 using namespace llvm::object;
 namespace {
 
 class XCOFFDumper {
   const object::XCOFFObjectFile &Obj;
   XCOFFYAML::Object YAMLObj;
   void dumpHeader();
+  std::error_code dumpSymbols();
 
 public:
-  XCOFFDumper(const object::XCOFFObjectFile &obj);
+  XCOFFDumper(const object::XCOFFObjectFile &obj) : Obj(obj) {}
+  std::error_code dump();
   XCOFFYAML::Object &getYAMLObj() { return YAMLObj; }
 };
 } // namespace
 
-XCOFFDumper::XCOFFDumper(const object::XCOFFObjectFile &obj) : Obj(obj) {
+std::error_code XCOFFDumper::dump() {
+  std::error_code EC;
   dumpHeader();
+  if ((EC = dumpSymbols()))
+    return EC;
 }
 
 void XCOFFDumper::dumpHeader() {
   const XCOFFFileHeader *FileHdrPtr = Obj.getFileHeader();
 
   YAMLObj.Header.Magic = FileHdrPtr->Magic;
   YAMLObj.Header.NumberOfSections = FileHdrPtr->NumberOfSections;
   YAMLObj.Header.TimeStamp = FileHdrPtr->TimeStamp;
   YAMLObj.Header.SymbolTableOffset = FileHdrPtr->SymbolTableOffset;
   YAMLObj.Header.NumberOfSymTableEntries = FileHdrPtr->NumberOfSymTableEntries;
   YAMLObj.Header.AuxHeaderSize = FileHdrPtr->AuxHeaderSize;
   YAMLObj.Header.Flags = FileHdrPtr->Flags;
 }
 
+std::error_code XCOFFDumper::dumpSymbols() {
+  std::vector<XCOFFYAML::Symbol> &Symbols = YAMLObj.Symbols;
+
+  for (const SymbolRef &S : Obj.symbols()) {
+    DataRefImpl SymbolDRI = S.getRawDataRefImpl();
+    const XCOFFSymbolEntry *SymbolEntPtr = Obj.toSymbolEntry(SymbolDRI);
+    XCOFFYAML::Symbol Sym;
+
+    Expected<StringRef> SymNameRefOrErr = Obj.getSymbolName(SymbolDRI);
+    if (!SymNameRefOrErr) {
+      return errorToErrorCode(SymNameRefOrErr.takeError());
+    }
+    Sym.SymbolName = SymNameRefOrErr.get();
+
+    Sym.Value = SymbolEntPtr->Value;
+
+    Expected<StringRef> SectionNameRefOrErr =
+        Obj.getSymbolSectionName(SymbolEntPtr);
+    if (!SectionNameRefOrErr) 
+      return errorToErrorCode(SectionNameRefOrErr.takeError());
+    
+    Sym.SectionName = SectionNameRefOrErr.get();
+
+    Sym.Type = SymbolEntPtr->SymbolType;
+    Sym.StorageClass = SymbolEntPtr->StorageClass;
+    Sym.NumberOfAuxEntries = SymbolEntPtr->NumberOfAuxEntries;
+    Symbols.push_back(Sym);
+  }
+
+  return std::error_code();
+}
+
 std::error_code xcoff2yaml(raw_ostream &Out,
                            const object::XCOFFObjectFile &Obj) {
   XCOFFDumper Dumper(Obj);
+
+  if (std::error_code EC = Dumper.dump())
+    return EC;
+
   yaml::Output Yout(Out);
   Yout << Dumper.getYAMLObj();
 
   return std::error_code();
 }