From d2fd4aeb5611e496bd394d38495c007c0d73a5a1 Mon Sep 17 00:00:00 2001 From: Zachary Turner Date: Wed, 1 Mar 2017 19:29:11 +0000 Subject: [PATCH] [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. llvm-svn: 296661 --- .../llvm/DebugInfo/MSF/BinaryStreamArray.h | 1 + .../DebugInfo/PDB/BinaryStreamTest.cpp | 74 ++++++++++--------- 2 files changed, 39 insertions(+), 36 deletions(-) diff --git a/llvm/include/llvm/DebugInfo/MSF/BinaryStreamArray.h b/llvm/include/llvm/DebugInfo/MSF/BinaryStreamArray.h index efef5d15208e..5aecc0d52fa2 100644 --- a/llvm/include/llvm/DebugInfo/MSF/BinaryStreamArray.h +++ b/llvm/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(Data.data()); } diff --git a/llvm/unittests/DebugInfo/PDB/BinaryStreamTest.cpp b/llvm/unittests/DebugInfo/PDB/BinaryStreamTest.cpp index 845a9c4be7f4..c849efdf82ed 100644 --- a/llvm/unittests/DebugInfo/PDB/BinaryStreamTest.cpp +++ b/llvm/unittests/DebugInfo/PDB/BinaryStreamTest.cpp @@ -48,8 +48,10 @@ namespace { class DiscontiguousStream : public WritableBinaryStream { public: - DiscontiguousStream(MutableArrayRef Data, endianness Endian) - : Data(Data), PartitionIndex(Data.size() / 2), Endian(Endian) {} + DiscontiguousStream(MutableArrayRef 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 Output; }; - void initializeInput(ArrayRef Input) { + void initializeInput(ArrayRef 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(InputData, Endians[I]); - auto InBrokenStream = - llvm::make_unique(BrokenInputData, Endians[I]); + auto InBrokenStream = llvm::make_unique( + 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(OutputData, Endians[I]); - Streams[I * 2 + 1].Output = - llvm::make_unique(BrokenOutputData, Endians[I]); + Streams[I * 2 + 1].Output = llvm::make_unique( + 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(InputData, Endians[I]); - Streams[I * 2 + 1].Output = - llvm::make_unique(BrokenInputData, Endians[I]); + Streams[I * 2 + 1].Output = llvm::make_unique( + 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(OutputData, Endians[I]); - Streams[I * 2 + 1].Input = - llvm::make_unique(BrokenOutputData, Endians[I]); + Streams[I * 2 + 1].Input = llvm::make_unique( + 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 InputData = {1, 2, 3, 4, 5}; - initializeInput(InputData); + initializeInput(InputData, 1); for (auto &Stream : Streams) { ArrayRef Buffer; @@ -236,7 +238,7 @@ TEST_F(BinaryStreamTest, BinaryByteStreamBounds) { TEST_F(BinaryStreamTest, StreamRefBounds) { std::vector InputData = {1, 2, 3, 4, 5}; - initializeInput(InputData); + initializeInput(InputData, 1); for (const auto &Stream : Streams) { ArrayRef 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 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 IntBytes(reinterpret_cast(Ints.data()), Ints.size() * sizeof(uint32_t)); - initializeInput(IntBytes); + initializeInput(IntBytes, alignof(uint32_t)); for (auto &Stream : Streams) { MutableArrayRef Buffer; @@ -352,7 +354,7 @@ TEST_F(BinaryStreamTest, VarStreamArray) { "Extra Longest Test Of All"); ArrayRef StringBytes( reinterpret_cast(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 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 Ints = {1, 2, 3, 4, 5}; ArrayRef IntBytes(reinterpret_cast(&Ints[0]), Ints.size() * sizeof(int)); - initializeInput(IntBytes); + initializeInput(IntBytes, alignof(int)); for (auto &Stream : Streams) { BinaryStreamReader Reader(*Stream.Input); ArrayRef IntsRef; @@ -481,8 +483,8 @@ TEST_F(BinaryStreamTest, StreamReaderEnum) { std::vector 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(&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 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 SourceBytes(reinterpret_cast(&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);