Support client-specified extra memory ranges

Change-Id: I378e2513a4894fb1548445b660bb3db86e281572
Reviewed-on: https://chromium-review.googlesource.com/329564
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>

client/client.gyp

@@ -46,6 +46,8 @@
         'settings.h',
         'simple_string_dictionary.cc',
         'simple_string_dictionary.h',
+        'simple_address_range_bag.cc',
+        'simple_address_range_bag.h',
         'simulate_crash.h',
         'simulate_crash_mac.cc',
         'simulate_crash_mac.h',

client/client_test.gyp

@@ -39,6 +39,7 @@
         'crashpad_client_win_test.cc',
         'prune_crash_reports_test.cc',
         'settings_test.cc',
+        'simple_address_range_bag_test.cc',
         'simple_string_dictionary_test.cc',
         'simulate_crash_mac_test.cc',
       ],

client/crashpad_info.cc

@@ -100,13 +100,13 @@ CrashpadInfo::CrashpadInfo()
       system_crash_reporter_forwarding_(TriState::kUnset),
       gather_indirectly_referenced_memory_(TriState::kUnset),
       padding_0_(0),
+      extra_memory_ranges_(nullptr),
       simple_annotations_(nullptr)
 #if !defined(NDEBUG) && defined(OS_WIN)
       ,
       invalid_read_detection_(0xbadc0de)
 #endif
 {
-
 }
 
 }  // namespace crashpad

client/crashpad_info.h

@@ -19,6 +19,7 @@
 
 #include "base/macros.h"
 #include "build/build_config.h"
+#include "client/simple_address_range_bag.h"
 #include "client/simple_string_dictionary.h"
 #include "util/misc/tri_state.h"
 
@@ -42,6 +43,22 @@ struct CrashpadInfo {
 
   CrashpadInfo();
 
+  //! \brief Sets the bag of extra memory ranges to be included in the snapshot.
+  //!
+  //! Extra memory ranges may exist in \a address_range_bag at the time that
+  //! this method is called, or they may be added, removed, or modified in \a
+  //! address_range_bag after this method is called.
+  //!
+  //! TODO(scottmg) This is currently only supported on Windows.
+  //!
+  //! \param[in] address_range_bag A bag of address ranges. The CrashpadInfo
+  //!     object does not take ownership of the SimpleAddressRangeBag object.
+  //!     It is the caller’s responsibility to ensure that this pointer remains
+  //!     valid while it is in effect for a CrashpadInfo object.
+  void set_extra_memory_ranges(SimpleAddressRangeBag* address_range_bag) {
+    extra_memory_ranges_ = address_range_bag;
+  }
+
   //! \brief Sets the simple annotations dictionary.
   //!
   //! Simple annotations set on a CrashpadInfo structure are interpreted by
@@ -135,6 +152,7 @@ struct CrashpadInfo {
   TriState system_crash_reporter_forwarding_;
   TriState gather_indirectly_referenced_memory_;
   uint8_t padding_0_;
+  SimpleAddressRangeBag* extra_memory_ranges_;  // weak
   SimpleStringDictionary* simple_annotations_;  // weak
 
 #if !defined(NDEBUG) && defined(OS_WIN)

client/simple_address_range_bag.cc

@@ -0,0 +1,45 @@
+// Copyright 2016 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 "client/simple_address_range_bag.h"
+
+#include "util/stdlib/cxx.h"
+
+#if CXX_LIBRARY_VERSION >= 2011
+#include <type_traits>
+#endif
+
+namespace crashpad {
+namespace {
+
+using SimpleAddressRangeBagForAssertion = TSimpleAddressRangeBag<1>;
+
+#if CXX_LIBRARY_VERSION >= 2011
+// In C++11, check that TSimpleAddressRangeBag has standard layout, which is
+// what is actually important.
+static_assert(
+    std::is_standard_layout<SimpleAddressRangeBagForAssertion>::value,
+    "SimpleAddressRangeBag must be standard layout");
+#else
+// In C++98 (ISO 14882), section 9.5.1 says that a union cannot have a member
+// with a non-trivial ctor, copy ctor, dtor, or assignment operator. Use this
+// property to ensure that Entry remains POD. This doesn’t work for C++11
+// because the requirements for unions have been relaxed.
+union Compile_Assert {
+  SimpleAddressRangeBagForAssertion::Entry Compile_Assert__entry_must_be_pod;
+};
+#endif
+
+}  // namespace
+}  // namespace crashpad

client/simple_address_range_bag.h

@@ -0,0 +1,193 @@
+// Copyright 2016 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_CLIENT_SIMPLE_ADDRESS_RANGE_BAG_H_
+#define CRASHPAD_CLIENT_SIMPLE_ADDRESS_RANGE_BAG_H_
+
+#include <stdint.h>
+
+#include "base/logging.h"
+#include "base/macros.h"
+#include "util/numeric/checked_range.h"
+
+namespace crashpad {
+
+//! \brief A bag implementation using a fixed amount of storage, so that it does
+//!     not perform any dynamic allocations for its operations.
+//!
+//! The actual bag storage (TSimpleAddressRangeBag::Entry) is POD, so that it
+//! can be transmitted over various IPC mechanisms.
+template <size_t NumEntries = 64>
+class TSimpleAddressRangeBag {
+ public:
+  //! Constant and publicly accessible version of the template parameter.
+  static const size_t num_entries = NumEntries;
+
+  //! \brief A single entry in the bag.
+  struct Entry {
+    //! \brief The base address of the range.
+    uint64_t base;
+
+    //! \brief The size of the range in bytes.
+    uint64_t size;
+
+    //! \brief Returns the validity of the entry.
+    //!
+    //! If #base and #size are both zero, the entry is considered inactive, and
+    //! this method returns `false`. Otherwise, returns `true`.
+    bool is_active() const {
+      return base != 0 || size != 0;
+    }
+  };
+
+  //! \brief An iterator to traverse all of the active entries in a
+  //!     TSimpleAddressRangeBag.
+  class Iterator {
+   public:
+    explicit Iterator(const TSimpleAddressRangeBag& bag)
+        : bag_(bag),
+          current_(0) {
+    }
+
+    //! \brief Returns the next entry in the bag, or `nullptr` if at the end of
+    //!     the collection.
+    const Entry* Next() {
+      while (current_ < bag_.num_entries) {
+        const Entry* entry = &bag_.entries_[current_++];
+        if (entry->is_active()) {
+          return entry;
+        }
+      }
+      return nullptr;
+    }
+
+   private:
+    const TSimpleAddressRangeBag& bag_;
+    size_t current_;
+
+    DISALLOW_COPY_AND_ASSIGN(Iterator);
+  };
+
+  TSimpleAddressRangeBag()
+      : entries_() {
+  }
+
+  TSimpleAddressRangeBag(const TSimpleAddressRangeBag& other) {
+    *this = other;
+  }
+
+  TSimpleAddressRangeBag& operator=(const TSimpleAddressRangeBag& other) {
+    memcpy(entries_, other.entries_, sizeof(entries_));
+    return *this;
+  }
+
+  //! \brief Returns the number of active entries. The upper limit for this is
+  //!     \a NumEntries.
+  size_t GetCount() const {
+    size_t count = 0;
+    for (size_t i = 0; i < num_entries; ++i) {
+      if (entries_[i].is_active()) {
+        ++count;
+      }
+    }
+    return count;
+  }
+
+  //! \brief Inserts the given range into the bag. Duplicates and overlapping
+  //! ranges are supported and allowed, but not coalesced.
+  //!
+  //! \param[in] range The range to be inserted. The range must have either a
+  //!     non-zero base address or size.
+  //!
+  //! \return `true` if there was space to insert the range into the bag,
+  //!     otherwise `false` with an error logged.
+  bool Insert(CheckedRange<uint64_t> range) {
+    DCHECK(range.base() != 0 || range.size() != 0);
+
+    for (size_t i = 0; i < num_entries; ++i) {
+      if (!entries_[i].is_active()) {
+        entries_[i].base = range.base();
+        entries_[i].size = range.size();
+        return true;
+      }
+    }
+
+    LOG(ERROR) << "no space available to insert range";
+    return false;
+  }
+
+  //! \brief Inserts the given range into the bag. Duplicates and overlapping
+  //! ranges are supported and allowed, but not coalesced.
+  //!
+  //! \param[in] base The base of the range to be inserted. May not be null.
+  //! \param[in] size The size of the range to be inserted. May not be zero.
+  //!
+  //! \return `true` if there was space to insert the range into the bag,
+  //!     otherwise `false` with an error logged.
+  bool Insert(void* base, size_t size) {
+    DCHECK(base != nullptr);
+    DCHECK_NE(0u, size);
+    return Insert(CheckedRange<uint64_t>(
+        base::checked_cast<uint64_t>(reinterpret_cast<uintptr_t>(base)),
+        base::checked_cast<uint64_t>(size)));
+  }
+
+  //! \brief Removes the given range from the bag.
+  //!
+  //! \param[in] range The range to be removed. The range must have either a
+  //!     non-zero base address or size.
+  //!
+  //! \return `true` if the range was found and removed, otherwise `false` with
+  //!     an error logged.
+  bool Remove(CheckedRange<uint64_t> range) {
+    DCHECK(range.base() != 0 || range.size() != 0);
+
+    for (size_t i = 0; i < num_entries; ++i) {
+      if (entries_[i].base == range.base() &&
+          entries_[i].size == range.size()) {
+        entries_[i].base = entries_[i].size = 0;
+        return true;
+      }
+    }
+
+    LOG(ERROR) << "did not find range to remove";
+    return false;
+  }
+
+  //! \brief Removes the given range from the bag.
+  //!
+  //! \param[in] base The base of the range to be removed. May not be null.
+  //! \param[in] size The size of the range to be removed. May not be zero.
+  //!
+  //! \return `true` if the range was found and removed, otherwise `false` with
+  //! an error logged.
+  bool Remove(void* base, size_t size) {
+    DCHECK(base != nullptr);
+    DCHECK_NE(0u, size);
+    return Remove(CheckedRange<uint64_t>(
+        base::checked_cast<uint64_t>(reinterpret_cast<uintptr_t>(base)),
+        base::checked_cast<uint64_t>(size)));
+  }
+
+
+ private:
+  Entry entries_[NumEntries];
+};
+
+//! \brief A TSimpleAddressRangeBag with default template parameters.
+using SimpleAddressRangeBag = TSimpleAddressRangeBag<64>;
+
+}  // namespace crashpad
+
+#endif  // CRASHPAD_CLIENT_SIMPLE_ADDRESS_RANGE_BAG_H_

client/simple_address_range_bag_test.cc

@@ -0,0 +1,109 @@
+// Copyright 2016 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 "client/simple_address_range_bag.h"
+
+#include "base/logging.h"
+#include "gtest/gtest.h"
+#include "test/gtest_death_check.h"
+
+namespace crashpad {
+namespace test {
+namespace {
+
+TEST(SimpleAddressRangeBag, Entry) {
+  using TestBag = TSimpleAddressRangeBag<15>;
+  TestBag bag;
+
+  const TestBag::Entry* entry = TestBag::Iterator(bag).Next();
+  EXPECT_FALSE(entry);
+
+  bag.Insert(reinterpret_cast<void*>(0x1000), 200);
+  entry = TestBag::Iterator(bag).Next();
+  ASSERT_TRUE(entry);
+  EXPECT_EQ(entry->base, 0x1000);
+  EXPECT_EQ(entry->size, 200);
+
+  bag.Remove(reinterpret_cast<void*>(0x1000), 200);
+  EXPECT_FALSE(entry->is_active());
+  EXPECT_EQ(entry->base, 0);
+  EXPECT_EQ(entry->size, 0);
+}
+
+TEST(SimpleAddressRangeBag, SimpleAddressRangeBag) {
+  SimpleAddressRangeBag bag;
+
+  EXPECT_TRUE(bag.Insert(reinterpret_cast<void*>(0x1000), 10));
+  EXPECT_TRUE(bag.Insert(reinterpret_cast<void*>(0x2000), 20));
+  EXPECT_TRUE(bag.Insert(CheckedRange<uint64_t>(0x3000, 30)));
+
+  EXPECT_EQ(bag.GetCount(), 3u);
+
+  // Duplicates added too.
+  EXPECT_TRUE(bag.Insert(CheckedRange<uint64_t>(0x3000, 30)));
+  EXPECT_TRUE(bag.Insert(CheckedRange<uint64_t>(0x3000, 30)));
+  EXPECT_EQ(bag.GetCount(), 5u);
+
+  // Can be removed 3 times, but not the 4th time.
+  EXPECT_TRUE(bag.Remove(CheckedRange<uint64_t>(0x3000, 30)));
+  EXPECT_TRUE(bag.Remove(CheckedRange<uint64_t>(0x3000, 30)));
+  EXPECT_TRUE(bag.Remove(CheckedRange<uint64_t>(0x3000, 30)));
+  EXPECT_EQ(bag.GetCount(), 2);
+  EXPECT_FALSE(bag.Remove(CheckedRange<uint64_t>(0x3000, 30)));
+  EXPECT_EQ(bag.GetCount(), 2);
+
+  EXPECT_TRUE(bag.Remove(reinterpret_cast<void*>(0x1000), 10));
+  EXPECT_TRUE(bag.Remove(reinterpret_cast<void*>(0x2000), 20));
+  EXPECT_EQ(bag.GetCount(), 0);
+}
+
+TEST(SimpleAddressRangeBag, CopyAndAssign) {
+  TSimpleAddressRangeBag<10> bag;
+  EXPECT_TRUE(bag.Insert(CheckedRange<uint64_t>(1, 2)));
+  EXPECT_TRUE(bag.Insert(CheckedRange<uint64_t>(3, 4)));
+  EXPECT_TRUE(bag.Insert(CheckedRange<uint64_t>(5, 6)));
+  EXPECT_TRUE(bag.Remove(CheckedRange<uint64_t>(3, 4)));
+  EXPECT_EQ(2u, bag.GetCount());
+
+  // Test copy.
+  TSimpleAddressRangeBag<10> bag_copy(bag);
+  EXPECT_EQ(2u, bag_copy.GetCount());
+  EXPECT_TRUE(bag_copy.Remove(CheckedRange<uint64_t>(1, 2)));
+  EXPECT_TRUE(bag_copy.Remove(CheckedRange<uint64_t>(5, 6)));
+  EXPECT_EQ(0u, bag_copy.GetCount());
+  EXPECT_EQ(2u, bag.GetCount());
+
+  // Test assign.
+  TSimpleAddressRangeBag<10> bag_assign;
+  bag_assign = bag;
+  EXPECT_EQ(2u, bag_assign.GetCount());
+  EXPECT_TRUE(bag_assign.Remove(CheckedRange<uint64_t>(1, 2)));
+  EXPECT_TRUE(bag_assign.Remove(CheckedRange<uint64_t>(5, 6)));
+  EXPECT_EQ(0u, bag_assign.GetCount());
+  EXPECT_EQ(2u, bag.GetCount());
+}
+
+// Running out of space shouldn't crash.
+TEST(SimpleAddressRangeBag, OutOfSpace) {
+  TSimpleAddressRangeBag<2> bag;
+  EXPECT_TRUE(bag.Insert(CheckedRange<uint64_t>(1, 2)));
+  EXPECT_TRUE(bag.Insert(CheckedRange<uint64_t>(3, 4)));
+  EXPECT_FALSE(bag.Insert(CheckedRange<uint64_t>(5, 6)));
+  EXPECT_EQ(2u, bag.GetCount());
+  EXPECT_FALSE(bag.Remove(CheckedRange<uint64_t>(5, 6)));
+}
+
+}  // namespace
+}  // namespace test
+}  // namespace crashpad

client/simple_string_dictionary.h

@@ -24,11 +24,6 @@
 
 namespace crashpad {
 
-// Opaque type for the serialized representation of a TSimpleStringDictionary.
-// One is created in TSimpleStringDictionary::Serialize and can be deserialized
-// using one of the constructors.
-struct SerializedSimpleStringDictionary;
-
 //! \brief A map/dictionary collection implementation using a fixed amount of
 //!     storage, so that it does not perform any dynamic allocations for its
 //!     operations.
@@ -112,16 +107,6 @@ class TSimpleStringDictionary {
     return *this;
   }
 
-  //! \brief Constructs a map from its serialized form. \a map should be the out
-  //!     parameter from Serialize(), and \a size should be its return value.
-  TSimpleStringDictionary(
-      const SerializedSimpleStringDictionary* map, size_t size) {
-    DCHECK_EQ(size, sizeof(entries_));
-    if (size == sizeof(entries_)) {
-      memcpy(entries_, map, size);
-    }
-  }
-
   //! \brief Returns the number of active key/value pairs. The upper limit for
   //!     this is \a NumEntries.
   size_t GetCount() const {
@@ -236,18 +221,6 @@ class TSimpleStringDictionary {
     DCHECK_EQ(GetEntryForKey(key), implicit_cast<Entry*>(nullptr));
   }
 
-  //! \brief Returns a serialized form of the map.
-  //!
-  //! Places a serialized version of the map into \a map and returns the size in
-  //! bytes. Both \a map and the size should be passed to the deserializing
-  //! constructor. Note that the serialized \a map is scoped to the lifetime of
-  //! the non-serialized instance of this class. The \a map data can be copied
-  //! across IPC boundaries.
-  size_t Serialize(const SerializedSimpleStringDictionary** map) const {
-    *map = reinterpret_cast<const SerializedSimpleStringDictionary*>(entries_);
-    return sizeof(entries_);
-  }
-
  private:
   const Entry* GetConstEntryForKey(const char* key) const {
     for (size_t i = 0; i < num_entries; ++i) {

client/simple_string_dictionary_test.cc

@@ -240,34 +240,6 @@ TEST(SimpleStringDictionary, AddRemove) {
   EXPECT_FALSE(map.GetValueForKey("mark"));
 }
 
-TEST(SimpleStringDictionary, Serialize) {
-  using TestMap = TSimpleStringDictionary<4, 5, 7>;
-  TestMap map;
-  map.SetKeyValue("one", "abc");
-  map.SetKeyValue("two", "def");
-  map.SetKeyValue("tre", "hig");
-
-  EXPECT_STREQ("abc", map.GetValueForKey("one"));
-  EXPECT_STREQ("def", map.GetValueForKey("two"));
-  EXPECT_STREQ("hig", map.GetValueForKey("tre"));
-
-  const SerializedSimpleStringDictionary* serialized;
-  size_t size = map.Serialize(&serialized);
-
-  SerializedSimpleStringDictionary* serialized_copy =
-      reinterpret_cast<SerializedSimpleStringDictionary*>(malloc(size));
-  ASSERT_TRUE(serialized_copy);
-  memcpy(serialized_copy, serialized, size);
-
-  TestMap deserialized(serialized_copy, size);
-  free(serialized_copy);
-
-  EXPECT_EQ(3u, deserialized.GetCount());
-  EXPECT_STREQ("abc", deserialized.GetValueForKey("one"));
-  EXPECT_STREQ("def", deserialized.GetValueForKey("two"));
-  EXPECT_STREQ("hig", deserialized.GetValueForKey("tre"));
-}
-
 // Running out of space shouldn't crash.
 TEST(SimpleStringDictionary, OutOfSpace) {
   TSimpleStringDictionary<3, 2, 2> map;

handler/win/crashy_test_program.cc

@@ -38,6 +38,10 @@
 #endif
 
 namespace crashpad {
+
+int* g_extra_memory_pointer;
+int* g_extra_memory_not_saved;
+
 namespace {
 
 CRITICAL_SECTION g_test_critical_section;
@@ -136,6 +140,33 @@ void SomeCrashyFunction() {
   *foo = 42;
 }
 
+void AllocateExtraMemoryToBeSaved(
+    crashpad::SimpleAddressRangeBag* extra_ranges) {
+  const size_t kNumInts = 2000;
+  int* extra_memory = new int[kNumInts];
+  g_extra_memory_pointer = extra_memory;
+  for (int i = 0; i < kNumInts; ++i)
+    extra_memory[i] = i * 13 + 2;
+  extra_ranges->Insert(extra_memory, sizeof(extra_memory[0]) * kNumInts);
+  extra_ranges->Insert(&g_extra_memory_pointer, sizeof(g_extra_memory_pointer));
+}
+
+void AllocateExtraUnsavedMemory(crashpad::SimpleAddressRangeBag* extra_ranges) {
+  // Allocate some extra memory, and then Insert() but also Remove() it so we
+  // can confirm it doesn't get saved.
+  const size_t kNumInts = 2000;
+  int* extra_memory = new int[kNumInts];
+  g_extra_memory_not_saved = extra_memory;
+  for (int i = 0; i < kNumInts; ++i)
+    extra_memory[i] = i * 17 + 7;
+  extra_ranges->Insert(extra_memory, sizeof(extra_memory[0]) * kNumInts);
+  extra_ranges->Insert(&g_extra_memory_not_saved,
+                       sizeof(g_extra_memory_not_saved));
+
+  // We keep the pointer's memory, but remove the pointed-to memory.
+  extra_ranges->Remove(extra_memory, sizeof(extra_memory[0]) * kNumInts);
+}
+
 int CrashyMain(int argc, wchar_t* argv[]) {
   CrashpadClient client;
 
@@ -165,6 +196,13 @@ int CrashyMain(int argc, wchar_t* argv[]) {
     return EXIT_FAILURE;
   }
 
+  crashpad::SimpleAddressRangeBag* extra_ranges =
+      new crashpad::SimpleAddressRangeBag();
+  crashpad::CrashpadInfo::GetCrashpadInfo()->set_extra_memory_ranges(
+      extra_ranges);
+  AllocateExtraMemoryToBeSaved(extra_ranges);
+  AllocateExtraUnsavedMemory(extra_ranges);
+
   // Load and unload some uncommonly used modules so we can see them in the list
   // reported by `lm`. At least two so that we confirm we got the size of
   // RTL_UNLOAD_EVENT_TRACE right.

snapshot/mac/module_snapshot_mac.cc

@@ -182,5 +182,10 @@ std::map<std::string, std::string> ModuleSnapshotMac::AnnotationsSimpleMap()
   return annotations_reader.SimpleMap();
 }
 
+std::set<CheckedRange<uint64_t>> ModuleSnapshotMac::ExtraMemoryRanges() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return std::set<CheckedRange<uint64_t>>();
+}
+
 }  // namespace internal
 }  // namespace crashpad

snapshot/mac/module_snapshot_mac.h

@@ -79,6 +79,7 @@ class ModuleSnapshotMac final : public ModuleSnapshot {
   std::string DebugFileName() const override;
   std::vector<std::string> AnnotationsVector() const override;
   std::map<std::string, std::string> AnnotationsSimpleMap() const override;
+  std::set<CheckedRange<uint64_t>> ExtraMemoryRanges() const override;
 
  private:
   std::string name_;

snapshot/minidump/module_snapshot_minidump.cc

@@ -135,6 +135,12 @@ ModuleSnapshotMinidump::AnnotationsSimpleMap() const {
   return annotations_simple_map_;
 }
 
+std::set<CheckedRange<uint64_t>> ModuleSnapshotMinidump::ExtraMemoryRanges()
+    const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return std::set<CheckedRange<uint64_t>>();
+}
+
 bool ModuleSnapshotMinidump::InitializeModuleCrashpadInfo(
     FileReaderInterface* file_reader,
     const MINIDUMP_LOCATION_DESCRIPTOR*

snapshot/minidump/module_snapshot_minidump.h

@@ -76,6 +76,7 @@ class ModuleSnapshotMinidump final : public ModuleSnapshot {
   std::string DebugFileName() const override;
   std::vector<std::string> AnnotationsVector() const override;
   std::map<std::string, std::string> AnnotationsSimpleMap() const override;
+  std::set<CheckedRange<uint64_t>> ExtraMemoryRanges() const override;
 
  private:
   // Initializes data carried in a MinidumpModuleCrashpadInfo structure on

snapshot/module_snapshot.h

@@ -19,10 +19,12 @@
 #include <sys/types.h>
 
 #include <map>
+#include <set>
 #include <string>
 #include <vector>
 
 #include "util/misc/uuid.h"
+#include "util/numeric/checked_range.h"
 
 namespace crashpad {
 
@@ -168,6 +170,10 @@ class ModuleSnapshot {
   //! system, or snapshot producer may be obtained by calling
   //! ProcessSnapshot::AnnotationsSimpleMap().
   virtual std::map<std::string, std::string> AnnotationsSimpleMap() const = 0;
+
+  //! \brief Returns a set of extra memory ranges specified in the module as
+  //!     being desirable to include in the crash dump.
+  virtual std::set<CheckedRange<uint64_t>> ExtraMemoryRanges() const = 0;
 };
 
 }  // namespace crashpad

snapshot/snapshot_test.gyp

@@ -80,6 +80,7 @@
         'minidump/process_snapshot_minidump_test.cc',
         'win/cpu_context_win_test.cc',
         'win/exception_snapshot_win_test.cc',
+        'win/extra_memory_ranges_test.cc',
         'win/pe_image_annotations_reader_test.cc',
         'win/pe_image_reader_test.cc',
         'win/process_reader_win_test.cc',
@@ -170,6 +171,18 @@
             'win/crashpad_snapshot_test_dump_without_crashing.cc',
           ],
         },
+        {
+          'target_name': 'crashpad_snapshot_test_extra_memory_ranges',
+          'type': 'executable',
+          'dependencies': [
+            '../client/client.gyp:crashpad_client',
+            '../compat/compat.gyp:crashpad_compat',
+            '../third_party/mini_chromium/mini_chromium.gyp:base',
+          ],
+          'sources': [
+            'win/crashpad_snapshot_test_extra_memory_ranges.cc',
+          ],
+        },
         {
           'target_name': 'crashpad_snapshot_test_image_reader',
           'type': 'executable',

snapshot/test/test_module_snapshot.cc

@@ -29,7 +29,8 @@ TestModuleSnapshot::TestModuleSnapshot()
       uuid_(),
       debug_file_name_(),
       annotations_vector_(),
-      annotations_simple_map_() {
+      annotations_simple_map_(),
+      extra_memory_ranges_() {
 }
 
 TestModuleSnapshot::~TestModuleSnapshot() {
@@ -93,5 +94,9 @@ std::map<std::string, std::string> TestModuleSnapshot::AnnotationsSimpleMap()
   return annotations_simple_map_;
 }
 
+std::set<CheckedRange<uint64_t>> TestModuleSnapshot::ExtraMemoryRanges() const {
+  return extra_memory_ranges_;
+}
+
 }  // namespace test
 }  // namespace crashpad

snapshot/test/test_module_snapshot.h

@@ -75,6 +75,10 @@ class TestModuleSnapshot final : public ModuleSnapshot {
       const std::map<std::string, std::string>& annotations_simple_map) {
     annotations_simple_map_ = annotations_simple_map;
   }
+  void SetExtraMemoryRanges(
+      const std::set<CheckedRange<uint64_t>>& extra_memory_ranges) {
+    extra_memory_ranges_ = extra_memory_ranges;
+  }
 
   // ModuleSnapshot:
 
@@ -95,6 +99,7 @@ class TestModuleSnapshot final : public ModuleSnapshot {
   std::string DebugFileName() const override;
   std::vector<std::string> AnnotationsVector() const override;
   std::map<std::string, std::string> AnnotationsSimpleMap() const override;
+  std::set<CheckedRange<uint64_t>> ExtraMemoryRanges() const override;
 
  private:
   std::string name_;
@@ -109,6 +114,7 @@ class TestModuleSnapshot final : public ModuleSnapshot {
   std::string debug_file_name_;
   std::vector<std::string> annotations_vector_;
   std::map<std::string, std::string> annotations_simple_map_;
+  std::set<CheckedRange<uint64_t>> extra_memory_ranges_;
 
   DISALLOW_COPY_AND_ASSIGN(TestModuleSnapshot);
 };

snapshot/win/crashpad_snapshot_test_extra_memory_ranges.cc

@@ -0,0 +1,54 @@
+// Copyright 2016 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 <windows.h>
+
+#include "base/logging.h"
+#include "client/crashpad_info.h"
+#include "util/file/file_io.h"
+
+
+int wmain(int argc, wchar_t* argv[]) {
+  using namespace crashpad;
+
+  CrashpadInfo* crashpad_info = CrashpadInfo::GetCrashpadInfo();
+
+  // This is "leaked" to crashpad_info.
+  SimpleAddressRangeBag* extra_ranges = new SimpleAddressRangeBag();
+  extra_ranges->Insert(CheckedRange<uint64_t>(0, 1));
+  extra_ranges->Insert(CheckedRange<uint64_t>(1, 0));
+  extra_ranges->Insert(CheckedRange<uint64_t>(0x1000000000ULL, 0x1000));
+  extra_ranges->Insert(CheckedRange<uint64_t>(0x2000, 0x2000000000ULL));
+  extra_ranges->Insert(CheckedRange<uint64_t>(1234, 5678));
+  extra_ranges->Insert(CheckedRange<uint64_t>(1234, 5678));
+  extra_ranges->Insert(CheckedRange<uint64_t>(1234, 5678));
+  crashpad_info->set_extra_memory_ranges(extra_ranges);
+
+  // Tell the parent that the environment has been set up.
+  HANDLE out = GetStdHandle(STD_OUTPUT_HANDLE);
+  PCHECK(out != INVALID_HANDLE_VALUE) << "GetStdHandle";
+  char c = ' ';
+  CheckedWriteFile(out, &c, sizeof(c));
+
+  HANDLE in = GetStdHandle(STD_INPUT_HANDLE);
+  PCHECK(in != INVALID_HANDLE_VALUE) << "GetStdHandle";
+  CheckedReadFile(in, &c, sizeof(c));
+  CHECK(c == 'd' || c == ' ');
+
+  // If 'd' we crash with a debug break, otherwise exit normally.
+  if (c == 'd')
+    __debugbreak();
+
+  return 0;
+}

snapshot/win/end_to_end_test.py

@@ -23,6 +23,7 @@ import sys
 import tempfile
 import time
 
+
 g_temp_dirs = []
 
 
@@ -268,6 +269,27 @@ def RunTests(cdb_path,
             r'80000094 00000095 80000096 00000097',
             'data pointed to by stack captured')
 
+  # Attempt to retrieve the value of g_extra_memory_pointer (by name), and then
+  # examine the memory at which it points. Both should have been saved.
+  out = CdbRun(cdb_path, dump_path,
+               'dd poi(crashy_program!crashpad::g_extra_memory_pointer)+0x1f30 '
+               'L8')
+  out.Check(r'0000655e 0000656b 00006578 00006585',
+            'extra memory range captured')
+  out.Check(r'\?\?\?\?\?\?\?\? \?\?\?\?\?\?\?\? '
+            r'\?\?\?\?\?\?\?\? \?\?\?\?\?\?\?\?',
+            '  and not memory after range')
+
+  out = CdbRun(cdb_path, dump_path,
+               'dd poi(crashy_program!crashpad::g_extra_memory_not_saved)'
+               '+0x1f30 L4')
+  # We save only the pointer, not the pointed-to data. If the pointer itself
+  # wasn't saved, then we won't get any memory printed, so here we're confirming
+  # the pointer was saved but the memory wasn't.
+  out.Check(r'\?\?\?\?\?\?\?\? \?\?\?\?\?\?\?\? '
+            r'\?\?\?\?\?\?\?\? \?\?\?\?\?\?\?\?',
+            'extra memory removal')
+
   if z7_dump_path:
     out = CdbRun(cdb_path, z7_dump_path, '.ecxr;lm')
     out.Check('This dump file has an exception of interest stored in it',

snapshot/win/extra_memory_ranges_test.cc

@@ -0,0 +1,133 @@
+// Copyright 2016 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/win/module_snapshot_win.h"
+
+#include <stdlib.h>
+#include <string.h>
+
+#include <string>
+
+#include "base/files/file_path.h"
+#include "build/build_config.h"
+#include "client/crashpad_info.h"
+#include "client/simple_address_range_bag.h"
+#include "gtest/gtest.h"
+#include "snapshot/win/process_snapshot_win.h"
+#include "test/paths.h"
+#include "test/win/child_launcher.h"
+#include "util/file/file_io.h"
+#include "util/win/process_info.h"
+
+namespace crashpad {
+namespace test {
+namespace {
+
+enum TestType {
+  // Don't crash, just test the CrashpadInfo interface.
+  kDontCrash = 0,
+
+  // The child process should crash by __debugbreak().
+  kCrashDebugBreak,
+};
+
+void TestExtraMemoryRanges(TestType type,
+                           const base::string16& directory_modification) {
+  // Spawn a child process, passing it the pipe name to connect to.
+  base::FilePath test_executable = Paths::Executable();
+  std::wstring child_test_executable =
+      test_executable.DirName()
+          .Append(directory_modification)
+          .Append(test_executable.BaseName().RemoveFinalExtension().value() +
+                  L"_extra_memory_ranges.exe")
+          .value();
+  ChildLauncher child(child_test_executable, L"");
+  child.Start();
+
+  // Wait for the child process to indicate that it's done setting up its
+  // annotations via the CrashpadInfo interface.
+  char c;
+  CheckedReadFile(child.stdout_read_handle(), &c, sizeof(c));
+
+  ProcessSnapshotWin snapshot;
+  ASSERT_TRUE(snapshot.Initialize(
+      child.process_handle(), ProcessSuspensionState::kRunning, 0));
+
+  // Verify the extra memory ranges set via the CrashpadInfo interface.
+  std::set<CheckedRange<uint64_t>> all_ranges;
+  for (const auto* module : snapshot.Modules()) {
+    for (const auto& range : module->ExtraMemoryRanges())
+      all_ranges.insert(range);
+  }
+
+  EXPECT_EQ(5u, all_ranges.size());
+  EXPECT_NE(all_ranges.end(), all_ranges.find(CheckedRange<uint64_t>(0, 1)));
+  EXPECT_NE(all_ranges.end(), all_ranges.find(CheckedRange<uint64_t>(1, 0)));
+  EXPECT_NE(all_ranges.end(),
+            all_ranges.find(CheckedRange<uint64_t>(1234, 5678)));
+  EXPECT_NE(all_ranges.end(),
+            all_ranges.find(CheckedRange<uint64_t>(0x1000000000ULL, 0x1000)));
+  EXPECT_NE(all_ranges.end(),
+            all_ranges.find(CheckedRange<uint64_t>(0x2000, 0x2000000000ULL)));
+
+  // Tell the child process to continue.
+  DWORD expected_exit_code;
+  switch (type) {
+    case kDontCrash:
+      c = ' ';
+      expected_exit_code = 0;
+      break;
+    case kCrashDebugBreak:
+      c = 'd';
+      expected_exit_code = STATUS_BREAKPOINT;
+      break;
+    default:
+      FAIL();
+  }
+  CheckedWriteFile(child.stdin_write_handle(), &c, sizeof(c));
+
+  EXPECT_EQ(expected_exit_code, child.WaitForExit());
+}
+
+TEST(ExtraMemoryRanges, DontCrash) {
+  TestExtraMemoryRanges(kDontCrash, FILE_PATH_LITERAL("."));
+}
+
+TEST(ExtraMemoryRanges, CrashDebugBreak) {
+  TestExtraMemoryRanges(kCrashDebugBreak, FILE_PATH_LITERAL("."));
+}
+
+#if defined(ARCH_CPU_64_BITS)
+TEST(ExtraMemoryRanges, DontCrashWOW64) {
+#ifndef NDEBUG
+  TestExtraMemoryRanges(kDontCrash, FILE_PATH_LITERAL("..\\..\\out\\Debug"));
+#else
+  TestExtraMemoryRanges(kDontCrash, FILE_PATH_LITERAL("..\\..\\out\\Release"));
+#endif
+}
+
+TEST(ExtraMemoryRanges, CrashDebugBreakWOW64) {
+#ifndef NDEBUG
+  TestExtraMemoryRanges(kCrashDebugBreak,
+                        FILE_PATH_LITERAL("..\\..\\out\\Debug"));
+#else
+  TestExtraMemoryRanges(kCrashDebugBreak,
+                        FILE_PATH_LITERAL("..\\..\\out\\Release"));
+#endif
+}
+#endif  // ARCH_CPU_64_BITS
+
+}  // namespace
+}  // namespace test
+}  // namespace crashpad

snapshot/win/module_snapshot_win.cc

@@ -15,6 +15,7 @@
 #include "snapshot/win/module_snapshot_win.h"
 
 #include "base/strings/utf_string_conversions.h"
+#include "client/simple_address_range_bag.h"
 #include "snapshot/win/pe_image_annotations_reader.h"
 #include "snapshot/win/pe_image_reader.h"
 #include "util/misc/tri_state.h"
@@ -185,6 +186,16 @@ std::map<std::string, std::string> ModuleSnapshotWin::AnnotationsSimpleMap()
   return annotations_reader.SimpleMap();
 }
 
+std::set<CheckedRange<uint64_t>> ModuleSnapshotWin::ExtraMemoryRanges() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  std::set<CheckedRange<uint64_t>> ranges;
+  if (process_reader_->Is64Bit())
+    GetCrashpadExtraMemoryRanges<process_types::internal::Traits64>(&ranges);
+  else
+    GetCrashpadExtraMemoryRanges<process_types::internal::Traits32>(&ranges);
+  return ranges;
+}
+
 template <class Traits>
 void ModuleSnapshotWin::GetCrashpadOptionsInternal(
     CrashpadInfoClientOptions* options) {
@@ -222,5 +233,34 @@ const VS_FIXEDFILEINFO* ModuleSnapshotWin::VSFixedFileInfo() const {
                                                     : nullptr;
 }
 
+template <class Traits>
+void ModuleSnapshotWin::GetCrashpadExtraMemoryRanges(
+    std::set<CheckedRange<uint64_t>>* ranges) const {
+  process_types::CrashpadInfo<Traits> crashpad_info;
+  if (!pe_image_reader_->GetCrashpadInfo(&crashpad_info))
+    return;
+
+  if (!crashpad_info.extra_address_ranges)
+    return;
+
+  std::vector<SimpleAddressRangeBag::Entry> simple_ranges(
+      SimpleAddressRangeBag::num_entries);
+  if (!process_reader_->ReadMemory(
+          crashpad_info.extra_address_ranges,
+          simple_ranges.size() * sizeof(simple_ranges[0]),
+          &simple_ranges[0])) {
+    LOG(WARNING) << "could not read simple address_ranges from "
+                 << base::UTF16ToUTF8(name_);
+    return;
+  }
+
+  for (const auto& entry : simple_ranges) {
+    if (entry.base != 0 || entry.size != 0) {
+      // Deduplication here is fine.
+      ranges->insert(CheckedRange<uint64_t>(entry.base, entry.size));
+    }
+  }
+}
+
 }  // namespace internal
 }  // namespace crashpad

snapshot/win/module_snapshot_win.h

@@ -85,11 +85,16 @@ class ModuleSnapshotWin final : public ModuleSnapshot {
   std::string DebugFileName() const override;
   std::vector<std::string> AnnotationsVector() const override;
   std::map<std::string, std::string> AnnotationsSimpleMap() const override;
+  std::set<CheckedRange<uint64_t>> ExtraMemoryRanges() const override;
 
  private:
   template <class Traits>
   void GetCrashpadOptionsInternal(CrashpadInfoClientOptions* options);
 
+  template <class Traits>
+  void GetCrashpadExtraMemoryRanges(
+      std::set<CheckedRange<uint64_t>>* ranges) const;
+
   // Initializes vs_fixed_file_info_ if it has not yet been initialized, and
   // returns a pointer to it. Returns nullptr on failure, with a message logged
   // on the first call.

snapshot/win/pe_image_reader.h

@@ -43,6 +43,7 @@ struct CrashpadInfo {
   uint8_t system_crash_reporter_forwarding;  // TriState.
   uint8_t gather_indirectly_referenced_memory;  // TriState.
   uint8_t padding_0;
+  typename Traits::Pointer extra_address_ranges;
   typename Traits::Pointer simple_annotations;
 };
 

snapshot/win/process_snapshot_win.cc

@@ -80,6 +80,12 @@ bool ProcessSnapshotWin::Initialize(
     memory_map_.push_back(new internal::MemoryMapRegionSnapshotWin(mbi));
   }
 
+  for (const auto& module : modules_) {
+    for (const auto& range : module->ExtraMemoryRanges()) {
+      AddMemorySnapshot(range.base(), range.size(), &extra_memory_);
+    }
+  }
+
   INITIALIZATION_STATE_SET_VALID(initialized_);
   return true;
 }
@@ -321,7 +327,8 @@ void ProcessSnapshotWin::GetCrashpadOptionsInternal(
     // If non-default values have been found for all options, the loop can end
     // early.
     if (local_options.crashpad_handler_behavior != TriState::kUnset &&
-        local_options.system_crash_reporter_forwarding != TriState::kUnset) {
+        local_options.system_crash_reporter_forwarding != TriState::kUnset &&
+        local_options.gather_indirectly_referenced_memory != TriState::kUnset) {
       break;
     }
   }

util/numeric/checked_range.h

@@ -16,6 +16,7 @@
 #define CRASHPAD_UTIL_NUMERIC_CHECKED_RANGE_H_
 
 #include <limits>
+#include <tuple>
 
 #include "base/logging.h"
 #include "base/numerics/safe_conversions.h"
@@ -125,6 +126,10 @@ class CheckedRange {
     return base() < that.end() && that.base() < end();
   }
 
+  bool operator<(const CheckedRange& other) const {
+    return std::tie(base_, size_) < std::tie(other.base_, other.size_);
+  }
+
  private:
   ValueType base_;
   SizeType size_;