[yaml2obj] Enable support for parsing 64-bit XCOFF.

[Esme]

Summary

Add support for yaml2obj to parse 64-bit XCOFF.

[Higuoxing]

llvm/include/llvm/BinaryFormat/XCOFF.h

263

It looks that FileHeader32 and FileHeader64 are very similar. Instead of creating a seperate struct, can we change the type of SymbolTableFileOffset to uint64_t in FileHeader32, and rename it to FileHeader?

struct FileHeader {
  uint16_t Magic;
  uint16_t NumberOfSections;
  int32_t TimeStamp;
  uint64_t SymbolTableFileOffset;
  int32_t NumberOfSymbolTableEntries;
  uint16_t AuxiliaryHeaderSize;
  uint16_t Flags;
};

263

Ditto.

struct SectionHeader {
  char Name[XCOFF::NameSize];
  uint64_t PhysicalAddress;
  uint64_t VirtualAddress;
  uint64_t Size;
  uint64_t FileOffsetToData;
  uint64_t FileOffsetToRelocations;
  uint64_t FileOffsetToLineNumbers;
  uint32_t NumberOfRelocations;
  uint32_t NumberOfLineNumbers;
  int32_t Flags;
  char Padding[4];
};

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

40

StringRef is small and pervasive enough in LLVM that it should always be passed by value. (https://bcain-llvm.readthedocs.io/projects/llvm/en/latest/ProgrammersManual/#the-stringref-class)

231

It looks that char Padding[4]; isn't used here?

[Esme]

Addressed Higuoxing's comments.

llvm/include/llvm/BinaryFormat/XCOFF.h

263

It seems we could remove these structs and define size constants directly.

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

231

Here we padded 4 bytes in the end of SectionsHeader64, which is a reserved field.

[jhenderson]

The basic code looks good, but I have no concept of whether it is correct or not.

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

68

If I'm following it correctly, XCOFF32 can use a string table, and XCOFF64 always does?

If that's the case it would be a good idea to split this change in two. The first adds string table support to XCOFF32 and the second adds XCOFF64 support. It also ensures we have testing for XCOFF32 + using a string table (which I don't think this change currently has, if I'm not mistaken).

350

I don't know if this makes sense, but do we need a test case where there are no symbols, and therefore no string table?

llvm/test/tools/yaml2obj/XCOFF/basic-doc64.yaml

89

Nit here and below.

[Esme]

Addressed James's comments.

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

350

The string table contains only long symbol names, so it's reasonable that there would be no string table without symbols.

[sfertile]

Thanks for working on this. I'm excited to finally replace all the precompiled objects we used for testing the XCOFF tools implementation. I've only got one initial comment since I'm not real familiar with this part fo the code yet, but will spend some more time going over this tomorrow.

llvm/include/llvm/BinaryFormat/XCOFF.h

29

Please put the replacement of FileHeader32/SectionHeader32 structs by FileHeaderSize32/SectionHeaderSize32 constants into an NFC patch and commit it separately.

[Esme]

Rebase.

[Esme]

Rebase.

[jhenderson]

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

278–279

Didn't notice this before when reviewing the 32-bit implementation: why is this check here? Same comment for other such checks.

350

Right, but we should probably have a test for that case, i.e. show that a no-symbol object produces no string table or an empty string table as appropriate.

llvm/test/tools/yaml2obj/XCOFF/basic-doc64.yaml

23

Similar to my comments on the string table review - it would make sense to have a check in this test which shows all the symbol names are in the string table and not elsewhere, I think, by dumping the string table contents.

[Esme]

As for the need of testing the string table, I will add such test after llvm-readobj --string-table is implemented.
For now, I can use the system tool dump under AIX to verify the results.

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

278–279

As Zheng's comment in D95505

We don't have to call write_zeros if PaddingSize is 0

[jhenderson]

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

278–279

You're right that we don't have to, but by adding the guard, you complicate the logic. Calling it is harmless.

For a similar case, you don't put an if (!x.empty()) around a loop which iterates over the elements of x, for example.

[sfertile]

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

68

Yes, 32-bit XCOFF can store the name directly in the symbol table entry if it fits (8 bytes or less), otherwise it gets placed into the string table and then the first 4 bytes of the field are zeros followed by a 4-byte offset into the string table. The 64-bit format avoids the extra complexity and always places the name in the string table.

135

Shouldn't this be updated to assign XCOFF64 magic number when Is64Bit is true? It seems odd not to, although looking at the other parts of the code I think it will still work. I missed the previous review, so can you help me understand why we need an InitFileHdr and the file header that's a member of Obj? The mixture of duplication, and sometimes having the relevant/correct info on Obj.Header and sometimes on InitFileHdr is rather cumbersome to follow.

[shchenz]

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

278–279

Either way is ok to me. Both ways have their pros and cons.
We used to add a guard for the same cases(write_zeros() for padding) in XCOFFObjectWriter.cpp.

[Esme]

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

135

Most of the key-values are optional in YAML, which allows users to write test inputs with more flexibility. For some optional fields, we have to calculated the default values that are derived from the contents of the YAML, like InitFileHdr.NumberOfSymTableEntries. And we have an option to override that value when writing to object file, that is, use the derived value when the field is omitted, otherwise keep the value that defined in YAML.

The MagicNumber in FileHeader is optional, and we use XCOFF::XCOFF32 as a default value if user did not define the key-value in YAML.

[Esme]

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

135

In addition, for some values such as offset, they must be finalized in advance to write a correct object file, regardless of whether the value is omitted.

[jhenderson]

Looks good barring a nit, from my point of view, but you should get someone who knows the XCOFF file format better to review this to ensure the content is correct and tested satisfactorily.

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

65–71

[Esme]

Thanks! @jhenderson
Hi, @sfertile, could you help to review the patch? thx.

[shchenz]

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

193

number of entries in symbol table should be after SymbolTableOffset. This should align with XCOFF32?

300–301

delete the check to align with others

328

Would it be more sense to use the defined const SymbolTableEntrySize for 18?

llvm/test/tools/yaml2obj/XCOFF/basic-doc64.yaml

74

The physicaladdress and the virtualaddress should be not 0 as the previous section's size is 8?

[Esme]

Address comments.

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

193

We should not change the order.
In XCOFF32, the order of fields is SymbolTableOffset, NumberOfSymTableEntries, AuxHeaderSize, Flags.
In XCOFF64, the order is SymbolTableOffset, AuxHeaderSize, Flags, NumberOfSymTableEntries.

llvm/test/tools/yaml2obj/XCOFF/basic-doc64.yaml

74

It's a non-data/text/bss section, so the address is set to 0 as the spec required. I will change the section type to bss to check the address value. thx.

[jhenderson]

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

65–71

Ping - this comment hasn't been addressed.

[Esme]

Addressed.

[shchenz]

LGTM. Thanks for doing this.

llvm/include/llvm/BinaryFormat/XCOFF.h

 template <unsigned> class SmallString;
 
 namespace XCOFF {
 
 // Constants used in the XCOFF definition.
 
 constexpr size_t FileNamePadSize = 6;
 constexpr size_t NameSize = 8;
+constexpr size_t FileHeaderSize32 = 20;
+constexpr size_t FileHeaderSize64 = 24;
+constexpr size_t SectionHeaderSize32 = 40;
+constexpr size_t SectionHeaderSize64 = 72;
 constexpr size_t SymbolTableEntrySize = 18;
 constexpr size_t RelocationSerializationSize32 = 10;
+constexpr size_t RelocationSerializationSize64 = 14;
 constexpr uint16_t RelocOverflow = 65535;
 constexpr uint8_t AllocRegNo = 31;
 
 enum ReservedSectionNum : int16_t { N_DEBUG = -2, N_ABS = -1, N_UNDEF = 0 };
 
 enum MagicNumber : uint16_t { XCOFF32 = 0x01DF, XCOFF64 = 0x01F7 };
 
 // x_smclas field of x_csect from system header: /usr/include/syms.h
enum RelocationType : uint8_t {
   R_TLSML = 0x25, ///< Module reference to the local TLS storage.
 
   R_TOCU = 0x30, ///< Relative to TOC upper. Specifies the high-order 16 bits of
                  ///< a large code model TOC-relative relocation.
   R_TOCL = 0x31 ///< Relative to TOC lower. Specifies the low-order 16 bits of a
                 ///< large code model TOC-relative relocation.
 };
 
-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;
-  uint16_t NumberOfRelocations;
-  uint16_t NumberOfLineNumbers;
-  int32_t Flags;
-};
-
 enum CFileStringType : uint8_t {
   XFT_FN = 0,  ///< Specifies the source-file name.
   XFT_CT = 1,  ///< Specifies the compiler time stamp.
   XFT_CV = 2,  ///< Specifies the compiler version number.
   XFT_CD = 128 ///< Specifies compiler-defined information.
 };
 
 enum CFileLangId : uint8_t {
   TB_C = 0,        ///< C language.

llvm/lib/MC/XCOFFObjectWriter.cpp

for (auto *Section : Sections) {
     // DefaultSectionAlign.
     Address = alignTo(Address, DefaultSectionAlign);
     Section->Size = Address - Section->Address;
   }
 
   SymbolTableEntryCount = SymbolTableIndex;
 
   // Calculate the RawPointer value for each section.
-  uint64_t RawPointer = sizeof(XCOFF::FileHeader32) + auxiliaryHeaderSize() +
-                        SectionCount * sizeof(XCOFF::SectionHeader32);
+  uint64_t RawPointer = XCOFF::FileHeaderSize32 + auxiliaryHeaderSize() +
+                        SectionCount * XCOFF::SectionHeaderSize32;
   for (auto *Sec : Sections) {
     if (Sec->Index == Section::UninitializedIndex || Sec->IsVirtual)
       continue;
 
     Sec->FileOffsetToData = RawPointer;
     RawPointer += Sec->Size;
     if (RawPointer > UINT32_MAX)
       report_fatal_error("Section raw data overflowed this object file.");

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

 //===- yaml2xcoff - Convert YAML to a xcoff object file -------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
 /// The xcoff component of yaml2obj.
 ///
 //===----------------------------------------------------------------------===//
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/BinaryFormat/XCOFF.h"
+#include "llvm/MC/StringTableBuilder.h"
 #include "llvm/Object/XCOFFObjectFile.h"
 #include "llvm/ObjectYAML/ObjectYAML.h"
 #include "llvm/ObjectYAML/yaml2obj.h"
 #include "llvm/Support/EndianStream.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 
 namespace {
 
 constexpr unsigned DefaultSectionAlign = 4;
 constexpr int16_t MaxSectionIndex = INT16_MAX;
 constexpr uint32_t MaxRawDataSize = UINT32_MAX;
 
 class XCOFFWriter {
 public:
   XCOFFWriter(XCOFFYAML::Object &Obj, raw_ostream &OS, yaml::ErrorHandler EH)
-      : Obj(Obj), W(OS, support::big), ErrHandler(EH) {}
+      : Obj(Obj), W(OS, support::big), ErrHandler(EH),
+        Strings(StringTableBuilder::XCOFF) {
+    is64Bit = Obj.Header.Magic == XCOFF::XCOFF64;
+  }
   bool writeXCOFF();
 
 private:
+  bool nameShouldBeInStringTable(StringRef SymbolName);
   bool initFileHeader(uint64_t CurrentOffset);
   bool initSectionHeader(uint64_t &CurrentOffset);
   bool initRelocations(uint64_t &CurrentOffset);
   bool assignAddressesAndIndices();
   void writeFileHeader();
   void writeSectionHeader();
   bool writeSectionData();
   bool writeRelocations();
   bool writeSymbols();
 
   XCOFFYAML::Object &Obj;
+  bool is64Bit = false;
   support::endian::Writer W;
   yaml::ErrorHandler ErrHandler;
+  StringTableBuilder Strings;
   uint64_t StartOffset;
   // Map the section name to its corrresponding section index.
   DenseMap<StringRef, int16_t> SectionIndexMap = {
       {StringRef("N_DEBUG"), XCOFF::N_DEBUG},
       {StringRef("N_ABS"), XCOFF::N_ABS},
       {StringRef("N_UNDEF"), XCOFF::N_UNDEF}};
   XCOFFYAML::FileHeader InitFileHdr = Obj.Header;
   std::vector<XCOFFYAML::Section> InitSections = Obj.Sections;
 };
 
+bool XCOFFWriter::nameShouldBeInStringTable(StringRef SymbolName) {
+  // For XCOFF64: The symbol name is always in string table or .debug section.
+  return (SymbolName.size() > XCOFF::NameSize) || is64Bit;
+}
+
 bool XCOFFWriter::initRelocations(uint64_t &CurrentOffset) {
   for (uint16_t I = 0, E = InitSections.size(); I < E; ++I) {
     if (!InitSections[I].Relocations.empty()) {
       InitSections[I].NumberOfRelocations = InitSections[I].Relocations.size();
       InitSections[I].FileOffsetToRelocations = CurrentOffset;
-      CurrentOffset += InitSections[I].NumberOfRelocations *
-                       XCOFF::RelocationSerializationSize32;
+      uint64_t RelSize = is64Bit ? XCOFF::RelocationSerializationSize64
+                                 : XCOFF::RelocationSerializationSize32;
+      CurrentOffset += InitSections[I].NumberOfRelocations * RelSize;
       if (CurrentOffset > MaxRawDataSize) {
         ErrHandler("maximum object size of" + Twine(MaxRawDataSize) +
                    "exceeded when writing relocation data");
         return false;
       }
     }
   }
   return true;
bool XCOFFWriter::initSectionHeader(uint64_t &CurrentOffset) {
   }
   if (!initRelocations(CurrentOffset))
     return false;
   return true;
 }
 
 bool XCOFFWriter::initFileHeader(uint64_t CurrentOffset) {
   // The default format of the object file is XCOFF32.
   InitFileHdr.Magic = XCOFF::XCOFF32;
   InitFileHdr.NumberOfSections = Obj.Sections.size();
+  InitFileHdr.NumberOfSymTableEntries = Obj.Symbols.size();
 
+  for (const XCOFFYAML::Symbol &YamlSym : Obj.Symbols) {
     // Add the number of auxiliary symbols to the total number.
-  InitFileHdr.NumberOfSymTableEntries = Obj.Symbols.size();
-  for (const XCOFFYAML::Symbol &YamlSym : Obj.Symbols)
     InitFileHdr.NumberOfSymTableEntries += YamlSym.NumberOfAuxEntries;
+    if (nameShouldBeInStringTable(YamlSym.SymbolName))
+      Strings.add(YamlSym.SymbolName);
+  }
+  // Finalize the string table.
+  Strings.finalize();
 
   // Calculate SymbolTableOffset for the file header.
   if (InitFileHdr.NumberOfSymTableEntries) {
     InitFileHdr.SymbolTableOffset = CurrentOffset;
     CurrentOffset +=
         InitFileHdr.NumberOfSymTableEntries * XCOFF::SymbolTableEntrySize;
     if (CurrentOffset > MaxRawDataSize) {
       ErrHandler("maximum object size of" + Twine(MaxRawDataSize) +
                  "exceeded when writing symbols");
       return false;
     }
   }
   // TODO: Calculate FileOffsetToLineNumbers when line number supported.
   return true;
 }
 
 bool XCOFFWriter::assignAddressesAndIndices() {
-  uint64_t CurrentOffset =
-      sizeof(XCOFF::FileHeader32) /* TODO: + auxiliaryHeaderSize() */ +
-      InitSections.size() * sizeof(XCOFF::SectionHeader32);
+  uint64_t CurrentOffset = 0;
+  Strings.clear();
+  uint64_t FileHdrSize =
+      is64Bit ? XCOFF::FileHeaderSize64 : XCOFF::FileHeaderSize32;
+  uint64_t SecHdrSize =
+      is64Bit ? XCOFF::SectionHeaderSize64 : XCOFF::SectionHeaderSize32;
+  CurrentOffset = FileHdrSize /* TODO: + auxiliaryHeaderSize() */ +
+                  InitSections.size() * SecHdrSize;
 
   // Calculate section header info.
   if (!initSectionHeader(CurrentOffset))
     return false;
   // Calculate file header info.
   return initFileHeader(CurrentOffset);
 }
 
 void XCOFFWriter::writeFileHeader() {
   W.write<uint16_t>(Obj.Header.Magic ? Obj.Header.Magic : InitFileHdr.Magic);
   W.write<uint16_t>(Obj.Header.NumberOfSections ? Obj.Header.NumberOfSections
                                                 : InitFileHdr.NumberOfSections);
   W.write<int32_t>(Obj.Header.TimeStamp);
+  if (is64Bit) {
+    W.write<uint64_t>(Obj.Header.SymbolTableOffset
+                          ? Obj.Header.SymbolTableOffset
+                          : InitFileHdr.SymbolTableOffset);
+    W.write<uint16_t>(Obj.Header.AuxHeaderSize);
+    W.write<uint16_t>(Obj.Header.Flags);
+    W.write<int32_t>(Obj.Header.NumberOfSymTableEntries
+                         ? Obj.Header.NumberOfSymTableEntries
+                         : InitFileHdr.NumberOfSymTableEntries);
+  } else {
     W.write<uint32_t>(Obj.Header.SymbolTableOffset
                           ? Obj.Header.SymbolTableOffset
                           : InitFileHdr.SymbolTableOffset);
     W.write<int32_t>(Obj.Header.NumberOfSymTableEntries
                          ? Obj.Header.NumberOfSymTableEntries
                          : InitFileHdr.NumberOfSymTableEntries);
     W.write<uint16_t>(Obj.Header.AuxHeaderSize);
     W.write<uint16_t>(Obj.Header.Flags);
   }
+}
 
 void XCOFFWriter::writeSectionHeader() {
   for (uint16_t I = 0, E = Obj.Sections.size(); I < E; ++I) {
     XCOFFYAML::Section YamlSec = Obj.Sections[I];
     XCOFFYAML::Section DerivedSec = InitSections[I];
     W.OS.write(YamlSec.SectionName.data() ? YamlSec.SectionName.data()
                                           : "N_NULL",
                sizeof(XCOFF::NameSize));
     // Virtual address is the same as physical address.
-    uint32_t SectionAddress =
+    uint64_t SectionAddress =
         YamlSec.Address ? YamlSec.Address : DerivedSec.Address;
+    if (is64Bit) {
+      W.write<uint64_t>(SectionAddress); // Physical address
+      W.write<uint64_t>(SectionAddress); // Virtual address
+      W.write<uint64_t>(YamlSec.Size ? YamlSec.Size : DerivedSec.Size);
+      W.write<uint64_t>(YamlSec.FileOffsetToData ? YamlSec.FileOffsetToData
+                                                 : DerivedSec.FileOffsetToData);
+      W.write<uint64_t>(YamlSec.FileOffsetToRelocations
+                            ? YamlSec.FileOffsetToRelocations
+                            : DerivedSec.FileOffsetToRelocations);
+      W.write<uint64_t>(YamlSec.FileOffsetToLineNumbers);
+      W.write<uint32_t>(YamlSec.NumberOfRelocations
+                            ? YamlSec.NumberOfRelocations
+                            : DerivedSec.NumberOfRelocations);
+      W.write<uint32_t>(YamlSec.NumberOfLineNumbers);
+      W.write<int32_t>(YamlSec.Flags);
+      W.OS.write_zeros(4);
+    } else {
       W.write<uint32_t>(SectionAddress); // Physical address
       W.write<uint32_t>(SectionAddress); // Virtual address
       W.write<uint32_t>(YamlSec.Size ? YamlSec.Size : DerivedSec.Size);
       W.write<uint32_t>(YamlSec.FileOffsetToData ? YamlSec.FileOffsetToData
                                                  : DerivedSec.FileOffsetToData);
       W.write<uint32_t>(YamlSec.FileOffsetToRelocations
                             ? YamlSec.FileOffsetToRelocations
                             : DerivedSec.FileOffsetToRelocations);
       W.write<uint32_t>(YamlSec.FileOffsetToLineNumbers);
       W.write<uint16_t>(YamlSec.NumberOfRelocations
                             ? YamlSec.NumberOfRelocations
                             : DerivedSec.NumberOfRelocations);
       W.write<uint16_t>(YamlSec.NumberOfLineNumbers);
       W.write<int32_t>(YamlSec.Flags);
     }
   }
+}
 
 bool XCOFFWriter::writeSectionData() {
   for (uint16_t I = 0, E = Obj.Sections.size(); I < E; ++I) {
     XCOFFYAML::Section YamlSec = Obj.Sections[I];
     if (YamlSec.SectionData.binary_size()) {
       // Fill the padding size with zeros.
       int64_t PaddingSize =
           InitSections[I].FileOffsetToData - (W.OS.tell() - StartOffset);
bool XCOFFWriter::writeRelocations() {
   for (uint16_t I = 0, E = Obj.Sections.size(); I < E; ++I) {
     XCOFFYAML::Section YamlSec = Obj.Sections[I];
     if (!YamlSec.Relocations.empty()) {
       int64_t PaddingSize =
           InitSections[I].FileOffsetToRelocations - (W.OS.tell() - StartOffset);
       if (PaddingSize < 0) {
         ErrHandler("redundant data was written before relocations");
         return false;
       }
       W.OS.write_zeros(PaddingSize);
       for (const XCOFFYAML::Relocation &YamlRel : YamlSec.Relocations) {
+        if (is64Bit)
+          W.write<uint64_t>(YamlRel.VirtualAddress);
+        else
           W.write<uint32_t>(YamlRel.VirtualAddress);
         W.write<uint32_t>(YamlRel.SymbolIndex);
         W.write<uint8_t>(YamlRel.Info);
         W.write<uint8_t>(YamlRel.Type);
       }
     }
   }
   return true;
 }
 
 bool XCOFFWriter::writeSymbols() {
   int64_t PaddingSize =
       (uint64_t)InitFileHdr.SymbolTableOffset - (W.OS.tell() - StartOffset);
   if (PaddingSize < 0) {
     ErrHandler("redundant data was written before symbols");
     return false;
   }
   W.OS.write_zeros(PaddingSize);
   for (const XCOFFYAML::Symbol &YamlSym : Obj.Symbols) {
-    if (YamlSym.SymbolName.size() > XCOFF::NameSize) {
-      ErrHandler("string table is not supported yet");
-      return false;
-    }
+    if (nameShouldBeInStringTable(YamlSym.SymbolName)) {
+      // For XCOFF32: A value of 0 indicates that the symbol name is in the
+      // string table or .debug section.
+      if (!is64Bit)
+        W.write<int32_t>(0);
+      W.write<uint32_t>(Strings.getOffset(YamlSym.SymbolName));
+    } else {
       W.OS.write(YamlSym.SymbolName.data(), sizeof(XCOFF::NameSize));
+    }
+    if (is64Bit)
+      W.write<uint64_t>(YamlSym.Value);
+    else
       W.write<uint32_t>(YamlSym.Value);
     W.write<int16_t>(SectionIndexMap[YamlSym.SectionName]);
     W.write<uint16_t>(YamlSym.Type);
     W.write<uint8_t>(YamlSym.StorageClass);
     W.write<uint8_t>(YamlSym.NumberOfAuxEntries);
 
     // Now output the auxiliary entry.
     for (uint8_t I = 0, E = YamlSym.NumberOfAuxEntries; I < E; ++I) {
       // TODO: Auxiliary entry is not supported yet.
       // The auxiliary entries for a symbol follow its symbol table entry. The
       // length of each auxiliary entry is the same as a symbol table entry (18
       // bytes). The format and quantity of auxiliary entries depend on the
       // storage class (n_sclass) and type (n_type) of the symbol table entry.
       W.OS.write_zeros(18);
     }
   }
   return true;
 }
 
 bool XCOFFWriter::writeXCOFF() {
   if (!assignAddressesAndIndices())
     return false;
   StartOffset = W.OS.tell();
   writeFileHeader();
   if (!Obj.Sections.empty()) {
     writeSectionHeader();
     if (!writeSectionData())
       return false;
     if (!writeRelocations())
       return false;
   }
-  if (!Obj.Symbols.empty())
-    return writeSymbols();
-  // TODO: Add support for the string table.
+  if (!Obj.Symbols.empty()) {
+    if (!writeSymbols())
+      return false;
+    // Write the string table.
+    Strings.write(W.OS);
+  }
   return true;
 }
 
 } // end anonymous namespace
 
 namespace llvm {
 namespace yaml {
 
 bool yaml2xcoff(XCOFFYAML::Object &Doc, raw_ostream &Out, ErrorHandler EH) {
-  bool Is64Bit = Doc.Header.Magic == XCOFF::XCOFF64;
-  if (Is64Bit) {
-    EH("only XCOFF32 is supported now");
-    return false;
-  }
   XCOFFWriter Writer(Doc, Out, EH);
   return Writer.writeXCOFF();
 }
 
 } // namespace yaml
 } // namespace llvm

llvm/test/tools/yaml2obj/XCOFF/basic-doc64.yaml

+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readobj --headers  %t | FileCheck %s --check-prefix=HEADERS64
+# RUN: llvm-readobj --symbols  %t | FileCheck %s --check-prefix=SYMBOLS64
+
+--- !XCOFF
+FileHeader:
+  MagicNumber:     0x01F7
+Sections:
+  - Name:          .text
+    Flags:         0x20
+    SectionData:   "9061FFF8808200008064000038630001906400008061FFF8386300019061FFF88061FFF88082000480840000888400007C6322149061FFF88061FFF84E8000200000000000092040800001010000000000000040000466756E310000600000007C0802A693E1FFFC900100089421FFB07C3F0B7838600000907F004880620008907F0044808300003884000190830000806300004BFFFF6D60000000807F0044806300004BFFFF5D60000000382100508001000883E1FFFC7C0803A64E8000200000000000092261800100010000006000046D61696E1F006162636400000000"
+  - Name:          .data
+    Flags:         0x40
+    SectionData:   "00000000000000FC0000000000000060000000FC00000000000000D800000108000000F800000000"
+    Relocations:
+      - Address:   0x3A
+        Type:      0x02
+  - Flags:         0x80
+Symbols:
+  - Name:            .file
+    Section:         N_DEBUG
+  - Name:            .text
+    Value:           0x0
+    Section:         .text
+    Type:            0x0
+    StorageClass:    C_HIDEXT
+  - Name:            i
+    Value:           0x0
+    Section:         .data
+    Type:            0x0
+    StorageClass:    C_EXT
+
+
+# HEADERS64:       Format: aix5coff64-rs6000
+# HEADERS64-NEXT:  Arch: powerpc64
+# HEADERS64-NEXT:  AddressSize: 64bit
+# HEADERS64-NEXT:  FileHeader {
+# HEADERS64-NEXT:    Magic: 0x1F7
+# HEADERS64-NEXT:    NumberOfSections: 3
+# HEADERS64-NEXT:    TimeStamp: None (0x0)
+# HEADERS64-NEXT:    SymbolTableOffset: 0x206
+# HEADERS64-NEXT:    SymbolTableEntries: 3
+# HEADERS64-NEXT:    OptionalHeaderSize: 0x0
+# HEADERS64-NEXT:    Flags: 0x0
+# HEADERS64-NEXT:  }
+
+# HEADERS64:       Sections [
+# HEADERS64-NEXT:    Section {
+# HEADERS64-NEXT:      Index: 1
+# HEADERS64-NEXT:      Name: .text
+# HEADERS64:           PhysicalAddress: 0x0
+# HEADERS64-NEXT:      VirtualAddress: 0x0
+# HEADERS64-NEXT:      Size: 0xE0
+# HEADERS64-NEXT:      RawDataOffset: 0xF0
+# HEADERS64-NEXT:      RelocationPointer: 0x0
+# HEADERS64-NEXT:      LineNumberPointer: 0x0
+# HEADERS64-NEXT:      NumberOfRelocations: 0
+# HEADERS64-NEXT:      NumberOfLineNumbers: 0
+# HEADERS64-NEXT:      Type: STYP_TEXT (0x20)
+# HEADERS64-NEXT:    }
+# HEADERS64-NEXT:    Section {
+# HEADERS64-NEXT:      Index: 2
+# HEADERS64-NEXT:      Name: .data
+# HEADERS64:           PhysicalAddress: 0xE0
+# HEADERS64-NEXT:      VirtualAddress: 0xE0
+# HEADERS64-NEXT:      Size: 0x28
+# HEADERS64-NEXT:      RawDataOffset: 0x1D0
+# HEADERS64-NEXT:      RelocationPointer: 0x1F8
+# HEADERS64-NEXT:      LineNumberPointer: 0x0
+# HEADERS64-NEXT:      NumberOfRelocations: 1
+# HEADERS64-NEXT:      NumberOfLineNumbers: 0
+# HEADERS64-NEXT:      Type: STYP_DATA (0x40)
+# HEADERS64-NEXT:    }
+# HEADERS64-NEXT:    Section {
+# HEADERS64-NEXT:      Index: 3
+# HEADERS64-NEXT:      Name: N_NULL
+# HEADERS64-NEXT:      PhysicalAddress: 0x108
+# HEADERS64-NEXT:      VirtualAddress: 0x108
+# HEADERS64-NEXT:      Size: 0x0
+# HEADERS64-NEXT:      RawDataOffset: 0x0
+# HEADERS64-NEXT:      RelocationPointer: 0x0
+# HEADERS64-NEXT:      LineNumberPointer: 0x0
+# HEADERS64-NEXT:      NumberOfRelocations: 0
+# HEADERS64-NEXT:      NumberOfLineNumbers: 0
+# HEADERS64-NEXT:      Type: STYP_BSS (0x80)
+# HEADERS64-NEXT:    }
+# HEADERS64-NEXT:  ]
+
+#SYMBOLS64:       Symbols [
+#SYMBOLS64-NEXT:    Symbol {
+#SYMBOLS64-NEXT:      Index: 0
+#SYMBOLS64-NEXT:      Name: 
+#SYMBOLS64-NEXT:      Value: 0xC00000000
+#SYMBOLS64-NEXT:      Section: N_DEBUG
+#SYMBOLS64-NEXT:      Type: 0x0
+#SYMBOLS64-NEXT:      StorageClass: C_NULL (0x0)
+#SYMBOLS64-NEXT:      NumberOfAuxEntries: 0
+#SYMBOLS64-NEXT:    }
+#SYMBOLS64-NEXT:    Symbol {
+#SYMBOLS64-NEXT:      Index: 1
+#SYMBOLS64-NEXT:      Name: 
+#SYMBOLS64-NEXT:      Value (RelocatableAddress): 0x400000000
+#SYMBOLS64-NEXT:      Section: .text    
+#SYMBOLS64:           Type: 0x0
+#SYMBOLS64-NEXT:      StorageClass: C_HIDEXT (0x6B)
+#SYMBOLS64-NEXT:      NumberOfAuxEntries: 0
+#SYMBOLS64-NEXT:    }
+#SYMBOLS64-NEXT:    Symbol {
+#SYMBOLS64-NEXT:      Index: 2
+#SYMBOLS64-NEXT:      Name: 
+#SYMBOLS64-NEXT:      Value (RelocatableAddress): 0xA00000000
+#SYMBOLS64-NEXT:      Section: .data
+#SYMBOLS64:           Type: 0x0
+#SYMBOLS64-NEXT:      StorageClass: C_EXT (0x2)
+#SYMBOLS64-NEXT:      NumberOfAuxEntries: 0
+#SYMBOLS64-NEXT:    }
+#SYMBOLS64-NEXT:  ]