From 54a7acb455ea8de25bddb61d79f716721fc1a074 Mon Sep 17 00:00:00 2001 From: Zachary Turner Date: Tue, 28 Feb 2017 17:49:34 +0000 Subject: [PATCH] [PDB] Add BinaryStreamError. This migrates the stream code away from MSFError to using its own custom Error class. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@296494 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/DebugInfo/MSF/BinaryByteStream.h | 27 +++------ include/llvm/DebugInfo/MSF/BinaryItemStream.h | 10 ++-- include/llvm/DebugInfo/MSF/BinaryStream.h | 10 ++++ .../llvm/DebugInfo/MSF/BinaryStreamError.h | 47 +++++++++++++++ .../llvm/DebugInfo/MSF/BinaryStreamReader.h | 21 +++---- include/llvm/DebugInfo/MSF/BinaryStreamRef.h | 30 +++++----- .../llvm/DebugInfo/MSF/BinaryStreamWriter.h | 6 +- lib/DebugInfo/MSF/BinaryStreamError.cpp | 58 +++++++++++++++++++ lib/DebugInfo/MSF/BinaryStreamReader.cpp | 7 +-- lib/DebugInfo/MSF/CMakeLists.txt | 1 + lib/DebugInfo/MSF/MappedBlockStream.cpp | 26 ++++----- lib/DebugInfo/PDB/Native/PDBFile.cpp | 9 ++- .../DebugInfo/PDB/MappedBlockStreamTest.cpp | 12 ++-- 13 files changed, 184 insertions(+), 80 deletions(-) create mode 100644 include/llvm/DebugInfo/MSF/BinaryStreamError.h create mode 100644 lib/DebugInfo/MSF/BinaryStreamError.cpp diff --git a/include/llvm/DebugInfo/MSF/BinaryByteStream.h b/include/llvm/DebugInfo/MSF/BinaryByteStream.h index caa1d7c295f..57e7598473e 100644 --- a/include/llvm/DebugInfo/MSF/BinaryByteStream.h +++ b/include/llvm/DebugInfo/MSF/BinaryByteStream.h @@ -14,7 +14,7 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/StringRef.h" #include "llvm/DebugInfo/MSF/BinaryStream.h" -#include "llvm/DebugInfo/MSF/MSFError.h" +#include "llvm/DebugInfo/MSF/BinaryStreamError.h" #include "llvm/Support/Error.h" #include "llvm/Support/FileOutputBuffer.h" #include "llvm/Support/MemoryBuffer.h" @@ -41,21 +41,16 @@ public: Error readBytes(uint32_t Offset, uint32_t Size, ArrayRef &Buffer) override { - if (Offset > Data.size()) - return make_error( - msf::msf_error_code::insufficient_buffer); - if (Data.size() < Size + Offset) - return make_error( - msf::msf_error_code::insufficient_buffer); + if (auto EC = checkOffset(Offset, Size)) + return EC; Buffer = Data.slice(Offset, Size); return Error::success(); } Error readLongestContiguousChunk(uint32_t Offset, ArrayRef &Buffer) override { - if (Offset >= Data.size()) - return make_error( - msf::msf_error_code::insufficient_buffer); + if (auto EC = checkOffset(Offset, 1)) + return EC; Buffer = Data.slice(Offset); return Error::success(); } @@ -119,12 +114,8 @@ public: if (Buffer.empty()) return Error::success(); - if (Data.size() < Buffer.size()) - return make_error( - msf::msf_error_code::insufficient_buffer); - if (Offset > Buffer.size() - Data.size()) - return make_error( - msf::msf_error_code::insufficient_buffer); + if (auto EC = checkOffset(Offset, Buffer.size())) + return EC; uint8_t *DataPtr = const_cast(Data.data()); ::memcpy(DataPtr + Offset, Buffer.data(), Buffer.size()); @@ -156,8 +147,8 @@ private: Error commit() override { if (FileBuffer->commit()) - return make_error( - msf::msf_error_code::insufficient_buffer); + return make_error( + stream_error_code::filesystem_error); return Error::success(); } diff --git a/include/llvm/DebugInfo/MSF/BinaryItemStream.h b/include/llvm/DebugInfo/MSF/BinaryItemStream.h index 5d4ed5b5118..50dadaa7bff 100644 --- a/include/llvm/DebugInfo/MSF/BinaryItemStream.h +++ b/include/llvm/DebugInfo/MSF/BinaryItemStream.h @@ -12,7 +12,7 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/DebugInfo/MSF/BinaryStream.h" -#include "llvm/DebugInfo/MSF/MSFError.h" +#include "llvm/DebugInfo/MSF/BinaryStreamError.h" #include "llvm/Support/Error.h" #include #include @@ -45,9 +45,10 @@ public: if (!ExpectedIndex) return ExpectedIndex.takeError(); const auto &Item = Items[*ExpectedIndex]; + if (auto EC = checkOffset(Offset, Size)) + return EC; if (Size > Traits::length(Item)) - return make_error( - msf::msf_error_code::insufficient_buffer); + return make_error(stream_error_code::stream_too_short); Buffer = Traits::bytes(Item).take_front(Size); return Error::success(); } @@ -81,8 +82,7 @@ private: ++CurrentIndex; } if (CurrentOffset != Offset) - return make_error( - msf::msf_error_code::insufficient_buffer); + return make_error(stream_error_code::stream_too_short); return CurrentIndex; } diff --git a/include/llvm/DebugInfo/MSF/BinaryStream.h b/include/llvm/DebugInfo/MSF/BinaryStream.h index 5a9ca34367b..74b48808c0d 100644 --- a/include/llvm/DebugInfo/MSF/BinaryStream.h +++ b/include/llvm/DebugInfo/MSF/BinaryStream.h @@ -11,6 +11,7 @@ #define LLVM_DEBUGINFO_MSF_BINARYSTREAM_H #include "llvm/ADT/ArrayRef.h" +#include "llvm/DebugInfo/MSF/BinaryStreamError.h" #include "llvm/Support/Endian.h" #include "llvm/Support/Error.h" #include @@ -43,6 +44,15 @@ public: /// \brief Return the number of bytes of data in this stream. virtual uint32_t getLength() = 0; + +protected: + Error checkOffset(uint32_t Offset, uint32_t DataSize) { + if (Offset > getLength()) + return make_error(stream_error_code::invalid_offset); + if (getLength() < DataSize + Offset) + return make_error(stream_error_code::stream_too_short); + return Error::success(); + } }; /// \brief A BinaryStream which can be read from as well as written to. Note diff --git a/include/llvm/DebugInfo/MSF/BinaryStreamError.h b/include/llvm/DebugInfo/MSF/BinaryStreamError.h new file mode 100644 index 00000000000..51959505609 --- /dev/null +++ b/include/llvm/DebugInfo/MSF/BinaryStreamError.h @@ -0,0 +1,47 @@ +//===- BinaryStreamError.h - Error extensions for Binary Streams *- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_DEBUGINFO_MSF_BINARYSTREAMERROR_H +#define LLVM_DEBUGINFO_MSF_BINARYSTREAMERROR_H + +#include "llvm/Support/Error.h" + +#include + +namespace llvm { +enum class stream_error_code { + unspecified, + stream_too_short, + invalid_array_size, + invalid_offset, + filesystem_error +}; + +/// Base class for errors originating when parsing raw PDB files +class BinaryStreamError : public ErrorInfo { +public: + static char ID; + explicit BinaryStreamError(stream_error_code C); + explicit BinaryStreamError(StringRef Context); + BinaryStreamError(stream_error_code C, StringRef Context); + + void log(raw_ostream &OS) const override; + std::error_code convertToErrorCode() const override; + + StringRef getErrorMessage() const; + + stream_error_code getErrorCode() const { return Code; } + +private: + std::string ErrMsg; + stream_error_code Code; +}; +} // namespace llvm + +#endif // LLVM_DEBUGINFO_MSF_BINARYSTREAMERROR_H diff --git a/include/llvm/DebugInfo/MSF/BinaryStreamReader.h b/include/llvm/DebugInfo/MSF/BinaryStreamReader.h index 507fa5b5689..1d3cf189ff1 100644 --- a/include/llvm/DebugInfo/MSF/BinaryStreamReader.h +++ b/include/llvm/DebugInfo/MSF/BinaryStreamReader.h @@ -150,7 +150,8 @@ public: } if (NumElements > UINT32_MAX / sizeof(T)) - return make_error(msf::msf_error_code::insufficient_buffer); + return make_error( + stream_error_code::invalid_array_size); if (auto EC = readBytes(Bytes, NumElements * sizeof(T))) return EC; @@ -193,16 +194,16 @@ public: Array = FixedStreamArray(); return Error::success(); } - uint32_t Length = NumItems * sizeof(T); - if (Length / sizeof(T) != NumItems) - return errorCodeToError( - make_error_code(std::errc::illegal_byte_sequence)); - if (Offset + Length > Stream.getLength()) - return make_error( - msf::msf_error_code::insufficient_buffer); - BinaryStreamRef View = Stream.slice(Offset, Length); + + if (NumItems > UINT32_MAX / sizeof(T)) + return make_error( + stream_error_code::invalid_array_size); + + BinaryStreamRef View; + if (auto EC = readStreamRef(View, NumItems * sizeof(T))) + return EC; + Array = FixedStreamArray(View); - Offset += Length; return Error::success(); } diff --git a/include/llvm/DebugInfo/MSF/BinaryStreamRef.h b/include/llvm/DebugInfo/MSF/BinaryStreamRef.h index 7cbe4e3292f..96f937b30f9 100644 --- a/include/llvm/DebugInfo/MSF/BinaryStreamRef.h +++ b/include/llvm/DebugInfo/MSF/BinaryStreamRef.h @@ -12,7 +12,7 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/DebugInfo/MSF/BinaryStream.h" -#include "llvm/DebugInfo/MSF/MSFError.h" +#include "llvm/DebugInfo/MSF/BinaryStreamError.h" #include "llvm/Support/Error.h" #include #include @@ -65,6 +65,14 @@ public: } protected: + Error checkOffset(uint32_t Offset, uint32_t DataSize) const { + if (Offset > getLength()) + return make_error(stream_error_code::invalid_offset); + if (getLength() < DataSize + Offset) + return make_error(stream_error_code::stream_too_short); + return Error::success(); + } + StreamType *Stream; uint32_t ViewOffset; uint32_t Length; @@ -98,12 +106,9 @@ public: /// the data, and an appropriate error code otherwise. Error readBytes(uint32_t Offset, uint32_t Size, ArrayRef &Buffer) const { - if (ViewOffset + Offset < Offset) - return make_error( - msf::msf_error_code::insufficient_buffer); - if (Size + Offset > Length) - return make_error( - msf::msf_error_code::insufficient_buffer); + if (auto EC = checkOffset(Offset, Size)) + return EC; + return Stream->readBytes(ViewOffset + Offset, Size, Buffer); } @@ -114,9 +119,8 @@ public: /// and an appropriate error code otherwise. Error readLongestContiguousChunk(uint32_t Offset, ArrayRef &Buffer) const { - if (Offset >= Length) - return make_error( - msf::msf_error_code::insufficient_buffer); + if (auto EC = checkOffset(Offset, 1)) + return EC; if (auto EC = Stream->readLongestContiguousChunk(Offset, Buffer)) return EC; @@ -152,9 +156,9 @@ public: /// stream at the specified location and the implementation could write the /// data, and an appropriate error code otherwise. Error writeBytes(uint32_t Offset, ArrayRef Data) const { - if (Data.size() + Offset > Length) - return make_error( - msf::msf_error_code::insufficient_buffer); + if (auto EC = checkOffset(Offset, Data.size())) + return EC; + return Stream->writeBytes(ViewOffset + Offset, Data); } diff --git a/include/llvm/DebugInfo/MSF/BinaryStreamWriter.h b/include/llvm/DebugInfo/MSF/BinaryStreamWriter.h index cd4660ef653..1c0da63745c 100644 --- a/include/llvm/DebugInfo/MSF/BinaryStreamWriter.h +++ b/include/llvm/DebugInfo/MSF/BinaryStreamWriter.h @@ -14,6 +14,7 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/DebugInfo/MSF/BinaryStreamArray.h" +#include "llvm/DebugInfo/MSF/BinaryStreamError.h" #include "llvm/DebugInfo/MSF/BinaryStreamRef.h" #include "llvm/Support/Endian.h" #include "llvm/Support/Error.h" @@ -123,10 +124,9 @@ public: template Error writeArray(ArrayRef Array) { if (Array.empty()) return Error::success(); - if (Array.size() > UINT32_MAX / sizeof(T)) - return make_error( - msf::msf_error_code::insufficient_buffer); + return make_error( + stream_error_code::invalid_array_size); return writeBytes( ArrayRef(reinterpret_cast(Array.data()), diff --git a/lib/DebugInfo/MSF/BinaryStreamError.cpp b/lib/DebugInfo/MSF/BinaryStreamError.cpp new file mode 100644 index 00000000000..2f299dc5b5e --- /dev/null +++ b/lib/DebugInfo/MSF/BinaryStreamError.cpp @@ -0,0 +1,58 @@ +//===- BinaryStreamError.cpp - Error extensions for streams -----*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "llvm/DebugInfo/MSF/BinaryStreamError.h" +#include "llvm/Support/ErrorHandling.h" + +using namespace llvm; + +char BinaryStreamError::ID = 0; + +BinaryStreamError::BinaryStreamError(stream_error_code C) + : BinaryStreamError(C, "") {} + +BinaryStreamError::BinaryStreamError(StringRef Context) + : BinaryStreamError(stream_error_code::unspecified, Context) {} + +BinaryStreamError::BinaryStreamError(stream_error_code C, StringRef Context) + : Code(C) { + ErrMsg = "Stream Error: "; + switch (C) { + case stream_error_code::unspecified: + ErrMsg += "An unspecified error has occurred."; + break; + case stream_error_code::stream_too_short: + ErrMsg += "The stream is too short to perform the requested operation."; + break; + case stream_error_code::invalid_array_size: + ErrMsg += "The buffer size is not a multiple of the array element size."; + break; + case stream_error_code::invalid_offset: + ErrMsg += "The specified offset is invalid for the current stream."; + break; + case stream_error_code::filesystem_error: + ErrMsg += "An I/O error occurred on the file system."; + break; + default: + llvm_unreachable("Unreachable!"); + } + + if (!Context.empty()) { + ErrMsg += " "; + ErrMsg += Context; + } +} + +void BinaryStreamError::log(raw_ostream &OS) const { OS << ErrMsg << "\n"; } + +StringRef BinaryStreamError::getErrorMessage() const { return ErrMsg; } + +std::error_code BinaryStreamError::convertToErrorCode() const { + return inconvertibleErrorCode(); +} diff --git a/lib/DebugInfo/MSF/BinaryStreamReader.cpp b/lib/DebugInfo/MSF/BinaryStreamReader.cpp index 170cc4cbeef..b6d7ee9d0b0 100644 --- a/lib/DebugInfo/MSF/BinaryStreamReader.cpp +++ b/lib/DebugInfo/MSF/BinaryStreamReader.cpp @@ -9,11 +9,10 @@ #include "llvm/DebugInfo/MSF/BinaryStreamReader.h" +#include "llvm/DebugInfo/MSF/BinaryStreamError.h" #include "llvm/DebugInfo/MSF/BinaryStreamRef.h" -#include "llvm/DebugInfo/MSF/MSFError.h" using namespace llvm; -using namespace llvm::msf; BinaryStreamReader::BinaryStreamReader(BinaryStreamRef S) : Stream(S), Offset(0) {} @@ -74,7 +73,7 @@ Error BinaryStreamReader::readStreamRef(BinaryStreamRef &Ref) { Error BinaryStreamReader::readStreamRef(BinaryStreamRef &Ref, uint32_t Length) { if (bytesRemaining() < Length) - return make_error(msf_error_code::insufficient_buffer); + return make_error(stream_error_code::stream_too_short); Ref = Stream.slice(Offset, Length); Offset += Length; return Error::success(); @@ -82,7 +81,7 @@ Error BinaryStreamReader::readStreamRef(BinaryStreamRef &Ref, uint32_t Length) { Error BinaryStreamReader::skip(uint32_t Amount) { if (Amount > bytesRemaining()) - return make_error(msf_error_code::insufficient_buffer); + return make_error(stream_error_code::stream_too_short); Offset += Amount; return Error::success(); } diff --git a/lib/DebugInfo/MSF/CMakeLists.txt b/lib/DebugInfo/MSF/CMakeLists.txt index 6938513fcde..5537b7008ce 100644 --- a/lib/DebugInfo/MSF/CMakeLists.txt +++ b/lib/DebugInfo/MSF/CMakeLists.txt @@ -1,4 +1,5 @@ add_llvm_library(LLVMDebugInfoMSF + BinaryStreamError.cpp BinaryStreamReader.cpp BinaryStreamWriter.cpp MappedBlockStream.cpp diff --git a/lib/DebugInfo/MSF/MappedBlockStream.cpp b/lib/DebugInfo/MSF/MappedBlockStream.cpp index c9ba25c0d7d..242088c9391 100644 --- a/lib/DebugInfo/MSF/MappedBlockStream.cpp +++ b/lib/DebugInfo/MSF/MappedBlockStream.cpp @@ -9,9 +9,9 @@ #include "llvm/DebugInfo/MSF/MappedBlockStream.h" +#include "llvm/DebugInfo/MSF/BinaryStreamError.h" #include "llvm/DebugInfo/MSF/IMSFFile.h" #include "llvm/DebugInfo/MSF/MSFCommon.h" -#include "llvm/DebugInfo/MSF/MSFError.h" #include "llvm/DebugInfo/MSF/MSFStreamLayout.h" using namespace llvm; @@ -89,10 +89,8 @@ MappedBlockStream::createFpmStream(const MSFLayout &Layout, Error MappedBlockStream::readBytes(uint32_t Offset, uint32_t Size, ArrayRef &Buffer) { // Make sure we aren't trying to read beyond the end of the stream. - if (Size > StreamLayout.Length) - return make_error(msf_error_code::insufficient_buffer); - if (Offset > StreamLayout.Length - Size) - return make_error(msf_error_code::insufficient_buffer); + if (auto EC = checkOffset(Offset, Size)) + return EC; if (tryReadContiguously(Offset, Size, Buffer)) return Error::success(); @@ -169,8 +167,9 @@ Error MappedBlockStream::readBytes(uint32_t Offset, uint32_t Size, Error MappedBlockStream::readLongestContiguousChunk(uint32_t Offset, ArrayRef &Buffer) { // Make sure we aren't trying to read beyond the end of the stream. - if (Offset >= StreamLayout.Length) - return make_error(msf_error_code::insufficient_buffer); + if (auto EC = checkOffset(Offset, 1)) + return EC; + uint32_t First = Offset / BlockSize; uint32_t Last = First; @@ -244,10 +243,8 @@ Error MappedBlockStream::readBytes(uint32_t Offset, uint32_t OffsetInBlock = Offset % BlockSize; // Make sure we aren't trying to read beyond the end of the stream. - if (Buffer.size() > StreamLayout.Length) - return make_error(msf_error_code::insufficient_buffer); - if (Offset > StreamLayout.Length - Buffer.size()) - return make_error(msf_error_code::insufficient_buffer); + if (auto EC = checkOffset(Offset, Buffer.size())) + return EC; uint32_t BytesLeft = Buffer.size(); uint32_t BytesWritten = 0; @@ -374,11 +371,8 @@ uint32_t WritableMappedBlockStream::getLength() { Error WritableMappedBlockStream::writeBytes(uint32_t Offset, ArrayRef Buffer) { // Make sure we aren't trying to write beyond the end of the stream. - if (Buffer.size() > getStreamLength()) - return make_error(msf_error_code::insufficient_buffer); - - if (Offset > getStreamLayout().Length - Buffer.size()) - return make_error(msf_error_code::insufficient_buffer); + if (auto EC = checkOffset(Offset, Buffer.size())) + return EC; uint32_t BlockNum = Offset / getBlockSize(); uint32_t OffsetInBlock = Offset % getBlockSize(); diff --git a/lib/DebugInfo/PDB/Native/PDBFile.cpp b/lib/DebugInfo/PDB/Native/PDBFile.cpp index 03257c40829..b2c8ac30e54 100644 --- a/lib/DebugInfo/PDB/Native/PDBFile.cpp +++ b/lib/DebugInfo/PDB/Native/PDBFile.cpp @@ -396,11 +396,10 @@ bool PDBFile::hasStringTable() { return IS->getNamedStreamIndex("/names") < getNumStreams(); } -/// Wrapper around MappedBlockStream::createIndexedStream() -/// that checks if a stream with that index actually exists. -/// If it does not, the return value will have an MSFError with -/// code msf_error_code::no_stream. Else, the return value will -/// contain the stream returned by createIndexedStream(). +/// Wrapper around MappedBlockStream::createIndexedStream() that checks if a +/// stream with that index actually exists. If it does not, the return value +/// will have an MSFError with code msf_error_code::no_stream. Else, the return +/// value will contain the stream returned by createIndexedStream(). Expected> PDBFile::safelyCreateIndexedStream(const MSFLayout &Layout, BinaryStreamRef MsfData, diff --git a/unittests/DebugInfo/PDB/MappedBlockStreamTest.cpp b/unittests/DebugInfo/PDB/MappedBlockStreamTest.cpp index 42f38a2e2e4..abeb4211165 100644 --- a/unittests/DebugInfo/PDB/MappedBlockStreamTest.cpp +++ b/unittests/DebugInfo/PDB/MappedBlockStreamTest.cpp @@ -42,16 +42,16 @@ public: Error readBytes(uint32_t Offset, uint32_t Size, ArrayRef &Buffer) override { - if (Offset + Size > Data.size()) - return make_error(msf_error_code::insufficient_buffer); + if (auto EC = checkOffset(Offset, Size)) + return EC; Buffer = Data.slice(Offset, Size); return Error::success(); } Error readLongestContiguousChunk(uint32_t Offset, ArrayRef &Buffer) override { - if (Offset >= Data.size()) - return make_error(msf_error_code::insufficient_buffer); + if (auto EC = checkOffset(Offset, 1)) + return EC; Buffer = Data.drop_front(Offset); return Error::success(); } @@ -59,8 +59,8 @@ public: uint32_t getLength() override { return Data.size(); } Error writeBytes(uint32_t Offset, ArrayRef SrcData) override { - if (Offset + SrcData.size() > Data.size()) - return make_error(msf_error_code::insufficient_buffer); + if (auto EC = checkOffset(Offset, SrcData.size())) + return EC; ::memcpy(&Data[Offset], SrcData.data(), SrcData.size()); return Error::success(); } -- 2.50.1