From bf556829d90992dfb777d8294f5c19e56527eaa2 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Fri, 18 Sep 2015 16:06:05 -0700 Subject: [PATCH] win: support x64 reading x86 (wow64) Removes the bitness-specific targets in favour of pulling binaries from the other build directory. This is to avoid the added complexity of duplicating all the targets for the x86 in x64 build. Overall, mostly templatizing more functions to support the wow64-flavoured structures. The only additional functionality required is reading the x86 TEB that's chained from the x64 TEB when running as WOW64. The crashing child test was switched to a manual CreateProcess because it needs to launch a binary other than itself. R=mark@chromium.org BUG=crashpad:50 Review URL: https://codereview.chromium.org/1349313003 . --- build/gyp_crashpad.py | 24 ++- build/run_tests.py | 5 +- snapshot/snapshot_test.gyp | 22 +++ snapshot/win/cpu_context_win.cc | 112 ++++++----- .../crashpad_snapshot_test_crashing_child.cc | 43 ++++ snapshot/win/exception_snapshot_win.cc | 74 +++---- snapshot/win/exception_snapshot_win.h | 12 +- snapshot/win/exception_snapshot_win_test.cc | 185 +++++++++--------- snapshot/win/process_reader_win.cc | 80 +++++--- snapshot/win/process_reader_win.h | 10 +- snapshot/win/process_reader_win_test.cc | 4 +- snapshot/win/thread_snapshot_win.cc | 10 +- util/util_test.gyp | 29 +-- util/win/process_info_test.cc | 32 +-- util/win/process_structs.h | 33 +++- 15 files changed, 404 insertions(+), 271 deletions(-) create mode 100644 snapshot/win/crashpad_snapshot_test_crashing_child.cc diff --git a/build/gyp_crashpad.py b/build/gyp_crashpad.py index 8e342ff..42818ec 100755 --- a/build/gyp_crashpad.py +++ b/build/gyp_crashpad.py @@ -24,6 +24,7 @@ sys.path.insert( import gyp + def main(args): if 'GYP_GENERATORS' not in os.environ: os.environ['GYP_GENERATORS'] = 'ninja' @@ -31,16 +32,25 @@ def main(args): crashpad_dir_or_dot = crashpad_dir if crashpad_dir is not '' else '.' args.extend(['-D', 'crashpad_standalone=1']) - args.extend(['--include', os.path.join(crashpad_dir, - 'third_party', - 'mini_chromium', - 'mini_chromium', - 'build', - 'common.gypi')]) + args.extend(['--include', + os.path.join(crashpad_dir, 'third_party', 'mini_chromium', + 'mini_chromium', 'build', 'common.gypi')]) args.extend(['--depth', crashpad_dir_or_dot]) args.append(os.path.join(crashpad_dir, 'crashpad.gyp')) - return gyp.main(args) + result = gyp.main(args) + if result != 0: + return result + + if sys.platform == 'win32': + # Also generate the x86 build. + result = gyp.main(args + ['-D', 'target_arch=ia32', '-G', 'config=Debug']) + if result != 0: + return result + result = gyp.main(args + ['-D', 'target_arch=ia32', '-G', 'config=Release']) + + return result + if __name__ == '__main__': sys.exit(main(sys.argv[1:])) diff --git a/build/run_tests.py b/build/run_tests.py index 8de6fc8..bc387a1 100755 --- a/build/run_tests.py +++ b/build/run_tests.py @@ -25,8 +25,9 @@ import sys # location in the recipe. def main(args): if len(args) != 1: - print >>sys.stderr, 'usage: run_tests.py {Debug|Release}' - return 1; + print >> sys.stderr, \ + 'usage: run_tests.py {Debug|Release|Debug_x64|Release_x64}' + return 1 crashpad_dir = \ os.path.join(os.path.dirname(os.path.abspath(__file__)), os.pardir) diff --git a/snapshot/snapshot_test.gyp b/snapshot/snapshot_test.gyp index 2349900..5ccd481 100644 --- a/snapshot/snapshot_test.gyp +++ b/snapshot/snapshot_test.gyp @@ -93,6 +93,11 @@ ], }, }], + ['OS=="win"', { + 'dependencies': [ + 'crashpad_snapshot_test_crashing_child', + ], + }], ], }, { @@ -129,5 +134,22 @@ }, ], }], + ['OS=="win"', { + 'targets': [ + { + 'target_name': 'crashpad_snapshot_test_crashing_child', + 'type': 'executable', + 'dependencies': [ + '../client/client.gyp:crashpad_client', + '../compat/compat.gyp:crashpad_compat', + '../third_party/mini_chromium/mini_chromium.gyp:base', + '../util/util.gyp:crashpad_util', + ], + 'sources': [ + 'win/crashpad_snapshot_test_crashing_child.cc', + ], + }, + ], + }], ], } diff --git a/snapshot/win/cpu_context_win.cc b/snapshot/win/cpu_context_win.cc index b2389ce..d4ce66d 100644 --- a/snapshot/win/cpu_context_win.cc +++ b/snapshot/win/cpu_context_win.cc @@ -21,10 +21,70 @@ namespace crashpad { +namespace { + +template +void CommonInitializeX86Context(const T& context, CPUContextX86* out) { + LOG_IF(ERROR, !(context.ContextFlags & WOW64_CONTEXT_i386)) + << "non-x86 context"; + memset(out, 0, sizeof(*out)); + + // We assume in this function that the WOW64_CONTEXT_* and x86 CONTEXT_* + // values for ContextFlags are identical. + + if (context.ContextFlags & WOW64_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 & WOW64_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 & WOW64_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 & WOW64_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 & WOW64_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 & WOW64_CONTEXT_FLOATING_POINT) { + CHECK(false) << "TODO(scottmg): extract x87 data"; + } +} + +} // namespace + #if defined(ARCH_CPU_64_BITS) void InitializeX86Context(const WOW64_CONTEXT& context, CPUContextX86* out) { - CHECK(false) << "TODO(scottmg) InitializeX86Context()"; + CommonInitializeX86Context(context, out); } void InitializeX64Context(const CONTEXT& context, CPUContextX86_64* out) { @@ -88,55 +148,7 @@ void InitializeX64Context(const CONTEXT& context, CPUContextX86_64* out) { #else // ARCH_CPU_64_BITS void InitializeX86Context(const CONTEXT& context, CPUContextX86* out) { - 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"; - } + CommonInitializeX86Context(context, out); } #endif // ARCH_CPU_64_BITS diff --git a/snapshot/win/crashpad_snapshot_test_crashing_child.cc b/snapshot/win/crashpad_snapshot_test_crashing_child.cc new file mode 100644 index 0000000..d74f6a6 --- /dev/null +++ b/snapshot/win/crashpad_snapshot_test_crashing_child.cc @@ -0,0 +1,43 @@ +// Copyright 2015 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 +#include + +#include "base/logging.h" +#include "client/crashpad_client.h" +#include "util/file/file_io.h" +#include "util/win/address_types.h" + +__declspec(noinline) crashpad::WinVMAddress CurrentAddress() { + return reinterpret_cast(_ReturnAddress()); +} + +int main(int argc, char* argv[]) { + CHECK_EQ(argc, 2); + + crashpad::CrashpadClient client; + CHECK(client.SetHandler(argv[1])); + CHECK(client.UseHandler()); + + HANDLE out = GetStdHandle(STD_OUTPUT_HANDLE); + CHECK_NE(out, INVALID_HANDLE_VALUE); + crashpad::WinVMAddress break_address = CurrentAddress(); + crashpad::CheckedWriteFile(out, &break_address, sizeof(break_address)); + + __debugbreak(); + + return 0; +} diff --git a/snapshot/win/exception_snapshot_win.cc b/snapshot/win/exception_snapshot_win.cc index 1156baf..731669a 100644 --- a/snapshot/win/exception_snapshot_win.cc +++ b/snapshot/win/exception_snapshot_win.cc @@ -17,7 +17,6 @@ #include "snapshot/win/cpu_context_win.h" #include "snapshot/win/process_reader_win.h" #include "util/win/nt_internals.h" -#include "util/win/process_structs.h" namespace crashpad { namespace internal { @@ -51,48 +50,41 @@ bool ExceptionSnapshotWin::Initialize(ProcessReaderWin* process_reader, } if (!found_thread) { - LOG(ERROR) << "thread ID " << thread_id << "not found in process"; + LOG(ERROR) << "thread ID " << thread_id << " not found in process"; return false; } else { thread_id_ = thread_id; } - EXCEPTION_POINTERS exception_pointers; - if (!process_reader->ReadMemory(exception_pointers_address, - sizeof(EXCEPTION_POINTERS), - &exception_pointers)) { - LOG(ERROR) << "EXCEPTION_POINTERS read failed"; - return false; - } - if (!exception_pointers.ExceptionRecord) { - LOG(ERROR) << "null ExceptionRecord"; - return false; - } - -#if defined(ARCH_CPU_64_BITS) - if (process_reader->Is64Bit()) { +#if defined(ARCH_CPU_32_BITS) + const bool is_64_bit = false; + using Context32 = CONTEXT; +#elif defined(ARCH_CPU_64_BITS) + const bool is_64_bit = process_reader->Is64Bit(); + using Context32 = WOW64_CONTEXT; + if (is_64_bit) { CONTEXT context_record; - if (!InitializeFromExceptionPointers( - *process_reader, exception_pointers, &context_record)) { + if (!InitializeFromExceptionPointers( + *process_reader, exception_pointers_address, &context_record)) { return false; } context_.architecture = kCPUArchitectureX86_64; context_.x86_64 = &context_union_.x86_64; InitializeX64Context(context_record, context_.x86_64); - } else { - CHECK(false) << "TODO(scottmg) WOW64"; - return false; } -#else - CONTEXT context_record; - if (!InitializeFromExceptionPointers( - *process_reader, exception_pointers, &context_record)) { - return false; +#endif + if (!is_64_bit) { + Context32 context_record; + if (!InitializeFromExceptionPointers( + *process_reader, exception_pointers_address, &context_record)) { + return false; + } + context_.architecture = kCPUArchitectureX86; + context_.x86 = &context_union_.x86; + InitializeX86Context(context_record, context_.x86); } - 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; @@ -128,14 +120,28 @@ const std::vector& ExceptionSnapshotWin::Codes() const { return codes_; } -template +template bool ExceptionSnapshotWin::InitializeFromExceptionPointers( const ProcessReaderWin& process_reader, - const EXCEPTION_POINTERS& exception_pointers, + WinVMAddress exception_pointers_address, ContextType* context_record) { + ExceptionPointersType exception_pointers; + if (!process_reader.ReadMemory(exception_pointers_address, + sizeof(exception_pointers), + &exception_pointers)) { + LOG(ERROR) << "EXCEPTION_POINTERS read failed"; + return false; + } + if (!exception_pointers.ExceptionRecord) { + LOG(ERROR) << "null ExceptionRecord"; + return false; + } + ExceptionRecordType first_record; if (!process_reader.ReadMemory( - reinterpret_cast(exception_pointers.ExceptionRecord), + static_cast(exception_pointers.ExceptionRecord), sizeof(first_record), &first_record)) { LOG(ERROR) << "ExceptionRecord"; @@ -152,7 +158,7 @@ bool ExceptionSnapshotWin::InitializeFromExceptionPointers( } if (!process_reader.ReadMemory( - reinterpret_cast(exception_pointers.ContextRecord), + static_cast(exception_pointers.ContextRecord), sizeof(*context_record), context_record)) { LOG(ERROR) << "ContextRecord"; diff --git a/snapshot/win/exception_snapshot_win.h b/snapshot/win/exception_snapshot_win.h index 277cd4a..1688b12 100644 --- a/snapshot/win/exception_snapshot_win.h +++ b/snapshot/win/exception_snapshot_win.h @@ -24,6 +24,7 @@ #include "snapshot/exception_snapshot.h" #include "util/misc/initialization_state_dcheck.h" #include "util/win/address_types.h" +#include "util/win/process_structs.h" namespace crashpad { @@ -61,11 +62,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); + template + bool InitializeFromExceptionPointers(const ProcessReaderWin& process_reader, + WinVMAddress exception_pointers_address, + ContextType* context_record); #if defined(ARCH_CPU_X86_FAMILY) union { diff --git a/snapshot/win/exception_snapshot_win_test.cc b/snapshot/win/exception_snapshot_win_test.cc index db558cd..7d7f18e 100644 --- a/snapshot/win/exception_snapshot_win_test.cc +++ b/snapshot/win/exception_snapshot_win_test.cc @@ -16,11 +16,14 @@ #include +#include "base/files/file_path.h" #include "base/strings/stringprintf.h" +#include "base/strings/string16.h" #include "base/strings/utf_string_conversions.h" #include "client/crashpad_client.h" #include "gtest/gtest.h" #include "snapshot/win/process_snapshot_win.h" +#include "test/paths.h" #include "test/win/win_child_process.h" #include "util/thread/thread.h" #include "util/win/exception_handler_server.h" @@ -46,56 +49,49 @@ HANDLE DuplicateEvent(HANDLE process, HANDLE event) { return nullptr; } -class ExceptionSnapshotWinTest : public testing::Test { +class Delegate : public ExceptionHandlerServer::Delegate { public: - class Delegate : public ExceptionHandlerServer::Delegate { - public: - Delegate(HANDLE server_ready, HANDLE completed_test_event) - : server_ready_(server_ready), - completed_test_event_(completed_test_event), - break_near_(nullptr) {} - ~Delegate() override {} + Delegate(HANDLE server_ready, HANDLE completed_test_event) + : server_ready_(server_ready), + completed_test_event_(completed_test_event), + break_near_(0) {} + ~Delegate() override {} - void set_break_near(void* break_near) { break_near_ = break_near; } + void set_break_near(WinVMAddress break_near) { break_near_ = break_near; } - void ExceptionHandlerServerStarted() override { SetEvent(server_ready_); } + void ExceptionHandlerServerStarted() override { SetEvent(server_ready_); } - unsigned int ExceptionHandlerServerException( - HANDLE process, - WinVMAddress exception_information_address) override { - ScopedProcessSuspend suspend(process); - ProcessSnapshotWin snapshot; - snapshot.Initialize(process, ProcessSuspensionState::kSuspended); - snapshot.InitializeException(exception_information_address); + unsigned int ExceptionHandlerServerException( + HANDLE process, + WinVMAddress exception_information_address) override { + ScopedProcessSuspend suspend(process); + ProcessSnapshotWin snapshot; + snapshot.Initialize(process, ProcessSuspensionState::kSuspended); + snapshot.InitializeException(exception_information_address); - // Confirm the exception record was read correctly. - EXPECT_NE(snapshot.Exception()->ThreadID(), 0u); - EXPECT_EQ(snapshot.Exception()->Exception(), EXCEPTION_BREAKPOINT); + // Confirm the exception record was read correctly. + EXPECT_NE(snapshot.Exception()->ThreadID(), 0u); + EXPECT_EQ(snapshot.Exception()->Exception(), EXCEPTION_BREAKPOINT); - // Verify the exception happened at the expected location with a bit of - // slop space to allow for reading the current PC before the exception - // happens. See CrashingChildProcess::Run(). - const uint64_t kAllowedOffset = 64; - EXPECT_GT(snapshot.Exception()->ExceptionAddress(), - reinterpret_cast(break_near_)); - EXPECT_LT(snapshot.Exception()->ExceptionAddress(), - reinterpret_cast(break_near_) + kAllowedOffset); + // Verify the exception happened at the expected location with a bit of + // slop space to allow for reading the current PC before the exception + // happens. See CrashingChildProcess::Run(). + const uint64_t kAllowedOffset = 64; + EXPECT_GT(snapshot.Exception()->ExceptionAddress(), break_near_); + EXPECT_LT(snapshot.Exception()->ExceptionAddress(), + break_near_ + kAllowedOffset); - SetEvent(completed_test_event_); + SetEvent(completed_test_event_); - return snapshot.Exception()->Exception(); - } - - private: - HANDLE server_ready_; // weak - HANDLE completed_test_event_; // weak - void* break_near_; - - DISALLOW_COPY_AND_ASSIGN(Delegate); - }; + return snapshot.Exception()->Exception(); + } private: - ScopedKernelHANDLE exception_happened_; + HANDLE server_ready_; // weak + HANDLE completed_test_event_; // weak + WinVMAddress break_near_; + + DISALLOW_COPY_AND_ASSIGN(Delegate); }; // Runs the ExceptionHandlerServer on a background thread. @@ -136,51 +132,7 @@ class ScopedStopServerAndJoinThread { DISALLOW_COPY_AND_ASSIGN(ScopedStopServerAndJoinThread); }; -std::string ReadString(FileHandle handle) { - size_t length = 0; - EXPECT_TRUE(LoggingReadFile(handle, &length, sizeof(length))); - scoped_ptr buffer(new char[length]); - EXPECT_TRUE(LoggingReadFile(handle, &buffer[0], length)); - return std::string(&buffer[0], length); -} - -void WriteString(FileHandle handle, const std::string& str) { - size_t length = str.size(); - EXPECT_TRUE(LoggingWriteFile(handle, &length, sizeof(length))); - EXPECT_TRUE(LoggingWriteFile(handle, &str[0], length)); -} - -__declspec(noinline) void* CurrentAddress() { - return _ReturnAddress(); -} - -class CrashingChildProcess final : public WinChildProcess { - public: - CrashingChildProcess() : WinChildProcess() {} - ~CrashingChildProcess() {} - - private: - int Run() override { - std::string pipe_name = ReadString(ReadPipeHandle()); - CrashpadClient client; - EXPECT_TRUE(client.SetHandler(pipe_name)); - EXPECT_TRUE(client.UseHandler()); - // Save the address where we're about to crash so the exception handler can - // verify it's in approximately the right location (with a bit of fudge for - // the code between here and the __debugbreak()). - void* break_address = CurrentAddress(); - LoggingWriteFile(WritePipeHandle(), &break_address, sizeof(break_address)); - __debugbreak(); - return 0; - }; -}; - -TEST_F(ExceptionSnapshotWinTest, ChildCrash) { - // Spawn a child process that will immediately crash (once we let it - // run below by telling it what to connect to). - WinChildProcess::EntryPoint(); - scoped_ptr handle = WinChildProcess::Launch(); - +void TestCrashingChild(const base::string16& directory_modification) { // Set up the registration server on a background thread. std::string pipe_name = "\\\\.\\pipe\\handler_test_pipe_" + base::StringPrintf("%08x", GetCurrentProcessId()); @@ -196,19 +148,74 @@ TEST_F(ExceptionSnapshotWinTest, ChildCrash) { &exception_handler_server, &server_thread); WaitForSingleObject(server_ready.get(), INFINITE); - // Allow the child to continue and tell it where to connect to. - WriteString(handle->write.get(), pipe_name); + + // 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"_crashing_child.exe") + .value(); + + // Create a pipe for the stdout of the child. + SECURITY_ATTRIBUTES security_attributes = {0}; + security_attributes.nLength = sizeof(SECURITY_ATTRIBUTES); + security_attributes.bInheritHandle = true; + HANDLE stdout_read; + HANDLE stdout_write; + ASSERT_TRUE(CreatePipe(&stdout_read, &stdout_write, &security_attributes, 0)); + ScopedFileHANDLE read_handle(stdout_read); + ScopedFileHANDLE write_handle(stdout_write); + ASSERT_TRUE(SetHandleInformation(read_handle.get(), HANDLE_FLAG_INHERIT, 0)); + + std::wstring command_line = + child_test_executable + L" " + base::UTF8ToUTF16(pipe_name); + STARTUPINFO startup_info = {0}; + startup_info.cb = sizeof(startup_info); + startup_info.hStdInput = GetStdHandle(STD_INPUT_HANDLE); + startup_info.hStdOutput = write_handle.get(); + startup_info.hStdError = GetStdHandle(STD_ERROR_HANDLE); + startup_info.dwFlags = STARTF_USESTDHANDLES; + PROCESS_INFORMATION process_information; + ASSERT_TRUE(CreateProcess(child_test_executable.c_str(), + &command_line[0], + nullptr, + nullptr, + true, + 0, + nullptr, + nullptr, + &startup_info, + &process_information)); + // Take ownership of the two process handles returned. + ScopedKernelHANDLE process_main_thread_handle(process_information.hThread); + ScopedKernelHANDLE process_handle(process_information.hProcess); // The child tells us (approximately) where it will crash. - void* break_near_address; + WinVMAddress break_near_address; LoggingReadFile( - handle->read.get(), &break_near_address, sizeof(break_near_address)); + read_handle.get(), &break_near_address, sizeof(break_near_address)); delegate.set_break_near(break_near_address); // Wait for the child to crash and the exception information to be validated. WaitForSingleObject(completed.get(), INFINITE); } +TEST(ExceptionSnapshotWinTest, ChildCrash) { + TestCrashingChild(FILE_PATH_LITERAL(".")); +} + +#if defined(ARCH_CPU_64_BITS) +TEST(ExceptionSnapshotWinTest, ChildCrashWOW64) { +#ifndef NDEBUG + TestCrashingChild(FILE_PATH_LITERAL("..\\..\\out\\Debug")); +#else + TestCrashingChild(FILE_PATH_LITERAL("..\\..\\out\\Release")); +#endif +} +#endif // ARCH_CPU_64_BITS + } // namespace } // namespace test } // namespace crashpad diff --git a/snapshot/win/process_reader_win.cc b/snapshot/win/process_reader_win.cc index 81a8670..487bf6c 100644 --- a/snapshot/win/process_reader_win.cc +++ b/snapshot/win/process_reader_win.cc @@ -116,23 +116,23 @@ HANDLE OpenThread( template bool FillThreadContextAndSuspendCount(HANDLE thread_handle, ProcessReaderWin::Thread* thread, - ProcessSuspensionState suspension_state) { + ProcessSuspensionState suspension_state, + bool is_64_reading_32) { // 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->id == reinterpret_cast*>( NtCurrentTeb())->ClientId.UniqueThread; - // TODO(scottmg): Handle cross-bitness in this function. - if (is_current_thread) { DCHECK(suspension_state == ProcessSuspensionState::kRunning); thread->suspend_count = 0; - RtlCaptureContext(&thread->context); + DCHECK(!is_64_reading_32); + RtlCaptureContext(&thread->context.native); } else { DWORD previous_suspend_count = SuspendThread(thread_handle); if (previous_suspend_count == -1) { - PLOG(ERROR) << "SuspendThread failed"; + PLOG(ERROR) << "SuspendThread"; return false; } DCHECK(previous_suspend_count > 0 || @@ -142,14 +142,28 @@ bool FillThreadContextAndSuspendCount(HANDLE thread_handle, (suspension_state == ProcessSuspensionState::kSuspended ? 1 : 0); memset(&thread->context, 0, sizeof(thread->context)); - thread->context.ContextFlags = CONTEXT_ALL; - if (!GetThreadContext(thread_handle, &thread->context)) { - PLOG(ERROR) << "GetThreadContext failed"; - return false; +#if defined(ARCH_CPU_32_BITS) + const bool is_native = true; +#elif defined(ARCH_CPU_64_BITS) + const bool is_native = !is_64_reading_32; + if (is_64_reading_32) { + thread->context.wow64.ContextFlags = CONTEXT_ALL; + if (!Wow64GetThreadContext(thread_handle, &thread->context.wow64)) { + PLOG(ERROR) << "Wow64GetThreadContext"; + return false; + } + } +#endif + if (is_native) { + thread->context.native.ContextFlags = CONTEXT_ALL; + if (!GetThreadContext(thread_handle, &thread->context.native)) { + PLOG(ERROR) << "GetThreadContext"; + return false; + } } if (!ResumeThread(thread_handle)) { - PLOG(ERROR) << "ResumeThread failed"; + PLOG(ERROR) << "ResumeThread"; return false; } } @@ -242,10 +256,11 @@ const std::vector& ProcessReaderWin::Threads() { initialized_threads_ = true; - if (process_info_.Is64Bit()) - ReadThreadData(); - else - ReadThreadData(); +#if defined(ARCH_CPU_64_BITS) + ReadThreadData(process_info_.IsWow64()); +#else + ReadThreadData(false); +#endif return threads_; } @@ -261,7 +276,7 @@ const std::vector& ProcessReaderWin::Modules() { } template -void ProcessReaderWin::ReadThreadData() { +void ProcessReaderWin::ReadThreadData(bool is_64_reading_32) { DCHECK(threads_.empty()); scoped_ptr buffer; @@ -280,8 +295,10 @@ void ProcessReaderWin::ReadThreadData() { if (!thread_handle.is_valid()) continue; - if (!FillThreadContextAndSuspendCount( - thread_handle.get(), &thread, suspension_state_)) { + if (!FillThreadContextAndSuspendCount(thread_handle.get(), + &thread, + suspension_state_, + is_64_reading_32)) { continue; } @@ -309,15 +326,32 @@ void ProcessReaderWin::ReadThreadData() { // 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)) { + thread.teb = thread_basic_info.TebBaseAddress; + if (ReadMemory(thread.teb, sizeof(tib), &tib)) { + WinVMAddress base = 0; + WinVMAddress limit = 0; + // If we're reading a WOW64 process, then the TIB we just retrieved is the + // x64 one. The first word of the x64 TIB points at the x86 TIB. See + // https://msdn.microsoft.com/en-us/library/dn424783.aspx + if (is_64_reading_32) { + process_types::NT_TIB tib32; + thread.teb = tib.Wow64Teb; + if (ReadMemory(thread.teb, sizeof(tib32), &tib32)) { + base = tib32.StackBase; + limit = tib32.StackLimit; + } + } else { + base = tib.StackBase; + limit = tib.StackLimit; + } + // 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_address = limit; + if (limit > base) { + LOG(ERROR) << "invalid stack range: " << base << " - " << limit; thread.stack_region_size = 0; } else { - thread.stack_region_size = tib.StackBase - tib.StackLimit; + thread.stack_region_size = base - limit; } } threads_.push_back(thread); diff --git a/snapshot/win/process_reader_win.h b/snapshot/win/process_reader_win.h index f23d0d5..73ad1c6 100644 --- a/snapshot/win/process_reader_win.h +++ b/snapshot/win/process_reader_win.h @@ -20,6 +20,7 @@ #include +#include "build/build_config.h" #include "util/misc/initialization_state_dcheck.h" #include "util/win/address_types.h" #include "util/win/process_info.h" @@ -43,7 +44,12 @@ class ProcessReaderWin { Thread(); ~Thread() {} - CONTEXT context; + union { + CONTEXT native; +#if defined(ARCH_CPU_64_BITS) + WOW64_CONTEXT wow64; +#endif; + } context; uint64_t id; WinVMAddress teb; WinVMAddress stack_region_address; @@ -108,7 +114,7 @@ class ProcessReaderWin { private: template - void ReadThreadData(); + void ReadThreadData(bool is_64_reading_32); HANDLE process_; ProcessInfo process_info_; diff --git a/snapshot/win/process_reader_win_test.cc b/snapshot/win/process_reader_win_test.cc index 9e2e52c..a87b945 100644 --- a/snapshot/win/process_reader_win_test.cc +++ b/snapshot/win/process_reader_win_test.cc @@ -108,9 +108,9 @@ TEST(ProcessReaderWin, SelfOneThread) { EXPECT_EQ(GetCurrentThreadId(), threads[0].id); #if defined(ARCH_CPU_64_BITS) - EXPECT_NE(0, threads[0].context.Rip); + EXPECT_NE(0, threads[0].context.native.Rip); #else - EXPECT_NE(0u, threads[0].context.Eip); + EXPECT_NE(0u, threads[0].context.native.Eip); #endif EXPECT_EQ(0, threads[0].suspend_count); diff --git a/snapshot/win/thread_snapshot_win.cc b/snapshot/win/thread_snapshot_win.cc index 7524e45..ce4b309 100644 --- a/snapshot/win/thread_snapshot_win.cc +++ b/snapshot/win/thread_snapshot_win.cc @@ -41,20 +41,16 @@ bool ThreadSnapshotWin::Initialize( if (process_reader->Is64Bit()) { context_.architecture = kCPUArchitectureX86_64; context_.x86_64 = &context_union_.x86_64; - InitializeX64Context(process_reader_thread.context, context_.x86_64); + InitializeX64Context(process_reader_thread.context.native, context_.x86_64); } else { context_.architecture = kCPUArchitectureX86; context_.x86 = &context_union_.x86; - InitializeX86Context( - *reinterpret_cast(&process_reader_thread.context), - context_.x86); + InitializeX86Context(process_reader_thread.context.wow64, context_.x86); } #else context_.architecture = kCPUArchitectureX86; context_.x86 = &context_union_.x86; - InitializeX86Context( - *reinterpret_cast(&process_reader_thread.context), - context_.x86); + InitializeX86Context(process_reader_thread.context.native, context_.x86); #endif // ARCH_CPU_X86_64 INITIALIZATION_STATE_SET_VALID(initialized_); diff --git a/util/util_test.gyp b/util/util_test.gyp index 88bbf27..1010b5e 100644 --- a/util/util_test.gyp +++ b/util/util_test.gyp @@ -94,8 +94,7 @@ }], ['OS=="win"', { 'dependencies': [ - 'crashpad_util_test_process_info_test_child_x64', - 'crashpad_util_test_process_info_test_child_x86', + 'crashpad_util_test_process_info_test_child', ], 'link_settings': { 'libraries': [ @@ -111,12 +110,11 @@ ['OS=="win"', { 'targets': [ { - 'target_name': 'crashpad_util_test_process_info_test_child_x64', + 'target_name': 'crashpad_util_test_process_info_test_child', 'type': 'executable', 'sources': [ 'win/process_info_test_child.cc', ], - 'msvs_configuration_platform': 'x64', # Set an unusually high load address to make sure that the main # executable still appears as the first element in # ProcessInfo::Modules(). @@ -126,29 +124,6 @@ '/BASE:0x78000000', '/FIXED', ], - 'MinimumRequiredVersion': '5.02', # Server 2003. - 'TargetMachine': '17', # x64. - }, - }, - }, - { - # Same as above, but explicitly x86 to test 64->32 access. - 'target_name': 'crashpad_util_test_process_info_test_child_x86', - 'type': 'executable', - 'sources': [ - 'win/process_info_test_child.cc', - ], - 'msvs_configuration_platform': 'x86', - # Set an unusually high load address to make sure that the main - # executable still appears as the first element in - # ProcessInfo::Modules(). - 'msvs_settings': { - 'VCLinkerTool': { - 'AdditionalOptions': [ - '/BASE:0x78000000', - '/FIXED', - ], - 'TargetMachine': '1', # x86. }, }, }, diff --git a/util/win/process_info_test.cc b/util/win/process_info_test.cc index a82a6bd..de1d529 100644 --- a/util/win/process_info_test.cc +++ b/util/win/process_info_test.cc @@ -103,7 +103,7 @@ TEST(ProcessInfo, Self) { EXPECT_GT(modules[1].timestamp, 0); } -void TestOtherProcess(const std::wstring& child_name_suffix) { +void TestOtherProcess(const base::string16& directory_modification) { ProcessInfo process_info; ::UUID system_uuid; @@ -120,9 +120,13 @@ void TestOtherProcess(const std::wstring& child_name_suffix) { ASSERT_TRUE(done.get()); base::FilePath test_executable = Paths::Executable(); + std::wstring child_test_executable = - test_executable.RemoveFinalExtension().value() + - L"_process_info_test_child_" + child_name_suffix + L".exe"; + test_executable.DirName() + .Append(directory_modification) + .Append(test_executable.BaseName().RemoveFinalExtension().value() + + L"_process_info_test_child.exe") + .value(); // TODO(scottmg): Command line escaping utility. std::wstring command_line = child_test_executable + L" " + started_uuid.ToString16() + L" " + @@ -155,8 +159,7 @@ void TestOtherProcess(const std::wstring& child_name_suffix) { std::vector modules; EXPECT_TRUE(process_info.Modules(&modules)); ASSERT_GE(modules.size(), 3u); - std::wstring child_name = L"\\crashpad_util_test_process_info_test_child_" + - child_name_suffix + L".exe"; + std::wstring child_name = L"\\crashpad_util_test_process_info_test_child.exe"; ASSERT_GE(modules[0].name.size(), child_name.size()); EXPECT_EQ(child_name, modules[0].name.substr(modules[0].name.size() - child_name.size())); @@ -173,19 +176,20 @@ void TestOtherProcess(const std::wstring& child_name_suffix) { wcslen(kLz32dllName))); } -// This test can't run the child if the host OS is x86, and can't read from the -// child if it is x86 and the child is x64, so it only makes sense to run this -// if we built as x64. +TEST(ProcessInfo, OtherProcess) { + TestOtherProcess(FILE_PATH_LITERAL(".")); +} + #if defined(ARCH_CPU_64_BITS) -TEST(ProcessInfo, OtherProcessX64) { - TestOtherProcess(L"x64"); +TEST(ProcessInfo, OtherProcessWOW64) { +#ifndef NDEBUG + TestOtherProcess(FILE_PATH_LITERAL("..\\..\\out\\Debug")); +#else + TestOtherProcess(FILE_PATH_LITERAL("..\\..\\out\\Release")); +#endif } #endif // ARCH_CPU_64_BITS -TEST(ProcessInfo, OtherProcessX86) { - TestOtherProcess(L"x86"); -} - } // namespace } // namespace test } // namespace crashpad diff --git a/util/win/process_structs.h b/util/win/process_structs.h index 7f4e4f5..79f6515 100644 --- a/util/win/process_structs.h +++ b/util/win/process_structs.h @@ -283,16 +283,22 @@ struct PEB { template struct NT_TIB { - typename Traits::Pointer ExceptionList; - typename Traits::Pointer StackBase; - typename Traits::Pointer StackLimit; - typename Traits::Pointer SubSystemTib; union { - typename Traits::Pointer FiberData; - BYTE Version[4]; + // See https://msdn.microsoft.com/en-us/library/dn424783.aspx. + typename Traits::Pointer Wow64Teb; + struct { + typename Traits::Pointer ExceptionList; + typename Traits::Pointer StackBase; + typename Traits::Pointer StackLimit; + typename Traits::Pointer SubSystemTib; + union { + typename Traits::Pointer FiberData; + BYTE Version[4]; + }; + typename Traits::Pointer ArbitraryUserPointer; + typename Traits::Pointer Self; + }; }; - typename Traits::Pointer ArbitraryUserPointer; - typename Traits::Pointer Self; }; // See https://msdn.microsoft.com/en-us/library/gg750647.aspx. @@ -417,7 +423,7 @@ struct SYSTEM_PROCESS_INFORMATION { template struct THREAD_BASIC_INFORMATION { union { - NTSTATUS ExitStatus; + LONG ExitStatus; typename Traits::Pad padding_for_x64_0; }; typename Traits::Pointer TebBaseAddress; @@ -427,6 +433,15 @@ struct THREAD_BASIC_INFORMATION { LONG BasePriority; }; +template +struct EXCEPTION_POINTERS { + typename Traits::Pointer ExceptionRecord; + typename Traits::Pointer ContextRecord; +}; + +using EXCEPTION_POINTERS32 = EXCEPTION_POINTERS; +using EXCEPTION_POINTERS64 = EXCEPTION_POINTERS; + #pragma pack(pop) //! \}