From 00d9a548e882278f6fcbfda638d97dd4bad55e9f Mon Sep 17 00:00:00 2001
From: Zachary Turner <zturner@google.com>
Date: Wed, 1 Mar 2017 01:04:16 +0000
Subject: [PATCH] [PDB] Add an additional test for BinaryStreamRef.

A bug was uncovered where if you have a StreamRef whose ViewOffset
is > 0, then when you call readLongestContiguousChunk it will
succeed even when it shouldn't, and it always return you a
buffer that was taken as if the ViewOffset was 0.

Fixed this bug and added a test for it.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@296556 91177308-0d34-0410-b5e6-96231b3b80d8
---
 include/llvm/DebugInfo/MSF/BinaryStreamRef.h |  3 +-
 unittests/DebugInfo/PDB/BinaryStreamTest.cpp | 60 +++++++++++++++++++-
 2 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/include/llvm/DebugInfo/MSF/BinaryStreamRef.h b/include/llvm/DebugInfo/MSF/BinaryStreamRef.h
index 96f937b30f9..b9c1923a85e 100644
--- a/include/llvm/DebugInfo/MSF/BinaryStreamRef.h
+++ b/include/llvm/DebugInfo/MSF/BinaryStreamRef.h
@@ -122,7 +122,8 @@ public:
     if (auto EC = checkOffset(Offset, 1))
       return EC;
 
-    if (auto EC = Stream->readLongestContiguousChunk(Offset, Buffer))
+    if (auto EC =
+            Stream->readLongestContiguousChunk(ViewOffset + Offset, Buffer))
       return EC;
     // This StreamRef might refer to a smaller window over a larger stream.  In
     // that case we will have read out more bytes than we should return, because
diff --git a/unittests/DebugInfo/PDB/BinaryStreamTest.cpp b/unittests/DebugInfo/PDB/BinaryStreamTest.cpp
index 04561987506..dbb3eb54484 100644
--- a/unittests/DebugInfo/PDB/BinaryStreamTest.cpp
+++ b/unittests/DebugInfo/PDB/BinaryStreamTest.cpp
@@ -135,12 +135,16 @@ public:
   void SetUp() override {
     Streams.clear();
     Streams.resize(NumStreams);
+    for (int I = 0; I < NumStreams; ++I)
+      Streams[I].IsContiguous = (I % 2 == 0);
+
     InputData.clear();
     OutputData.clear();
   }
 
 protected:
   struct StreamPair {
+    bool IsContiguous;
     std::unique_ptr<BinaryStream> Input;
     std::unique_ptr<WritableBinaryStream> Output;
   };
@@ -210,7 +214,7 @@ protected:
 };
 
 // Tests that a we can read from a BinaryByteStream without a StreamReader.
-TEST_F(BinaryStreamTest, BinaryByteStreamProperties) {
+TEST_F(BinaryStreamTest, BinaryByteStreamBounds) {
   std::vector<uint8_t> InputData = {1, 2, 3, 4, 5};
   initializeInput(InputData);
 
@@ -229,8 +233,60 @@ TEST_F(BinaryStreamTest, BinaryByteStreamProperties) {
   }
 }
 
+TEST_F(BinaryStreamTest, StreamRefBounds) {
+  std::vector<uint8_t> InputData = {1, 2, 3, 4, 5};
+  initializeInput(InputData);
+
+  for (const auto &Stream : Streams) {
+    ArrayRef<uint8_t> Buffer;
+    BinaryStreamRef Ref(*Stream.Input);
+
+    // Read 1 byte from offset 2 should work
+    ASSERT_EQ(InputData.size(), Ref.getLength());
+    ASSERT_NO_ERROR(Ref.readBytes(2, 1, Buffer));
+    EXPECT_EQ(makeArrayRef(InputData).slice(2, 1), Buffer);
+
+    // Reading everything from offset 2 on.
+    ASSERT_NO_ERROR(Ref.readLongestContiguousChunk(2, Buffer));
+    if (Stream.IsContiguous)
+      EXPECT_EQ(makeArrayRef(InputData).slice(2), Buffer);
+    else
+      EXPECT_FALSE(Buffer.empty());
+
+    // Reading 6 bytes from offset 0 is too big.
+    EXPECT_ERROR(Ref.readBytes(0, 6, Buffer));
+    EXPECT_ERROR(Ref.readLongestContiguousChunk(6, Buffer));
+
+    // Reading 1 byte from offset 2 after dropping 1 byte is the same as reading
+    // 1 byte from offset 3.
+    Ref = Ref.drop_front(1);
+    ASSERT_NO_ERROR(Ref.readBytes(2, 1, Buffer));
+    if (Stream.IsContiguous)
+      EXPECT_EQ(makeArrayRef(InputData).slice(3, 1), Buffer);
+    else
+      EXPECT_FALSE(Buffer.empty());
+
+    // Reading everything from offset 2 on after dropping 1 byte.
+    ASSERT_NO_ERROR(Ref.readLongestContiguousChunk(2, Buffer));
+    if (Stream.IsContiguous)
+      EXPECT_EQ(makeArrayRef(InputData).slice(3), Buffer);
+    else
+      EXPECT_FALSE(Buffer.empty());
+
+    // Reading 2 bytes from offset 2 after dropping 2 bytes is the same as
+    // reading 2 bytes from offset 4, and should fail.
+    Ref = Ref.drop_front(1);
+    EXPECT_ERROR(Ref.readBytes(2, 2, Buffer));
+
+    // But if we read the longest contiguous chunk instead, we should still
+    // get the 1 byte at the end.
+    ASSERT_NO_ERROR(Ref.readLongestContiguousChunk(2, Buffer));
+    EXPECT_EQ(makeArrayRef(InputData).take_back(), Buffer);
+  }
+}
+
 // Test that we can write to a BinaryStream without a StreamWriter.
-TEST_F(BinaryStreamTest, MutableBinaryByteStreamProperties) {
+TEST_F(BinaryStreamTest, MutableBinaryByteStreamBounds) {
   std::vector<uint8_t> InputData = {'T', 'e', 's', 't', '\0'};
   initializeInput(InputData);
   initializeOutput(InputData.size());