From: Zachary Turner Date: Thu, 25 May 2017 21:12:00 +0000 (+0000) Subject: Fix a bug in MappedBlockStream. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=63d6d7548d9edf129ea79cc6a04fb601510c7684;p=llvm 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. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@303916 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/DebugInfo/MSF/MappedBlockStream.h b/include/llvm/DebugInfo/MSF/MappedBlockStream.h index c91f6f725c8..d68f5f70c83 100644 --- a/include/llvm/DebugInfo/MSF/MappedBlockStream.h +++ b/include/llvm/DebugInfo/MSF/MappedBlockStream.h @@ -43,8 +43,8 @@ class MappedBlockStream : public BinaryStream { friend class WritableMappedBlockStream; public: static std::unique_ptr - 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 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 &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 - 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 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); diff --git a/lib/DebugInfo/MSF/MappedBlockStream.cpp b/lib/DebugInfo/MSF/MappedBlockStream.cpp index 57953cfa338..dfdeb841421 100644 --- a/lib/DebugInfo/MSF/MappedBlockStream.cpp +++ b/lib/DebugInfo/MSF/MappedBlockStream.cpp @@ -45,18 +45,17 @@ static Interval intersect(const Interval &I1, const Interval &I2) { 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::createStream(uint32_t BlockSize, uint32_t NumBlocks, +MappedBlockStream::createStream(uint32_t BlockSize, const MSFStreamLayout &Layout, BinaryStreamRef MsfData) { return llvm::make_unique>( - BlockSize, NumBlocks, Layout, MsfData); + BlockSize, Layout, MsfData); } std::unique_ptr MappedBlockStream::createIndexedStream( @@ -66,7 +65,7 @@ std::unique_ptr MappedBlockStream::createIndexedStream( SL.Blocks = Layout.StreamMap[StreamIndex]; SL.Length = Layout.StreamSizes[StreamIndex]; return llvm::make_unique>( - Layout.SB->BlockSize, Layout.SB->NumBlocks, SL, MsfData); + Layout.SB->BlockSize, SL, MsfData); } std::unique_ptr @@ -75,7 +74,7 @@ MappedBlockStream::createDirectoryStream(const MSFLayout &Layout, 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 @@ -83,7 +82,7 @@ MappedBlockStream::createFpmStream(const MSFLayout &Layout, 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::readLongestContiguousChunk(uint32_t Offset, 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::fixCacheAfterWrite(uint32_t Offset, } 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::createStream(uint32_t BlockSize, uint32_t NumBlocks, +WritableMappedBlockStream::createStream(uint32_t BlockSize, const MSFStreamLayout &Layout, WritableBinaryStreamRef MsfData) { return llvm::make_unique>( - BlockSize, NumBlocks, Layout, MsfData); + BlockSize, Layout, MsfData); } std::unique_ptr @@ -334,7 +332,7 @@ WritableMappedBlockStream::createIndexedStream(const MSFLayout &Layout, 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 @@ -343,7 +341,7 @@ WritableMappedBlockStream::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 @@ -351,7 +349,7 @@ WritableMappedBlockStream::createFpmStream(const MSFLayout &Layout, 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, diff --git a/unittests/DebugInfo/PDB/MappedBlockStreamTest.cpp b/unittests/DebugInfo/PDB/MappedBlockStreamTest.cpp index 9f8940b77f2..9d90e265df3 100644 --- a/unittests/DebugInfo/PDB/MappedBlockStreamTest.cpp +++ b/unittests/DebugInfo/PDB/MappedBlockStreamTest.cpp @@ -75,12 +75,19 @@ private: MutableArrayRef 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, ReadBeyondEndOfStreamRef) { // 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, ReadOntoNonEmptyBuffer) { // 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, ZeroCopyReadContiguousBreak) { // 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, CopyReadNonContiguousBreak) { // 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, InvalidReadSizeNoBreak) { // 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, InvalidReadSizeContiguousBreak) { // 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, InvalidReadSizeNonContiguousBreak) { // 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, ZeroCopyReadNoBreak) { // 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, UnalignedOverlappingRead) { // 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, WriteBeyondEndOfStream) { DiscontiguousStream F(BlocksAry, Data); auto S = WritableMappedBlockStream::createStream( - F.block_size(), F.block_count(), F.layout(), F); + F.block_size(), F.layout(), F); ArrayRef Buffer; EXPECT_ERROR(S->writeBytes(0, ArrayRef(LargeBuffer))); @@ -256,7 +255,7 @@ TEST(MappedBlockStreamTest, TestWriteBytesNoBreakBoundary) { 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 Buffer; EXPECT_NO_ERROR(S->readBytes(0, 1, Buffer)); @@ -289,7 +288,7 @@ TEST(MappedBlockStreamTest, TestWriteBytesBreakBoundary) { DiscontiguousStream F(BlocksAry, Data); auto S = WritableMappedBlockStream::createStream( - F.block_size(), F.block_count(), F.layout(), F); + F.block_size(), F.layout(), F); ArrayRef Buffer; EXPECT_NO_ERROR(S->writeBytes(0, TestData)); @@ -308,7 +307,7 @@ TEST(MappedBlockStreamTest, TestWriteThenRead) { 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, TestWriteContiguousStreamRef) { 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, TestWriteDiscontiguousStreamRef) { 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);