From 7217cc0a8f26e1301d0756b9e452a6c08d72b531 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Fri, 26 Feb 2016 14:42:47 -0800 Subject: [PATCH] Support client-specified extra memory ranges Change-Id: I378e2513a4894fb1548445b660bb3db86e281572 Reviewed-on: https://chromium-review.googlesource.com/329564 Reviewed-by: Robert Sesek Reviewed-by: Mark Mentovai --- client/client.gyp | 2 + client/client_test.gyp | 1 + client/crashpad_info.cc | 2 +- client/crashpad_info.h | 18 ++ client/simple_address_range_bag.cc | 45 ++++ client/simple_address_range_bag.h | 193 ++++++++++++++++++ client/simple_address_range_bag_test.cc | 109 ++++++++++ client/simple_string_dictionary.h | 27 --- client/simple_string_dictionary_test.cc | 28 --- handler/win/crashy_test_program.cc | 38 ++++ snapshot/mac/module_snapshot_mac.cc | 5 + snapshot/mac/module_snapshot_mac.h | 1 + snapshot/minidump/module_snapshot_minidump.cc | 6 + snapshot/minidump/module_snapshot_minidump.h | 1 + snapshot/module_snapshot.h | 6 + snapshot/snapshot_test.gyp | 13 ++ snapshot/test/test_module_snapshot.cc | 7 +- snapshot/test/test_module_snapshot.h | 6 + ...shpad_snapshot_test_extra_memory_ranges.cc | 54 +++++ snapshot/win/end_to_end_test.py | 22 ++ snapshot/win/extra_memory_ranges_test.cc | 133 ++++++++++++ snapshot/win/module_snapshot_win.cc | 40 ++++ snapshot/win/module_snapshot_win.h | 5 + snapshot/win/pe_image_reader.h | 1 + snapshot/win/process_snapshot_win.cc | 9 +- util/numeric/checked_range.h | 5 + 26 files changed, 719 insertions(+), 58 deletions(-) create mode 100644 client/simple_address_range_bag.cc create mode 100644 client/simple_address_range_bag.h create mode 100644 client/simple_address_range_bag_test.cc create mode 100644 snapshot/win/crashpad_snapshot_test_extra_memory_ranges.cc create mode 100644 snapshot/win/extra_memory_ranges_test.cc diff --git a/client/client.gyp b/client/client.gyp index 3524be41..9892fb6c 100644 --- a/client/client.gyp +++ b/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', diff --git a/client/client_test.gyp b/client/client_test.gyp index 80cddc84..66ec59ea 100644 --- a/client/client_test.gyp +++ b/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', ], diff --git a/client/crashpad_info.cc b/client/crashpad_info.cc index 46e73210..5913be81 100644 --- a/client/crashpad_info.cc +++ b/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 diff --git a/client/crashpad_info.h b/client/crashpad_info.h index 377d194e..4dd61abe 100644 --- a/client/crashpad_info.h +++ b/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) diff --git a/client/simple_address_range_bag.cc b/client/simple_address_range_bag.cc new file mode 100644 index 00000000..50ecfbc4 --- /dev/null +++ b/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 +#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::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 diff --git a/client/simple_address_range_bag.h b/client/simple_address_range_bag.h new file mode 100644 index 00000000..336a748d --- /dev/null +++ b/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 + +#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 +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 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( + base::checked_cast(reinterpret_cast(base)), + base::checked_cast(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 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( + base::checked_cast(reinterpret_cast(base)), + base::checked_cast(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_ diff --git a/client/simple_address_range_bag_test.cc b/client/simple_address_range_bag_test.cc new file mode 100644 index 00000000..53cca5f0 --- /dev/null +++ b/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(0x1000), 200); + entry = TestBag::Iterator(bag).Next(); + ASSERT_TRUE(entry); + EXPECT_EQ(entry->base, 0x1000); + EXPECT_EQ(entry->size, 200); + + bag.Remove(reinterpret_cast(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(0x1000), 10)); + EXPECT_TRUE(bag.Insert(reinterpret_cast(0x2000), 20)); + EXPECT_TRUE(bag.Insert(CheckedRange(0x3000, 30))); + + EXPECT_EQ(bag.GetCount(), 3u); + + // Duplicates added too. + EXPECT_TRUE(bag.Insert(CheckedRange(0x3000, 30))); + EXPECT_TRUE(bag.Insert(CheckedRange(0x3000, 30))); + EXPECT_EQ(bag.GetCount(), 5u); + + // Can be removed 3 times, but not the 4th time. + EXPECT_TRUE(bag.Remove(CheckedRange(0x3000, 30))); + EXPECT_TRUE(bag.Remove(CheckedRange(0x3000, 30))); + EXPECT_TRUE(bag.Remove(CheckedRange(0x3000, 30))); + EXPECT_EQ(bag.GetCount(), 2); + EXPECT_FALSE(bag.Remove(CheckedRange(0x3000, 30))); + EXPECT_EQ(bag.GetCount(), 2); + + EXPECT_TRUE(bag.Remove(reinterpret_cast(0x1000), 10)); + EXPECT_TRUE(bag.Remove(reinterpret_cast(0x2000), 20)); + EXPECT_EQ(bag.GetCount(), 0); +} + +TEST(SimpleAddressRangeBag, CopyAndAssign) { + TSimpleAddressRangeBag<10> bag; + EXPECT_TRUE(bag.Insert(CheckedRange(1, 2))); + EXPECT_TRUE(bag.Insert(CheckedRange(3, 4))); + EXPECT_TRUE(bag.Insert(CheckedRange(5, 6))); + EXPECT_TRUE(bag.Remove(CheckedRange(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(1, 2))); + EXPECT_TRUE(bag_copy.Remove(CheckedRange(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(1, 2))); + EXPECT_TRUE(bag_assign.Remove(CheckedRange(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(1, 2))); + EXPECT_TRUE(bag.Insert(CheckedRange(3, 4))); + EXPECT_FALSE(bag.Insert(CheckedRange(5, 6))); + EXPECT_EQ(2u, bag.GetCount()); + EXPECT_FALSE(bag.Remove(CheckedRange(5, 6))); +} + +} // namespace +} // namespace test +} // namespace crashpad diff --git a/client/simple_string_dictionary.h b/client/simple_string_dictionary.h index 2fb19bba..ea142fcd 100644 --- a/client/simple_string_dictionary.h +++ b/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(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(entries_); - return sizeof(entries_); - } - private: const Entry* GetConstEntryForKey(const char* key) const { for (size_t i = 0; i < num_entries; ++i) { diff --git a/client/simple_string_dictionary_test.cc b/client/simple_string_dictionary_test.cc index 6396c2fe..69e892b4 100644 --- a/client/simple_string_dictionary_test.cc +++ b/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(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; diff --git a/handler/win/crashy_test_program.cc b/handler/win/crashy_test_program.cc index 51ff0129..a3331398 100644 --- a/handler/win/crashy_test_program.cc +++ b/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. diff --git a/snapshot/mac/module_snapshot_mac.cc b/snapshot/mac/module_snapshot_mac.cc index dd9e8097..4f4df2a5 100644 --- a/snapshot/mac/module_snapshot_mac.cc +++ b/snapshot/mac/module_snapshot_mac.cc @@ -182,5 +182,10 @@ std::map ModuleSnapshotMac::AnnotationsSimpleMap() return annotations_reader.SimpleMap(); } +std::set> ModuleSnapshotMac::ExtraMemoryRanges() const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + return std::set>(); +} + } // namespace internal } // namespace crashpad diff --git a/snapshot/mac/module_snapshot_mac.h b/snapshot/mac/module_snapshot_mac.h index b1a232fd..0c6bd69a 100644 --- a/snapshot/mac/module_snapshot_mac.h +++ b/snapshot/mac/module_snapshot_mac.h @@ -79,6 +79,7 @@ class ModuleSnapshotMac final : public ModuleSnapshot { std::string DebugFileName() const override; std::vector AnnotationsVector() const override; std::map AnnotationsSimpleMap() const override; + std::set> ExtraMemoryRanges() const override; private: std::string name_; diff --git a/snapshot/minidump/module_snapshot_minidump.cc b/snapshot/minidump/module_snapshot_minidump.cc index 973d3b27..c83de596 100644 --- a/snapshot/minidump/module_snapshot_minidump.cc +++ b/snapshot/minidump/module_snapshot_minidump.cc @@ -135,6 +135,12 @@ ModuleSnapshotMinidump::AnnotationsSimpleMap() const { return annotations_simple_map_; } +std::set> ModuleSnapshotMinidump::ExtraMemoryRanges() + const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + return std::set>(); +} + bool ModuleSnapshotMinidump::InitializeModuleCrashpadInfo( FileReaderInterface* file_reader, const MINIDUMP_LOCATION_DESCRIPTOR* diff --git a/snapshot/minidump/module_snapshot_minidump.h b/snapshot/minidump/module_snapshot_minidump.h index b9b7d559..dc375f03 100644 --- a/snapshot/minidump/module_snapshot_minidump.h +++ b/snapshot/minidump/module_snapshot_minidump.h @@ -76,6 +76,7 @@ class ModuleSnapshotMinidump final : public ModuleSnapshot { std::string DebugFileName() const override; std::vector AnnotationsVector() const override; std::map AnnotationsSimpleMap() const override; + std::set> ExtraMemoryRanges() const override; private: // Initializes data carried in a MinidumpModuleCrashpadInfo structure on diff --git a/snapshot/module_snapshot.h b/snapshot/module_snapshot.h index a1bafd92..3a1c3216 100644 --- a/snapshot/module_snapshot.h +++ b/snapshot/module_snapshot.h @@ -19,10 +19,12 @@ #include #include +#include #include #include #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 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> ExtraMemoryRanges() const = 0; }; } // namespace crashpad diff --git a/snapshot/snapshot_test.gyp b/snapshot/snapshot_test.gyp index 9bfcf8b3..4ca9f4d5 100644 --- a/snapshot/snapshot_test.gyp +++ b/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', diff --git a/snapshot/test/test_module_snapshot.cc b/snapshot/test/test_module_snapshot.cc index cfe9a1de..c853a780 100644 --- a/snapshot/test/test_module_snapshot.cc +++ b/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 TestModuleSnapshot::AnnotationsSimpleMap() return annotations_simple_map_; } +std::set> TestModuleSnapshot::ExtraMemoryRanges() const { + return extra_memory_ranges_; +} + } // namespace test } // namespace crashpad diff --git a/snapshot/test/test_module_snapshot.h b/snapshot/test/test_module_snapshot.h index ff9bdbd5..bbe6193d 100644 --- a/snapshot/test/test_module_snapshot.h +++ b/snapshot/test/test_module_snapshot.h @@ -75,6 +75,10 @@ class TestModuleSnapshot final : public ModuleSnapshot { const std::map& annotations_simple_map) { annotations_simple_map_ = annotations_simple_map; } + void SetExtraMemoryRanges( + const std::set>& 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 AnnotationsVector() const override; std::map AnnotationsSimpleMap() const override; + std::set> ExtraMemoryRanges() const override; private: std::string name_; @@ -109,6 +114,7 @@ class TestModuleSnapshot final : public ModuleSnapshot { std::string debug_file_name_; std::vector annotations_vector_; std::map annotations_simple_map_; + std::set> extra_memory_ranges_; DISALLOW_COPY_AND_ASSIGN(TestModuleSnapshot); }; diff --git a/snapshot/win/crashpad_snapshot_test_extra_memory_ranges.cc b/snapshot/win/crashpad_snapshot_test_extra_memory_ranges.cc new file mode 100644 index 00000000..58b145e5 --- /dev/null +++ b/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 + +#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(0, 1)); + extra_ranges->Insert(CheckedRange(1, 0)); + extra_ranges->Insert(CheckedRange(0x1000000000ULL, 0x1000)); + extra_ranges->Insert(CheckedRange(0x2000, 0x2000000000ULL)); + extra_ranges->Insert(CheckedRange(1234, 5678)); + extra_ranges->Insert(CheckedRange(1234, 5678)); + extra_ranges->Insert(CheckedRange(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; +} diff --git a/snapshot/win/end_to_end_test.py b/snapshot/win/end_to_end_test.py index d4f2da02..404d6d3a 100755 --- a/snapshot/win/end_to_end_test.py +++ b/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', diff --git a/snapshot/win/extra_memory_ranges_test.cc b/snapshot/win/extra_memory_ranges_test.cc new file mode 100644 index 00000000..b623eccf --- /dev/null +++ b/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 +#include + +#include + +#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> 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(0, 1))); + EXPECT_NE(all_ranges.end(), all_ranges.find(CheckedRange(1, 0))); + EXPECT_NE(all_ranges.end(), + all_ranges.find(CheckedRange(1234, 5678))); + EXPECT_NE(all_ranges.end(), + all_ranges.find(CheckedRange(0x1000000000ULL, 0x1000))); + EXPECT_NE(all_ranges.end(), + all_ranges.find(CheckedRange(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 diff --git a/snapshot/win/module_snapshot_win.cc b/snapshot/win/module_snapshot_win.cc index 8948b2ab..b86e2029 100644 --- a/snapshot/win/module_snapshot_win.cc +++ b/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 ModuleSnapshotWin::AnnotationsSimpleMap() return annotations_reader.SimpleMap(); } +std::set> ModuleSnapshotWin::ExtraMemoryRanges() const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + std::set> ranges; + if (process_reader_->Is64Bit()) + GetCrashpadExtraMemoryRanges(&ranges); + else + GetCrashpadExtraMemoryRanges(&ranges); + return ranges; +} + template void ModuleSnapshotWin::GetCrashpadOptionsInternal( CrashpadInfoClientOptions* options) { @@ -222,5 +233,34 @@ const VS_FIXEDFILEINFO* ModuleSnapshotWin::VSFixedFileInfo() const { : nullptr; } +template +void ModuleSnapshotWin::GetCrashpadExtraMemoryRanges( + std::set>* ranges) const { + process_types::CrashpadInfo crashpad_info; + if (!pe_image_reader_->GetCrashpadInfo(&crashpad_info)) + return; + + if (!crashpad_info.extra_address_ranges) + return; + + std::vector 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(entry.base, entry.size)); + } + } +} + } // namespace internal } // namespace crashpad diff --git a/snapshot/win/module_snapshot_win.h b/snapshot/win/module_snapshot_win.h index 99327de5..85aeb123 100644 --- a/snapshot/win/module_snapshot_win.h +++ b/snapshot/win/module_snapshot_win.h @@ -85,11 +85,16 @@ class ModuleSnapshotWin final : public ModuleSnapshot { std::string DebugFileName() const override; std::vector AnnotationsVector() const override; std::map AnnotationsSimpleMap() const override; + std::set> ExtraMemoryRanges() const override; private: template void GetCrashpadOptionsInternal(CrashpadInfoClientOptions* options); + template + void GetCrashpadExtraMemoryRanges( + std::set>* 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. diff --git a/snapshot/win/pe_image_reader.h b/snapshot/win/pe_image_reader.h index feffa8a8..96529bf8 100644 --- a/snapshot/win/pe_image_reader.h +++ b/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; }; diff --git a/snapshot/win/process_snapshot_win.cc b/snapshot/win/process_snapshot_win.cc index 9c72855f..41c39027 100644 --- a/snapshot/win/process_snapshot_win.cc +++ b/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; } } diff --git a/util/numeric/checked_range.h b/util/numeric/checked_range.h index 69fe9df8..09dfc227 100644 --- a/util/numeric/checked_range.h +++ b/util/numeric/checked_range.h @@ -16,6 +16,7 @@ #define CRASHPAD_UTIL_NUMERIC_CHECKED_RANGE_H_ #include +#include #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_;