commit 579264bd5999931aea53b8e9bb7687f93a828247
author Zachary Turner <zturner@google.com> Thu Dec 06 16:55:00 2018 +0000
committer Zachary Turner <zturner@google.com> Thu Dec 06 16:55:00 2018 +0000
tree 6149f93ae63a3f1f76500f79e2437cc8abd5a730
parent bb650daeaf5d7dd9786ce90431c4c68afb09fabc

Support skewed stream arrays.

VarStreamArray was built on the assumption that it is backed by a
StreamRef, and offset 0 of that StreamRef is the first byte of the first
record in the array.

This is a logical and intuitive assumption, but unfortunately we have
use cases where it doesn't hold. Specifically, a PDB module's symbol
stream is prefixed by 4 bytes containing a magic value, and the first
byte of record data in the array is actually at offset 4 of this byte
sequence.

Previously, we would just truncate the first 4 bytes and then construct
the VarStreamArray with the resulting StreamRef, so that offset 0 of the
underlying stream did correspond to the first byte of the first record,
but this is problematic, because symbol records reference other symbol
records by the absolute offset including that initial magic 4 bytes. So
if another record wants to refer to the first record in the array, it
would say "the record at offset 4".

This led to extremely confusing hacks and semantics in loading code, and
after spending 30 minutes trying to get some math right and failing, I
decided to fix this in the underlying implementation of VarStreamArray.
Now, we can say that a stream is skewed by a particular amount. This
way, when we access a record by absolute offset, we can use the same
values that the records themselves contain, instead of having to do
fixups.

Differential Revision: https://reviews.llvm.org/D55344

llvm-svn: 348499

llvm/lib/DebugInfo/CodeView/CVSymbolVisitor.cpp

diff --git a/llvm/lib/DebugInfo/CodeView/CVSymbolVisitor.cpp b/llvm/lib/DebugInfo/CodeView/CVSymbolVisitor.cpp
index 44a6774..cbcaa56 100644
--- a/llvm/lib/DebugInfo/CodeView/CVSymbolVisitor.cpp
+++ b/llvm/lib/DebugInfo/CodeView/CVSymbolVisitor.cpp
@@ -75,7 +75,7 @@
 Error CVSymbolVisitor::visitSymbolStream(const CVSymbolArray &Symbols,
                                          uint32_t InitialOffset) {
   for (auto I : Symbols) {
-    if (auto EC = visitSymbolRecord(I, InitialOffset))
+    if (auto EC = visitSymbolRecord(I, InitialOffset + Symbols.skew()))
       return EC;
     InitialOffset += I.length();
   }

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

diff --git a/llvm/lib/DebugInfo/PDB/Native/ModuleDebugStream.cpp b/llvm/lib/DebugInfo/PDB/Native/ModuleDebugStream.cpp
index 6464b85..5ff7c15 100644
--- a/llvm/lib/DebugInfo/PDB/Native/ModuleDebugStream.cpp
+++ b/llvm/lib/DebugInfo/PDB/Native/ModuleDebugStream.cpp
@@ -47,7 +47,8 @@
 
   if (auto EC = Reader.readInteger(Signature))
     return EC;
-  if (auto EC = Reader.readSubstream(SymbolsSubstream, SymbolSize - 4))
+  Reader.setOffset(0);
+  if (auto EC = Reader.readSubstream(SymbolsSubstream, SymbolSize))
     return EC;
   if (auto EC = Reader.readSubstream(C11LinesSubstream, C11Size))
     return EC;
@@ -55,8 +56,8 @@
     return EC;
 
   BinaryStreamReader SymbolReader(SymbolsSubstream.StreamData);
-  if (auto EC =
-          SymbolReader.readArray(SymbolArray, SymbolReader.bytesRemaining()))
+  if (auto EC = SymbolReader.readArray(
+          SymbolArray, SymbolReader.bytesRemaining(), sizeof(uint32_t)))
     return EC;
 
   BinaryStreamReader SubsectionsReader(C13LinesSubstream.StreamData);
@@ -98,9 +99,7 @@
 }
 
 CVSymbol ModuleDebugStreamRef::readSymbolAtOffset(uint32_t Offset) const {
-  // Offsets include the size of the 4-byte magic at the beginning, but lookup
-  // doesn't take that into account, so subtract it here.
-  auto Iter = SymbolArray.at(Offset - 4);
+  auto Iter = SymbolArray.at(Offset);
   assert(Iter != SymbolArray.end());
   return *Iter;
 }