From bbd00c3a91fac9a25a0351337602e6477987a7b7 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Fri, 9 Oct 2015 13:39:39 -0700 Subject: [PATCH 01/18] win: Test some basic ! windbg commands R=mark@chromium.org BUG=crashpad:20, crashpad:46, crashpad:52 Review URL: https://codereview.chromium.org/1397833004 . --- build/run_tests.py | 2 +- snapshot/win/end_to_end_test.py | 162 ++++++++++++++++++++++++++++++-- 2 files changed, 154 insertions(+), 10 deletions(-) diff --git a/build/run_tests.py b/build/run_tests.py index 9d8935ef..43bb3158 100755 --- a/build/run_tests.py +++ b/build/run_tests.py @@ -64,7 +64,7 @@ def main(args): print name print '-' * 80 subprocess.check_call( - [sys.executable, os.path.join(crashpad_dir, name), args[0]]) + [sys.executable, os.path.join(crashpad_dir, name), out_dir]) return 0 diff --git a/snapshot/win/end_to_end_test.py b/snapshot/win/end_to_end_test.py index 0a13befd..64de3b21 100644 --- a/snapshot/win/end_to_end_test.py +++ b/snapshot/win/end_to_end_test.py @@ -15,7 +15,26 @@ # limitations under the License. import os +import random +import re +import subprocess import sys +import tempfile + +g_temp_dirs = [] + + +def MakeTempDir(): + global g_temp_dirs + new_dir = tempfile.mkdtemp() + g_temp_dirs.append(new_dir) + return new_dir + + +def CleanUpTempDirs(): + global g_temp_dirs + for d in g_temp_dirs: + subprocess.call(['rmdir', '/s', '/q', d], shell=True) def FindInstalledWindowsApplication(app_path): @@ -35,6 +54,9 @@ def FindInstalledWindowsApplication(app_path): def GetCdbPath(): + """Search in some reasonable places to find cdb.exe. Searches x64 before x86 + and newer versions before older versions. + """ possible_paths = ( os.path.join('Windows Kits', '10', 'Debuggers', 'x64'), os.path.join('Windows Kits', '10', 'Debuggers', 'x86'), @@ -44,12 +66,7 @@ def GetCdbPath(): os.path.join('Windows Kits', '8.0', 'Debuggers', 'x86'), 'Debugging Tools For Windows (x64)', 'Debugging Tools For Windows (x86)', - 'Debugging Tools For Windows', - os.path.join('win_toolchain', 'vs2013_files', 'win8sdk', 'Debuggers', - 'x64'), - os.path.join('win_toolchain', 'vs2013_files', 'win8sdk', 'Debuggers', - 'x86'), - ) + 'Debugging Tools For Windows',) for possible_path in possible_paths: app_path = os.path.join(possible_path, 'cdb.exe') app_path = FindInstalledWindowsApplication(app_path) @@ -58,10 +75,137 @@ def GetCdbPath(): return None +def GetDumpFromCrashyProgram(out_dir, pipe_name): + """Initialize a crash database, run crashpad_handler, run crashy_program + connecting to the crash_handler. Returns the minidump generated by + crash_handler for further testing. + """ + test_database = MakeTempDir() + + try: + if subprocess.call( + [os.path.join(out_dir, 'crashpad_database_util.exe'), '--create', + '--database=' + test_database]) != 0: + print 'could not initialize report database' + return None + + handler = subprocess.Popen([ + os.path.join(out_dir, 'crashpad_handler.exe'), + '--pipe-name=' + pipe_name, + '--database=' + test_database + ]) + + subprocess.call([os.path.join(out_dir, 'crashy_program.exe'), pipe_name]) + + out = subprocess.check_output([ + os.path.join(out_dir, 'crashpad_database_util.exe'), + '--database=' + test_database, + '--show-completed-reports', + '--show-all-report-info', + ]) + for line in out.splitlines(): + if line.strip().startswith('Path:'): + return line.partition(':')[2].strip() + + finally: + if handler: + handler.kill() + + +class CdbRun(object): + """Run cdb.exe passing it a cdb command and capturing the output. + `Check()` searches for regex patterns in sequence allowing verification of + expected output. + """ + + def __init__(self, cdb_path, dump_path, command): + # Run a command line that loads the dump, runs the specified cdb command, + # and then quits, and capturing stdout. + self.out = subprocess.check_output([ + cdb_path, + '-z', dump_path, + '-c', command + ';q' + ]) + + def Check(self, pattern, message): + match_obj = re.search(pattern, self.out) + if match_obj: + # Matched. Consume up to end of match. + self.out = self.out[match_obj.end(0):] + print 'ok - %s' % message + else: + print >>sys.stderr, '-' * 80 + print >>sys.stderr, 'FAILED - %s' % message + print >>sys.stderr, '-' * 80 + print >>sys.stderr, 'did not match:\n %s' % pattern + print >>sys.stderr, '-' * 80 + print >>sys.stderr, 'remaining output was:\n %s' % self.out + print >>sys.stderr, '-' * 80 + sys.exit(1) + + +def RunTests(cdb_path, dump_path, pipe_name): + """Runs various tests in sequence. Runs a new cdb instance on the dump for + each block of tests to reduce the chances that output from one command is + confused for output from another. + """ + out = CdbRun(cdb_path, dump_path, '.ecxr') + out.Check('This dump file has an exception of interest stored in it', + 'captured exception') + out.Check( + 'crashy_program!crashpad::`anonymous namespace\'::SomeCrashyFunction', + 'exception at correct location') + + out = CdbRun(cdb_path, dump_path, '!peb') + out.Check(r'PEB at', 'found the PEB') + out.Check(r'Ldr\.InMemoryOrderModuleList:.*\d+ \. \d+', 'PEB_LDR_DATA saved') + out.Check(r'Base TimeStamp Module', 'module list present') + pipe_name_escaped = pipe_name.replace('\\', '\\\\') + out.Check(r'CommandLine: *\'.*crashy_program.exe *' + pipe_name_escaped, + 'some PEB data is correct') + out.Check(r'SystemRoot=C:\\Windows', 'some of environment captured') + + out = CdbRun(cdb_path, dump_path, '!teb') + out.Check(r'TEB at', 'found the TEB') + out.Check(r'ExceptionList:\s+[0-9a-fA-F]+', 'some valid teb data') + out.Check(r'LastErrorValue:\s+2', 'correct LastErrorValue') + + out = CdbRun(cdb_path, dump_path, '!gle') + out.Check('LastErrorValue: \(Win32\) 0x2 \(2\) - The system cannot find the ' + 'file specified.', '!gle gets last error') + out.Check('LastStatusValue: \(NTSTATUS\) 0xc000000f - {File Not Found} The ' + 'file %hs does not exist.', '!gle gets last ntstatus') + + # Locks. + out = CdbRun(cdb_path, dump_path, '!locks') + out.Check(r'CritSec crashy_program!crashpad::`anonymous namespace\'::' + r'g_test_critical_section', 'lock was captured') + out.Check(r'\*\*\* Locked', 'lock debug info was captured, and is locked') + + def main(args): - cdb_path = GetCdbPath() - print 'cdb_path:', cdb_path - return 0 + try: + if len(args) != 1: + print >>sys.stderr, 'must supply out dir' + return 1 + + cdb_path = GetCdbPath() + if not cdb_path: + print >>sys.stderr, 'could not find cdb' + return 1 + + pipe_name = r'\\.\pipe\end-to-end_%s_%s' % ( + os.getpid(), str(random.getrandbits(64))) + + dump_path = GetDumpFromCrashyProgram(args[0], pipe_name) + if not dump_path: + return 1 + + RunTests(cdb_path, dump_path, pipe_name) + + return 0 + finally: + CleanUpTempDirs() if __name__ == '__main__': From 52238122e99c23af3ee7fc3186ca0dd0e79a4532 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Fri, 9 Oct 2015 13:59:35 -0700 Subject: [PATCH 02/18] Fix for cdb tests There's a problem running crashpad_handler, but I'm not sure what it is. I think an exception is getting swallowed because my handling of `handler` was incorrect, so correctly initialize that to see the exception. https://build.chromium.org/p/client.crashpad/builders/crashpad_win_x64_rel/builds/36/steps/run%20tests/logs/stdio """ UnboundLocalError: local variable 'handler' referenced before assignment """ (I also realized the !locks code hasn't landed yet so disable those tests for now too.) R=mark@chromium.org BUG=crashpad:46 Review URL: https://codereview.chromium.org/1391023006 . --- snapshot/win/end_to_end_test.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/snapshot/win/end_to_end_test.py b/snapshot/win/end_to_end_test.py index 64de3b21..6b55884a 100644 --- a/snapshot/win/end_to_end_test.py +++ b/snapshot/win/end_to_end_test.py @@ -81,6 +81,7 @@ def GetDumpFromCrashyProgram(out_dir, pipe_name): crash_handler for further testing. """ test_database = MakeTempDir() + handler = None try: if subprocess.call( @@ -106,7 +107,6 @@ def GetDumpFromCrashyProgram(out_dir, pipe_name): for line in out.splitlines(): if line.strip().startswith('Path:'): return line.partition(':')[2].strip() - finally: if handler: handler.kill() @@ -177,10 +177,11 @@ def RunTests(cdb_path, dump_path, pipe_name): 'file %hs does not exist.', '!gle gets last ntstatus') # Locks. - out = CdbRun(cdb_path, dump_path, '!locks') - out.Check(r'CritSec crashy_program!crashpad::`anonymous namespace\'::' - r'g_test_critical_section', 'lock was captured') - out.Check(r'\*\*\* Locked', 'lock debug info was captured, and is locked') + if False: # The code for these isn't landed yet. + out = CdbRun(cdb_path, dump_path, '!locks') + out.Check(r'CritSec crashy_program!crashpad::`anonymous namespace\'::' + r'g_test_critical_section', 'lock was captured') + out.Check(r'\*\*\* Locked', 'lock debug info was captured, and is locked') def main(args): From d7ee79cb362e55a40532c514e300b830fd7fc307 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Fri, 9 Oct 2015 14:43:11 -0700 Subject: [PATCH 03/18] Fix path to binary dir in cdb test Oops, was passing the out dir (...\crashpad\out), not the binary dir (...\crashpad\out\Debug). Didn't notice because I was running the script directly, rather than via run_tests.py. :/ R=mark@chromium.org BUG=crashpad:46 Review URL: https://codereview.chromium.org/1394343005 . --- build/run_tests.py | 2 +- snapshot/win/end_to_end_test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/build/run_tests.py b/build/run_tests.py index 43bb3158..f6306852 100755 --- a/build/run_tests.py +++ b/build/run_tests.py @@ -64,7 +64,7 @@ def main(args): print name print '-' * 80 subprocess.check_call( - [sys.executable, os.path.join(crashpad_dir, name), out_dir]) + [sys.executable, os.path.join(crashpad_dir, name), binary_dir]) return 0 diff --git a/snapshot/win/end_to_end_test.py b/snapshot/win/end_to_end_test.py index 6b55884a..9d8ab27f 100644 --- a/snapshot/win/end_to_end_test.py +++ b/snapshot/win/end_to_end_test.py @@ -187,7 +187,7 @@ def RunTests(cdb_path, dump_path, pipe_name): def main(args): try: if len(args) != 1: - print >>sys.stderr, 'must supply out dir' + print >>sys.stderr, 'must supply binary dir' return 1 cdb_path = GetCdbPath() From c3f4e2d8eb35b30494c478cd370f77916aa8e187 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Fri, 9 Oct 2015 16:28:19 -0700 Subject: [PATCH 04/18] Ensure _NT_SYMBOL_PATH is set for bot runs in cdb test Getting closer... Some tests passed on the last run, but the ones that rely on having ntdll symbols fail on the bot. With `_NT_SYMBOL_PATH` set, cdb will be able to download the PDBs so will be able to dump data for `ntdll!_PEB`, etc. R=mark@chromium.org BUG=crashpad:46 Review URL: https://codereview.chromium.org/1402643002 . --- snapshot/win/end_to_end_test.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/snapshot/win/end_to_end_test.py b/snapshot/win/end_to_end_test.py index 9d8ab27f..48c58a76 100644 --- a/snapshot/win/end_to_end_test.py +++ b/snapshot/win/end_to_end_test.py @@ -195,6 +195,12 @@ def main(args): print >>sys.stderr, 'could not find cdb' return 1 + # Make sure we can download Windows symbols. + if not os.environ.get('_NT_SYMBOL_PATH'): + symbol_dir = MakeTempDir() + os.environ['_NT_SYMBOL_PATH'] = ( + 'SRV*' + symbol_dir + '*https://msdl.microsoft.com/download/symbols') + pipe_name = r'\\.\pipe\end-to-end_%s_%s' % ( os.getpid(), str(random.getrandbits(64))) From 4212d3e4ad0c2a9b820fa2cc7711ea6ab777dc0d Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Fri, 9 Oct 2015 16:50:14 -0700 Subject: [PATCH 05/18] make cdb test using SYSTEMROOT case-insensitive R=mark@chromium.org BUG=crashpad:46 Review URL: https://codereview.chromium.org/1390913008 . --- snapshot/win/end_to_end_test.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/snapshot/win/end_to_end_test.py b/snapshot/win/end_to_end_test.py index 48c58a76..bbf442ee 100644 --- a/snapshot/win/end_to_end_test.py +++ b/snapshot/win/end_to_end_test.py @@ -127,8 +127,8 @@ class CdbRun(object): '-c', command + ';q' ]) - def Check(self, pattern, message): - match_obj = re.search(pattern, self.out) + def Check(self, pattern, message, re_flags=0): + match_obj = re.search(pattern, self.out, re_flags) if match_obj: # Matched. Consume up to end of match. self.out = self.out[match_obj.end(0):] @@ -163,7 +163,8 @@ def RunTests(cdb_path, dump_path, pipe_name): pipe_name_escaped = pipe_name.replace('\\', '\\\\') out.Check(r'CommandLine: *\'.*crashy_program.exe *' + pipe_name_escaped, 'some PEB data is correct') - out.Check(r'SystemRoot=C:\\Windows', 'some of environment captured') + out.Check(r'SystemRoot=C:\\Windows', 'some of environment captured', + re.IGNORECASE) out = CdbRun(cdb_path, dump_path, '!teb') out.Check(r'TEB at', 'found the TEB') From 937d3d710c9e062127c5556243155eec25c6c7cb Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Tue, 13 Oct 2015 12:37:44 -0700 Subject: [PATCH 06/18] Mostly-boilerplate to add MemoryMapSnapshot Follows https://codereview.chromium.org/1375313005. Adds MINIDUMP_MEMORY_INFO for non-win in dbghelp.h. R=mark@chromium.org BUG=crashpad:20, crashpad:46 Review URL: https://codereview.chromium.org/1377133006 . --- compat/non_win/dbghelp.h | 36 ++++++++++++++++ snapshot/mac/process_snapshot_mac.cc | 6 +++ snapshot/mac/process_snapshot_mac.h | 2 + snapshot/memory_map_region_snapshot.h | 35 ++++++++++++++++ .../minidump/process_snapshot_minidump.cc | 7 ++++ snapshot/minidump/process_snapshot_minidump.h | 1 + snapshot/process_snapshot.h | 9 ++++ snapshot/snapshot.gyp | 2 + snapshot/test/test_process_snapshot.cc | 9 ++++ snapshot/test/test_process_snapshot.h | 12 ++++++ .../win/memory_map_region_snapshot_win.cc | 41 +++++++++++++++++++ snapshot/win/memory_map_region_snapshot_win.h | 37 +++++++++++++++++ snapshot/win/process_snapshot_win.cc | 15 +++++++ snapshot/win/process_snapshot_win.h | 4 ++ 14 files changed, 216 insertions(+) create mode 100644 snapshot/memory_map_region_snapshot.h create mode 100644 snapshot/win/memory_map_region_snapshot_win.cc create mode 100644 snapshot/win/memory_map_region_snapshot_win.h diff --git a/compat/non_win/dbghelp.h b/compat/non_win/dbghelp.h index 8537687a..4fcd796b 100644 --- a/compat/non_win/dbghelp.h +++ b/compat/non_win/dbghelp.h @@ -836,6 +836,42 @@ struct __attribute__((packed, aligned(4))) MINIDUMP_MISC_INFO_4 //! \brief The latest known version of the MINIDUMP_MISC_INFO structure. typedef MINIDUMP_MISC_INFO_4 MINIDUMP_MISC_INFO_N; +//! \brief Describes a region of memory. +struct __attribute__((packed, aligned(4))) MINIDUMP_MEMORY_INFO { + //! \brief The base address of the region of pages. + uint64_t BaseAddress; + + //! \brief The base address of a range of pages in this region. The page is + //! contained within this memory region. + uint64_t AllocationBase; + + //! \brief The memory protection when the region was initially allocated. This + //! member can be one of the memory protection options (such as + //! `PAGE_EXECUTE`, `PAGE_NOACCESS`, etc.), along with `PAGE_GUARD` or + //! `PAGE_NOCACHE`, as needed. + uint32_t AllocationProtect; + + uint32_t __alignment1; + + //! \brief The size of the region beginning at the base address in which all + //! pages have identical attributes, in bytes. + uint64_t RegionSize; + + //! \brief The state of the pages in the region. This can be one of + //! `MEM_COMMIT`, `MEM_FREE`, or `MEM_RESERVE`. + uint32_t State; + + //! \brief The access protection of the pages in the region. This member is + //! one of the values listed for the #AllocationProtect member. + uint32_t Protect; + + //! \brief The type of pages in the region. This can be one of `MEM_IMAGE`, + //! `MEM_MAPPED`, or `MEM_PRIVATE`. + uint32_t Type; + + uint32_t __alignment2; +}; + //! \brief Minidump file type values for MINIDUMP_HEADER::Flags. These bits //! describe the types of data carried within a minidump file. enum MINIDUMP_TYPE { diff --git a/snapshot/mac/process_snapshot_mac.cc b/snapshot/mac/process_snapshot_mac.cc index 7560317a..215536cc 100644 --- a/snapshot/mac/process_snapshot_mac.cc +++ b/snapshot/mac/process_snapshot_mac.cc @@ -186,6 +186,12 @@ const ExceptionSnapshot* ProcessSnapshotMac::Exception() const { return exception_.get(); } +std::vector ProcessSnapshotMac::MemoryMap() + const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + return std::vector(); +} + std::vector ProcessSnapshotMac::ExtraMemory() const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); return std::vector(); diff --git a/snapshot/mac/process_snapshot_mac.h b/snapshot/mac/process_snapshot_mac.h index 6fb726e0..8712679c 100644 --- a/snapshot/mac/process_snapshot_mac.h +++ b/snapshot/mac/process_snapshot_mac.h @@ -33,6 +33,7 @@ #include "snapshot/mac/process_reader.h" #include "snapshot/mac/system_snapshot_mac.h" #include "snapshot/mac/thread_snapshot_mac.h" +#include "snapshot/memory_map_region_snapshot.h" #include "snapshot/module_snapshot.h" #include "snapshot/process_snapshot.h" #include "snapshot/system_snapshot.h" @@ -126,6 +127,7 @@ class ProcessSnapshotMac final : public ProcessSnapshot { std::vector Threads() const override; std::vector Modules() const override; const ExceptionSnapshot* Exception() const override; + std::vector MemoryMap() const override; std::vector ExtraMemory() const override; private: diff --git a/snapshot/memory_map_region_snapshot.h b/snapshot/memory_map_region_snapshot.h new file mode 100644 index 00000000..4d271f7a --- /dev/null +++ b/snapshot/memory_map_region_snapshot.h @@ -0,0 +1,35 @@ +// 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. + +#ifndef CRASHPAD_SNAPSHOT_MEMORY_MAP_REGION_SNAPSHOT_H_ +#define CRASHPAD_SNAPSHOT_MEMORY_MAP_REGION_SNAPSHOT_H_ + +#include +#include + +namespace crashpad { + +//! \brief An abstract interface to a snapshot representing a region of the +//! memory map present in the snapshot process. +class MemoryMapRegionSnapshot { + public: + virtual ~MemoryMapRegionSnapshot() {} + + //! \brief Gets a MINIDUMP_MEMORY_INFO representing the region. + virtual const MINIDUMP_MEMORY_INFO& AsMinidumpMemoryInfo() const = 0; +}; + +} // namespace crashpad + +#endif // CRASHPAD_SNAPSHOT_MEMORY_MAP_REGION_SNAPSHOT_H_ diff --git a/snapshot/minidump/process_snapshot_minidump.cc b/snapshot/minidump/process_snapshot_minidump.cc index 0ad0d144..d07274f3 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::MemoryMap() + const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + NOTREACHED(); // https://code.google.com/p/crashpad/issues/detail?id=10 + return std::vector(); +} + std::vector ProcessSnapshotMinidump::ExtraMemory() const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); diff --git a/snapshot/minidump/process_snapshot_minidump.h b/snapshot/minidump/process_snapshot_minidump.h index 6de64c96..c02fcc9b 100644 --- a/snapshot/minidump/process_snapshot_minidump.h +++ b/snapshot/minidump/process_snapshot_minidump.h @@ -69,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 MemoryMap() const override; std::vector ExtraMemory() const override; private: diff --git a/snapshot/process_snapshot.h b/snapshot/process_snapshot.h index f2fceef2..0a2f4f9f 100644 --- a/snapshot/process_snapshot.h +++ b/snapshot/process_snapshot.h @@ -27,6 +27,7 @@ namespace crashpad { class ExceptionSnapshot; +class MemoryMapRegionSnapshot; class MemorySnapshot; class ModuleSnapshot; class SystemSnapshot; @@ -162,6 +163,14 @@ class ProcessSnapshot { //! an exception, returns `nullptr`. virtual const ExceptionSnapshot* Exception() const = 0; + //! \brief Returns MemoryMapRegionSnapshot objects reflecting the regions + //! of the memory map in the snapshot process at the time of the snapshot. + //! + //! \return A vector of MemoryMapRegionSnapshot objects. 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 MemoryMap() const = 0; + //! \brief Returns a vector of additional memory blocks that should be //! included in a minidump. //! diff --git a/snapshot/snapshot.gyp b/snapshot/snapshot.gyp index 6ad9f189..76d07aa6 100644 --- a/snapshot/snapshot.gyp +++ b/snapshot/snapshot.gyp @@ -91,6 +91,8 @@ 'win/cpu_context_win.h', 'win/exception_snapshot_win.cc', 'win/exception_snapshot_win.h', + 'win/memory_map_region_snapshot_win.cc', + 'win/memory_map_region_snapshot_win.h', 'win/memory_snapshot_win.cc', 'win/memory_snapshot_win.h', 'win/module_snapshot_win.cc', diff --git a/snapshot/test/test_process_snapshot.cc b/snapshot/test/test_process_snapshot.cc index 1d21212d..d388c601 100644 --- a/snapshot/test/test_process_snapshot.cc +++ b/snapshot/test/test_process_snapshot.cc @@ -33,6 +33,7 @@ TestProcessSnapshot::TestProcessSnapshot() threads_(), modules_(), exception_(), + memory_map_(), extra_memory_() { } @@ -98,6 +99,14 @@ const ExceptionSnapshot* TestProcessSnapshot::Exception() const { return exception_.get(); } +std::vector TestProcessSnapshot::MemoryMap() + const { + std::vector memory_map; + for (const auto& item : memory_map_) + memory_map.push_back(item); + return memory_map; +} + std::vector TestProcessSnapshot::ExtraMemory() const { std::vector extra_memory; for (const auto& em : extra_memory_) diff --git a/snapshot/test/test_process_snapshot.h b/snapshot/test/test_process_snapshot.h index a30d82a3..2f5cd45a 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_map_region_snapshot.h" #include "snapshot/memory_snapshot.h" #include "snapshot/module_snapshot.h" #include "snapshot/process_snapshot.h" @@ -96,6 +97,15 @@ class TestProcessSnapshot final : public ProcessSnapshot { exception_ = exception.Pass(); } + //! \brief Adds a memory map region snapshot to be returned by MemoryMap(). + //! + //! \param[in] region The memory map region snapshot that will be included in + //! MemoryMap(). The TestProcessSnapshot object takes ownership of \a + //! region. + void AddMemoryMapRegion(scoped_ptr region) { + memory_map_.push_back(region.release()); + } + //! \brief Add a memory snapshot to be returned by ExtraMemory(). //! //! \param[in] extra_memory The memory snapshot that will be included in @@ -120,6 +130,7 @@ class TestProcessSnapshot final : public ProcessSnapshot { std::vector Threads() const override; std::vector Modules() const override; const ExceptionSnapshot* Exception() const override; + std::vector MemoryMap() const override; std::vector ExtraMemory() const override; private: @@ -136,6 +147,7 @@ class TestProcessSnapshot final : public ProcessSnapshot { PointerVector threads_; PointerVector modules_; scoped_ptr exception_; + PointerVector memory_map_; PointerVector extra_memory_; DISALLOW_COPY_AND_ASSIGN(TestProcessSnapshot); diff --git a/snapshot/win/memory_map_region_snapshot_win.cc b/snapshot/win/memory_map_region_snapshot_win.cc new file mode 100644 index 00000000..c64254c6 --- /dev/null +++ b/snapshot/win/memory_map_region_snapshot_win.cc @@ -0,0 +1,41 @@ +// 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 "snapshot/win/memory_map_region_snapshot_win.h" + +namespace crashpad { +namespace internal { + +MemoryMapRegionSnapshotWin::MemoryMapRegionSnapshotWin( + const MEMORY_BASIC_INFORMATION64& mbi) + : memory_info_() { + memory_info_.BaseAddress = mbi.BaseAddress; + memory_info_.AllocationBase = mbi.AllocationBase; + memory_info_.AllocationProtect = mbi.AllocationProtect; + memory_info_.RegionSize = mbi.RegionSize; + memory_info_.State = mbi.State; + memory_info_.Protect = mbi.Protect; + memory_info_.Type = mbi.Type; +} + +MemoryMapRegionSnapshotWin::~MemoryMapRegionSnapshotWin() { +} + +const MINIDUMP_MEMORY_INFO& MemoryMapRegionSnapshotWin::AsMinidumpMemoryInfo() + const { + return memory_info_; +} + +} // namespace internal +} // namespace crashpad diff --git a/snapshot/win/memory_map_region_snapshot_win.h b/snapshot/win/memory_map_region_snapshot_win.h new file mode 100644 index 00000000..725c1256 --- /dev/null +++ b/snapshot/win/memory_map_region_snapshot_win.h @@ -0,0 +1,37 @@ +// 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. + +#ifndef CRASHPAD_SNAPSHOT_WIN_MEMORY_MAP_REGION_SNAPSHOT_WIN_H_ +#define CRASHPAD_SNAPSHOT_WIN_MEMORY_MAP_REGION_SNAPSHOT_WIN_H_ + +#include "snapshot/memory_map_region_snapshot.h" + +namespace crashpad { +namespace internal { + +class MemoryMapRegionSnapshotWin : public MemoryMapRegionSnapshot { + public: + explicit MemoryMapRegionSnapshotWin(const MEMORY_BASIC_INFORMATION64& mbi); + ~MemoryMapRegionSnapshotWin() override; + + virtual const MINIDUMP_MEMORY_INFO& AsMinidumpMemoryInfo() const override; + + private: + MINIDUMP_MEMORY_INFO memory_info_; +}; + +} // namespace internal +} // namespace crashpad + +#endif // CRASHPAD_SNAPSHOT_WIN_MEMORY_MAP_REGION_SNAPSHOT_WIN_H_ diff --git a/snapshot/win/process_snapshot_win.cc b/snapshot/win/process_snapshot_win.cc index 26de3db3..98e955c0 100644 --- a/snapshot/win/process_snapshot_win.cc +++ b/snapshot/win/process_snapshot_win.cc @@ -18,6 +18,7 @@ #include "base/logging.h" #include "base/strings/stringprintf.h" +#include "snapshot/win/memory_snapshot_win.h" #include "snapshot/win/module_snapshot_win.h" #include "util/win/registration_protocol_win.h" #include "util/win/time.h" @@ -30,6 +31,7 @@ ProcessSnapshotWin::ProcessSnapshotWin() threads_(), modules_(), exception_(), + memory_map_(), process_reader_(), report_id_(), client_id_(), @@ -60,6 +62,11 @@ bool ProcessSnapshotWin::Initialize(HANDLE process, InitializeThreads(); InitializeModules(); + for (const MEMORY_BASIC_INFORMATION64& mbi : + process_reader_.GetProcessInfo().MemoryInfo()) { + memory_map_.push_back(new internal::MemoryMapRegionSnapshotWin(mbi)); + } + INITIALIZATION_STATE_SET_VALID(initialized_); return true; } @@ -187,6 +194,14 @@ const ExceptionSnapshot* ProcessSnapshotWin::Exception() const { return exception_.get(); } +std::vector ProcessSnapshotWin::MemoryMap() + const { + std::vector memory_map; + for (const auto& item : memory_map_) + memory_map.push_back(item); + return memory_map; +} + std::vector ProcessSnapshotWin::ExtraMemory() const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); std::vector extra_memory; diff --git a/snapshot/win/process_snapshot_win.h b/snapshot/win/process_snapshot_win.h index c21dbc93..a38ac5e9 100644 --- a/snapshot/win/process_snapshot_win.h +++ b/snapshot/win/process_snapshot_win.h @@ -27,12 +27,14 @@ #include "client/crashpad_info.h" #include "snapshot/crashpad_info_client_options.h" #include "snapshot/exception_snapshot.h" +#include "snapshot/memory_map_region_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_map_region_snapshot_win.h" #include "snapshot/win/memory_snapshot_win.h" #include "snapshot/win/module_snapshot_win.h" #include "snapshot/win/system_snapshot_win.h" @@ -126,6 +128,7 @@ class ProcessSnapshotWin final : public ProcessSnapshot { std::vector Threads() const override; std::vector Modules() const override; const ExceptionSnapshot* Exception() const override; + std::vector MemoryMap() const override; std::vector ExtraMemory() const override; private: @@ -162,6 +165,7 @@ class ProcessSnapshotWin final : public ProcessSnapshot { PointerVector threads_; PointerVector modules_; scoped_ptr exception_; + PointerVector memory_map_; ProcessReaderWin process_reader_; UUID report_id_; UUID client_id_; From 019a0cec8b8e0fd5f47c564772aa7a7d88921b79 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Tue, 13 Oct 2015 13:15:44 -0700 Subject: [PATCH 07/18] win: Write memory map info as MINIDUMP_MEMORY_INFO[_LIST] Makes !vprot work in windbg, e.g. 0:000> !vprot 0x970000 BaseAddress: 00970000 AllocationBase: 00970000 AllocationProtect: 00000004 PAGE_READWRITE RegionSize: 00001000 State: 00001000 MEM_COMMIT Protect: 00000001 PAGE_NOACCESS Type: 00020000 MEM_PRIVATE ... 0:000> !vprot 0x97a000 BaseAddress: 0097a000 AllocationBase: 00970000 AllocationProtect: 00000004 PAGE_READWRITE RegionSize: 00001000 State: 00001000 MEM_COMMIT Protect: 00000140 PAGE_EXECUTE_READWRITE + PAGE_GUARD Type: 00020000 MEM_PRIVATE Follows https://codereview.chromium.org/1377133006. R=mark@chromium.org BUG=crashpad:20, crashpad:46 Review URL: https://codereview.chromium.org/1379873005 . --- compat/non_win/dbghelp.h | 28 +++- compat/non_win/winnt.h | 35 +++++ handler/win/crashy_test_program.cc | 42 ++++++ minidump/minidump.gyp | 2 + minidump/minidump_extensions.h | 5 + minidump/minidump_file_writer.cc | 9 ++ minidump/minidump_memory_info_writer.cc | 85 ++++++++++++ minidump/minidump_memory_info_writer.h | 71 ++++++++++ minidump/minidump_memory_info_writer_test.cc | 126 ++++++++++++++++++ minidump/minidump_test.gyp | 1 + minidump/test/minidump_writable_test_util.cc | 16 +++ minidump/test/minidump_writable_test_util.h | 14 +- snapshot/snapshot_test.gyp | 2 + .../test/test_memory_map_region_snapshot.cc | 37 +++++ .../test/test_memory_map_region_snapshot.h | 47 +++++++ 15 files changed, 511 insertions(+), 9 deletions(-) create mode 100644 minidump/minidump_memory_info_writer.cc create mode 100644 minidump/minidump_memory_info_writer.h create mode 100644 minidump/minidump_memory_info_writer_test.cc create mode 100644 snapshot/test/test_memory_map_region_snapshot.cc create mode 100644 snapshot/test/test_memory_map_region_snapshot.h diff --git a/compat/non_win/dbghelp.h b/compat/non_win/dbghelp.h index 4fcd796b..6fed86bd 100644 --- a/compat/non_win/dbghelp.h +++ b/compat/non_win/dbghelp.h @@ -167,6 +167,9 @@ enum MINIDUMP_STREAM_TYPE { //! MINIDUMP_MISC_INFO::Flags1, that indicates which data is present and //! valid. MiscInfoStream = 15, + + //! \brief The stream type for MINIDUMP_MEMORY_INFO_LIST. + MemoryInfoListStream = 16, }; //! \brief Information about the CPU (or CPUs) that ran the process that the @@ -847,8 +850,8 @@ struct __attribute__((packed, aligned(4))) MINIDUMP_MEMORY_INFO { //! \brief The memory protection when the region was initially allocated. This //! member can be one of the memory protection options (such as - //! `PAGE_EXECUTE`, `PAGE_NOACCESS`, etc.), along with `PAGE_GUARD` or - //! `PAGE_NOCACHE`, as needed. + //! \ref PAGE_x PAGE_EXECUTE, \ref PAGE_x PAGE_NOACCESS, etc.), along with + //! \ref PAGE_x PAGE_GUARD or \ref PAGE_x PAGE_NOCACHE, as needed. uint32_t AllocationProtect; uint32_t __alignment1; @@ -858,20 +861,35 @@ struct __attribute__((packed, aligned(4))) MINIDUMP_MEMORY_INFO { uint64_t RegionSize; //! \brief The state of the pages in the region. This can be one of - //! `MEM_COMMIT`, `MEM_FREE`, or `MEM_RESERVE`. + //! \ref MEM_x MEM_COMMIT, \ref MEM_x MEM_FREE, or \ref MEM_x MEM_RESERVE. uint32_t State; //! \brief The access protection of the pages in the region. This member is //! one of the values listed for the #AllocationProtect member. uint32_t Protect; - //! \brief The type of pages in the region. This can be one of `MEM_IMAGE`, - //! `MEM_MAPPED`, or `MEM_PRIVATE`. + //! \brief The type of pages in the region. This can be one of \ref MEM_x + //! MEM_IMAGE, \ref MEM_x MEM_MAPPED, or \ref MEM_x MEM_PRIVATE. uint32_t Type; uint32_t __alignment2; }; +//! \brief Contains a list of memory regions. +struct __attribute__((packed, aligned(4))) MINIDUMP_MEMORY_INFO_LIST { + //! \brief The size of the header data for the stream, in bytes. This is + //! generally sizeof(MINIDUMP_MEMORY_INFO_LIST). + uint32_t SizeOfHeader; + + //! \brief The size of each entry following the header, in bytes. This is + //! generally sizeof(MINIDUMP_MEMORY_INFO). + uint32_t SizeOfEntry; + + //! \brief The number of entries in the stream. These are generally + //! MINIDUMP_MEMORY_INFO structures. The entries follow the header. + uint64_t NumberOfEntries; +}; + //! \brief Minidump file type values for MINIDUMP_HEADER::Flags. These bits //! describe the types of data carried within a minidump file. enum MINIDUMP_TYPE { diff --git a/compat/non_win/winnt.h b/compat/non_win/winnt.h index d61504c0..03167aa5 100644 --- a/compat/non_win/winnt.h +++ b/compat/non_win/winnt.h @@ -179,4 +179,39 @@ struct IMAGE_DEBUG_MISC { #define VER_PLATFORM_WIN32_NT 2 //! \} +//! \anchor PAGE_x +//! \name PAGE_* +//! +//! \brief Memory protection constants for MINIDUMP_MEMORY_INFO::Protect and +//! MINIDUMP_MEMORY_INFO::AllocationProtect. +//! \{ +#define PAGE_NOACCESS 0x1 +#define PAGE_READONLY 0x2 +#define PAGE_READWRITE 0x4 +#define PAGE_WRITECOPY 0x8 +#define PAGE_EXECUTE 0x10 +#define PAGE_EXECUTE_READ 0x20 +#define PAGE_EXECUTE_READWRITE 0x40 +#define PAGE_EXECUTE_WRITECOPY 0x80 +#define PAGE_GUARD 0x100 +#define PAGE_NOCACHE 0x200 +#define PAGE_WRITECOMBINE 0x400 +//! \} + +//! \anchor MEM_x +//! \name MEM_* +//! +//! \brief Memory state and type constants for MINIDUMP_MEMORY_INFO::State and +//! MINIDUMP_MEMORY_INFO::Type. +//! \{ +#define MEM_COMMIT 0x1000 +#define MEM_RESERVE 0x2000 +#define MEM_DECOMMIT 0x4000 +#define MEM_RELEASE 0x8000 +#define MEM_FREE 0x10000 +#define MEM_PRIVATE 0x20000 +#define MEM_MAPPED 0x40000 +#define MEM_RESET 0x80000 +//! \} + #endif // CRASHPAD_COMPAT_NON_WIN_WINNT_H_ diff --git a/handler/win/crashy_test_program.cc b/handler/win/crashy_test_program.cc index 622a5e5d..638098a6 100644 --- a/handler/win/crashy_test_program.cc +++ b/handler/win/crashy_test_program.cc @@ -20,6 +20,7 @@ #define STATUS_NO_SUCH_FILE static_cast(0xC000000F) #endif +#include "base/basictypes.h" #include "base/logging.h" #include "client/crashpad_client.h" #include "tools/tool_support.h" @@ -35,6 +36,45 @@ ULONG RtlNtStatusToDosError(NTSTATUS status) { return rtl_nt_status_to_dos_error(status); } +void AllocateMemoryOfVariousProtections() { + SYSTEM_INFO system_info; + GetSystemInfo(&system_info); + + const size_t kPageSize = system_info.dwPageSize; + + const uint32_t kPageTypes[] = { + PAGE_NOACCESS, + PAGE_READONLY, + PAGE_READWRITE, + PAGE_EXECUTE, + PAGE_EXECUTE_READ, + PAGE_EXECUTE_READWRITE, + + // PAGE_NOACCESS is invalid with PAGE_GUARD. + PAGE_READONLY | PAGE_GUARD, + PAGE_READWRITE | PAGE_GUARD, + PAGE_EXECUTE | PAGE_GUARD, + PAGE_EXECUTE_READ | PAGE_GUARD, + PAGE_EXECUTE_READWRITE | PAGE_GUARD, + }; + + // All of these allocations are leaked, we want to view them in windbg via + // !vprot. + void* reserve = VirtualAlloc( + nullptr, arraysize(kPageTypes) * kPageSize, MEM_RESERVE, PAGE_READWRITE); + PCHECK(reserve) << "VirtualAlloc MEM_RESERVE"; + uintptr_t reserve_as_int = reinterpret_cast(reserve); + + for (size_t i = 0; i < arraysize(kPageTypes); ++i) { + void* result = + VirtualAlloc(reinterpret_cast(reserve_as_int + (kPageSize * i)), + kPageSize, + MEM_COMMIT, + kPageTypes[i]); + PCHECK(result) << "VirtualAlloc MEM_COMMIT " << i; + } +} + void SomeCrashyFunction() { // SetLastError and NTSTATUS so that we have something to view in !gle in // windbg. RtlNtStatusToDosError() stores STATUS_NO_SUCH_FILE into the @@ -61,6 +101,8 @@ int CrashyMain(int argc, char* argv[]) { return 1; } + AllocateMemoryOfVariousProtections(); + SomeCrashyFunction(); return 0; diff --git a/minidump/minidump.gyp b/minidump/minidump.gyp index f7da929c..21041a9e 100644 --- a/minidump/minidump.gyp +++ b/minidump/minidump.gyp @@ -44,6 +44,8 @@ 'minidump_extensions.h', 'minidump_file_writer.cc', 'minidump_file_writer.h', + 'minidump_memory_info_writer.cc', + 'minidump_memory_info_writer.h', 'minidump_memory_writer.cc', 'minidump_memory_writer.h', 'minidump_misc_info_writer.cc', diff --git a/minidump/minidump_extensions.h b/minidump/minidump_extensions.h index 95fdbddf..3cb39dc2 100644 --- a/minidump/minidump_extensions.h +++ b/minidump/minidump_extensions.h @@ -77,6 +77,11 @@ enum MinidumpStreamType : uint32_t { //! \sa MiscInfoStream kMinidumpStreamTypeMiscInfo = MiscInfoStream, + //! \brief The stream type for MINIDUMP_MEMORY_INFO_LIST. + //! + //! \sa MemoryInfoListStream + kMinidumpStreamTypeMemoryInfoList = MemoryInfoListStream, + // 0x4350 = "CP" //! \brief The stream type for MinidumpCrashpadInfo. diff --git a/minidump/minidump_file_writer.cc b/minidump/minidump_file_writer.cc index f0c8d4b8..88986561 100644 --- a/minidump/minidump_file_writer.cc +++ b/minidump/minidump_file_writer.cc @@ -17,6 +17,7 @@ #include "base/logging.h" #include "minidump/minidump_crashpad_info_writer.h" #include "minidump/minidump_exception_writer.h" +#include "minidump/minidump_memory_info_writer.h" #include "minidump/minidump_memory_writer.h" #include "minidump/minidump_misc_info_writer.h" #include "minidump/minidump_module_writer.h" @@ -99,6 +100,14 @@ void MinidumpFileWriter::InitializeFromSnapshot( AddStream(crashpad_info.Pass()); } + std::vector memory_map_snapshot = + process_snapshot->MemoryMap(); + if (!memory_map_snapshot.empty()) { + auto memory_info_list = make_scoped_ptr(new MinidumpMemoryInfoListWriter()); + memory_info_list->InitializeFromSnapshot(memory_map_snapshot); + AddStream(memory_info_list.Pass()); + } + memory_list->AddFromSnapshot(process_snapshot->ExtraMemory()); AddStream(memory_list.Pass()); diff --git a/minidump/minidump_memory_info_writer.cc b/minidump/minidump_memory_info_writer.cc new file mode 100644 index 00000000..8b6bdc8c --- /dev/null +++ b/minidump/minidump_memory_info_writer.cc @@ -0,0 +1,85 @@ +// 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 "minidump/minidump_memory_info_writer.h" + +#include "base/logging.h" +#include "snapshot/memory_map_region_snapshot.h" +#include "util/file/file_writer.h" + +namespace crashpad { + +MinidumpMemoryInfoListWriter::MinidumpMemoryInfoListWriter() + : memory_info_list_base_(), items_() { +} + +MinidumpMemoryInfoListWriter::~MinidumpMemoryInfoListWriter() { +} + +void MinidumpMemoryInfoListWriter::InitializeFromSnapshot( + const std::vector& memory_map) { + DCHECK_EQ(state(), kStateMutable); + + DCHECK(items_.empty()); + for (const auto& region : memory_map) + items_.push_back(region->AsMinidumpMemoryInfo()); +} + +bool MinidumpMemoryInfoListWriter::Freeze() { + DCHECK_EQ(state(), kStateMutable); + + if (!MinidumpStreamWriter::Freeze()) + return false; + + memory_info_list_base_.SizeOfHeader = sizeof(MINIDUMP_MEMORY_INFO_LIST); + memory_info_list_base_.SizeOfEntry = sizeof(MINIDUMP_MEMORY_INFO); + memory_info_list_base_.NumberOfEntries = items_.size(); + + return true; +} + +size_t MinidumpMemoryInfoListWriter::SizeOfObject() { + DCHECK_GE(state(), kStateFrozen); + return sizeof(memory_info_list_base_) + sizeof(items_[0]) * items_.size(); +} + +std::vector +MinidumpMemoryInfoListWriter::Children() { + DCHECK_GE(state(), kStateFrozen); + return std::vector(); +} + +bool MinidumpMemoryInfoListWriter::WriteObject( + FileWriterInterface* file_writer) { + DCHECK_EQ(state(), kStateWritable); + + WritableIoVec iov; + iov.iov_base = &memory_info_list_base_; + iov.iov_len = sizeof(memory_info_list_base_); + std::vector iovecs(1, iov); + + for (const auto& minidump_memory_info : items_) { + iov.iov_base = &minidump_memory_info; + iov.iov_len = sizeof(minidump_memory_info); + iovecs.push_back(iov); + } + + return file_writer->WriteIoVec(&iovecs); +} + +MinidumpStreamType MinidumpMemoryInfoListWriter::StreamType() const { + return kMinidumpStreamTypeMemoryInfoList; +} + +} // namespace crashpad diff --git a/minidump/minidump_memory_info_writer.h b/minidump/minidump_memory_info_writer.h new file mode 100644 index 00000000..6f6ea59b --- /dev/null +++ b/minidump/minidump_memory_info_writer.h @@ -0,0 +1,71 @@ +// 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. + +#ifndef CRASHPAD_MINIDUMP_MINIDUMP_MEMORY_INFO_WRITER_H_ +#define CRASHPAD_MINIDUMP_MINIDUMP_MEMORY_INFO_WRITER_H_ + +#include +#include +#include + +#include + +#include "base/basictypes.h" +#include "minidump/minidump_stream_writer.h" +#include "minidump/minidump_writable.h" + +namespace crashpad { + +class MemoryMapRegionSnapshot; +class MinidumpContextWriter; +class MinidumpMemoryListWriter; +class MinidumpMemoryWriter; + +//! \brief The writer for a MINIDUMP_MEMORY_INFO_LIST stream in a minidump file, +//! containing a list of MINIDUMP_MEMORY_INFO objects. +class MinidumpMemoryInfoListWriter final + : public internal::MinidumpStreamWriter { + public: + MinidumpMemoryInfoListWriter(); + ~MinidumpMemoryInfoListWriter() override; + + //! \brief Initializes a MINIDUMP_MEMORY_INFO_LIST based on \a memory_map. + //! + //! \param[in] memory_map The vector of memory map region snapshots to use as + //! source data. + //! + //! \note Valid in #kStateMutable. + void InitializeFromSnapshot( + const std::vector& memory_map); + + protected: + // MinidumpWritable: + bool Freeze() override; + size_t SizeOfObject() override; + std::vector Children() override; + bool WriteObject(FileWriterInterface* file_writer) override; + + // MinidumpStreamWriter: + MinidumpStreamType StreamType() const override; + + private: + MINIDUMP_MEMORY_INFO_LIST memory_info_list_base_; + std::vector items_; + + DISALLOW_COPY_AND_ASSIGN(MinidumpMemoryInfoListWriter); +}; + +} // namespace crashpad + +#endif // CRASHPAD_MINIDUMP_MINIDUMP_MEMORY_INFO_WRITER_H_ diff --git a/minidump/minidump_memory_info_writer_test.cc b/minidump/minidump_memory_info_writer_test.cc new file mode 100644 index 00000000..21081d53 --- /dev/null +++ b/minidump/minidump_memory_info_writer_test.cc @@ -0,0 +1,126 @@ +// 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 "minidump/minidump_memory_info_writer.h" + +#include + +#include "gtest/gtest.h" +#include "minidump/minidump_file_writer.h" +#include "minidump/test/minidump_file_writer_test_util.h" +#include "minidump/test/minidump_writable_test_util.h" +#include "snapshot/test/test_memory_map_region_snapshot.h" +#include "util/file/string_file.h" + +namespace crashpad { +namespace test { +namespace { + +// The memory info list is expected to be the only stream. +void GetMemoryInfoListStream( + const std::string& file_contents, + const MINIDUMP_MEMORY_INFO_LIST** memory_info_list) { + const size_t kDirectoryOffset = sizeof(MINIDUMP_HEADER); + const size_t kMemoryInfoListStreamOffset = + kDirectoryOffset + sizeof(MINIDUMP_DIRECTORY); + + const MINIDUMP_DIRECTORY* directory; + const MINIDUMP_HEADER* header = + MinidumpHeaderAtStart(file_contents, &directory); + ASSERT_NO_FATAL_FAILURE(VerifyMinidumpHeader(header, 1, 0)); + ASSERT_TRUE(directory); + + const size_t kDirectoryIndex = 0; + + ASSERT_EQ(kMinidumpStreamTypeMemoryInfoList, + directory[kDirectoryIndex].StreamType); + EXPECT_EQ(kMemoryInfoListStreamOffset, + directory[kDirectoryIndex].Location.Rva); + + *memory_info_list = + MinidumpWritableAtLocationDescriptor( + file_contents, directory[kDirectoryIndex].Location); + ASSERT_TRUE(memory_info_list); +} + +TEST(MinidumpMemoryInfoWriter, Empty) { + MinidumpFileWriter minidump_file_writer; + auto memory_info_list_writer = + make_scoped_ptr(new MinidumpMemoryInfoListWriter()); + minidump_file_writer.AddStream(memory_info_list_writer.Pass()); + + StringFile string_file; + ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); + + ASSERT_EQ(sizeof(MINIDUMP_HEADER) + sizeof(MINIDUMP_DIRECTORY) + + sizeof(MINIDUMP_MEMORY_INFO_LIST), + string_file.string().size()); + + const MINIDUMP_MEMORY_INFO_LIST* memory_info_list = nullptr; + ASSERT_NO_FATAL_FAILURE( + GetMemoryInfoListStream(string_file.string(), &memory_info_list)); + + EXPECT_EQ(0u, memory_info_list->NumberOfEntries); +} + +TEST(MinidumpMemoryInfoWriter, OneRegion) { + MinidumpFileWriter minidump_file_writer; + auto memory_info_list_writer = + make_scoped_ptr(new MinidumpMemoryInfoListWriter()); + + auto memory_map_region = make_scoped_ptr(new TestMemoryMapRegionSnapshot()); + + MINIDUMP_MEMORY_INFO mmi = {0}; + mmi.BaseAddress = 0x12340000; + mmi.AllocationBase = 0x12000000; + mmi.AllocationProtect = PAGE_READWRITE; + mmi.RegionSize = 0x6000; + mmi.State = MEM_COMMIT; + mmi.Protect = PAGE_NOACCESS; + mmi.Type = MEM_PRIVATE; + memory_map_region->SetMindumpMemoryInfo(mmi); + + std::vector memory_map; + memory_map.push_back(memory_map_region.get()); + memory_info_list_writer->InitializeFromSnapshot(memory_map); + + minidump_file_writer.AddStream(memory_info_list_writer.Pass()); + + StringFile string_file; + ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); + + ASSERT_EQ(sizeof(MINIDUMP_HEADER) + sizeof(MINIDUMP_DIRECTORY) + + sizeof(MINIDUMP_MEMORY_INFO_LIST) + + sizeof(MINIDUMP_MEMORY_INFO), + string_file.string().size()); + + const MINIDUMP_MEMORY_INFO_LIST* memory_info_list = nullptr; + ASSERT_NO_FATAL_FAILURE( + GetMemoryInfoListStream(string_file.string(), &memory_info_list)); + + EXPECT_EQ(1u, memory_info_list->NumberOfEntries); + const MINIDUMP_MEMORY_INFO* memory_info = + reinterpret_cast(&memory_info_list[1]); + EXPECT_EQ(mmi.BaseAddress, memory_info->BaseAddress); + EXPECT_EQ(mmi.AllocationBase, memory_info->AllocationBase); + EXPECT_EQ(mmi.AllocationProtect, memory_info->AllocationProtect); + EXPECT_EQ(mmi.RegionSize, memory_info->RegionSize); + EXPECT_EQ(mmi.State, memory_info->State); + EXPECT_EQ(mmi.Protect, memory_info->Protect); + EXPECT_EQ(mmi.Type, memory_info->Type); +} + +} // namespace +} // namespace test +} // namespace crashpad diff --git a/minidump/minidump_test.gyp b/minidump/minidump_test.gyp index 35b7d1a9..09b80cc2 100644 --- a/minidump/minidump_test.gyp +++ b/minidump/minidump_test.gyp @@ -37,6 +37,7 @@ 'minidump_crashpad_info_writer_test.cc', 'minidump_exception_writer_test.cc', 'minidump_file_writer_test.cc', + 'minidump_memory_info_writer_test.cc', 'minidump_memory_writer_test.cc', 'minidump_misc_info_writer_test.cc', 'minidump_module_crashpad_info_writer_test.cc', diff --git a/minidump/test/minidump_writable_test_util.cc b/minidump/test/minidump_writable_test_util.cc index d16aed94..ef00f40a 100644 --- a/minidump/test/minidump_writable_test_util.cc +++ b/minidump/test/minidump_writable_test_util.cc @@ -181,6 +181,14 @@ struct MinidumpThreadListTraits { } }; +struct MinidumpMemoryInfoListTraits { + using ListType = MINIDUMP_MEMORY_INFO_LIST; + enum : size_t { kElementSize = sizeof(MINIDUMP_MEMORY_INFO) }; + static size_t ElementCount(const ListType* list) { + return static_cast(list->NumberOfEntries); + } +}; + struct MinidumpModuleCrashpadInfoListTraits { using ListType = MinidumpModuleCrashpadInfoList; enum : size_t { kElementSize = sizeof(MinidumpModuleCrashpadInfoLink) }; @@ -244,6 +252,14 @@ const MINIDUMP_THREAD_LIST* MinidumpWritableAtLocationDescriptor< file_contents, location); } +template <> +const MINIDUMP_MEMORY_INFO_LIST* MinidumpWritableAtLocationDescriptor< + MINIDUMP_MEMORY_INFO_LIST>(const std::string& file_contents, + const MINIDUMP_LOCATION_DESCRIPTOR& location) { + return MinidumpListAtLocationDescriptor( + file_contents, location); +} + template <> const MinidumpModuleCrashpadInfoList* MinidumpWritableAtLocationDescriptor( diff --git a/minidump/test/minidump_writable_test_util.h b/minidump/test/minidump_writable_test_util.h index ce489686..9cba8be0 100644 --- a/minidump/test/minidump_writable_test_util.h +++ b/minidump/test/minidump_writable_test_util.h @@ -90,6 +90,7 @@ MINIDUMP_ALLOW_OVERSIZED_DATA(MINIDUMP_DIRECTORY); MINIDUMP_ALLOW_OVERSIZED_DATA(MINIDUMP_MEMORY_LIST); MINIDUMP_ALLOW_OVERSIZED_DATA(MINIDUMP_MODULE_LIST); MINIDUMP_ALLOW_OVERSIZED_DATA(MINIDUMP_THREAD_LIST); +MINIDUMP_ALLOW_OVERSIZED_DATA(MINIDUMP_MEMORY_INFO_LIST); MINIDUMP_ALLOW_OVERSIZED_DATA(MinidumpModuleCrashpadInfoList); MINIDUMP_ALLOW_OVERSIZED_DATA(MinidumpRVAList); MINIDUMP_ALLOW_OVERSIZED_DATA(MinidumpSimpleStringDictionary); @@ -138,10 +139,10 @@ const T* TMinidumpWritableAtLocationDescriptor( //! - With a MINIDUMP_HEADER template parameter, a template specialization //! ensures that the structure’s magic number and version fields are correct. //! - With a MINIDUMP_MEMORY_LIST, MINIDUMP_THREAD_LIST, MINIDUMP_MODULE_LIST, -//! or MinidumpSimpleStringDictionary template parameter, template -//! specializations ensure that the size given by \a location matches the -//! size expected of a stream containing the number of elements it claims to -//! have. +//! MINIDUMP_MEMORY_INFO_LIST, or MinidumpSimpleStringDictionary template +//! parameter, template specializations ensure that the size given by \a +//! location matches the size expected of a stream containing the number of +//! elements it claims to have. //! - With an IMAGE_DEBUG_MISC, CodeViewRecordPDB20, or CodeViewRecordPDB70 //! template parameter, template specializations ensure that the structure //! has the expected format including any magic number and the `NUL`- @@ -189,6 +190,11 @@ const MINIDUMP_THREAD_LIST* MinidumpWritableAtLocationDescriptor< MINIDUMP_THREAD_LIST>(const std::string& file_contents, const MINIDUMP_LOCATION_DESCRIPTOR& location); +template <> +const MINIDUMP_MEMORY_INFO_LIST* MinidumpWritableAtLocationDescriptor< + MINIDUMP_MEMORY_INFO_LIST>(const std::string& file_contents, + const MINIDUMP_LOCATION_DESCRIPTOR& location); + template <> const CodeViewRecordPDB20* MinidumpWritableAtLocationDescriptor< CodeViewRecordPDB20>(const std::string& file_contents, diff --git a/snapshot/snapshot_test.gyp b/snapshot/snapshot_test.gyp index fd7aaefe..51274e6c 100644 --- a/snapshot/snapshot_test.gyp +++ b/snapshot/snapshot_test.gyp @@ -34,6 +34,8 @@ 'test/test_cpu_context.h', 'test/test_exception_snapshot.cc', 'test/test_exception_snapshot.h', + 'test/test_memory_map_region_snapshot.cc', + 'test/test_memory_map_region_snapshot.h', 'test/test_memory_snapshot.cc', 'test/test_memory_snapshot.h', 'test/test_module_snapshot.cc', diff --git a/snapshot/test/test_memory_map_region_snapshot.cc b/snapshot/test/test_memory_map_region_snapshot.cc new file mode 100644 index 00000000..53500c64 --- /dev/null +++ b/snapshot/test/test_memory_map_region_snapshot.cc @@ -0,0 +1,37 @@ +// 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 "snapshot/test/test_memory_map_region_snapshot.h" + +namespace crashpad { +namespace test { + +TestMemoryMapRegionSnapshot::TestMemoryMapRegionSnapshot() : memory_info_() { +} + +TestMemoryMapRegionSnapshot::~TestMemoryMapRegionSnapshot() { +} + +void TestMemoryMapRegionSnapshot::SetMindumpMemoryInfo( + const MINIDUMP_MEMORY_INFO& mmi) { + memory_info_ = mmi; +} + +const MINIDUMP_MEMORY_INFO& TestMemoryMapRegionSnapshot::AsMinidumpMemoryInfo() + const { + return memory_info_; +} + +} // namespace test +} // namespace crashpad diff --git a/snapshot/test/test_memory_map_region_snapshot.h b/snapshot/test/test_memory_map_region_snapshot.h new file mode 100644 index 00000000..e9edc5ca --- /dev/null +++ b/snapshot/test/test_memory_map_region_snapshot.h @@ -0,0 +1,47 @@ +// 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. + +#ifndef CRASHPAD_SNAPSHOT_TEST_TEST_MEMORY_MAP_REGION_SNAPSHOT_H_ +#define CRASHPAD_SNAPSHOT_TEST_TEST_MEMORY_MAP_REGION_SNAPSHOT_H_ + +#include + +#include "base/basictypes.h" +#include "snapshot/memory_map_region_snapshot.h" + +namespace crashpad { +namespace test { + +//! \brief A test MemoryMapRegionSnapshot that can carry arbitrary data for +//! testing purposes. +class TestMemoryMapRegionSnapshot final : public MemoryMapRegionSnapshot { + public: + TestMemoryMapRegionSnapshot(); + ~TestMemoryMapRegionSnapshot() override; + + void SetMindumpMemoryInfo(const MINIDUMP_MEMORY_INFO& mmi); + + // MemoryMapRegionSnapshot: + const MINIDUMP_MEMORY_INFO& AsMinidumpMemoryInfo() const override; + + private: + MINIDUMP_MEMORY_INFO memory_info_; + + DISALLOW_COPY_AND_ASSIGN(TestMemoryMapRegionSnapshot); +}; + +} // namespace test +} // namespace crashpad + +#endif // CRASHPAD_SNAPSHOT_TEST_TEST_MEMORY_MAP_REGION_SNAPSHOT_H_ From f059c2104861426fcb6c51ce062a2be57459649e Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Tue, 13 Oct 2015 13:19:19 -0700 Subject: [PATCH 08/18] Update mini_chromium to c9625ad5d23c25fbb477e7dbb4c1c8b9a9360f41 c9625ad5d23c Update base/numerics/* and base/template_util.h R=scottmg@chromium.org Review URL: https://codereview.chromium.org/1407603002 . --- DEPS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEPS b/DEPS index 03727b8d..b4e7e3d4 100644 --- a/DEPS +++ b/DEPS @@ -28,7 +28,7 @@ deps = { '01528c7244837168a1c80f06ff60fa5a9793c824', 'crashpad/third_party/mini_chromium/mini_chromium': Var('chromium_git') + '/chromium/mini_chromium@' + - '133a8c61c2567da9e72d4551a7cd5d5d2836e25c', + 'c9625ad5d23c25fbb477e7dbb4c1c8b9a9360f41', } hooks = [ From 1f1a24cb51444ee1c5fe9efb20bf870e84a9c620 Mon Sep 17 00:00:00 2001 From: Robert Sesek Date: Wed, 14 Oct 2015 16:56:04 -0700 Subject: [PATCH 09/18] Declare the random number generator lambda for std::random_shuffle as a local. When not building against the C++11 library headers, the compiler cannot treat the lambda as lvalue. When building against the C++11 library headers, it is converted to an rvalue. BUG=chromium:542321 R=mark@chromium.org Review URL: https://codereview.chromium.org/1406733003 . --- client/prune_crash_reports_test.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/client/prune_crash_reports_test.cc b/client/prune_crash_reports_test.cc index 50a27e18..7ffe67bc 100644 --- a/client/prune_crash_reports_test.cc +++ b/client/prune_crash_reports_test.cc @@ -207,9 +207,10 @@ TEST(PruneCrashReports, PruneOrder) { reports.push_back(temp); } // The randomness from std::rand() is not, so use a better rand() instead. - std::random_shuffle(reports.begin(), reports.end(), [](int rand_max) { + const auto random_generator = [](int rand_max) { return base::RandUint64() % rand_max; - }); + }; + std::random_shuffle(reports.begin(), reports.end(), random_generator); std::vector pending_reports( reports.begin(), reports.begin() + 5); std::vector completed_reports( From 4893a9b76d68176ba08857003ea4281d918768cf Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Thu, 15 Oct 2015 13:18:08 -0700 Subject: [PATCH 10/18] win: Capture some CRITICAL_SECTION debugging data Capture the memory for the loader lock (can be inspected by !cs), as well as all locks that were created with .DebugInfo which can be viewed with !locks. e.g. 0:000> !cs ntdll!LdrpLoaderLock ----------------------------------------- Critical section = 0x778d6410 (ntdll!LdrpLoaderLock+0x0) DebugInfo = 0x778d6b6c NOT LOCKED LockSemaphore = 0x0 SpinCount = 0x04000000 0:000> !locks -v CritSec ntdll!RtlpProcessHeapsListLock+0 at 778d7620 LockCount NOT LOCKED RecursionCount 0 OwningThread 0 EntryCount 0 ContentionCount 0 CritSec +7a0248 at 007a0248 LockCount NOT LOCKED RecursionCount 0 OwningThread 0 EntryCount 0 ContentionCount 0 CritSec crashy_program!g_critical_section_with_debug_info+0 at 01342c48 LockCount NOT LOCKED RecursionCount 0 OwningThread 0 EntryCount 0 ContentionCount 0 CritSec crashy_program!crashpad::`anonymous namespace'::g_test_critical_section+0 at 01342be0 WaiterWoken No LockCount 0 RecursionCount 1 OwningThread 34b8 EntryCount 0 ContentionCount 0 *** Locked Scanned 4 critical sections R=mark@chromium.org BUG=crashpad:52 Review URL: https://codereview.chromium.org/1392093003 . --- client/crashpad_client_win.cc | 37 ++++ handler/win/crash_report_exception_handler.cc | 6 +- handler/win/crash_report_exception_handler.h | 3 +- handler/win/crashy_test_program.cc | 18 ++ snapshot/crashpad_info_client_options_test.cc | 8 +- snapshot/win/end_to_end_test.py | 10 +- snapshot/win/exception_snapshot_win_test.cc | 14 +- snapshot/win/process_snapshot_win.cc | 158 +++++++++++++++--- snapshot/win/process_snapshot_win.h | 23 ++- snapshot/win/process_snapshot_win_test.cc | 4 +- tools/generate_dump.cc | 3 +- util/win/exception_handler_server.cc | 17 +- util/win/exception_handler_server.h | 6 +- util/win/exception_handler_server_test.cc | 3 +- util/win/process_structs.h | 30 ++++ util/win/registration_protocol_win.h | 12 +- 16 files changed, 297 insertions(+), 55 deletions(-) diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc index 19591600..7a1e547a 100644 --- a/client/crashpad_client_win.cc +++ b/client/crashpad_client_win.cc @@ -48,6 +48,13 @@ base::Lock* g_non_crash_dump_lock; // dump. crashpad::ExceptionInformation g_non_crash_exception_information; +// A CRITICAL_SECTION initialized with +// RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO to force it to be allocated with a +// valid .DebugInfo field. The address of this critical section is given to the +// handler. All critical sections with debug info are linked in a doubly-linked +// list, so this allows the handler to capture all of them. +CRITICAL_SECTION g_critical_section_with_debug_info; + LONG WINAPI UnhandledExceptionHandler(EXCEPTION_POINTERS* exception_pointers) { // Tracks whether a thread has already entered UnhandledExceptionHandler. static base::subtle::AtomicWord have_crashed; @@ -94,6 +101,18 @@ LONG WINAPI UnhandledExceptionHandler(EXCEPTION_POINTERS* exception_pointers) { return EXCEPTION_CONTINUE_SEARCH; } +BOOL CrashpadInitializeCriticalSectionEx( + CRITICAL_SECTION* critical_section, + DWORD spin_count, + DWORD flags) { + static decltype(InitializeCriticalSectionEx)* initialize_critical_section_ex = + reinterpret_cast(GetProcAddress( + LoadLibrary(L"kernel32.dll"), "InitializeCriticalSectionEx")); + if (!initialize_critical_section_ex) + return false; + return initialize_critical_section_ex(critical_section, spin_count, flags); +} + } // namespace namespace crashpad { @@ -118,6 +137,7 @@ bool CrashpadClient::SetHandler(const std::string& ipc_port) { DCHECK_EQ(g_signal_exception, INVALID_HANDLE_VALUE); DCHECK_EQ(g_signal_non_crash_dump, INVALID_HANDLE_VALUE); DCHECK_EQ(g_non_crash_dump_done, INVALID_HANDLE_VALUE); + DCHECK(!g_critical_section_with_debug_info.DebugInfo); ClientToServerMessage message; memset(&message, 0, sizeof(message)); @@ -129,6 +149,23 @@ bool CrashpadClient::SetHandler(const std::string& ipc_port) { message.registration.non_crash_exception_information = reinterpret_cast(&g_non_crash_exception_information); + // We create this dummy CRITICAL_SECTION with the + // RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO flag set to have an entry point + // into the doubly-linked list of RTL_CRITICAL_SECTION_DEBUG objects. This + // allows us to walk the list at crash time to gather data for !locks. A + // debugger would instead inspect ntdll!RtlCriticalSectionList to get the head + // of the list. But that is not an exported symbol, so on an arbitrary client + // machine, we don't have a way of getting that pointer. + if (CrashpadInitializeCriticalSectionEx( + &g_critical_section_with_debug_info, + 0, + RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO)) { + message.registration.critical_section_address = + reinterpret_cast(&g_critical_section_with_debug_info); + } else { + PLOG(ERROR) << "InitializeCriticalSectionEx"; + } + ServerToClientMessage response = {0}; if (!SendToCrashHandlerServer( diff --git a/handler/win/crash_report_exception_handler.cc b/handler/win/crash_report_exception_handler.cc index 843ac0f5..c33020e5 100644 --- a/handler/win/crash_report_exception_handler.cc +++ b/handler/win/crash_report_exception_handler.cc @@ -42,14 +42,16 @@ void CrashReportExceptionHandler::ExceptionHandlerServerStarted() { unsigned int CrashReportExceptionHandler::ExceptionHandlerServerException( HANDLE process, - WinVMAddress exception_information_address) { + WinVMAddress exception_information_address, + WinVMAddress debug_critical_section_address) { const unsigned int kFailedTerminationCode = 0xffff7002; ScopedProcessSuspend suspend(process); ProcessSnapshotWin process_snapshot; if (!process_snapshot.Initialize(process, - ProcessSuspensionState::kSuspended)) { + ProcessSuspensionState::kSuspended, + debug_critical_section_address)) { LOG(WARNING) << "ProcessSnapshotWin::Initialize failed"; return kFailedTerminationCode; } diff --git a/handler/win/crash_report_exception_handler.h b/handler/win/crash_report_exception_handler.h index 60e6b6ad..f03da161 100644 --- a/handler/win/crash_report_exception_handler.h +++ b/handler/win/crash_report_exception_handler.h @@ -60,7 +60,8 @@ class CrashReportExceptionHandler : public ExceptionHandlerServer::Delegate { void ExceptionHandlerServerStarted() override; unsigned int ExceptionHandlerServerException( HANDLE process, - WinVMAddress exception_information_address) override; + WinVMAddress exception_information_address, + WinVMAddress debug_critical_section_address) override; private: CrashReportDatabase* database_; // weak diff --git a/handler/win/crashy_test_program.cc b/handler/win/crashy_test_program.cc index 638098a6..0694a88b 100644 --- a/handler/win/crashy_test_program.cc +++ b/handler/win/crashy_test_program.cc @@ -28,6 +28,8 @@ namespace crashpad { namespace { +CRITICAL_SECTION g_test_critical_section; + ULONG RtlNtStatusToDosError(NTSTATUS status) { static decltype(::RtlNtStatusToDosError)* rtl_nt_status_to_dos_error = reinterpret_cast( @@ -75,6 +77,18 @@ void AllocateMemoryOfVariousProtections() { } } +BOOL CrashpadInitializeCriticalSectionEx( + CRITICAL_SECTION* critical_section, + DWORD spin_count, + DWORD flags) { + static decltype(InitializeCriticalSectionEx)* initialize_critical_section_ex = + reinterpret_cast(GetProcAddress( + LoadLibrary(L"kernel32.dll"), "InitializeCriticalSectionEx")); + if (!initialize_critical_section_ex) + return false; + return initialize_critical_section_ex(critical_section, spin_count, flags); +} + void SomeCrashyFunction() { // SetLastError and NTSTATUS so that we have something to view in !gle in // windbg. RtlNtStatusToDosError() stores STATUS_NO_SUCH_FILE into the @@ -103,6 +117,10 @@ int CrashyMain(int argc, char* argv[]) { AllocateMemoryOfVariousProtections(); + CrashpadInitializeCriticalSectionEx( + &g_test_critical_section, 0, RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO); + EnterCriticalSection(&g_test_critical_section); + SomeCrashyFunction(); return 0; diff --git a/snapshot/crashpad_info_client_options_test.cc b/snapshot/crashpad_info_client_options_test.cc index 933b77e1..d57f1c14 100644 --- a/snapshot/crashpad_info_client_options_test.cc +++ b/snapshot/crashpad_info_client_options_test.cc @@ -75,8 +75,8 @@ TEST(CrashpadInfoClientOptions, OneModule) { ASSERT_TRUE(process_snapshot.Initialize(mach_task_self())); #elif defined(OS_WIN) ProcessSnapshotWin process_snapshot; - ASSERT_TRUE(process_snapshot.Initialize(GetCurrentProcess(), - ProcessSuspensionState::kRunning)); + ASSERT_TRUE(process_snapshot.Initialize( + GetCurrentProcess(), ProcessSuspensionState::kRunning, 0)); #else #error Port. #endif // OS_MACOSX @@ -189,8 +189,8 @@ TEST(CrashpadInfoClientOptions, TwoModules) { ASSERT_TRUE(process_snapshot.Initialize(mach_task_self())); #elif defined(OS_WIN) ProcessSnapshotWin process_snapshot; - ASSERT_TRUE(process_snapshot.Initialize(GetCurrentProcess(), - ProcessSuspensionState::kRunning)); + ASSERT_TRUE(process_snapshot.Initialize( + GetCurrentProcess(), ProcessSuspensionState::kRunning, 0)); #else #error Port. #endif // OS_MACOSX diff --git a/snapshot/win/end_to_end_test.py b/snapshot/win/end_to_end_test.py index bbf442ee..42179409 100644 --- a/snapshot/win/end_to_end_test.py +++ b/snapshot/win/end_to_end_test.py @@ -177,12 +177,10 @@ def RunTests(cdb_path, dump_path, pipe_name): out.Check('LastStatusValue: \(NTSTATUS\) 0xc000000f - {File Not Found} The ' 'file %hs does not exist.', '!gle gets last ntstatus') - # Locks. - if False: # The code for these isn't landed yet. - out = CdbRun(cdb_path, dump_path, '!locks') - out.Check(r'CritSec crashy_program!crashpad::`anonymous namespace\'::' - r'g_test_critical_section', 'lock was captured') - out.Check(r'\*\*\* Locked', 'lock debug info was captured, and is locked') + out = CdbRun(cdb_path, dump_path, '!locks') + out.Check(r'CritSec crashy_program!crashpad::`anonymous namespace\'::' + r'g_test_critical_section', 'lock was captured') + out.Check(r'\*\*\* Locked', 'lock debug info was captured, and is locked') def main(args): diff --git a/snapshot/win/exception_snapshot_win_test.cc b/snapshot/win/exception_snapshot_win_test.cc index 3fc8c6d1..7c3b3691 100644 --- a/snapshot/win/exception_snapshot_win_test.cc +++ b/snapshot/win/exception_snapshot_win_test.cc @@ -88,10 +88,13 @@ class CrashingDelegate : public ExceptionHandlerServer::Delegate { unsigned int ExceptionHandlerServerException( HANDLE process, - WinVMAddress exception_information_address) override { + WinVMAddress exception_information_address, + WinVMAddress debug_critical_section_address) override { ScopedProcessSuspend suspend(process); ProcessSnapshotWin snapshot; - snapshot.Initialize(process, ProcessSuspensionState::kSuspended); + snapshot.Initialize(process, + ProcessSuspensionState::kSuspended, + debug_critical_section_address); snapshot.InitializeException(exception_information_address); // Confirm the exception record was read correctly. @@ -186,10 +189,13 @@ class SimulateDelegate : public ExceptionHandlerServer::Delegate { unsigned int ExceptionHandlerServerException( HANDLE process, - WinVMAddress exception_information_address) override { + WinVMAddress exception_information_address, + WinVMAddress debug_critical_section_address) override { ScopedProcessSuspend suspend(process); ProcessSnapshotWin snapshot; - snapshot.Initialize(process, ProcessSuspensionState::kSuspended); + snapshot.Initialize(process, + ProcessSuspensionState::kSuspended, + debug_critical_section_address); snapshot.InitializeException(exception_information_address); EXPECT_TRUE(snapshot.Exception()); EXPECT_EQ(0x517a7ed, snapshot.Exception()->Exception()); diff --git a/snapshot/win/process_snapshot_win.cc b/snapshot/win/process_snapshot_win.cc index 98e955c0..e3f4baad 100644 --- a/snapshot/win/process_snapshot_win.cc +++ b/snapshot/win/process_snapshot_win.cc @@ -43,8 +43,10 @@ ProcessSnapshotWin::ProcessSnapshotWin() ProcessSnapshotWin::~ProcessSnapshotWin() { } -bool ProcessSnapshotWin::Initialize(HANDLE process, - ProcessSuspensionState suspension_state) { +bool ProcessSnapshotWin::Initialize( + HANDLE process, + ProcessSuspensionState suspension_state, + WinVMAddress debug_critical_section_address) { INITIALIZATION_STATE_SET_INITIALIZING(initialized_); GetTimeOfDay(&snapshot_time_); @@ -54,10 +56,13 @@ bool ProcessSnapshotWin::Initialize(HANDLE process, system_.Initialize(&process_reader_); - if (process_reader_.Is64Bit()) - InitializePebData(); - else - InitializePebData(); + if (process_reader_.Is64Bit()) { + InitializePebData( + debug_critical_section_address); + } else { + InitializePebData( + debug_critical_section_address); + } InitializeThreads(); InitializeModules(); @@ -205,8 +210,8 @@ std::vector ProcessSnapshotWin::MemoryMap() std::vector ProcessSnapshotWin::ExtraMemory() const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); std::vector extra_memory; - for (const auto& peb_memory : peb_memory_) - extra_memory.push_back(peb_memory); + for (const auto& em : extra_memory_) + extra_memory.push_back(em); return extra_memory; } @@ -235,11 +240,12 @@ void ProcessSnapshotWin::InitializeModules() { } template -void ProcessSnapshotWin::InitializePebData() { +void ProcessSnapshotWin::InitializePebData( + WinVMAddress debug_critical_section_address) { WinVMAddress peb_address; WinVMSize peb_size; process_reader_.GetProcessInfo().Peb(&peb_address, &peb_size); - AddMemorySnapshot(peb_address, peb_size, &peb_memory_); + AddMemorySnapshot(peb_address, peb_size, &extra_memory_); process_types::PEB peb_data; if (!process_reader_.ReadMemory(peb_address, peb_size, &peb_data)) { @@ -248,7 +254,7 @@ void ProcessSnapshotWin::InitializePebData() { } process_types::PEB_LDR_DATA peb_ldr_data; - AddMemorySnapshot(peb_data.Ldr, sizeof(peb_ldr_data), &peb_memory_); + AddMemorySnapshot(peb_data.Ldr, sizeof(peb_ldr_data), &extra_memory_); if (!process_reader_.ReadMemory( peb_data.Ldr, sizeof(peb_ldr_data), &peb_ldr_data)) { LOG(ERROR) << "ReadMemory PEB_LDR_DATA"; @@ -257,17 +263,17 @@ void ProcessSnapshotWin::InitializePebData() { AddMemorySnapshotForLdrLIST_ENTRY( peb_ldr_data.InLoadOrderModuleList, offsetof(process_types::LDR_DATA_TABLE_ENTRY, InLoadOrderLinks), - &peb_memory_); + &extra_memory_); AddMemorySnapshotForLdrLIST_ENTRY( peb_ldr_data.InMemoryOrderModuleList, offsetof(process_types::LDR_DATA_TABLE_ENTRY, InMemoryOrderLinks), - &peb_memory_); + &extra_memory_); AddMemorySnapshotForLdrLIST_ENTRY( peb_ldr_data.InInitializationOrderModuleList, offsetof(process_types::LDR_DATA_TABLE_ENTRY, InInitializationOrderLinks), - &peb_memory_); + &extra_memory_); } process_types::RTL_USER_PROCESS_PARAMETERS process_parameters; @@ -278,27 +284,38 @@ void ProcessSnapshotWin::InitializePebData() { return; } AddMemorySnapshot( - peb_data.ProcessParameters, sizeof(process_parameters), &peb_memory_); + peb_data.ProcessParameters, sizeof(process_parameters), &extra_memory_); AddMemorySnapshotForUNICODE_STRING( - process_parameters.CurrentDirectory.DosPath, &peb_memory_); - AddMemorySnapshotForUNICODE_STRING(process_parameters.DllPath, &peb_memory_); + process_parameters.CurrentDirectory.DosPath, &extra_memory_); + AddMemorySnapshotForUNICODE_STRING(process_parameters.DllPath, + &extra_memory_); AddMemorySnapshotForUNICODE_STRING(process_parameters.ImagePathName, - &peb_memory_); + &extra_memory_); AddMemorySnapshotForUNICODE_STRING(process_parameters.CommandLine, - &peb_memory_); + &extra_memory_); AddMemorySnapshotForUNICODE_STRING(process_parameters.WindowTitle, - &peb_memory_); + &extra_memory_); AddMemorySnapshotForUNICODE_STRING(process_parameters.DesktopInfo, - &peb_memory_); + &extra_memory_); AddMemorySnapshotForUNICODE_STRING(process_parameters.ShellInfo, - &peb_memory_); + &extra_memory_); AddMemorySnapshotForUNICODE_STRING(process_parameters.RuntimeData, - &peb_memory_); + &extra_memory_); AddMemorySnapshot( process_parameters.Environment, DetermineSizeOfEnvironmentBlock(process_parameters.Environment), - &peb_memory_); + &extra_memory_); + + // Walk the loader lock which is directly referenced by the PEB. It may or may + // not have a .DebugInfo list, but doesn't on more recent OSs (it does on + // Vista). If it does, then we may walk the lock list more than once, but + // AddMemorySnapshot() will take care of deduplicating the added regions. + ReadLocks(peb_data.LoaderLock, &extra_memory_); + + // Traverse the locks with valid .DebugInfo if a starting point was supplied. + if (debug_critical_section_address) + ReadLocks(debug_critical_section_address, &extra_memory_); } void ProcessSnapshotWin::AddMemorySnapshot( @@ -399,4 +416,97 @@ WinVMSize ProcessSnapshotWin::DetermineSizeOfEnvironmentBlock( return env_block.size() * sizeof(env_block[0]); } +template +void ProcessSnapshotWin::ReadLocks( + WinVMAddress start, + PointerVector* into) { + // We're walking the RTL_CRITICAL_SECTION_DEBUG ProcessLocksList, but starting + // from an actual RTL_CRITICAL_SECTION, so start by getting to the first + // RTL_CRITICAL_SECTION_DEBUG. + + process_types::RTL_CRITICAL_SECTION critical_section; + if (!process_reader_.ReadMemory( + start, sizeof(critical_section), &critical_section)) { + LOG(ERROR) << "failed to read RTL_CRITICAL_SECTION"; + return; + } + + const decltype(critical_section.DebugInfo) kInvalid = + static_cast(-1); + if (critical_section.DebugInfo == kInvalid) + return; + + const WinVMAddress start_address_backward = critical_section.DebugInfo; + WinVMAddress current_address = start_address_backward; + WinVMAddress last_good_address; + + // Typically, this seems to be a circular list, but it's not clear that it + // always is, so follow Blink fields back to the head (or where we started) + // before following Flink to capture memory. + do { + last_good_address = current_address; + // Read the RTL_CRITICAL_SECTION_DEBUG structure to get ProcessLocksList. + process_types::RTL_CRITICAL_SECTION_DEBUG critical_section_debug; + if (!process_reader_.ReadMemory(current_address, + sizeof(critical_section_debug), + &critical_section_debug)) { + LOG(ERROR) << "failed to read RTL_CRITICAL_SECTION_DEBUG"; + return; + } + + if (critical_section_debug.ProcessLocksList.Blink == 0) { + // At the head of the list. + break; + } + + // Move to the previous RTL_CRITICAL_SECTION_DEBUG by walking + // ProcessLocksList.Blink. + current_address = + critical_section_debug.ProcessLocksList.Blink - + offsetof(process_types::RTL_CRITICAL_SECTION_DEBUG, + ProcessLocksList); + } while (current_address != start_address_backward && + current_address != kInvalid); + + if (current_address == kInvalid) { + // Unexpectedly encountered a bad record, so step back one. + current_address = last_good_address; + } + + const WinVMAddress start_address_forward = current_address; + + // current_address is now the head of the list, walk Flink to add the whole + // list. + do { + // Read the RTL_CRITICAL_SECTION_DEBUG structure to get ProcessLocksList. + process_types::RTL_CRITICAL_SECTION_DEBUG critical_section_debug; + if (!process_reader_.ReadMemory(current_address, + sizeof(critical_section_debug), + &critical_section_debug)) { + LOG(ERROR) << "failed to read RTL_CRITICAL_SECTION_DEBUG"; + return; + } + + // Add both RTL_CRITICAL_SECTION_DEBUG and RTL_CRITICAL_SECTION to the extra + // memory to be saved. + AddMemorySnapshot(current_address, + sizeof(process_types::RTL_CRITICAL_SECTION_DEBUG), + into); + AddMemorySnapshot(critical_section_debug.CriticalSection, + sizeof(process_types::RTL_CRITICAL_SECTION), + into); + + if (critical_section_debug.ProcessLocksList.Flink == 0) + break; + + // Move to the next RTL_CRITICAL_SECTION_DEBUG by walking + // ProcessLocksList.Flink. + current_address = + critical_section_debug.ProcessLocksList.Flink - + offsetof(process_types::RTL_CRITICAL_SECTION_DEBUG, + ProcessLocksList); + } while (current_address != start_address_forward && + current_address != kInvalid); +} + } // namespace crashpad diff --git a/snapshot/win/process_snapshot_win.h b/snapshot/win/process_snapshot_win.h index a38ac5e9..c9edfac3 100644 --- a/snapshot/win/process_snapshot_win.h +++ b/snapshot/win/process_snapshot_win.h @@ -59,12 +59,18 @@ class ProcessSnapshotWin final : public ProcessSnapshot { //! \param[in] process The handle to create a snapshot from. //! \param[in] suspension_state Whether \a process has been suspended by the //! caller. + //! \param[in] debug_critical_section_address The address in the target + //! process's address space of a `CRITICAL_SECTION` allocated with valid + //! `.DebugInfo`. Used as a starting point to walk the process's locks. + //! May be `0`. //! //! \return `true` if the snapshot could be created, `false` otherwise with //! an appropriate message logged. //! //! \sa ScopedProcessSuspend - bool Initialize(HANDLE process, ProcessSuspensionState suspension_state); + bool Initialize(HANDLE process, + ProcessSuspensionState suspension_state, + WinVMAddress debug_critical_section_address); //! \brief Initializes the object's exception. //! @@ -138,9 +144,10 @@ class ProcessSnapshotWin final : public ProcessSnapshot { // Initializes modules_ on behalf of Initialize(). void InitializeModules(); - // Initializes peb_memory_ on behalf of Initialize(). + // Initializes various memory blocks reachable from the PEB on behalf of + // Initialize(). template - void InitializePebData(); + void InitializePebData(WinVMAddress debug_critical_section_address); void AddMemorySnapshot(WinVMAddress address, WinVMSize size, @@ -160,8 +167,16 @@ class ProcessSnapshotWin final : public ProcessSnapshot { WinVMSize DetermineSizeOfEnvironmentBlock( WinVMAddress start_of_environment_block); + // Starting from the address of a CRITICAL_SECTION, walks the doubly-linked + // list stored in RTL_CRITICAL_SECTION.DebugInfo.ProcessLocksList adding both + // the RTL_CRITICAL_SECTION and the RTL_CRITICAL_SECTION_DEBUG memory blocks + // to the snapshot. + template + void ReadLocks(WinVMAddress start, + PointerVector* into); + internal::SystemSnapshotWin system_; - PointerVector peb_memory_; + PointerVector extra_memory_; PointerVector threads_; PointerVector modules_; scoped_ptr exception_; diff --git a/snapshot/win/process_snapshot_win_test.cc b/snapshot/win/process_snapshot_win_test.cc index 35cfc68c..0c4c3436 100644 --- a/snapshot/win/process_snapshot_win_test.cc +++ b/snapshot/win/process_snapshot_win_test.cc @@ -52,8 +52,8 @@ void TestImageReaderChild(const base::string16& directory_modification) { ScopedProcessSuspend suspend(child.process_handle()); ProcessSnapshotWin process_snapshot; - ASSERT_TRUE(process_snapshot.Initialize(child.process_handle(), - ProcessSuspensionState::kSuspended)); + ASSERT_TRUE(process_snapshot.Initialize( + child.process_handle(), ProcessSuspensionState::kSuspended, 0)); ASSERT_GE(process_snapshot.Modules().size(), 2u); diff --git a/tools/generate_dump.cc b/tools/generate_dump.cc index e3679749..0b7d39e3 100644 --- a/tools/generate_dump.cc +++ b/tools/generate_dump.cc @@ -186,7 +186,8 @@ int GenerateDumpMain(int argc, char* argv[]) { if (!process_snapshot.Initialize(process.get(), options.suspend ? ProcessSuspensionState::kSuspended - : ProcessSuspensionState::kRunning)) { + : ProcessSuspensionState::kRunning, + 0)) { return EXIT_FAILURE; } #endif // OS_MACOSX diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc index 840b1767..9cc69e67 100644 --- a/util/win/exception_handler_server.cc +++ b/util/win/exception_handler_server.cc @@ -105,6 +105,7 @@ class ClientData { ScopedKernelHANDLE process, WinVMAddress crash_exception_information_address, WinVMAddress non_crash_exception_information_address, + WinVMAddress debug_critical_section_address, WAITORTIMERCALLBACK crash_dump_request_callback, WAITORTIMERCALLBACK non_crash_dump_request_callback, WAITORTIMERCALLBACK process_end_callback) @@ -124,7 +125,8 @@ class ClientData { crash_exception_information_address_( crash_exception_information_address), non_crash_exception_information_address_( - non_crash_exception_information_address) { + non_crash_exception_information_address), + debug_critical_section_address_(debug_critical_section_address) { RegisterThreadPoolWaits(crash_dump_request_callback, non_crash_dump_request_callback, process_end_callback); @@ -155,6 +157,9 @@ class ClientData { WinVMAddress non_crash_exception_information_address() const { return non_crash_exception_information_address_; } + WinVMAddress debug_critical_section_address() const { + return debug_critical_section_address_; + } HANDLE process() const { return process_.get(); } private: @@ -219,6 +224,7 @@ class ClientData { ScopedKernelHANDLE process_; WinVMAddress crash_exception_information_address_; WinVMAddress non_crash_exception_information_address_; + WinVMAddress debug_critical_section_address_; DISALLOW_COPY_AND_ASSIGN(ClientData); }; @@ -412,6 +418,7 @@ bool ExceptionHandlerServer::ServiceClientConnection( ScopedKernelHANDLE(client_process), message.registration.crash_exception_information, message.registration.non_crash_exception_information, + message.registration.critical_section_address, &OnCrashDumpEvent, &OnNonCrashDumpEvent, &OnProcessEnd); @@ -465,7 +472,9 @@ void __stdcall ExceptionHandlerServer::OnCrashDumpEvent(void* ctx, BOOLEAN) { // Capture the exception. unsigned int exit_code = client->delegate()->ExceptionHandlerServerException( - client->process(), client->crash_exception_information_address()); + client->process(), + client->crash_exception_information_address(), + client->debug_critical_section_address()); TerminateProcess(client->process(), exit_code); } @@ -478,7 +487,9 @@ void __stdcall ExceptionHandlerServer::OnNonCrashDumpEvent(void* ctx, BOOLEAN) { // Capture the exception. client->delegate()->ExceptionHandlerServerException( - client->process(), client->non_crash_exception_information_address()); + client->process(), + client->non_crash_exception_information_address(), + client->debug_critical_section_address()); bool result = SetEvent(client->non_crash_dump_completed_event()); PLOG_IF(ERROR, !result) << "SetEvent"; diff --git a/util/win/exception_handler_server.h b/util/win/exception_handler_server.h index 69466f84..6928a6d9 100644 --- a/util/win/exception_handler_server.h +++ b/util/win/exception_handler_server.h @@ -49,11 +49,15 @@ class ExceptionHandlerServer { //! lifetime of this handle is not passed to the delegate. //! \param[in] exception_information_address The address in the client's //! address space of an ExceptionInformation structure. + //! \param[in] debug_critical_section_address The address in the client's + //! address space of a `CRITICAL_SECTION` allocated with a valid + //! `.DebugInfo` field, or `0` if unavailable. //! \return The exit code that should be used when terminating the client //! process. virtual unsigned int ExceptionHandlerServerException( HANDLE process, - WinVMAddress exception_information_address) = 0; + WinVMAddress exception_information_address, + WinVMAddress debug_critical_section_address) = 0; }; //! \brief Constructs the exception handling server. diff --git a/util/win/exception_handler_server_test.cc b/util/win/exception_handler_server_test.cc index 0c6a4799..0d87086b 100644 --- a/util/win/exception_handler_server_test.cc +++ b/util/win/exception_handler_server_test.cc @@ -64,7 +64,8 @@ class TestDelegate : public ExceptionHandlerServer::Delegate { } unsigned int ExceptionHandlerServerException( HANDLE process, - WinVMAddress exception_information_address) override { + WinVMAddress exception_information_address, + WinVMAddress debug_critical_section_address) override { return 0; } diff --git a/util/win/process_structs.h b/util/win/process_structs.h index 9c3be181..143431f0 100644 --- a/util/win/process_structs.h +++ b/util/win/process_structs.h @@ -232,6 +232,7 @@ struct PEB { typename Traits::Pointer UnicodeCaseTableData; DWORD NumberOfProcessors; DWORD NtGlobalFlag; + DWORD alignment_for_x86; LARGE_INTEGER CriticalSectionTimeout; typename Traits::UnsignedIntegral HeapSegmentReserve; typename Traits::UnsignedIntegral HeapSegmentCommit; @@ -454,6 +455,35 @@ struct EXCEPTION_POINTERS { using EXCEPTION_POINTERS32 = EXCEPTION_POINTERS; using EXCEPTION_POINTERS64 = EXCEPTION_POINTERS; +// This is defined in winnt.h, but not for cross-bitness. +template +struct RTL_CRITICAL_SECTION { + typename Traits::Pointer DebugInfo; + LONG LockCount; + LONG RecursionCount; + typename Traits::Pointer OwningThread; + typename Traits::Pointer LockSemaphore; + typename Traits::UnsignedIntegral SpinCount; +}; + +template +struct RTL_CRITICAL_SECTION_DEBUG { + union { + struct { + WORD Type; + WORD CreatorBackTraceIndex; + }; + typename Traits::Pad alignment_for_x64; + }; + typename Traits::Pointer CriticalSection; + LIST_ENTRY ProcessLocksList; + DWORD EntryCount; + DWORD ContentionCount; + DWORD Flags; + WORD CreatorBackTraceIndexHigh; + WORD SpareWORD; +}; + #pragma pack(pop) //! \} diff --git a/util/win/registration_protocol_win.h b/util/win/registration_protocol_win.h index 7c59bcdd..9a7bd296 100644 --- a/util/win/registration_protocol_win.h +++ b/util/win/registration_protocol_win.h @@ -49,15 +49,23 @@ struct RegistrationRequest { //! \brief The PID of the client process. DWORD client_process_id; - //! \brief The address, in the client process address space, of an + //! \brief The address, in the client process's address space, of an //! ExceptionInformation structure, used when handling a crash dump //! request. WinVMAddress crash_exception_information; - //! \brief The address, in the client process address space, of an + //! \brief The address, in the client process's address space, of an //! ExceptionInformation structure, used when handling a non-crashing dump //! request. WinVMAddress non_crash_exception_information; + + //! \brief The address, in the client process's address space, of a + //! `CRITICAL_SECTION` allocated with a valid .DebugInfo field. This can + //! be accomplished by using the + //! `RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO` flag to + //! `InitializeCriticalSectionEx()`. This value can be `0`, however then + //! limited lock data will be available in minidumps. + WinVMAddress critical_section_address; }; //! \brief A message only sent to the server by itself to trigger shutdown. From 71cc0a28a4960ba050c0be05b36b870df9d20348 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Thu, 15 Oct 2015 15:03:18 -0700 Subject: [PATCH 11/18] Add flush to output to try to diagnose locks failure end_to_end_test.py started failing after landing https://codereview.chromium.org/1392093003/ but I'm not sure why. It seems https://build.chromium.org/p/client.crashpad/builders/crashpad_win_x64_dbg/builds/45/steps/run%20tests/logs/stdio to be aborting in a place that doesn't make much sense, so try adding flushes to see if there's output getting lost. R=mark@chromium.org Review URL: https://codereview.chromium.org/1410633002 . --- snapshot/win/end_to_end_test.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/snapshot/win/end_to_end_test.py b/snapshot/win/end_to_end_test.py index 42179409..75dafde9 100644 --- a/snapshot/win/end_to_end_test.py +++ b/snapshot/win/end_to_end_test.py @@ -133,6 +133,7 @@ class CdbRun(object): # Matched. Consume up to end of match. self.out = self.out[match_obj.end(0):] print 'ok - %s' % message + sys.stdout.flush() else: print >>sys.stderr, '-' * 80 print >>sys.stderr, 'FAILED - %s' % message @@ -141,6 +142,7 @@ class CdbRun(object): print >>sys.stderr, '-' * 80 print >>sys.stderr, 'remaining output was:\n %s' % self.out print >>sys.stderr, '-' * 80 + sys.stderr.flush() sys.exit(1) From d1e49bd221d8584b77566da52ffc293203360c3d Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Fri, 16 Oct 2015 14:55:14 -0700 Subject: [PATCH 12/18] Fix CRITICAL_SECTION test I thought I had confirmed that this still allocated and ignored the flag on older OSs, but I must have not had the PLOG active yet? I'm not sure what I did. (I might try to blame VMware as it has an annoying habit of caching old binaries when you use it's "Shared Folders" feature to point at the dev machine's build dir.) I confirmed that it does work on Win8 and Win10 but doesn't on Win XP and Win 7. R=mark@chromium.org BUG=crashpad:52 Review URL: https://codereview.chromium.org/1405243002 . --- client/crashpad_client_win.cc | 21 +---- handler/handler.gyp | 1 + handler/win/crashy_test_program.cc | 20 ++--- snapshot/win/end_to_end_test.py | 5 +- util/util.gyp | 2 + util/util_test.gyp | 1 + util/win/critical_section_with_debug_info.cc | 76 +++++++++++++++++++ util/win/critical_section_with_debug_info.h | 34 +++++++++ .../critical_section_with_debug_info_test.cc | 34 +++++++++ util/win/registration_protocol_win.h | 8 +- 10 files changed, 164 insertions(+), 38 deletions(-) create mode 100644 util/win/critical_section_with_debug_info.cc create mode 100644 util/win/critical_section_with_debug_info.h create mode 100644 util/win/critical_section_with_debug_info_test.cc diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc index 7a1e547a..3b3eaa6a 100644 --- a/client/crashpad_client_win.cc +++ b/client/crashpad_client_win.cc @@ -23,6 +23,7 @@ #include "base/strings/utf_string_conversions.h" #include "base/synchronization/lock.h" #include "util/file/file_io.h" +#include "util/win/critical_section_with_debug_info.h" #include "util/win/registration_protocol_win.h" #include "util/win/scoped_handle.h" @@ -101,18 +102,6 @@ LONG WINAPI UnhandledExceptionHandler(EXCEPTION_POINTERS* exception_pointers) { return EXCEPTION_CONTINUE_SEARCH; } -BOOL CrashpadInitializeCriticalSectionEx( - CRITICAL_SECTION* critical_section, - DWORD spin_count, - DWORD flags) { - static decltype(InitializeCriticalSectionEx)* initialize_critical_section_ex = - reinterpret_cast(GetProcAddress( - LoadLibrary(L"kernel32.dll"), "InitializeCriticalSectionEx")); - if (!initialize_critical_section_ex) - return false; - return initialize_critical_section_ex(critical_section, spin_count, flags); -} - } // namespace namespace crashpad { @@ -156,14 +145,10 @@ bool CrashpadClient::SetHandler(const std::string& ipc_port) { // debugger would instead inspect ntdll!RtlCriticalSectionList to get the head // of the list. But that is not an exported symbol, so on an arbitrary client // machine, we don't have a way of getting that pointer. - if (CrashpadInitializeCriticalSectionEx( - &g_critical_section_with_debug_info, - 0, - RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO)) { + if (InitializeCriticalSectionWithDebugInfoIfPossible( + &g_critical_section_with_debug_info)) { message.registration.critical_section_address = reinterpret_cast(&g_critical_section_with_debug_info); - } else { - PLOG(ERROR) << "InitializeCriticalSectionEx"; } ServerToClientMessage response = {0}; diff --git a/handler/handler.gyp b/handler/handler.gyp index 8f8324c0..28bc9e35 100644 --- a/handler/handler.gyp +++ b/handler/handler.gyp @@ -82,6 +82,7 @@ '../client/client.gyp:crashpad_client', '../third_party/mini_chromium/mini_chromium.gyp:base', '../tools/tools.gyp:crashpad_tool_support', + '../util/util.gyp:crashpad_util', ], 'include_dirs': [ '..', diff --git a/handler/win/crashy_test_program.cc b/handler/win/crashy_test_program.cc index 0694a88b..4057df89 100644 --- a/handler/win/crashy_test_program.cc +++ b/handler/win/crashy_test_program.cc @@ -24,6 +24,7 @@ #include "base/logging.h" #include "client/crashpad_client.h" #include "tools/tool_support.h" +#include "util/win/critical_section_with_debug_info.h" namespace crashpad { namespace { @@ -77,18 +78,6 @@ void AllocateMemoryOfVariousProtections() { } } -BOOL CrashpadInitializeCriticalSectionEx( - CRITICAL_SECTION* critical_section, - DWORD spin_count, - DWORD flags) { - static decltype(InitializeCriticalSectionEx)* initialize_critical_section_ex = - reinterpret_cast(GetProcAddress( - LoadLibrary(L"kernel32.dll"), "InitializeCriticalSectionEx")); - if (!initialize_critical_section_ex) - return false; - return initialize_critical_section_ex(critical_section, spin_count, flags); -} - void SomeCrashyFunction() { // SetLastError and NTSTATUS so that we have something to view in !gle in // windbg. RtlNtStatusToDosError() stores STATUS_NO_SUCH_FILE into the @@ -117,9 +106,10 @@ int CrashyMain(int argc, char* argv[]) { AllocateMemoryOfVariousProtections(); - CrashpadInitializeCriticalSectionEx( - &g_test_critical_section, 0, RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO); - EnterCriticalSection(&g_test_critical_section); + if (InitializeCriticalSectionWithDebugInfoIfPossible( + &g_test_critical_section)) { + EnterCriticalSection(&g_test_critical_section); + } SomeCrashyFunction(); diff --git a/snapshot/win/end_to_end_test.py b/snapshot/win/end_to_end_test.py index 75dafde9..a09edd46 100644 --- a/snapshot/win/end_to_end_test.py +++ b/snapshot/win/end_to_end_test.py @@ -15,6 +15,7 @@ # limitations under the License. import os +import platform import random import re import subprocess @@ -182,7 +183,9 @@ def RunTests(cdb_path, dump_path, pipe_name): out = CdbRun(cdb_path, dump_path, '!locks') out.Check(r'CritSec crashy_program!crashpad::`anonymous namespace\'::' r'g_test_critical_section', 'lock was captured') - out.Check(r'\*\*\* Locked', 'lock debug info was captured, and is locked') + if float(platform.win32_ver()[0]) != 7: + # We can't allocate CRITICAL_SECTIONs with .DebugInfo on Win 7. + out.Check(r'\*\*\* Locked', 'lock debug info was captured, and is locked') def main(args): diff --git a/util/util.gyp b/util/util.gyp index 02e3b02c..26ed3d29 100644 --- a/util/util.gyp +++ b/util/util.gyp @@ -152,6 +152,8 @@ 'win/capture_context.asm', 'win/capture_context.h', 'win/checked_win_address_range.h', + 'win/critical_section_with_debug_info.cc', + 'win/critical_section_with_debug_info.h', 'win/exception_handler_server.cc', 'win/exception_handler_server.h', 'win/module_version.cc', diff --git a/util/util_test.gyp b/util/util_test.gyp index dc9ed60a..7de1ffcb 100644 --- a/util/util_test.gyp +++ b/util/util_test.gyp @@ -80,6 +80,7 @@ 'thread/thread_log_messages_test.cc', 'thread/thread_test.cc', 'win/capture_context_test.cc', + 'win/critical_section_with_debug_info_test.cc', 'win/exception_handler_server_test.cc', 'win/process_info_test.cc', 'win/scoped_process_suspend_test.cc', diff --git a/util/win/critical_section_with_debug_info.cc b/util/win/critical_section_with_debug_info.cc new file mode 100644 index 00000000..91387ae9 --- /dev/null +++ b/util/win/critical_section_with_debug_info.cc @@ -0,0 +1,76 @@ +// 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 "util/win/critical_section_with_debug_info.h" + +#include "base/logging.h" + +namespace crashpad { + +namespace { + +BOOL CrashpadInitializeCriticalSectionEx( + CRITICAL_SECTION* critical_section, + DWORD spin_count, + DWORD flags) { + static decltype(InitializeCriticalSectionEx)* initialize_critical_section_ex = + reinterpret_cast(GetProcAddress( + LoadLibrary(L"kernel32.dll"), "InitializeCriticalSectionEx")); + if (!initialize_critical_section_ex) { + PLOG(ERROR) << "GetProcAddress"; + return false; + } + bool ret = + initialize_critical_section_ex(critical_section, spin_count, flags); + if (!ret) { + PLOG(ERROR) << "InitializeCriticalSectionEx"; + return false; + } + return true; +} + +} // namespace + +bool InitializeCriticalSectionWithDebugInfoIfPossible( + CRITICAL_SECTION* critical_section) { + // On XP and Vista, a plain initialization causes the CRITICAL_SECTION to be + // allocated with .DebugInfo. On 8 and above, we can pass an additional flag + // to InitializeCriticalSectionEx() to force the .DebugInfo on. Before Win 8, + // that flag causes InitializeCriticalSectionEx() to fail. So, for XP, Vista, + // and 7 we use InitializeCriticalSection(), and for 8 and above, + // InitializeCriticalSectionEx() with the additional flag. + // + // TODO(scottmg): Try to find a solution for Win 7. It's unclear how to force + // it on for Win 7, however the Loader Lock does have .DebugInfo so there may + // be a way to do it. The comments in winnt.h imply that perhaps it's passed + // to InitializeCriticalSectionAndSpinCount() as the top bits of the spin + // count, but that doesn't appear to work. For now, we initialize a valid + // CRITICAL_SECTION, but without .DebugInfo. + + const DWORD version = GetVersion(); + const DWORD major_version = LOBYTE(LOWORD(version)); + const DWORD minor_version = HIBYTE(LOWORD(version)); + const bool win7_or_lower = + major_version < 6 || (major_version == 6 && minor_version <= 1); + + if (win7_or_lower) { + InitializeCriticalSection(critical_section); + return true; + } + + return CrashpadInitializeCriticalSectionEx( + critical_section, 0, RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO); +} + +} // namespace crashpad diff --git a/util/win/critical_section_with_debug_info.h b/util/win/critical_section_with_debug_info.h new file mode 100644 index 00000000..12716857 --- /dev/null +++ b/util/win/critical_section_with_debug_info.h @@ -0,0 +1,34 @@ +// 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. + +#ifndef CRASHPAD_UTIL_WIN_CRITICAL_SECTION_WITH_DEBUG_INFO_H_ +#define CRASHPAD_UTIL_WIN_CRITICAL_SECTION_WITH_DEBUG_INFO_H_ + +#include + +namespace crashpad { + +//! \brief Equivalent to `InitializeCritialSection()`, but attempts to allocate +//! with a valid `.DebugInfo` field on versions of Windows where it's +//! possible to do so. +//! +//! \return `true` on success, or `false` on failure with a message logged. +//! Success means that the critical section was successfully initialized, +//! but it does not necessarily have a valid `.DebugInfo` field. +bool InitializeCriticalSectionWithDebugInfoIfPossible( + CRITICAL_SECTION* critical_section); + +} // namespace crashpad + +#endif // CRASHPAD_UTIL_WIN_CRITICAL_SECTION_WITH_DEBUG_INFO_H_ diff --git a/util/win/critical_section_with_debug_info_test.cc b/util/win/critical_section_with_debug_info_test.cc new file mode 100644 index 00000000..dcc59655 --- /dev/null +++ b/util/win/critical_section_with_debug_info_test.cc @@ -0,0 +1,34 @@ +// 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 "util/win/critical_section_with_debug_info.h" + +#include "gtest/gtest.h" + +namespace crashpad { +namespace test { +namespace { + +TEST(CriticalSectionWithDebugInfo, CriticalSectionWithDebugInfo) { + CRITICAL_SECTION critical_section; + ASSERT_TRUE( + InitializeCriticalSectionWithDebugInfoIfPossible(&critical_section)); + EnterCriticalSection(&critical_section); + LeaveCriticalSection(&critical_section); + DeleteCriticalSection(&critical_section); +} + +} // namespace +} // namespace test +} // namespace crashpad diff --git a/util/win/registration_protocol_win.h b/util/win/registration_protocol_win.h index 9a7bd296..a691931d 100644 --- a/util/win/registration_protocol_win.h +++ b/util/win/registration_protocol_win.h @@ -61,10 +61,10 @@ struct RegistrationRequest { //! \brief The address, in the client process's address space, of a //! `CRITICAL_SECTION` allocated with a valid .DebugInfo field. This can - //! be accomplished by using the - //! `RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO` flag to - //! `InitializeCriticalSectionEx()`. This value can be `0`, however then - //! limited lock data will be available in minidumps. + //! be accomplished by using + //! InitializeCriticalSectionWithDebugInfoIfPossible() or equivalent. This + //! value can be `0`, however then limited lock data will be available in + //! minidumps. WinVMAddress critical_section_address; }; From 7de04b02f85ddfeee778b71f804d4d30c971ccd8 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Fri, 16 Oct 2015 15:31:32 -0700 Subject: [PATCH 13/18] win: Add Handles() to ProcessInfo To eventually be used to fill out MINIDUMP_HANDLE_DESCRIPTOR. R=mark@chromium.org BUG=crashpad:21, crashpad:46, crashpad:52 Review URL: https://codereview.chromium.org/1400413002 . --- util/win/nt_internals.cc | 16 ++++ util/win/nt_internals.h | 9 ++ util/win/process_info.cc | 170 ++++++++++++++++++++++++++++++++++ util/win/process_info.h | 40 ++++++++ util/win/process_info_test.cc | 112 ++++++++++++++++++++++ util/win/process_structs.h | 17 ++++ 6 files changed, 364 insertions(+) diff --git a/util/win/nt_internals.cc b/util/win/nt_internals.cc index 01f889b9..3fb5ef20 100644 --- a/util/win/nt_internals.cc +++ b/util/win/nt_internals.cc @@ -72,6 +72,22 @@ NTSTATUS NtOpenThread(PHANDLE thread_handle, static_cast(client_id)); } +NTSTATUS NtQueryObject(HANDLE handle, + OBJECT_INFORMATION_CLASS object_information_class, + void* object_information, + ULONG object_information_length, + ULONG* return_length) { + static decltype(::NtQueryObject)* nt_query_object = + reinterpret_cast( + GetProcAddress(LoadLibrary(L"ntdll.dll"), "NtQueryObject")); + DCHECK(nt_query_object); + return nt_query_object(handle, + object_information_class, + object_information, + object_information_length, + return_length); +} + // Explicit instantiations with the only 2 valid template arguments to avoid // putting the body of the function in the header. template NTSTATUS NtOpenThread( diff --git a/util/win/nt_internals.h b/util/win/nt_internals.h index 4743c434..aab884f5 100644 --- a/util/win/nt_internals.h +++ b/util/win/nt_internals.h @@ -27,6 +27,9 @@ namespace crashpad { // winternal.h defines THREADINFOCLASS, but not all members. enum { ThreadBasicInformation = 0 }; +// winternal.h defines SYSTEM_INFORMATION_CLASS, but not all members. +enum { SystemExtendedHandleInformation = 64 }; + NTSTATUS NtQuerySystemInformation( SYSTEM_INFORMATION_CLASS system_information_class, PVOID system_information, @@ -45,4 +48,10 @@ NTSTATUS NtOpenThread(PHANDLE thread_handle, POBJECT_ATTRIBUTES object_attributes, const process_types::CLIENT_ID* client_id); +NTSTATUS NtQueryObject(HANDLE handle, + OBJECT_INFORMATION_CLASS object_information_class, + void* object_information, + ULONG object_information_length, + ULONG* return_length); + } // namespace crashpad diff --git a/util/win/process_info.cc b/util/win/process_info.cc index fa413cc3..01748ff4 100644 --- a/util/win/process_info.cc +++ b/util/win/process_info.cc @@ -20,12 +20,15 @@ #include #include "base/logging.h" +#include "base/memory/scoped_ptr.h" #include "base/strings/stringprintf.h" #include "base/template_util.h" #include "build/build_config.h" #include "util/numeric/safe_assignment.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" namespace crashpad { @@ -127,6 +130,34 @@ MEMORY_BASIC_INFORMATION64 MemoryBasicInformationToMemoryBasicInformation64( return mbi64; } +// NtQueryObject with a retry for size mismatch as well as a minimum size to +// retrieve (and expect). +scoped_ptr QueryObject( + HANDLE handle, + OBJECT_INFORMATION_CLASS object_information_class, + ULONG minimum_size) { + ULONG size = minimum_size; + ULONG return_length; + scoped_ptr buffer(new uint8_t[size]); + NTSTATUS status = crashpad::NtQueryObject( + handle, object_information_class, buffer.get(), size, &return_length); + if (status == STATUS_INFO_LENGTH_MISMATCH) { + DCHECK_GT(return_length, size); + size = return_length; + buffer.reset(new uint8_t[size]); + status = crashpad::NtQueryObject( + handle, object_information_class, buffer.get(), size, &return_length); + } + + if (!NT_SUCCESS(status)) { + NTSTATUS_LOG(ERROR, status) << "NtQueryObject"; + return scoped_ptr(); + } + + DCHECK_GE(return_length, minimum_size); + return buffer.Pass(); +} + } // namespace template @@ -314,20 +345,150 @@ bool ReadMemoryInfo(HANDLE process, bool is_64_bit, ProcessInfo* process_info) { return true; } +std::vector ProcessInfo::BuildHandleVector( + HANDLE process) const { + ULONG buffer_size = 2 * 1024 * 1024; + scoped_ptr buffer(new uint8_t[buffer_size]); + + // Typically if the buffer were too small, STATUS_INFO_LENGTH_MISMATCH would + // return the correct size in the final argument, but it does not for + // SystemExtendedHandleInformation, so we loop and attempt larger sizes. + NTSTATUS status; + ULONG returned_length; + for (int tries = 0; tries < 5; ++tries) { + status = crashpad::NtQuerySystemInformation( + static_cast(SystemExtendedHandleInformation), + buffer.get(), + buffer_size, + &returned_length); + if (NT_SUCCESS(status) || status != STATUS_INFO_LENGTH_MISMATCH) + break; + + buffer_size *= 2; + buffer.reset(); + buffer.reset(new uint8_t[buffer_size]); + } + + if (!NT_SUCCESS(status)) { + NTSTATUS_LOG(ERROR, status) + << "NtQuerySystemInformation SystemExtendedHandleInformation"; + return std::vector(); + } + + const auto& system_handle_information_ex = + *reinterpret_cast( + buffer.get()); + + DCHECK_LE(offsetof(process_types::SYSTEM_HANDLE_INFORMATION_EX, Handles) + + system_handle_information_ex.NumberOfHandles * + sizeof(system_handle_information_ex.Handles[0]), + returned_length); + + std::vector handles; + + for (size_t i = 0; i < system_handle_information_ex.NumberOfHandles; ++i) { + const auto& handle = system_handle_information_ex.Handles[i]; + if (handle.UniqueProcessId != process_id_) + continue; + + Handle result_handle; + result_handle.handle = + static_cast(reinterpret_cast(handle.HandleValue)); + result_handle.attributes = handle.HandleAttributes; + result_handle.granted_access = handle.GrantedAccess; + + // TODO(scottmg): Could special case for self. + HANDLE dup_handle; + if (DuplicateHandle(process, + reinterpret_cast(handle.HandleValue), + GetCurrentProcess(), + &dup_handle, + 0, + false, + DUPLICATE_SAME_ACCESS)) { + // Some handles cannot be duplicated, for example, handles of type + // EtwRegistration. If we fail to duplicate, then we can't gather any more + // information, but include the information that we do have already. + ScopedKernelHANDLE scoped_dup_handle(dup_handle); + + scoped_ptr object_basic_information_buffer = + QueryObject(dup_handle, + ObjectBasicInformation, + sizeof(PUBLIC_OBJECT_BASIC_INFORMATION)); + if (object_basic_information_buffer) { + PUBLIC_OBJECT_BASIC_INFORMATION* object_basic_information = + reinterpret_cast( + object_basic_information_buffer.get()); + // The Attributes and GrantedAccess sometimes differ slightly between + // the data retrieved in SYSTEM_HANDLE_INFORMATION_EX and + // PUBLIC_OBJECT_TYPE_INFORMATION. We prefer the values in + // SYSTEM_HANDLE_INFORMATION_EX because they were retrieved from the + // target process, rather than on the duplicated handle, so don't use + // them here. + + // Subtract one to account for our DuplicateHandle() and another for + // NtQueryObject() while the query was being executed. + DCHECK_GT(object_basic_information->PointerCount, 2u); + result_handle.pointer_count = + object_basic_information->PointerCount - 2; + + // Subtract one to account for our DuplicateHandle(). + DCHECK_GT(object_basic_information->HandleCount, 1u); + result_handle.handle_count = object_basic_information->HandleCount - 1; + } + + scoped_ptr object_type_information_buffer = + QueryObject(dup_handle, + ObjectTypeInformation, + sizeof(PUBLIC_OBJECT_TYPE_INFORMATION)); + if (object_type_information_buffer) { + PUBLIC_OBJECT_TYPE_INFORMATION* object_type_information = + reinterpret_cast( + object_type_information_buffer.get()); + + DCHECK_EQ(object_type_information->TypeName.Length % + sizeof(result_handle.type_name[0]), + 0u); + result_handle.type_name = + std::wstring(object_type_information->TypeName.Buffer, + object_type_information->TypeName.Length / + sizeof(result_handle.type_name[0])); + } + } + + handles.push_back(result_handle); + } + return handles; +} + ProcessInfo::Module::Module() : name(), dll_base(0), size(0), timestamp() { } ProcessInfo::Module::~Module() { } +ProcessInfo::Handle::Handle() + : type_name(), + handle(0), + attributes(0), + granted_access(0), + pointer_count(0), + handle_count(0) { +} + +ProcessInfo::Handle::~Handle() { +} + ProcessInfo::ProcessInfo() : process_id_(), inherited_from_process_id_(), + process_(), command_line_(), peb_address_(0), peb_size_(0), modules_(), memory_info_(), + handles_(), is_64_bit_(false), is_wow64_(false), initialized_() { @@ -339,6 +500,8 @@ ProcessInfo::~ProcessInfo() { bool ProcessInfo::Initialize(HANDLE process) { INITIALIZATION_STATE_SET_INITIALIZING(initialized_); + process_ = process; + is_wow64_ = IsProcessWow64(process); if (is_wow64_) { @@ -439,6 +602,13 @@ ProcessInfo::GetReadableRanges( return GetReadableRangesOfMemoryMap(range, MemoryInfo()); } +const std::vector& ProcessInfo::Handles() { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + if (handles_.empty()) + handles_ = BuildHandleVector(process_); + return handles_; +} + std::vector> GetReadableRangesOfMemoryMap( const CheckedRange& range, const std::vector& memory_info) { diff --git a/util/win/process_info.h b/util/win/process_info.h index dac7b7c9..019e945c 100644 --- a/util/win/process_info.h +++ b/util/win/process_info.h @@ -50,6 +50,39 @@ class ProcessInfo { time_t timestamp; }; + struct Handle { + Handle(); + ~Handle(); + + //! \brief A string representation of the handle's type. + std::wstring type_name; + + //! \brief The handle's value. + //! + //! See https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203 on + //! 32 bits being the correct size for `HANDLE`s, even on Windows x64. + uint32_t handle; + + //! \brief The attributes for the handle, e.g. `OBJ_INHERIT`, + //! `OBJ_CASE_INSENSITIVE`, etc. + uint32_t attributes; + + //! \brief The `ACCESS_MASK` for the handle in this process. + //! + //! See + //! http://blogs.msdn.com/b/openspecification/archive/2010/04/01/about-the-access-mask-structure.aspx + //! for more information. + uint32_t granted_access; + + //! \brief The number of kernel references to the object that this handle + //! refers to. + uint32_t pointer_count; + + //! \brief The number of open handles to the object that this handle refers + //! to. + uint32_t handle_count; + }; + ProcessInfo(); ~ProcessInfo(); @@ -106,6 +139,9 @@ class ProcessInfo { std::vector> GetReadableRanges( const CheckedRange& range) const; + //! \brief Retrieves information about open handles in the target process. + const std::vector& Handles(); + private: template friend bool GetProcessBasicInformation(HANDLE process, @@ -122,13 +158,17 @@ class ProcessInfo { bool is_64_bit, ProcessInfo* process_info); + std::vector BuildHandleVector(HANDLE process) const; + pid_t process_id_; pid_t inherited_from_process_id_; + HANDLE process_; std::wstring command_line_; WinVMAddress peb_address_; WinVMSize peb_size_; std::vector modules_; std::vector memory_info_; + std::vector handles_; bool is_64_bit_; bool is_wow64_; InitializationStateDcheck initialized_; diff --git a/util/win/process_info_test.cc b/util/win/process_info_test.cc index ce9e8ac7..4a8a209f 100644 --- a/util/win/process_info_test.cc +++ b/util/win/process_info_test.cc @@ -20,8 +20,12 @@ #include "base/files/file_path.h" #include "base/memory/scoped_ptr.h" +#include "base/rand_util.h" +#include "base/strings/stringprintf.h" +#include "base/strings/utf_string_conversions.h" #include "build/build_config.h" #include "gtest/gtest.h" +#include "test/scoped_temp_dir.h" #include "test/paths.h" #include "test/win/child_launcher.h" #include "util/file/file_io.h" @@ -510,6 +514,114 @@ TEST(ProcessInfo, ReadableRanges) { &bytes_read)); } +struct ScopedRegistryKeyCloseTraits { + static HKEY InvalidValue() { + return nullptr; + } + static void Free(HKEY key) { + RegCloseKey(key); + } +}; +using ScopedRegistryKey = + base::ScopedGeneric; + +TEST(ProcessInfo, Handles) { + ScopedTempDir temp_dir; + + ScopedFileHandle file(LoggingOpenFileForWrite( + temp_dir.path().Append(FILE_PATH_LITERAL("test_file")), + FileWriteMode::kTruncateOrCreate, + FilePermissions::kWorldReadable)); + ASSERT_TRUE(file.is_valid()); + + SECURITY_ATTRIBUTES security_attributes = {0}; + security_attributes.bInheritHandle = true; + ScopedFileHandle inherited_file(CreateFile(L"CONOUT$", + GENERIC_WRITE, + 0, + &security_attributes, + OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, + nullptr)); + ASSERT_TRUE(inherited_file.is_valid()); + + HKEY key; + ASSERT_EQ(ERROR_SUCCESS, + RegOpenKeyEx( + HKEY_CURRENT_USER, L"SOFTWARE\\Microsoft", 0, KEY_READ, &key)); + ScopedRegistryKey scoped_key(key); + ASSERT_TRUE(scoped_key.is_valid()); + + std::wstring mapping_name = + base::UTF8ToUTF16(base::StringPrintf("Global\\test_mapping_%d_%I64x", + GetCurrentProcessId(), + base::RandUint64())); + ScopedKernelHANDLE mapping(CreateFileMapping(INVALID_HANDLE_VALUE, + nullptr, + PAGE_READWRITE, + 0, + 1024, + mapping_name.c_str())); + ASSERT_TRUE(mapping.is_valid()); + + ProcessInfo info; + info.Initialize(GetCurrentProcess()); + bool found_file_handle = false; + bool found_inherited_file_handle = false; + bool found_key_handle = false; + bool found_mapping_handle = false; + for (auto handle : info.Handles()) { + if (reinterpret_cast(file.get()) == handle.handle) { + EXPECT_FALSE(found_file_handle); + found_file_handle = true; + EXPECT_EQ(L"File", handle.type_name); + EXPECT_EQ(1, handle.handle_count); + EXPECT_NE(0u, handle.pointer_count); + EXPECT_EQ(STANDARD_RIGHTS_READ | STANDARD_RIGHTS_WRITE | SYNCHRONIZE, + handle.granted_access & STANDARD_RIGHTS_ALL); + EXPECT_EQ(0, handle.attributes); + } + if (reinterpret_cast(inherited_file.get()) == handle.handle) { + EXPECT_FALSE(found_inherited_file_handle); + found_inherited_file_handle = true; + EXPECT_EQ(L"File", handle.type_name); + EXPECT_EQ(1, handle.handle_count); + EXPECT_NE(0u, handle.pointer_count); + EXPECT_EQ(STANDARD_RIGHTS_READ | STANDARD_RIGHTS_WRITE | SYNCHRONIZE, + handle.granted_access & STANDARD_RIGHTS_ALL); + + // OBJ_INHERIT from ntdef.h, but including that conflicts with other + // headers. + const int kObjInherit = 0x2; + EXPECT_EQ(kObjInherit, handle.attributes); + } + if (reinterpret_cast(scoped_key.get()) == handle.handle) { + EXPECT_FALSE(found_key_handle); + found_key_handle = true; + EXPECT_EQ(L"Key", handle.type_name); + EXPECT_EQ(1, handle.handle_count); + EXPECT_NE(0u, handle.pointer_count); + EXPECT_EQ(STANDARD_RIGHTS_READ, + handle.granted_access & STANDARD_RIGHTS_ALL); + EXPECT_EQ(0, handle.attributes); + } + if (reinterpret_cast(mapping.get()) == handle.handle) { + EXPECT_FALSE(found_mapping_handle); + found_mapping_handle = true; + EXPECT_EQ(L"Section", handle.type_name); + EXPECT_EQ(1, handle.handle_count); + EXPECT_NE(0u, handle.pointer_count); + EXPECT_EQ(DELETE | READ_CONTROL | WRITE_DAC | WRITE_OWNER | + STANDARD_RIGHTS_READ | STANDARD_RIGHTS_WRITE, + handle.granted_access & STANDARD_RIGHTS_ALL); + EXPECT_EQ(0, handle.attributes); + } + } + EXPECT_TRUE(found_file_handle); + EXPECT_TRUE(found_key_handle); + EXPECT_TRUE(found_mapping_handle); +} + } // namespace } // namespace test } // namespace crashpad diff --git a/util/win/process_structs.h b/util/win/process_structs.h index 143431f0..5850ddec 100644 --- a/util/win/process_structs.h +++ b/util/win/process_structs.h @@ -484,6 +484,23 @@ struct RTL_CRITICAL_SECTION_DEBUG { WORD SpareWORD; }; +struct SYSTEM_HANDLE_TABLE_ENTRY_INFO_EX { + void* Object; + ULONG_PTR UniqueProcessId; + HANDLE HandleValue; + ULONG GrantedAccess; + USHORT CreatorBackTraceIndex; + USHORT ObjectTypeIndex; + ULONG HandleAttributes; + ULONG Reserved; +}; + +struct SYSTEM_HANDLE_INFORMATION_EX { + ULONG_PTR NumberOfHandles; + ULONG_PTR Reserved; + SYSTEM_HANDLE_TABLE_ENTRY_INFO_EX Handles[1]; +}; + #pragma pack(pop) //! \} From 4600643a78e8080713bfef6b12b7ab7b6956c6bf Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Fri, 16 Oct 2015 15:58:40 -0700 Subject: [PATCH 14/18] Some plumbing for the beginning of getting handles into snapshot/minidump Follows https://codereview.chromium.org/1400413002/. R=mark@chromium.org BUG=crashpad:21, crashpad:46, crashpad:52 Review URL: https://codereview.chromium.org/1407643004 . --- compat/non_win/dbghelp.h | 65 +++++++++++++++++++ snapshot/handle_snapshot.cc | 31 +++++++++ snapshot/handle_snapshot.h | 56 ++++++++++++++++ .../minidump/process_snapshot_minidump.cc | 6 ++ snapshot/minidump/process_snapshot_minidump.h | 1 + snapshot/process_snapshot.h | 7 ++ snapshot/snapshot.gyp | 2 + snapshot/test/test_process_snapshot.cc | 5 ++ snapshot/test/test_process_snapshot.h | 9 +++ snapshot/win/process_snapshot_win.cc | 15 +++++ snapshot/win/process_snapshot_win.h | 1 + util/win/process_info.cc | 2 +- util/win/process_info.h | 8 ++- 13 files changed, 205 insertions(+), 3 deletions(-) create mode 100644 snapshot/handle_snapshot.cc create mode 100644 snapshot/handle_snapshot.h diff --git a/compat/non_win/dbghelp.h b/compat/non_win/dbghelp.h index 6fed86bd..baf088ba 100644 --- a/compat/non_win/dbghelp.h +++ b/compat/non_win/dbghelp.h @@ -621,6 +621,71 @@ struct __attribute__((packed, aligned(4))) MINIDUMP_MEMORY_LIST { MINIDUMP_MEMORY_DESCRIPTOR MemoryRanges[0]; }; +//! \brief Contains the state of an individual system handle at the time the +//! snapshot was taken. This structure is Windows-specific. +//! +//! \sa MINIDUMP_HANDLE_DESCRIPTOR_2 +struct __attribute__((packed, aligned(4))) MINIDUMP_HANDLE_DESCRIPTOR { + //! \brief The Windows `HANDLE` value. + uint64_t Handle; + + //! \brief An RVA to a MINIDUMP_STRING structure that specifies the object + //! type of the handle. This member can be zero. + RVA TypeNameRva; + + //! \brief An RVA to a MINIDUMP_STRING structure that specifies the object + //! name of the handle. This member can be zero. + RVA ObjectNameRva; + + //! \brief The attributes for the handle, this corresponds to `OBJ_INHERIT`, + //! `OBJ_CASE_INSENSITIVE`, etc. + uint32_t Attributes; + + //! \brief The `ACCESS_MASK` for the handle. + uint32_t GrantedAccess; + + //! \brief This is the number of open handles to the object that this handle + //! refers to. + uint32_t HandleCount; + + //! \brief This is the number kernel references to the object that this + //! handle refers to. + uint32_t PointerCount; +}; + +//! \brief Contains the state of an individual system handle at the time the +//! snapshot was taken. This structure is Windows-specific. +//! +//! \sa MINIDUMP_HANDLE_DESCRIPTOR +struct __attribute__((packed, aligned(4))) MINIDUMP_HANDLE_DESCRIPTOR_2 + : public MINIDUMP_HANDLE_DESCRIPTOR { + //! \brief An RVA to a MINIDUMP_HANDLE_OBJECT_INFORMATION structure that + //! specifies object-specific information. This member can be zero if + //! there is no extra information. + RVA ObjectInfoRva; + + //! \brief Must be zero. + uint32_t Reserved0; +}; + +//! \brief Represents the header for a handle data stream. +struct __attribute((packed, aligned(4))) MINIDUMP_HANDLE_DATA_STREAM { + //! \brief The size of the header information for the stream, in bytes. This + //! value is `sizeof(MINIDUMP_HANDLE_DATA_STREAM)`. + uint32_t SizeOfHeader; + + //! \brief The size of a descriptor in the stream, in bytes. This value is + //! `sizeof(MINIDUMP_HANDLE_DESCRIPTOR)` or + //! `sizeof(MINIDUMP_HANDLE_DESCRIPTOR_2)`. + uint32_t SizeOfDescriptor; + + //! \brief The number of descriptors in the stream. + uint32_t NumberOfDescriptors; + + //! \brief Must be zero. + uint32_t Reserved; +}; + //! \anchor MINIDUMP_MISCx //! \name MINIDUMP_MISC* //! diff --git a/snapshot/handle_snapshot.cc b/snapshot/handle_snapshot.cc new file mode 100644 index 00000000..331b6b35 --- /dev/null +++ b/snapshot/handle_snapshot.cc @@ -0,0 +1,31 @@ +// 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 "snapshot/handle_snapshot.h" + +namespace crashpad { + +HandleSnapshot::HandleSnapshot() + : type_name(), + handle(0), + attributes(0), + granted_access(0), + pointer_count(0), + handle_count(0) { +} + +HandleSnapshot::~HandleSnapshot() { +} + +} // namespace crashpad diff --git a/snapshot/handle_snapshot.h b/snapshot/handle_snapshot.h new file mode 100644 index 00000000..b86706e7 --- /dev/null +++ b/snapshot/handle_snapshot.h @@ -0,0 +1,56 @@ +// 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. + +#ifndef CRASHPAD_SNAPSHOT_HANDLE_SNAPSHOT_H_ +#define CRASHPAD_SNAPSHOT_HANDLE_SNAPSHOT_H_ + +#include + +#include + +namespace crashpad { + +struct HandleSnapshot { + HandleSnapshot(); + ~HandleSnapshot(); + + //! \brief A string representation of the handle's type. + std::wstring type_name; + + //! \brief The handle's value. + uint32_t handle; + + //! \brief The attributes for the handle, e.g. `OBJ_INHERIT`, + //! `OBJ_CASE_INSENSITIVE`, etc. + uint32_t attributes; + + //! \brief The ACCESS_MASK for the handle in this process. + //! + //! See + //! http://blogs.msdn.com/b/openspecification/archive/2010/04/01/about-the-access-mask-structure.aspx + //! for more information. + uint32_t granted_access; + + //! \brief The number of kernel references to the object that this handle + //! refers to. + uint32_t pointer_count; + + //! \brief The number of open handles to the object that this handle refers + //! to. + uint32_t handle_count; +}; + +} // namespace crashpad + +#endif // CRASHPAD_SNAPSHOT_HANDLE_SNAPSHOT_H_ diff --git a/snapshot/minidump/process_snapshot_minidump.cc b/snapshot/minidump/process_snapshot_minidump.cc index d07274f3..2d6c6dbf 100644 --- a/snapshot/minidump/process_snapshot_minidump.cc +++ b/snapshot/minidump/process_snapshot_minidump.cc @@ -184,6 +184,12 @@ std::vector ProcessSnapshotMinidump::MemoryMap() return std::vector(); } +std::vector ProcessSnapshotMinidump::Handles() const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + NOTREACHED(); // https://code.google.com/p/crashpad/issues/detail?id=10 + return std::vector(); +} + std::vector ProcessSnapshotMinidump::ExtraMemory() const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); diff --git a/snapshot/minidump/process_snapshot_minidump.h b/snapshot/minidump/process_snapshot_minidump.h index c02fcc9b..9384eaff 100644 --- a/snapshot/minidump/process_snapshot_minidump.h +++ b/snapshot/minidump/process_snapshot_minidump.h @@ -70,6 +70,7 @@ class ProcessSnapshotMinidump final : public ProcessSnapshot { std::vector Modules() const override; const ExceptionSnapshot* Exception() const override; std::vector MemoryMap() const override; + std::vector Handles() const override; std::vector ExtraMemory() const override; private: diff --git a/snapshot/process_snapshot.h b/snapshot/process_snapshot.h index 0a2f4f9f..19d34607 100644 --- a/snapshot/process_snapshot.h +++ b/snapshot/process_snapshot.h @@ -22,6 +22,7 @@ #include #include +#include "snapshot/handle_snapshot.h" #include "util/misc/uuid.h" namespace crashpad { @@ -171,6 +172,12 @@ class ProcessSnapshot { //! the ProcessSnapshot object that they were obtained from. virtual std::vector MemoryMap() const = 0; + //! \brief Returns HandleSnapshot objects reflecting the open handles in the + //! snapshot process at the time of the snapshot. + //! + //! \return A vector of HandleSnapshot objects. + virtual std::vector Handles() const = 0; + //! \brief Returns a vector of additional memory blocks that should be //! included in a minidump. //! diff --git a/snapshot/snapshot.gyp b/snapshot/snapshot.gyp index 76d07aa6..0a78e90c 100644 --- a/snapshot/snapshot.gyp +++ b/snapshot/snapshot.gyp @@ -36,6 +36,8 @@ 'crashpad_info_client_options.cc', 'crashpad_info_client_options.h', 'exception_snapshot.h', + 'handle_snapshot.cc', + 'handle_snapshot.h', 'mac/cpu_context_mac.cc', 'mac/cpu_context_mac.h', 'mac/exception_snapshot_mac.cc', diff --git a/snapshot/test/test_process_snapshot.cc b/snapshot/test/test_process_snapshot.cc index d388c601..c022dc41 100644 --- a/snapshot/test/test_process_snapshot.cc +++ b/snapshot/test/test_process_snapshot.cc @@ -34,6 +34,7 @@ TestProcessSnapshot::TestProcessSnapshot() modules_(), exception_(), memory_map_(), + handles_(), extra_memory_() { } @@ -107,6 +108,10 @@ std::vector TestProcessSnapshot::MemoryMap() return memory_map; } +std::vector TestProcessSnapshot::Handles() const { + return handles_; +} + std::vector TestProcessSnapshot::ExtraMemory() const { std::vector extra_memory; for (const auto& em : extra_memory_) diff --git a/snapshot/test/test_process_snapshot.h b/snapshot/test/test_process_snapshot.h index 2f5cd45a..ec124481 100644 --- a/snapshot/test/test_process_snapshot.h +++ b/snapshot/test/test_process_snapshot.h @@ -106,6 +106,13 @@ class TestProcessSnapshot final : public ProcessSnapshot { memory_map_.push_back(region.release()); } + //! \brief Adds a handle snapshot to be returned by Handles(). + //! + //! \param[in] region The handle snapshot that will be included in Handles(). + void AddHandle(const HandleSnapshot& handle) { + handles_.push_back(handle); + } + //! \brief Add a memory snapshot to be returned by ExtraMemory(). //! //! \param[in] extra_memory The memory snapshot that will be included in @@ -131,6 +138,7 @@ class TestProcessSnapshot final : public ProcessSnapshot { std::vector Modules() const override; const ExceptionSnapshot* Exception() const override; std::vector MemoryMap() const override; + std::vector Handles() const override; std::vector ExtraMemory() const override; private: @@ -148,6 +156,7 @@ class TestProcessSnapshot final : public ProcessSnapshot { PointerVector modules_; scoped_ptr exception_; PointerVector memory_map_; + std::vector handles_; PointerVector extra_memory_; DISALLOW_COPY_AND_ASSIGN(TestProcessSnapshot); diff --git a/snapshot/win/process_snapshot_win.cc b/snapshot/win/process_snapshot_win.cc index e3f4baad..ca26d4b8 100644 --- a/snapshot/win/process_snapshot_win.cc +++ b/snapshot/win/process_snapshot_win.cc @@ -207,6 +207,21 @@ std::vector ProcessSnapshotWin::MemoryMap() return memory_map; } +std::vector ProcessSnapshotWin::Handles() const { + std::vector result; + for (const auto& handle : process_reader_.GetProcessInfo().Handles()) { + HandleSnapshot snapshot; + snapshot.type_name = handle.type_name; + snapshot.handle = handle.handle; + snapshot.attributes = handle.attributes; + snapshot.granted_access = handle.granted_access; + snapshot.pointer_count = handle.pointer_count; + snapshot.handle_count = handle.handle_count; + result.push_back(snapshot); + } + return result; +} + std::vector ProcessSnapshotWin::ExtraMemory() const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); std::vector extra_memory; diff --git a/snapshot/win/process_snapshot_win.h b/snapshot/win/process_snapshot_win.h index c9edfac3..a7238fd7 100644 --- a/snapshot/win/process_snapshot_win.h +++ b/snapshot/win/process_snapshot_win.h @@ -135,6 +135,7 @@ class ProcessSnapshotWin final : public ProcessSnapshot { std::vector Modules() const override; const ExceptionSnapshot* Exception() const override; std::vector MemoryMap() const override; + std::vector Handles() const override; std::vector ExtraMemory() const override; private: diff --git a/util/win/process_info.cc b/util/win/process_info.cc index 01748ff4..ee5668b4 100644 --- a/util/win/process_info.cc +++ b/util/win/process_info.cc @@ -602,7 +602,7 @@ ProcessInfo::GetReadableRanges( return GetReadableRangesOfMemoryMap(range, MemoryInfo()); } -const std::vector& ProcessInfo::Handles() { +const std::vector& ProcessInfo::Handles() const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); if (handles_.empty()) handles_ = BuildHandleVector(process_); diff --git a/util/win/process_info.h b/util/win/process_info.h index 019e945c..41b92598 100644 --- a/util/win/process_info.h +++ b/util/win/process_info.h @@ -140,7 +140,7 @@ class ProcessInfo { const CheckedRange& range) const; //! \brief Retrieves information about open handles in the target process. - const std::vector& Handles(); + const std::vector& Handles() const; private: template @@ -168,7 +168,11 @@ class ProcessInfo { WinVMSize peb_size_; std::vector modules_; std::vector memory_info_; - std::vector handles_; + + // Handles() is logically const, but updates this member on first retrieval. + // See https://code.google.com/p/crashpad/issues/detail?id=9. + mutable std::vector handles_; + bool is_64_bit_; bool is_wow64_; InitializationStateDcheck initialized_; From 30678f1e8232c7ed8dff28efa9efde52cd348dc6 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Fri, 16 Oct 2015 16:33:40 -0700 Subject: [PATCH 15/18] Fix Mac build after https://codereview.chromium.org/1407643004 Oops. R=mark@chromium.org BUG=crashpad:21, crashpad:52 Review URL: https://codereview.chromium.org/1409823003 . --- snapshot/mac/process_snapshot_mac.cc | 5 +++++ snapshot/mac/process_snapshot_mac.h | 1 + 2 files changed, 6 insertions(+) diff --git a/snapshot/mac/process_snapshot_mac.cc b/snapshot/mac/process_snapshot_mac.cc index 215536cc..b37123c8 100644 --- a/snapshot/mac/process_snapshot_mac.cc +++ b/snapshot/mac/process_snapshot_mac.cc @@ -192,6 +192,11 @@ std::vector ProcessSnapshotMac::MemoryMap() return std::vector(); } +std::vector ProcessSnapshotMac::Handles() const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + return std::vector(); +} + std::vector ProcessSnapshotMac::ExtraMemory() const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); return std::vector(); diff --git a/snapshot/mac/process_snapshot_mac.h b/snapshot/mac/process_snapshot_mac.h index 8712679c..dd5b9c3a 100644 --- a/snapshot/mac/process_snapshot_mac.h +++ b/snapshot/mac/process_snapshot_mac.h @@ -128,6 +128,7 @@ class ProcessSnapshotMac final : public ProcessSnapshot { std::vector Modules() const override; const ExceptionSnapshot* Exception() const override; std::vector MemoryMap() const override; + std::vector Handles() const override; std::vector ExtraMemory() const override; private: From 07dbc3259c568fc08c868a7e14502ce617d469d9 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Mon, 19 Oct 2015 11:43:58 -0400 Subject: [PATCH 16/18] Use an even better random number generation scheme in the prune test base::RandInt(0, max - 1) has a uniform distribution. base::RandUint64() % max does not. TEST=crashpad_client_test PruneCrashReports.PruneOrder R=rsesek@chromium.org Review URL: https://codereview.chromium.org/1417443002 . --- client/prune_crash_reports_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/prune_crash_reports_test.cc b/client/prune_crash_reports_test.cc index 7ffe67bc..bb9ea8e2 100644 --- a/client/prune_crash_reports_test.cc +++ b/client/prune_crash_reports_test.cc @@ -208,7 +208,7 @@ TEST(PruneCrashReports, PruneOrder) { } // The randomness from std::rand() is not, so use a better rand() instead. const auto random_generator = [](int rand_max) { - return base::RandUint64() % rand_max; + return base::RandInt(0, rand_max - 1); }; std::random_shuffle(reports.begin(), reports.end(), random_generator); std::vector pending_reports( From 1818dbbb0851527281a281622b36a742664baa3f Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Mon, 19 Oct 2015 13:40:50 -0400 Subject: [PATCH 17/18] win: Fix crashpad_util_test ProcessInfo.Handles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This new test from 7de04b02f85d was failing on Windows 10. I started by adding the hint, which produced “CreateFileMapping: Access is denied. (0x5)”. Switching the “Global\” to “Local\” fixes the test for me. TEST=crashpad_util_test ProcessInfo.Handles R=scottmg@chromium.org Review URL: https://codereview.chromium.org/1407993003 . --- util/win/process_info_test.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/util/win/process_info_test.cc b/util/win/process_info_test.cc index 4a8a209f..925d0062 100644 --- a/util/win/process_info_test.cc +++ b/util/win/process_info_test.cc @@ -25,6 +25,7 @@ #include "base/strings/utf_string_conversions.h" #include "build/build_config.h" #include "gtest/gtest.h" +#include "test/errors.h" #include "test/scoped_temp_dir.h" #include "test/paths.h" #include "test/win/child_launcher.h" @@ -553,7 +554,7 @@ TEST(ProcessInfo, Handles) { ASSERT_TRUE(scoped_key.is_valid()); std::wstring mapping_name = - base::UTF8ToUTF16(base::StringPrintf("Global\\test_mapping_%d_%I64x", + base::UTF8ToUTF16(base::StringPrintf("Local\\test_mapping_%d_%I64x", GetCurrentProcessId(), base::RandUint64())); ScopedKernelHANDLE mapping(CreateFileMapping(INVALID_HANDLE_VALUE, @@ -562,7 +563,7 @@ TEST(ProcessInfo, Handles) { 0, 1024, mapping_name.c_str())); - ASSERT_TRUE(mapping.is_valid()); + ASSERT_TRUE(mapping.is_valid()) << ErrorMessage("CreateFileMapping"); ProcessInfo info; info.Initialize(GetCurrentProcess()); From 2adcd13fd66b0e375c0799090cd2847a6b6d87ce Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Mon, 19 Oct 2015 13:51:52 -0400 Subject: [PATCH 18/18] =?UTF-8?q?Update=20developer=20documentation=20to?= =?UTF-8?q?=20recommend=20the=20=E2=80=9Cfetch=E2=80=9D=20tool?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit “fetch crashpad“ is possible since depot_tools ea1b3d5ed88b. R=rsesek@chromium.org Review URL: https://codereview.chromium.org/1408133003 . --- doc/developing.ad | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/doc/developing.ad b/doc/developing.ad index 67c3ce4d..efb45536 100644 --- a/doc/developing.ad +++ b/doc/developing.ad @@ -53,23 +53,25 @@ https://chromium.googlesource.com/crashpad/crashpad. Although it is possible to check out this repository directly with `git clone`, Crashpad’s dependencies are managed by https://dev.chromium.org/developers/how-tos/depottools#TOC-gclient[`gclient`] -instead of Git submodules, so to work on Crashpad, it is best to use `gclient` -to get the source code. +instead of Git submodules, so to work on Crashpad, it is best to use `fetch` to +get the source code. -`gclient` is part of the +`fetch` and `gclient` are part of the https://dev.chromium.org/developers/how-tos/depottools[depot_tools]. There’s no -need to install it separately. +need to install them separately. === Initial Checkout [subs="verbatim,quotes"] ---- -$ *mkdir \~/crashpad* +$ *mkdir ~/crashpad* $ *cd ~/crashpad* -$ *gclient config --unmanaged https://chromium.googlesource.com/crashpad/crashpad* -$ *gclient sync* +$ *fetch crashpad* ---- +`fetch crashpad` performs the initial `gclient sync`, establishing a +fully-functional local checkout. + === Subsequent Checkouts [subs="verbatim,quotes"] @@ -85,7 +87,7 @@ Crashpad uses https://gyp.googlecode.com/[GYP] to generate https://martine.github.io/ninja/[Ninja] build files. The build is described by `.gyp` files throughout the Crashpad source code tree. The `build/gyp_crashpad.py` script runs GYP properly for Crashpad, and is also -called when you run `gclient sync` or `gclient runhooks`. +called when you run `fetch crashpad`, `gclient sync`, or `gclient runhooks`. The Ninja build files and build output are in the `out` directory. Both debug- and release-mode configurations are available. The examples below show the debug