[yaml2obj][XCOFF] Customize the string table.

[Esme]

Summary

The patch is trying to add support for yaml2obj customizing the string table.

1. length and contents can be specified.
2. symbol names in the string table will be overwritten by specific values.
3. symbol names that are not overwritten by specific values will still be stored in the string table, ex. the specific values can't cover all symbol names.
4. the emission of string table can be suppressed.

[jhenderson]

I'm going to come back to looking at this another day, as I'm out of time now.

I do wonder if it would make sense to model the behaviour of symbol names and the string table on the existing ELF behaviour, i.e. what happens when a name isn't present etc. Perhaps worth looking at ELF and mirroring the approach?

llvm/include/llvm/ObjectYAML/XCOFFYAML.h

63

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

39

Seems like this is an unnecessary member variable? Just do Obj.StrTbl.isSuppress everywhere you need it.

148

152

Don't use const & for StringRef types - the type is lightweight and designed to be copied.

247

Customizing is one aspect of initialisation.

[Higuoxing]

llvm/test/tools/yaml2obj/XCOFF/string-table.yaml

47–49

It looks that the string table is reversed?

[Higuoxing]

llvm/lib/MC/StringTableBuilder.cpp

80 ↗

(On Diff #363983)

Personally, I don't like this approach. I would do the overwriting job in the client side. Besides, this change is dangerous, what will happen if Length > Size ?

[Higuoxing]

llvm/lib/MC/StringTableBuilder.cpp

80 ↗

(On Diff #363983)

Sorry, I made a mistake here. I realize that you're modifying the first 4-byte of the buffer. But I still think we should do it in the client side.

[jhenderson]

Having thought about it more, I think we should have a slightly different behaviour for the case where we have symbol names and string table contents. It's likely if you've specified string table contents, you want to override the entire table contents with the specified strings. In this case, I'd do the following:

1. Internally generate the string table data as-if the strings weren't specified explicitly. Use this for populating symbol name offsets.
2. Once the offsets have been generated, throw away that data and use the YAML-specified Strings as the entire table contents.

Finally, you probably need a way to specify the table as raw contents. This allows you to have a test case for a table which is too short for the length field. This could be a "Contents" field (see ELF for similar examples for sections), which cannot be specified with other parameters (yaml2obj should emit an error in this case).

At some point, you should also consider an obj2yaml implementation that makes use of the StringTable property too, if the input is malformed somehow, but that can be a separate patch.

llvm/lib/MC/StringTableBuilder.cpp

80 ↗

(On Diff #363983)

+1 to not modifying the StringTableBuilder interface.

The majority of the clients don't have a need to specify a different size to the length. yaml2obj is different in that it is deliberately trying to create an invalid string table.

llvm/test/tools/yaml2obj/XCOFF/string-table.yaml

2

This test needs a couple more cases:

1. The Length field is specified to be less than the total length of explicitly-specified strings (the data should be written, but the length field specified to be the shorter value).
2. As 1), but where there are implicit contents.
3. As 1), but where the length is greater than the content length (0 bytes should be used for padding probably).
4. As 2), but where the length is greater than the content length (again, pad with 0 bytes).

13–17

Maybe have a case 1b, which is identical but with an empty StringTable tag?

138

We probably want an error emitted by yaml2obj if Suppress is specified at the same time as other string table parameters. This will necessitate two more cases: 1) with Length + Suppress and 2) with Length + Strings

[Esme]

Thank you! @jhenderson and @Higuoxing
After reconsidered from a user's perspective and referred to ELF's approach, I modified some features of the patch.
We can see these features from cases in llvm/test/tools/yaml2obj/XCOFF/string-table.yaml.

This allows you to have a test case for a table which is too short for the length field.

I don't think we could do it because we only generate the string table if its size is bigger than 4: StrTbl.getSize() > 4.
I didn't add the raw "Contents" field because I'm not sure this feature is necessary.

not modifying the StringTableBuilder interface.

I agree that my old way of changing the length value was very unreasonable.
But to pad the string table to the specified length have to be done in the StringTableBuilder interface.

llvm/test/tools/yaml2obj/XCOFF/string-table.yaml

47–49

Yes, it looks like this is how StringTableBuilder stores names. And I thinks it's fine?

[Higuoxing]

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

465–466

I believe You're are able to pad the string table to a specified length without modifying the interface of StringTableBuilder. Please let me know if it doesn't work :)

[Esme]

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

465–466

If so, we have to rewrite the length value in the string table, otherwise these padding zeros will not be treated as string table contents.

[Higuoxing]

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

465–466

I think you can do that by creating a temporary buffer.

1. Serialize string table builder's content together with padding zeros to a temporary buffer.
2. Modify the first 4-byte of the temporary buffer.
3. Serialize the temporary buffer to the actual output stream.

@jhenderson any ideas?

[jhenderson]

In D107421#2936139, @Esme wrote:

Thank you! @jhenderson and @Higuoxing
After reconsidered from a user's perspective and referred to ELF's approach, I modified some features of the patch.
We can see these features from cases in llvm/test/tools/yaml2obj/XCOFF/string-table.yaml.

This allows you to have a test case for a table which is too short for the length field.

I don't think we could do it because we only generate the string table if its size is bigger than 4: StrTbl.getSize() > 4.

It sounds to me like this is an arbitrary an unnecessary check in yaml2obj?

I didn't add the raw "Contents" field because I'm not sure this feature is necessary.

Remember, the goal of yaml2obj is to help craft inputs for tests. How else could you test the case where the string table data is too short/not null terminated etc? Similarly, don't forget that obj2yaml should be able to cope with somewhat malformed input, although in this case, that might not apply.

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

162

I don't think "unoverwritten" is a word. How about "Names that are not overwritten..."?

465–466

Sounds like a reasonable way to do it to me.

[Esme]

Address comment.

[Higuoxing]

The codes generally looks good to me.

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

60

Might be good to rename StrTbl to StrTblBuilder or something else to distinguish it from Obj.StrTbl.

439

Nit: first 4-bytes -> first 4-byte

[Esme]

Address comment.

[jhenderson]

I'm in two minds about whether a length greater than the string length should write padding data. I'm inclined to think we need another optional field. In the ELF case, for sections, we have two relevant fields, a Size field and a ShSize field. The Size field increases the actual content size by padding with zeroes as necessary. It is an error to specify a value for the Size field that is smaller than any Content size. If ShSize is specified, the section header sh_size value is written to that value, regardless of the section content. It has no impact on the section content. If no ShSize is specified, the Size value is used. As such, if you want to change the actual data written, you use Size. Otherwise, you use ShSize.

I've been thinking about it, and I think for XCOFF string tables, the Length field is approximately equivalent to the ELF sh_size section header field. As such, I reckon it should be treated similarly to the ShSize field. You therefore probably want another field, roughly equivalent to the Size field of the ELF section, maybe called ContentSize? I think it would work like this:

1. If RawContent is specified and no ContentSize is specified, or ContentSize matches the size of the RawContent data, write the RawContent as-is and nothing else.
2. If RawContent is specified and ContentSize is less than the RawContent data size, emit an error.
3. If RawContent is specified and ContentSize is greater than the RawContent data size, pad the RawContent with trailing zeroes so that its size now matches ContentSize.
4. It is an error if both RawContent the Strings member are specified.
5. As you've already implemented, Strings should replace the used symbol names. If there are more symbols than strings, add them to the list.
6. It is an error if RawContent is not used and ContentSize is used and ContentSize is less than the size of the data that would otherwise be written.
7. If RawContent is not used and ContentSize is greater than the size of the data that would be written, pad with zeroes, as in case 3 above.
8. It is an error if the Length field is specified as well as RawContent.
9. Otherwise, if specified, use the value of the Length field for the first 4 bytes of the table (note that this means that ContentSize <=3 will always be an error without RawContent). The value may not make sense for the data that is beign written. This is deliberate.
10. If Length is not specified, use the value as defined by the data size AFTER trailing zeroes are added (i.e. Length represents the length of the data written).

Hopefully these steps will provide a good basis for the updated tests.

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

424

I'm not sure we need this special case? What's the use-case for it? Could a user just as easily use raw content to specify it?

429–433

I think we shoul support this case. In other words, if a user has specified a Length field that is shorter than the strings that should be written, just write the specified length field and then all strings with no padding.

440

llvm/lib/ObjectYAML/XCOFFYAML.cpp

155

I don't think we need the Suppress field anymore - a user could use an empty RawContent in this case.

llvm/test/tools/obj2yaml/XCOFF/aix.yaml

80

Either leave this in, or fix the check in this patch.

[Esme]

@jhenderson Thank you so much for your comments which are very valuable, and apologize that I took so long to update the patch as I was busy with other works.

[Esme]

llvm/test/tools/obj2yaml/XCOFF/aix.yaml

80

The null stringtable data is generated automatically, which I think is within the scope of obj2yaml's work?

StringTable:
  TableSize:       0
  Length:          0
  RawContent:      ''

[jhenderson]

llvm/include/llvm/ObjectYAML/XCOFFYAML.h

63–68

These fields probably all want to be Optional values. You need to be able to distinguish the cases where a field has a value of zero/is an empty array/is an empty string from where the field is missing. This is often important for the error case handling, if nothing else.

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

149

You don't need additional space for the Length field if you are using RawContent. If RawContent is used, then you should just have that content, plus some possible trailing padding if TableSize is greater than than the size of the RawContent.

157

This is shorter.

410

As noted above - the RawContent should be just that (plus possible trailing padding for a large TableSize). This + 4 business seems therefore incorrect to me.

I'd expect the code to look something like this:

if(Obj.StrTbl.RawContent) {
  Obj.StrTbl.RawContent.writeAsBinary(W.OS);
  if (Obj.StrTbl.TableSize && *Obj.StrTbl.TableSize > Obj.StrTbl.RawContent.size())
    W.OS.write_zeros(Obj.StrTbl.TableSize - Obj.StrTble.RawContent.size());
  return;
}

[Esme]

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

149

I think it would be confusing that the RawContent includes the length field and also the TableSize is specified, since there would be 3 different values for the string table size, ie. the length field in RawContent, the size of RawContent and the TableSize value.

For example, the RawContent is "000000090062006300" (where the "00000009" is the length field) and the TableSize is 10, then what would the string table look like? "00000009006200630000" or "00000010006200630000"?
And what if the RawContent is incorrect (ex. 000000070062006300) but the TableSize (ex. 9+) is correct?

Besides, I think it is not very convenient for users to calculate the content size for the input RawContent even they don't want to test the length value.

Any thoughts?

[Esme]

llvm/include/llvm/ObjectYAML/XCOFFYAML.h

63–68

Yes, agreed. It looks like some fields in other structs need such change as well, maybe we need a new patch to do this?

[jhenderson]

llvm/include/llvm/ObjectYAML/XCOFFYAML.h

63–68

If you feel it would be useful in other cases, then those cases can be modified in other patches, but the StringTable class should be done in this patch, since it impacts how you customise it.

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

149

I suggested ContentSize, previously as the name, not TableSize, as I feel it is clear that it is tied to the RawContent field. Another more verbose name could be "AmountOfDataToWriteAtTheStringTableOffset" which obviously clarifies the intent but is too long.

I think you are missing the use-case here. When we use "RawContent" it is because we want to precisely control what is written at the offset of where the string table should start. This allows testing of various things like an empty or very short table, for example, or specific data patterns that might be necessary for some other reason. It will likely be fairly rare that RawContent should be used, and even rarer where it should be used and the Length field needs to be valid. In those limited cases, I think it is fine to require the user to provide the length field as part of the data. I also think it would be confusing to implicitly write additional data in addition to "RawContent", because that is different to how content is usually written in yaml2obj.

To answer the questions directly:

For example, the RawContent is "000000090062006300" (where the "00000009" is the length field) and the TableSize is 10, then what would the string table look like? "00000009006200630000" or "00000010006200630000"?

yaml2obj would just write the requested RawContent, and pad with zeroes up to TableSize. The TableSize is the total amount of data written, so there should be a total of 10 bytes written. The data would be 00000009006200630000.

And what if the RawContent is incorrect (ex. 000000070062006300) but the TableSize (ex. 9+) is correct?

As above, yaml2obj would just write the requested RawContent, and pad with zeroes up to TableSize.

This is covered by my points 1 to 3 in my out-of-line cases list from earlier.

[Esme]

Address comment.

[jhenderson]

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

413

I think this second clause can be an assert right? We've already reported an error if the ContentSize is less than the RawContent size, and I believe write_zeros is harmless to call with a zero size.

420

I'm not sure why you're doing this. Length should be optional and a default value generated for it, based on other specified fields, if an explicit one is not specified.

433

Unless I'm missing something, this doesn't seem right. You need 4 bytes for the Length field, and then the entire string table contents. The Length field doesn't overwrite the first four bytes worth of strings...

[Esme]

Address comment.

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

420

If neither Length nor ContentSize is specified, we follow the original code logic, i.e. use StrTblBuilder.write(W .OS) to write the whole table directly with a calculated default length value.
If Length or ContentSize is specified, we have to serialize the string table and rewrite the length field manually (as Higuoxing's comment).
Since the latter approach is more time-consuming, I am doing this here to determine in advance whether we can write the string table directly.

433

StrTblBuilder.write(Ptr); writes the whole content of string table (including the length field) into the buffer and we don't want to change the interface of StringTableBuilder, that's why I have to rewrite the length field here.

void StringTableBuilder::write(uint8_t *Buf) const {
...
  else if (K == XCOFF)
    support::endian::write32be(Buf, Size);
}

[jhenderson]

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

420

Ah, thanks. I missed that bit.

It probably would be helpful to extend the comment to say something to the effect that the Length field is written automatically by the StrTblBuilder.write code.

433

Thanks. Perhaps update the comment to say "Replace the first 4 bytes, which contain the auto-generated Length value, with the specified value."

llvm/test/tools/yaml2obj/XCOFF/string-table.yaml

53

Perhaps test the range edge, by using CONTENTSIZE == actual content size - 1.

It would also be a good idea to have an explicit ContentSize that matches the actual size.

89

129

130

131

As said elsewhere, "unoverwritten" isn't a word.

174

209

Up to you, but you can probably omit the "yaml2obj:" bit of these error message checks (same throughout).

237

Remove duplicate word.

243–244

254

This is an llvm-readobj test case, not a yaml2obj test case. Please remove it from this patch.

270

Probably point out that in this case, the Length value is less than the true length?

Remind me: does the Length value include the size of the Length field itself? If not, this value looks too large for what is reported in the StringTable.

280

Strictly, this is also an llvm-readobj test. Really, we want this case tested in llvm-readobj testing, but have something here to show we can actually generate this input.

I can't think of a way to inspect the actual Length value, short of manually dumping the hex bytes using od at the appropriate offset. We do this in a few other places in these sorts of tests, where there is no other option.

The comment also needs to say how this value is invalid. In this case, I think it's because the end of the string table appears to be off the end of the file?

293

I'd move this case up to around Case 4, since it is similar.

[Esme]

Address comment.
Thanks! @jhenderson

llvm/test/tools/yaml2obj/XCOFF/string-table.yaml

270

Yes, the Length value includes the size of the Length field itself.

[jhenderson]

llvm/lib/Object/XCOFFObjectFile.cpp

811 ↗

(On Diff #371299)

Remove debug printing...

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

422

As suggested already, I recommend extending this comment to say why you can just write it (i.e. the Length field is already included).

Also, when referring to field names, I'd make the name capitalized, i.e. "Length", to match the name in the YAML.

445

Fix this message (you seem to have caused it to regress since your previous diff).

llvm/test/tools/yaml2obj/XCOFF/string-table.yaml

76

Similar comment to elsewhere. When referring to the Length YAML field, use a capital letter. Applies here and below.

292

Probably also dump the bytes that refer to the name, to show that this is the actual string table and not some other abitrariy block where we happen to have written the length value.

[Esme]

Address comment.

[jhenderson]

LGTM, thanks!

llvm/include/llvm/ObjectYAML/XCOFFYAML.h

struct Symbol {
   StringRef SymbolName;
   llvm::yaml::Hex64 Value; // Symbol value; storage class-dependent.
   StringRef SectionName;
   llvm::yaml::Hex16 Type;
   XCOFF::StorageClass StorageClass;
   uint8_t NumberOfAuxEntries;
 };
 
+struct StringTable {
+  Optional<uint32_t> ContentSize; // The total size of the string table.
+  Optional<uint32_t> Length;      // The value of the length field for the first
+                                  // 4 bytes of the table.
+  Optional<std::vector<StringRef>> Strings;
+  Optional<yaml::BinaryRef> RawContent;
+};
+
 struct Object {
   FileHeader Header;
   std::vector<Section> Sections;
   std::vector<Symbol> Symbols;
+  StringTable StrTbl;
   Object();
 };
 } // namespace XCOFFYAML
 } // namespace llvm
 
 LLVM_YAML_IS_SEQUENCE_VECTOR(XCOFFYAML::Symbol)
 LLVM_YAML_IS_SEQUENCE_VECTOR(XCOFFYAML::Relocation)
 LLVM_YAML_IS_SEQUENCE_VECTOR(XCOFFYAML::Section)

 template <> struct MappingTraits<XCOFFYAML::Relocation> {
   static void mapping(IO &IO, XCOFFYAML::Relocation &R);
 };
 
 template <> struct MappingTraits<XCOFFYAML::Section> {
   static void mapping(IO &IO, XCOFFYAML::Section &Sec);
 };
 
+template <> struct MappingTraits<XCOFFYAML::StringTable> {
+  static void mapping(IO &IO, XCOFFYAML::StringTable &Str);
+};
+
 template <> struct MappingTraits<XCOFFYAML::Object> {
   static void mapping(IO &IO, XCOFFYAML::Object &Obj);
 };
 
 } // namespace yaml
 } // namespace llvm
 
 #endif // LLVM_OBJECTYAML_XCOFFYAML_H

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

 
 #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"
 #include "llvm/Support/LEB128.h"
+#include "llvm/Support/MemoryBuffer.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),
-        Strings(StringTableBuilder::XCOFF) {
+        StrTblBuilder(StringTableBuilder::XCOFF) {
     Is64Bit = Obj.Header.Magic == (llvm::yaml::Hex16)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 initStringTable();
   bool assignAddressesAndIndices();
   void writeFileHeader();
   void writeSectionHeader();
   bool writeSectionData();
   bool writeRelocations();
   bool writeSymbols();
+  void writeStringTable();
 
   XCOFFYAML::Object &Obj;
   bool Is64Bit = false;
   support::endian::Writer W;
   yaml::ErrorHandler ErrHandler;
-  StringTableBuilder Strings;
+  StringTableBuilder StrTblBuilder;
   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;
if (InitSections[I].SectionData.binary_size()) {
       CurrentOffset = alignTo(CurrentOffset, DefaultSectionAlign);
       InitSections[I].Size = CurrentOffset - InitSections[I].FileOffsetToData;
       CurrentSecAddr += InitSections[I].Size;
     }
   }
   return initRelocations(CurrentOffset);
 }
 
+bool XCOFFWriter::initStringTable() {
+  if (Obj.StrTbl.RawContent) {
+    size_t RawSize = Obj.StrTbl.RawContent->binary_size();
+    if (Obj.StrTbl.Strings || Obj.StrTbl.Length) {
+      ErrHandler(
+          "can't specify Strings or Length when RawContent is specified");
+      return false;
+    }
+    if (Obj.StrTbl.ContentSize && *Obj.StrTbl.ContentSize < RawSize) {
+      ErrHandler("specified ContentSize (" + Twine(*Obj.StrTbl.ContentSize) +
+                 ") is less than the RawContent data size (" + Twine(RawSize) +
+                 ")");
+      return false;
+    }
+    return true;
+  }
+  if (Obj.StrTbl.ContentSize && *Obj.StrTbl.ContentSize <= 3) {
+    ErrHandler("ContentSize shouldn't be less than 4 without RawContent");
+    return false;
+  }
+
+  // Build the string table.
+  StrTblBuilder.clear();
+
+  if (Obj.StrTbl.Strings) {
+    // All specified strings should be added to the string table.
+    for (StringRef StringEnt : *Obj.StrTbl.Strings)
+      StrTblBuilder.add(StringEnt);
+
+    size_t StrTblIdx = 0;
+    size_t NumOfStrings = Obj.StrTbl.Strings->size();
+    for (XCOFFYAML::Symbol &YamlSym : Obj.Symbols) {
+      if (nameShouldBeInStringTable(YamlSym.SymbolName)) {
+        if (StrTblIdx < NumOfStrings) {
+          // Overwrite the symbol name with the specified string.
+          YamlSym.SymbolName = (*Obj.StrTbl.Strings)[StrTblIdx];
+          ++StrTblIdx;
+        } else
+          // Names that are not overwritten are still stored in the string
+          // table.
+          StrTblBuilder.add(YamlSym.SymbolName);
+      }
+    }
+  } else {
+    for (XCOFFYAML::Symbol &YamlSym : Obj.Symbols) {
+      if (nameShouldBeInStringTable(YamlSym.SymbolName))
+        StrTblBuilder.add(YamlSym.SymbolName);
+    }
+  }
+
+  StrTblBuilder.finalize();
+
+  size_t StrTblSize = StrTblBuilder.getSize();
+  if (Obj.StrTbl.ContentSize && *Obj.StrTbl.ContentSize < StrTblSize) {
+    ErrHandler("specified ContentSize (" + Twine(*Obj.StrTbl.ContentSize) +
+               ") is less than the size of the data that would otherwise be "
+               "written (" +
+               Twine(StrTblSize) + ")");
+    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) {
+  for (const XCOFFYAML::Symbol &YamlSym : Obj.Symbols)
     // Add the number of auxiliary symbols to the total number.
     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() {
-  Strings.clear();
   uint64_t FileHdrSize =
       Is64Bit ? XCOFF::FileHeaderSize64 : XCOFF::FileHeaderSize32;
   uint64_t SecHdrSize =
       Is64Bit ? XCOFF::SectionHeaderSize64 : XCOFF::SectionHeaderSize32;
   uint64_t CurrentOffset = FileHdrSize /* TODO: + auxiliaryHeaderSize() */ +
                            InitSections.size() * SecHdrSize;
 
   // Calculate section header info.
   if (!initSectionHeader(CurrentOffset))
     return false;
+
   // Calculate file header info.
-  return initFileHeader(CurrentOffset);
+  if (!initFileHeader(CurrentOffset))
+    return false;
+
+  // Initialize the string table.
+  return initStringTable();
 }
 
 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) {
bool XCOFFWriter::writeSymbols() {
   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 (Is64Bit) {
       W.write<uint64_t>(YamlSym.Value);
-      W.write<uint32_t>(Strings.getOffset(YamlSym.SymbolName));
+      W.write<uint32_t>(StrTblBuilder.getOffset(YamlSym.SymbolName));
     } else {
       if (nameShouldBeInStringTable(YamlSym.SymbolName)) {
         // For XCOFF32: A value of 0 indicates that the symbol name is in the
         // string table.
         W.write<int32_t>(0);
-        W.write<uint32_t>(Strings.getOffset(YamlSym.SymbolName));
+        W.write<uint32_t>(StrTblBuilder.getOffset(YamlSym.SymbolName));
       } else {
         writeName(YamlSym.SymbolName, W);
       }
       W.write<uint32_t>(YamlSym.Value);
     }
     W.write<int16_t>(
         YamlSym.SectionName.size() ? SectionIndexMap[YamlSym.SectionName] : 0);
     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(XCOFF::SymbolTableEntrySize);
     }
   }
   return true;
 }
 
+void XCOFFWriter::writeStringTable() {
+  if (Obj.StrTbl.RawContent) {
+    Obj.StrTbl.RawContent->writeAsBinary(W.OS);
+    if (Obj.StrTbl.ContentSize) {
+      assert(*Obj.StrTbl.ContentSize >= Obj.StrTbl.RawContent->binary_size() &&
+             "Specified ContentSize is less than the RawContent size.");
+      W.OS.write_zeros(*Obj.StrTbl.ContentSize -
+                       Obj.StrTbl.RawContent->binary_size());
+    }
+    return;
+  }
+
+  size_t StrTblBuilderSize = StrTblBuilder.getSize();
+  // If neither Length nor ContentSize is specified, write the StrTblBuilder
+  // directly, which contains the auto-generated Length value.
+  if (!Obj.StrTbl.Length && !Obj.StrTbl.ContentSize) {
+    if (StrTblBuilderSize <= 4)
+      return;
+    StrTblBuilder.write(W.OS);
+    return;
+  }
+
+  // Serialize the string table's content to a temporary buffer.
+  std::unique_ptr<WritableMemoryBuffer> Buf =
+      WritableMemoryBuffer::getNewMemBuffer(StrTblBuilderSize);
+  uint8_t *Ptr = reinterpret_cast<uint8_t *>(Buf->getBufferStart());
+  StrTblBuilder.write(Ptr);
+  // Replace the first 4 bytes, which contain the auto-generated Length value,
+  // with the specified value.
+  memset(Ptr, 0, 4);
+  support::endian::write32be(Ptr, Obj.StrTbl.Length ? *Obj.StrTbl.Length
+                                                    : *Obj.StrTbl.ContentSize);
+  // Copy the buffer content to the actual output stream.
+  W.OS.write(Buf->getBufferStart(), Buf->getBufferSize());
+  // Add zeros as padding after strings.
+  if (Obj.StrTbl.ContentSize) {
+    assert(*Obj.StrTbl.ContentSize >= StrTblBuilderSize &&
+           "Specified ContentSize is less than the StringTableBuilder size.");
+    W.OS.write_zeros(*Obj.StrTbl.ContentSize - StrTblBuilderSize);
+  }
+}
+
 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() && !writeSymbols())
     return false;
-  // Write the string table.
-  if (Strings.getSize() > 4)
-    Strings.write(W.OS);
+  writeStringTable();
   return true;
 }
 
 } // end anonymous namespace
 
 namespace llvm {
 namespace yaml {
 
 bool yaml2xcoff(XCOFFYAML::Object &Doc, raw_ostream &Out, ErrorHandler EH) {
   XCOFFWriter Writer(Doc, Out, EH);
   return Writer.writeXCOFF();
 }
 
 } // namespace yaml
 } // namespace llvm

llvm/lib/ObjectYAML/XCOFFYAML.cpp

void MappingTraits<XCOFFYAML::Section>::mapping(IO &IO,
   IO.mapOptional("NumberOfRelocations", Sec.NumberOfRelocations);
   IO.mapOptional("NumberOfLineNumbers", Sec.NumberOfLineNumbers);
   IO.mapOptional("Flags", NC->Flags);
   IO.mapOptional("SectionData", Sec.SectionData);
   IO.mapOptional("Relocations", Sec.Relocations);
 }
 
 void MappingTraits<XCOFFYAML::Symbol>::mapping(IO &IO, XCOFFYAML::Symbol &S) {
-  IO.mapRequired("Name", S.SymbolName);
+  IO.mapOptional("Name", S.SymbolName);
   IO.mapOptional("Value", S.Value);
   IO.mapOptional("Section", S.SectionName);
   IO.mapOptional("Type", S.Type);
   IO.mapOptional("StorageClass", S.StorageClass);
   IO.mapOptional("NumberOfAuxEntries", S.NumberOfAuxEntries);
 }
 
+void MappingTraits<XCOFFYAML::StringTable>::mapping(IO &IO, XCOFFYAML::StringTable &Str) {
+  IO.mapOptional("ContentSize", Str.ContentSize);
+  IO.mapOptional("Length", Str.Length);
+  IO.mapOptional("Strings", Str.Strings);
+  IO.mapOptional("RawContent", Str.RawContent);
+}
+
 void MappingTraits<XCOFFYAML::Object>::mapping(IO &IO, XCOFFYAML::Object &Obj) {
   IO.mapTag("!XCOFF", true);
   IO.mapRequired("FileHeader", Obj.Header);
   IO.mapOptional("Sections", Obj.Sections);
   IO.mapOptional("Symbols", Obj.Symbols);
+  IO.mapOptional("StringTable", Obj.StrTbl);
 }
 
 } // namespace yaml
 } // namespace llvm

llvm/test/tools/obj2yaml/XCOFF/aix.yaml

 # CHECK-NEXT:     StorageClass:    C_HIDEXT
 # CHECK-NEXT:     NumberOfAuxEntries: 1
 # CHECK-NEXT:   - Name:      TestforXcoff
 # CHECK-NEXT:     Value:     0x78
 # CHECK-NEXT:     Section: .data
 # CHECK-NEXT:     Type:      0x0
 # CHECK-NEXT:     StorageClass:    C_HIDEXT
 # CHECK-NEXT:     NumberOfAuxEntries: 1
-# CHECK-NEXT: ...
+## TODO: Dump the string table.

llvm/test/tools/yaml2obj/XCOFF/string-table.yaml

+## Check that yaml2obj is able to customize the string table.
+## `ContentSize`, `Length`, `Strings` and/or `RawContent` can be specified in
+##  YAML. Here we test the behaviour in various cases.
+
+## Case 1: yaml2obj writes the default content (i.e. long symbol names in
+##         XCOFF32 or any symbol names in XCOFF64) when no StringTable field
+##         is specified.
+# RUN: yaml2obj --docnum=1 %s -DSYMNAME='nameInStrTbl' -o %t1
+# RUN: llvm-readobj %t1 --string-table | FileCheck %s --check-prefix=CASE1
+
+# CASE1:      StringTable {
+# CASE1-NEXT:   Length: 17
+# CASE1-NEXT:   [     4]   nameInStrTbl
+# CASE1-NEXT: }
+
+--- !XCOFF
+FileHeader:
+  MagicNumber: 0x1DF
+Symbols:
+  - Name: [[SYMNAME=<none>]]
+  - Name: [[SYMNAME2=<none>]]
+StringTable:
+  ContentSize: [[CONTENTSIZE=<none>]]
+  Length: [[LENGTHVALUE=<none>]]
+  RawContent: [[RAWCONTENT=<none>]]
+
+## We can specify `ContentSize` only when the value is equal to or greater
+## than the content size. For greater cases, zeros are added as padding.
+## Cases 2-6 are trying to check this.
+
+## Case 2: produce a string table with a specified `ContentSize`. In this case,
+##         there is no default content and the content is filled with zeroes.
+# RUN: yaml2obj --docnum=1 %s -DCONTENTSIZE=20 -o %t2
+# RUN: llvm-readobj %t2 -s --string-table | FileCheck %s --check-prefix=CASE2
+
+# CASE2:      StringTable {
+# CASE2-NEXT:   Length: 20
+# CASE2-NEXT: }
+
+## Case 3: if the value of `ContentSize` is greater than the content size,
+##         yaml2obj adds zeros as padding after the default content.
+# RUN: yaml2obj --docnum=1 %s -DSYMNAME='nameInStrTbl' -DCONTENTSIZE=20 -o %t3
+# RUN: llvm-readobj %t3 --string-table | FileCheck %s --check-prefix=CASE3
+
+# CASE3:      StringTable {
+# CASE3-NEXT:   Length: 20
+# CASE3-NEXT:   [     4]   nameInStrTbl
+# CASE3-NEXT: }
+
+## Case 4: the value of `ContentSize` matches the actual content size.
+# RUN: yaml2obj --docnum=1 %s -DSYMNAME='nameInStrTbl' -DCONTENTSIZE=17 -o %t4
+# RUN: llvm-readobj %t4 --string-table | FileCheck %s --check-prefix=CASE4
+
+# CASE4:      StringTable {
+# CASE4-NEXT:   Length: 17
+# CASE4-NEXT:   [     4]   nameInStrTbl
+# CASE4-NEXT: }
+
+## Case 5: an error is reported when the value of "ContentSize" is less than
+##         the content size.
+# RUN: not yaml2obj --docnum=1 %s -DSYMNAME='nameInStrTbl' -DCONTENTSIZE=16 \
+# RUN:   -o %t5 2>&1 | FileCheck %s --check-prefix=CASE5
+
+# CASE5: error: specified ContentSize (16) is less than the size of the data that would otherwise be written (17)
+
+## Case 6: an error is reported when `ContentSize` is less than 4 without
+##         `RawContent`.
+# RUN: not yaml2obj --docnum=1 %s -DCONTENTSIZE=3 -o %t6 2>&1 \
+# RUN:   | FileCheck %s --check-prefix=CASE6
+
+# CASE6: error: ContentSize shouldn't be less than 4 without RawContent
+
+## We can specify `Strings` for a string table. Default contents (ie. symbol
+## names in string table) will be overwritten by specified values. Cases 7-9
+## are trying to check this function.
+
+## Case 7: produce a string table with specified `Strings` directly. In this
+##         case, there is no default content.
+# RUN: yaml2obj --docnum=2 %s -o %t7
+# RUN: llvm-readobj %t7 --string-table | FileCheck %s --check-prefix=CASE7
+
+# CASE7:      StringTable {
+# CASE7-NEXT:   Length: 8
+# CASE7-NEXT:   [     4]   b
+# CASE7-NEXT:   [     6]   a
+# CASE7-NEXT: }
+
+--- !XCOFF
+FileHeader:
+  MagicNumber: 0x1DF
+Symbols:
+  - Name: [[SYMNAME=<none>]]
+  - Name: [[SYMNAME2=<none>]]
+  - Name: [[SYMNAME3=<none>]]
+StringTable:
+  ContentSize: [[CONTENTSIZE=<none>]]
+  Length: [[LENGTHVALUE=<none>]]
+  RawContent: [[RAWCONTENT=<none>]]
+  Strings:
+    - a
+    - b
+
+## Case 8: if the number of `Strings` is greater than or equal to the number
+##         of default strings, all default strings will be overwritten by
+##         specified ones.
+# RUN: yaml2obj --docnum=2 %s -DSYMNAME='nameInStrTbl' -o %t8
+# RUN: llvm-readobj %t8 -s --string-table | FileCheck %s --check-prefix=CASE8
+
+# CASE8:      Symbols [
+# CASE8-NEXT:   Symbol {
+# CASE8-NEXT:     Index: 0
+# CASE8-NEXT:     Name: a
+# CASE8-NEXT:     Value: 0x0
+# CASE8-NEXT:     Section: N_UNDEF
+# CASE8-NEXT:     Type: 0x0
+# CASE8-NEXT:     StorageClass: C_NULL (0x0)
+# CASE8-NEXT:     NumberOfAuxEntries: 0
+# CASE8-NEXT:   }
+# CASE8-NEXT:   Symbol {
+# CASE8-NEXT:     Index: 1
+# CASE8-NEXT:     Name: <none>
+# CASE8-NEXT:     Value: 0x0
+# CASE8-NEXT:     Section: N_UNDEF
+# CASE8-NEXT:     Type: 0x0
+# CASE8-NEXT:     StorageClass: C_NULL (0x0)
+# CASE8-NEXT:     NumberOfAuxEntries: 0
+# CASE8-NEXT:   }
+# CASE8-NEXT:   Symbol {
+# CASE8-NEXT:     Index: 2
+# CASE8-NEXT:     Name: <none>
+# CASE8-NEXT:     Value: 0x0
+# CASE8-NEXT:     Section: N_UNDEF
+# CASE8-NEXT:     Type: 0x0
+# CASE8-NEXT:     StorageClass: C_NULL (0x0)
+# CASE8-NEXT:     NumberOfAuxEntries: 0
+# CASE8-NEXT:   }
+# CASE8-NEXT: ]
+# CASE8-NEXT: StringTable {
+# CASE8-NEXT:   Length: 8
+# CASE8-NEXT:   [     4]   b
+# CASE8-NEXT:   [     6]   a
+# CASE8-NEXT: }
+
+## Case 9: if the number of `Strings` is less than the number of default
+##         strings, default strings will be partially overwritten. The
+##         remaining strings will still be stored after the specified strings
+##         in the string table.
+# RUN: yaml2obj --docnum=2 %s -DSYMNAME='nameInStrTbl' \
+# RUN:   -DSYMNAME2='name2InStrTbl' -DSYMNAME3='name3InStrTbl' -o %t9
+# RUN: llvm-readobj %t9 -s --string-table | FileCheck %s --check-prefix=CASE9
+
+# CASE9:      Symbols [
+# CASE9-NEXT:   Symbol {
+# CASE9-NEXT:     Index: 0
+# CASE9-NEXT:     Name: a
+# CASE9-NEXT:     Value: 0x0
+# CASE9-NEXT:     Section: N_UNDEF
+# CASE9-NEXT:     Type: 0x0
+# CASE9-NEXT:     StorageClass: C_NULL (0x0)
+# CASE9-NEXT:     NumberOfAuxEntries: 0
+# CASE9-NEXT:   }
+# CASE9-NEXT:   Symbol {
+# CASE9-NEXT:     Index: 1
+# CASE9-NEXT:     Name: b
+# CASE9-NEXT:     Value: 0x0
+# CASE9-NEXT:     Section: N_UNDEF
+# CASE9-NEXT:     Type: 0x0
+# CASE9-NEXT:     StorageClass: C_NULL (0x0)
+# CASE9-NEXT:     NumberOfAuxEntries: 0
+# CASE9-NEXT:   }
+# CASE9-NEXT:   Symbol {
+# CASE9-NEXT:     Index: 2
+# CASE9-NEXT:     Name: name3InStrTbl
+# CASE9-NEXT:     Value: 0x0
+# CASE9-NEXT:     Section: N_UNDEF
+# CASE9-NEXT:     Type: 0x0
+# CASE9-NEXT:     StorageClass: C_NULL (0x0)
+# CASE9-NEXT:     NumberOfAuxEntries: 0
+# CASE9-NEXT:   }
+# CASE9-NEXT: ]
+# CASE9-NEXT: StringTable {
+# CASE9-NEXT:   Length: 22
+# CASE9-NEXT:   [     4]   name3InStrTbl
+# CASE9-NEXT:   [    12]   b
+# CASE9-NEXT:   [    14]   a
+# CASE9-NEXT: }
+
+## We can specify both `ContentSize` and `Strings` when `ContentSize` is equal
+## to or greater than the content size. Cases 10-12 are trying to check this.
+
+## Case 10: produce a string table with specified `ContentSize` and `Strings`
+##          when the value is greater than the size of specified strings.
+##          In this case, there is no default content.
+# RUN: yaml2obj --docnum=2 %s -DCONTENTSIZE=20 -o %t10
+# RUN: llvm-readobj %t10 --string-table | FileCheck %s --check-prefix=CASE10
+
+# CASE10:      StringTable {
+# CASE10-NEXT:   Length: 20
+# CASE10-NEXT:   [     4]   b
+# CASE10-NEXT:   [     6]   a
+# CASE10-NEXT: }
+
+## Case 11: for a string table with default contents, we can specify
+##          `ContentSize` and `Strings` when the `ContentSize` is greater
+##          than the data that would otherwise be written.
+# RUN: yaml2obj --docnum=2 %s -DCONTENTSIZE=30 -DSYMNAME='nameInStrTbl' \
+# RUN:   -DSYMNAME2='name2InStrTbl' -DSYMNAME3='name3InStrTbl' -o %t11
+# RUN: llvm-readobj %t11 --string-table | FileCheck %s --check-prefix=CASE11
+
+# CASE11:      StringTable {
+# CASE11-NEXT:   Length: 30
+# CASE11-NEXT:   [     4]   name3InStrTbl
+# CASE11-NEXT:   [    12]   b
+# CASE11-NEXT:   [    14]   a
+# CASE11-NEXT: }
+
+## Case 12: an error is reported when the value of `ContentSize` is less
+##          than the final content size. None of `ContentSize`, `Strings` or
+##          default contents is empty in this case.
+# RUN: not yaml2obj --docnum=2 %s  -DCONTENTSIZE=10 -DSYMNAME='nameInStrTbl' \
+# RUN:   -DSYMNAME2='name2InStrTbl' -DSYMNAME3='name3InStrTbl' -o %t12 2>&1 \
+# RUN:   | FileCheck %s --check-prefix=CASE12
+
+# CASE12: error: specified ContentSize (10) is less than the size of the data that would otherwise be written (22)
+
+## We can use `RawContent` to generate a string table. Cases 13-16 are trying to
+## check the `RawContent`.
+
+## Case 13: if `RawContent` is specified and no `ContentSize` is specified.
+##          Write the `RawContent` data.
+# RUN: yaml2obj --docnum=1 %s -DRAWCONTENT="000000090062006300" -o %t13
+# RUN: llvm-readobj %t13 --string-table | FileCheck %s --check-prefix=CASE13
+
+# CASE13:      StringTable {
+# CASE13-NEXT:   Length: 9
+# CASE13-NEXT:   [     5]   b
+# CASE13-NEXT:   [     7]   c
+# CASE13-NEXT: }
+
+## Case 14: if `RawContent` is specified and `ContentSize` matches the size
+##          of the `RawContent` data. Write the `RawContent` data.
+# RUN: yaml2obj --docnum=1 %s -DRAWCONTENT="000000090062006300" -DCONTENTSIZE=9 -o %t14
+# RUN: llvm-readobj %t14 --string-table | FileCheck %s --check-prefix=CASE14
+
+# CASE14:      StringTable {
+# CASE14-NEXT:   Length: 9
+# CASE14-NEXT:   [     5]   b
+# CASE14-NEXT:   [     7]   c
+# CASE14-NEXT: }
+
+## Case 15: an error is reported when `ContentSize` is less than the `RawContent`
+##          data size.
+# RUN: not yaml2obj --docnum=1 %s -DRAWCONTENT="000000090062006300" -DCONTENTSIZE=6 \
+# RUN:   -o %t15 2>&1 | FileCheck %s --check-prefix=CASE15
+
+# CASE15: error: specified ContentSize (6) is less than the RawContent data size (9)
+
+## Case 16: if `RawContent` is specified and `ContentSize` is greater than the
+##          `RawContent` data size, pad the RawContent with trailing zeroes.
+# RUN: yaml2obj --docnum=1 %s -DRAWCONTENT="000000090062006300" -DCONTENTSIZE=11 -o %t16
+# RUN: llvm-readobj %t16 --string-table | FileCheck %s --check-prefix=CASE16
+
+# CASE16:      StringTable {
+# CASE16-NEXT:   Length: 9
+# CASE16-NEXT:   [     5]   b
+# CASE16-NEXT:   [     7]   c
+# CASE16-NEXT: }
+
+## We can specify `Length`. Use the value of the `Length` field for the first
+## 4 bytes of the table. The value may not make sense for the data that is
+## being written. Cases 17-20 are trying to check this.
+
+## Case 17: report an error if the `Length` is specified as well as `RawContent`.
+# RUN: not yaml2obj --docnum=1 %s -DRAWCONTENT="0062006300" -DLENGTHVALUE=9 \
+# RUN:    -o %t17 2>&1 | FileCheck %s --check-prefix=CASE17
+
+# CASE17: error: can't specify Strings or Length when RawContent is specified
+
+## Case 18: report an error if both `RawContent` and `Strings` are specified.
+# RUN: not yaml2obj --docnum=2 %s -DRAWCONTENT="0062006300" -o %t18 2>&1 \
+# RUN:   | FileCheck %s --check-prefix=CASE18
+
+# CASE18: error: can't specify Strings or Length when RawContent is specified
+
+## Case 19: use the value of the `Length` field for the first 4 bytes of the
+##          table. We dump the string table from the offset of 0x38.
+# RUN: yaml2obj --docnum=1 %s -DSYMNAME='nameInStrTbl' -DLENGTHVALUE=20 -o %t19
+# RUN: od -A n -t x1 -v -j 0x38 %t19 | FileCheck %s --check-prefix=CASE19
+
+# CASE19:      00 00 00 14 6e 61 6d 65 49 6e 53 74 72 54 62 6c
+# CASE19-NEXT: 00