Add MinidumpMemoryListWriter::AddFromSnapshot(), everything downstream,

and its test.

TEST=minidump_test MinidumpMemoryWriter.AddFromSnapshot
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/641603006

minidump/minidump_memory_writer.cc

@@ -14,15 +14,76 @@
 
 #include "minidump/minidump_memory_writer.h"
 
+#include "base/auto_reset.h"
 #include "base/logging.h"
+#include "snapshot/memory_snapshot.h"
 #include "util/file/file_writer.h"
 #include "util/numeric/safe_assignment.h"
 
 namespace crashpad {
+namespace {
+
+class SnapshotMinidumpMemoryWriter final : public MinidumpMemoryWriter,
+                                           public MemorySnapshot::Delegate {
+ public:
+  explicit SnapshotMinidumpMemoryWriter(const MemorySnapshot* memory_snapshot)
+      : MinidumpMemoryWriter(),
+        MemorySnapshot::Delegate(),
+        memory_snapshot_(memory_snapshot),
+        file_writer_(nullptr) {
+  }
+
+  ~SnapshotMinidumpMemoryWriter() override {}
+
+  // MemorySnapshot::Delegate:
+
+  bool MemorySnapshotDelegateRead(void* data, size_t size) override {
+    DCHECK_EQ(state(), kStateWritable);
+    DCHECK_EQ(size, MemoryRangeSize());
+    return file_writer_->Write(data, size);
+  }
+
+ protected:
+  // MinidumpMemoryWriter:
+
+  bool WriteObject(FileWriterInterface* file_writer) override {
+    DCHECK_EQ(state(), kStateWritable);
+    DCHECK(!file_writer_);
+
+    base::AutoReset<FileWriterInterface*> file_writer_reset(&file_writer_,
+                                                            file_writer);
+
+    // This will result in MemorySnapshotDelegateRead() being called.
+    return memory_snapshot_->Read(this);
+  }
+
+  uint64_t MemoryRangeBaseAddress() const override {
+    DCHECK_EQ(state(), kStateFrozen);
+    return memory_snapshot_->Address();
+  }
+
+  size_t MemoryRangeSize() const override {
+    DCHECK_GE(state(), kStateFrozen);
+    return memory_snapshot_->Size();
+  }
+
+ private:
+  const MemorySnapshot* memory_snapshot_;
+  FileWriterInterface* file_writer_;
+
+  DISALLOW_COPY_AND_ASSIGN(SnapshotMinidumpMemoryWriter);
+};
+
+}  // namespace
 
 MinidumpMemoryWriter::~MinidumpMemoryWriter() {
 }
 
+scoped_ptr<MinidumpMemoryWriter> MinidumpMemoryWriter::CreateFromSnapshot(
+    const MemorySnapshot* memory_snapshot) {
+  return make_scoped_ptr(new SnapshotMinidumpMemoryWriter(memory_snapshot));
+}
+
 const MINIDUMP_MEMORY_DESCRIPTOR*
 MinidumpMemoryWriter::MinidumpMemoryDescriptor() const {
   DCHECK_EQ(state(), kStateWritable);
@@ -109,6 +170,17 @@ MinidumpMemoryListWriter::MinidumpMemoryListWriter()
 MinidumpMemoryListWriter::~MinidumpMemoryListWriter() {
 }
 
+void MinidumpMemoryListWriter::AddFromSnapshot(
+    const std::vector<const MemorySnapshot*>& memory_snapshots) {
+  DCHECK_EQ(state(), kStateMutable);
+
+  for (const MemorySnapshot* memory_snapshot : memory_snapshots) {
+    scoped_ptr<MinidumpMemoryWriter> memory =
+        MinidumpMemoryWriter::CreateFromSnapshot(memory_snapshot);
+    AddMemory(memory.Pass());
+  }
+}
+
 void MinidumpMemoryListWriter::AddMemory(
     scoped_ptr<MinidumpMemoryWriter> memory_writer) {
   DCHECK_EQ(state(), kStateMutable);

minidump/minidump_memory_writer.h

@@ -29,6 +29,8 @@
 
 namespace crashpad {
 
+class MemorySnapshot;
+
 //! \brief The base class for writers of memory ranges pointed to by
 //!     MINIDUMP_MEMORY_DESCRIPTOR objects in a minidump file.
 //!
@@ -41,6 +43,16 @@ class MinidumpMemoryWriter : public internal::MinidumpWritable {
  public:
   ~MinidumpMemoryWriter() override;
 
+  //! \brief Creates a concrete initialized MinidumpMemoryWriter based on \a
+  //!     memory_snapshot.
+  //!
+  //! \param[in] memory_snapshot The memory snapshot to use as source data.
+  //!
+  //! \return An object of a MinidumpMemoryWriter subclass initialized using the
+  //!     source data in \a memory_snapshot.
+  static scoped_ptr<MinidumpMemoryWriter> CreateFromSnapshot(
+      const MemorySnapshot* memory_snapshot);
+
   //! \brief Returns a MINIDUMP_MEMORY_DESCRIPTOR referencing the data that this
   //!     object writes.
   //!
@@ -121,6 +133,17 @@ class MinidumpMemoryListWriter final : public internal::MinidumpStreamWriter {
   MinidumpMemoryListWriter();
   ~MinidumpMemoryListWriter() override;
 
+  //! \brief Adds a concrete initialized MinidumpMemoryWriter for each memory
+  //!     snapshot in \a memory_snapshots to the MINIDUMP_MEMORY_LIST.
+  //!
+  //! Memory snapshots are added in the fashion of AddMemory().
+  //!
+  //! \param[in] memory_snapshots The memory snapshots to use as source data.
+  //!
+  //! \note Valid in #kStateMutable.
+  void AddFromSnapshot(
+      const std::vector<const MemorySnapshot*>& memory_snapshots);
+
   //! \brief Adds a MinidumpMemoryWriter to the MINIDUMP_MEMORY_LIST.
   //!
   //! This object takes ownership of \a memory_writer and becomes its parent in

minidump/minidump_memory_writer_test.cc

@@ -18,6 +18,7 @@
 #include <stdint.h>
 
 #include "base/basictypes.h"
+#include "base/strings/stringprintf.h"
 #include "gtest/gtest.h"
 #include "minidump/minidump_extensions.h"
 #include "minidump/minidump_file_writer.h"
@@ -25,7 +26,9 @@
 #include "minidump/test/minidump_file_writer_test_util.h"
 #include "minidump/test/minidump_memory_writer_test_util.h"
 #include "minidump/test/minidump_writable_test_util.h"
+#include "snapshot/test/test_memory_snapshot.h"
 #include "util/file/string_file_writer.h"
+#include "util/stdlib/pointer_container.h"
 
 namespace crashpad {
 namespace test {
@@ -235,7 +238,7 @@ TEST(MinidumpMemoryWriter, ExtraMemory) {
   MinidumpFileWriter minidump_file_writer;
 
   const uint64_t kBaseAddress0 = 0x1000;
-  const uint64_t kSize0 = 0x0400;
+  const size_t kSize0 = 0x0400;
   const uint8_t kValue0 = '1';
   auto test_memory_stream =
       make_scoped_ptr(new TestMemoryStream(kBaseAddress0, kSize0, kValue0));
@@ -246,7 +249,7 @@ TEST(MinidumpMemoryWriter, ExtraMemory) {
   minidump_file_writer.AddStream(test_memory_stream.Pass());
 
   const uint64_t kBaseAddress1 = 0x2000;
-  const uint64_t kSize1 = 0x0400;
+  const size_t kSize1 = 0x0400;
   const uint8_t kValue1 = 'm';
 
   auto memory_writer = make_scoped_ptr(
@@ -297,6 +300,62 @@ TEST(MinidumpMemoryWriter, ExtraMemory) {
   }
 }
 
+TEST(MinidumpMemoryWriter, AddFromSnapshot) {
+  MINIDUMP_MEMORY_DESCRIPTOR expect_memory_descriptors[3] = {};
+  uint8_t values[arraysize(expect_memory_descriptors)] = {};
+
+  expect_memory_descriptors[0].StartOfMemoryRange = 0;
+  expect_memory_descriptors[0].Memory.DataSize = 0x1000;
+  values[0] = 0x01;
+
+  expect_memory_descriptors[1].StartOfMemoryRange = 0x1000;
+  expect_memory_descriptors[1].Memory.DataSize = 0x2000;
+  values[1] = 0xf4;
+
+  expect_memory_descriptors[2].StartOfMemoryRange = 0x7654321000000000;
+  expect_memory_descriptors[2].Memory.DataSize = 0x800;
+  values[2] = 0xa9;
+
+  PointerVector<TestMemorySnapshot> memory_snapshots_owner;
+  std::vector<const MemorySnapshot*> memory_snapshots;
+  for (size_t index = 0;
+       index < arraysize(expect_memory_descriptors);
+       ++index) {
+    TestMemorySnapshot* memory_snapshot = new TestMemorySnapshot();
+    memory_snapshots_owner.push_back(memory_snapshot);
+    memory_snapshot->SetAddress(
+        expect_memory_descriptors[index].StartOfMemoryRange);
+    memory_snapshot->SetSize(expect_memory_descriptors[index].Memory.DataSize);
+    memory_snapshot->SetValue(values[index]);
+    memory_snapshots.push_back(memory_snapshot);
+  }
+
+  auto memory_list_writer = make_scoped_ptr(new MinidumpMemoryListWriter());
+  memory_list_writer->AddFromSnapshot(memory_snapshots);
+
+  MinidumpFileWriter minidump_file_writer;
+  minidump_file_writer.AddStream(memory_list_writer.Pass());
+
+  StringFileWriter file_writer;
+  ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer));
+
+  const MINIDUMP_MEMORY_LIST* memory_list;
+  ASSERT_NO_FATAL_FAILURE(
+      GetMemoryListStream(file_writer.string(), &memory_list, 1));
+
+  ASSERT_EQ(3u, memory_list->NumberOfMemoryRanges);
+
+  for (size_t index = 0; index < memory_list->NumberOfMemoryRanges; ++index) {
+    SCOPED_TRACE(base::StringPrintf("index %zu", index));
+    ExpectMinidumpMemoryDescriptorAndContents(
+        &expect_memory_descriptors[index],
+        &memory_list->MemoryRanges[index],
+        file_writer.string(),
+        values[index],
+        index == memory_list->NumberOfMemoryRanges - 1);
+  }
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace crashpad

minidump/minidump_module_crashpad_info_writer.h

@@ -113,9 +113,9 @@ class MinidumpModuleCrashpadInfoListWriter final
   //!
   //! \param[in] module_snapshots The module snapshots to use as source data.
   //!
-  //! \note Valid in #kStateMutable. AddModule() may not be called before this
-  //!     method, and it is not normally necessary to call AddModule() after
-  //!     this method.
+  //! \note Valid in #kStateMutable. No mutator methods may be called before
+  //!     this method, and it is not normally necessary to call any mutator
+  //!     methods after this method.
   void InitializeFromSnapshot(
       const std::vector<const ModuleSnapshot*>& module_snapshots);
 

minidump/minidump_module_writer.h

@@ -320,9 +320,9 @@ class MinidumpModuleListWriter final : public internal::MinidumpStreamWriter {
   //!
   //! \param[in] module_snapshots The module snapshots to use as source data.
   //!
-  //! \note Valid in #kStateMutable. AddModule() may not be called before this
-  //!     this method, and it is not normally necessary to call AddModule()
-  //!     after this method.
+  //! \note Valid in #kStateMutable. No mutator methods may be called before
+  //!     this method, and it is not normally necessary to call any mutator
+  //!     methods after this method.
   void InitializeFromSnapshot(
       const std::vector<const ModuleSnapshot*>& module_snapshots);
 

minidump/minidump_module_writer_test.cc

@@ -649,16 +649,9 @@ void InitializeTestModuleSnapshotFromMinidumpModule(
 
 TEST(MinidumpModuleWriter, InitializeFromSnapshot) {
   MINIDUMP_MODULE expect_modules[3] = {};
-  const char* module_paths[3] = {};
-  const char* module_names[3] = {};
-  UUID uuids[3] = {};
-
-  static_assert(arraysize(expect_modules) == arraysize(module_paths),
-                "array sizes must be equal");
-  static_assert(arraysize(expect_modules) == arraysize(module_names),
-                "array sizes must be equal");
-  static_assert(arraysize(expect_modules) == arraysize(uuids),
-                "array sizes must be equal");
+  const char* module_paths[arraysize(expect_modules)] = {};
+  const char* module_names[arraysize(expect_modules)] = {};
+  UUID uuids[arraysize(expect_modules)] = {};
 
   expect_modules[0].BaseOfImage = 0x100101000;
   expect_modules[0].SizeOfImage = 0xf000;

snapshot/mac/memory_snapshot_mac.cc

@@ -52,6 +52,11 @@ size_t MemorySnapshotMac::Size() const {
 
 bool MemorySnapshotMac::Read(Delegate* delegate) const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+
+  if (size_ == 0) {
+    return delegate->MemorySnapshotDelegateRead(nullptr, size_);
+  }
+
   scoped_ptr<uint8_t[]> buffer(new uint8_t[size_]);
   if (!process_reader_->Memory()->Read(address_, size_, buffer.get())) {
     return false;

snapshot/memory_snapshot.h

@@ -39,7 +39,8 @@ class MemorySnapshot {
     //!
     //! \param[in] data A pointer to the data that was read. The callee does not
     //!     take ownership of this data. This data is only valid for the
-    //!     duration of the call to this method.
+    //!     duration of the call to this method. This parameter may be `nullptr`
+    //!     if \a size is `0`.
     //! \param[in] size The size of the data that was read.
     //!
     //! \return `true` on success, `false` on failure. MemoryDelegate::Read()

snapshot/snapshot.gyp

@@ -86,6 +86,8 @@
         '..',
       ],
       'sources': [
+        'test/test_memory_snapshot.cc',
+        'test/test_memory_snapshot.h',
         'test/test_module_snapshot.cc',
         'test/test_module_snapshot.h',
       ],

snapshot/test/test_memory_snapshot.cc

@@ -0,0 +1,47 @@
+// Copyright 2014 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 "snapshot/test/test_memory_snapshot.h"
+
+#include <string>
+
+namespace crashpad {
+namespace test {
+
+TestMemorySnapshot::TestMemorySnapshot()
+    : address_(0), size_(0), value_('\0') {
+}
+
+TestMemorySnapshot::~TestMemorySnapshot() {
+}
+
+uint64_t TestMemorySnapshot::Address() const {
+  return address_;
+}
+
+size_t TestMemorySnapshot::Size() const {
+  return size_;
+}
+
+bool TestMemorySnapshot::Read(Delegate* delegate) const {
+  if (size_ == 0) {
+    return delegate->MemorySnapshotDelegateRead(nullptr, size_);
+  }
+
+  std::string buffer(size_, value_);
+  return delegate->MemorySnapshotDelegateRead(&buffer[0], size_);
+}
+
+}  // namespace test
+}  // namespace crashpad

snapshot/test/test_memory_snapshot.h

@@ -0,0 +1,60 @@
+// Copyright 2014 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_SNAPSHOT_TEST_TEST_MEMORY_SNAPSHOT_H_
+#define CRASHPAD_SNAPSHOT_TEST_TEST_MEMORY_SNAPSHOT_H_
+
+#include <stdint.h>
+#include <sys/types.h>
+
+#include "base/basictypes.h"
+#include "snapshot/memory_snapshot.h"
+
+namespace crashpad {
+namespace test {
+
+//! \brief A test MemorySnapshot that can carry arbitrary data for testing
+//!     purposes.
+class TestMemorySnapshot final : public MemorySnapshot {
+ public:
+  TestMemorySnapshot();
+  ~TestMemorySnapshot();
+
+  void SetAddress(uint64_t address) { address_ = address; }
+  void SetSize(size_t size) { size_ = size; }
+
+  //! \brief Sets the value to fill the test memory region with.
+  //!
+  //! \param[in] value The value to be written to \a delegate when Read() is
+  //!     called. This value will be repeated Size() times.
+  void SetValue(char value) { value_ = value; }
+
+  // MemorySnapshot:
+
+  uint64_t Address() const override;
+  size_t Size() const override;
+  bool Read(Delegate* delegate) const override;
+
+ private:
+  uint64_t address_;
+  size_t size_;
+  char value_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestMemorySnapshot);
+};
+
+}  // namespace test
+}  // namespace crashpad
+
+#endif  // CRASHPAD_SNAPSHOT_TEST_TEST_MEMORY_SNAPSHOT_H_