diff --git a/handler/win/crashy_test_program.cc b/handler/win/crashy_test_program.cc index be03d4cd..4355ae00 100644 --- a/handler/win/crashy_test_program.cc +++ b/handler/win/crashy_test_program.cc @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include #include #include @@ -39,6 +40,11 @@ namespace { CRITICAL_SECTION g_test_critical_section; +unsigned char g_test_memory[] = { + 99, 98, 97, 96, 95, 94, 93, 92, 91, 90, + 89, 88, 87, 86, 85, 84, 83, 82, 81, 80, +}; + ULONG RtlNtStatusToDosError(NTSTATUS status) { static const auto rtl_nt_status_to_dos_error = GET_FUNCTION_REQUIRED(L"ntdll.dll", ::RtlNtStatusToDosError); @@ -90,6 +96,12 @@ void SomeCrashyFunction() { // LastStatusError of the TEB as a side-effect, and we'll be setting // ERROR_FILE_NOT_FOUND for GetLastError(). SetLastError(RtlNtStatusToDosError(STATUS_NO_SUCH_FILE)); + + // Set a register to point at some memory we can test to confirm it makes it + // into the minidump. We use __movsb as a way to set SI/DI without needing an + // external .asm file. + __movsb(g_test_memory, g_test_memory, 0); + volatile int* foo = reinterpret_cast(7); *foo = 42; } diff --git a/minidump/minidump_exception_writer.h b/minidump/minidump_exception_writer.h index a81abfb2..68fe03d2 100644 --- a/minidump/minidump_exception_writer.h +++ b/minidump/minidump_exception_writer.h @@ -31,6 +31,7 @@ namespace crashpad { class ExceptionSnapshot; class MinidumpContextWriter; +class MinidumpMemoryListWriter; //! \brief The writer for a MINIDUMP_EXCEPTION_STREAM stream in a minidump file. class MinidumpExceptionWriter final : public internal::MinidumpStreamWriter { diff --git a/minidump/minidump_file_writer.cc b/minidump/minidump_file_writer.cc index b9a9eeab..9b540d32 100644 --- a/minidump/minidump_file_writer.cc +++ b/minidump/minidump_file_writer.cc @@ -28,6 +28,7 @@ #include "minidump/minidump_thread_id_map.h" #include "minidump/minidump_thread_writer.h" #include "minidump/minidump_writer_util.h" +#include "snapshot/exception_snapshot.h" #include "snapshot/process_snapshot.h" #include "util/file/file_writer.h" #include "util/numeric/safe_assignment.h" @@ -119,6 +120,8 @@ void MinidumpFileWriter::InitializeFromSnapshot( } memory_list->AddFromSnapshot(process_snapshot->ExtraMemory()); + if (exception_snapshot) + memory_list->AddFromSnapshot(exception_snapshot->ExtraMemory()); AddStream(std::move(memory_list)); } diff --git a/snapshot/exception_snapshot.h b/snapshot/exception_snapshot.h index bd8ad8a6..eef9f47a 100644 --- a/snapshot/exception_snapshot.h +++ b/snapshot/exception_snapshot.h @@ -19,6 +19,8 @@ #include +#include "snapshot/memory_snapshot.h" + namespace crashpad { struct CPUContext; @@ -103,6 +105,15 @@ class ExceptionSnapshot { //! `RaiseException()`. See the documentation for `ExceptionInformation` in //! `EXCEPTION_RECORD`. virtual const std::vector& Codes() const = 0; + + //! \brief Returns a vector of additional memory blocks that should be + //! included in a minidump. + //! + //! \return A 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 ThreadSnapshot object that they + //! were obtained from. + virtual std::vector ExtraMemory() const = 0; }; } // namespace crashpad diff --git a/snapshot/mac/exception_snapshot_mac.cc b/snapshot/mac/exception_snapshot_mac.cc index b5725d38..c5d32b5b 100644 --- a/snapshot/mac/exception_snapshot_mac.cc +++ b/snapshot/mac/exception_snapshot_mac.cc @@ -249,5 +249,10 @@ const std::vector& ExceptionSnapshotMac::Codes() const { return codes_; } +std::vector ExceptionSnapshotMac::ExtraMemory() const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + return std::vector(); +} + } // namespace internal } // namespace crashpad diff --git a/snapshot/mac/exception_snapshot_mac.h b/snapshot/mac/exception_snapshot_mac.h index 325d9516..17720728 100644 --- a/snapshot/mac/exception_snapshot_mac.h +++ b/snapshot/mac/exception_snapshot_mac.h @@ -68,6 +68,7 @@ class ExceptionSnapshotMac final : public ExceptionSnapshot { uint32_t ExceptionInfo() const override; uint64_t ExceptionAddress() const override; const std::vector& Codes() const override; + virtual std::vector ExtraMemory() const override; private: #if defined(ARCH_CPU_X86_FAMILY) diff --git a/snapshot/snapshot.gyp b/snapshot/snapshot.gyp index 3f7c63ee..29711b06 100644 --- a/snapshot/snapshot.gyp +++ b/snapshot/snapshot.gyp @@ -89,6 +89,8 @@ 'process_snapshot.h', 'system_snapshot.h', 'thread_snapshot.h', + 'win/capture_context_memory.cc', + 'win/capture_context_memory.h', 'win/cpu_context_win.cc', 'win/cpu_context_win.h', 'win/exception_snapshot_win.cc', diff --git a/snapshot/test/test_exception_snapshot.cc b/snapshot/test/test_exception_snapshot.cc index 73d121ce..6adfeaf8 100644 --- a/snapshot/test/test_exception_snapshot.cc +++ b/snapshot/test/test_exception_snapshot.cc @@ -55,5 +55,12 @@ const std::vector& TestExceptionSnapshot::Codes() const { return codes_; } +std::vector TestExceptionSnapshot::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_exception_snapshot.h b/snapshot/test/test_exception_snapshot.h index a17f3dc1..638c0c92 100644 --- a/snapshot/test/test_exception_snapshot.h +++ b/snapshot/test/test_exception_snapshot.h @@ -20,8 +20,10 @@ #include #include "base/macros.h" +#include "base/memory/scoped_ptr.h" #include "snapshot/cpu_context.h" #include "snapshot/exception_snapshot.h" +#include "util/stdlib/pointer_container.h" namespace crashpad { namespace test { @@ -57,6 +59,9 @@ class TestExceptionSnapshot final : public ExceptionSnapshot { exception_address_ = exception_address; } void SetCodes(const std::vector& codes) { codes_ = codes; } + void AddExtraMemory(scoped_ptr extra_memory) { + extra_memory_.push_back(extra_memory.release()); + } // ExceptionSnapshot: @@ -66,6 +71,7 @@ class TestExceptionSnapshot final : public ExceptionSnapshot { uint32_t ExceptionInfo() const override; uint64_t ExceptionAddress() const override; const std::vector& Codes() const override; + std::vector ExtraMemory() const override; private: union { @@ -78,6 +84,7 @@ class TestExceptionSnapshot final : public ExceptionSnapshot { uint32_t exception_info_; uint64_t exception_address_; std::vector codes_; + PointerVector extra_memory_; DISALLOW_COPY_AND_ASSIGN(TestExceptionSnapshot); }; diff --git a/snapshot/win/capture_context_memory.cc b/snapshot/win/capture_context_memory.cc new file mode 100644 index 00000000..e4469008 --- /dev/null +++ b/snapshot/win/capture_context_memory.cc @@ -0,0 +1,103 @@ +// 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/capture_context_memory.h" + +#include + +#include + +#include "snapshot/win/memory_snapshot_win.h" +#include "snapshot/win/process_reader_win.h" + +namespace crashpad { +namespace internal { + +namespace { + +void MaybeCaptureMemoryAround(ProcessReaderWin* process_reader, + WinVMAddress address, + PointerVector* into) { + const WinVMAddress non_address_offset = 0x10000; + if (address < non_address_offset) + return; + if (process_reader->Is64Bit()) { + if (address >= std::numeric_limits::max() - non_address_offset) + return; + } else { + if (address >= std::numeric_limits::max() - non_address_offset) + return; + } + + const WinVMSize kRegisterByteOffset = 32; + const WinVMAddress target = address - kRegisterByteOffset; + const WinVMSize size = 128; + auto ranges = process_reader->GetProcessInfo().GetReadableRanges( + CheckedRange(target, size)); + for (const auto& range : ranges) { + internal::MemorySnapshotWin* snapshot = new internal::MemorySnapshotWin(); + snapshot->Initialize(process_reader, range.base(), range.size()); + into->push_back(snapshot); + } +} + +} // namespace + +void CaptureMemoryPointedToByContext(const CPUContext& context, + ProcessReaderWin* process_reader, + const ProcessReaderWin::Thread& thread, + PointerVector* into) { +#if defined(ARCH_CPU_X86_64) + if (context.architecture == kCPUArchitectureX86_64) { + MaybeCaptureMemoryAround(process_reader, context.x86_64->rax, into); + MaybeCaptureMemoryAround(process_reader, context.x86_64->rbx, into); + MaybeCaptureMemoryAround(process_reader, context.x86_64->rcx, into); + MaybeCaptureMemoryAround(process_reader, context.x86_64->rdx, into); + MaybeCaptureMemoryAround(process_reader, context.x86_64->rdi, into); + MaybeCaptureMemoryAround(process_reader, context.x86_64->rsi, into); + if (context.x86_64->rbp < thread.stack_region_address || + context.x86_64->rbp >= + thread.stack_region_address + thread.stack_region_size) { + MaybeCaptureMemoryAround(process_reader, context.x86_64->rbp, into); + } + MaybeCaptureMemoryAround(process_reader, context.x86_64->r8, into); + MaybeCaptureMemoryAround(process_reader, context.x86_64->r9, into); + MaybeCaptureMemoryAround(process_reader, context.x86_64->r10, into); + MaybeCaptureMemoryAround(process_reader, context.x86_64->r11, into); + MaybeCaptureMemoryAround(process_reader, context.x86_64->r12, into); + MaybeCaptureMemoryAround(process_reader, context.x86_64->r13, into); + MaybeCaptureMemoryAround(process_reader, context.x86_64->r14, into); + MaybeCaptureMemoryAround(process_reader, context.x86_64->r15, into); + MaybeCaptureMemoryAround(process_reader, context.x86_64->rip, into); + } else { +#endif + MaybeCaptureMemoryAround(process_reader, context.x86->eax, into); + MaybeCaptureMemoryAround(process_reader, context.x86->ebx, into); + MaybeCaptureMemoryAround(process_reader, context.x86->ecx, into); + MaybeCaptureMemoryAround(process_reader, context.x86->edx, into); + MaybeCaptureMemoryAround(process_reader, context.x86->edi, into); + MaybeCaptureMemoryAround(process_reader, context.x86->esi, into); + if (context.x86->ebp < thread.stack_region_address || + context.x86->ebp >= + thread.stack_region_address + thread.stack_region_size) { + MaybeCaptureMemoryAround(process_reader, context.x86->ebp, into); + } + MaybeCaptureMemoryAround(process_reader, context.x86->eip, into); +#if defined(ARCH_CPU_X86_64) + } +#endif +} + +} // namespace internal +} // namespace crashpad diff --git a/snapshot/win/capture_context_memory.h b/snapshot/win/capture_context_memory.h new file mode 100644 index 00000000..d4f4e038 --- /dev/null +++ b/snapshot/win/capture_context_memory.h @@ -0,0 +1,43 @@ +// 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_SNAPSHOT_WIN_CAPTURE_CONTEXT_MEMORY_H_ +#define CRASHPAD_SNAPSHOT_WIN_CAPTURE_CONTEXT_MEMORY_H_ + +#include "snapshot/cpu_context.h" +#include "snapshot/win/process_reader_win.h" +#include "util/stdlib/pointer_container.h" + +namespace crashpad { +namespace internal { + +class MemorySnapshotWin; + +//! \brief For all registers that appear to be pointer-like in \a context, +//! captures a small amount of memory near their pointed to location. +//! +//! \param[in] context The context to inspect. +//! \param[in] process_reader A ProcessReaderWin to read from the target +//! process. +//! \param[in] thread The thread to which the context belongs. +//! \param[out] into A vector of pointers to append new ranges to. +void CaptureMemoryPointedToByContext(const CPUContext& context, + ProcessReaderWin* process_reader, + const ProcessReaderWin::Thread& thread, + PointerVector* into); + +} // namespace internal +} // namespace crashpad + +#endif // CRASHPAD_SNAPSHOT_WIN_CAPTURE_CONTEXT_MEMORY_H_ diff --git a/snapshot/win/end_to_end_test.py b/snapshot/win/end_to_end_test.py index f3172ba7..4f453c7e 100755 --- a/snapshot/win/end_to_end_test.py +++ b/snapshot/win/end_to_end_test.py @@ -247,6 +247,10 @@ def RunTests(cdb_path, r'FreeOwnStackAndBreak.*\nquit:', 'at correct location, no additional stack entries') + out = CdbRun(cdb_path, dump_path, '.ecxr; db /c14 edi') + out.Check(r'63 62 61 60 5f 5e 5d 5c-5b 5a 59 58 57 56 55 54 53 52 51 50', + 'data pointed to by registers captured') + 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/exception_snapshot_win.cc b/snapshot/win/exception_snapshot_win.cc index 07b1c8ea..9ec16f69 100644 --- a/snapshot/win/exception_snapshot_win.cc +++ b/snapshot/win/exception_snapshot_win.cc @@ -14,7 +14,10 @@ #include "snapshot/win/exception_snapshot_win.h" +#include "snapshot/memory_snapshot.h" +#include "snapshot/win/capture_context_memory.h" #include "snapshot/win/cpu_context_win.h" +#include "snapshot/win/memory_snapshot_win.h" #include "snapshot/win/process_reader_win.h" #include "util/win/nt_internals.h" @@ -41,15 +44,15 @@ bool ExceptionSnapshotWin::Initialize(ProcessReaderWin* process_reader, WinVMAddress exception_pointers_address) { INITIALIZATION_STATE_SET_INITIALIZING(initialized_); - bool found_thread = false; + const ProcessReaderWin::Thread* thread = nullptr; for (const auto& loop_thread : process_reader->Threads()) { if (thread_id == loop_thread.id) { - found_thread = true; + thread = &loop_thread; break; } } - if (!found_thread) { + if (!thread) { LOG(ERROR) << "thread ID " << thread_id << " not found in process"; return false; } else { @@ -86,6 +89,9 @@ bool ExceptionSnapshotWin::Initialize(ProcessReaderWin* process_reader, InitializeX86Context(context_record, context_.x86); } + CaptureMemoryPointedToByContext( + context_, process_reader, *thread, &extra_memory_); + INITIALIZATION_STATE_SET_VALID(initialized_); return true; } @@ -120,6 +126,15 @@ const std::vector& ExceptionSnapshotWin::Codes() const { return codes_; } +std::vector ExceptionSnapshotWin::ExtraMemory() const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + std::vector result; + result.reserve(extra_memory_.size()); + for (const auto& em : extra_memory_) + result.push_back(em); + return result; +} + template diff --git a/snapshot/win/exception_snapshot_win.h b/snapshot/win/exception_snapshot_win.h index 63d5bbfe..3e66dd26 100644 --- a/snapshot/win/exception_snapshot_win.h +++ b/snapshot/win/exception_snapshot_win.h @@ -23,6 +23,7 @@ #include "snapshot/cpu_context.h" #include "snapshot/exception_snapshot.h" #include "util/misc/initialization_state_dcheck.h" +#include "util/stdlib/pointer_container.h" #include "util/win/address_types.h" #include "util/win/process_structs.h" @@ -32,6 +33,8 @@ class ProcessReaderWin; namespace internal { +class MemorySnapshotWin; + class ExceptionSnapshotWin final : public ExceptionSnapshot { public: ExceptionSnapshotWin(); @@ -60,6 +63,7 @@ class ExceptionSnapshotWin final : public ExceptionSnapshot { uint32_t ExceptionInfo() const override; uint64_t ExceptionAddress() const override; const std::vector& Codes() const override; + std::vector ExtraMemory() const override; private: template codes_; + PointerVector extra_memory_; uint64_t thread_id_; uint64_t exception_address_; uint32_t exception_flags_; diff --git a/snapshot/win/thread_snapshot_win.cc b/snapshot/win/thread_snapshot_win.cc index 9c8c9ea6..f3446e98 100644 --- a/snapshot/win/thread_snapshot_win.cc +++ b/snapshot/win/thread_snapshot_win.cc @@ -17,6 +17,7 @@ #include #include "base/logging.h" +#include "snapshot/win/capture_context_memory.h" #include "snapshot/win/cpu_context_win.h" #include "snapshot/win/process_reader_win.h" @@ -75,6 +76,9 @@ bool ThreadSnapshotWin::Initialize( InitializeX86Context(process_reader_thread.context.native, context_.x86); #endif // ARCH_CPU_X86_64 + CaptureMemoryPointedToByContext( + context_, process_reader, thread_, &pointed_to_memory_); + INITIALIZATION_STATE_SET_VALID(initialized_); return true; } @@ -111,9 +115,13 @@ uint64_t ThreadSnapshotWin::ThreadSpecificDataAddress() const { std::vector ThreadSnapshotWin::ExtraMemory() const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); - // TODO(scottmg): Ensure this region is readable, and make sure we don't - // discard the entire dump if it isn't. https://crashpad.chromium.org/bug/59 - return std::vector(1, &teb_); + std::vector result; + result.reserve(1 + pointed_to_memory_.size()); + result.push_back(&teb_); + std::copy(pointed_to_memory_.begin(), + pointed_to_memory_.end(), + std::back_inserter(result)); + return result; } } // namespace internal diff --git a/snapshot/win/thread_snapshot_win.h b/snapshot/win/thread_snapshot_win.h index 0e07110e..74e6bc6e 100644 --- a/snapshot/win/thread_snapshot_win.h +++ b/snapshot/win/thread_snapshot_win.h @@ -76,6 +76,7 @@ class ThreadSnapshotWin final : public ThreadSnapshot { internal::MemorySnapshotWin teb_; ProcessReaderWin::Thread thread_; InitializationStateDcheck initialized_; + PointerVector pointed_to_memory_; DISALLOW_COPY_AND_ASSIGN(ThreadSnapshotWin); };