[pdb] Add unit tests for PDB MappedBlockStream and zero copy

Zachary Turner · Zachary Turner · commit 90b8b8db2e02 · 2016-05-31T22:41:52.000Z
Differential Revision: http://reviews.llvm.org/D20837 Reviewed By: ruiu llvm-svn: 271346
diff --git a/llvm/include/llvm/DebugInfo/PDB/Raw/IPDBFile.h b/llvm/include/llvm/DebugInfo/PDB/Raw/IPDBFile.h
@@ -19,25 +19,19 @@
 namespace llvm {
 namespace pdb {
 
-struct IPDBFile {
+class IPDBFile {
 public:
   virtual ~IPDBFile() {}
 
   virtual uint32_t getBlockSize() const = 0;
   virtual uint32_t getBlockCount() const = 0;
-  virtual uint32_t getNumDirectoryBytes() const = 0;
-  virtual uint32_t getBlockMapIndex() const = 0;
-  virtual uint32_t getNumDirectoryBlocks() const = 0;
-  virtual uint64_t getBlockMapOffset() const = 0;
 
   virtual uint32_t getNumStreams() const = 0;
   virtual uint32_t getStreamByteSize(uint32_t StreamIndex) const = 0;
   virtual ArrayRef<uint32_t> getStreamBlockList(uint32_t StreamIndex) const = 0;
 
   virtual StringRef getBlockData(uint32_t BlockIndex,
                                  uint32_t NumBytes) const = 0;
-
-  virtual ArrayRef<support::ulittle32_t> getDirectoryBlockArray() = 0;
 };
 }
 }
diff --git a/llvm/include/llvm/DebugInfo/PDB/Raw/InfoStream.h b/llvm/include/llvm/DebugInfo/PDB/Raw/InfoStream.h
@@ -21,6 +21,7 @@
 
 namespace llvm {
 namespace pdb {
+class PDBFile;
 class InfoStream {
 public:
   InfoStream(PDBFile &File);
diff --git a/llvm/include/llvm/DebugInfo/PDB/Raw/MappedBlockStream.h b/llvm/include/llvm/DebugInfo/PDB/Raw/MappedBlockStream.h
@@ -21,17 +21,19 @@
 namespace llvm {
 namespace pdb {
 
-class PDBFile;
+class IPDBFile;
 
 class MappedBlockStream : public codeview::StreamInterface {
 public:
-  MappedBlockStream(uint32_t StreamIdx, const PDBFile &File);
+  MappedBlockStream(uint32_t StreamIdx, const IPDBFile &File);
 
   Error readBytes(uint32_t Offset, uint32_t Size,
                   ArrayRef<uint8_t> &Buffer) const override;
 
   uint32_t getLength() const override { return StreamLength; }
 
+  uint32_t getNumBytesCopied() const;
+
 private:
   Error readBytes(uint32_t Offset, MutableArrayRef<uint8_t> Buffer) const;
   bool tryReadContiguously(uint32_t Offset, uint32_t Size,
@@ -41,7 +43,7 @@ class MappedBlockStream : public codeview::StreamInterface {
   std::vector<uint32_t> BlockList;
   mutable llvm::BumpPtrAllocator Pool;
   mutable DenseMap<uint32_t, uint8_t *> CacheMap;
-  const PDBFile &Pdb;
+  const IPDBFile &Pdb;
 };
 
 } // end namespace pdb
diff --git a/llvm/include/llvm/DebugInfo/PDB/Raw/PDBFile.h b/llvm/include/llvm/DebugInfo/PDB/Raw/PDBFile.h
@@ -39,18 +39,18 @@ class PDBFile : public IPDBFile {
 
   uint32_t getBlockSize() const override;
   uint32_t getBlockCount() const override;
-  uint32_t getNumDirectoryBytes() const override;
-  uint32_t getBlockMapIndex() const override;
-  uint32_t getNumDirectoryBlocks() const override;
-  uint64_t getBlockMapOffset() const override;
+  uint32_t getNumDirectoryBytes() const;
+  uint32_t getBlockMapIndex() const;
+  uint32_t getNumDirectoryBlocks() const;
+  uint64_t getBlockMapOffset() const;
 
   uint32_t getNumStreams() const override;
   uint32_t getStreamByteSize(uint32_t StreamIndex) const override;
   ArrayRef<uint32_t> getStreamBlockList(uint32_t StreamIndex) const override;
 
   StringRef getBlockData(uint32_t BlockIndex, uint32_t NumBytes) const override;
 
-  ArrayRef<support::ulittle32_t> getDirectoryBlockArray() override;
+  ArrayRef<support::ulittle32_t> getDirectoryBlockArray();
 
   Error parseFileHeaders();
   Error parseStreamData();
diff --git a/llvm/include/llvm/Support/Allocator.h b/llvm/include/llvm/Support/Allocator.h
@@ -278,6 +278,8 @@ class BumpPtrAllocatorImpl
     return TotalMemory;
   }
 
+  size_t getBytesAllocated() const { return BytesAllocated; }
+
   void PrintStats() const {
     detail::printBumpPtrAllocatorStats(Slabs.size(), BytesAllocated,
                                        getTotalMemory());
diff --git a/llvm/lib/DebugInfo/PDB/Raw/InfoStream.cpp b/llvm/lib/DebugInfo/PDB/Raw/InfoStream.cpp
@@ -11,6 +11,7 @@
 #include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/DebugInfo/CodeView/StreamReader.h"
+#include "llvm/DebugInfo/PDB/Raw/PDBFile.h"
 #include "llvm/DebugInfo/PDB/Raw/RawConstants.h"
 #include "llvm/DebugInfo/PDB/Raw/RawError.h"
 
diff --git a/llvm/lib/DebugInfo/PDB/Raw/MappedBlockStream.cpp b/llvm/lib/DebugInfo/PDB/Raw/MappedBlockStream.cpp
@@ -14,7 +14,8 @@
 using namespace llvm;
 using namespace llvm::pdb;
 
-MappedBlockStream::MappedBlockStream(uint32_t StreamIdx, const PDBFile &File) : Pdb(File) {
+MappedBlockStream::MappedBlockStream(uint32_t StreamIdx, const IPDBFile &File)
+    : Pdb(File) {
   if (StreamIdx >= Pdb.getNumStreams()) {
     StreamLength = 0;
   } else {
@@ -119,3 +120,7 @@ Error MappedBlockStream::readBytes(uint32_t Offset,
   return Error::success();
 
 }
+
+uint32_t MappedBlockStream::getNumBytesCopied() const {
+  return static_cast<uint32_t>(Pool.getBytesAllocated());
+}
diff --git a/llvm/lib/DebugInfo/PDB/Raw/ModStream.cpp b/llvm/lib/DebugInfo/PDB/Raw/ModStream.cpp
@@ -11,6 +11,7 @@
 
 #include "llvm/DebugInfo/CodeView/StreamReader.h"
 #include "llvm/DebugInfo/PDB/Raw/ModInfo.h"
+#include "llvm/DebugInfo/PDB/Raw/PDBFile.h"
 #include "llvm/DebugInfo/PDB/Raw/RawError.h"
 
 using namespace llvm;
diff --git a/llvm/lib/DebugInfo/PDB/Raw/SymbolStream.cpp b/llvm/lib/DebugInfo/PDB/Raw/SymbolStream.cpp
@@ -13,6 +13,7 @@
 #include "llvm/DebugInfo/CodeView/StreamReader.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.h"
 #include "llvm/DebugInfo/PDB/Raw/MappedBlockStream.h"
+#include "llvm/DebugInfo/PDB/Raw/PDBFile.h"
 #include "llvm/DebugInfo/PDB/Raw/RawConstants.h"
 #include "llvm/DebugInfo/PDB/Raw/RawError.h"
 
diff --git a/llvm/lib/DebugInfo/PDB/Raw/TpiStream.cpp b/llvm/lib/DebugInfo/PDB/Raw/TpiStream.cpp
@@ -13,6 +13,7 @@
 #include "llvm/DebugInfo/CodeView/StreamReader.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.h"
 #include "llvm/DebugInfo/PDB/Raw/MappedBlockStream.h"
+#include "llvm/DebugInfo/PDB/Raw/PDBFile.h"
 #include "llvm/DebugInfo/PDB/Raw/RawConstants.h"
 #include "llvm/DebugInfo/PDB/Raw/RawError.h"
 
diff --git a/llvm/unittests/DebugInfo/PDB/CMakeLists.txt b/llvm/unittests/DebugInfo/PDB/CMakeLists.txt
@@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 set(DebugInfoPDBSources
+  MappedBlockStreamTest.cpp
   PDBApiTest.cpp
   )
 
diff --git a/llvm/unittests/DebugInfo/PDB/MappedBlockStreamTest.cpp b/llvm/unittests/DebugInfo/PDB/MappedBlockStreamTest.cpp
@@ -0,0 +1,161 @@
+//===- llvm/unittest/DebugInfo/PDB/MappedBlockStreamTest.cpp --------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include <unordered_map>
+
+#include "llvm/DebugInfo/CodeView/StreamReader.h"
+#include "llvm/DebugInfo/CodeView/StreamRef.h"
+#include "llvm/DebugInfo/PDB/Raw/IPDBFile.h"
+#include "llvm/DebugInfo/PDB/Raw/MappedBlockStream.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace llvm::codeview;
+using namespace llvm::pdb;
+
+namespace {
+
+#define EXPECT_NO_ERROR(Err)                                                   \
+  {                                                                            \
+    auto E = std::move(Err);                                                   \
+    EXPECT_FALSE(static_cast<bool>(E));                                        \
+    if (E)                                                                     \
+      consumeError(std::move(E));                                              \
+  }
+
+#define EXPECT_ERROR(Err)                                                      \
+  {                                                                            \
+    auto E = std::move(Err);                                                   \
+    EXPECT_TRUE(static_cast<bool>(E));                                         \
+    if (E)                                                                     \
+      consumeError(std::move(E));                                              \
+  }
+
+class DiscontiguousFile : public IPDBFile {
+public:
+  DiscontiguousFile()
+      : Blocks{0, 1, 2, 5, 4, 3, 6, 7, 8, 9},
+        Data{'A', 'B', 'C', 'F', 'E', 'D', 'G', 'H', 'I', 'J'} {}
+
+  virtual uint32_t getBlockSize() const override { return 1; }
+  virtual uint32_t getBlockCount() const override { return 10; }
+  virtual uint32_t getNumStreams() const override { return 1; }
+  virtual uint32_t getStreamByteSize(uint32_t StreamIndex) const override {
+    return getBlockCount() * getBlockSize();
+  }
+  virtual ArrayRef<uint32_t>
+  getStreamBlockList(uint32_t StreamIndex) const override {
+    if (StreamIndex != 0)
+      return ArrayRef<uint32_t>();
+    return Blocks;
+  }
+  virtual StringRef getBlockData(uint32_t BlockIndex,
+                                 uint32_t NumBytes) const override {
+    return StringRef(&Data[BlockIndex], NumBytes);
+  }
+
+private:
+  std::vector<uint32_t> Blocks;
+  std::vector<char> Data;
+};
+
+// Tests that a read which is entirely contained within a single block works
+// and does not allocate.
+TEST(MappedBlockStreamTest, ZeroCopyReadNoBreak) {
+  DiscontiguousFile F;
+  MappedBlockStream S(0, F);
+  StreamReader R(S);
+  StringRef Str;
+  EXPECT_NO_ERROR(R.readFixedString(Str, 1));
+  EXPECT_EQ(Str, StringRef("A"));
+  EXPECT_EQ(0, S.getNumBytesCopied());
+}
+
+// Tests that a read which outputs into a full destination buffer works and
+// does not fail due to the length of the output buffer.
+TEST(MappedBlockStreamTest, ReadOntoNonEmptyBuffer) {
+  DiscontiguousFile F;
+  MappedBlockStream S(0, F);
+  StreamReader R(S);
+  StringRef Str = "ZYXWVUTSRQPONMLKJIHGFEDCBA";
+  EXPECT_NO_ERROR(R.readFixedString(Str, 1));
+  EXPECT_EQ(Str, StringRef("A"));
+  EXPECT_EQ(0, S.getNumBytesCopied());
+}
+
+// Tests that a read which crosses a block boundary, but where the subsequent
+// blocks are still contiguous in memory to the previous block works and does
+// not allocate memory.
+TEST(MappedBlockStreamTest, ZeroCopyReadContiguousBreak) {
+  DiscontiguousFile F;
+  MappedBlockStream S(0, F);
+  StreamReader R(S);
+  StringRef Str;
+  EXPECT_NO_ERROR(R.readFixedString(Str, 2));
+  EXPECT_EQ(Str, StringRef("AB"));
+  EXPECT_EQ(0, S.getNumBytesCopied());
+
+  R.setOffset(6);
+  EXPECT_NO_ERROR(R.readFixedString(Str, 4));
+  EXPECT_EQ(Str, StringRef("GHIJ"));
+  EXPECT_EQ(0, S.getNumBytesCopied());
+}
+
+// Tests that a read which crosses a block boundary and cannot be referenced
+// contiguously works and allocates only the precise amount of bytes
+// requested.
+TEST(MappedBlockStreamTest, CopyReadNonContiguousBreak) {
+  DiscontiguousFile F;
+  MappedBlockStream S(0, F);
+  StreamReader R(S);
+  StringRef Str;
+  EXPECT_NO_ERROR(R.readFixedString(Str, 10));
+  EXPECT_EQ(Str, StringRef("ABCDEFGHIJ"));
+  EXPECT_EQ(10, S.getNumBytesCopied());
+}
+
+// Test that an out of bounds read which doesn't cross a block boundary
+// fails and allocates no memory.
+TEST(MappedBlockStreamTest, InvalidReadSizeNoBreak) {
+  DiscontiguousFile F;
+  MappedBlockStream S(0, F);
+  StreamReader R(S);
+  StringRef Str;
+
+  R.setOffset(10);
+  EXPECT_ERROR(R.readFixedString(Str, 1));
+  EXPECT_EQ(0, S.getNumBytesCopied());
+}
+
+// Test that an out of bounds read which crosses a contiguous block boundary
+// fails and allocates no memory.
+TEST(MappedBlockStreamTest, InvalidReadSizeContiguousBreak) {
+  DiscontiguousFile F;
+  MappedBlockStream S(0, F);
+  StreamReader R(S);
+  StringRef Str;
+
+  R.setOffset(6);
+  EXPECT_ERROR(R.readFixedString(Str, 5));
+  EXPECT_EQ(0, S.getNumBytesCopied());
+}
+
+// Test that an out of bounds read which crosses a discontiguous block
+// boundary fails and allocates no memory.
+TEST(MappedBlockStreamTest, InvalidReadSizeNonContiguousBreak) {
+  DiscontiguousFile F;
+  MappedBlockStream S(0, F);
+  StreamReader R(S);
+  StringRef Str;
+
+  EXPECT_ERROR(R.readFixedString(Str, 11));
+  EXPECT_EQ(0, S.getNumBytesCopied());
+}
+
+} // end anonymous namespace

Original file line number	Diff line number	Diff line change
`@@ -278,6 +278,8 @@ class BumpPtrAllocatorImpl`
`278`	`278`	`return TotalMemory;`
`279`	`279`	`}`
`280`	`280`
	`281`	`+ size_t getBytesAllocated() const { return BytesAllocated; }`
	`282`	`+`
`281`	`283`	`void PrintStats() const {`
`282`	`284`	`detail::printBumpPtrAllocatorStats(Slabs.size(), BytesAllocated,`
`283`	`285`	`getTotalMemory());`
Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS`
`3`	`3`	`)`
`4`	`4`
`5`	`5`	`set(DebugInfoPDBSources`
	`6`	`+ MappedBlockStreamTest.cpp`
`6`	`7`	`PDBApiTest.cpp`
`7`	`8`	`)`
`8`	`9`