[PDB] Fix an issue writing the publics stream.

In the refactor to merge the publics and globals stream, a bug
was introduced that wrote the wrong value for one of the fields
of the PublicsStreamHeader.  This caused debugging in WinDbg
to break.

We had no way of dumping any of these fields, so in addition to
fixing the bug I've added dumping support for them along with a
test that verifies the correct value is written.

llvm-svn: 310439

diff --git a/lld/test/COFF/pdb-publics-import.test b/lld/test/COFF/pdb-publics-import.test
index 4c644dd..1c75e90 100644 (file)
--- a/lld/test/COFF/pdb-publics-import.test
+++ b/lld/test/COFF/pdb-publics-import.test
@@ -11,6 +11,7 @@ RUN: llvm-pdbutil dump %t2.pdb -publics -section-contribs | FileCheck %s
 
 CHECK:                             Public Symbols
 CHECK-NEXT: ============================================================
+CHECK-NEXT:    Records
 CHECK-NEXT:      112 | S_PUB32 [size = 20] `main`
 CHECK-NEXT:            flags = function, addr = 0001:0000
 CHECK-NEXT:       64 | S_PUB32 [size = 24] `exportfn1`

diff --git a/lld/test/COFF/pdb.test b/lld/test/COFF/pdb.test
index 612b198..bf91a4b 100644 (file)
--- a/lld/test/COFF/pdb.test
+++ b/lld/test/COFF/pdb.test
@@ -173,21 +173,26 @@ RAW-NEXT:            0x100A: `ret42-sub.c`
 RAW-NEXT:            0x1008: `D:\b\vc140.pdb`
 RAW-NEXT:            0x1006: ` -I"C:\Program Files (x86)\Windows Kits\8.1\include\um" -I"C:\Program Files (x86)\Windows Kits\8.1\include\winrt" -TC -X`
 RAW:                            Public Symbols
-RAW-NEXT:============================================================
-RAW-NEXT:      20 | S_PUB32 [size = 20] `main`
-RAW-NEXT:           flags = function, addr = 0002:0000
-RAW-NEXT:       0 | S_PUB32 [size = 20] `foo`
-RAW-NEXT:           flags = function, addr = 0002:0016
-RAW-NOT: S_PUB32
-RAW-NEXT:  Hash Records
-RAW-NEXT:    off = 21, refcnt = 1
-RAW-NEXT:    off = 1, refcnt = 1
-RAW-NEXT:  Hash Buckets
-RAW-NEXT:    0x00000000
-RAW-NEXT:    0x0000000c
-RAW-NEXT:  Address Map
-RAW-NEXT:    off = 20
-RAW-NEXT:    off = 0
+RAW-NEXT: ============================================================
+RAW-NEXT:   Publics Header
+RAW-NEXT:     sym hash = 556, thunk table addr = 0000:0000
+RAW-NEXT:   GSI Header
+RAW-NEXT:     sig = 0xFFFFFFFF, hdr = 0xF12F091A, hr size = 16, num buckets = 524
+RAW-NEXT:   Records
+RAW-NEXT:       20 | S_PUB32 [size = 20] `main`
+RAW-NEXT:            flags = function, addr = 0002:0000
+RAW-NEXT:        0 | S_PUB32 [size = 20] `foo`
+RAW-NEXT:            flags = function, addr = 0002:0016
+RAW-NOT:             S_PUB32
+RAW-NEXT:   Hash Entries
+RAW-NEXT:     off = 21, refcnt = 1
+RAW-NEXT:     off = 1, refcnt = 1
+RAW-NEXT:   Hash Buckets
+RAW-NEXT:     0x00000000
+RAW-NEXT:     0x0000000c
+RAW-NEXT:   Address Map
+RAW-NEXT:     off = 20
+RAW-NEXT:     off = 0
 RAW:                            Section Headers
 RAW-NEXT: ============================================================
 RAW:        SECTION HEADER #1

diff --git a/llvm/include/llvm/DebugInfo/PDB/Native/GlobalsStream.h b/llvm/include/llvm/DebugInfo/PDB/Native/GlobalsStream.h
index 615ca74..6351b35 100644 (file)
--- a/llvm/include/llvm/DebugInfo/PDB/Native/GlobalsStream.h
+++ b/llvm/include/llvm/DebugInfo/PDB/Native/GlobalsStream.h
@@ -57,6 +57,11 @@ public:
 
   Error read(BinaryStreamReader &Reader);
 
+  uint32_t getVerSignature() const { return HashHdr->VerSignature; }
+  uint32_t getVerHeader() const { return HashHdr->VerHdr; }
+  uint32_t getHashRecordSize() const { return HashHdr->HrSize; }
+  uint32_t getNumBuckets() const { return HashHdr->NumBuckets; }
+
   typedef GSIHashHeader iterator;
   GSIHashIterator begin() const { return GSIHashIterator(HashRecords.begin()); }
   GSIHashIterator end() const { return GSIHashIterator(HashRecords.end()); }

diff --git a/llvm/include/llvm/DebugInfo/PDB/Native/PublicsStream.h b/llvm/include/llvm/DebugInfo/PDB/Native/PublicsStream.h
index f28628d..2d0222a 100644 (file)
--- a/llvm/include/llvm/DebugInfo/PDB/Native/PublicsStream.h
+++ b/llvm/include/llvm/DebugInfo/PDB/Native/PublicsStream.h
@@ -32,7 +32,8 @@ public:
   Error reload();
 
   uint32_t getSymHash() const;
-  uint32_t getAddrMap() const;
+  uint16_t getThunkTableSection() const;
+  uint32_t getThunkTableOffset() const;
   const GSIHashTable &getPublicsTable() const { return PublicsTable; }
   FixedStreamArray<support::ulittle32_t> getAddressMap() const {
     return AddressMap;

diff --git a/llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp b/llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp
index eb7a0bb..46951a0 100644 (file)
--- a/llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp
+++ b/llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp
@@ -153,7 +153,6 @@ Error GSIStreamBuilder::finalizeMsfLayout() {
   if (!Idx)
     return Idx.takeError();
   PSH->StreamIndex = *Idx;
-
   Idx = Msf.addStream(calculateGlobalsHashStreamSize());
   if (!Idx)
     return Idx.takeError();
@@ -253,32 +252,22 @@ Error GSIStreamBuilder::commitSymbolRecordStream(
 
 Error GSIStreamBuilder::commitPublicsHashStream(
     WritableBinaryStreamRef Stream) {
-  // Skip the publics stream header so that we can write the GSH header first.
-  // Then seek back to the beginning and update the publics stream header with
-  // the byte offset after the GSH header.
   BinaryStreamWriter Writer(Stream);
-  Writer.setOffset(sizeof(PublicsStreamHeader));
-
-  if (auto EC = PSH->commit(Writer))
-    return EC;
-  uint32_t OffsetAfterGSIHashes = Writer.getOffset();
-
-  Writer.setOffset(0);
-
-  // FIXME: Fill these in. They are for incremental linking.
   PublicsStreamHeader Header;
-  Header.AddrMap = PSH->Records.size() * 4;
 
+  // FIXME: Fill these in. They are for incremental linking.
   Header.NumThunks = 0;
   Header.SizeOfThunk = 0;
   Header.ISectThunkTable = 0;
   Header.OffThunkTable = 0;
   Header.NumSections = 0;
-  Header.SymHash = OffsetAfterGSIHashes;
+  Header.SymHash = PSH->calculateSerializedLength();
+  Header.AddrMap = PSH->Records.size() * 4;
   if (auto EC = Writer.writeObject(Header))
     return EC;
 
-  Writer.setOffset(OffsetAfterGSIHashes);
+  if (auto EC = PSH->commit(Writer))
+    return EC;
 
   std::vector<ulittle32_t> AddrMap = computeAddrMap(PSH->Records);
   if (auto EC = Writer.writeArray(makeArrayRef(AddrMap)))

diff --git a/llvm/lib/DebugInfo/PDB/Native/PublicsStream.cpp b/llvm/lib/DebugInfo/PDB/Native/PublicsStream.cpp
index c716720..a3a44ce 100644 (file)
--- a/llvm/lib/DebugInfo/PDB/Native/PublicsStream.cpp
+++ b/llvm/lib/DebugInfo/PDB/Native/PublicsStream.cpp
@@ -46,7 +46,12 @@ PublicsStream::PublicsStream(std::unique_ptr<MappedBlockStream> Stream)
 PublicsStream::~PublicsStream() = default;
 
 uint32_t PublicsStream::getSymHash() const { return Header->SymHash; }
-uint32_t PublicsStream::getAddrMap() const { return Header->AddrMap; }
+uint16_t PublicsStream::getThunkTableSection() const {
+  return Header->ISectThunkTable;
+}
+uint32_t PublicsStream::getThunkTableOffset() const {
+  return Header->OffThunkTable;
+}
 
 // Publics stream contains fixed-size headers and a serialized hash table.
 // This implementation is not complete yet. It reads till the end of the

diff --git a/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp b/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp
index 73e4a14..bc0bb08 100644 (file)
--- a/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp
+++ b/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp
@@ -896,6 +896,13 @@ Error DumpOutputStyle::dumpPublics() {
   auto &Publics = Err(File.getPDBPublicsStream());
 
   const GSIHashTable &PublicsTable = Publics.getPublicsTable();
+  if (opts::dump::DumpPublicExtras) {
+    P.printLine("Publics Header");
+    AutoIndent Indent(P);
+    P.formatLine("sym hash = {0}, thunk table addr = {1}", Publics.getSymHash(),
+                 formatSegmentOffset(Publics.getThunkTableSection(),
+                                     Publics.getThunkTableOffset()));
+  }
   Err(dumpSymbolsFromGSI(PublicsTable, opts::dump::DumpPublicExtras));
 
   // Skip the rest if we aren't dumping extras.
@@ -941,30 +948,42 @@ Error DumpOutputStyle::dumpSymbolsFromGSI(const GSIHashTable &Table,
   auto ExpectedIds = initializeTypes(StreamIPI);
   if (!ExpectedIds)
     return ExpectedIds.takeError();
-  SymbolVisitorCallbackPipeline Pipeline;
-  SymbolDeserializer Deserializer(nullptr, CodeViewContainer::Pdb);
-  MinimalSymbolDumper Dumper(P, opts::dump::DumpSymRecordBytes, *ExpectedIds,
-                             *ExpectedTypes);
-
-  Pipeline.addCallbackToPipeline(Deserializer);
-  Pipeline.addCallbackToPipeline(Dumper);
-  CVSymbolVisitor Visitor(Pipeline);
-
-  BinaryStreamRef SymStream =
-      ExpectedSyms->getSymbolArray().getUnderlyingStream();
-  for (uint32_t PubSymOff : Table) {
-    Expected<CVSymbol> Sym = readSymbolFromStream(SymStream, PubSymOff);
-    if (!Sym)
-      return Sym.takeError();
-    if (auto E = Visitor.visitSymbolRecord(*Sym, PubSymOff))
-      return E;
+
+  if (HashExtras) {
+    P.printLine("GSI Header");
+    AutoIndent Indent(P);
+    P.formatLine("sig = {0:X}, hdr = {1:X}, hr size = {2}, num buckets = {3}",
+                 Table.getVerSignature(), Table.getVerHeader(),
+                 Table.getHashRecordSize(), Table.getNumBuckets());
+  }
+
+  {
+    P.printLine("Records");
+    SymbolVisitorCallbackPipeline Pipeline;
+    SymbolDeserializer Deserializer(nullptr, CodeViewContainer::Pdb);
+    MinimalSymbolDumper Dumper(P, opts::dump::DumpSymRecordBytes, *ExpectedIds,
+                               *ExpectedTypes);
+
+    Pipeline.addCallbackToPipeline(Deserializer);
+    Pipeline.addCallbackToPipeline(Dumper);
+    CVSymbolVisitor Visitor(Pipeline);
+
+    BinaryStreamRef SymStream =
+        ExpectedSyms->getSymbolArray().getUnderlyingStream();
+    for (uint32_t PubSymOff : Table) {
+      Expected<CVSymbol> Sym = readSymbolFromStream(SymStream, PubSymOff);
+      if (!Sym)
+        return Sym.takeError();
+      if (auto E = Visitor.visitSymbolRecord(*Sym, PubSymOff))
+        return E;
+    }
   }
 
   // Return early if we aren't dumping public hash table and address map info.
   if (!HashExtras)
     return Error::success();
 
-  P.formatLine("Hash Records");
+  P.formatLine("Hash Entries");
   {
     AutoIndent Indent2(P);
     for (const PSHashRecord &HR : Table.HashRecords)