From: Zachary Turner <zturner@google.com>
Date: Wed, 1 Mar 2017 19:29:11 +0000 (+0000)
Subject: [PDB] Fix and re-enable BinaryStreamArray test.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ed2fd5866d8388fa8c282395836f227c735fed27;p=llvm

[PDB] Fix and re-enable BinaryStreamArray test.

This was due to the test stream choosing an arbitrary partition
index for introducing the discontinuity rather than choosing
an index that would be correctly aligned for the type of data.

Also added an assertion into FixedStreamArray so that this will
be caught on all bots in the future, and not just the UBSan bot.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@296661 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/include/llvm/DebugInfo/MSF/BinaryStreamArray.h b/include/llvm/DebugInfo/MSF/BinaryStreamArray.h
index efef5d15208..5aecc0d52fa 100644
--- a/include/llvm/DebugInfo/MSF/BinaryStreamArray.h
+++ b/include/llvm/DebugInfo/MSF/BinaryStreamArray.h
@@ -243,6 +243,7 @@ public:
       // an exact multiple of the element size.
       consumeError(std::move(EC));
     }
+    assert(llvm::alignmentAdjustment(Data.data(), alignof(T)) == 0);
     return *reinterpret_cast<const T *>(Data.data());
   }
 
diff --git a/unittests/DebugInfo/PDB/BinaryStreamTest.cpp b/unittests/DebugInfo/PDB/BinaryStreamTest.cpp
index 845a9c4be7f..c849efdf82e 100644
--- a/unittests/DebugInfo/PDB/BinaryStreamTest.cpp
+++ b/unittests/DebugInfo/PDB/BinaryStreamTest.cpp
@@ -48,8 +48,10 @@ namespace {
 
 class DiscontiguousStream : public WritableBinaryStream {
 public:
-  DiscontiguousStream(MutableArrayRef<uint8_t> Data, endianness Endian)
-      : Data(Data), PartitionIndex(Data.size() / 2), Endian(Endian) {}
+  DiscontiguousStream(MutableArrayRef<uint8_t> Data, endianness Endian,
+                      uint32_t Align)
+      : Data(Data), PartitionIndex(alignDown(Data.size() / 2, Align)),
+        Endian(Endian) {}
 
   endianness getEndian() const override { return Endian; }
 
@@ -150,12 +152,12 @@ protected:
     std::unique_ptr<WritableBinaryStream> Output;
   };
 
-  void initializeInput(ArrayRef<uint8_t> Input) {
+  void initializeInput(ArrayRef<uint8_t> Input, uint32_t Align) {
     InputData = Input;
 
     BrokenInputData.resize(InputData.size());
     if (!Input.empty()) {
-      uint32_t PartitionIndex = InputData.size() / 2;
+      uint32_t PartitionIndex = alignDown(InputData.size() / 2, Align);
       uint32_t RightBytes = InputData.size() - PartitionIndex;
       uint32_t LeftBytes = PartitionIndex;
       if (RightBytes > 0)
@@ -167,41 +169,41 @@ protected:
     for (uint32_t I = 0; I < NumEndians; ++I) {
       auto InByteStream =
           llvm::make_unique<BinaryByteStream>(InputData, Endians[I]);
-      auto InBrokenStream =
-          llvm::make_unique<DiscontiguousStream>(BrokenInputData, Endians[I]);
+      auto InBrokenStream = llvm::make_unique<DiscontiguousStream>(
+          BrokenInputData, Endians[I], Align);
 
       Streams[I * 2].Input = std::move(InByteStream);
       Streams[I * 2 + 1].Input = std::move(InBrokenStream);
     }
   }
 
-  void initializeOutput(uint32_t Size) {
+  void initializeOutput(uint32_t Size, uint32_t Align) {
     OutputData.resize(Size);
     BrokenOutputData.resize(Size);
 
     for (uint32_t I = 0; I < NumEndians; ++I) {
       Streams[I * 2].Output =
           llvm::make_unique<MutableBinaryByteStream>(OutputData, Endians[I]);
-      Streams[I * 2 + 1].Output =
-          llvm::make_unique<DiscontiguousStream>(BrokenOutputData, Endians[I]);
+      Streams[I * 2 + 1].Output = llvm::make_unique<DiscontiguousStream>(
+          BrokenOutputData, Endians[I], Align);
     }
   }
 
-  void initializeOutputFromInput() {
+  void initializeOutputFromInput(uint32_t Align) {
     for (uint32_t I = 0; I < NumEndians; ++I) {
       Streams[I * 2].Output =
           llvm::make_unique<MutableBinaryByteStream>(InputData, Endians[I]);
-      Streams[I * 2 + 1].Output =
-          llvm::make_unique<DiscontiguousStream>(BrokenInputData, Endians[I]);
+      Streams[I * 2 + 1].Output = llvm::make_unique<DiscontiguousStream>(
+          BrokenInputData, Endians[I], Align);
     }
   }
 
-  void initializeInputFromOutput() {
+  void initializeInputFromOutput(uint32_t Align) {
     for (uint32_t I = 0; I < NumEndians; ++I) {
       Streams[I * 2].Input =
           llvm::make_unique<BinaryByteStream>(OutputData, Endians[I]);
-      Streams[I * 2 + 1].Input =
-          llvm::make_unique<DiscontiguousStream>(BrokenOutputData, Endians[I]);
+      Streams[I * 2 + 1].Input = llvm::make_unique<DiscontiguousStream>(
+          BrokenOutputData, Endians[I], Align);
     }
   }
 
@@ -217,7 +219,7 @@ protected:
 // Tests that a we can read from a BinaryByteStream without a StreamReader.
 TEST_F(BinaryStreamTest, BinaryByteStreamBounds) {
   std::vector<uint8_t> InputData = {1, 2, 3, 4, 5};
-  initializeInput(InputData);
+  initializeInput(InputData, 1);
 
   for (auto &Stream : Streams) {
     ArrayRef<uint8_t> Buffer;
@@ -236,7 +238,7 @@ TEST_F(BinaryStreamTest, BinaryByteStreamBounds) {
 
 TEST_F(BinaryStreamTest, StreamRefBounds) {
   std::vector<uint8_t> InputData = {1, 2, 3, 4, 5};
-  initializeInput(InputData);
+  initializeInput(InputData, 1);
 
   for (const auto &Stream : Streams) {
     ArrayRef<uint8_t> Buffer;
@@ -289,8 +291,8 @@ TEST_F(BinaryStreamTest, StreamRefBounds) {
 // Test that we can write to a BinaryStream without a StreamWriter.
 TEST_F(BinaryStreamTest, MutableBinaryByteStreamBounds) {
   std::vector<uint8_t> InputData = {'T', 'e', 's', 't', '\0'};
-  initializeInput(InputData);
-  initializeOutput(InputData.size());
+  initializeInput(InputData, 1);
+  initializeOutput(InputData.size(), 1);
 
   // For every combination of input stream and output stream.
   for (auto &Stream : Streams) {
@@ -330,7 +332,7 @@ TEST_F(BinaryStreamTest, FixedStreamArray) {
   ArrayRef<uint8_t> IntBytes(reinterpret_cast<uint8_t *>(Ints.data()),
                              Ints.size() * sizeof(uint32_t));
 
-  initializeInput(IntBytes);
+  initializeInput(IntBytes, alignof(uint32_t));
 
   for (auto &Stream : Streams) {
     MutableArrayRef<uint8_t> Buffer;
@@ -352,7 +354,7 @@ TEST_F(BinaryStreamTest, VarStreamArray) {
                         "Extra Longest Test Of All");
   ArrayRef<uint8_t> StringBytes(
       reinterpret_cast<const uint8_t *>(Strings.data()), Strings.size());
-  initializeInput(StringBytes);
+  initializeInput(StringBytes, 1);
 
   struct StringExtractor {
   public:
@@ -392,7 +394,7 @@ TEST_F(BinaryStreamTest, VarStreamArray) {
 TEST_F(BinaryStreamTest, StreamReaderBounds) {
   std::vector<uint8_t> Bytes;
 
-  initializeInput(Bytes);
+  initializeInput(Bytes, 1);
   for (auto &Stream : Streams) {
     StringRef S;
     BinaryStreamReader Reader(*Stream.Input);
@@ -401,7 +403,7 @@ TEST_F(BinaryStreamTest, StreamReaderBounds) {
   }
 
   Bytes.resize(5);
-  initializeInput(Bytes);
+  initializeInput(Bytes, 1);
   for (auto &Stream : Streams) {
     StringRef S;
     BinaryStreamReader Reader(*Stream.Input);
@@ -420,8 +422,8 @@ TEST_F(BinaryStreamTest, StreamReaderIntegers) {
   constexpr uint32_t Size =
       sizeof(Little) + sizeof(Big) + sizeof(NS) + sizeof(NI) + sizeof(NUL);
 
-  initializeOutput(Size);
-  initializeInputFromOutput();
+  initializeOutput(Size, alignof(support::ulittle64_t));
+  initializeInputFromOutput(alignof(support::ulittle64_t));
 
   for (auto &Stream : Streams) {
     BinaryStreamWriter Writer(*Stream.Output);
@@ -454,13 +456,13 @@ TEST_F(BinaryStreamTest, StreamReaderIntegers) {
   }
 }
 
-TEST_F(BinaryStreamTest, DISABLED_StreamReaderIntegerArray) {
+TEST_F(BinaryStreamTest, StreamReaderIntegerArray) {
   // 1. Arrays of integers
   std::vector<int> Ints = {1, 2, 3, 4, 5};
   ArrayRef<uint8_t> IntBytes(reinterpret_cast<uint8_t *>(&Ints[0]),
                              Ints.size() * sizeof(int));
 
-  initializeInput(IntBytes);
+  initializeInput(IntBytes, alignof(int));
   for (auto &Stream : Streams) {
     BinaryStreamReader Reader(*Stream.Input);
     ArrayRef<int> IntsRef;
@@ -481,8 +483,8 @@ TEST_F(BinaryStreamTest, StreamReaderEnum) {
 
   std::vector<MyEnum> Enums = {MyEnum::Bar, MyEnum::Baz, MyEnum::Foo};
 
-  initializeOutput(Enums.size() * sizeof(MyEnum));
-  initializeInputFromOutput();
+  initializeOutput(Enums.size() * sizeof(MyEnum), alignof(MyEnum));
+  initializeInputFromOutput(alignof(MyEnum));
   for (auto &Stream : Streams) {
     BinaryStreamWriter Writer(*Stream.Output);
     for (auto Value : Enums)
@@ -515,7 +517,7 @@ TEST_F(BinaryStreamTest, StreamReaderObject) {
 
   const uint8_t *Bytes = reinterpret_cast<const uint8_t *>(&Foos[0]);
 
-  initializeInput(makeArrayRef(Bytes, 2 * sizeof(Foo)));
+  initializeInput(makeArrayRef(Bytes, 2 * sizeof(Foo)), alignof(Foo));
 
   for (auto &Stream : Streams) {
     // 1. Reading object pointers.
@@ -534,7 +536,7 @@ TEST_F(BinaryStreamTest, StreamReaderStrings) {
   std::vector<uint8_t> Bytes = {'O',  'n', 'e', '\0', 'T', 'w', 'o',
                                 '\0', 'T', 'h', 'r',  'e', 'e', '\0',
                                 'F',  'o', 'u', 'r',  '\0'};
-  initializeInput(Bytes);
+  initializeInput(Bytes, 1);
 
   for (auto &Stream : Streams) {
     BinaryStreamReader Reader(*Stream.Input);
@@ -574,7 +576,7 @@ TEST_F(BinaryStreamTest, StreamReaderStrings) {
 }
 
 TEST_F(BinaryStreamTest, StreamWriterBounds) {
-  initializeOutput(5);
+  initializeOutput(5, 1);
 
   for (auto &Stream : Streams) {
     BinaryStreamWriter Writer(*Stream.Output);
@@ -600,8 +602,8 @@ TEST_F(BinaryStreamTest, StreamWriterIntegerArrays) {
   ArrayRef<uint8_t> SourceBytes(reinterpret_cast<uint8_t *>(&SourceInts[0]),
                                 SourceInts.size() * sizeof(int));
 
-  initializeInput(SourceBytes);
-  initializeOutputFromInput();
+  initializeInput(SourceBytes, alignof(int));
+  initializeOutputFromInput(alignof(int));
 
   for (auto &Stream : Streams) {
     BinaryStreamReader Reader(*Stream.Input);
@@ -625,8 +627,8 @@ TEST_F(BinaryStreamTest, StringWriterStrings) {
   size_t Length = 0;
   for (auto S : Strings)
     Length += S.size() + 1;
-  initializeOutput(Length);
-  initializeInputFromOutput();
+  initializeOutput(Length, 1);
+  initializeInputFromOutput(1);
 
   for (auto &Stream : Streams) {
     BinaryStreamWriter Writer(*Stream.Output);