[llvm] r303916 - Fix a bug in MappedBlockStream.

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Thu May 25 14:12:01 PDT 2017


Author: zturner
Date: Thu May 25 16:12:00 2017
New Revision: 303916

URL: http://llvm.org/viewvc/llvm-project?rev=303916&view=rev
Log:
Fix a bug in MappedBlockStream.

It was using the number of blocks of the entire PDB file as the number
of blocks of each stream that was created.  This was only an issue in
the readLongestContiguousChunk function, which  was never called prior.
This bug surfaced when I updated an algorithm to use this function and
the algorithm broke.

Modified:
    llvm/trunk/include/llvm/DebugInfo/MSF/MappedBlockStream.h
    llvm/trunk/lib/DebugInfo/MSF/MappedBlockStream.cpp
    llvm/trunk/unittests/DebugInfo/PDB/MappedBlockStreamTest.cpp

Modified: llvm/trunk/include/llvm/DebugInfo/MSF/MappedBlockStream.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/MSF/MappedBlockStream.h?rev=303916&r1=303915&r2=303916&view=diff
==============================================================================
--- llvm/trunk/include/llvm/DebugInfo/MSF/MappedBlockStream.h (original)
+++ llvm/trunk/include/llvm/DebugInfo/MSF/MappedBlockStream.h Thu May 25 16:12:00 2017
@@ -43,8 +43,8 @@ class MappedBlockStream : public BinaryS
   friend class WritableMappedBlockStream;
 public:
   static std::unique_ptr<MappedBlockStream>
-  createStream(uint32_t BlockSize, uint32_t NumBlocks,
-               const MSFStreamLayout &Layout, BinaryStreamRef MsfData);
+  createStream(uint32_t BlockSize, const MSFStreamLayout &Layout,
+               BinaryStreamRef MsfData);
 
   static std::unique_ptr<MappedBlockStream>
   createIndexedStream(const MSFLayout &Layout, BinaryStreamRef MsfData,
@@ -74,12 +74,11 @@ public:
   void invalidateCache();
 
   uint32_t getBlockSize() const { return BlockSize; }
-  uint32_t getNumBlocks() const { return NumBlocks; }
+  uint32_t getNumBlocks() const { return StreamLayout.Blocks.size(); }
   uint32_t getStreamLength() const { return StreamLayout.Length; }
 
 protected:
-  MappedBlockStream(uint32_t BlockSize, uint32_t NumBlocks,
-                    const MSFStreamLayout &StreamLayout,
+  MappedBlockStream(uint32_t BlockSize, const MSFStreamLayout &StreamLayout,
                     BinaryStreamRef MsfData);
 
 private:
@@ -91,7 +90,6 @@ private:
                            ArrayRef<uint8_t> &Buffer);
 
   const uint32_t BlockSize;
-  const uint32_t NumBlocks;
   const MSFStreamLayout StreamLayout;
   BinaryStreamRef MsfData;
 
@@ -103,8 +101,8 @@ private:
 class WritableMappedBlockStream : public WritableBinaryStream {
 public:
   static std::unique_ptr<WritableMappedBlockStream>
-  createStream(uint32_t BlockSize, uint32_t NumBlocks,
-               const MSFStreamLayout &Layout, WritableBinaryStreamRef MsfData);
+  createStream(uint32_t BlockSize, const MSFStreamLayout &Layout,
+               WritableBinaryStreamRef MsfData);
 
   static std::unique_ptr<WritableMappedBlockStream>
   createIndexedStream(const MSFLayout &Layout, WritableBinaryStreamRef MsfData,
@@ -139,7 +137,7 @@ public:
   uint32_t getStreamLength() const { return ReadInterface.getStreamLength(); }
 
 protected:
-  WritableMappedBlockStream(uint32_t BlockSize, uint32_t NumBlocks,
+  WritableMappedBlockStream(uint32_t BlockSize,
                             const MSFStreamLayout &StreamLayout,
                             WritableBinaryStreamRef MsfData);
 

Modified: llvm/trunk/lib/DebugInfo/MSF/MappedBlockStream.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/MSF/MappedBlockStream.cpp?rev=303916&r1=303915&r2=303916&view=diff
==============================================================================
--- llvm/trunk/lib/DebugInfo/MSF/MappedBlockStream.cpp (original)
+++ llvm/trunk/lib/DebugInfo/MSF/MappedBlockStream.cpp Thu May 25 16:12:00 2017
@@ -45,18 +45,17 @@ static Interval intersect(const Interval
                         std::min(I1.second, I2.second));
 }
 
-MappedBlockStream::MappedBlockStream(uint32_t BlockSize, uint32_t NumBlocks,
+MappedBlockStream::MappedBlockStream(uint32_t BlockSize,
                                      const MSFStreamLayout &Layout,
                                      BinaryStreamRef MsfData)
-    : BlockSize(BlockSize), NumBlocks(NumBlocks), StreamLayout(Layout),
-      MsfData(MsfData) {}
+    : BlockSize(BlockSize), StreamLayout(Layout), MsfData(MsfData) {}
 
 std::unique_ptr<MappedBlockStream>
-MappedBlockStream::createStream(uint32_t BlockSize, uint32_t NumBlocks,
+MappedBlockStream::createStream(uint32_t BlockSize,
                                 const MSFStreamLayout &Layout,
                                 BinaryStreamRef MsfData) {
   return llvm::make_unique<MappedBlockStreamImpl<MappedBlockStream>>(
-      BlockSize, NumBlocks, Layout, MsfData);
+      BlockSize, Layout, MsfData);
 }
 
 std::unique_ptr<MappedBlockStream> MappedBlockStream::createIndexedStream(
@@ -66,7 +65,7 @@ std::unique_ptr<MappedBlockStream> Mappe
   SL.Blocks = Layout.StreamMap[StreamIndex];
   SL.Length = Layout.StreamSizes[StreamIndex];
   return llvm::make_unique<MappedBlockStreamImpl<MappedBlockStream>>(
-      Layout.SB->BlockSize, Layout.SB->NumBlocks, SL, MsfData);
+      Layout.SB->BlockSize, SL, MsfData);
 }
 
 std::unique_ptr<MappedBlockStream>
@@ -75,7 +74,7 @@ MappedBlockStream::createDirectoryStream
   MSFStreamLayout SL;
   SL.Blocks = Layout.DirectoryBlocks;
   SL.Length = Layout.SB->NumDirectoryBytes;
-  return createStream(Layout.SB->BlockSize, Layout.SB->NumBlocks, SL, MsfData);
+  return createStream(Layout.SB->BlockSize, SL, MsfData);
 }
 
 std::unique_ptr<MappedBlockStream>
@@ -83,7 +82,7 @@ MappedBlockStream::createFpmStream(const
                                    BinaryStreamRef MsfData) {
   MSFStreamLayout SL;
   initializeFpmStreamLayout(Layout, SL);
-  return createStream(Layout.SB->BlockSize, Layout.SB->NumBlocks, SL, MsfData);
+  return createStream(Layout.SB->BlockSize, SL, MsfData);
 }
 
 Error MappedBlockStream::readBytes(uint32_t Offset, uint32_t Size,
@@ -173,7 +172,7 @@ Error MappedBlockStream::readLongestCont
   uint32_t First = Offset / BlockSize;
   uint32_t Last = First;
 
-  while (Last < NumBlocks - 1) {
+  while (Last < getNumBlocks() - 1) {
     if (StreamLayout.Blocks[Last] != StreamLayout.Blocks[Last + 1] - 1)
       break;
     ++Last;
@@ -313,17 +312,16 @@ void MappedBlockStream::fixCacheAfterWri
 }
 
 WritableMappedBlockStream::WritableMappedBlockStream(
-    uint32_t BlockSize, uint32_t NumBlocks, const MSFStreamLayout &Layout,
+    uint32_t BlockSize, const MSFStreamLayout &Layout,
     WritableBinaryStreamRef MsfData)
-    : ReadInterface(BlockSize, NumBlocks, Layout, MsfData),
-      WriteInterface(MsfData) {}
+    : ReadInterface(BlockSize, Layout, MsfData), WriteInterface(MsfData) {}
 
 std::unique_ptr<WritableMappedBlockStream>
-WritableMappedBlockStream::createStream(uint32_t BlockSize, uint32_t NumBlocks,
+WritableMappedBlockStream::createStream(uint32_t BlockSize,
                                         const MSFStreamLayout &Layout,
                                         WritableBinaryStreamRef MsfData) {
   return llvm::make_unique<MappedBlockStreamImpl<WritableMappedBlockStream>>(
-      BlockSize, NumBlocks, Layout, MsfData);
+      BlockSize, Layout, MsfData);
 }
 
 std::unique_ptr<WritableMappedBlockStream>
@@ -334,7 +332,7 @@ WritableMappedBlockStream::createIndexed
   MSFStreamLayout SL;
   SL.Blocks = Layout.StreamMap[StreamIndex];
   SL.Length = Layout.StreamSizes[StreamIndex];
-  return createStream(Layout.SB->BlockSize, Layout.SB->NumBlocks, SL, MsfData);
+  return createStream(Layout.SB->BlockSize, SL, MsfData);
 }
 
 std::unique_ptr<WritableMappedBlockStream>
@@ -343,7 +341,7 @@ WritableMappedBlockStream::createDirecto
   MSFStreamLayout SL;
   SL.Blocks = Layout.DirectoryBlocks;
   SL.Length = Layout.SB->NumDirectoryBytes;
-  return createStream(Layout.SB->BlockSize, Layout.SB->NumBlocks, SL, MsfData);
+  return createStream(Layout.SB->BlockSize, SL, MsfData);
 }
 
 std::unique_ptr<WritableMappedBlockStream>
@@ -351,7 +349,7 @@ WritableMappedBlockStream::createFpmStre
                                            WritableBinaryStreamRef MsfData) {
   MSFStreamLayout SL;
   initializeFpmStreamLayout(Layout, SL);
-  return createStream(Layout.SB->BlockSize, Layout.SB->NumBlocks, SL, MsfData);
+  return createStream(Layout.SB->BlockSize, SL, MsfData);
 }
 
 Error WritableMappedBlockStream::readBytes(uint32_t Offset, uint32_t Size,

Modified: llvm/trunk/unittests/DebugInfo/PDB/MappedBlockStreamTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/DebugInfo/PDB/MappedBlockStreamTest.cpp?rev=303916&r1=303915&r2=303916&view=diff
==============================================================================
--- llvm/trunk/unittests/DebugInfo/PDB/MappedBlockStreamTest.cpp (original)
+++ llvm/trunk/unittests/DebugInfo/PDB/MappedBlockStreamTest.cpp Thu May 25 16:12:00 2017
@@ -75,12 +75,19 @@ private:
   MutableArrayRef<uint8_t> Data;
 };
 
+TEST(MappedBlockStreamTest, NumBlocks) {
+  DiscontiguousStream F(BlocksAry, DataAry);
+  auto S = MappedBlockStream::createStream(F.block_size(), F.layout(), F);
+  EXPECT_EQ(F.block_size(), S->getBlockSize());
+  EXPECT_EQ(F.layout().Blocks.size(), S->getNumBlocks());
+
+}
+
 // Tests that a read which is entirely contained within a single block works
 // and does not allocate.
 TEST(MappedBlockStreamTest, ReadBeyondEndOfStreamRef) {
   DiscontiguousStream F(BlocksAry, DataAry);
-  auto S = MappedBlockStream::createStream(F.block_size(), F.block_count(),
-                                           F.layout(), F);
+  auto S = MappedBlockStream::createStream(F.block_size(), F.layout(), F);
 
   BinaryStreamReader R(*S);
   BinaryStreamRef SR;
@@ -95,8 +102,7 @@ TEST(MappedBlockStreamTest, ReadBeyondEn
 // does not fail due to the length of the output buffer.
 TEST(MappedBlockStreamTest, ReadOntoNonEmptyBuffer) {
   DiscontiguousStream F(BlocksAry, DataAry);
-  auto S = MappedBlockStream::createStream(F.block_size(), F.block_count(),
-                                           F.layout(), F);
+  auto S = MappedBlockStream::createStream(F.block_size(), F.layout(), F);
 
   BinaryStreamReader R(*S);
   StringRef Str = "ZYXWVUTSRQPONMLKJIHGFEDCBA";
@@ -110,7 +116,7 @@ TEST(MappedBlockStreamTest, ReadOntoNonE
 // not allocate memory.
 TEST(MappedBlockStreamTest, ZeroCopyReadContiguousBreak) {
   DiscontiguousStream F(BlocksAry, DataAry);
-  auto S = MappedBlockStream::createStream(F.block_size(), F.block_count(),
+  auto S = MappedBlockStream::createStream(F.block_size(),
                                            F.layout(), F);
   BinaryStreamReader R(*S);
   StringRef Str;
@@ -129,8 +135,7 @@ TEST(MappedBlockStreamTest, ZeroCopyRead
 // requested.
 TEST(MappedBlockStreamTest, CopyReadNonContiguousBreak) {
   DiscontiguousStream F(BlocksAry, DataAry);
-  auto S = MappedBlockStream::createStream(F.block_size(), F.block_count(),
-                                           F.layout(), F);
+  auto S = MappedBlockStream::createStream(F.block_size(), F.layout(), F);
   BinaryStreamReader R(*S);
   StringRef Str;
   EXPECT_NO_ERROR(R.readFixedString(Str, 10));
@@ -142,8 +147,7 @@ TEST(MappedBlockStreamTest, CopyReadNonC
 // fails and allocates no memory.
 TEST(MappedBlockStreamTest, InvalidReadSizeNoBreak) {
   DiscontiguousStream F(BlocksAry, DataAry);
-  auto S = MappedBlockStream::createStream(F.block_size(), F.block_count(),
-                                           F.layout(), F);
+  auto S = MappedBlockStream::createStream(F.block_size(), F.layout(), F);
   BinaryStreamReader R(*S);
   StringRef Str;
 
@@ -156,8 +160,7 @@ TEST(MappedBlockStreamTest, InvalidReadS
 // fails and allocates no memory.
 TEST(MappedBlockStreamTest, InvalidReadSizeContiguousBreak) {
   DiscontiguousStream F(BlocksAry, DataAry);
-  auto S = MappedBlockStream::createStream(F.block_size(), F.block_count(),
-                                           F.layout(), F);
+  auto S = MappedBlockStream::createStream(F.block_size(), F.layout(), F);
   BinaryStreamReader R(*S);
   StringRef Str;
 
@@ -170,8 +173,7 @@ TEST(MappedBlockStreamTest, InvalidReadS
 // boundary fails and allocates no memory.
 TEST(MappedBlockStreamTest, InvalidReadSizeNonContiguousBreak) {
   DiscontiguousStream F(BlocksAry, DataAry);
-  auto S = MappedBlockStream::createStream(F.block_size(), F.block_count(),
-                                           F.layout(), F);
+  auto S = MappedBlockStream::createStream(F.block_size(), F.layout(), F);
   BinaryStreamReader R(*S);
   StringRef Str;
 
@@ -183,8 +185,7 @@ TEST(MappedBlockStreamTest, InvalidReadS
 // beyond the end of a StreamRef fails.
 TEST(MappedBlockStreamTest, ZeroCopyReadNoBreak) {
   DiscontiguousStream F(BlocksAry, DataAry);
-  auto S = MappedBlockStream::createStream(F.block_size(), F.block_count(),
-                                           F.layout(), F);
+  auto S = MappedBlockStream::createStream(F.block_size(), F.layout(), F);
   BinaryStreamReader R(*S);
   StringRef Str;
   EXPECT_NO_ERROR(R.readFixedString(Str, 1));
@@ -197,8 +198,7 @@ TEST(MappedBlockStreamTest, ZeroCopyRead
 // previous allocation.
 TEST(MappedBlockStreamTest, UnalignedOverlappingRead) {
   DiscontiguousStream F(BlocksAry, DataAry);
-  auto S = MappedBlockStream::createStream(F.block_size(), F.block_count(),
-                                           F.layout(), F);
+  auto S = MappedBlockStream::createStream(F.block_size(), F.layout(), F);
   BinaryStreamReader R(*S);
   StringRef Str1;
   StringRef Str2;
@@ -218,8 +218,7 @@ TEST(MappedBlockStreamTest, UnalignedOve
 // still works correctly and allocates again from the shared pool.
 TEST(MappedBlockStreamTest, UnalignedOverlappingReadFail) {
   DiscontiguousStream F(BlocksAry, DataAry);
-  auto S = MappedBlockStream::createStream(F.block_size(), F.block_count(),
-                                           F.layout(), F);
+  auto S = MappedBlockStream::createStream(F.block_size(), F.layout(), F);
   BinaryStreamReader R(*S);
   StringRef Str1;
   StringRef Str2;
@@ -243,7 +242,7 @@ TEST(MappedBlockStreamTest, WriteBeyondE
 
   DiscontiguousStream F(BlocksAry, Data);
   auto S = WritableMappedBlockStream::createStream(
-      F.block_size(), F.block_count(), F.layout(), F);
+      F.block_size(), F.layout(), F);
   ArrayRef<uint8_t> Buffer;
 
   EXPECT_ERROR(S->writeBytes(0, ArrayRef<uint8_t>(LargeBuffer)));
@@ -256,7 +255,7 @@ TEST(MappedBlockStreamTest, TestWriteByt
   static uint8_t Data[] = {'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J'};
   DiscontiguousStream F(BlocksAry, Data);
   auto S = WritableMappedBlockStream::createStream(
-      F.block_size(), F.block_count(), F.layout(), F);
+      F.block_size(), F.layout(), F);
   ArrayRef<uint8_t> Buffer;
 
   EXPECT_NO_ERROR(S->readBytes(0, 1, Buffer));
@@ -289,7 +288,7 @@ TEST(MappedBlockStreamTest, TestWriteByt
 
   DiscontiguousStream F(BlocksAry, Data);
   auto S = WritableMappedBlockStream::createStream(
-      F.block_size(), F.block_count(), F.layout(), F);
+      F.block_size(), F.layout(), F);
   ArrayRef<uint8_t> Buffer;
 
   EXPECT_NO_ERROR(S->writeBytes(0, TestData));
@@ -308,7 +307,7 @@ TEST(MappedBlockStreamTest, TestWriteThe
 
   DiscontiguousStream F(Blocks, Data);
   auto S = WritableMappedBlockStream::createStream(
-      F.block_size(), F.block_count(), F.layout(), F);
+      F.block_size(), F.layout(), F);
 
   enum class MyEnum : uint32_t { Val1 = 2908234, Val2 = 120891234 };
   using support::ulittle32_t;
@@ -400,7 +399,7 @@ TEST(MappedBlockStreamTest, TestWriteCon
 
   DiscontiguousStream F(DestBlocks, DestData);
   auto DestStream = WritableMappedBlockStream::createStream(
-      F.block_size(), F.block_count(), F.layout(), F);
+      F.block_size(), F.layout(), F);
 
   // First write "Test Str" into the source stream.
   MutableBinaryByteStream SourceStream(SrcData, little);
@@ -435,9 +434,9 @@ TEST(MappedBlockStreamTest, TestWriteDis
   DiscontiguousStream SrcF(SrcBlocks, SrcData);
 
   auto Dest = WritableMappedBlockStream::createStream(
-      DestF.block_size(), DestF.block_count(), DestF.layout(), DestF);
+      DestF.block_size(), DestF.layout(), DestF);
   auto Src = WritableMappedBlockStream::createStream(
-      SrcF.block_size(), SrcF.block_count(), SrcF.layout(), SrcF);
+      SrcF.block_size(), SrcF.layout(), SrcF);
 
   // First write "Test Str" into the source stream.
   BinaryStreamWriter SourceWriter(*Src);




More information about the llvm-commits mailing list