From ff52791faf302cf95c860ee13dd9ea637a7775a9 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Wed, 6 May 2015 11:09:31 -0700 Subject: [PATCH] Get generate_dump compiling on Windows Sort of works in that the process is opened, modules retrieved, etc. but eventually CHECKs due to missing functionality in snapshot. Follows https://codereview.chromium.org/1119783005/. R=mark@chromium.org BUG=crashpad:1 Review URL: https://codereview.chromium.org/1120383003 --- DEPS | 2 +- compat/win/sys/types.h | 3 +- minidump/minidump_system_info_writer.cc | 3 ++ tools/crashpad_database_util.cc | 31 +++-------- tools/generate_dump.cc | 71 ++++++++++++++++++++----- tools/tool_support.cc | 31 +++++++++++ tools/tool_support.h | 20 +++++++ tools/tools.gyp | 56 ++++++++++--------- util/win/process_info.cc | 11 ++-- 9 files changed, 160 insertions(+), 68 deletions(-) diff --git a/DEPS b/DEPS index f8e86a65..470d029d 100644 --- a/DEPS +++ b/DEPS @@ -28,7 +28,7 @@ deps = { '32ca1cd8e010d013a606a752fb49a603a3598071', # svn r2015 'crashpad/third_party/mini_chromium/mini_chromium': Var('chromium_git') + '/chromium/mini_chromium@' + - 'bf64e609259be3faf24e32aa899491f22fa1fb90', + '597e041ee33d5c27574a78cfd256d3a70e8c1fe1', } hooks = [ diff --git a/compat/win/sys/types.h b/compat/win/sys/types.h index 6cd8ab3d..b6463a0c 100644 --- a/compat/win/sys/types.h +++ b/compat/win/sys/types.h @@ -22,11 +22,10 @@ #ifdef _WIN64 typedef int64_t ssize_t; -typedef uint64_t pid_t; #else typedef __w64 int ssize_t; -typedef __w64 unsigned int pid_t; #endif +typedef unsigned int pid_t; #endif // CRASHPAD_COMPAT_WIN_SYS_TYPES_H_ diff --git a/minidump/minidump_system_info_writer.cc b/minidump/minidump_system_info_writer.cc index 4797d6ea..309f3a39 100644 --- a/minidump/minidump_system_info_writer.cc +++ b/minidump/minidump_system_info_writer.cc @@ -154,6 +154,9 @@ void MinidumpSystemInfoWriter::InitializeFromSnapshot( case SystemSnapshot::kOperatingSystemMacOSX: operating_system = kMinidumpOSMacOSX; break; + case SystemSnapshot::kOperatingSystemWindows: + operating_system = kMinidumpOSWin32NT; + break; default: NOTREACHED(); operating_system = kMinidumpOSUnknown; diff --git a/tools/crashpad_database_util.cc b/tools/crashpad_database_util.cc index bcb742c9..c06cb576 100644 --- a/tools/crashpad_database_util.cc +++ b/tools/crashpad_database_util.cc @@ -38,16 +38,6 @@ #include "util/file/file_reader.h" #include "util/misc/uuid.h" -#if defined(OS_POSIX) -base::FilePath::StringType UTF8ToFilePathStringType(const char* path) { - return path; -} -#elif defined(OS_WIN) -base::FilePath::StringType UTF8ToFilePathStringType(const char* path) { - return base::UTF8ToUTF16(path); -} -#endif - namespace crashpad { namespace { @@ -253,8 +243,9 @@ void ShowReports(const std::vector& reports, } int DatabaseUtilMain(int argc, char* argv[]) { - const base::FilePath me( - base::FilePath(UTF8ToFilePathStringType(argv[0])).BaseName()); + const base::FilePath argv0( + ToolSupport::CommandLineArgumentToFilePathStringType(argv[0])); + const base::FilePath me(argv0.BaseName()); enum OptionFlags { // “Short” (single-character) options. @@ -364,8 +355,8 @@ int DatabaseUtilMain(int argc, char* argv[]) { break; } case kOptionNewReport: { - options.new_report_paths.push_back( - base::FilePath(UTF8ToFilePathStringType(optarg))); + options.new_report_paths.push_back(base::FilePath( + ToolSupport::CommandLineArgumentToFilePathStringType(optarg))); break; } case kOptionUTC: { @@ -425,7 +416,8 @@ int DatabaseUtilMain(int argc, char* argv[]) { } scoped_ptr database(CrashReportDatabase::Initialize( - base::FilePath(UTF8ToFilePathStringType(options.database)))); + base::FilePath(ToolSupport::CommandLineArgumentToFilePathStringType( + options.database)))); if (!database) { return EXIT_FAILURE; } @@ -585,13 +577,6 @@ int main(int argc, char* argv[]) { } #elif defined(OS_WIN) int wmain(int argc, wchar_t* argv[]) { - scoped_ptr argv_as_utf8(new char*[argc]); - std::vector storage; - storage.reserve(argc); - for (int i = 0; i < argc; ++i) { - storage.push_back(base::UTF16ToUTF8(argv[i])); - argv_as_utf8[i] = &storage[i][0]; - } - return crashpad::DatabaseUtilMain(argc, argv_as_utf8.get()); + return crashpad::ToolSupport::Wmain(argc, argv, crashpad::DatabaseUtilMain); } #endif // OS_POSIX diff --git a/tools/generate_dump.cc b/tools/generate_dump.cc index ecb1bda0..818d232d 100644 --- a/tools/generate_dump.cc +++ b/tools/generate_dump.cc @@ -14,27 +14,35 @@ #include #include -#include -#include #include #include -#include +#include #include #include "base/logging.h" -#include "base/mac/scoped_mach_port.h" #include "base/memory/scoped_ptr.h" #include "base/strings/stringprintf.h" +#include "build/build_config.h" #include "minidump/minidump_file_writer.h" -#include "snapshot/mac/process_snapshot_mac.h" #include "tools/tool_support.h" #include "util/file/file_writer.h" -#include "util/mach/scoped_task_suspend.h" -#include "util/mach/task_for_pid.h" #include "util/posix/drop_privileges.h" #include "util/stdlib/string_number_conversion.h" +#if defined(OS_MACOSX) +#include +#include + +#include "base/mac/scoped_mach_port.h" +#include "snapshot/mac/process_snapshot_mac.h" +#include "util/mach/scoped_task_suspend.h" +#include "util/mach/task_for_pid.h" +#elif defined(OS_WIN) +#include "base/strings/utf_string_conversions.h" +#include "snapshot/win/process_snapshot_win.h" +#endif // OS_MACOSX + namespace crashpad { namespace { @@ -44,21 +52,23 @@ struct Options { bool suspend; }; -void Usage(const std::string& me) { +void Usage(const base::FilePath& me) { fprintf(stderr, -"Usage: %s [OPTION]... PID\n" +"Usage: %" PRFilePath " [OPTION]... PID\n" "Generate a minidump file containing a snapshot of a running process.\n" "\n" " -r, --no-suspend don't suspend the target process during dump generation\n" " -o, --output=FILE write the minidump to FILE instead of minidump.PID\n" " --help display this help and exit\n" " --version output version information and exit\n", - me.c_str()); + me.value().c_str()); ToolSupport::UsageTail(me); } int GenerateDumpMain(int argc, char* argv[]) { - const std::string me(basename(argv[0])); + const base::FilePath argv0( + ToolSupport::CommandLineArgumentToFilePathStringType(argv[0])); + const base::FilePath me(argv0.BaseName()); enum OptionFlags { // “Short” (single-character) options. @@ -113,10 +123,14 @@ int GenerateDumpMain(int argc, char* argv[]) { } if (!StringToNumber(argv[0], &options.pid) || options.pid <= 0) { - fprintf(stderr, "%s: invalid PID: %s\n", me.c_str(), argv[0]); + fprintf(stderr, + "%" PRFilePath ": invalid PID: %s\n", + me.value().c_str(), + argv[0]); return EXIT_FAILURE; } +#if defined(OS_MACOSX) task_t task = TaskForPID(options.pid); if (task == TASK_NULL) { return EXIT_FAILURE; @@ -134,24 +148,49 @@ int GenerateDumpMain(int argc, char* argv[]) { } LOG(WARNING) << "operating on myself"; } +#elif defined(OS_WIN) + ScopedKernelHANDLE process( + OpenProcess(PROCESS_ALL_ACCESS, false, options.pid)); + if (!process.is_valid()) { + LOG(ERROR) << "could not open process " << options.pid; + return EXIT_FAILURE; + } +#endif // OS_MACOSX if (options.dump_path.empty()) { options.dump_path = base::StringPrintf("minidump.%d", options.pid); } { +#if defined(OS_MACOSX) scoped_ptr suspend; if (options.suspend) { suspend.reset(new ScopedTaskSuspend(task)); } +#elif defined(OS_WIN) + if (options.suspend) { + LOG(ERROR) << "TODO(scottmg): --no-suspend is required for now."; + return EXIT_FAILURE; + } +#endif // OS_MACOSX +#if defined(OS_MACOSX) ProcessSnapshotMac process_snapshot; if (!process_snapshot.Initialize(task)) { return EXIT_FAILURE; } +#elif defined(OS_WIN) + ProcessSnapshotWin process_snapshot; + if (!process_snapshot.Initialize(process.get())) { + return EXIT_FAILURE; + } +#endif // OS_MACOSX FileWriter file_writer; - if (!file_writer.Open(base::FilePath(options.dump_path), + base::FilePath dump_path( + ToolSupport::CommandLineArgumentToFilePathStringType( + options.dump_path)); + if (!file_writer.Open(dump_path, FileWriteMode::kTruncateOrCreate, FilePermissions::kWorldReadable)) { return EXIT_FAILURE; @@ -173,6 +212,12 @@ int GenerateDumpMain(int argc, char* argv[]) { } // namespace } // namespace crashpad +#if defined(OS_POSIX) int main(int argc, char* argv[]) { return crashpad::GenerateDumpMain(argc, argv); } +#elif defined(OS_WIN) +int wmain(int argc, wchar_t* argv[]) { + return crashpad::ToolSupport::Wmain(argc, argv, crashpad::GenerateDumpMain); +} +#endif // OS_POSIX diff --git a/tools/tool_support.cc b/tools/tool_support.cc index f4960246..e911f634 100644 --- a/tools/tool_support.cc +++ b/tools/tool_support.cc @@ -16,6 +16,10 @@ #include +#include + +#include "base/memory/scoped_ptr.h" +#include "base/strings/utf_string_conversions.h" #include "package.h" namespace crashpad { @@ -67,4 +71,31 @@ void ToolSupport::UsageHint(const std::string& me, const char* hint) { } #endif // OS_POSIX +#if defined(OS_WIN) + +// static +int ToolSupport::Wmain(int argc, wchar_t* argv[], int (*entry)(int, char* [])) { + scoped_ptr argv_as_utf8(new char* [argc + 1]); + std::vector storage; + storage.reserve(argc); + for (int i = 0; i < argc; ++i) { + storage.push_back(base::UTF16ToUTF8(argv[i])); + argv_as_utf8[i] = &storage[i][0]; + } + argv_as_utf8[argc] = nullptr; + return entry(argc, argv_as_utf8.get()); +} + +#endif // OS_WIN + +// static +base::FilePath::StringType ToolSupport::CommandLineArgumentToFilePathStringType( + const base::StringPiece& path) { +#if defined(OS_POSIX) + return path.as_string(); +#elif defined(OS_WIN) + return base::UTF8ToUTF16(path); +#endif // OS_POSIX +} + } // namespace crashpad diff --git a/tools/tool_support.h b/tools/tool_support.h index 47976bc9..9099691a 100644 --- a/tools/tool_support.h +++ b/tools/tool_support.h @@ -19,6 +19,7 @@ #include "base/basictypes.h" #include "base/files/file_path.h" +#include "base/strings/string_piece.h" #include "build/build_config.h" namespace crashpad { @@ -53,6 +54,25 @@ class ToolSupport { static void UsageHint(const std::string& me, const char* hint); #endif // OS_POSIX +#if defined(OS_WIN) || DOXYGEN + //! \brief Converts \a argv `wchar_t` UTF-16 to UTF-8, and passes onwards to a + //! UTF-8 entry point. + //! + //! \return The return value of \a entry. + static int Wmain(int argc, wchar_t* argv[], int (*entry)(int, char*[])); +#endif // OS_WIN + + //! \brief Converts a command line argument to the string type suitable for + //! base::FilePath. + //! + //! On POSIX, this is a no-op. On Windows, assumes that Wmain() was used, and + //! the input argument was converted from UTF-16 in a `wchar_t*` to UTF-8 in a + //! `char*`. This undoes that transformation. + //! + //! \sa Wmain() + static base::FilePath::StringType CommandLineArgumentToFilePathStringType( + const base::StringPiece& arg); + private: DISALLOW_IMPLICIT_CONSTRUCTORS(ToolSupport); }; diff --git a/tools/tools.gyp b/tools/tools.gyp index 49c17056..48763005 100644 --- a/tools/tools.gyp +++ b/tools/tools.gyp @@ -48,6 +48,36 @@ 'crashpad_database_util.cc', ], }, + { + 'target_name': 'generate_dump', + 'type': 'executable', + 'dependencies': [ + 'crashpad_tool_support', + '../compat/compat.gyp:crashpad_compat', + '../minidump/minidump.gyp:crashpad_minidump', + '../snapshot/snapshot.gyp:crashpad_snapshot', + '../third_party/mini_chromium/mini_chromium.gyp:base', + '../util/util.gyp:crashpad_util', + ], + 'include_dirs': [ + '..', + ], + 'sources': [ + 'generate_dump.cc', + ], + 'conditions': [ + ['OS=="mac"', { + 'xcode_settings': { + 'OTHER_LDFLAGS': [ + '-sectcreate', + '__TEXT', + '__info_plist', + '<(sectaskaccess_info_plist)' + ], + }, + }], + ], + } ], 'conditions': [ ['OS=="mac"', { @@ -118,32 +148,6 @@ ], }, }, - { - 'target_name': 'generate_dump', - 'type': 'executable', - 'dependencies': [ - 'crashpad_tool_support', - '../compat/compat.gyp:crashpad_compat', - '../minidump/minidump.gyp:crashpad_minidump', - '../snapshot/snapshot.gyp:crashpad_snapshot', - '../third_party/mini_chromium/mini_chromium.gyp:base', - '../util/util.gyp:crashpad_util', - ], - 'include_dirs': [ - '..', - ], - 'sources': [ - 'generate_dump.cc', - ], - 'xcode_settings': { - 'OTHER_LDFLAGS': [ - '-sectcreate', - '__TEXT', - '__info_plist', - '<(sectaskaccess_info_plist)' - ], - }, - }, { 'target_name': 'on_demand_service_tool', 'type': 'executable', diff --git a/util/win/process_info.cc b/util/win/process_info.cc index 73c9bf85..ca8277ed 100644 --- a/util/win/process_info.cc +++ b/util/win/process_info.cc @@ -259,9 +259,14 @@ bool ProcessInfo::Initialize(HANDLE process) { LOG(ERROR) << "NtQueryInformationProcess incorrect size"; return false; } - process_id_ = process_basic_information.UniqueProcessId; - inherited_from_process_id_ = - process_basic_information.InheritedFromUniqueProcessId; + + // See https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203 on + // 32 bit being the correct size for HANDLEs for proceses, even on Windows + // x64. API functions (e.g. OpenProcess) take only a DWORD, so there's no + // sense in maintaining the top bits. + process_id_ = static_cast(process_basic_information.UniqueProcessId); + inherited_from_process_id_ = static_cast( + process_basic_information.InheritedFromUniqueProcessId); // We now want to read the PEB to gather the rest of our information. The // PebBaseAddress as returned above is what we want for 64-on-64 and 32-on-32,