diff --git a/minidump/minidump_file_writer.cc b/minidump/minidump_file_writer.cc index 3d321c0b..33c4806c 100644 --- a/minidump/minidump_file_writer.cc +++ b/minidump/minidump_file_writer.cc @@ -93,6 +93,8 @@ void MinidumpFileWriter::InitializeFromSnapshot( auto crashpad_info = make_scoped_ptr(new MinidumpCrashpadInfoWriter()); crashpad_info->InitializeFromSnapshot(process_snapshot); + memory_list->AddFromSnapshot(process_snapshot->ExtraMemory()); + // Since the MinidumpCrashpadInfo stream is an extension, it’s safe to not add // it to the minidump file if it wouldn’t carry any useful information. if (crashpad_info->IsUseful()) { diff --git a/minidump/minidump_file_writer_test.cc b/minidump/minidump_file_writer_test.cc index 147d5c8a..44c39530 100644 --- a/minidump/minidump_file_writer_test.cc +++ b/minidump/minidump_file_writer_test.cc @@ -28,6 +28,7 @@ #include "minidump/test/minidump_writable_test_util.h" #include "snapshot/test/test_cpu_context.h" #include "snapshot/test/test_exception_snapshot.h" +#include "snapshot/test/test_memory_snapshot.h" #include "snapshot/test/test_module_snapshot.h" #include "snapshot/test/test_process_snapshot.h" #include "snapshot/test/test_system_snapshot.h" @@ -252,6 +253,14 @@ TEST(MinidumpFileWriter, InitializeFromSnapshot_Basic) { system_snapshot->SetOperatingSystem(SystemSnapshot::kOperatingSystemMacOSX); process_snapshot.SetSystem(system_snapshot.Pass()); + auto peb_snapshot = make_scoped_ptr(new TestMemorySnapshot()); + const uint64_t kPebAddress = 0x07f90000; + peb_snapshot->SetAddress(kPebAddress); + const size_t kPebSize = 0x280; + peb_snapshot->SetSize(kPebSize); + peb_snapshot->SetValue('p'); + process_snapshot.AddExtraMemory(peb_snapshot.Pass()); + MinidumpFileWriter minidump_file_writer; minidump_file_writer.InitializeFromSnapshot(&process_snapshot); @@ -283,6 +292,13 @@ TEST(MinidumpFileWriter, InitializeFromSnapshot_Basic) { EXPECT_EQ(kMinidumpStreamTypeMemoryList, directory[4].StreamType); EXPECT_TRUE(MinidumpWritableAtLocationDescriptor( string_file.string(), directory[4].Location)); + + const MINIDUMP_MEMORY_LIST* memory_list = + MinidumpWritableAtLocationDescriptor( + string_file.string(), directory[4].Location); + EXPECT_EQ(1u, memory_list->NumberOfMemoryRanges); + EXPECT_EQ(kPebAddress, memory_list->MemoryRanges[0].StartOfMemoryRange); + EXPECT_EQ(kPebSize, memory_list->MemoryRanges[0].Memory.DataSize); } TEST(MinidumpFileWriter, InitializeFromSnapshot_Exception) { diff --git a/snapshot/mac/process_snapshot_mac.cc b/snapshot/mac/process_snapshot_mac.cc index 8bbacded..7560317a 100644 --- a/snapshot/mac/process_snapshot_mac.cc +++ b/snapshot/mac/process_snapshot_mac.cc @@ -186,6 +186,11 @@ const ExceptionSnapshot* ProcessSnapshotMac::Exception() const { return exception_.get(); } +std::vector ProcessSnapshotMac::ExtraMemory() const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + return std::vector(); +} + void ProcessSnapshotMac::InitializeThreads() { const std::vector& process_reader_threads = process_reader_.Threads(); diff --git a/snapshot/mac/process_snapshot_mac.h b/snapshot/mac/process_snapshot_mac.h index c7447385..6fb726e0 100644 --- a/snapshot/mac/process_snapshot_mac.h +++ b/snapshot/mac/process_snapshot_mac.h @@ -126,6 +126,7 @@ class ProcessSnapshotMac final : public ProcessSnapshot { std::vector Threads() const override; std::vector Modules() const override; const ExceptionSnapshot* Exception() const override; + std::vector ExtraMemory() const override; private: // Initializes threads_ on behalf of Initialize(). diff --git a/snapshot/minidump/process_snapshot_minidump.cc b/snapshot/minidump/process_snapshot_minidump.cc index 5344118c..0ad0d144 100644 --- a/snapshot/minidump/process_snapshot_minidump.cc +++ b/snapshot/minidump/process_snapshot_minidump.cc @@ -177,6 +177,13 @@ const ExceptionSnapshot* ProcessSnapshotMinidump::Exception() const { return nullptr; } +std::vector ProcessSnapshotMinidump::ExtraMemory() + const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + NOTREACHED(); // https://code.google.com/p/crashpad/issues/detail?id=10 + return std::vector(); +} + bool ProcessSnapshotMinidump::InitializeCrashpadInfo() { const auto& stream_it = stream_map_.find(kMinidumpStreamTypeCrashpadInfo); if (stream_it == stream_map_.end()) { diff --git a/snapshot/minidump/process_snapshot_minidump.h b/snapshot/minidump/process_snapshot_minidump.h index 44ce5d56..6de64c96 100644 --- a/snapshot/minidump/process_snapshot_minidump.h +++ b/snapshot/minidump/process_snapshot_minidump.h @@ -27,6 +27,7 @@ #include "minidump/minidump_extensions.h" #include "snapshot/exception_snapshot.h" #include "snapshot/minidump/module_snapshot_minidump.h" +#include "snapshot/memory_snapshot.h" #include "snapshot/module_snapshot.h" #include "snapshot/process_snapshot.h" #include "snapshot/system_snapshot.h" @@ -68,6 +69,7 @@ class ProcessSnapshotMinidump final : public ProcessSnapshot { std::vector Threads() const override; std::vector Modules() const override; const ExceptionSnapshot* Exception() const override; + std::vector ExtraMemory() const override; private: // Initializes data carried in a MinidumpCrashpadInfo stream on behalf of diff --git a/snapshot/process_snapshot.h b/snapshot/process_snapshot.h index 42df05c9..f2fceef2 100644 --- a/snapshot/process_snapshot.h +++ b/snapshot/process_snapshot.h @@ -27,6 +27,7 @@ namespace crashpad { class ExceptionSnapshot; +class MemorySnapshot; class ModuleSnapshot; class SystemSnapshot; class ThreadSnapshot; @@ -160,6 +161,15 @@ class ProcessSnapshot { //! object that it was obtained from. If the snapshot is not a result of //! an exception, returns `nullptr`. virtual const ExceptionSnapshot* Exception() const = 0; + + //! \brief Returns a vector of additional memory blocks that should be + //! included in a minidump. + //! + //! \return An vector of MemorySnapshot objects that will be included in the + //! crash dump. The caller does not take ownership of these objects, they + //! are scoped to the lifetime of the ProcessSnapshot object that they + //! were obtained from. + virtual std::vector ExtraMemory() const = 0; }; } // namespace crashpad diff --git a/snapshot/test/test_process_snapshot.cc b/snapshot/test/test_process_snapshot.cc index c87d7a63..1d21212d 100644 --- a/snapshot/test/test_process_snapshot.cc +++ b/snapshot/test/test_process_snapshot.cc @@ -32,7 +32,8 @@ TestProcessSnapshot::TestProcessSnapshot() system_(), threads_(), modules_(), - exception_() { + exception_(), + extra_memory_() { } TestProcessSnapshot::~TestProcessSnapshot() { @@ -97,5 +98,12 @@ const ExceptionSnapshot* TestProcessSnapshot::Exception() const { return exception_.get(); } +std::vector TestProcessSnapshot::ExtraMemory() const { + std::vector extra_memory; + for (const auto& em : extra_memory_) + extra_memory.push_back(em); + return extra_memory; +} + } // namespace test } // namespace crashpad diff --git a/snapshot/test/test_process_snapshot.h b/snapshot/test/test_process_snapshot.h index 3d742f16..361559f5 100644 --- a/snapshot/test/test_process_snapshot.h +++ b/snapshot/test/test_process_snapshot.h @@ -26,6 +26,7 @@ #include "base/basictypes.h" #include "base/memory/scoped_ptr.h" #include "snapshot/exception_snapshot.h" +#include "snapshot/memory_snapshot.h" #include "snapshot/module_snapshot.h" #include "snapshot/process_snapshot.h" #include "snapshot/system_snapshot.h" @@ -95,6 +96,14 @@ class TestProcessSnapshot final : public ProcessSnapshot { exception_ = exception.Pass(); } + //! \brief Add a memory snapshot to be returned by ExtraMemory(). + //! + //! \param[in] peb The memory snapshot that will be included in ExtraMemory(). + //! The TestProcessSnapshot object takes ownership of \a extra_memory. + void AddExtraMemory(scoped_ptr extra_memory) { + extra_memory_.push_back(extra_memory.release()); + } + // ProcessSnapshot: pid_t ProcessID() const override; @@ -110,6 +119,7 @@ class TestProcessSnapshot final : public ProcessSnapshot { std::vector Threads() const override; std::vector Modules() const override; const ExceptionSnapshot* Exception() const override; + std::vector ExtraMemory() const override; private: pid_t process_id_; @@ -125,6 +135,7 @@ class TestProcessSnapshot final : public ProcessSnapshot { PointerVector threads_; PointerVector modules_; scoped_ptr exception_; + PointerVector extra_memory_; DISALLOW_COPY_AND_ASSIGN(TestProcessSnapshot); }; diff --git a/snapshot/win/process_reader_win.cc b/snapshot/win/process_reader_win.cc index 487bf6c6..bcdf4483 100644 --- a/snapshot/win/process_reader_win.cc +++ b/snapshot/win/process_reader_win.cc @@ -275,6 +275,11 @@ const std::vector& ProcessReaderWin::Modules() { return modules_; } +const ProcessInfo& ProcessReaderWin::GetProcessInfo() const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + return process_info_; +} + template void ProcessReaderWin::ReadThreadData(bool is_64_reading_32) { DCHECK(threads_.empty()); diff --git a/snapshot/win/process_reader_win.h b/snapshot/win/process_reader_win.h index 73ad1c61..934db724 100644 --- a/snapshot/win/process_reader_win.h +++ b/snapshot/win/process_reader_win.h @@ -82,9 +82,6 @@ class ProcessReaderWin { //! \return `true` if the target task is a 64-bit process. bool Is64Bit() const { return process_info_.Is64Bit(); } - pid_t ProcessID() const { return process_info_.ProcessID(); } - pid_t ParentProcessID() const { return process_info_.ParentProcessID(); } - bool ReadMemory(WinVMAddress at, WinVMSize num_bytes, void* into) const; //! \brief Determines the target process' start time. @@ -112,6 +109,9 @@ class ProcessReaderWin { //! `0`) corresponds to the main executable. const std::vector& Modules(); + //! \return A ProcessInfo object for the process being read. + const ProcessInfo& GetProcessInfo() const; + private: template void ReadThreadData(bool is_64_reading_32); diff --git a/snapshot/win/process_reader_win_test.cc b/snapshot/win/process_reader_win_test.cc index a87b9454..2c4660e2 100644 --- a/snapshot/win/process_reader_win_test.cc +++ b/snapshot/win/process_reader_win_test.cc @@ -38,7 +38,7 @@ TEST(ProcessReaderWin, SelfBasic) { EXPECT_TRUE(process_reader.Is64Bit()); #endif - EXPECT_EQ(GetCurrentProcessId(), process_reader.ProcessID()); + EXPECT_EQ(GetCurrentProcessId(), process_reader.GetProcessInfo().ProcessID()); const char kTestMemory[] = "Some test memory"; char buffer[arraysize(kTestMemory)]; diff --git a/snapshot/win/process_snapshot_win.cc b/snapshot/win/process_snapshot_win.cc index d468c6ce..b9df62c0 100644 --- a/snapshot/win/process_snapshot_win.cc +++ b/snapshot/win/process_snapshot_win.cc @@ -48,6 +48,10 @@ bool ProcessSnapshotWin::Initialize(HANDLE process, return false; system_.Initialize(&process_reader_); + WinVMAddress peb_address; + WinVMSize peb_size; + process_reader_.GetProcessInfo().Peb(&peb_address, &peb_size); + peb_.Initialize(&process_reader_, peb_address, peb_size); InitializeThreads(); InitializeModules(); @@ -112,12 +116,12 @@ void ProcessSnapshotWin::GetCrashpadOptions( pid_t ProcessSnapshotWin::ProcessID() const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); - return process_reader_.ProcessID(); + return process_reader_.GetProcessInfo().ProcessID(); } pid_t ProcessSnapshotWin::ParentProcessID() const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); - return process_reader_.ParentProcessID(); + return process_reader_.GetProcessInfo().ParentProcessID(); } void ProcessSnapshotWin::SnapshotTime(timeval* snapshot_time) const { @@ -179,6 +183,13 @@ const ExceptionSnapshot* ProcessSnapshotWin::Exception() const { return exception_.get(); } +std::vector ProcessSnapshotWin::ExtraMemory() const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + std::vector extra_memory; + extra_memory.push_back(&peb_); + return extra_memory; +} + void ProcessSnapshotWin::InitializeThreads() { const std::vector& process_reader_threads = process_reader_.Threads(); diff --git a/snapshot/win/process_snapshot_win.h b/snapshot/win/process_snapshot_win.h index 5c0b1b35..bba9828f 100644 --- a/snapshot/win/process_snapshot_win.h +++ b/snapshot/win/process_snapshot_win.h @@ -27,11 +27,13 @@ #include "client/crashpad_info.h" #include "snapshot/crashpad_info_client_options.h" #include "snapshot/exception_snapshot.h" +#include "snapshot/memory_snapshot.h" #include "snapshot/module_snapshot.h" #include "snapshot/process_snapshot.h" #include "snapshot/system_snapshot.h" #include "snapshot/thread_snapshot.h" #include "snapshot/win/exception_snapshot_win.h" +#include "snapshot/win/memory_snapshot_win.h" #include "snapshot/win/module_snapshot_win.h" #include "snapshot/win/system_snapshot_win.h" #include "snapshot/win/thread_snapshot_win.h" @@ -123,6 +125,7 @@ class ProcessSnapshotWin final : public ProcessSnapshot { std::vector Threads() const override; std::vector Modules() const override; const ExceptionSnapshot* Exception() const override; + std::vector ExtraMemory() const override; private: // Initializes threads_ on behalf of Initialize(). @@ -132,6 +135,7 @@ class ProcessSnapshotWin final : public ProcessSnapshot { void InitializeModules(); internal::SystemSnapshotWin system_; + internal::MemorySnapshotWin peb_; PointerVector threads_; PointerVector modules_; scoped_ptr exception_; diff --git a/util/win/process_info.cc b/util/win/process_info.cc index 64fb79b0..ff8689ed 100644 --- a/util/win/process_info.cc +++ b/util/win/process_info.cc @@ -110,7 +110,8 @@ template bool GetProcessBasicInformation(HANDLE process, bool is_wow64, ProcessInfo* process_info, - WinVMAddress* peb_address) { + WinVMAddress* peb_address, + WinVMSize* peb_size) { ULONG bytes_returned; process_types::PROCESS_BASIC_INFORMATION process_basic_information; NTSTATUS status = @@ -143,6 +144,7 @@ bool GetProcessBasicInformation(HANDLE process, // The address of this is found by a second call to NtQueryInformationProcess. if (!is_wow64) { *peb_address = process_basic_information.PebBaseAddress; + *peb_size = sizeof(process_types::PEB); } else { ULONG_PTR wow64_peb_address; status = crashpad::NtQueryInformationProcess(process, @@ -159,6 +161,7 @@ bool GetProcessBasicInformation(HANDLE process, return false; } *peb_address = wow64_peb_address; + *peb_size = sizeof(process_types::PEB); } return true; @@ -260,6 +263,8 @@ ProcessInfo::ProcessInfo() : process_id_(), inherited_from_process_id_(), command_line_(), + peb_address_(0), + peb_size_(0), modules_(), is_64_bit_(false), is_wow64_(false), @@ -293,13 +298,12 @@ bool ProcessInfo::Initialize(HANDLE process) { } #endif - WinVMAddress peb_address; #if ARCH_CPU_64_BITS bool result = GetProcessBasicInformation( - process, is_wow64_, this, &peb_address); + process, is_wow64_, this, &peb_address_, &peb_size_); #else bool result = GetProcessBasicInformation( - process, false, this, &peb_address); + process, false, this, &peb_address_, &peb_size_); #endif // ARCH_CPU_64_BITS if (!result) { @@ -308,9 +312,9 @@ bool ProcessInfo::Initialize(HANDLE process) { } result = is_64_bit_ ? ReadProcessData( - process, peb_address, this) - : ReadProcessData( - process, peb_address, this); + process, peb_address_, this) + : ReadProcessData( + process, peb_address_, this); if (!result) { LOG(ERROR) << "ReadProcessData failed"; return false; @@ -346,6 +350,11 @@ bool ProcessInfo::CommandLine(std::wstring* command_line) const { return true; } +void ProcessInfo::Peb(WinVMAddress* peb_address, WinVMSize* peb_size) const { + *peb_address = peb_address_; + *peb_size = peb_size_; +} + bool ProcessInfo::Modules(std::vector* modules) const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); *modules = modules_; diff --git a/util/win/process_info.h b/util/win/process_info.h index 02514739..ed07ab09 100644 --- a/util/win/process_info.h +++ b/util/win/process_info.h @@ -78,6 +78,13 @@ class ProcessInfo { //! Block. bool CommandLine(std::wstring* command_line) const; + //! \brief Gets the address and size of the process's Process Environment + //! Block. + //! + //! \param[out] peb_address The address of the Process Environment Block. + //! \param[out] peb_size The size of the Process Environment Block. + void Peb(WinVMAddress* peb_address, WinVMSize* peb_size) const; + //! \brief Retrieves the modules loaded into the target process. //! //! The modules are enumerated in initialization order as detailed in the @@ -90,7 +97,8 @@ class ProcessInfo { friend bool GetProcessBasicInformation(HANDLE process, bool is_wow64, ProcessInfo* process_info, - WinVMAddress* peb_address); + WinVMAddress* peb_address, + WinVMSize* peb_size); template friend bool ReadProcessData(HANDLE process, WinVMAddress peb_address_vmaddr, @@ -99,6 +107,8 @@ class ProcessInfo { pid_t process_id_; pid_t inherited_from_process_id_; std::wstring command_line_; + WinVMAddress peb_address_; + WinVMSize peb_size_; std::vector modules_; bool is_64_bit_; bool is_wow64_;