minidump: Allow for user extension streams computed at crash time

Bug: crashpad:167
Change-Id: I94c143353482f100a31d6afb0685b38757a662d6
Reviewed-on: https://chromium-review.googlesource.com/455976
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>

minidump/minidump.gyp

@@ -72,6 +72,8 @@
         'minidump_thread_writer.h',
         'minidump_unloaded_module_writer.cc',
         'minidump_unloaded_module_writer.h',
+        'minidump_user_extension_stream_data_source.cc',
+        'minidump_user_extension_stream_data_source.h',
         'minidump_user_stream_writer.cc',
         'minidump_user_stream_writer.h',
         'minidump_writable.cc',

minidump/minidump_file_writer.cc

@@ -29,6 +29,7 @@
 #include "minidump/minidump_thread_id_map.h"
 #include "minidump/minidump_thread_writer.h"
 #include "minidump/minidump_unloaded_module_writer.h"
+#include "minidump/minidump_user_extension_stream_data_source.h"
 #include "minidump/minidump_user_stream_writer.h"
 #include "minidump/minidump_writer_util.h"
 #include "snapshot/exception_snapshot.h"
@@ -199,6 +200,19 @@ bool MinidumpFileWriter::AddStream(
   return true;
 }
 
+bool MinidumpFileWriter::AddUserExtensionStream(
+    std::unique_ptr<MinidumpUserExtensionStreamDataSource>
+        user_extension_stream_data) {
+  DCHECK_EQ(state(), kStateMutable);
+
+  auto user_stream = base::WrapUnique(new MinidumpUserStreamWriter());
+  user_stream->InitializeFromBuffer(user_extension_stream_data->stream_type(),
+                                    user_extension_stream_data->buffer(),
+                                    user_extension_stream_data->buffer_size());
+
+  return AddStream(std::move(user_stream));
+}
+
 bool MinidumpFileWriter::WriteEverything(FileWriterInterface* file_writer) {
   DCHECK_EQ(state(), kStateMutable);
 

minidump/minidump_file_writer.h

@@ -33,6 +33,7 @@
 namespace crashpad {
 
 class ProcessSnapshot;
+class MinidumpUserExtensionStreamDataSource;
 
 //! \brief The root-level object in a minidump file.
 //!
@@ -61,7 +62,11 @@ class MinidumpFileWriter final : public internal::MinidumpWritable {
   //!  - kMinidumpStreamTypeThreadList
   //!  - kMinidumpStreamTypeException (if present)
   //!  - kMinidumpStreamTypeModuleList
+  //!  - kMinidumpStreamTypeUnloadedModuleList (if present)
   //!  - kMinidumpStreamTypeCrashpadInfo (if present)
+  //!  - kMinidumpStreamTypeMemoryInfoList (if present)
+  //!  - kMinidumpStreamTypeHandleData (if present)
+  //!  - User streams (if present)
   //!  - kMinidumpStreamTypeMemoryList
   //!
   //! \param[in] process_snapshot The process snapshot to use as source data.
@@ -95,6 +100,30 @@ class MinidumpFileWriter final : public internal::MinidumpWritable {
   //!     with a message logged.
   bool AddStream(std::unique_ptr<internal::MinidumpStreamWriter> stream);
 
+  //! \brief Adds a user extension stream to the minidump file and arranges for
+  //!     a MINIDUMP_DIRECTORY entry to point to it.
+  //!
+  //! This object takes ownership of \a user_extension_stream_data.
+  //!
+  //! At most one object of each stream type (as obtained from
+  //! internal::MinidumpStreamWriter::StreamType()) may be added to a
+  //! MinidumpFileWriter object. If an attempt is made to add a stream whose
+  //! type matches an existing stream’s type, this method discards the new
+  //! stream.
+  //!
+  //! \param[in] user_extension_stream_data The stream data to add to the
+  //!    minidump file. Note that the buffer this object points to must be valid
+  //!    through WriteEverything().
+  //!
+  //! \note Valid in #kStateMutable.
+  //!
+  //! \return `true` on success. `false` on failure, as occurs when an attempt
+  //!     is made to add a stream whose type matches an existing stream’s type,
+  //!     with a message logged.
+  bool AddUserExtensionStream(
+      std::unique_ptr<MinidumpUserExtensionStreamDataSource>
+          user_extension_stream_data);
+
   // MinidumpWritable:
 
   //! \copydoc internal::MinidumpWritable::WriteEverything()

minidump/minidump_file_writer_test.cc

@@ -25,6 +25,7 @@
 #include "base/memory/ptr_util.h"
 #include "gtest/gtest.h"
 #include "minidump/minidump_stream_writer.h"
+#include "minidump/minidump_user_extension_stream_data_source.h"
 #include "minidump/minidump_writable.h"
 #include "minidump/test/minidump_file_writer_test_util.h"
 #include "minidump/test/minidump_writable_test_util.h"
@@ -127,6 +128,50 @@ TEST(MinidumpFileWriter, OneStream) {
   EXPECT_EQ(0, memcmp(stream_data, expected_stream.c_str(), kStreamSize));
 }
 
+TEST(MinidumpFileWriter, AddUserExtensionStream) {
+  MinidumpFileWriter minidump_file;
+  const time_t kTimestamp = 0x155d2fb8;
+  minidump_file.SetTimestamp(kTimestamp);
+
+  static const uint8_t kStreamData[] = "Hello World!";
+  const size_t kStreamSize = arraysize(kStreamData);
+  const MinidumpStreamType kStreamType = static_cast<MinidumpStreamType>(0x4d);
+
+  auto stream = base::WrapUnique(new MinidumpUserExtensionStreamDataSource(
+      kStreamType, kStreamData, kStreamSize));
+  ASSERT_TRUE(minidump_file.AddUserExtensionStream(std::move(stream)));
+
+  // Adding the same stream type a second time should fail.
+  stream = base::WrapUnique(new MinidumpUserExtensionStreamDataSource(
+      kStreamType, kStreamData, kStreamSize));
+  ASSERT_FALSE(minidump_file.AddUserExtensionStream(std::move(stream)));
+
+  StringFile string_file;
+  ASSERT_TRUE(minidump_file.WriteEverything(&string_file));
+
+  const size_t kDirectoryOffset = sizeof(MINIDUMP_HEADER);
+  const size_t kStreamOffset = kDirectoryOffset + sizeof(MINIDUMP_DIRECTORY);
+  const size_t kFileSize = kStreamOffset + kStreamSize;
+
+  ASSERT_EQ(kFileSize, string_file.string().size());
+
+  const MINIDUMP_DIRECTORY* directory;
+  const MINIDUMP_HEADER* header =
+      MinidumpHeaderAtStart(string_file.string(), &directory);
+  ASSERT_NO_FATAL_FAILURE(VerifyMinidumpHeader(header, 1, kTimestamp));
+  ASSERT_TRUE(directory);
+
+  EXPECT_EQ(kStreamType, directory[0].StreamType);
+  EXPECT_EQ(kStreamSize, directory[0].Location.DataSize);
+  EXPECT_EQ(kStreamOffset, directory[0].Location.Rva);
+
+  const uint8_t* stream_data = MinidumpWritableAtLocationDescriptor<uint8_t>(
+      string_file.string(), directory[0].Location);
+  ASSERT_TRUE(stream_data);
+
+  EXPECT_EQ(0, memcmp(stream_data, kStreamData, kStreamSize));
+}
+
 TEST(MinidumpFileWriter, ThreeStreams) {
   MinidumpFileWriter minidump_file;
   const time_t kTimestamp = 0x155d2fb8;

minidump/minidump_user_extension_stream_data_source.cc

@@ -0,0 +1,30 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "minidump/minidump_user_extension_stream_data_source.h"
+
+namespace crashpad {
+
+MinidumpUserExtensionStreamDataSource::MinidumpUserExtensionStreamDataSource(
+    uint32_t stream_type,
+    const void* buffer,
+    size_t buffer_size)
+    : stream_type_(static_cast<MinidumpStreamType>(stream_type)),
+      buffer_(buffer),
+      buffer_size_(buffer_size) {}
+
+MinidumpUserExtensionStreamDataSource::
+    ~MinidumpUserExtensionStreamDataSource() {}
+
+}  // namespace crashpad

minidump/minidump_user_extension_stream_data_source.h

@@ -0,0 +1,55 @@
+// Copyright 2017 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef CRASHPAD_MINIDUMP_MINIDUMP_USER_EXTENSION_STREAM_DATA_SOURCE_H_
+#define CRASHPAD_MINIDUMP_MINIDUMP_USER_EXTENSION_STREAM_DATA_SOURCE_H_
+
+#include <stdint.h>
+#include <sys/types.h>
+
+#include "base/macros.h"
+
+#include "minidump/minidump_extensions.h"
+
+namespace crashpad {
+
+//! \brief Describes a user extension data stream in a minidump.
+class MinidumpUserExtensionStreamDataSource {
+ public:
+  //! \brief Constructs a MinidumpUserExtensionStreamDataSource.
+  //!
+  //! \param[in] stream_type The type of the user extension stream.
+  //! \param[in] buffer Points to the data for this stream. \a buffer is not
+  //!     owned, and must outlive the use of this object.
+  //! \param[in] buffer_size The length of data in \a buffer.
+  MinidumpUserExtensionStreamDataSource(uint32_t stream_type,
+                                        const void* buffer,
+                                        size_t buffer_size);
+  ~MinidumpUserExtensionStreamDataSource();
+
+  MinidumpStreamType stream_type() const { return stream_type_; }
+  const void* buffer() const { return buffer_; }
+  size_t buffer_size() const { return buffer_size_; }
+
+ private:
+  MinidumpStreamType stream_type_;
+  const void* buffer_;  // weak
+  size_t buffer_size_;
+
+  DISALLOW_COPY_AND_ASSIGN(MinidumpUserExtensionStreamDataSource);
+};
+
+}  // namespace crashpad
+
+#endif  // CRASHPAD_MINIDUMP_MINIDUMP_USER_EXTENSION_STREAM_DATA_SOURCE_H_

minidump/minidump_user_stream_writer.cc

@@ -14,13 +14,67 @@
 
 #include "minidump/minidump_user_stream_writer.h"
 
+#include "base/memory/ptr_util.h"
 #include "util/file/file_writer.h"
 
 namespace crashpad {
 
-MinidumpUserStreamWriter::MinidumpUserStreamWriter()
-    : stream_type_(0), reader_() {
-}
+class MinidumpUserStreamWriter::ContentsWriter {
+ public:
+  virtual ~ContentsWriter() {}
+  virtual bool WriteContents(FileWriterInterface* writer) = 0;
+  virtual size_t GetSize() const = 0;
+};
+
+class MinidumpUserStreamWriter::SnapshotContentsWriter final
+    : public MinidumpUserStreamWriter::ContentsWriter,
+      public MemorySnapshot::Delegate {
+ public:
+  explicit SnapshotContentsWriter(const MemorySnapshot* snapshot)
+      : snapshot_(snapshot), writer_(nullptr) {}
+
+  bool WriteContents(FileWriterInterface* writer) override {
+    DCHECK(!writer_);
+
+    writer_ = writer;
+    if (!snapshot_)
+      return true;
+
+    return snapshot_->Read(this);
+  }
+
+  size_t GetSize() const override { return snapshot_ ? snapshot_->Size() : 0; };
+
+  bool MemorySnapshotDelegateRead(void* data, size_t size) override {
+    return writer_->Write(data, size);
+  }
+
+ private:
+  const MemorySnapshot* snapshot_;
+  FileWriterInterface* writer_;
+
+  DISALLOW_COPY_AND_ASSIGN(SnapshotContentsWriter);
+};
+
+class MinidumpUserStreamWriter::BufferContentsWriter final
+    : public MinidumpUserStreamWriter::ContentsWriter {
+ public:
+  BufferContentsWriter(const void* buffer, size_t buffer_size)
+      : buffer_(buffer), buffer_size_(buffer_size) {}
+
+  bool WriteContents(FileWriterInterface* writer) override {
+    return writer->Write(buffer_, buffer_size_);
+  }
+  size_t GetSize() const override { return buffer_size_; }
+
+ private:
+  const void* buffer_;
+  size_t buffer_size_;
+
+  DISALLOW_COPY_AND_ASSIGN(BufferContentsWriter);
+};
+
+MinidumpUserStreamWriter::MinidumpUserStreamWriter() : stream_type_() {}
 
 MinidumpUserStreamWriter::~MinidumpUserStreamWriter() {
 }
@@ -28,10 +82,23 @@ MinidumpUserStreamWriter::~MinidumpUserStreamWriter() {
 void MinidumpUserStreamWriter::InitializeFromSnapshot(
     const UserMinidumpStream* stream) {
   DCHECK_EQ(state(), kStateMutable);
+  DCHECK(!contents_writer_.get());
 
-  stream_type_ = stream->stream_type();
-  if (stream->memory())
-    stream->memory()->Read(&reader_);
+  stream_type_ = static_cast<MinidumpStreamType>(stream->stream_type());
+  contents_writer_ =
+      base::WrapUnique(new SnapshotContentsWriter(stream->memory()));
+}
+
+void MinidumpUserStreamWriter::InitializeFromBuffer(
+    MinidumpStreamType stream_type,
+    const void* buffer,
+    size_t buffer_size) {
+  DCHECK_EQ(state(), kStateMutable);
+  DCHECK(!contents_writer_.get());
+
+  stream_type_ = stream_type;
+  contents_writer_ =
+      base::WrapUnique(new BufferContentsWriter(buffer, buffer_size));
 }
 
 bool MinidumpUserStreamWriter::Freeze() {
@@ -42,7 +109,8 @@ bool MinidumpUserStreamWriter::Freeze() {
 
 size_t MinidumpUserStreamWriter::SizeOfObject() {
   DCHECK_GE(state(), kStateFrozen);
-  return reader_.size();
+
+  return contents_writer_->GetSize();
 }
 
 std::vector<internal::MinidumpWritable*>
@@ -53,21 +121,12 @@ MinidumpUserStreamWriter::Children() {
 
 bool MinidumpUserStreamWriter::WriteObject(FileWriterInterface* file_writer) {
   DCHECK_EQ(state(), kStateWritable);
-  return file_writer->Write(reader_.data(), reader_.size());
+
+  return contents_writer_->WriteContents(file_writer);
 }
 
 MinidumpStreamType MinidumpUserStreamWriter::StreamType() const {
   return static_cast<MinidumpStreamType>(stream_type_);
 }
 
-MinidumpUserStreamWriter::MemoryReader::~MemoryReader() {}
-
-bool MinidumpUserStreamWriter::MemoryReader::MemorySnapshotDelegateRead(
-    void* data,
-    size_t size) {
-  data_.resize(size);
-  memcpy(&data_[0], data, size);
-  return true;
-}
-
 }  // namespace crashpad

minidump/minidump_user_stream_writer.h

@@ -22,6 +22,7 @@
 #include <vector>
 
 #include "base/macros.h"
+#include "minidump/minidump_extensions.h"
 #include "minidump/minidump_stream_writer.h"
 #include "minidump/minidump_writable.h"
 #include "snapshot/module_snapshot.h"
@@ -41,6 +42,18 @@ class MinidumpUserStreamWriter final : public internal::MinidumpStreamWriter {
   //! \note Valid in #kStateMutable.
   void InitializeFromSnapshot(const UserMinidumpStream* stream);
 
+  //! \brief Initializes a MINIDUMP_USER_STREAM based on \a stream_type,
+  //!     \a buffer and \a buffer_size.
+  //!
+  //! \param[in] stream_type The type of the stream.
+  //! \param[in] buffer The data for the stream.
+  //! \param[in] buffer_size The length of \a buffer, and the resulting stream.
+  //!
+  //! \note Valid in #kStateMutable.
+  void InitializeFromBuffer(MinidumpStreamType stream_type,
+                            const void* buffer,
+                            size_t buffer_size);
+
  protected:
   // MinidumpWritable:
   bool Freeze() override;
@@ -52,22 +65,13 @@ class MinidumpUserStreamWriter final : public internal::MinidumpStreamWriter {
   MinidumpStreamType StreamType() const override;
 
  private:
-  class MemoryReader : public MemorySnapshot::Delegate {
-   public:
-    ~MemoryReader() override;
-    bool MemorySnapshotDelegateRead(void* data, size_t size) override;
+  class ContentsWriter;
+  class SnapshotContentsWriter;
+  class BufferContentsWriter;
 
-    const void* data() const {
-      return reinterpret_cast<const void*>(data_.data());
-    }
-    size_t size() const { return data_.size(); }
+  std::unique_ptr<ContentsWriter> contents_writer_;
 
-   private:
-    std::vector<uint8_t> data_;
-  };
-
-  uint32_t stream_type_;
-  MemoryReader reader_;
+  MinidumpStreamType stream_type_;
 
   DISALLOW_COPY_AND_ASSIGN(MinidumpUserStreamWriter);
 };

minidump/minidump_user_stream_writer_test.cc

@@ -32,7 +32,8 @@ namespace {
 // The user stream is expected to be the only stream.
 void GetUserStream(const std::string& file_contents,
                    MINIDUMP_LOCATION_DESCRIPTOR* user_stream_location,
-                   uint32_t stream_type) {
+                   uint32_t stream_type,
+                   size_t stream_size) {
   const size_t kDirectoryOffset = sizeof(MINIDUMP_HEADER);
   const size_t kUserStreamOffset =
       kDirectoryOffset + sizeof(MINIDUMP_DIRECTORY);
@@ -47,13 +48,16 @@ void GetUserStream(const std::string& file_contents,
 
   ASSERT_EQ(stream_type, directory[kDirectoryIndex].StreamType);
   EXPECT_EQ(kUserStreamOffset, directory[kDirectoryIndex].Location.Rva);
+  EXPECT_EQ(stream_size, directory[kDirectoryIndex].Location.DataSize);
   *user_stream_location = directory[kDirectoryIndex].Location;
 }
 
-TEST(MinidumpUserStreamWriter, NoData) {
+constexpr MinidumpStreamType kTestStreamId =
+    static_cast<MinidumpStreamType>(0x123456);
+
+TEST(MinidumpUserStreamWriter, InitializeFromSnapshotNoData) {
   MinidumpFileWriter minidump_file_writer;
   auto user_stream_writer = base::WrapUnique(new MinidumpUserStreamWriter());
-  const uint32_t kTestStreamId = 0x123456;
   auto stream =
       base::WrapUnique(new UserMinidumpStream(kTestStreamId, nullptr));
   user_stream_writer->InitializeFromSnapshot(stream.get());
@@ -67,14 +71,29 @@ TEST(MinidumpUserStreamWriter, NoData) {
 
   MINIDUMP_LOCATION_DESCRIPTOR user_stream_location;
   ASSERT_NO_FATAL_FAILURE(GetUserStream(
-      string_file.string(), &user_stream_location, kTestStreamId));
-  EXPECT_EQ(0u, user_stream_location.DataSize);
+      string_file.string(), &user_stream_location, kTestStreamId, 0u));
 }
 
-TEST(MinidumpUserStreamWriter, OneStream) {
+TEST(MinidumpUserStreamWriter, InitializeFromBufferNoData) {
+  MinidumpFileWriter minidump_file_writer;
+  auto user_stream_writer = base::WrapUnique(new MinidumpUserStreamWriter());
+  user_stream_writer->InitializeFromBuffer(kTestStreamId, nullptr, 0);
+  minidump_file_writer.AddStream(std::move(user_stream_writer));
+
+  StringFile string_file;
+  ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
+
+  ASSERT_EQ(sizeof(MINIDUMP_HEADER) + sizeof(MINIDUMP_DIRECTORY),
+            string_file.string().size());
+
+  MINIDUMP_LOCATION_DESCRIPTOR user_stream_location;
+  ASSERT_NO_FATAL_FAILURE(GetUserStream(
+      string_file.string(), &user_stream_location, kTestStreamId, 0u));
+}
+
+TEST(MinidumpUserStreamWriter, InitializeFromSnapshotOneStream) {
   MinidumpFileWriter minidump_file_writer;
   auto user_stream_writer = base::WrapUnique(new MinidumpUserStreamWriter());
-  const uint32_t kTestStreamId = 0x123456;
 
   TestMemorySnapshot* test_data = new TestMemorySnapshot();
   test_data->SetAddress(97865);
@@ -92,10 +111,33 @@ TEST(MinidumpUserStreamWriter, OneStream) {
   ASSERT_EQ(sizeof(MINIDUMP_HEADER) + sizeof(MINIDUMP_DIRECTORY) + kStreamSize,
             string_file.string().size());
 
-  MINIDUMP_LOCATION_DESCRIPTOR user_stream_location;
+  MINIDUMP_LOCATION_DESCRIPTOR user_stream_location = {};
   ASSERT_NO_FATAL_FAILURE(GetUserStream(
-      string_file.string(), &user_stream_location, kTestStreamId));
-  EXPECT_EQ(kStreamSize, user_stream_location.DataSize);
+      string_file.string(), &user_stream_location, kTestStreamId, kStreamSize));
+  const std::string stream_data = string_file.string().substr(
+      user_stream_location.Rva, user_stream_location.DataSize);
+  EXPECT_EQ(std::string(kStreamSize, 'c'), stream_data);
+}
+
+TEST(MinidumpUserStreamWriter, InitializeFromBufferOneStream) {
+  MinidumpFileWriter minidump_file_writer;
+  auto user_stream_writer = base::WrapUnique(new MinidumpUserStreamWriter());
+
+  const size_t kStreamSize = 128;
+  std::vector<uint8_t> data(kStreamSize, 'c');
+  user_stream_writer->InitializeFromBuffer(
+      kTestStreamId, &data[0], data.size());
+  minidump_file_writer.AddStream(std::move(user_stream_writer));
+
+  StringFile string_file;
+  ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file));
+
+  ASSERT_EQ(sizeof(MINIDUMP_HEADER) + sizeof(MINIDUMP_DIRECTORY) + kStreamSize,
+            string_file.string().size());
+
+  MINIDUMP_LOCATION_DESCRIPTOR user_stream_location = {};
+  ASSERT_NO_FATAL_FAILURE(GetUserStream(
+      string_file.string(), &user_stream_location, kTestStreamId, kStreamSize));
   const std::string stream_data = string_file.string().substr(
       user_stream_location.Rva, user_stream_location.DataSize);
   EXPECT_EQ(std::string(kStreamSize, 'c'), stream_data);