[LLD/PDB] Output actual records to the globals stream

[zturner]

Summary

Previously we were just writing an empty globals stream. I hypothesized that fixing this would fix the last remaining known issue with using WinDbg, which is that we could not display local variables. Unfortunately this patch only gets us halfway there. Previously when displaying locals you would get an error message saying that private symbol information could not be found, and running the "lm" command in WinDbg would say that only public symbols were available. After this patch, the lm command reports that private symbols are present, but attempting to display a local variable still reports the same error. So there is something else missing.

[rnk]

llvm/lib/DebugInfo/CodeView/RecordName.cpp

280

This isn't really "computing" the name, it just gets it. getSymbolName seems more appropriate. It should also return a StringRef. This also needs to be really fast, because we do it on every external symbol. Setting up a deserialization pipeline is probably too much work.

Do any of these require any more work than knowing the offset of the name member of the symbol record? I'm thinking we might want to switch to compute the offset, and do the StringRef.split('\0') trick to find the null terminator.

llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp

95

We do this a lot, this is going to happen once per record. It would be nice to avoid copying the bytes before we hash them.

[zturner]

Made getting symbol name faster.

[rnk]

llvm/lib/DebugInfo/CodeView/RecordName.cpp

259–260

LLVM frequently factors large switches like this into separate functions. In this case, you can return the offset or some sentinel value like 0 or -1. This saves duplication of Sym.content().drop_front(Offset) and break becomes return 35.

327–329

Ouch, I was hoping all names were at constant offsets. We can put this in a separate if to implement the previously suggested refactoring.

344–345

IMO it would be nicer to avoid the use of the lambda. We have alternatives like strnlen and .split('\0').

[zturner]

Update

[rnk]

lgtm

[zturner]

I had to edit a bunch of tests since we moved some symbols from module stream to globals stream. Re-uploading one more time to make sure that the changes to the test are sound.

[rnk]

lld/COFF/PDB.cpp

476

We don't seem to have test coverage for this. Do you mind adding one? You might be able to modify one of the yaml test cases...

lld/test/COFF/pdb-comdat.test

57 ↗

(On Diff #110504)

Oh right, these do only go in the globals section. Woops.

[zturner]

Added a test for all global symbol types.

lld/COFF/PDB.cpp

 #include "Config.h"
 #include "Error.h"
 #include "SymbolTable.h"
 #include "Symbols.h"
 #include "Writer.h"
 #include "llvm/DebugInfo/CodeView/CVDebugRecord.h"
 #include "llvm/DebugInfo/CodeView/DebugSubsectionRecord.h"
 #include "llvm/DebugInfo/CodeView/LazyRandomTypeCollection.h"
+#include "llvm/DebugInfo/CodeView/RecordName.h"
 #include "llvm/DebugInfo/CodeView/SymbolDeserializer.h"
 #include "llvm/DebugInfo/CodeView/SymbolSerializer.h"
 #include "llvm/DebugInfo/CodeView/TypeDeserializer.h"
 #include "llvm/DebugInfo/CodeView/TypeDumpVisitor.h"
 #include "llvm/DebugInfo/CodeView/TypeIndexDiscovery.h"
 #include "llvm/DebugInfo/CodeView/TypeStreamMerger.h"
 #include "llvm/DebugInfo/CodeView/TypeTableBuilder.h"
 #include "llvm/DebugInfo/MSF/MSFBuilder.h"
static void scopeStackClose(SmallVectorImpl<SymbolScope> &Stack,
   if (Stack.empty()) {
     warn("symbol scopes are not balanced in " + File->getName());
     return;
   }
   SymbolScope S = Stack.pop_back_val();
   S.OpeningRecord->PtrEnd = CurOffset;
 }
 
+static bool symbolGoesInModuleStream(const CVSymbol &Sym) {
+  switch (Sym.kind()) {
+  case SymbolKind::S_GDATA32:
+  case SymbolKind::S_CONSTANT:
+  case SymbolKind::S_UDT:
+  // We really should not be seeing S_PROCREF and S_LPROCREF in the first place
+  // since they are synthesized by the linker in response to S_GPROC32 and
+  // S_LPROC32, but if we do see them, don't put them in the module stream I
+  // guess.
+  case SymbolKind::S_PROCREF:
+  case SymbolKind::S_LPROCREF:
+    return false;
+  // S_GDATA32 does not go in the module stream, but S_LDATA32 does.
+  case SymbolKind::S_LDATA32:
+  default:
+    return true;
+  }
+}
+
+static bool symbolGoesInGlobalsStream(const CVSymbol &Sym) {
+  switch (Sym.kind()) {
+  case SymbolKind::S_CONSTANT:
+  case SymbolKind::S_UDT:
+  case SymbolKind::S_GDATA32:
+  // S_LDATA32 goes in both the module stream and the globals stream.
+  case SymbolKind::S_LDATA32:
+  case SymbolKind::S_GPROC32:
+  case SymbolKind::S_LPROC32:
+  // We really should not be seeing S_PROCREF and S_LPROCREF in the first place
+  // since they are synthesized by the linker in response to S_GPROC32 and
+  // S_LPROC32, but if we do see them, copy them straight through.
+  case SymbolKind::S_PROCREF:
+  case SymbolKind::S_LPROCREF:
+    return true;
+  default:
+    return false;
+  }
+}
+
+static void addGlobalSymbol(pdb::GSIStreamBuilder &Builder, ObjFile &File,
+                            const CVSymbol &Sym) {
+  switch (Sym.kind()) {
+  case SymbolKind::S_CONSTANT:
+  case SymbolKind::S_UDT:
+  case SymbolKind::S_GDATA32:
+  case SymbolKind::S_LDATA32:
+  case SymbolKind::S_PROCREF:
+  case SymbolKind::S_LPROCREF:
+    Builder.addGlobalSymbol(Sym);
+    break;
+  case SymbolKind::S_GPROC32:
+  case SymbolKind::S_LPROC32: {
+    std::string Name = computeSymbolName(Sym);
+    SymbolRecordKind K = SymbolRecordKind::ProcRefSym;
+    if (Sym.kind() == SymbolKind::S_LPROC32)
+      K = SymbolRecordKind::LocalProcRef;
+    ProcRefSym PS(K);
+    PS.Module = static_cast<uint16_t>(File.ModuleDBI->getModuleIndex());
+    // For some reason, MSVC seems to add one to this value.
+    ++PS.Module;
+    PS.Name = Name;
+    PS.SumName = 0;
+    PS.SymOffset = File.ModuleDBI->getNextSymbolOffset();
+    Builder.addGlobalSymbol(PS);
+    break;
+  }
+  default:
+    llvm_unreachable("Invalid symbol kind!");
+  }
+}
+
 static void mergeSymbolRecords(BumpPtrAllocator &Alloc, ObjFile *File,
+                               pdb::GSIStreamBuilder &GsiBuilder,
                                const CVIndexMap &IndexMap,
                                const TypeTableBuilder &IDTable,
                                BinaryStreamRef SymData) {
   // FIXME: Improve error recovery by warning and skipping records when
   // possible.
   CVSymbolArray Syms;
   BinaryStreamReader Reader(SymData);
   ExitOnErr(Reader.readArray(Syms, Reader.getLength()));
for (CVSymbol Sym : Syms) {
 
     // Fill in "Parent" and "End" fields by maintaining a stack of scopes.
     CVSymbol NewSym(NewKind, NewData);
     if (symbolOpensScope(NewKind))
       scopeStackOpen(Scopes, File->ModuleDBI->getNextSymbolOffset(), NewSym);
     else if (symbolEndsScope(NewKind))
       scopeStackClose(Scopes, File->ModuleDBI->getNextSymbolOffset(), File);
 
+    // Add the symbol to the globals stream if necessary.  Do this before adding
+    // the symbol to the module since we may need to get the next symbol offset,
+    // and writing to the module's symbol stream will update that offset.
+    if (symbolGoesInGlobalsStream(NewSym))
+      addGlobalSymbol(GsiBuilder, *File, NewSym);
+
     // Add the symbol to the module.
+    if (symbolGoesInModuleStream(NewSym))
       File->ModuleDBI->addSymbol(NewSym);
   }
 }
 
 // Allocate memory for a .debug$S section and relocate it.
 static ArrayRef<uint8_t> relocateDebugChunk(BumpPtrAllocator &Alloc,
                                             SectionChunk *DebugChunk) {
   uint8_t *Buffer = Alloc.Allocate<uint8_t>(DebugChunk->getSize());
   assert(DebugChunk->OutputSectionOff == 0 &&
for (const DebugSubsectionRecord &SS : Subsections) {
         ExitOnErr(Checksums.initialize(SS.getRecordData()));
         break;
       case DebugSubsectionKind::Lines:
         // We can add the relocated line table directly to the PDB without
         // modification because the file checksum offsets will stay the same.
         File->ModuleDBI->addDebugSubsection(SS);
         break;
       case DebugSubsectionKind::Symbols:
-        mergeSymbolRecords(Alloc, File, IndexMap, IDTable, SS.getRecordData());
+        mergeSymbolRecords(Alloc, File, Builder.getGsiBuilder(), IndexMap,
+                           IDTable, SS.getRecordData());
         break;
       default:
         // FIXME: Process the rest of the subsections.
         break;
       }
     }
 
     if (Checksums.valid()) {
void PDBLinker::addObjectsToPDB() {
   Builder.getStringTableBuilder().setStrings(PDBStrTab);
 
   // Construct TPI stream contents.
   addTypeInfo(Builder.getTpiBuilder(), TypeTable);
 
   // Construct IPI stream contents.
   addTypeInfo(Builder.getIpiBuilder(), IDTable);
 
-  // Compute the public symbols.
+  // Compute the public and global symbols.
+  auto &GsiBuilder = Builder.getGsiBuilder();
   std::vector<PublicSym32> Publics;
-  Symtab->forEachSymbol([&Publics](Symbol *S) {
+  Symtab->forEachSymbol([&Publics, &GsiBuilder](Symbol *S) {
     // Only emit defined, live symbols that have a chunk.
     auto *Def = dyn_cast<Defined>(S->body());
     if (Def && Def->isLive() && Def->getChunk())
       Publics.push_back(createPublic(Def));
   });
 
   if (!Publics.empty()) {
     // Sort the public symbols and add them to the stream.
     std::sort(Publics.begin(), Publics.end(),
               [](const PublicSym32 &L, const PublicSym32 &R) {
                 return L.Name < R.Name;
               });
-    auto &GsiBuilder = Builder.getGsiBuilder();
     for (const PublicSym32 &Pub : Publics)
       GsiBuilder.addPublicSymbol(Pub);
   }
 }
 
 static void addLinkerModuleSymbols(StringRef Path,
                                    pdb::DbiModuleDescriptorBuilder &Mod,
                                    BumpPtrAllocator &Allocator) {

llvm/include/llvm/DebugInfo/CodeView/RecordName.h

-//===- TypeName.h --------------------------------------------- *- C++ --*-===//
+//===- RecordName.h ------------------------------------------- *- C++ --*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_DEBUGINFO_CODEVIEW_TYPENAME_H
-#define LLVM_DEBUGINFO_CODEVIEW_TYPENAME_H
+#ifndef LLVM_DEBUGINFO_CODEVIEW_RECORDNAME_H
+#define LLVM_DEBUGINFO_CODEVIEW_RECORDNAME_H
 
+#include "llvm/DebugInfo/CodeView/SymbolRecord.h"
 #include "llvm/DebugInfo/CodeView/TypeCollection.h"
 #include "llvm/DebugInfo/CodeView/TypeIndex.h"
 
 namespace llvm {
 namespace codeview {
 std::string computeTypeName(TypeCollection &Types, TypeIndex Index);
-}
+std::string computeSymbolName(CVSymbol Sym);
+} // namespace codeview
 } // namespace llvm
 
 #endif

llvm/include/llvm/DebugInfo/CodeView/TypeDeserializer.h

if (auto EC = I.Mapping.visitKnownRecord(CVT, Record))
       return EC;
     if (auto EC = I.Mapping.visitTypeEnd(CVT))
       return EC;
     return Error::success();
   }
 
   template <typename T>
   static Expected<T> deserializeAs(ArrayRef<uint8_t> Data) {
-    CVType CVT;
-    CVT.RecordData = Data;
-    MappingInfo I(CVT.content());
     const RecordPrefix *Prefix =
         reinterpret_cast<const RecordPrefix *>(Data.data());
     TypeRecordKind K =
         static_cast<TypeRecordKind>(uint16_t(Prefix->RecordKind));
     T Record(K);
+    CVType CVT(static_cast<TypeLeafKind>(K), Data);
     if (auto EC = deserializeAs<T>(CVT, Record))
       return std::move(EC);
     return Record;
   }
 
   Error visitTypeBegin(CVType &Record) override {
     assert(!Mapping && "Already in a type mapping!");
     Mapping = llvm::make_unique<MappingInfo>(Record.content());

llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h

public:
   static const int HfaKindMask = 0x1800;
   static const int WinRTKindShift = 14;
   static const int WinRTKindMask = 0xC000;
 
   bool hasUniqueName() const {
     return (Options & ClassOptions::HasUniqueName) != ClassOptions::None;
   }
 
+  bool isNested() const {
+    return (Options & ClassOptions::Nested) != ClassOptions::None;
+  }
+
   uint16_t getMemberCount() const { return MemberCount; }
   ClassOptions getOptions() const { return Options; }
   TypeIndex getFieldList() const { return FieldList; }
   StringRef getName() const { return Name; }
   StringRef getUniqueName() const { return UniqueName; }
 
   uint16_t MemberCount;
   ClassOptions Options;

llvm/include/llvm/DebugInfo/PDB/Native/GSIStreamBuilder.h

public:
   Error commit(const msf::MSFLayout &Layout, WritableBinaryStreamRef Buffer);
 
   uint32_t getPublicsStreamIndex() const;
   uint32_t getGlobalsStreamIndex() const;
   uint32_t getRecordStreamIdx() const { return RecordStreamIdx; }
 
   void addPublicSymbol(const codeview::PublicSym32 &Pub);
 
+  void addGlobalSymbol(const codeview::ProcRefSym &Sym);
+  void addGlobalSymbol(const codeview::DataSym &Sym);
+  void addGlobalSymbol(const codeview::ConstantSym &Sym);
+  void addGlobalSymbol(const codeview::UDTSym &Sym);
+  void addGlobalSymbol(const codeview::CVSymbol &Sym);
+
 private:
   uint32_t calculatePublicsHashStreamSize() const;
   uint32_t calculateGlobalsHashStreamSize() const;
   Error commitSymbolRecordStream(WritableBinaryStreamRef Stream);
   Error commitPublicsHashStream(WritableBinaryStreamRef Stream);
   Error commitGlobalsHashStream(WritableBinaryStreamRef Stream);
 
   uint32_t RecordStreamIdx = kInvalidStreamIndex;
   msf::MSFBuilder &Msf;
   std::unique_ptr<GSIHashStreamBuilder> PSH;
   std::unique_ptr<GSIHashStreamBuilder> GSH;
 };
 } // namespace pdb
 } // namespace llvm
 
 #endif

llvm/lib/DebugInfo/CodeView/CMakeLists.txt

add_llvm_library(LLVMDebugInfoCodeView
   DebugSubsectionRecord.cpp
   DebugSubsectionVisitor.cpp
   DebugSymbolRVASubsection.cpp
   DebugSymbolsSubsection.cpp
   EnumTables.cpp
   Formatters.cpp
   LazyRandomTypeCollection.cpp
   Line.cpp
+  RecordName.cpp
   RecordSerialization.cpp
   StringsAndChecksums.cpp
   SymbolRecordMapping.cpp
   SymbolDumper.cpp
   SymbolSerializer.cpp
   TypeDumpVisitor.cpp
   TypeIndex.cpp
   TypeIndexDiscovery.cpp
-  TypeName.cpp
   TypeRecordMapping.cpp
   TypeSerializer.cpp
   TypeStreamMerger.cpp
   TypeTableCollection.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/DebugInfo/CodeView
   )

llvm/lib/DebugInfo/CodeView/LazyRandomTypeCollection.cpp

 //===- LazyRandomTypeCollection.cpp ---------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/DebugInfo/CodeView/LazyRandomTypeCollection.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/DebugInfo/CodeView/CodeViewError.h"
-#include "llvm/DebugInfo/CodeView/TypeName.h"
+#include "llvm/DebugInfo/CodeView/RecordName.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.h"
 #include "llvm/Support/BinaryStreamReader.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 #include <algorithm>
 #include <cassert>
 #include <cstdint>
 #include <iterator>

llvm/lib/DebugInfo/CodeView/RecordName.cpp

-//===- TypeName.cpp ------------------------------------------- *- C++ --*-===//
+//===- RecordName.cpp ----------------------------------------- *- C++ --*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
-#include "llvm/DebugInfo/CodeView/TypeName.h"
+#include "llvm/DebugInfo/CodeView/RecordName.h"
 
 #include "llvm/ADT/SmallString.h"
+#include "llvm/DebugInfo/CodeView/CVSymbolVisitor.h"
 #include "llvm/DebugInfo/CodeView/CVTypeVisitor.h"
+#include "llvm/DebugInfo/CodeView/SymbolDeserializer.h"
+#include "llvm/DebugInfo/CodeView/SymbolVisitorCallbackPipeline.h"
+#include "llvm/DebugInfo/CodeView/SymbolVisitorCallbacks.h"
 #include "llvm/DebugInfo/CodeView/TypeVisitorCallbacks.h"
 #include "llvm/Support/FormatVariadic.h"
 
 using namespace llvm;
 using namespace llvm::codeview;
 
 namespace {
 class TypeNameComputer : public TypeVisitorCallbacks {
std::string llvm::codeview::computeTypeName(TypeCollection &Types,
   TypeNameComputer Computer(Types);
   CVType Record = Types.getType(Index);
   if (auto EC = visitTypeRecord(Record, Index, Computer)) {
     consumeError(std::move(EC));
     return "<unknown UDT>";
   }
   return Computer.name();
 }
+
+namespace {
+struct SymbolNameComputer : public SymbolVisitorCallbacks {
+  std::string Name;
+
+#define GET_SYMBOL_NAME_METHOD(Type)                                           \
+  Error visitKnownRecord(CVSymbol &CVR, Type &S) override {                    \
+    Name = S.Name;                                                             \
+    return Error::success();                                                   \
+  }
+
+  GET_SYMBOL_NAME_METHOD(ProcSym);
+  GET_SYMBOL_NAME_METHOD(Thunk32Sym);
+  GET_SYMBOL_NAME_METHOD(SectionSym);
+  GET_SYMBOL_NAME_METHOD(CoffGroupSym);
+  GET_SYMBOL_NAME_METHOD(PublicSym32);
+  GET_SYMBOL_NAME_METHOD(RegisterSym);
+  GET_SYMBOL_NAME_METHOD(LocalSym);
+  GET_SYMBOL_NAME_METHOD(BlockSym);
+  GET_SYMBOL_NAME_METHOD(LabelSym);
+  GET_SYMBOL_NAME_METHOD(ObjNameSym);
+  GET_SYMBOL_NAME_METHOD(ExportSym);
+  GET_SYMBOL_NAME_METHOD(FileStaticSym);
+  GET_SYMBOL_NAME_METHOD(UDTSym);
+  GET_SYMBOL_NAME_METHOD(BPRelativeSym);
+  GET_SYMBOL_NAME_METHOD(RegRelativeSym);
+  GET_SYMBOL_NAME_METHOD(ConstantSym);
+  GET_SYMBOL_NAME_METHOD(DataSym);
+  GET_SYMBOL_NAME_METHOD(ThreadLocalDataSym);
+};
+} // namespace
+
+std::string llvm::codeview::computeSymbolName(CVSymbol Sym) {
+  SymbolVisitorCallbackPipeline Pipeline;
+  // Since we only have a single record and there is no padding, the container
+  // type does not matter.
+  SymbolDeserializer Deserializer(nullptr, CodeViewContainer::ObjectFile);
+  SymbolNameComputer Computer;
+  Pipeline.addCallbackToPipeline(Deserializer);
+  Pipeline.addCallbackToPipeline(Computer);
+  CVSymbolVisitor Visitor(Pipeline);
+  if (auto EC = Visitor.visitSymbolRecord(Sym))
+    return "<unknown symbol>";
+  return Computer.Name;
+}

llvm/lib/DebugInfo/CodeView/TypeTableCollection.cpp

 //===- TypeTableCollection.cpp -------------------------------- *- C++ --*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/DebugInfo/CodeView/TypeTableCollection.h"
 
 #include "llvm/DebugInfo/CodeView/CVTypeVisitor.h"
-#include "llvm/DebugInfo/CodeView/TypeName.h"
+#include "llvm/DebugInfo/CodeView/RecordName.h"
 #include "llvm/DebugInfo/CodeView/TypeTableBuilder.h"
 #include "llvm/Support/BinaryByteStream.h"
 #include "llvm/Support/BinaryStreamReader.h"
 
 using namespace llvm;
 using namespace llvm::codeview;
 
 TypeTableCollection::TypeTableCollection(ArrayRef<ArrayRef<uint8_t>> Records)

llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp

 //===- DbiStreamBuilder.cpp - PDB Dbi Stream Creation -----------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/DebugInfo/PDB/Native/GSIStreamBuilder.h"
 
+#include "llvm/DebugInfo/CodeView/RecordName.h"
 #include "llvm/DebugInfo/CodeView/SymbolDeserializer.h"
 #include "llvm/DebugInfo/CodeView/SymbolRecord.h"
 #include "llvm/DebugInfo/CodeView/SymbolSerializer.h"
 #include "llvm/DebugInfo/MSF/MSFBuilder.h"
 #include "llvm/DebugInfo/MSF/MSFCommon.h"
 #include "llvm/DebugInfo/MSF/MappedBlockStream.h"
 #include "llvm/DebugInfo/PDB/Native/GlobalsStream.h"
 #include "llvm/DebugInfo/PDB/Native/Hash.h"
 #include "llvm/Support/BinaryItemStream.h"
 #include "llvm/Support/BinaryStreamWriter.h"
 #include <algorithm>
 #include <vector>
 
 using namespace llvm;
 using namespace llvm::msf;
 using namespace llvm::pdb;
 using namespace llvm::codeview;
 
-static StringRef getSymbolName(const CVSymbol &Sym) {
-  assert(Sym.kind() == S_PUB32 && "handle other kinds");
-  PublicSym32 PSL =
-      cantFail(SymbolDeserializer::deserializeAs<PublicSym32>(Sym));
-  return PSL.Name;
-}
-
 struct llvm::pdb::GSIHashStreamBuilder {
   std::vector<CVSymbol> Records;
   uint32_t StreamIndex;
   std::vector<PSHashRecord> HashRecords;
   std::array<support::ulittle32_t, (IPHR_HASH + 32) / 32> HashBitmap;
   std::vector<support::ulittle32_t> HashBuckets;
 
   uint32_t calculateSerializedLength() const;
   uint32_t calculateRecordByteSize() const;
   Error commit(BinaryStreamWriter &Writer);
   void finalizeBuckets(uint32_t RecordZeroOffset);
+
+  template <typename T> void addSymbol(const T &Symbol, MSFBuilder &Msf) {
+    T Copy(Symbol);
+    Records.push_back(SymbolSerializer::writeOneSymbol(Copy, Msf.getAllocator(),
+                                                       CodeViewContainer::Pdb));
+  }
+  void addSymbol(const CVSymbol &Symbol) { Records.push_back(Symbol); }
 };
 
 uint32_t GSIHashStreamBuilder::calculateSerializedLength() const {
   uint32_t Size = sizeof(GSIHashHeader);
   Size += HashRecords.size() * sizeof(PSHashRecord);
   Size += HashBitmap.size() * sizeof(uint32_t);
   Size += HashBuckets.size() * sizeof(uint32_t);
   return Size;
void GSIHashStreamBuilder::finalizeBuckets(uint32_t RecordZeroOffset) {
   uint32_t SymOffset = RecordZeroOffset;
   for (const CVSymbol &Sym : Records) {
     PSHashRecord HR;
     // Add one when writing symbol offsets to disk. See GSI1::fixSymRecs.
     HR.Off = SymOffset + 1;
     HR.CRef = 1; // Always use a refcount of 1.
 
     // Hash the name to figure out which bucket this goes into.
-    StringRef Name = getSymbolName(Sym);
+    std::string Name = computeSymbolName(Sym);
     size_t BucketIdx = hashStringV1(Name) % IPHR_HASH;
     TmpBuckets[BucketIdx].push_back(HR); // FIXME: Does order matter?
 
     SymOffset += Sym.length();
   }
 
   // Compute the three tables: the hash records in bucket and chain order, the
   // bucket presence bitmap, and the bucket chain start offsets.
uint32_t GSIStreamBuilder::getPublicsStreamIndex() const {
   return PSH->StreamIndex;
 }
 
 uint32_t GSIStreamBuilder::getGlobalsStreamIndex() const {
   return GSH->StreamIndex;
 }
 
 void GSIStreamBuilder::addPublicSymbol(const PublicSym32 &Pub) {
-  PublicSym32 Copy(Pub);
-  PSH->Records.push_back(SymbolSerializer::writeOneSymbol(
-      Copy, Msf.getAllocator(), CodeViewContainer::Pdb));
+  PSH->addSymbol(Pub, Msf);
+}
+
+void GSIStreamBuilder::addGlobalSymbol(const ProcRefSym &Sym) {
+  GSH->addSymbol(Sym, Msf);
+}
+
+void GSIStreamBuilder::addGlobalSymbol(const DataSym &Sym) {
+  GSH->addSymbol(Sym, Msf);
+}
+
+void GSIStreamBuilder::addGlobalSymbol(const ConstantSym &Sym) {
+  GSH->addSymbol(Sym, Msf);
+}
+
+void GSIStreamBuilder::addGlobalSymbol(const UDTSym &Sym) {
+  GSH->addSymbol(Sym, Msf);
+}
+
+void GSIStreamBuilder::addGlobalSymbol(const codeview::CVSymbol &Sym) {
+  GSH->addSymbol(Sym);
 }
 
 static Error writeRecords(BinaryStreamWriter &Writer,
                           ArrayRef<CVSymbol> Records) {
   BinaryItemStream<CVSymbol> ItemStream(support::endianness::little);
   ItemStream.setItems(Records);
   BinaryStreamRef RecordsRef(ItemStream);
   return Writer.writeStreamRef(RecordsRef);