[llvm-pdbdump] Add the ability to merge PDBs

[zturner]

Summary

By adding a new merge subcommand, we can now give arbitrarily many PDBs to llvm-pdbdump as inputs and get back one PDB. Currently it only merges type streams and ignores everything else. In the future it can merge symbol streams, module information, and other types of stuff.

Having this command allows us to write a test for type merging, which is also done in this patch.

[zturner]

Added a slightly more complicated test case involving two identical procedure types.

[inglorion]

I feel like llvm-pdbdump already does a ton of things. Having a tool to merge pdbs seems useful, but I'm not sure llvm-pdbdump should be that tool. If we want all that functionality in a single tool I think we should at least rename it to something like llvm-pdbtool (because we're not really dumping pdbs now), but I would actually prefer having this be a separate tool.

[zturner]

What would be the advantage of having it be a separate tool? I kind of like the fact that I don't have to remember which tool to use when I want something, which kind of solves the problem of tools with overlapping responsibilities. For example, I can never remember whether to use llvm-objdump or llvm-readobj. Seems to me they should be the same tool. Like "if you want to do something to a PDB, use llvm-pdbdump". That said, I do agree it does more than just dumping now, so perhaps a new name is in order soon. I wouldn't want to mix a change like that in with other changes though.

[inglorion]

What would be the advantage of having it be a separate tool?

Partially the usual "less code, so easier to understand, faster to compile, fewer opportunities to create strong coupling".

I also seem to have the opposite problem when figuring out how to do something. Instead of struggling to remember which of the tools I know exist I need to use, I struggle to discover that a tool exists at all (I can create pdbs from YAML?) or struggle to remember what the subcommand is called (ls or autocomplete will tell me what the name of the binary is, but I then need to run it to figure out if a subcommand exists and how it's spelled).

I kind of like the fact that I don't have to remember which tool to use when I want something, which kind of solves the problem of tools with overlapping responsibilities. For example, I can never remember whether to use llvm-objdump or llvm-readobj.

In this particular example, I suspect that one is inspired by objdump and one is inspired by readobj. Those do indeed largely do the same things, so it's unfortunate that there are two. On the other hand, things like inspecting pdbs and merging pdbs seem pretty distinct to me.

Seems to me they should be the same tool. Like "if you want to do something to a PDB, use llvm-pdbdump". That said, I do agree it does more than just dumping now, so perhaps a new name is in order soon. I wouldn't want to mix a change like that in with other changes though.

Agreed that if we're going to rename the tool, that should go in a separate change.

[inglorion]

Then again, I don't feel strongly enough about my preference for separate tools to really hold this up. I'm ok with just renaming this to llvm-pdbtool or some such. The code looks fine to me apart from the thing I mentioned inline.

llvm/tools/llvm-pdbdump/llvm-pdbdump.cpp

1019

The other subcommands take function arguments, instead of accessing the globals. Can we do that here as well?

[zturner]

llvm/tools/llvm-pdbdump/llvm-pdbdump.cpp

1019

Actually it's a little bit of both. There are so many options that it's impractical to pass the entire set of options to every subcommand's implementation, and they frequently access the globals. In this instance I did it this way because passing a reference to a cl::list<std::string> felt a little awkward. In the other cases we don't have this problem because there are a fixed number of input files, but in this case there can be arbitrarily many.

[inglorion]

llvm/tools/llvm-pdbdump/llvm-pdbdump.cpp

1019

You're right, the others use a combination of arguments and globals. I'd still prefer it if you made this one an argument. I agree that passing a reference to a cl::list feels a little awkward. How about an ArrayRef? IIRC, you can get those out of cl::lists using the operator ArrayRef.

[zturner]

llvm/tools/llvm-pdbdump/llvm-pdbdump.cpp

1019

Didn't realize there was an operator ArrayRef. If that's the case, I agree that seems like a good solution.

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

private:
   uint32_t calculateIndexOffsetSize() const;
   Error finalize();
 
   msf::MSFBuilder &Msf;
   BumpPtrAllocator &Allocator;
 
   size_t TypeRecordBytes = 0;
 
-  Optional<PdbRaw_TpiVer> VerHeader;
+  PdbRaw_TpiVer VerHeader = PdbRaw_TpiVer::PdbTpiV80;
   std::vector<ArrayRef<uint8_t>> TypeRecords;
   std::vector<uint32_t> TypeHashes;
   std::vector<codeview::TypeIndexOffset> TypeIndexOffsets;
   uint32_t HashStreamIndex = kInvalidStreamIndex;
   std::unique_ptr<BinaryByteStream> HashValueStream;
 
   const TpiStreamHeader *Header;
   uint32_t Idx;
 };
 }
 }
 
 #endif

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

 Error TpiStreamBuilder::finalize() {
   if (Header)
     return Error::success();
 
   TpiStreamHeader *H = Allocator.Allocate<TpiStreamHeader>();
 
   uint32_t Count = TypeRecords.size();
 
-  H->Version = *VerHeader;
+  H->Version = VerHeader;
   H->HeaderSize = sizeof(TpiStreamHeader);
   H->TypeIndexBegin = codeview::TypeIndex::FirstNonSimpleIndex;
   H->TypeIndexEnd = H->TypeIndexBegin + Count;
   H->TypeRecordBytes = TypeRecordBytes;
 
   H->HashStreamIndex = HashStreamIndex;
   H->HashAuxStreamIndex = kInvalidStreamIndex;
   H->HashKeySize = sizeof(ulittle32_t);

llvm/test/DebugInfo/PDB/Inputs/merge1.yaml

+---
+TpiStream:
+  Records:
+    # uint32_t* [Index: 0x1000]
+    - Kind:            LF_POINTER
+      Pointer:         
+        ReferentType:    117
+        Attrs:           32778
+    # int64_t* [Index: 0x1001]
+    - Kind:            LF_POINTER
+      Pointer:         
+        ReferentType:    118
+        Attrs:           32778
+    # struct OnlyInMerge1 [Index: 0x1002]
+    - Kind:            LF_STRUCTURE
+      Class:           
+        MemberCount:     0
+        Options:         [ None, ForwardReference, HasUniqueName ]
+        FieldList:       0
+        Name:            'OnlyInMerge1'
+        UniqueName:      'OnlyInMerge1'
+        DerivationList:  0
+        VTableShape:     0
+        Size:            0
+    # uint32_t** [Index: 0x1003]
+    - Kind:            LF_POINTER
+      Pointer:         
+        ReferentType:    4096
+        Attrs:           32778
+    # uint32_t*** [Index: 0x1004]
+    - Kind:            LF_POINTER
+      Pointer:         
+        ReferentType:    4099
+        Attrs:           32778
+    # int64_t* [Index: 0x1005]
+    - Kind:            LF_POINTER
+      Pointer:         
+        ReferentType:    4097
+        Attrs:           32778
+    # [uint32_t, uint32_t*, uint32_t**] [Index: 0x1006]
+    - Kind:            LF_ARGLIST
+      ArgList:         
+        ArgIndices:      [ 117, 4096, 4099 ]
+    # uint32_t (uint32_t, uint32_t*, uint32_t**) [Index: 0x1007]
+    - Kind:            LF_PROCEDURE
+      Procedure:       
+        ReturnType:      117
+        CallConv:        NearC
+        Options:         [ None ]
+        ParameterCount:  0
+        ArgumentList:    4102
+...

llvm/test/DebugInfo/PDB/Inputs/merge2.yaml

+---
+TpiStream:
+  Records:         
+    # uint32_t* [Index: 0x1000]
+    - Kind:            LF_POINTER
+      Pointer:         
+        ReferentType:    117     
+        Attrs:           32778
+    # uint32_t** [Index: 0x1001]
+    - Kind:            LF_POINTER
+      Pointer:         
+        ReferentType:    4096    
+        Attrs:           32778
+    # uint32_t*** [Index: 0x1002]
+    - Kind:            LF_POINTER
+      Pointer:         
+        ReferentType:    4097    
+        Attrs:           32778
+    # [uint32_t, uint32_t*, uint32_t**] [Index: 0x1003]
+    - Kind:            LF_ARGLIST
+      ArgList:         
+        ArgIndices:      [ 117, 4096, 4097 ]
+    # uint32_t (uint32_t, uint32_t*, uint32_t**) [Index: 0x1004]
+    - Kind:            LF_PROCEDURE
+      Procedure:       
+        ReturnType:      117
+        CallConv:        NearC
+        Options:         [ None ]
+        ParameterCount:  0
+        ArgumentList:    4099
+    # int64_t* [Index: 0x1005]
+    - Kind:            LF_POINTER
+      Pointer:         
+        ReferentType:    118     
+        Attrs:           32778
+    # int64_t** [Index: 0x1006]
+    - Kind:            LF_POINTER
+      Pointer:         
+        ReferentType:    4101
+        Attrs:           32778
+    # struct OnlyInMerge2 [Index: 0x1007]
+    - Kind:            LF_STRUCTURE
+      Class:           
+        MemberCount:     0
+        Options:         [ None, ForwardReference, HasUniqueName ]
+        FieldList:       0
+        Name:            'OnlyInMerge2'
+        UniqueName:      'OnlyInMerge2'
+        DerivationList:  0
+        VTableShape:     0
+        Size:            0
+...

llvm/test/DebugInfo/PDB/pdbdump-mergetypes.test

+; RUN: llvm-pdbdump yaml2pdb -pdb=%t.1.pdb %p/Inputs/merge1.yaml
+; RUN: llvm-pdbdump yaml2pdb -pdb=%t.2.pdb %p/Inputs/merge2.yaml
+; RUN: llvm-pdbdump merge -pdb=%t.3.pdb %t.1.pdb %t.2.pdb
+; RUN: llvm-pdbdump raw -tpi-records %t.3.pdb | FileCheck -check-prefix=MERGED %s
+; RUN: llvm-pdbdump raw -tpi-records %t.3.pdb | FileCheck -check-prefix=ARGLIST %s
+
+
+MERGED: Type Info Stream (TPI)
+MERGED: Record count: 9
+MERGED-DAG: PointeeType: unsigned
+MERGED-DAG: PointeeType: unsigned*
+MERGED-DAG: PointeeType: unsigned**
+MERGED-DAG: PointeeType: __int64
+MERGED-DAG: PointeeType: __int64*
+MERGED-DAG: Name: OnlyInMerge1
+MERGED-DAG: Name: OnlyInMerge2
+MERGED-DAG: TypeLeafKind: LF_ARGLIST
+
+ARGLIST: TypeLeafKind: LF_ARGLIST
+ARGLIST-NEXT: NumArgs: 3
+ARGLIST-NEXT: Arguments [
+ARGLIST-NEXT: ArgType: unsigned
+ARGLIST-NEXT: ArgType: unsigned*
+ARGLIST-NEXT: ArgType: unsigned**

llvm/tools/llvm-pdbdump/llvm-pdbdump.cpp

 #include "YAMLOutputStyle.h"
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Config/config.h"
+#include "llvm/DebugInfo/CodeView/LazyRandomTypeCollection.h"
 #include "llvm/DebugInfo/CodeView/ModuleDebugFileChecksumFragment.h"
 #include "llvm/DebugInfo/CodeView/ModuleDebugInlineeLinesFragment.h"
 #include "llvm/DebugInfo/CodeView/ModuleDebugLineFragment.h"
+#include "llvm/DebugInfo/CodeView/TypeStreamMerger.h"
 #include "llvm/DebugInfo/MSF/MSFBuilder.h"
 #include "llvm/DebugInfo/PDB/GenericError.h"
 #include "llvm/DebugInfo/PDB/IPDBEnumChildren.h"
 #include "llvm/DebugInfo/PDB/IPDBRawSymbol.h"
 #include "llvm/DebugInfo/PDB/IPDBSession.h"
 #include "llvm/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.h"
 #include "llvm/DebugInfo/PDB/Native/DbiStream.h"
 #include "llvm/DebugInfo/PDB/Native/DbiStreamBuilder.h"

 cl::SubCommand
     PdbToYamlSubcommand("pdb2yaml",
                         "Generate a detailed YAML description of a PDB File");
 
 cl::SubCommand
     AnalyzeSubcommand("analyze",
                       "Analyze various aspects of a PDB's structure");
 
+cl::SubCommand MergeSubcommand("merge",
+                               "Merge multiple PDBs into a single PDB");
+
 cl::OptionCategory TypeCategory("Symbol Type Options");
 cl::OptionCategory FilterCategory("Filtering and Sorting Options");
 cl::OptionCategory OtherOptions("Other Options");
 
 namespace pretty {
 cl::list<std::string> InputFilenames(cl::Positional,
                                      cl::desc("<input PDB files>"),
                                      cl::OneOrMore, cl::sub(PrettySubcommand));

 
 namespace analyze {
 cl::opt<bool> StringTable("hash-collisions", cl::desc("Find hash collisions"),
                           cl::sub(AnalyzeSubcommand), cl::init(false));
 cl::list<std::string> InputFilename(cl::Positional,
                                     cl::desc("<input PDB file>"), cl::Required,
                                     cl::sub(AnalyzeSubcommand));
 }
+
+namespace merge {
+cl::list<std::string> InputFilenames(cl::Positional,
+                                     cl::desc("<input PDB files>"),
+                                     cl::OneOrMore, cl::sub(MergeSubcommand));
+cl::opt<std::string>
+    PdbOutputFile("pdb", cl::desc("the name of the PDB file to write"),
+                  cl::sub(MergeSubcommand));
+}
 }
 
 static ExitOnError ExitOnErr;
 
 static void yamlToPdb(StringRef Path) {
   BumpPtrAllocator Allocator;
   ErrorOr<std::unique_ptr<MemoryBuffer>> ErrorOrBuffer =
       MemoryBuffer::getFileOrSTDIN(Path, /*FileSize=*/-1,
if (opts::pretty::Externals) {
     Dumper.start(*GlobalScope);
   }
   if (opts::pretty::Lines) {
     Printer.NewLine();
   }
   outs().flush();
 }
 
+static void mergePdbs() {
+  BumpPtrAllocator Allocator;
+  TypeTableBuilder MergedTpi(Allocator);
+  TypeTableBuilder MergedIpi(Allocator);
+
+  // Create a Tpi and Ipi type table with all types from all input files.
+  for (const auto &Path : opts::merge::InputFilenames) {
+    std::unique_ptr<IPDBSession> Session;
+    auto &File = loadPDB(Path, Session);
+    if (File.hasPDBTpiStream()) {
+      auto &Tpi = ExitOnErr(File.getPDBTpiStream());
+      ExitOnErr(codeview::mergeTypeStreams(MergedIpi, MergedTpi, nullptr,
+                                           Tpi.typeArray()));
+    }
+    if (File.hasPDBIpiStream()) {
+      auto &Ipi = ExitOnErr(File.getPDBIpiStream());
+      ExitOnErr(codeview::mergeTypeStreams(MergedIpi, MergedTpi, nullptr,
+                                           Ipi.typeArray()));
+    }
+  }
+
+  // Then write the PDB.
+  PDBFileBuilder Builder(Allocator);
+  ExitOnErr(Builder.initialize(4096));
+  // Add each of the reserved streams.  We might not put any data in them,
+  // but at least they have to be present.
+  for (uint32_t I = 0; I < kSpecialStreamCount; ++I)
+    ExitOnErr(Builder.getMsfBuilder().addStream(0));
+
+  auto &DestTpi = Builder.getTpiBuilder();
+  auto &DestIpi = Builder.getIpiBuilder();
+  MergedTpi.ForEachRecord(
+      [&DestTpi](TypeIndex TI, MutableArrayRef<uint8_t> Data) {
+        DestTpi.addTypeRecord(Data, None);
+      });
+  MergedIpi.ForEachRecord(
+      [&DestIpi](TypeIndex TI, MutableArrayRef<uint8_t> Data) {
+        DestIpi.addTypeRecord(Data, None);
+      });
+
+  SmallString<64> OutFile = opts::merge::PdbOutputFile;
+  if (OutFile.empty()) {
+    OutFile = opts::merge::InputFilenames[0];
+    llvm::sys::path::replace_extension(OutFile, "merged.pdb");
+  }
+  ExitOnErr(Builder.commit(OutFile));
+}
+
 int main(int argc_, const char *argv_[]) {
   // Print a stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal(argv_[0]);
   PrettyStackTraceProgram X(argc_, argv_);
 
   ExitOnErr.setBanner("llvm-pdbdump: ");
 
   SmallVector<const char *, 256> argv;
if (opts::PdbToYamlSubcommand) {
     std::for_each(opts::raw::InputFilenames.begin(),
                   opts::raw::InputFilenames.end(), dumpRaw);
   } else if (opts::DiffSubcommand) {
     if (opts::diff::InputFilenames.size() != 2) {
       errs() << "diff subcommand expects exactly 2 arguments.\n";
       exit(1);
     }
     diff(opts::diff::InputFilenames[0], opts::diff::InputFilenames[1]);
+  } else if (opts::MergeSubcommand) {
+    if (opts::merge::InputFilenames.size() < 2) {
+      errs() << "merge subcommand requires at least 2 input files.\n";
+      exit(1);
+    }
+    mergePdbs();
   }
 
   outs().flush();
   return 0;
 }