[pdb] Improve StreamInterface to support writing

[zturner]

Summary

This adds support for writing to a StreamInterface, including a MappedBlockStream which might be discontiguous.

No code actually uses this yet, but I've added a bunch of unit tests to illustrate the idea and demonstrate that it works.

There were two unexpected complexities that arose with this patch.

First, due to the nature of how we need to allocate from a pool when a read breaks a stream boundary, things get complicated when you go and start writing. If you just write to the underlying stream (which could be discontiguous at the point you write), a person holding onto a pool pointer will now have a stale view of the contents.

Second, we previously assumed that any two reads from the same stream offset would have the same size (due to the nature of how streams are laid out as records that don't often change). This is a very inconvenient assumption when trying to write unit tests, where you may want to write something, read it back out, then write over it, then read it back out again. So I removed this assumption.

The result is somewhat convoluted, but the idea is this:

read:
   1. try to read contiguously.  If so, return it.
   2. Otherwise check if there is a cached pool allocation
      that is the same size or bigger than the request.  If so, return it.
   3. Otherwise make a new pool allocation for the given offset and
      size, copy the data into it, and add it to the list of allocations for
      this offset.

write:
   1. Write all the data across the discontiguous sequence of blocks.
   2. Iterate every cached allocation, looking for any allocation which
      overlaps this request.
   3. For those which overlap, memcpy the overlapped portion of the
      cached allocation with the corresponding data from the new write

While this is O(n) in number of cached allocations, this is again most
common in tests, and will not be a frequent occurrence in real world
situations, so I think it's ok.

[ruiu]

IIUC, this is a patch to allow users to read and write arbitrary streams at arbitrary position just like files on a file system (I haven't read through yet, so correct me if I'm wrong.) I wonder if it is actually going to be needed. In most cases, we are going to create a new stream in PDB (by copying a stream and appending some data at end of it or in the middle of it) and then atomically update the MSF so that the old stream number now refers to the new stream, no? If that's the case, we open most streams as read-only and write to streams without seeking back.

[zturner]

In D21157#452974, @ruiu wrote:

IIUC, this is a patch to allow users to read and write arbitrary streams at arbitrary position just like files on a file system (I haven't read through yet, so correct me if I'm wrong.) I wonder if it is actually going to be needed. In most cases, we are going to create a new stream in PDB (by copying a stream and appending some data at end of it or in the middle of it) and then atomically update the MSF so that the old stream number now refers to the new stream, no? If that's the case, we open most streams as read-only and write to streams without seeking back.

While it might not be an immediate priority for LLD, I think it will be needed in the future for LLD, and I also think it will be very helpful for some of the improvements tools I have in mind. For example, we might want to find out how our tools would handle a specific type of CodeView record, or how Microsoft's tool would handle a specific type of CodeView record. But it's hard to figure out how to get Microsoft to generate that record type, or get it with just the right flag that we want to test. With the YAML stuff, we will be able to write YAML however we want, and generate a PDB that contains that YAML. We may even want to have the option to take a small fragment of YAML that describes 1 single record, and inject it into an existing PDB file.

For those kind of operations, we will need the ability to "append" to a PDB. This means handling complications like what happens if you want to write to a PDB, but there aren't enough blocks. You have to allocate a new block at the end of the file, then go update the directory, and then write to the PDB while making use of the newly allocated block.

Finally, I think it will be very helpful for testing. Because when we write a test to make sure we can write PDBs, the easiest way to start doing that is to output some YAML, generate a PDB from it, then dump it, and make sure the dump of the new PDB and the old PDB looks the same. In order to do that we will need to be able to maintain the structure (stream order, etc).

While it does allow writing into the middle of a stream at an arbitrary position, that wasn't the primary motivating factor. The primary motivating factor is just to be able to treat discontiguous sequences of blocks as contiguous when writing, so that the person doing the writing doesn't have to worry about seeking around to different blocks. As long as you give it the stream map, it just works.

[ruiu]

If you like the readable and writable streams, I don't have a strong opinion to oppose it, since I don't really understand the need here.

But it is a bit concerning that the use case you wrote are currently all hypothetical, and we are writing relatively complex code particularly for cache consistency. Do we need to update the cached entries? It might be a good property to guarantee, but we may be able to just say that "when you write something to a stream, all data objects you read before are invalid now -- so don't use it or read it again." It's hard for me to imagine a use case in which you read, write and read some data again and again from/to the same stream. Even if you need it in future, you can add it when it is needed, no?

include/llvm/DebugInfo/CodeView/StreamWriter.h

32–33

I'd like to name them writeUint{16,32} instead of defining as a overloaded function. Sometimes argument type is not obvious in a local context (uses of auto makes it much harder).

[zturner]

In D21157#453015, @ruiu wrote:

If you like the readable and writable streams, I don't have a strong opinion to oppose it, since I don't really understand the need here.

So imagine we want to test that our PDB writing code works. How would we do it? Because we aren't generating any PDBs yet. Even if we imagine that 6 months down the line LLD has full support for generating PDBs, how complete will it be? What if there's one record type that we don't generate correctly, but we don't know because none of our tests compile the right code to exercise it?

The need is basically to be able to enable better testing. Our support for dumping PDBs is becoming more and more complete, and only a few things remain. It does this by reading the PDB, building a bunch of internal data structures, and printing them. When it's time for LLD to generate PDBs, it will also probably use some internal data structures to represent the various streams and records. Since the ones we're using for reading are already well tested and work pretty well, it would be nice to find a way to reuse as much as possible for writing, because they already work. Furthermore, by using the same code, we can start testing arbitrarily complex PDBs right now, and then when it comes time for LLD to start writing PDBs, all of the code will already be tested. The YAML stuff, for example, is just a description of the PDB and the records. So we read the YAML, use the fields to fill out the structures that describe the PDB, and then use the StreamWriter to write that into an actual PDB. And then LLD will do almost the same thing, except instead of coming from YAML, the "description" will come from IR metadata. But everything else will be the same. Fill out the fields, write the file.

But it is a bit concerning that the use case you wrote are currently all hypothetical, and we are writing relatively complex code particularly for cache consistency. Do we need to update the cached entries? It might be a good property to guarantee, but we may be able to just say that "when you write something to a stream, all data objects you read before are invalid now -- so don't use it or read it again." It's hard for me to imagine a use case in which you read, write and read some data again and again from/to the same stream. Even if you need it in future, you can add it when it is needed, no?

I thought about doing it that way, but it seemed overly harsh to say if you even write one byte to a stream, any outstanding references pointing into the stream are invalidated. Mostly I did it because it made the tests much cleaner and shorter since I didn't have to construct brand new objects every time just to write one integer.

[zturner]

include/llvm/DebugInfo/CodeView/StreamWriter.h

32–33

Hmm, it turns out this makes writeEnum difficult to implement, because it relies on the overload. It can still be done with something like this:

template<typename T, typename U = std::underlying_type<T>::type> Error writeEnum(T Num);

template<typename T> Error writeEnum<T, uint32_t> Error writeEnum(T Num) {
  return writeUInt32(static_cast<uint32_t>(Num));
}

template<typename T> Error writeEnum<T, uint16_t> Error writeEnum(T Num) {
  return writeUInt16(static_cast<uint16_t>(Num));
}

but I start to wonder if it's an improvement.

[majnemer]

lib/DebugInfo/PDB/Raw/MappedBlockStream.cpp

47–48

Doesn't this mean that larger queries that happened earlier are not reused?

Let's say we start with an empty cache.
Then a read request for 4096 bytes from offset 0 happens.
Then a read request for 1024 bytes at offset 1024 happens.
Won't the second request not share with the first?

[zturner]

lib/DebugInfo/PDB/Raw/MappedBlockStream.cpp

47–48

You are correct. And I thought of that, but it makes the data structure more complicated, and I didn't think it was worth it for a code path that would almost never get exercised anyway. This is mostly here so you don't shoot yourself in the foot, but as Rui mentioned earlier, read/write on the same pdb is not going to be a common thing at all, so I made it just smart enough that if you use it your program will still work, but still stupid enough that I didn't have to devise a new complicated data structure.

[zturner]

Ob, one more thing. Larger queries will still be reused IF offset of the
previous request is the same as offset of the current request.

What doesn't work is if they overlap but start at different locations. Eg

Read 1: offset 0, length 1024
Read 2: offset 512, length 256

Even in that case though, if the 256 byte read didn't cross the
discontinuity, it would be satisfied without the cache. So probability is
very low of running into this

[llvm-commits]

On second thought i think i can actually make that work. I was thinking
you'd want a full blown interval tree, but it could be done using the same
walk I'm doing for writing. Since this is still a rare case, I'm fine
adding this, or not adding it, depending on what people prefer

• msg-20328-562.txt162 BDownload

[ruiu]

Although I agree with you that this is good for testing and completeness is generally good, but how about runtime performance? Updating read buffers incurs runtime cost and hence slows down PDB file generation. The most important thing for the linker is performance, and the current good number comes from the fact that we just do less instead of doing everything efficiently. If we add too much to the PDB reader/writer library beforehand, we risk the usability of the library for the linker. Aiming completeness is not always a win.

Being said that, the most difficult point of PDB reading/writing is to understand the format, so once we get a complete picture, it should be much easier to speed it up (by eliminating features or whatever) or write another one. So I think it's your call. If this helps creating PDB and help us understand the PDB as a file format (I guess it is), it's probably a good change.

[zturner]

In D21157#453549, @ruiu wrote:

Although I agree with you that this is good for testing and completeness is generally good, but how about runtime performance? Updating read buffers incurs runtime cost and hence slows down PDB file generation. The most important thing for the linker is performance, and the current good number comes from the fact that we just do less instead of doing everything efficiently. If we add too much to the PDB reader/writer library beforehand, we risk the usability of the library for the linker. Aiming completeness is not always a win.

Being said that, the most difficult point of PDB reading/writing is to understand the format, so once we get a complete picture, it should be much easier to speed it up (by eliminating features or whatever) or write another one. So I think it's your call. If this helps creating PDB and help us understand the PDB as a file format (I guess it is), it's probably a good change.

Yea, I agree we will want to be careful of performance. I think if it's a problem then we can rename MappedBlockStream to ReadWritePdbStream and then make a WriteOnlyPdbStream that doesn't do any of this and has none of the caching logic at all. I think we have enough extensibility points to get the performance we need, but I guess we don't know until we try. Worst case scenario I guess we have a second codepath for writing, but at least with all the testing we will understand the format. I'm hoping we can reuse though.

[ruiu]

LGTM

include/llvm/DebugInfo/PDB/Raw/MappedBlockStream.h

55–58

Is this different from MutableArrayRef?

[zturner]

include/llvm/DebugInfo/PDB/Raw/MappedBlockStream.h

55–58

That's a good point, I should fix that. Thanks

[chapuni]

llvm/trunk/unittests/DebugInfo/PDB/MappedBlockStreamTest.cpp

317 ↗

(On Diff #60309)

I am afraid if it generated temporary objects. Fixed in r272457.

include/llvm/DebugInfo/CodeView/ByteStream.h

 #define LLVM_DEBUGINFO_CODEVIEW_BYTESTREAM_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/DebugInfo/CodeView/StreamInterface.h"
 #include "llvm/Support/Error.h"
 #include <cstdint>
 #include <memory>
+#include <type_traits>
 
 namespace llvm {
 namespace codeview {
 class StreamReader;
 
-class ByteStream : public StreamInterface {
+template <bool Writable = false> class ByteStream : public StreamInterface {
+  typedef typename std::conditional<Writable, MutableArrayRef<uint8_t>,
+                                    ArrayRef<uint8_t>>::type ArrayType;
+
 public:
   ByteStream();
-  explicit ByteStream(ArrayRef<uint8_t> Data);
+  explicit ByteStream(ArrayType Data);
   ~ByteStream() override;
 
   Error readBytes(uint32_t Offset, uint32_t Size,
                   ArrayRef<uint8_t> &Buffer) const override;
 
+  Error writeBytes(uint32_t Offset, ArrayRef<uint8_t> Buffer) const override;
+
   uint32_t getLength() const override;
 
   ArrayRef<uint8_t> data() const { return Data; }
   StringRef str() const;
 
 private:
-  ArrayRef<uint8_t> Data;
+  ArrayType Data;
 };
 
+extern template class ByteStream<true>;
+extern template class ByteStream<false>;
+
 } // end namespace pdb
 } // end namespace llvm
 
 #endif // LLVM_DEBUGINFO_CODEVIEW_BYTESTREAM_H

include/llvm/DebugInfo/CodeView/CodeViewError.h

 
 #include <string>
 
 namespace llvm {
 namespace codeview {
 enum class cv_error_code {
   unspecified = 1,
   insufficient_buffer,
+  operation_unsupported,
   corrupt_record,
 };
 
 /// Base class for errors originating when parsing raw PDB files
 class CodeViewError : public ErrorInfo<CodeViewError> {
 public:
   static char ID;
   CodeViewError(cv_error_code C);

include/llvm/DebugInfo/CodeView/StreamArray.h

public:
   Iterator begin(bool *HadError = nullptr) const {
     return Iterator(*this, E, HadError);
   }
 
   Iterator end() const { return Iterator(E); }
 
   const Extractor &getExtractor() const { return E; }
 
+  StreamRef getUnderlyingStream() const { return Stream; }
+
 private:
   StreamRef Stream;
   Extractor E;
 };
 
 template <typename ValueType, typename Extractor> class VarStreamArrayIterator {
   typedef VarStreamArrayIterator<ValueType, Extractor> IterType;
   typedef VarStreamArray<ValueType, Extractor> ArrayType;

include/llvm/DebugInfo/CodeView/StreamInterface.h

 /// discontiguous blocks.  The implementation is required to return references
 /// to stable memory, so if this is not possible (for example in the case of
 /// a PDB file with discontiguous blocks, it must keep its own pool of temp
 /// storage.
 class StreamInterface {
 public:
   virtual ~StreamInterface() {}
 
+  // Given an offset into the stream and a number of bytes, attempt to read
+  // the bytes and set the output ArrayRef to point to a reference into the
+  // stream, without copying any data.
   virtual Error readBytes(uint32_t Offset, uint32_t Size,
                           ArrayRef<uint8_t> &Buffer) const = 0;
 
+  // Attempt to write the given bytes into the stream at the desired offset.
+  // This will always necessitate a copy.  Cannot shrink or grow the stream,
+  // only writes into existing allocated space.
+  virtual Error writeBytes(uint32_t Offset, ArrayRef<uint8_t> Data) const = 0;
+
   virtual uint32_t getLength() const = 0;
 };
 
 } // end namespace codeview
 } // end namespace llvm
 
 #endif // LLVM_DEBUGINFO_CODEVIEW_STREAMINTERFACE_H

include/llvm/DebugInfo/CodeView/StreamRef.h

public:
 
   Error readBytes(uint32_t Offset, uint32_t Size,
                   ArrayRef<uint8_t> &Buffer) const override {
     if (Size + Offset > Length)
       return make_error<CodeViewError>(cv_error_code::insufficient_buffer);
     return Stream->readBytes(ViewOffset + Offset, Size, Buffer);
   }
 
+  Error writeBytes(uint32_t Offset, ArrayRef<uint8_t> Data) const override {
+    if (Data.size() + Offset > Length)
+      return make_error<CodeViewError>(cv_error_code::insufficient_buffer);
+    return Stream->writeBytes(ViewOffset + Offset, Data);
+  }
+
   uint32_t getLength() const override { return Length; }
   StreamRef drop_front(uint32_t N) const {
     if (!Stream)
       return StreamRef();
 
     N = std::min(N, Length);
     return StreamRef(*Stream, ViewOffset + N, Length - N);
   }

include/llvm/DebugInfo/CodeView/StreamWriter.h

+//===- StreamWriter.h - Writes bytes and objects to a stream ----*- 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_CODEVIEW_STREAMWRITER_H
+#define LLVM_DEBUGINFO_CODEVIEW_STREAMWRITER_H
+
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/DebugInfo/CodeView/CodeViewError.h"
+#include "llvm/DebugInfo/CodeView/StreamArray.h"
+#include "llvm/DebugInfo/CodeView/StreamInterface.h"
+#include "llvm/Support/Endian.h"
+#include "llvm/Support/Error.h"
+
+#include <string>
+
+namespace llvm {
+namespace codeview {
+
+class StreamRef;
+
+class StreamWriter {
+public:
+  StreamWriter(StreamRef Stream);
+
+  Error writeBytes(ArrayRef<uint8_t> Buffer);
+  Error writeInteger(uint16_t Dest);
+  Error writeInteger(uint32_t Dest);
+  Error writeZeroString(StringRef Str);
+  Error writeFixedString(StringRef Str);
+  Error writeStreamRef(StreamRef Ref);
+  Error writeStreamRef(StreamRef Ref, uint32_t Size);
+
+  template <typename T> Error writeEnum(T Num) {
+    return writeInteger(static_cast<std::underlying_type<T>::type>(Num));
+  }
+
+  template <typename T> Error writeObject(const T &Obj) {
+    return writeBytes(
+        ArrayRef<uint8_t>(reinterpret_cast<const uint8_t *>(&Obj), sizeof(T)));
+  }
+
+  template <typename T> Error writeArray(ArrayRef<T> Array) {
+    if (Array.size() == 0)
+      return Error::success();
+
+    if (Array.size() > UINT32_MAX / sizeof(T))
+      return make_error<CodeViewError>(cv_error_code::insufficient_buffer);
+
+    return writeBytes(
+        ArrayRef<uint8_t>(reinterpret_cast<const uint8_t *>(Array.data()),
+                          Array.size() * sizeof(T)));
+  }
+
+  template <typename T, typename U>
+  Error writeArray(VarStreamArray<T, U> Array) {
+    return writeStreamRef(Array.getUnderlyingStream());
+  }
+
+  template <typename T> Error writeArray(FixedStreamArray<T> Array) {
+    return writeStreamRef(Array.getUnderlyingStream());
+  }
+
+  void setOffset(uint32_t Off) { Offset = Off; }
+  uint32_t getOffset() const { return Offset; }
+  uint32_t getLength() const { return Stream.getLength(); }
+  uint32_t bytesRemaining() const { return getLength() - getOffset(); }
+
+private:
+  StreamRef Stream;
+  uint32_t Offset;
+};
+} // namespace codeview
+} // namespace llvm
+
+#endif // LLVM_DEBUGINFO_CODEVIEW_STREAMREADER_H

include/llvm/DebugInfo/PDB/Raw/IPDBFile.h

 //===- IPDBFile.h - Abstract base class for a PDB file ----------*- 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_PDB_RAW_IPDBFILE_H
 #define LLVM_DEBUGINFO_PDB_RAW_IPDBFILE_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/DebugInfo/CodeView/StreamArray.h"
 #include "llvm/Support/Endian.h"
+#include "llvm/Support/Error.h"
 
 #include <stdint.h>
 
 namespace llvm {
 namespace pdb {
 
 class IPDBFile {
 public:
   virtual ~IPDBFile() {}
 
   virtual uint32_t getBlockSize() const = 0;
   virtual uint32_t getBlockCount() const = 0;
 
   virtual uint32_t getNumStreams() const = 0;
   virtual uint32_t getStreamByteSize(uint32_t StreamIndex) const = 0;
   virtual ArrayRef<support::ulittle32_t>
   getStreamBlockList(uint32_t StreamIndex) const = 0;
 
   virtual ArrayRef<uint8_t> getBlockData(uint32_t BlockIndex,
                                          uint32_t NumBytes) const = 0;
+  virtual Error setBlockData(uint32_t BlockIndex, uint32_t Offset,
+                             ArrayRef<uint8_t> Data) const = 0;
 };
 }
 }
 
 #endif // LLVM_DEBUGINFO_PDB_RAW_IPDBFILE_H

include/llvm/DebugInfo/PDB/Raw/MappedBlockStream.h

 class IPDBFile;
 class IPDBStreamData;
 class PDBFile;
 
 class MappedBlockStream : public codeview::StreamInterface {
 public:
   Error readBytes(uint32_t Offset, uint32_t Size,
                   ArrayRef<uint8_t> &Buffer) const override;
+  Error writeBytes(uint32_t Offset, ArrayRef<uint8_t> Buffer) const override;
 
   uint32_t getLength() const override;
 
   uint32_t getNumBytesCopied() const;
 
   static Expected<std::unique_ptr<MappedBlockStream>>
   createIndexedStream(uint32_t StreamIdx, const IPDBFile &File);
   static Expected<std::unique_ptr<MappedBlockStream>>
   createDirectoryStream(const PDBFile &File);
 
 protected:
   MappedBlockStream(std::unique_ptr<IPDBStreamData> Data, const IPDBFile &File);
 
   Error readBytes(uint32_t Offset, MutableArrayRef<uint8_t> Buffer) const;
   bool tryReadContiguously(uint32_t Offset, uint32_t Size,
                            ArrayRef<uint8_t> &Buffer) const;
 
   const IPDBFile &Pdb;
   std::unique_ptr<IPDBStreamData> Data;
 
+  struct CacheEntry {
+    uint8_t *Ptr;
+    uint32_t AllocSize;
+  };
   mutable llvm::BumpPtrAllocator Pool;
-  mutable DenseMap<uint32_t, uint8_t *> CacheMap;
+  mutable DenseMap<uint32_t, std::vector<CacheEntry>> CacheMap;
 };
 
 } // end namespace pdb
 } // end namespace llvm
 
 #endif // LLVM_DEBUGINFO_PDB_RAW_MAPPEDBLOCKSTREAM_H

include/llvm/DebugInfo/PDB/Raw/PDBFile.h

public:
 
   uint32_t getNumStreams() const override;
   uint32_t getStreamByteSize(uint32_t StreamIndex) const override;
   ArrayRef<support::ulittle32_t>
   getStreamBlockList(uint32_t StreamIndex) const override;
 
   ArrayRef<uint8_t> getBlockData(uint32_t BlockIndex,
                                  uint32_t NumBytes) const override;
+  Error setBlockData(uint32_t BlockIndex, uint32_t Offset,
+                     ArrayRef<uint8_t> Data) const override;
 
   ArrayRef<support::ulittle32_t> getDirectoryBlockArray() const;
 
   Error parseFileHeaders();
   Error parseStreamData();
 
   static uint64_t bytesToBlocks(uint64_t NumBytes, uint64_t BlockSize) {
     return alignTo(NumBytes, BlockSize) / BlockSize;

include/llvm/DebugInfo/PDB/Raw/RawError.h

 namespace llvm {
 namespace pdb {
 enum class raw_error_code {
   unspecified = 1,
   feature_unsupported,
   corrupt_file,
   insufficient_buffer,
   no_stream,
-  index_out_of_bounds
+  index_out_of_bounds,
+  invalid_block_address,
+  not_writable,
 };
 
 /// Base class for errors originating when parsing raw PDB files
 class RawError : public ErrorInfo<RawError> {
 public:
   static char ID;
   RawError(raw_error_code C);
   RawError(const std::string &Context);

include/llvm/Support/MathExtras.h

 }
 
 /// \brief Sign extend number in the bottom B bits of X to a 64-bit int.
 /// Requires 0 < B <= 64.
 inline int64_t SignExtend64(uint64_t X, unsigned B) {
   return int64_t(X << (64 - B)) >> (64 - B);
 }
 
+/// \brief Subtract two unsigned integers, X and Y, of type T and return their
+/// absolute value.
+template <typename T>
+typename std::enable_if<std::is_unsigned<T>::value, T>::type
+AbsoluteDifference(T X, T Y) {
+  return std::max(X, Y) - std::min(X, Y);
+}
+
 /// \brief Add two unsigned integers, X and Y, of type T.
 /// Clamp the result to the maximum representable value of T on overflow.
 /// ResultOverflowed indicates if the result is larger than the maximum
 /// representable value of type T.
 template <typename T>
 typename std::enable_if<std::is_unsigned<T>::value, T>::type
 SaturatingAdd(T X, T Y, bool *ResultOverflowed = nullptr) {
   bool Dummy;

lib/DebugInfo/CodeView/ByteStream.cpp

 #include "llvm/DebugInfo/CodeView/ByteStream.h"
 #include "llvm/DebugInfo/CodeView/CodeViewError.h"
 #include "llvm/DebugInfo/CodeView/StreamReader.h"
 #include <cstring>
 
 using namespace llvm;
 using namespace llvm::codeview;
 
-ByteStream::ByteStream() {}
+template class ByteStream<true>;
+template class ByteStream<false>;
 
-ByteStream::ByteStream(ArrayRef<uint8_t> Data) : Data(Data) {}
+template <bool Writable> ByteStream<Writable>::ByteStream() {}
 
-ByteStream::~ByteStream() {}
+template <bool Writable>
+ByteStream<Writable>::ByteStream(ArrayType Data) : Data(Data) {}
 
-Error ByteStream::readBytes(uint32_t Offset, uint32_t Size,
+template <bool Writable> ByteStream<Writable>::~ByteStream() {}
+
+template <bool Writable>
+Error ByteStream<Writable>::readBytes(uint32_t Offset, uint32_t Size,
                                       ArrayRef<uint8_t> &Buffer) const {
   if (Data.size() < Size + Offset)
     return make_error<CodeViewError>(cv_error_code::insufficient_buffer);
   Buffer = Data.slice(Offset, Size);
   return Error::success();
 }
 
-uint32_t ByteStream::getLength() const { return Data.size(); }
+template <>
+Error ByteStream<false>::writeBytes(uint32_t Offset,
+                                    ArrayRef<uint8_t> Buffer) const {
+  return make_error<CodeViewError>(cv_error_code::operation_unsupported,
+                                   "ByteStream is immutable.");
+}
+
+template <>
+Error ByteStream<true>::writeBytes(uint32_t Offset,
+                                   ArrayRef<uint8_t> Buffer) const {
+  if (Data.size() < Buffer.size())
+    return make_error<CodeViewError>(cv_error_code::insufficient_buffer);
+  if (Offset > Data.size() - Buffer.size())
+    return make_error<CodeViewError>(cv_error_code::insufficient_buffer);
+
+  ::memcpy(Data.data() + Offset, Buffer.data(), Buffer.size());
+  return Error::success();
+}
+
+template <bool Writable> uint32_t ByteStream<Writable>::getLength() const {
+  return Data.size();
+}
 
-StringRef ByteStream::str() const {
+template <bool Writable> StringRef ByteStream<Writable>::str() const {
   const char *CharData = reinterpret_cast<const char *>(Data.data());
   return StringRef(CharData, Data.size());
 }

lib/DebugInfo/CodeView/CMakeLists.txt

 add_llvm_library(LLVMDebugInfoCodeView
   ByteStream.cpp
   CodeViewError.cpp
   EnumTables.cpp
   FieldListRecordBuilder.cpp
   Line.cpp
   ListRecordBuilder.cpp
   MemoryTypeTableBuilder.cpp
   MethodListRecordBuilder.cpp
   ModuleSubstream.cpp
   ModuleSubstreamVisitor.cpp
   RecordSerialization.cpp
   StreamReader.cpp
+  StreamWriter.cpp
   SymbolDumper.cpp
   TypeDumper.cpp
   TypeRecord.cpp
   TypeRecordBuilder.cpp
   TypeStreamMerger.cpp
   TypeTableBuilder.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/DebugInfo/CodeView
   )

lib/DebugInfo/CodeView/CodeViewError.cpp

std::string message(int Condition) const override {
     switch (static_cast<cv_error_code>(Condition)) {
     case cv_error_code::unspecified:
       return "An unknown error has occurred.";
     case cv_error_code::insufficient_buffer:
       return "The buffer is not large enough to read the requested number of "
              "bytes.";
     case cv_error_code::corrupt_record:
       return "The CodeView record is corrupted.";
+    case cv_error_code::operation_unsupported:
+      return "The requested operation is not supported.";
     }
     llvm_unreachable("Unrecognized cv_error_code");
   }
 };
 } // end anonymous namespace
 
 static ManagedStatic<CodeViewErrorCategory> Category;
 

lib/DebugInfo/CodeView/StreamWriter.cpp

+//===- StreamWrite.cpp - Writes bytes and objects to a stream -------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/DebugInfo/CodeView/StreamWriter.h"
+
+#include "llvm/DebugInfo/CodeView/CodeViewError.h"
+#include "llvm/DebugInfo/CodeView/StreamRef.h"
+
+using namespace llvm;
+using namespace llvm::codeview;
+
+StreamWriter::StreamWriter(StreamRef S) : Stream(S), Offset(0) {}
+
+Error StreamWriter::writeBytes(ArrayRef<uint8_t> Buffer) {
+  if (auto EC = Stream.writeBytes(Offset, Buffer))
+    return EC;
+  Offset += Buffer.size();
+  return Error::success();
+}
+
+Error StreamWriter::writeInteger(uint16_t Int) {
+  return writeObject(support::ulittle16_t(Int));
+}
+
+Error StreamWriter::writeInteger(uint32_t Int) {
+  return writeObject(support::ulittle32_t(Int));
+}
+
+Error StreamWriter::writeZeroString(StringRef Str) {
+  if (auto EC = writeFixedString(Str))
+    return EC;
+  if (auto EC = writeObject('\0'))
+    return EC;
+
+  return Error::success();
+}
+
+Error StreamWriter::writeFixedString(StringRef Str) {
+  ArrayRef<uint8_t> Bytes(Str.bytes_begin(), Str.bytes_end());
+  if (auto EC = Stream.writeBytes(Offset, Bytes))
+    return EC;
+
+  Offset += Str.size();
+  return Error::success();
+}
+
+Error StreamWriter::writeStreamRef(StreamRef Ref) {
+  if (auto EC = writeStreamRef(Ref, Ref.getLength()))
+    return EC;
+  Offset += Ref.getLength();
+  return Error::success();
+}
+
+Error StreamWriter::writeStreamRef(StreamRef Ref, uint32_t Length) {
+  // This is a bit tricky, since we don't exactly want to read the entire
+  // stream into a contiguous array.  We need to think about how to do
+  // this properly.
+  return make_error<CodeViewError>(cv_error_code::operation_unsupported);
+}

lib/DebugInfo/CodeView/TypeDumper.cpp

 bool CVTypeDumper::dump(const CVTypeArray &Types) {
   assert(W && "printer should not be null");
   CVTypeDumperImpl Dumper(*this, *W, PrintRecordBytes);
   Dumper.visitTypeStream(Types);
   return !Dumper.hadError();
 }
 
 bool CVTypeDumper::dump(ArrayRef<uint8_t> Data) {
-  ByteStream Stream(Data);
+  ByteStream<> Stream(Data);
   CVTypeArray Types;
   StreamReader Reader(Stream);
   if (auto EC = Reader.readArray(Types, Reader.getLength())) {
     consumeError(std::move(EC));
     return false;
   }
 
   return dump(Types);
 }
 
 void CVTypeDumper::setPrinter(ScopedPrinter *P) {
   static ScopedPrinter NullP(llvm::nulls());
   W = P ? P : &NullP;
 }

lib/DebugInfo/PDB/Raw/MappedBlockStream.cpp

Error MappedBlockStream::readBytes(uint32_t Offset, uint32_t Size,
   if (Size > Data->getLength())
     return make_error<RawError>(raw_error_code::insufficient_buffer);
   if (Offset > Data->getLength() - Size)
     return make_error<RawError>(raw_error_code::insufficient_buffer);
 
   if (tryReadContiguously(Offset, Size, Buffer))
     return Error::success();
 
   auto CacheIter = CacheMap.find(Offset);
   if (CacheIter != CacheMap.end()) {
-    // In a more general solution, we would need to guarantee that the
-    // cached allocation is at least the requested size.  In practice, since
-    // these are CodeView / PDB records, we know they are always formatted
-    // the same way and never change, so we should never be requesting two
-    // allocations from the same address with different sizes.
-    Buffer = ArrayRef<uint8_t>(CacheIter->second, Size);
+    // Try to find an alloc that was large enough for this request.
+    for (auto &Entry : CacheIter->second) {
+      if (Entry.AllocSize >= Size) {
+        Buffer = ArrayRef<uint8_t>(Entry.Ptr, Size);
         return Error::success();
       }
+    }
+  }
 
   // Otherwise allocate a large enough buffer in the pool, memcpy the data
-  // into it, and return an ArrayRef to that.
+  // into it, and return an ArrayRef to that.  Do not touch existing pool
+  // allocations, as existing clients may be holding a pointer which must
+  // not be invalidated.
   uint8_t *WriteBuffer = Pool.Allocate<uint8_t>(Size);
-
   if (auto EC = readBytes(Offset, MutableArrayRef<uint8_t>(WriteBuffer, Size)))
     return EC;
-  CacheMap.insert(std::make_pair(Offset, WriteBuffer));
+
+  if (CacheIter != CacheMap.end()) {
+    CacheIter->second.push_back(CacheEntry{WriteBuffer, Size});
+  } else {
+    std::vector<CacheEntry> List;
+    List.push_back(CacheEntry{WriteBuffer, Size});
+    CacheMap.insert(std::make_pair(Offset, List));
+  }
   Buffer = ArrayRef<uint8_t>(WriteBuffer, Size);
   return Error::success();
 }
 
 uint32_t MappedBlockStream::getLength() const { return Data->getLength(); }
 
 bool MappedBlockStream::tryReadContiguously(uint32_t Offset, uint32_t Size,
                                             ArrayRef<uint8_t> &Buffer) const {
while (BytesLeft > 0) {
 
     BytesWritten += BytesInChunk;
     BytesLeft -= BytesInChunk;
     ++BlockNum;
     OffsetInBlock = 0;
   }
 
   return Error::success();
+}
+
+typedef std::pair<uint32_t, uint32_t> Interval;
+Interval intersect(const Interval &I1, const Interval &I2) {
+  return std::make_pair(std::max(I1.first, I2.first),
+                        std::min(I1.second, I2.second));
+}
+
+Error MappedBlockStream::writeBytes(uint32_t Offset,
+                                    ArrayRef<uint8_t> Buffer) const {
+  // Make sure we aren't trying to write beyond the end of the stream.
+  if (Buffer.size() > Data->getLength())
+    return make_error<RawError>(raw_error_code::insufficient_buffer);
+
+  if (Offset > Data->getLength() - Buffer.size())
+    return make_error<RawError>(raw_error_code::insufficient_buffer);
+
+  uint32_t BlockNum = Offset / Pdb.getBlockSize();
+  uint32_t OffsetInBlock = Offset % Pdb.getBlockSize();
 
+  uint32_t BytesLeft = Buffer.size();
+  auto BlockList = Data->getStreamBlocks();
+  uint32_t BytesWritten = 0;
+  while (BytesLeft > 0) {
+    uint32_t StreamBlockAddr = BlockList[BlockNum];
+    uint32_t BytesToWriteInChunk =
+        std::min(BytesLeft, Pdb.getBlockSize() - OffsetInBlock);
+
+    const uint8_t *Chunk = Buffer.data() + BytesWritten;
+    ArrayRef<uint8_t> ChunkData(Chunk, BytesToWriteInChunk);
+    if (auto EC = Pdb.setBlockData(StreamBlockAddr, OffsetInBlock, ChunkData))
+      return EC;
+
+    BytesLeft -= BytesToWriteInChunk;
+    BytesWritten += BytesToWriteInChunk;
+    ++BlockNum;
+    OffsetInBlock = 0;
+  }
+
+  // If this write overlapped a read which previously came from the pool,
+  // someone may still be holding a pointer to that alloc which is now invalid.
+  // Compute the overlapping range and update the cache entry, so any
+  // outstanding buffers are automatically updated.
+  for (const auto &MapEntry : CacheMap) {
+    // If the end of the written extent precedes the beginning of the cached
+    // extent, ignore this map entry.
+    if (Offset + BytesWritten < MapEntry.first)
+      continue;
+    for (const auto &Alloc : MapEntry.second) {
+      // If the end of the cached extent precedes the beginning of the written
+      // extent, ignore this alloc.
+      if (MapEntry.first + Alloc.AllocSize < Offset)
+        continue;
+
+      // If we get here, they are guaranteed to overlap.
+      Interval WriteInterval = std::make_pair(Offset, Offset + BytesWritten);
+      Interval CachedInterval =
+          std::make_pair(MapEntry.first, MapEntry.first + Alloc.AllocSize);
+      // If they overlap, we need to write the new data into the overlapping
+      // range.
+      auto Intersection = intersect(WriteInterval, CachedInterval);
+      assert(Intersection.first <= Intersection.second);
+
+      uint32_t Length = Intersection.second - Intersection.first;
+      uint32_t SrcOffset =
+          AbsoluteDifference(WriteInterval.first, Intersection.first);
+      uint32_t DestOffset =
+          AbsoluteDifference(CachedInterval.first, Intersection.first);
+      ::memcpy(Alloc.Ptr + DestOffset, Buffer.data() + SrcOffset, Length);
+    }
+  }
+
+  return Error::success();
 }
 
 uint32_t MappedBlockStream::getNumBytesCopied() const {
   return static_cast<uint32_t>(Pool.getBytesAllocated());
 }
 
 Expected<std::unique_ptr<MappedBlockStream>>
 MappedBlockStream::createIndexedStream(uint32_t StreamIdx,

lib/DebugInfo/PDB/Raw/PDBFile.cpp

ArrayRef<uint8_t> PDBFile::getBlockData(uint32_t BlockIndex,
   uint64_t StreamBlockOffset = blockToOffset(BlockIndex, getBlockSize());
 
   return ArrayRef<uint8_t>(
       reinterpret_cast<const uint8_t *>(Context->Buffer->getBufferStart()) +
           StreamBlockOffset,
       NumBytes);
 }
 
+Error PDBFile::setBlockData(uint32_t BlockIndex, uint32_t Offset,
+                            ArrayRef<uint8_t> Data) const {
+  if (BlockIndex >= getBlockCount())
+    return make_error<RawError>(raw_error_code::invalid_block_address);
+
+  if (Offset > getBlockSize() - Data.size())
+    return make_error<RawError>(raw_error_code::insufficient_buffer);
+
+  return make_error<RawError>(raw_error_code::not_writable);
+}
+
 Error PDBFile::parseFileHeaders() {
   std::error_code EC;
   MemoryBufferRef BufferRef = *Context->Buffer;
 
   // Make sure the file is sufficiently large to hold a super block.
   // Do this before attempting to read the super block.
   if (BufferRef.getBufferSize() < sizeof(SuperBlock))
     return make_error<RawError>(raw_error_code::corrupt_file,

lib/DebugInfo/PDB/Raw/RawError.cpp

case raw_error_code::corrupt_file:
       return "The PDB file is corrupt.";
     case raw_error_code::insufficient_buffer:
       return "The buffer is not large enough to read the requested number of "
              "bytes.";
     case raw_error_code::no_stream:
       return "The specified stream could not be loaded.";
     case raw_error_code::index_out_of_bounds:
       return "The specified item does not exist in the array.";
+    case raw_error_code::invalid_block_address:
+      return "The specified block address is not valid.";
+    case raw_error_code::not_writable:
+      return "The PDB does not support writing.";
     }
     llvm_unreachable("Unrecognized raw_error_code");
   }
 };
 } // end anonymous namespace
 
 static ManagedStatic<RawErrorCategory> Category;
 

tools/llvm-readobj/COFFDumper.cpp

void COFFDumper::printCodeViewSymbolsSubsection(StringRef Subsection,
                                                 const SectionRef &Section,
                                                 StringRef SectionContents) {
   ArrayRef<uint8_t> BinaryData(Subsection.bytes_begin(),
                                Subsection.bytes_end());
   auto CODD = llvm::make_unique<COFFObjectDumpDelegate>(*this, Section, Obj,
                                                         SectionContents);
 
   CVSymbolDumper CVSD(W, CVTD, std::move(CODD), opts::CodeViewSubsectionBytes);
-  ByteStream Stream(BinaryData);
+  ByteStream<> Stream(BinaryData);
   CVSymbolArray Symbols;
   StreamReader Reader(Stream);
   if (auto EC = Reader.readArray(Symbols, Reader.getLength())) {
     consumeError(std::move(EC));
     W.flush();
     error(object_error::parse_failed);
   }
 
if (SectionName == ".debug$T") {
       StringRef Data;
       error(S.getContents(Data));
       uint32_t Magic;
       error(consume(Data, Magic));
       if (Magic != 4)
         error(object_error::parse_failed);
       ArrayRef<uint8_t> Bytes(reinterpret_cast<const uint8_t *>(Data.data()),
                               Data.size());
-      ByteStream Stream(Bytes);
+      ByteStream<> Stream(Bytes);
       CVTypeArray Types;
       StreamReader Reader(Stream);
       if (auto EC = Reader.readArray(Types, Reader.getLength())) {
         consumeError(std::move(EC));
         W.flush();
         error(object_error::parse_failed);
       }
 

unittests/DebugInfo/PDB/MappedBlockStreamTest.cpp

 //===- llvm/unittest/DebugInfo/PDB/MappedBlockStreamTest.cpp --------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include <unordered_map>
 
 #include "llvm/DebugInfo/CodeView/StreamReader.h"
 #include "llvm/DebugInfo/CodeView/StreamRef.h"
+#include "llvm/DebugInfo/CodeView/StreamWriter.h"
 #include "llvm/DebugInfo/PDB/Raw/IPDBFile.h"
 #include "llvm/DebugInfo/PDB/Raw/IPDBStreamData.h"
 #include "llvm/DebugInfo/PDB/Raw/IndexedStreamData.h"
 #include "llvm/DebugInfo/PDB/Raw/MappedBlockStream.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
 using namespace llvm::codeview;
#define EXPECT_ERROR(Err)                                                      \
   {                                                                            \
     auto E = Err;                                                              \
     EXPECT_TRUE(static_cast<bool>(E));                                         \
     if (E)                                                                     \
       consumeError(std::move(E));                                              \
   }
 
 static const uint32_t BlocksAry[] = {0, 1, 2, 5, 4, 3, 6, 7, 8, 9};
-static const char DataAry[] = {'A', 'B', 'C', 'F', 'E',
-                               'D', 'G', 'H', 'I', 'J'};
+static uint8_t DataAry[] = {'A', 'B', 'C', 'F', 'E', 'D', 'G', 'H', 'I', 'J'};
 
 class DiscontiguousFile : public IPDBFile {
 public:
-  DiscontiguousFile()
-      : Blocks(std::begin(BlocksAry), std::end(BlocksAry)),
-        Data(std::begin(DataAry), std::end(DataAry)) {}
-
-  virtual uint32_t getBlockSize() const override { return 1; }
-  virtual uint32_t getBlockCount() const override { return 10; }
-  virtual uint32_t getNumStreams() const override { return 1; }
-  virtual uint32_t getStreamByteSize(uint32_t StreamIndex) const override {
+  DiscontiguousFile(ArrayRef<uint32_t> Blocks, MutableArrayRef<uint8_t> Data)
+      : Blocks(Blocks.begin(), Blocks.end()), Data(Data.begin(), Data.end()) {}
+
+  uint32_t getBlockSize() const override { return 1; }
+  uint32_t getBlockCount() const override { return Blocks.size(); }
+  uint32_t getNumStreams() const override { return 1; }
+  uint32_t getStreamByteSize(uint32_t StreamIndex) const override {
     return getBlockCount() * getBlockSize();
   }
-  virtual ArrayRef<support::ulittle32_t>
+  ArrayRef<support::ulittle32_t>
   getStreamBlockList(uint32_t StreamIndex) const override {
     if (StreamIndex != 0)
       return ArrayRef<support::ulittle32_t>();
     return Blocks;
   }
-  virtual ArrayRef<uint8_t> getBlockData(uint32_t BlockIndex,
-                                         uint32_t NumBytes) const override {
+  ArrayRef<uint8_t> getBlockData(uint32_t BlockIndex,
     return ArrayRef<uint8_t>(&Data[BlockIndex], NumBytes);
   }
 
+  Error setBlockData(uint32_t BlockIndex, uint32_t Offset,
+                     ArrayRef<uint8_t> SrcData) const override {
+    if (BlockIndex >= Blocks.size())
+      return make_error<CodeViewError>(cv_error_code::insufficient_buffer);
+    if (Offset > getBlockSize() - SrcData.size())
+      return make_error<CodeViewError>(cv_error_code::insufficient_buffer);
+    ::memcpy(&Data[BlockIndex] + Offset, SrcData.data(), SrcData.size());
+    return Error::success();
+  }
+
 private:
   std::vector<support::ulittle32_t> Blocks;
-  std::vector<uint8_t> Data;
+  MutableArrayRef<uint8_t> Data;
 };
 
 class MappedBlockStreamImpl : public MappedBlockStream {
 public:
   MappedBlockStreamImpl(std::unique_ptr<IPDBStreamData> Data,
                         const IPDBFile &File)
       : MappedBlockStream(std::move(Data), File) {}
 };
 
 // Tests that a read which is entirely contained within a single block works
 // and does not allocate.
 TEST(MappedBlockStreamTest, ReadBeyondEndOfStreamRef) {
-  DiscontiguousFile F;
+  DiscontiguousFile F(BlocksAry, DataAry);
   MappedBlockStreamImpl S(llvm::make_unique<IndexedStreamData>(0, F), F);
   StreamReader R(S);
   StreamRef SR;
   EXPECT_NO_ERROR(R.readStreamRef(SR, 0U));
   ArrayRef<uint8_t> Buffer;
   EXPECT_ERROR(SR.readBytes(0U, 1U, Buffer));
   EXPECT_NO_ERROR(R.readStreamRef(SR, 1U));
   EXPECT_ERROR(SR.readBytes(1U, 1U, Buffer));
 }
 
 // Tests that a read which outputs into a full destination buffer works and
 // does not fail due to the length of the output buffer.
 TEST(MappedBlockStreamTest, ReadOntoNonEmptyBuffer) {
-  DiscontiguousFile F;
+  DiscontiguousFile F(BlocksAry, DataAry);
   MappedBlockStreamImpl S(llvm::make_unique<IndexedStreamData>(0, F), F);
   StreamReader R(S);
   StringRef Str = "ZYXWVUTSRQPONMLKJIHGFEDCBA";
   EXPECT_NO_ERROR(R.readFixedString(Str, 1));
   EXPECT_EQ(Str, StringRef("A"));
   EXPECT_EQ(0U, S.getNumBytesCopied());
 }
 
 // Tests that a read which crosses a block boundary, but where the subsequent
 // blocks are still contiguous in memory to the previous block works and does
 // not allocate memory.
 TEST(MappedBlockStreamTest, ZeroCopyReadContiguousBreak) {
-  DiscontiguousFile F;
+  DiscontiguousFile F(BlocksAry, DataAry);
   MappedBlockStreamImpl S(llvm::make_unique<IndexedStreamData>(0, F), F);
   StreamReader R(S);
   StringRef Str;
   EXPECT_NO_ERROR(R.readFixedString(Str, 2));
   EXPECT_EQ(Str, StringRef("AB"));
   EXPECT_EQ(0U, S.getNumBytesCopied());
 
   R.setOffset(6);
   EXPECT_NO_ERROR(R.readFixedString(Str, 4));
   EXPECT_EQ(Str, StringRef("GHIJ"));
   EXPECT_EQ(0U, S.getNumBytesCopied());
 }
 
 // Tests that a read which crosses a block boundary and cannot be referenced
 // contiguously works and allocates only the precise amount of bytes
 // requested.
 TEST(MappedBlockStreamTest, CopyReadNonContiguousBreak) {
-  DiscontiguousFile F;
+  DiscontiguousFile F(BlocksAry, DataAry);
   MappedBlockStreamImpl S(llvm::make_unique<IndexedStreamData>(0, F), F);
   StreamReader R(S);
   StringRef Str;
   EXPECT_NO_ERROR(R.readFixedString(Str, 10));
   EXPECT_EQ(Str, StringRef("ABCDEFGHIJ"));
   EXPECT_EQ(10U, S.getNumBytesCopied());
 }
 
 // Test that an out of bounds read which doesn't cross a block boundary
 // fails and allocates no memory.
 TEST(MappedBlockStreamTest, InvalidReadSizeNoBreak) {
-  DiscontiguousFile F;
+  DiscontiguousFile F(BlocksAry, DataAry);
   MappedBlockStreamImpl S(llvm::make_unique<IndexedStreamData>(0, F), F);
   StreamReader R(S);
   StringRef Str;
 
   R.setOffset(10);
   EXPECT_ERROR(R.readFixedString(Str, 1));
   EXPECT_EQ(0U, S.getNumBytesCopied());
 }
 
 // Test that an out of bounds read which crosses a contiguous block boundary
 // fails and allocates no memory.
 TEST(MappedBlockStreamTest, InvalidReadSizeContiguousBreak) {
-  DiscontiguousFile F;
+  DiscontiguousFile F(BlocksAry, DataAry);
   MappedBlockStreamImpl S(llvm::make_unique<IndexedStreamData>(0, F), F);
   StreamReader R(S);
   StringRef Str;
 
   R.setOffset(6);
   EXPECT_ERROR(R.readFixedString(Str, 5));
   EXPECT_EQ(0U, S.getNumBytesCopied());
 }
 
 // Test that an out of bounds read which crosses a discontiguous block
 // boundary fails and allocates no memory.
 TEST(MappedBlockStreamTest, InvalidReadSizeNonContiguousBreak) {
-  DiscontiguousFile F;
+  DiscontiguousFile F(BlocksAry, DataAry);
   MappedBlockStreamImpl S(llvm::make_unique<IndexedStreamData>(0, F), F);
   StreamReader R(S);
   StringRef Str;
 
   EXPECT_ERROR(R.readFixedString(Str, 11));
   EXPECT_EQ(0U, S.getNumBytesCopied());
 }
 
 // Tests that a read which is entirely contained within a single block but
 // beyond the end of a StreamRef fails.
 TEST(MappedBlockStreamTest, ZeroCopyReadNoBreak) {
-  DiscontiguousFile F;
+  DiscontiguousFile F(BlocksAry, DataAry);
   MappedBlockStreamImpl S(llvm::make_unique<IndexedStreamData>(0, F), F);
   StreamReader R(S);
   StringRef Str;
   EXPECT_NO_ERROR(R.readFixedString(Str, 1));
   EXPECT_EQ(Str, StringRef("A"));
   EXPECT_EQ(0U, S.getNumBytesCopied());
 }
 
+TEST(MappedBlockStreamTest, WriteBeyondEndOfStream) {
+  static uint8_t Data[] = {'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J'};
+  static uint8_t LargeBuffer[] = {'0', '1', '2', '3', '4', '5',
+                                  '6', '7', '8', '9', 'A'};
+  static uint8_t SmallBuffer[] = {'0', '1', '2'};
+  static_assert(sizeof(LargeBuffer) > sizeof(Data),
+                "LargeBuffer is not big enough");
+
+  DiscontiguousFile F(BlocksAry, Data);
+  MappedBlockStreamImpl S(llvm::make_unique<IndexedStreamData>(0, F), F);
+  ArrayRef<uint8_t> Buffer;
+
+  EXPECT_ERROR(S.writeBytes(0, ArrayRef<uint8_t>(LargeBuffer)));
+  EXPECT_NO_ERROR(S.writeBytes(0, ArrayRef<uint8_t>(SmallBuffer)));
+  EXPECT_NO_ERROR(S.writeBytes(7, ArrayRef<uint8_t>(SmallBuffer)));
+  EXPECT_ERROR(S.writeBytes(8, ArrayRef<uint8_t>(SmallBuffer)));
+}
+
+TEST(MappedBlockStreamTest, TestWriteBytesNoBreakBoundary) {
+  static uint8_t Data[] = {'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J'};
+  DiscontiguousFile F(BlocksAry, Data);
+  MappedBlockStreamImpl S(llvm::make_unique<IndexedStreamData>(0, F), F);
+  ArrayRef<uint8_t> Buffer;
+
+  EXPECT_NO_ERROR(S.readBytes(0, 1, Buffer));
+  EXPECT_EQ(Buffer, ArrayRef<uint8_t>('A'));
+  EXPECT_NO_ERROR(S.readBytes(9, 1, Buffer));
+  EXPECT_EQ(Buffer, ArrayRef<uint8_t>('J'));
+
+  EXPECT_NO_ERROR(S.writeBytes(0, ArrayRef<uint8_t>('J')));
+  EXPECT_NO_ERROR(S.writeBytes(9, ArrayRef<uint8_t>('A')));
+
+  EXPECT_NO_ERROR(S.readBytes(0, 1, Buffer));
+  EXPECT_EQ(Buffer, ArrayRef<uint8_t>('J'));
+  EXPECT_NO_ERROR(S.readBytes(9, 1, Buffer));
+  EXPECT_EQ(Buffer, ArrayRef<uint8_t>('A'));
+
+  EXPECT_NO_ERROR(S.writeBytes(0, ArrayRef<uint8_t>('A')));
+  EXPECT_NO_ERROR(S.writeBytes(9, ArrayRef<uint8_t>('J')));
+
+  EXPECT_NO_ERROR(S.readBytes(0, 1, Buffer));
+  EXPECT_EQ(Buffer, ArrayRef<uint8_t>('A'));
+  EXPECT_NO_ERROR(S.readBytes(9, 1, Buffer));
+  EXPECT_EQ(Buffer, ArrayRef<uint8_t>('J'));
+}
+
+TEST(MappedBlockStreamTest, TestWriteBytesBreakBoundary) {
+  static uint8_t Data[] = {'0', '0', '0', '0', '0', '0', '0', '0', '0', '0'};
+  static uint8_t TestData[] = {'T', 'E', 'S', 'T', 'I', 'N', 'G', '.'};
+  static uint8_t Expected[] = {'T', 'E', 'S', 'N', 'I',
+                               'T', 'G', '.', '0', '0'};
+
+  DiscontiguousFile F(BlocksAry, Data);
+  MappedBlockStreamImpl S(llvm::make_unique<IndexedStreamData>(0, F), F);
+  ArrayRef<uint8_t> Buffer;
+
+  EXPECT_NO_ERROR(S.writeBytes(0, TestData));
+  // First just compare the memory, then compare the result of reading the
+  // string out.
+  EXPECT_EQ(ArrayRef<uint8_t>(Data), ArrayRef<uint8_t>(Expected));
+
+  EXPECT_NO_ERROR(S.readBytes(0, 8, Buffer));
+  EXPECT_EQ(Buffer, ArrayRef<uint8_t>(TestData));
+}
+
+TEST(MappedBlockStreamTest, TestWriteThenRead) {
+  std::vector<uint8_t> DataBytes(10);
+  MutableArrayRef<uint8_t> Data(DataBytes);
+  const uint32_t Blocks[] = {2, 1, 0, 6, 3, 4, 5, 7, 9, 8};
+
+  DiscontiguousFile F(Blocks, Data);
+  MappedBlockStreamImpl S(llvm::make_unique<IndexedStreamData>(0, F), F);
+
+  enum class MyEnum : uint32_t { Val1 = 2908234, Val2 = 120891234 };
+
+  uint16_t u16[] = {31468, 0};
+  uint32_t u32[] = {890723408, 0};
+  MyEnum Enum[] = {MyEnum::Val1, MyEnum::Val2};
+  StringRef ZStr[] = {"Zero Str", ""};
+  StringRef FStr[] = {"Fixed Str", ""};
+  ArrayRef<uint8_t> byteArray[] = {{'1', '2'}, {'0', '0'}};
+  ArrayRef<uint32_t> intArray[] = {{890723408, 29082234}, {0, 0}};
+
+  StreamReader Reader(S);
+  StreamWriter Writer(S);
+  EXPECT_NO_ERROR(Writer.writeInteger(u16[0]));
+  EXPECT_NO_ERROR(Reader.readInteger(u16[1]));
+  EXPECT_EQ(u16[0], u16[1]);
+  EXPECT_EQ(DataBytes,
+            std::vector<uint8_t>({0, 0x7A, 0xEC, 0, 0, 0, 0, 0, 0, 0}));
+
+  Reader.setOffset(0);
+  Writer.setOffset(0);
+  ::memset(DataBytes.data(), 0, 10);
+  EXPECT_NO_ERROR(Writer.writeInteger(u32[0]));
+  EXPECT_NO_ERROR(Reader.readInteger(u32[1]));
+  EXPECT_EQ(u32[0], u32[1]);
+  EXPECT_EQ(DataBytes,
+            std::vector<uint8_t>({0x17, 0x5C, 0x50, 0, 0, 0, 0x35, 0, 0, 0}));
+
+  Reader.setOffset(0);
+  Writer.setOffset(0);
+  ::memset(DataBytes.data(), 0, 10);
+  EXPECT_NO_ERROR(Writer.writeEnum(Enum[0]));
+  EXPECT_NO_ERROR(Reader.readEnum(Enum[1]));
+  EXPECT_EQ(Enum[0], Enum[1]);
+  EXPECT_EQ(DataBytes,
+            std::vector<uint8_t>({0x2C, 0x60, 0x4A, 0, 0, 0, 0, 0, 0, 0}));
+
+  Reader.setOffset(0);
+  Writer.setOffset(0);
+  ::memset(DataBytes.data(), 0, 10);
+  EXPECT_NO_ERROR(Writer.writeZeroString(ZStr[0]));
+  EXPECT_NO_ERROR(Reader.readZeroString(ZStr[1]));
+  EXPECT_EQ(ZStr[0], ZStr[1]);
+  EXPECT_EQ(DataBytes, std::vector<uint8_t>(
+                           {'r', 'e', 'Z', ' ', 'S', 't', 'o', 'r', 0, 0}));
+
+  Reader.setOffset(0);
+  Writer.setOffset(0);
+  ::memset(DataBytes.data(), 0, 10);
+  EXPECT_NO_ERROR(Writer.writeFixedString(FStr[0]));
+  EXPECT_NO_ERROR(Reader.readFixedString(FStr[1], FStr[0].size()));
+  EXPECT_EQ(FStr[0], FStr[1]);
+  EXPECT_EQ(DataBytes, std::vector<uint8_t>(
+                           {'x', 'i', 'F', 'd', ' ', 'S', 'e', 't', 0, 'r'}));
+
+  Reader.setOffset(0);
+  Writer.setOffset(0);
+  ::memset(DataBytes.data(), 0, 10);
+  EXPECT_NO_ERROR(Writer.writeArray(byteArray[0]));
+  EXPECT_NO_ERROR(Reader.readArray(byteArray[1], byteArray[0].size()));
+  EXPECT_EQ(byteArray[0], byteArray[1]);
+  EXPECT_EQ(DataBytes,
+            std::vector<uint8_t>({0, 0x32, 0x31, 0, 0, 0, 0, 0, 0, 0}));
+
+  Reader.setOffset(0);
+  Writer.setOffset(0);
+  ::memset(DataBytes.data(), 0, 10);
+  EXPECT_NO_ERROR(Writer.writeArray(intArray[0]));
+  EXPECT_NO_ERROR(Reader.readArray(intArray[1], intArray[0].size()));
+  EXPECT_EQ(intArray[0], intArray[1]);
+  EXPECT_EQ(DataBytes, std::vector<uint8_t>({0x17, 0x5C, 0x50, 0x7A, 0xC2, 0xBB,
+                                             0x35, 0x01, 0, 0}));
+}
+
 } // end anonymous namespace