From 8ce88d89536615bcc332859fcd7ba00e43cd80db Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Wed, 16 Sep 2015 12:42:20 -0700 Subject: [PATCH] win x86: Grab bag of restructuring to get tests working on x86-on-x86 A few function implementations that were missing, various switches for functions/functionality that didn't exist on XP, and far too long figuring out what exactly was wrong with SYSTEM_PROCESS_INFORMATION on x86 (the "alignment_for_x86" fields). R=mark@chromium.org BUG=crashpad:1, crashpad:50, chromium:531663 Review URL: https://codereview.chromium.org/1336823002 . --- snapshot/win/cpu_context_win.cc | 137 +++++++++++++++------ snapshot/win/cpu_context_win_test.cc | 25 +++- snapshot/win/exception_snapshot_win.cc | 76 +++++++----- snapshot/win/exception_snapshot_win.h | 6 + snapshot/win/pe_image_reader_test.cc | 14 ++- snapshot/win/process_reader_win.cc | 155 ++++++++++++++---------- snapshot/win/process_reader_win.h | 5 +- snapshot/win/process_reader_win_test.cc | 48 +++++++- snapshot/win/thread_snapshot_win.cc | 6 +- test/win/win_child_process_test.cc | 7 +- util/util_test.gyp | 1 + util/win/process_info.cc | 141 +++++++++++---------- util/win/process_info.h | 7 +- util/win/process_structs.h | 130 +++++++++++++------- 14 files changed, 507 insertions(+), 251 deletions(-) diff --git a/snapshot/win/cpu_context_win.cc b/snapshot/win/cpu_context_win.cc index a77ea247..b2389ce9 100644 --- a/snapshot/win/cpu_context_win.cc +++ b/snapshot/win/cpu_context_win.cc @@ -28,48 +28,115 @@ void InitializeX86Context(const WOW64_CONTEXT& context, CPUContextX86* out) { } void InitializeX64Context(const CONTEXT& context, CPUContextX86_64* out) { - out->rax = context.Rax; - out->rbx = context.Rbx; - out->rcx = context.Rcx; - out->rdx = context.Rdx; - out->rdi = context.Rdi; - out->rsi = context.Rsi; - out->rbp = context.Rbp; - out->rsp = context.Rsp; - out->r8 = context.R8; - out->r9 = context.R9; - out->r10 = context.R10; - out->r11 = context.R11; - out->r12 = context.R12; - out->r13 = context.R13; - out->r14 = context.R14; - out->r15 = context.R15; - out->rip = context.Rip; - out->rflags = context.EFlags; - out->cs = context.SegCs; - out->fs = context.SegFs; - out->gs = context.SegGs; + memset(out, 0, sizeof(*out)); - out->dr0 = context.Dr0; - out->dr1 = context.Dr1; - out->dr2 = context.Dr2; - out->dr3 = context.Dr3; - // DR4 and DR5 are obsolete synonyms for DR6 and DR7, see - // http://en.wikipedia.org/wiki/X86_debug_register. - out->dr4 = context.Dr6; - out->dr5 = context.Dr7; - out->dr6 = context.Dr6; - out->dr7 = context.Dr7; + LOG_IF(ERROR, !(context.ContextFlags & CONTEXT_AMD64)) << "non-x64 context"; - static_assert(sizeof(out->fxsave) == sizeof(context.FltSave), - "types must be equivalent"); - memcpy(&out->fxsave, &context.FltSave.ControlWord, sizeof(out->fxsave)); + if (context.ContextFlags & CONTEXT_CONTROL) { + out->cs = context.SegCs; + out->rflags = context.EFlags; + out->rip = context.Rip; + out->rsp = context.Rsp; + // SegSs ignored. + } + + if (context.ContextFlags & CONTEXT_INTEGER) { + out->rax = context.Rax; + out->rbx = context.Rbx; + out->rcx = context.Rcx; + out->rdx = context.Rdx; + out->rdi = context.Rdi; + out->rsi = context.Rsi; + out->rbp = context.Rbp; + out->r8 = context.R8; + out->r9 = context.R9; + out->r10 = context.R10; + out->r11 = context.R11; + out->r12 = context.R12; + out->r13 = context.R13; + out->r14 = context.R14; + out->r15 = context.R15; + } + + if (context.ContextFlags & CONTEXT_SEGMENTS) { + out->fs = context.SegFs; + out->gs = context.SegGs; + // SegDs ignored. + // SegEs ignored. + } + + if (context.ContextFlags & CONTEXT_DEBUG_REGISTERS) { + out->dr0 = context.Dr0; + out->dr1 = context.Dr1; + out->dr2 = context.Dr2; + out->dr3 = context.Dr3; + // DR4 and DR5 are obsolete synonyms for DR6 and DR7, see + // https://en.wikipedia.org/wiki/X86_debug_register. + out->dr4 = context.Dr6; + out->dr5 = context.Dr7; + out->dr6 = context.Dr6; + out->dr7 = context.Dr7; + } + + if (context.ContextFlags & CONTEXT_FLOATING_POINT) { + static_assert(sizeof(out->fxsave) == sizeof(context.FltSave), + "types must be equivalent"); + memcpy(&out->fxsave, &context.FltSave.ControlWord, sizeof(out->fxsave)); + } } #else // ARCH_CPU_64_BITS void InitializeX86Context(const CONTEXT& context, CPUContextX86* out) { - CHECK(false) << "TODO(scottmg) InitializeX86Context()"; + memset(out, 0, sizeof(*out)); + + LOG_IF(ERROR, !(context.ContextFlags & CONTEXT_i386)) << "non-x86 context"; + + if (context.ContextFlags & CONTEXT_CONTROL) { + out->ebp = context.Ebp; + out->eip = context.Eip; + out->cs = static_cast(context.SegCs); + out->eflags = context.EFlags; + out->esp = context.Esp; + out->ss = static_cast(context.SegSs); + } + + if (context.ContextFlags & CONTEXT_INTEGER) { + out->eax = context.Eax; + out->ebx = context.Ebx; + out->ecx = context.Ecx; + out->edx = context.Edx; + out->edi = context.Edi; + out->esi = context.Esi; + } + + if (context.ContextFlags & CONTEXT_SEGMENTS) { + out->ds = static_cast(context.SegDs); + out->es = static_cast(context.SegEs); + out->fs = static_cast(context.SegFs); + out->gs = static_cast(context.SegGs); + } + + if (context.ContextFlags & CONTEXT_DEBUG_REGISTERS) { + out->dr0 = context.Dr0; + out->dr1 = context.Dr1; + out->dr2 = context.Dr2; + out->dr3 = context.Dr3; + // DR4 and DR5 are obsolete synonyms for DR6 and DR7, see + // https://en.wikipedia.org/wiki/X86_debug_register. + out->dr4 = context.Dr6; + out->dr5 = context.Dr7; + out->dr6 = context.Dr6; + out->dr7 = context.Dr7; + } + + if (context.ContextFlags & CONTEXT_EXTENDED_REGISTERS) { + static_assert(sizeof(out->fxsave) == sizeof(context.ExtendedRegisters), + "types must be equivalent"); + memcpy(&out->fxsave, &context.ExtendedRegisters, sizeof(out->fxsave)); + } else if (context.ContextFlags & CONTEXT_FLOATING_POINT) { + CHECK(false) << "TODO(scottmg): extract x87 data"; + } } #endif // ARCH_CPU_64_BITS diff --git a/snapshot/win/cpu_context_win_test.cc b/snapshot/win/cpu_context_win_test.cc index 99d7c928..0c7484b7 100644 --- a/snapshot/win/cpu_context_win_test.cc +++ b/snapshot/win/cpu_context_win_test.cc @@ -27,10 +27,12 @@ namespace { #if defined(ARCH_CPU_X86_64) TEST(CPUContextWin, InitializeX64Context) { - CONTEXT context; + CONTEXT context = {0}; context.Rax = 10; context.FltSave.TagWord = 11; context.Dr0 = 12; + context.ContextFlags = + CONTEXT_INTEGER | CONTEXT_FLOATING_POINT | CONTEXT_DEBUG_REGISTERS; // Test the simple case, where everything in the CPUContextX86_64 argument is // set directly from the supplied thread, float, and debug state parameters. @@ -43,9 +45,26 @@ TEST(CPUContextWin, InitializeX64Context) { } } -#else // ARCH_CPU_X86_64 +#else -#error ARCH_CPU_X86 +TEST(CPUContextWin, InitializeX86Context) { + CONTEXT context = {0}; + context.ContextFlags = + CONTEXT_INTEGER | CONTEXT_EXTENDED_REGISTERS | CONTEXT_DEBUG_REGISTERS; + context.Eax = 1; + context.ExtendedRegisters[4] = 2; // FTW. + context.Dr0 = 3; + + // Test the simple case, where everything in the CPUContextX86 argument is + // set directly from the supplied thread, float, and debug state parameters. + { + CPUContextX86 cpu_context_x86 = {}; + InitializeX86Context(context, &cpu_context_x86); + EXPECT_EQ(1u, cpu_context_x86.eax); + EXPECT_EQ(2u, cpu_context_x86.fxsave.ftw); + EXPECT_EQ(3u, cpu_context_x86.dr0); + } +} #endif // ARCH_CPU_X86_64 diff --git a/snapshot/win/exception_snapshot_win.cc b/snapshot/win/exception_snapshot_win.cc index a32c9294..1156baf3 100644 --- a/snapshot/win/exception_snapshot_win.cc +++ b/snapshot/win/exception_snapshot_win.cc @@ -69,42 +69,30 @@ bool ExceptionSnapshotWin::Initialize(ProcessReaderWin* process_reader, return false; } +#if defined(ARCH_CPU_64_BITS) if (process_reader->Is64Bit()) { - EXCEPTION_RECORD64 first_record; - if (!process_reader->ReadMemory( - reinterpret_cast(exception_pointers.ExceptionRecord), - sizeof(first_record), - &first_record)) { - LOG(ERROR) << "ExceptionRecord"; + CONTEXT context_record; + if (!InitializeFromExceptionPointers( + *process_reader, exception_pointers, &context_record)) { return false; } - exception_code_ = first_record.ExceptionCode; - exception_flags_ = first_record.ExceptionFlags; - exception_address_ = first_record.ExceptionAddress; - for (DWORD i = 0; i < first_record.NumberParameters; ++i) - codes_.push_back(first_record.ExceptionInformation[i]); - if (first_record.ExceptionRecord) { - // https://code.google.com/p/crashpad/issues/detail?id=43 - LOG(WARNING) << "dropping chained ExceptionRecord"; - } - context_.architecture = kCPUArchitectureX86_64; context_.x86_64 = &context_union_.x86_64; - // We assume 64-on-64 here in that we're relying on the CONTEXT definition - // to be the x64 one. - CONTEXT context_record; - if (!process_reader->ReadMemory( - reinterpret_cast(exception_pointers.ContextRecord), - sizeof(context_record), - &context_record)) { - LOG(ERROR) << "ContextRecord"; - return false; - } InitializeX64Context(context_record, context_.x86_64); } else { - CHECK(false) << "TODO(scottmg) x86"; + CHECK(false) << "TODO(scottmg) WOW64"; return false; } +#else + CONTEXT context_record; + if (!InitializeFromExceptionPointers( + *process_reader, exception_pointers, &context_record)) { + return false; + } + context_.architecture = kCPUArchitectureX86; + context_.x86 = &context_union_.x86; + InitializeX86Context(context_record, context_.x86); +#endif // ARCH_CPU_64_BITS INITIALIZATION_STATE_SET_VALID(initialized_); return true; @@ -140,5 +128,39 @@ const std::vector& ExceptionSnapshotWin::Codes() const { return codes_; } +template +bool ExceptionSnapshotWin::InitializeFromExceptionPointers( + const ProcessReaderWin& process_reader, + const EXCEPTION_POINTERS& exception_pointers, + ContextType* context_record) { + ExceptionRecordType first_record; + if (!process_reader.ReadMemory( + reinterpret_cast(exception_pointers.ExceptionRecord), + sizeof(first_record), + &first_record)) { + LOG(ERROR) << "ExceptionRecord"; + return false; + } + exception_code_ = first_record.ExceptionCode; + exception_flags_ = first_record.ExceptionFlags; + exception_address_ = first_record.ExceptionAddress; + for (DWORD i = 0; i < first_record.NumberParameters; ++i) + codes_.push_back(first_record.ExceptionInformation[i]); + if (first_record.ExceptionRecord) { + // https://code.google.com/p/crashpad/issues/detail?id=43 + LOG(WARNING) << "dropping chained ExceptionRecord"; + } + + if (!process_reader.ReadMemory( + reinterpret_cast(exception_pointers.ContextRecord), + sizeof(*context_record), + context_record)) { + LOG(ERROR) << "ContextRecord"; + return false; + } + + return true; +} + } // namespace internal } // namespace crashpad diff --git a/snapshot/win/exception_snapshot_win.h b/snapshot/win/exception_snapshot_win.h index 588c4ac9..277cd4a5 100644 --- a/snapshot/win/exception_snapshot_win.h +++ b/snapshot/win/exception_snapshot_win.h @@ -61,6 +61,12 @@ class ExceptionSnapshotWin final : public ExceptionSnapshot { const std::vector& Codes() const override; private: + template + bool InitializeFromExceptionPointers( + const ProcessReaderWin& process_reader, + const EXCEPTION_POINTERS& exception_pointers, + ContextType* context_record); + #if defined(ARCH_CPU_X86_FAMILY) union { CPUContextX86 x86; diff --git a/snapshot/win/pe_image_reader_test.cc b/snapshot/win/pe_image_reader_test.cc index 8da329e4..4a3bbd22 100644 --- a/snapshot/win/pe_image_reader_test.cc +++ b/snapshot/win/pe_image_reader_test.cc @@ -14,6 +14,7 @@ #include "snapshot/win/pe_image_reader.h" +#define PSAPI_VERSION 1 #include #include "gtest/gtest.h" @@ -25,6 +26,17 @@ namespace crashpad { namespace test { namespace { +BOOL CrashpadGetModuleInformation(HANDLE process, + HMODULE module, + MODULEINFO* module_info, + DWORD cb) { + static decltype(GetModuleInformation)* get_module_information = + reinterpret_cast( + GetProcAddress(LoadLibrary(L"psapi.dll"), "GetModuleInformation")); + DCHECK(get_module_information); + return get_module_information(process, module, module_info, cb); +} + TEST(PEImageReader, DebugDirectory) { PEImageReader pe_image_reader; ProcessReaderWin process_reader; @@ -32,7 +44,7 @@ TEST(PEImageReader, DebugDirectory) { ProcessSuspensionState::kRunning)); HMODULE self = reinterpret_cast(&__ImageBase); MODULEINFO module_info; - ASSERT_TRUE(GetModuleInformation( + ASSERT_TRUE(CrashpadGetModuleInformation( GetCurrentProcess(), self, &module_info, sizeof(module_info))); EXPECT_EQ(self, module_info.lpBaseOfDll); EXPECT_TRUE(pe_image_reader.Initialize(&process_reader, diff --git a/snapshot/win/process_reader_win.cc b/snapshot/win/process_reader_win.cc index bad81cd7..81a86704 100644 --- a/snapshot/win/process_reader_win.cc +++ b/snapshot/win/process_reader_win.cc @@ -19,6 +19,7 @@ #include "base/memory/scoped_ptr.h" #include "base/numerics/safe_conversions.h" #include "util/win/nt_internals.h" +#include "util/win/ntstatus_logging.h" #include "util/win/process_structs.h" #include "util/win/scoped_handle.h" #include "util/win/time.h" @@ -57,10 +58,8 @@ process_types::SYSTEM_PROCESS_INFORMATION* GetProcessInformation( // This must be in retry loop, as we're racing with process creation on the // system to find a buffer large enough to hold all process information. for (int tries = 0; tries < 20; ++tries) { - const int kSystemExtendedProcessInformation = 57; status = crashpad::NtQuerySystemInformation( - static_cast( - kSystemExtendedProcessInformation), + SystemProcessInformation, reinterpret_cast(buffer->get()), buffer_size, &buffer_size); @@ -77,7 +76,7 @@ process_types::SYSTEM_PROCESS_INFORMATION* GetProcessInformation( } if (!NT_SUCCESS(status)) { - LOG(ERROR) << "NtQuerySystemInformation failed: " << std::hex << status; + NTSTATUS_LOG(ERROR, status) << "NtQuerySystemInformation"; return nullptr; } @@ -95,16 +94,17 @@ process_types::SYSTEM_PROCESS_INFORMATION* GetProcessInformation( } template -HANDLE OpenThread(const process_types::SYSTEM_EXTENDED_THREAD_INFORMATION< - Traits>& thread_info) { +HANDLE OpenThread( + const process_types::SYSTEM_THREAD_INFORMATION& thread_info) { HANDLE handle; - ACCESS_MASK query_access = THREAD_GET_CONTEXT | THREAD_SUSPEND_RESUME; + ACCESS_MASK query_access = + THREAD_GET_CONTEXT | THREAD_SUSPEND_RESUME | THREAD_QUERY_INFORMATION; OBJECT_ATTRIBUTES object_attributes; InitializeObjectAttributes(&object_attributes, nullptr, 0, nullptr, nullptr); NTSTATUS status = crashpad::NtOpenThread( &handle, query_access, &object_attributes, &thread_info.ClientId); if (!NT_SUCCESS(status)) { - LOG(ERROR) << "NtOpenThread failed"; + NTSTATUS_LOG(ERROR, status) << "NtOpenThread"; return nullptr; } return handle; @@ -114,19 +114,15 @@ HANDLE OpenThread(const process_types::SYSTEM_EXTENDED_THREAD_INFORMATION< // side-effect of returning the SuspendCount of the thread on success, so we // fill out these two pieces of semi-unrelated data in the same function. template -void FillThreadContextAndSuspendCount( - const process_types::SYSTEM_EXTENDED_THREAD_INFORMATION& - thread_info, - ProcessReaderWin::Thread* thread, - ProcessSuspensionState suspension_state) { +bool FillThreadContextAndSuspendCount(HANDLE thread_handle, + ProcessReaderWin::Thread* thread, + ProcessSuspensionState suspension_state) { // Don't suspend the thread if it's this thread. This is really only for test // binaries, as we won't be walking ourselves, in general. - bool is_current_thread = thread_info.ClientId.UniqueThread == + bool is_current_thread = thread->id == reinterpret_cast*>( NtCurrentTeb())->ClientId.UniqueThread; - ScopedKernelHANDLE thread_handle(OpenThread(thread_info)); - // TODO(scottmg): Handle cross-bitness in this function. if (is_current_thread) { @@ -134,10 +130,10 @@ void FillThreadContextAndSuspendCount( thread->suspend_count = 0; RtlCaptureContext(&thread->context); } else { - DWORD previous_suspend_count = SuspendThread(thread_handle.get()); + DWORD previous_suspend_count = SuspendThread(thread_handle); if (previous_suspend_count == -1) { PLOG(ERROR) << "SuspendThread failed"; - return; + return false; } DCHECK(previous_suspend_count > 0 || suspension_state == ProcessSuspensionState::kRunning); @@ -147,15 +143,18 @@ void FillThreadContextAndSuspendCount( memset(&thread->context, 0, sizeof(thread->context)); thread->context.ContextFlags = CONTEXT_ALL; - if (!GetThreadContext(thread_handle.get(), &thread->context)) { + if (!GetThreadContext(thread_handle, &thread->context)) { PLOG(ERROR) << "GetThreadContext failed"; - return; + return false; } - if (!ResumeThread(thread_handle.get())) { + if (!ResumeThread(thread_handle)) { PLOG(ERROR) << "ResumeThread failed"; + return false; } } + + return true; } } // namespace @@ -198,7 +197,7 @@ bool ProcessReaderWin::Initialize(HANDLE process, bool ProcessReaderWin::ReadMemory(WinVMAddress at, WinVMSize num_bytes, - void* into) { + void* into) const { SIZE_T bytes_read; if (!ReadProcessMemory(process_, reinterpret_cast(at), @@ -243,50 +242,10 @@ const std::vector& ProcessReaderWin::Threads() { initialized_threads_ = true; - DCHECK(threads_.empty()); - -#if ARCH_CPU_32_BITS - using SizeTraits = process_types::internal::Traits32; -#else - using SizeTraits = process_types::internal::Traits64; -#endif - scoped_ptr buffer; - process_types::SYSTEM_PROCESS_INFORMATION* process_information = - GetProcessInformation(process_, &buffer); - if (!process_information) - return threads_; - - for (unsigned long i = 0; i < process_information->NumberOfThreads; ++i) { - const process_types::SYSTEM_EXTENDED_THREAD_INFORMATION& - thread_info = process_information->Threads[i]; - Thread thread; - thread.id = thread_info.ClientId.UniqueThread; - - FillThreadContextAndSuspendCount(thread_info, &thread, suspension_state_); - - // TODO(scottmg): I believe we could reverse engineer the PriorityClass from - // the Priority, BasePriority, and - // https://msdn.microsoft.com/en-us/library/windows/desktop/ms685100 . - // MinidumpThreadWriter doesn't handle it yet in any case, so investigate - // both of those at the same time if it's useful. - thread.priority_class = NORMAL_PRIORITY_CLASS; - - thread.priority = thread_info.Priority; - thread.teb = thread_info.TebBase; - - // While there are semi-documented fields in the thread structure called - // StackBase and StackLimit, they don't appear to be correct in practice (or - // at least, I don't know how to interpret them). Instead, read the TIB - // (Thread Information Block) which is the first element of the TEB, and use - // its stack fields. - process_types::NT_TIB tib; - if (ReadMemory(thread_info.TebBase, sizeof(tib), &tib)) { - // Note, "backwards" because of direction of stack growth. - thread.stack_region_address = tib.StackLimit; - thread.stack_region_size = tib.StackBase - tib.StackLimit; - } - threads_.push_back(thread); - } + if (process_info_.Is64Bit()) + ReadThreadData(); + else + ReadThreadData(); return threads_; } @@ -301,4 +260,68 @@ const std::vector& ProcessReaderWin::Modules() { return modules_; } +template +void ProcessReaderWin::ReadThreadData() { + DCHECK(threads_.empty()); + + scoped_ptr buffer; + process_types::SYSTEM_PROCESS_INFORMATION* process_information = + GetProcessInformation(process_, &buffer); + if (!process_information) + return; + + for (unsigned long i = 0; i < process_information->NumberOfThreads; ++i) { + const process_types::SYSTEM_THREAD_INFORMATION& thread_info = + process_information->Threads[i]; + ProcessReaderWin::Thread thread; + thread.id = thread_info.ClientId.UniqueThread; + + ScopedKernelHANDLE thread_handle(OpenThread(thread_info)); + if (!thread_handle.is_valid()) + continue; + + if (!FillThreadContextAndSuspendCount( + thread_handle.get(), &thread, suspension_state_)) { + continue; + } + + // TODO(scottmg): I believe we could reverse engineer the PriorityClass from + // the Priority, BasePriority, and + // https://msdn.microsoft.com/en-us/library/windows/desktop/ms685100 . + // MinidumpThreadWriter doesn't handle it yet in any case, so investigate + // both of those at the same time if it's useful. + thread.priority_class = NORMAL_PRIORITY_CLASS; + + thread.priority = thread_info.Priority; + + process_types::THREAD_BASIC_INFORMATION thread_basic_info; + NTSTATUS status = crashpad::NtQueryInformationThread( + thread_handle.get(), + static_cast(ThreadBasicInformation), + &thread_basic_info, + sizeof(thread_basic_info), + nullptr); + if (!NT_SUCCESS(status)) { + NTSTATUS_LOG(ERROR, status) << "NtQueryInformationThread"; + continue; + } + + // Read the TIB (Thread Information Block) which is the first element of the + // TEB, for its stack fields. + process_types::NT_TIB tib; + if (ReadMemory(thread_basic_info.TebBaseAddress, sizeof(tib), &tib)) { + // Note, "backwards" because of direction of stack growth. + thread.stack_region_address = tib.StackLimit; + if (tib.StackLimit > tib.StackBase) { + LOG(ERROR) << "invalid stack range: " << tib.StackBase << " - " + << tib.StackLimit; + thread.stack_region_size = 0; + } else { + thread.stack_region_size = tib.StackBase - tib.StackLimit; + } + } + threads_.push_back(thread); + } +} + } // namespace crashpad diff --git a/snapshot/win/process_reader_win.h b/snapshot/win/process_reader_win.h index 3f49f335..f23d0d53 100644 --- a/snapshot/win/process_reader_win.h +++ b/snapshot/win/process_reader_win.h @@ -79,7 +79,7 @@ class ProcessReaderWin { 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); + bool ReadMemory(WinVMAddress at, WinVMSize num_bytes, void* into) const; //! \brief Determines the target process' start time. //! @@ -107,6 +107,9 @@ class ProcessReaderWin { const std::vector& Modules(); private: + template + void ReadThreadData(); + HANDLE process_; ProcessInfo process_info_; std::vector threads_; diff --git a/snapshot/win/process_reader_win_test.cc b/snapshot/win/process_reader_win_test.cc index 1cb4bdda..9e2e52cd 100644 --- a/snapshot/win/process_reader_win_test.cc +++ b/snapshot/win/process_reader_win_test.cc @@ -19,6 +19,8 @@ #include "gtest/gtest.h" #include "test/win/win_multiprocess.h" +#include "util/synchronization/semaphore.h" +#include "util/thread/thread.h" #include "util/win/scoped_process_suspend.h" namespace crashpad { @@ -104,7 +106,7 @@ TEST(ProcessReaderWin, SelfOneThread) { // thread, not exactly one thread. ASSERT_GE(threads.size(), 1u); - EXPECT_EQ(GetThreadId(GetCurrentThread()), threads[0].id); + EXPECT_EQ(GetCurrentThreadId(), threads[0].id); #if defined(ARCH_CPU_64_BITS) EXPECT_NE(0, threads[0].context.Rip); #else @@ -120,14 +122,36 @@ class ProcessReaderChildThreadSuspendCount final : public WinMultiprocess { ~ProcessReaderChildThreadSuspendCount() {} private: + enum : unsigned int { kCreatedThreads = 3 }; + + class SleepingThread : public Thread { + public: + SleepingThread() : done_(nullptr) {} + + void SetHandle(Semaphore* done) { + done_= done; + } + + void ThreadMain() override { + done_->Wait(); + }; + + private: + Semaphore* done_; + }; + void WinMultiprocessParent() override { + char c; + CheckedReadFile(ReadPipeHandle(), &c, sizeof(c)); + ASSERT_EQ(' ', c); + { ProcessReaderWin process_reader; ASSERT_TRUE(process_reader.Initialize(ChildProcess(), ProcessSuspensionState::kRunning)); const auto& threads = process_reader.Threads(); - ASSERT_FALSE(threads.empty()); + ASSERT_GE(threads.size(), kCreatedThreads + 1); for (const auto& thread : threads) EXPECT_EQ(0u, thread.suspend_count); } @@ -142,19 +166,33 @@ class ProcessReaderChildThreadSuspendCount final : public WinMultiprocess { // Confirm that thread counts are adjusted correctly for the process being // suspended. const auto& threads = process_reader.Threads(); - ASSERT_FALSE(threads.empty()); + ASSERT_GE(threads.size(), kCreatedThreads + 1); for (const auto& thread : threads) EXPECT_EQ(0u, thread.suspend_count); } } void WinMultiprocessChild() override { - WinVMAddress address = reinterpret_cast(kTestMemory); - CheckedWriteFile(WritePipeHandle(), &address, sizeof(address)); + // Create three dummy threads so we can confirm we read successfully read + // more than just the main thread. + SleepingThread threads[kCreatedThreads]; + Semaphore done(0); + for (auto& thread : threads) + thread.SetHandle(&done); + for (auto& thread : threads) + thread.Start(); + + char c = ' '; + CheckedWriteFile(WritePipeHandle(), &c, sizeof(c)); // Wait for the parent to signal that it's OK to exit by closing its end of // the pipe. CheckedReadFileAtEOF(ReadPipeHandle()); + + for (int i = 0; i < arraysize(threads); ++i) + done.Signal(); + for (auto& thread : threads) + thread.Join(); } DISALLOW_COPY_AND_ASSIGN(ProcessReaderChildThreadSuspendCount); diff --git a/snapshot/win/thread_snapshot_win.cc b/snapshot/win/thread_snapshot_win.cc index 1026cfb9..7524e455 100644 --- a/snapshot/win/thread_snapshot_win.cc +++ b/snapshot/win/thread_snapshot_win.cc @@ -50,7 +50,11 @@ bool ThreadSnapshotWin::Initialize( context_.x86); } #else -#error ARCH_CPU_X86 + context_.architecture = kCPUArchitectureX86; + context_.x86 = &context_union_.x86; + InitializeX86Context( + *reinterpret_cast(&process_reader_thread.context), + context_.x86); #endif // ARCH_CPU_X86_64 INITIALIZATION_STATE_SET_VALID(initialized_); diff --git a/test/win/win_child_process_test.cc b/test/win/win_child_process_test.cc index bdf627a8..e191dcc9 100644 --- a/test/win/win_child_process_test.cc +++ b/test/win/win_child_process_test.cc @@ -14,6 +14,7 @@ #include "test/win/win_child_process.h" +#include #include #include "base/basictypes.h" @@ -49,7 +50,7 @@ class TestWinChildProcess final : public WinChildProcess { int Run() override { int value = ReadInt(ReadPipeHandle()); WriteInt(WritePipeHandle(), value); - return EXIT_SUCCESS; + return testing::Test::HasFailure() ? EXIT_FAILURE : EXIT_SUCCESS; } DISALLOW_COPY_AND_ASSIGN(TestWinChildProcess); @@ -58,7 +59,9 @@ class TestWinChildProcess final : public WinChildProcess { TEST(WinChildProcessTest, WinChildProcess) { WinChildProcess::EntryPoint(); - WinChildProcess::Launch(); + scoped_ptr handles = WinChildProcess::Launch(); + WriteInt(handles->write.get(), 1); + ASSERT_EQ(1, ReadInt(handles->read.get())); } TEST(WinChildProcessTest, MultipleChildren) { diff --git a/util/util_test.gyp b/util/util_test.gyp index 98879eb6..88bbf271 100644 --- a/util/util_test.gyp +++ b/util/util_test.gyp @@ -126,6 +126,7 @@ '/BASE:0x78000000', '/FIXED', ], + 'MinimumRequiredVersion': '5.02', # Server 2003. 'TargetMachine': '17', # x64. }, }, diff --git a/util/win/process_info.cc b/util/win/process_info.cc index ca8277ed..f7d1c5de 100644 --- a/util/win/process_info.cc +++ b/util/win/process_info.cc @@ -17,7 +17,10 @@ #include #include "base/logging.h" +#include "base/strings/stringprintf.h" +#include "build/build_config.h" #include "util/numeric/safe_assignment.h" +#include "util/win/ntstatus_logging.h" #include "util/win/process_structs.h" namespace crashpad { @@ -80,7 +83,8 @@ bool ReadUnicodeString(HANDLE process, return true; } -template bool ReadStruct(HANDLE process, WinVMAddress at, T* into) { +template +bool ReadStruct(HANDLE process, WinVMAddress at, T* into) { SIZE_T bytes_read; if (!ReadProcessMemory(process, reinterpret_cast(at), @@ -101,14 +105,72 @@ template bool ReadStruct(HANDLE process, WinVMAddress at, T* into) { } // namespace +template +bool GetProcessBasicInformation(HANDLE process, + bool is_wow64, + ProcessInfo* process_info, + WinVMAddress* peb_address) { + ULONG bytes_returned; + process_types::PROCESS_BASIC_INFORMATION process_basic_information; + NTSTATUS status = + crashpad::NtQueryInformationProcess(process, + ProcessBasicInformation, + &process_basic_information, + sizeof(process_basic_information), + &bytes_returned); + if (!NT_SUCCESS(status)) { + NTSTATUS_LOG(ERROR, status) << "NtQueryInformationProcess"; + return false; + } + if (bytes_returned != sizeof(process_basic_information)) { + LOG(ERROR) << "NtQueryInformationProcess incorrect size"; + return false; + } + + // See https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203 on + // 32 bit being the correct size for HANDLEs for proceses, even on Windows + // x64. API functions (e.g. OpenProcess) take only a DWORD, so there's no + // sense in maintaining the top bits. + process_info->process_id_ = + static_cast(process_basic_information.UniqueProcessId); + process_info->inherited_from_process_id_ = static_cast( + process_basic_information.InheritedFromUniqueProcessId); + + // We now want to read the PEB to gather the rest of our information. The + // PebBaseAddress as returned above is what we want for 64-on-64 and 32-on-32, + // but for Wow64, we want to read the 32 bit PEB (a Wow64 process has both). + // The address of this is found by a second call to NtQueryInformationProcess. + if (!is_wow64) { + *peb_address = process_basic_information.PebBaseAddress; + } else { + ULONG_PTR wow64_peb_address; + status = crashpad::NtQueryInformationProcess(process, + ProcessWow64Information, + &wow64_peb_address, + sizeof(wow64_peb_address), + &bytes_returned); + if (!NT_SUCCESS(status)) { + NTSTATUS_LOG(ERROR, status), "NtQueryInformationProcess"; + return false; + } + if (bytes_returned != sizeof(wow64_peb_address)) { + LOG(ERROR) << "NtQueryInformationProcess incorrect size"; + return false; + } + *peb_address = wow64_peb_address; + } + + return true; +} + template bool ReadProcessData(HANDLE process, WinVMAddress peb_address_vmaddr, ProcessInfo* process_info) { Traits::Pointer peb_address; if (!AssignIfInRange(&peb_address, peb_address_vmaddr)) { - LOG(ERROR) << "peb_address_vmaddr " << peb_address_vmaddr - << " out of range"; + LOG(ERROR) << base::StringPrintf("peb address 0x%x out of range", + peb_address_vmaddr); return false; } @@ -232,73 +294,28 @@ bool ProcessInfo::Initialize(HANDLE process) { } #endif - ULONG bytes_returned; - // We assume this process is not running on Wow64. The - // PROCESS_BASIC_INFORMATION uses the OS size (that is, even Wow64 has a 64 - // bit one.) - // TODO(scottmg): Either support running as Wow64, or check/resolve this at a - // higher level. -#if ARCH_CPU_32_BITS - process_types::PROCESS_BASIC_INFORMATION - process_basic_information; + WinVMAddress peb_address; +#if ARCH_CPU_64_BITS + bool result = GetProcessBasicInformation( + process, is_wow64_, this, &peb_address); #else - process_types::PROCESS_BASIC_INFORMATION - process_basic_information; -#endif - NTSTATUS status = - crashpad::NtQueryInformationProcess(process, - ProcessBasicInformation, - &process_basic_information, - sizeof(process_basic_information), - &bytes_returned); - if (status < 0) { - LOG(ERROR) << "NtQueryInformationProcess: status=" << status; - return false; - } - if (bytes_returned != sizeof(process_basic_information)) { - LOG(ERROR) << "NtQueryInformationProcess incorrect size"; + bool result = GetProcessBasicInformation( + process, false, this, &peb_address); +#endif // ARCH_CPU_64_BITS + + if (!result) { + LOG(ERROR) << "GetProcessBasicInformation failed"; return false; } - // See https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203 on - // 32 bit being the correct size for HANDLEs for proceses, even on Windows - // x64. API functions (e.g. OpenProcess) take only a DWORD, so there's no - // sense in maintaining the top bits. - process_id_ = static_cast(process_basic_information.UniqueProcessId); - inherited_from_process_id_ = static_cast( - process_basic_information.InheritedFromUniqueProcessId); - - // We now want to read the PEB to gather the rest of our information. The - // PebBaseAddress as returned above is what we want for 64-on-64 and 32-on-32, - // but for Wow64, we want to read the 32 bit PEB (a Wow64 process has both). - // The address of this is found by a second call to NtQueryInformationProcess. - WinVMAddress peb_address = process_basic_information.PebBaseAddress; - if (is_wow64_) { - ULONG_PTR wow64_peb_address; - status = - crashpad::NtQueryInformationProcess(process, - ProcessWow64Information, - &wow64_peb_address, - sizeof(wow64_peb_address), - &bytes_returned); - if (status < 0) { - LOG(ERROR) << "NtQueryInformationProcess: status=" << status; - return false; - } - if (bytes_returned != sizeof(wow64_peb_address)) { - LOG(ERROR) << "NtQueryInformationProcess incorrect size"; - return false; - } - peb_address = wow64_peb_address; - } - - // Read the PEB data using the correct word size. - bool result = is_64_bit_ ? ReadProcessData( + result = is_64_bit_ ? ReadProcessData( process, peb_address, this) : ReadProcessData( process, peb_address, this); - if (!result) + if (!result) { + LOG(ERROR) << "ReadProcessData failed"; return false; + } INITIALIZATION_STATE_SET_VALID(initialized_); return true; diff --git a/util/win/process_info.h b/util/win/process_info.h index 5d272c42..02514739 100644 --- a/util/win/process_info.h +++ b/util/win/process_info.h @@ -86,7 +86,12 @@ class ProcessInfo { bool Modules(std::vector* modules) const; private: - template + template + friend bool GetProcessBasicInformation(HANDLE process, + bool is_wow64, + ProcessInfo* process_info, + WinVMAddress* peb_address); + template friend bool ReadProcessData(HANDLE process, WinVMAddress peb_address_vmaddr, ProcessInfo* process_info); diff --git a/util/win/process_structs.h b/util/win/process_structs.h index 65f0aac0..7f4e4f59 100644 --- a/util/win/process_structs.h +++ b/util/win/process_structs.h @@ -311,36 +311,70 @@ struct TEB { CLIENT_ID ClientId; }; -// See https://msdn.microsoft.com/en-us/library/gg750724.aspx for the base -// structure, and -// http://processhacker.sourceforge.net/doc/struct___s_y_s_t_e_m___e_x_t_e_n_d_e_d___t_h_r_e_a_d___i_n_f_o_r_m_a_t_i_o_n.html -// for the extension part. +// See https://msdn.microsoft.com/en-us/library/gg750724.aspx. template -struct SYSTEM_EXTENDED_THREAD_INFORMATION { - LARGE_INTEGER KernelTime; - LARGE_INTEGER UserTime; - LARGE_INTEGER CreateTime; +struct SYSTEM_THREAD_INFORMATION { union { - ULONG WaitTime; - typename Traits::Pad padding_for_x64_0; + struct { + LARGE_INTEGER KernelTime; + LARGE_INTEGER UserTime; + LARGE_INTEGER CreateTime; + union { + ULONG WaitTime; + typename Traits::Pad padding_for_x64_0; + }; + typename Traits::Pointer StartAddress; + CLIENT_ID ClientId; + LONG Priority; + LONG BasePriority; + ULONG ContextSwitches; + ULONG ThreadState; + union { + ULONG WaitReason; + typename Traits::Pad padding_for_x64_1; + }; + }; + LARGE_INTEGER alignment_for_x86[8]; }; - typename Traits::Pointer StartAddress; - CLIENT_ID ClientId; - LONG Priority; - LONG BasePriority; - ULONG ContextSwitches; - ULONG ThreadState; +}; + +// There's an extra field in the x64 VM_COUNTERS (or maybe it's VM_COUNTERS_EX, +// it's not clear), so we just make separate specializations for 32/64. +template +class VM_COUNTERS {}; + +template <> +class VM_COUNTERS { + SIZE_T PeakVirtualSize; + SIZE_T VirtualSize; + ULONG PageFaultCount; + SIZE_T PeakWorkingSetSize; + SIZE_T WorkingSetSize; + SIZE_T QuotaPeakPagedPoolUsage; + SIZE_T QuotaPagedPoolUsage; + SIZE_T QuotaPeakNonPagedPoolUsage; + SIZE_T QuotaNonPagedPoolUsage; + SIZE_T PagefileUsage; + SIZE_T PeakPagefileUsage; +}; + +template <> +class VM_COUNTERS { + SIZE_T PeakVirtualSize; + SIZE_T VirtualSize; union { - ULONG WaitReason; - typename Traits::Pad padding_for_x64_1; + ULONG PageFaultCount; + internal::Traits64::Pad padding_for_x64; }; - typename Traits::Pointer StackBase; // These don't appear to be correct. - typename Traits::Pointer StackLimit; - typename Traits::Pointer Win32StartAddress; - typename Traits::Pointer TebBase; - typename Traits::Pointer Reserved; - typename Traits::Pointer Reserved2; - typename Traits::Pointer Reserved3; + SIZE_T PeakWorkingSetSize; + SIZE_T WorkingSetSize; + SIZE_T QuotaPeakPagedPoolUsage; + SIZE_T QuotaPagedPoolUsage; + SIZE_T QuotaPeakNonPagedPoolUsage; + SIZE_T QuotaNonPagedPoolUsage; + SIZE_T PagefileUsage; + SIZE_T PeakPagefileUsage; + SIZE_T PrivateUsage; }; // See http://undocumented.ntinternals.net/source/usermode/undocumented%20functions/system%20information/structures/system_process_information.html @@ -348,7 +382,10 @@ template struct SYSTEM_PROCESS_INFORMATION { ULONG NextEntryOffset; ULONG NumberOfThreads; - LARGE_INTEGER Reserved[3]; + LARGE_INTEGER WorkingSetPrivateSize; + ULONG HardFaultCount; + ULONG NumberOfThreadsHighWatermark; + ULONGLONG CycleTime; LARGE_INTEGER CreateTime; LARGE_INTEGER UserTime; LARGE_INTEGER KernelTime; @@ -366,29 +403,28 @@ struct SYSTEM_PROCESS_INFORMATION { typename Traits::Pad padding_for_x64_2; }; ULONG HandleCount; - ULONG Reserved2[3]; - SIZE_T PeakVirtualSize; - SIZE_T VirtualSize; + ULONG SessionId; + typename Traits::Pointer UniqueProcessKey; union { - ULONG PageFaultCount; - typename Traits::Pad padding_for_x64_3; + VM_COUNTERS VirtualMemoryCounters; + LARGE_INTEGER alignment_for_x86[6]; }; - SIZE_T PeakWorkingSetSize; - SIZE_T WorkingSetSize; - SIZE_T QuotaPeakPagedPoolUsage; - SIZE_T QuotaPagedPoolUsage; - SIZE_T QuotaPeakNonPagedPoolUsage; - SIZE_T QuotaNonPagedPoolUsage; - SIZE_T PagefileUsage; - SIZE_T PeakPagefileUsage; - SIZE_T PrivatePageCount; - LARGE_INTEGER ReadOperationCount; - LARGE_INTEGER WriteOperationCount; - LARGE_INTEGER OtherOperationCount; - LARGE_INTEGER ReadTransferCount; - LARGE_INTEGER WriteTransferCount; - LARGE_INTEGER OtherTransferCount; - SYSTEM_EXTENDED_THREAD_INFORMATION Threads[1]; + IO_COUNTERS IoCounters; + SYSTEM_THREAD_INFORMATION Threads[1]; +}; + +// http://undocumented.ntinternals.net/source/usermode/structures/thread_basic_information.html +template +struct THREAD_BASIC_INFORMATION { + union { + NTSTATUS ExitStatus; + typename Traits::Pad padding_for_x64_0; + }; + typename Traits::Pointer TebBaseAddress; + CLIENT_ID ClientId; + typename Traits::Pointer AffinityMask; + ULONG Priority; + LONG BasePriority; }; #pragma pack(pop)