From 20045d38679c35f3b5886c665706dac4cbb003b4 Mon Sep 17 00:00:00 2001
From: Mark Mentovai <mark@chromium.org>
Date: Thu, 5 Nov 2015 13:48:27 -0500
Subject: [PATCH 01/12] Add buildtools to make depot_tools-wrapped clang-format
work
Crashpad is mostly friendly with clang-format and has its own
.clang-format file. Adding buildtools makes it possible to use the
depot_tools clang-format wrapper.
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1414903006 .
---
DEPS | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/DEPS b/DEPS
index 34313f8e..b5e38f1b 100644
--- a/DEPS
+++ b/DEPS
@@ -29,9 +29,40 @@ deps = {
'crashpad/third_party/mini_chromium/mini_chromium':
Var('chromium_git') + '/chromium/mini_chromium@' +
'8e12d3df2f1c0fcd84d649f4619323558db63a85',
+ 'buildtools':
+ Var('chromium_git') + '/chromium/buildtools.git@' +
+ 'c2f259809d5ede3275df5ea0842f0431990c4f98',
}
hooks = [
+ {
+ 'name': 'clang_format_mac',
+ 'pattern': '.',
+ 'action': [
+ 'download_from_google_storage',
+ '--platform=^darwin$',
+ '--no_resume',
+ '--no_auth',
+ '--bucket=chromium-clang-format',
+ '--output=buildtools/mac/clang-format',
+ '--sha1_file',
+ 'buildtools/mac/clang-format.sha1',
+ ],
+ },
+ {
+ 'name': 'clang_format_win',
+ 'pattern': '.',
+ 'action': [
+ 'download_from_google_storage',
+ '--platform=^win32$',
+ '--no_resume',
+ '--no_auth',
+ '--bucket=chromium-clang-format',
+ '--output=buildtools/win/clang-format.exe',
+ '--sha1_file',
+ 'buildtools/win/clang-format.exe.sha1',
+ ],
+ },
{
'name': 'gyp',
'pattern': '\.gypi?$',
From 2eeaa3ac549a6646031cac05f8d1424ff4946157 Mon Sep 17 00:00:00 2001
From: Mark Mentovai <mark@chromium.org>
Date: Thu, 5 Nov 2015 14:00:26 -0500
Subject: [PATCH 02/12] win: Add HandleToInt() and IntToHandle()
This consolidates all of the twisted casts and comments that discuss how
HANDLEs are really only 32 bits wide even in 64-bit processes on 64-bit
operating systems into a single location.
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1422503015 .
---
client/crashpad_client_win.cc | 19 ++++----
handler/main.cc | 13 +++---
test/win/win_child_process.cc | 9 ++--
util/util.gyp | 2 +
util/util_test.gyp | 1 +
util/win/exception_handler_server.cc | 13 +++---
util/win/handle.cc | 36 +++++++++++++++
util/win/handle.h | 65 ++++++++++++++++++++++++++++
util/win/handle_test.cc | 53 +++++++++++++++++++++++
util/win/process_info.cc | 12 +++--
util/win/process_info.h | 3 --
util/win/process_info_test.cc | 11 +++--
util/win/registration_protocol_win.h | 5 ---
13 files changed, 194 insertions(+), 48 deletions(-)
create mode 100644 util/win/handle.cc
create mode 100644 util/win/handle.h
create mode 100644 util/win/handle_test.cc
diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc
index a2506cb7..46b61372 100644
--- a/client/crashpad_client_win.cc
+++ b/client/crashpad_client_win.cc
@@ -26,6 +26,7 @@
#include "util/file/file_io.h"
#include "util/win/command_line.h"
#include "util/win/critical_section_with_debug_info.h"
+#include "util/win/handle.h"
#include "util/win/registration_protocol_win.h"
#include "util/win/scoped_handle.h"
@@ -166,13 +167,9 @@ bool CrashpadClient::StartHandler(
base::UTF8ToUTF16(kv.first + '=' + kv.second)),
&command_line);
}
-
- // According to
- // https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203, HANDLEs
- // are always 32 bits.
AppendCommandLineArgument(
base::UTF8ToUTF16(base::StringPrintf("--handshake-handle=0x%x",
- pipe_write)),
+ HandleToInt(pipe_write))),
&command_line);
STARTUPINFO startup_info = {};
@@ -271,12 +268,12 @@ bool CrashpadClient::UseHandler() {
}
// The server returns these already duplicated to be valid in this process.
- g_signal_exception = reinterpret_cast<HANDLE>(
- static_cast<uintptr_t>(response.registration.request_crash_dump_event));
- g_signal_non_crash_dump = reinterpret_cast<HANDLE>(static_cast<uintptr_t>(
- response.registration.request_non_crash_dump_event));
- g_non_crash_dump_done = reinterpret_cast<HANDLE>(static_cast<uintptr_t>(
- response.registration.non_crash_dump_completed_event));
+ g_signal_exception =
+ IntToHandle(response.registration.request_crash_dump_event);
+ g_signal_non_crash_dump =
+ IntToHandle(response.registration.request_non_crash_dump_event);
+ g_non_crash_dump_done =
+ IntToHandle(response.registration.non_crash_dump_completed_event);
g_non_crash_dump_lock = new base::Lock();
diff --git a/handler/main.cc b/handler/main.cc
index 7b1f7304..fcc3f71c 100644
--- a/handler/main.cc
+++ b/handler/main.cc
@@ -46,6 +46,7 @@
#include <windows.h>
#include "handler/win/crash_report_exception_handler.h"
#include "util/win/exception_handler_server.h"
+#include "util/win/handle.h"
#endif // OS_MACOSX
namespace crashpad {
@@ -181,18 +182,14 @@ int HandlerMain(int argc, char* argv[]) {
}
#elif defined(OS_WIN)
case kOptionHandshakeHandle: {
- // According to
- // https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203,
- // HANDLEs are always 32 bits. This construction is used to read a full
- // 32-bit number presented by the client, which uses 0x%x format, and
- // sign-extend it.
+ // Use unsigned int, because the handle was presented by the client in
+ // 0x%x format.
unsigned int handle_uint;
if (!StringToNumber(optarg, &handle_uint) ||
- (options.handshake_handle = reinterpret_cast<HANDLE>(
- static_cast<int>(handle_uint))) == INVALID_HANDLE_VALUE) {
+ (options.handshake_handle = IntToHandle(handle_uint)) ==
+ INVALID_HANDLE_VALUE) {
ToolSupport::UsageHint(me, "--handshake-handle requires a HANDLE");
return EXIT_FAILURE;
- }
break;
}
case kOptionPipeName: {
diff --git a/test/win/win_child_process.cc b/test/win/win_child_process.cc
index 577b6b63..7d179337 100644
--- a/test/win/win_child_process.cc
+++ b/test/win/win_child_process.cc
@@ -25,6 +25,7 @@
#include "gtest/gtest.h"
#include "util/stdlib/string_number_conversion.h"
#include "util/string/split_string.h"
+#include "util/win/handle.h"
#include "test/paths.h"
namespace crashpad {
@@ -152,8 +153,8 @@ WinChildProcess::WinChildProcess() {
unsigned int write, read;
CHECK(StringToNumber(left, &write));
CHECK(StringToNumber(right, &read));
- pipe_write_.reset(reinterpret_cast<HANDLE>(static_cast<uintptr_t>(write)));
- pipe_read_.reset(reinterpret_cast<HANDLE>(static_cast<uintptr_t>(read)));
+ pipe_write_.reset(IntToHandle(write));
+ pipe_read_.reset(IntToHandle(read));
// Notify the parent that it's OK to proceed. We only need to wait to get to
// the process entry point, but this is the easiest place we can notify.
@@ -193,8 +194,8 @@ scoped_ptr<WinChildProcess::Handles> WinChildProcess::Launch() {
test_info->test_case_name(),
test_info->name(),
kIsMultiprocessChild,
- write_for_child,
- read_for_child.get()));
+ HandleToInt(write_for_child.get()),
+ HandleToInt(read_for_child.get())));
// Command-line buffer cannot be constant, per CreateProcess signature.
handles_for_parent->process = LaunchCommandLine(&command_line[0]);
diff --git a/util/util.gyp b/util/util.gyp
index 65791b5a..685032c7 100644
--- a/util/util.gyp
+++ b/util/util.gyp
@@ -160,6 +160,8 @@
'win/exception_handler_server.h',
'win/get_function.cc',
'win/get_function.h',
+ 'win/handle.cc',
+ 'win/handle.h',
'win/module_version.cc',
'win/module_version.h',
'win/nt_internals.cc',
diff --git a/util/util_test.gyp b/util/util_test.gyp
index 40bb64fd..9d235fa3 100644
--- a/util/util_test.gyp
+++ b/util/util_test.gyp
@@ -84,6 +84,7 @@
'win/critical_section_with_debug_info_test.cc',
'win/exception_handler_server_test.cc',
'win/get_function_test.cc',
+ 'win/handle_test.cc',
'win/process_info_test.cc',
'win/scoped_process_suspend_test.cc',
'win/time_test.cc',
diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc
index 2035c578..ce5687d7 100644
--- a/util/win/exception_handler_server.cc
+++ b/util/win/exception_handler_server.cc
@@ -28,6 +28,7 @@
#include "util/misc/tri_state.h"
#include "util/misc/uuid.h"
#include "util/win/get_function.h"
+#include "util/win/handle.h"
#include "util/win/registration_protocol_win.h"
#include "util/win/xp_compat.h"
@@ -489,14 +490,14 @@ bool ExceptionHandlerServer::ServiceClientConnection(
// Duplicate the events back to the client so they can request a dump.
ServerToClientMessage response;
response.registration.request_crash_dump_event =
- base::checked_cast<uint32_t>(reinterpret_cast<uintptr_t>(DuplicateEvent(
- client->process(), client->crash_dump_requested_event())));
+ HandleToInt(DuplicateEvent(
+ client->process(), client->crash_dump_requested_event()));
response.registration.request_non_crash_dump_event =
- base::checked_cast<uint32_t>(reinterpret_cast<uintptr_t>(DuplicateEvent(
- client->process(), client->non_crash_dump_requested_event())));
+ HandleToInt(DuplicateEvent(
+ client->process(), client->non_crash_dump_requested_event()));
response.registration.non_crash_dump_completed_event =
- base::checked_cast<uint32_t>(reinterpret_cast<uintptr_t>(DuplicateEvent(
- client->process(), client->non_crash_dump_completed_event())));
+ HandleToInt(DuplicateEvent(
+ client->process(), client->non_crash_dump_completed_event()));
if (!LoggingWriteFile(service_context.pipe(), &response, sizeof(response)))
return false;
diff --git a/util/win/handle.cc b/util/win/handle.cc
new file mode 100644
index 00000000..c53f5438
--- /dev/null
+++ b/util/win/handle.cc
@@ -0,0 +1,36 @@
+// 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/handle.h"
+
+#include <stdint.h>
+
+#include "base/numerics/safe_conversions.h"
+
+namespace crashpad {
+
+// These functions use “int” for the 32-bit integer handle type because
+// sign-extension needs to work correctly. INVALID_HANDLE_VALUE is defined as
+// ((HANDLE)(LONG_PTR)-1), and this needs to round-trip through an integer and
+// back to the same HANDLE value.
+
+int HandleToInt(HANDLE handle) {
+ return base::checked_cast<int>(reinterpret_cast<intptr_t>(handle));
+}
+
+HANDLE IntToHandle(int handle_int) {
+ return reinterpret_cast<HANDLE>(static_cast<intptr_t>(handle_int));
+}
+
+} // namespace crashpad
diff --git a/util/win/handle.h b/util/win/handle.h
new file mode 100644
index 00000000..c5a96cae
--- /dev/null
+++ b/util/win/handle.h
@@ -0,0 +1,65 @@
+// 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_HANDLE_TO_INT_H_
+#define CRASHPAD_UTIL_WIN_HANDLE_TO_INT_H_
+
+#include <windows.h>
+
+namespace crashpad {
+
+//! \brief Converts a `HANDLE` to an `int`.
+//!
+//! `HANDLE` is a `typedef` for `void *`, but `HANDLE` values aren’t necessarily
+//! pointers to anything. Only 32 bits of shareable `HANDLE`s are significant,
+//! even in 64-bit processes on 64-bit operating systems. See <a
+//! href="https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203">Interprocess
+//! Communication Between 32-bit and 64-bit Applications</a>.
+//!
+//! This function safely converts a shareable `HANDLE` to an `int` similarly to
+//! a cast operation. It checks that the operation can be performed safely, and
+//! aborts execution if it cannot.
+//!
+//! \param[in] handle The shareable `HANDLE` to convert.
+//!
+//! \return An equivalent `int`, truncated (if necessary) from \a handle. If
+//! truncation would have resulted in an `int` that could not be converted
+//! back to \a handle, aborts execution.
+//!
+//! \sa IntToHandle()
+int HandleToInt(HANDLE handle);
+
+//! \brief Converts an `int` to an `HANDLE`.
+//!
+//! `HANDLE` is a `typedef` for `void *`, but `HANDLE` values aren’t necessarily
+//! pointers to anything. Only 32 bits of shareable `HANDLE`s are significant,
+//! even in 64-bit processes on 64-bit operating systems. See <a
+//! href="https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203">Interprocess
+//! Communication Between 32-bit and 64-bit Applications</a>.
+//!
+//! This function safely convert an `int` to a shareable `HANDLE` similarly to a
+//! cast operation.
+//!
+//! \param[in] handle_int The `int` to convert. This must have been produced by
+//! HandleToInt(), possibly in a different process.
+//!
+//! \return An equivalent shareable `HANDLE`, sign-extended (if necessary) from
+//! \a handle_int.
+//!
+//! \sa HandleToInt()
+HANDLE IntToHandle(int handle_int);
+
+} // namespace crashpad
+
+#endif // CRASHPAD_UTIL_WIN_HANDLE_TO_INT_H_
diff --git a/util/win/handle_test.cc b/util/win/handle_test.cc
new file mode 100644
index 00000000..60e5037b
--- /dev/null
+++ b/util/win/handle_test.cc
@@ -0,0 +1,53 @@
+// 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/handle.h"
+
+#include <stdint.h>
+
+#include <limits>
+
+#include "gtest/gtest.h"
+
+namespace crashpad {
+namespace test {
+namespace {
+
+TEST(Handle, HandleToInt) {
+ EXPECT_EQ(0, HandleToInt(nullptr));
+ EXPECT_EQ(-1, HandleToInt(INVALID_HANDLE_VALUE));
+ EXPECT_EQ(1, HandleToInt(reinterpret_cast<HANDLE>(1)));
+ EXPECT_EQ(std::numeric_limits<int>::max(),
+ HandleToInt(reinterpret_cast<HANDLE>(
+ static_cast<intptr_t>(std::numeric_limits<int>::max()))));
+ EXPECT_EQ(std::numeric_limits<int>::min(),
+ HandleToInt(reinterpret_cast<HANDLE>(
+ static_cast<intptr_t>(std::numeric_limits<int>::min()))));
+}
+
+TEST(Handle, IntToHandle) {
+ EXPECT_EQ(nullptr, IntToHandle(0));
+ EXPECT_EQ(INVALID_HANDLE_VALUE, IntToHandle(-1));
+ EXPECT_EQ(reinterpret_cast<HANDLE>(1), IntToHandle(1));
+ EXPECT_EQ(reinterpret_cast<HANDLE>(
+ static_cast<intptr_t>(std::numeric_limits<int>::max())),
+ IntToHandle(std::numeric_limits<int>::max()));
+ EXPECT_EQ(reinterpret_cast<HANDLE>(
+ static_cast<intptr_t>(std::numeric_limits<int>::min())),
+ IntToHandle(std::numeric_limits<int>::min()));
+}
+
+} // namespace
+} // namespace test
+} // namespace crashpad
diff --git a/util/win/process_info.cc b/util/win/process_info.cc
index d5316590..50f947ce 100644
--- a/util/win/process_info.cc
+++ b/util/win/process_info.cc
@@ -26,6 +26,7 @@
#include "build/build_config.h"
#include "util/numeric/safe_assignment.h"
#include "util/win/get_function.h"
+#include "util/win/handle.h"
#include "util/win/nt_internals.h"
#include "util/win/ntstatus_logging.h"
#include "util/win/process_structs.h"
@@ -181,10 +182,8 @@ bool GetProcessBasicInformation(HANDLE process,
return false;
}
- // See https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203 on
- // 32 bit being the correct size for HANDLEs for proceses, even on Windows
- // x64. API functions (e.g. OpenProcess) take only a DWORD, so there's no
- // sense in maintaining the top bits.
+ // API functions (e.g. OpenProcess) take only a DWORD, so there's no sense in
+ // maintaining the top bits.
process_info->process_id_ =
static_cast<DWORD>(process_basic_information.UniqueProcessId);
process_info->inherited_from_process_id_ = static_cast<DWORD>(
@@ -390,15 +389,14 @@ std::vector<ProcessInfo::Handle> ProcessInfo::BuildHandleVector(
continue;
Handle result_handle;
- result_handle.handle =
- static_cast<uint32_t>(reinterpret_cast<uintptr_t>(handle.HandleValue));
+ result_handle.handle = HandleToInt(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>(handle.HandleValue),
+ handle.HandleValue,
GetCurrentProcess(),
&dup_handle,
0,
diff --git a/util/win/process_info.h b/util/win/process_info.h
index ca1935bb..49a6f354 100644
--- a/util/win/process_info.h
+++ b/util/win/process_info.h
@@ -58,9 +58,6 @@ class ProcessInfo {
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`,
diff --git a/util/win/process_info_test.cc b/util/win/process_info_test.cc
index 0fc52a58..a6f9dc1c 100644
--- a/util/win/process_info_test.cc
+++ b/util/win/process_info_test.cc
@@ -33,6 +33,7 @@
#include "util/misc/uuid.h"
#include "util/win/command_line.h"
#include "util/win/get_function.h"
+#include "util/win/handle.h"
#include "util/win/scoped_handle.h"
namespace crashpad {
@@ -573,7 +574,8 @@ TEST(ProcessInfo, Handles) {
bool found_key_handle = false;
bool found_mapping_handle = false;
for (auto handle : info.Handles()) {
- if (reinterpret_cast<uint32_t>(file.get()) == handle.handle) {
+ const int handle_int = implicit_cast<int>(handle.handle);
+ if (handle_int == HandleToInt(file.get())) {
EXPECT_FALSE(found_file_handle);
found_file_handle = true;
EXPECT_EQ(L"File", handle.type_name);
@@ -583,7 +585,7 @@ TEST(ProcessInfo, Handles) {
handle.granted_access & STANDARD_RIGHTS_ALL);
EXPECT_EQ(0, handle.attributes);
}
- if (reinterpret_cast<uint32_t>(inherited_file.get()) == handle.handle) {
+ if (handle_int == HandleToInt(inherited_file.get())) {
EXPECT_FALSE(found_inherited_file_handle);
found_inherited_file_handle = true;
EXPECT_EQ(L"File", handle.type_name);
@@ -597,7 +599,7 @@ TEST(ProcessInfo, Handles) {
const int kObjInherit = 0x2;
EXPECT_EQ(kObjInherit, handle.attributes);
}
- if (reinterpret_cast<uint32_t>(scoped_key.get()) == handle.handle) {
+ if (handle_int == HandleToInt(scoped_key.get())) {
EXPECT_FALSE(found_key_handle);
found_key_handle = true;
EXPECT_EQ(L"Key", handle.type_name);
@@ -607,7 +609,7 @@ TEST(ProcessInfo, Handles) {
handle.granted_access & STANDARD_RIGHTS_ALL);
EXPECT_EQ(0, handle.attributes);
}
- if (reinterpret_cast<uint32_t>(mapping.get()) == handle.handle) {
+ if (handle_int == HandleToInt(mapping.get())) {
EXPECT_FALSE(found_mapping_handle);
found_mapping_handle = true;
EXPECT_EQ(L"Section", handle.type_name);
@@ -620,6 +622,7 @@ TEST(ProcessInfo, Handles) {
}
}
EXPECT_TRUE(found_file_handle);
+ EXPECT_TRUE(found_inherited_file_handle);
EXPECT_TRUE(found_key_handle);
EXPECT_TRUE(found_mapping_handle);
}
diff --git a/util/win/registration_protocol_win.h b/util/win/registration_protocol_win.h
index a691931d..f2d9e29c 100644
--- a/util/win/registration_protocol_win.h
+++ b/util/win/registration_protocol_win.h
@@ -93,11 +93,6 @@ struct ClientToServerMessage {
};
//! \brief A client registration response.
-//!
-//! See <a
-//! href="https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203">Interprocess
-//! Communication Between 32-bit and 64-bit Applications</a> for details on
-//! communicating handle values between processes of varying bitness.
struct RegistrationResponse {
//! \brief An event `HANDLE`, valid in the client process, that should be
//! signaled to request a crash report. 64-bit clients should convert the
From 827e045279e15429ac5753e3e97ad2c0d53dc84a Mon Sep 17 00:00:00 2001
From: Mark Mentovai <mark@chromium.org>
Date: Thu, 5 Nov 2015 14:06:18 -0500
Subject: [PATCH 03/12] win: Fix build after 2eeaa3ac549a
Review URL: https://codereview.chromium.org/1424713008 .
---
handler/main.cc | 1 +
1 file changed, 1 insertion(+)
diff --git a/handler/main.cc b/handler/main.cc
index fcc3f71c..0ec6e7f2 100644
--- a/handler/main.cc
+++ b/handler/main.cc
@@ -190,6 +190,7 @@ int HandlerMain(int argc, char* argv[]) {
INVALID_HANDLE_VALUE) {
ToolSupport::UsageHint(me, "--handshake-handle requires a HANDLE");
return EXIT_FAILURE;
+ }
break;
}
case kOptionPipeName: {
From 82ffeaa0f0866e04e9e3b4f36fe27143cc2f233e Mon Sep 17 00:00:00 2001
From: Mark Mentovai <mark@chromium.org>
Date: Thu, 5 Nov 2015 15:08:28 -0500
Subject: [PATCH 04/12] win: crashpad_util_test ProcessInfo.Handles doesn't
work with CONOUT$
In 2eeaa3ac549a, I added a check to make sure that the expected CONOUT$
handle was found. Its omission seemed to be unintentional. The tests
passed for me on Windows 10, but failed on the bots. I can reproduce
the failures locally on Windows 7.
Doing the inheritance test with a file other than CONOUT$ fixes the
immediate problem, but we should find out why this CONOUT$ handle isn't
showing up in the handles list on Windows 7, fix it, and add back a
test.
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1428753009 .
---
util/win/process_info_test.cc | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/util/win/process_info_test.cc b/util/win/process_info_test.cc
index a6f9dc1c..7bf0f4ef 100644
--- a/util/win/process_info_test.cc
+++ b/util/win/process_info_test.cc
@@ -538,14 +538,16 @@ TEST(ProcessInfo, Handles) {
ASSERT_TRUE(file.is_valid());
SECURITY_ATTRIBUTES security_attributes = {0};
+ security_attributes.nLength = sizeof(security_attributes);
security_attributes.bInheritHandle = true;
- ScopedFileHandle inherited_file(CreateFile(L"CONOUT$",
- GENERIC_WRITE,
- 0,
- &security_attributes,
- OPEN_EXISTING,
- FILE_ATTRIBUTE_NORMAL,
- nullptr));
+ ScopedFileHandle inherited_file(CreateFile(
+ temp_dir.path().Append(FILE_PATH_LITERAL("inheritable")).value().c_str(),
+ GENERIC_WRITE,
+ 0,
+ &security_attributes,
+ CREATE_NEW,
+ FILE_ATTRIBUTE_NORMAL,
+ nullptr));
ASSERT_TRUE(inherited_file.is_valid());
HKEY key;
From 9a9076656fc499044a8ed2c570b7cd523afd380d Mon Sep 17 00:00:00 2001
From: Mark Mentovai <mark@chromium.org>
Date: Fri, 6 Nov 2015 10:15:58 -0500
Subject: [PATCH 05/12] win: Fix ClockMonotonicNanoseconds()
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1419533014 .
---
util/misc/clock_win.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/util/misc/clock_win.cc b/util/misc/clock_win.cc
index 731e98d4..7231205d 100644
--- a/util/misc/clock_win.cc
+++ b/util/misc/clock_win.cc
@@ -39,7 +39,7 @@ uint64_t ClockMonotonicNanoseconds() {
QueryPerformanceCounter(&time);
int64_t frequency = QpcFrequency();
int64_t whole_seconds = time.QuadPart / frequency;
- int64_t leftover_ticks = time.QuadPart / (whole_seconds * frequency);
+ int64_t leftover_ticks = time.QuadPart % frequency;
const int64_t kNanosecondsPerSecond = static_cast<const int64_t>(1E9);
return (whole_seconds * kNanosecondsPerSecond) +
((leftover_ticks * kNanosecondsPerSecond) / frequency);
From e75e8c800f3f2f9ca1b131b2a0599a2fb5ca80f9 Mon Sep 17 00:00:00 2001
From: Scott Graham <scottmg@chromium.org>
Date: Fri, 6 Nov 2015 10:43:39 -0800
Subject: [PATCH 06/12] win: Lower integrity level of connection pipe
This is necessary to be able to connect to crashpad_handler from a
Chrome renderer.
R=jschuh@chromium.org, mark@chromium.org
BUG=chromium:546288
Review URL: https://codereview.chromium.org/1405093013 .
---
test/win/win_child_process.cc | 12 +-----
util/util.gyp | 3 ++
util/win/command_line_test.cc | 12 +-----
util/win/exception_handler_server.cc | 57 ++++++++++++++++++++++------
util/win/scoped_local_alloc.cc | 26 +++++++++++++
util/win/scoped_local_alloc.h | 33 ++++++++++++++++
6 files changed, 110 insertions(+), 33 deletions(-)
create mode 100644 util/win/scoped_local_alloc.cc
create mode 100644 util/win/scoped_local_alloc.h
diff --git a/test/win/win_child_process.cc b/test/win/win_child_process.cc
index 7d179337..b6cbbbd2 100644
--- a/test/win/win_child_process.cc
+++ b/test/win/win_child_process.cc
@@ -26,6 +26,7 @@
#include "util/stdlib/string_number_conversion.h"
#include "util/string/split_string.h"
#include "util/win/handle.h"
+#include "util/win/scoped_local_alloc.h"
#include "test/paths.h"
namespace crashpad {
@@ -34,20 +35,11 @@ namespace test {
namespace {
const char kIsMultiprocessChild[] = "--is-multiprocess-child";
-struct LocalFreeTraits {
- static HLOCAL InvalidValue() { return nullptr; }
- static void Free(HLOCAL mem) {
- if (LocalFree(mem) != nullptr)
- PLOG(ERROR) << "LocalFree";
- }
-};
-
-using ScopedLocalFree = base::ScopedGeneric<HLOCAL, LocalFreeTraits>;
bool GetSwitch(const char* switch_name, std::string* value) {
int num_args;
wchar_t** args = CommandLineToArgvW(GetCommandLine(), &num_args);
- ScopedLocalFree scoped_args(args); // Take ownership.
+ ScopedLocalAlloc scoped_args(args); // Take ownership.
if (!args) {
PLOG(FATAL) << "CommandLineToArgvW";
return false;
diff --git a/util/util.gyp b/util/util.gyp
index 685032c7..5f4064a1 100644
--- a/util/util.gyp
+++ b/util/util.gyp
@@ -175,6 +175,8 @@
'win/registration_protocol_win.h',
'win/scoped_handle.cc',
'win/scoped_handle.h',
+ 'win/scoped_local_alloc.cc',
+ 'win/scoped_local_alloc.h',
'win/scoped_process_suspend.cc',
'win/scoped_process_suspend.h',
'win/time.cc',
@@ -246,6 +248,7 @@
['OS=="win"', {
'link_settings': {
'libraries': [
+ '-ladvapi32.lib',
'-lrpcrt4.lib',
'-lwinhttp.lib',
],
diff --git a/util/win/command_line_test.cc b/util/win/command_line_test.cc
index f3955697..d3179275 100644
--- a/util/win/command_line_test.cc
+++ b/util/win/command_line_test.cc
@@ -22,22 +22,12 @@
#include "base/scoped_generic.h"
#include "gtest/gtest.h"
#include "test/errors.h"
+#include "util/win/scoped_local_alloc.h"
namespace crashpad {
namespace test {
namespace {
-struct LocalAllocTraits {
- static HLOCAL InvalidValue() {
- return nullptr;
- }
-
- static void Free(HLOCAL memory) {
- PLOG_IF(ERROR, LocalFree(memory) != nullptr) << "LocalFree";
- }
-};
-using ScopedLocalAlloc = base::ScopedGeneric<HLOCAL, LocalAllocTraits>;
-
// Calls AppendCommandLineArgument() for every argument in argv, then calls
// CommandLineToArgvW() to decode the string into a vector again, and compares
// the input and output.
diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc
index ce5687d7..3a41a1de 100644
--- a/util/win/exception_handler_server.cc
+++ b/util/win/exception_handler_server.cc
@@ -14,6 +14,7 @@
#include "util/win/exception_handler_server.h"
+#include <sddl.h>
#include <string.h>
#include "base/logging.h"
@@ -30,6 +31,7 @@
#include "util/win/get_function.h"
#include "util/win/handle.h"
#include "util/win/registration_protocol_win.h"
+#include "util/win/scoped_local_alloc.h"
#include "util/win/xp_compat.h"
namespace crashpad {
@@ -44,19 +46,50 @@ const size_t kPipeInstances = 2;
//
// If first_instance is true, the named pipe instance will be created with
// FILE_FLAG_FIRST_PIPE_INSTANCE. This ensures that the the pipe name is not
-// already in use when created.
+// already in use when created. The first instance will be created with an
+// untrusted integrity SACL so instances of this pipe can be connected to by
+// processes of any integrity level.
HANDLE CreateNamedPipeInstance(const std::wstring& pipe_name,
bool first_instance) {
- return CreateNamedPipe(pipe_name.c_str(),
- PIPE_ACCESS_DUPLEX |
- (first_instance ? FILE_FLAG_FIRST_PIPE_INSTANCE
- : 0),
- PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT,
- kPipeInstances,
- 512,
- 512,
- 0,
- nullptr);
+ SECURITY_ATTRIBUTES security_attributes;
+ SECURITY_ATTRIBUTES* security_attributes_pointer = nullptr;
+ ScopedLocalAlloc scoped_sec_desc;
+
+ if (first_instance) {
+ // Pre-Vista does not have integrity levels.
+ const DWORD version = GetVersion();
+ const DWORD major_version = LOBYTE(LOWORD(version));
+ const bool is_vista_or_later = major_version >= 6;
+ if (is_vista_or_later) {
+ // Mandatory Label, no ACE flags, no ObjectType, integrity level
+ // untrusted.
+ const wchar_t kSddl[] = L"S:(ML;;;;;S-1-16-0)";
+
+ PSECURITY_DESCRIPTOR sec_desc;
+ PCHECK(ConvertStringSecurityDescriptorToSecurityDescriptor(
+ kSddl, SDDL_REVISION_1, &sec_desc, nullptr))
+ << "ConvertStringSecurityDescriptorToSecurityDescriptor";
+
+ // Take ownership of the allocated SECURITY_DESCRIPTOR.
+ scoped_sec_desc.reset(sec_desc);
+
+ memset(&security_attributes, 0, sizeof(security_attributes));
+ security_attributes.nLength = sizeof(SECURITY_ATTRIBUTES);
+ security_attributes.lpSecurityDescriptor = sec_desc;
+ security_attributes.bInheritHandle = FALSE;
+ security_attributes_pointer = &security_attributes;
+ }
+ }
+
+ return CreateNamedPipe(
+ pipe_name.c_str(),
+ PIPE_ACCESS_DUPLEX | (first_instance ? FILE_FLAG_FIRST_PIPE_INSTANCE : 0),
+ PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT,
+ kPipeInstances,
+ 512,
+ 512,
+ 0,
+ security_attributes_pointer);
}
decltype(GetNamedPipeClientProcessId)* GetNamedPipeClientProcessIdFunction() {
@@ -320,7 +353,7 @@ void ExceptionHandlerServer::Run(Delegate* delegate) {
if (first_pipe_instance_.is_valid()) {
pipe = first_pipe_instance_.release();
} else {
- pipe = CreateNamedPipeInstance(pipe_name_, false);
+ pipe = CreateNamedPipeInstance(pipe_name_, i == 0);
PCHECK(pipe != INVALID_HANDLE_VALUE) << "CreateNamedPipe";
}
diff --git a/util/win/scoped_local_alloc.cc b/util/win/scoped_local_alloc.cc
new file mode 100644
index 00000000..e9388ee1
--- /dev/null
+++ b/util/win/scoped_local_alloc.cc
@@ -0,0 +1,26 @@
+// 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/scoped_local_alloc.h"
+
+#include "base/logging.h"
+
+namespace crashpad {
+
+// static
+void LocalAllocTraits::Free(HLOCAL memory) {
+ PLOG_IF(ERROR, LocalFree(memory) != nullptr) << "LocalFree";
+}
+
+} // namespace crashpad
diff --git a/util/win/scoped_local_alloc.h b/util/win/scoped_local_alloc.h
new file mode 100644
index 00000000..26a59ed8
--- /dev/null
+++ b/util/win/scoped_local_alloc.h
@@ -0,0 +1,33 @@
+// 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_SCOPED_LOCAL_ALLOC_H_
+#define CRASHPAD_UTIL_WIN_SCOPED_LOCAL_ALLOC_H_
+
+#include <windows.h>
+
+#include "base/scoped_generic.h"
+
+namespace crashpad {
+
+struct LocalAllocTraits {
+ static HLOCAL InvalidValue() { return nullptr; }
+ static void Free(HLOCAL mem);
+};
+
+using ScopedLocalAlloc = base::ScopedGeneric<HLOCAL, LocalAllocTraits>;
+
+} // namespace crashpad
+
+#endif // CRASHPAD_UTIL_WIN_SCOPED_LOCAL_ALLOC_H_
From 20869d94682cb72b3e4544b47cc27059aa0af3ad Mon Sep 17 00:00:00 2001
From: Scott Graham <scottmg@chromium.org>
Date: Fri, 6 Nov 2015 10:52:09 -0800
Subject: [PATCH 07/12] Break crashpad_handler into lib and exe for Windows
I've heard/lived enough horror stories about AV, outbound-blocking
firewalls, etc. on Windows, that I think the best approach is to have
chrome.exe embed the majority of crashpad_handler and jump to it as
early as possible when running in that mode.
So, move most of crashpad_handler into a static_library with just main()
in the executable target.
R=mark@chromium.org
BUG=chromium:546288, crashpad:27
Review URL: https://codereview.chromium.org/1416873016 .
---
handler/handler.gyp | 24 ++-
handler/handler_main.cc | 331 ++++++++++++++++++++++++++++++++++++++++
handler/handler_main.h | 28 ++++
handler/main.cc | 313 +------------------------------------
4 files changed, 382 insertions(+), 314 deletions(-)
create mode 100644 handler/handler_main.cc
create mode 100644 handler/handler_main.h
diff --git a/handler/handler.gyp b/handler/handler.gyp
index 20df3d73..e7f54c82 100644
--- a/handler/handler.gyp
+++ b/handler/handler.gyp
@@ -19,8 +19,10 @@
],
'targets': [
{
- 'target_name': 'crashpad_handler',
- 'type': 'executable',
+ # This target exists so that the crashpad_handler can be embedded into
+ # another binary.
+ 'target_name': 'crashpad_handler_lib',
+ 'type': 'static_library',
'dependencies': [
'../client/client.gyp:crashpad_client',
'../compat/compat.gyp:crashpad_compat',
@@ -40,10 +42,26 @@
'mac/crash_report_exception_handler.h',
'mac/exception_handler_server.cc',
'mac/exception_handler_server.h',
- 'main.cc',
+ 'handler_main.cc',
+ 'handler_main.h',
'win/crash_report_exception_handler.cc',
'win/crash_report_exception_handler.h',
],
+ },
+ {
+ 'target_name': 'crashpad_handler',
+ 'type': 'executable',
+ 'dependencies': [
+ '../third_party/mini_chromium/mini_chromium.gyp:base',
+ '../tools/tools.gyp:crashpad_tool_support',
+ 'crashpad_handler_lib',
+ ],
+ 'include_dirs': [
+ '..',
+ ],
+ 'sources': [
+ 'main.cc',
+ ],
'conditions': [
['OS=="mac"', {
diff --git a/handler/handler_main.cc b/handler/handler_main.cc
new file mode 100644
index 00000000..11f14ef1
--- /dev/null
+++ b/handler/handler_main.cc
@@ -0,0 +1,331 @@
+// Copyright 2014 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 "handler/handler_main.h"
+
+#include <getopt.h>
+#include <stdlib.h>
+
+#include <map>
+#include <string>
+
+#include "base/files/file_path.h"
+#include "base/files/scoped_file.h"
+#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/strings/utf_string_conversions.h"
+#include "build/build_config.h"
+#include "client/crash_report_database.h"
+#include "client/crashpad_client.h"
+#include "tools/tool_support.h"
+#include "handler/crash_report_upload_thread.h"
+#include "util/file/file_io.h"
+#include "util/stdlib/map_insert.h"
+#include "util/stdlib/string_number_conversion.h"
+#include "util/string/split_string.h"
+#include "util/synchronization/semaphore.h"
+
+#if defined(OS_MACOSX)
+#include <libgen.h>
+#include "base/mac/scoped_mach_port.h"
+#include "handler/mac/crash_report_exception_handler.h"
+#include "handler/mac/exception_handler_server.h"
+#include "util/mach/child_port_handshake.h"
+#include "util/mach/mach_extensions.h"
+#include "util/posix/close_stdio.h"
+#elif defined(OS_WIN)
+#include <windows.h>
+#include "handler/win/crash_report_exception_handler.h"
+#include "util/win/exception_handler_server.h"
+#include "util/win/handle.h"
+#endif // OS_MACOSX
+
+namespace crashpad {
+
+namespace {
+
+void Usage(const base::FilePath& me) {
+ fprintf(stderr,
+"Usage: %" PRFilePath " [OPTION]...\n"
+"Crashpad's exception handler server.\n"
+"\n"
+" --annotation=KEY=VALUE set a process annotation in each crash report\n"
+" --database=PATH store the crash report database at PATH\n"
+#if defined(OS_MACOSX)
+" --handshake-fd=FD establish communication with the client over FD\n"
+" --mach-service=SERVICE register SERVICE with the bootstrap server\n"
+" --reset-own-crash-exception-port-to-system-default\n"
+" reset the server's exception handler to default\n"
+#elif defined(OS_WIN)
+" --handshake-handle=HANDLE\n"
+" create a new pipe and send its name via HANDLE\n"
+" --pipe-name=PIPE communicate with the client over PIPE\n"
+#endif // OS_MACOSX
+" --url=URL send crash reports to this Breakpad server URL,\n"
+" only if uploads are enabled for the database\n"
+" --help display this help and exit\n"
+" --version output version information and exit\n",
+ me.value().c_str());
+ ToolSupport::UsageTail(me);
+}
+
+} // namespace
+
+int HandlerMain(int argc, char* argv[]) {
+ const base::FilePath argv0(
+ ToolSupport::CommandLineArgumentToFilePathStringType(argv[0]));
+ const base::FilePath me(argv0.BaseName());
+
+ enum OptionFlags {
+ // Long options without short equivalents.
+ kOptionLastChar = 255,
+ kOptionAnnotation,
+ kOptionDatabase,
+#if defined(OS_MACOSX)
+ kOptionHandshakeFD,
+ kOptionMachService,
+ kOptionResetOwnCrashExceptionPortToSystemDefault,
+#elif defined(OS_WIN)
+ kOptionHandshakeHandle,
+ kOptionPipeName,
+#endif // OS_MACOSX
+ kOptionURL,
+
+ // Standard options.
+ kOptionHelp = -2,
+ kOptionVersion = -3,
+ };
+
+ struct {
+ std::map<std::string, std::string> annotations;
+ std::string url;
+ const char* database;
+#if defined(OS_MACOSX)
+ int handshake_fd;
+ std::string mach_service;
+ bool reset_own_crash_exception_port_to_system_default;
+#elif defined(OS_WIN)
+ HANDLE handshake_handle;
+ std::string pipe_name;
+#endif // OS_MACOSX
+ } options = {};
+#if defined(OS_MACOSX)
+ options.handshake_fd = -1;
+#elif defined(OS_WIN)
+ options.handshake_handle = INVALID_HANDLE_VALUE;
+#endif
+
+ const option long_options[] = {
+ {"annotation", required_argument, nullptr, kOptionAnnotation},
+ {"database", required_argument, nullptr, kOptionDatabase},
+#if defined(OS_MACOSX)
+ {"handshake-fd", required_argument, nullptr, kOptionHandshakeFD},
+ {"mach-service", required_argument, nullptr, kOptionMachService},
+ {"reset-own-crash-exception-port-to-system-default",
+ no_argument,
+ nullptr,
+ kOptionResetOwnCrashExceptionPortToSystemDefault},
+#elif defined(OS_WIN)
+ {"handshake-handle", required_argument, nullptr, kOptionHandshakeHandle},
+ {"pipe-name", required_argument, nullptr, kOptionPipeName},
+#endif // OS_MACOSX
+ {"url", required_argument, nullptr, kOptionURL},
+ {"help", no_argument, nullptr, kOptionHelp},
+ {"version", no_argument, nullptr, kOptionVersion},
+ {nullptr, 0, nullptr, 0},
+ };
+
+ int opt;
+ while ((opt = getopt_long(argc, argv, "", long_options, nullptr)) != -1) {
+ switch (opt) {
+ case kOptionAnnotation: {
+ std::string key;
+ std::string value;
+ if (!SplitString(optarg, '=', &key, &value)) {
+ ToolSupport::UsageHint(me, "--annotation requires KEY=VALUE");
+ return EXIT_FAILURE;
+ }
+ std::string old_value;
+ if (!MapInsertOrReplace(&options.annotations, key, value, &old_value)) {
+ LOG(WARNING) << "duplicate key " << key << ", discarding value "
+ << old_value;
+ }
+ break;
+ }
+ case kOptionDatabase: {
+ options.database = optarg;
+ break;
+ }
+#if defined(OS_MACOSX)
+ case kOptionHandshakeFD: {
+ if (!StringToNumber(optarg, &options.handshake_fd) ||
+ options.handshake_fd < 0) {
+ ToolSupport::UsageHint(me,
+ "--handshake-fd requires a file descriptor");
+ return EXIT_FAILURE;
+ }
+ break;
+ }
+ case kOptionMachService: {
+ options.mach_service = optarg;
+ break;
+ }
+ case kOptionResetOwnCrashExceptionPortToSystemDefault: {
+ options.reset_own_crash_exception_port_to_system_default = true;
+ break;
+ }
+#elif defined(OS_WIN)
+ case kOptionHandshakeHandle: {
+ // Use unsigned int, because the handle was presented by the client in
+ // 0x%x format.
+ unsigned int handle_uint;
+ if (!StringToNumber(optarg, &handle_uint) ||
+ (options.handshake_handle = IntToHandle(handle_uint)) ==
+ INVALID_HANDLE_VALUE) {
+ ToolSupport::UsageHint(me, "--handshake-handle requires a HANDLE");
+ return EXIT_FAILURE;
+ }
+ break;
+ }
+ case kOptionPipeName: {
+ options.pipe_name = optarg;
+ break;
+ }
+#endif // OS_MACOSX
+ case kOptionURL: {
+ options.url = optarg;
+ break;
+ }
+ case kOptionHelp: {
+ Usage(me);
+ return EXIT_SUCCESS;
+ }
+ case kOptionVersion: {
+ ToolSupport::Version(me);
+ return EXIT_SUCCESS;
+ }
+ default: {
+ ToolSupport::UsageHint(me, nullptr);
+ return EXIT_FAILURE;
+ }
+ }
+ }
+ argc -= optind;
+ argv += optind;
+
+#if defined(OS_MACOSX)
+ if (options.handshake_fd < 0 && options.mach_service.empty()) {
+ ToolSupport::UsageHint(me, "--handshake-fd or --mach-service is required");
+ return EXIT_FAILURE;
+ }
+ if (options.handshake_fd >= 0 && !options.mach_service.empty()) {
+ ToolSupport::UsageHint(
+ me, "--handshake-fd and --mach-service are incompatible");
+ return EXIT_FAILURE;
+ }
+#elif defined(OS_WIN)
+ if (options.handshake_handle == INVALID_HANDLE_VALUE &&
+ options.pipe_name.empty()) {
+ ToolSupport::UsageHint(me, "--handshake-handle or --pipe-name is required");
+ return EXIT_FAILURE;
+ }
+ if (options.handshake_handle != INVALID_HANDLE_VALUE &&
+ !options.pipe_name.empty()) {
+ ToolSupport::UsageHint(
+ me, "--handshake-handle and --pipe-name are incompatible");
+ return EXIT_FAILURE;
+ }
+#endif // OS_MACOSX
+
+ if (!options.database) {
+ ToolSupport::UsageHint(me, "--database is required");
+ return EXIT_FAILURE;
+ }
+
+ if (argc) {
+ ToolSupport::UsageHint(me, nullptr);
+ return EXIT_FAILURE;
+ }
+
+#if defined(OS_MACOSX)
+ if (options.mach_service.empty()) {
+ // Don’t do this when being run by launchd. See launchd.plist(5).
+ CloseStdinAndStdout();
+ }
+
+ if (options.reset_own_crash_exception_port_to_system_default) {
+ CrashpadClient::UseSystemDefaultHandler();
+ }
+
+ base::mac::ScopedMachReceiveRight receive_right;
+
+ if (options.handshake_fd >= 0) {
+ receive_right.reset(
+ ChildPortHandshake::RunServerForFD(
+ base::ScopedFD(options.handshake_fd),
+ ChildPortHandshake::PortRightType::kReceiveRight));
+ } else if (!options.mach_service.empty()) {
+ receive_right = BootstrapCheckIn(options.mach_service);
+ }
+
+ if (!receive_right.is_valid()) {
+ return EXIT_FAILURE;
+ }
+
+ ExceptionHandlerServer exception_handler_server(
+ receive_right.Pass(), !options.mach_service.empty());
+#elif defined(OS_WIN)
+ ExceptionHandlerServer exception_handler_server(!options.pipe_name.empty());
+
+ std::string pipe_name;
+ if (!options.pipe_name.empty()) {
+ exception_handler_server.SetPipeName(base::UTF8ToUTF16(options.pipe_name));
+ } else if (options.handshake_handle != INVALID_HANDLE_VALUE) {
+ std::wstring pipe_name = exception_handler_server.CreatePipe();
+
+ uint32_t pipe_name_length = static_cast<uint32_t>(pipe_name.size());
+ if (!LoggingWriteFile(options.handshake_handle,
+ &pipe_name_length,
+ sizeof(pipe_name_length))) {
+ return EXIT_FAILURE;
+ }
+ if (!LoggingWriteFile(options.handshake_handle,
+ pipe_name.c_str(),
+ pipe_name.size() * sizeof(pipe_name[0]))) {
+ return EXIT_FAILURE;
+ }
+ }
+#endif // OS_MACOSX
+
+ scoped_ptr<CrashReportDatabase> database(CrashReportDatabase::Initialize(
+ base::FilePath(ToolSupport::CommandLineArgumentToFilePathStringType(
+ options.database))));
+ if (!database) {
+ return EXIT_FAILURE;
+ }
+
+ CrashReportUploadThread upload_thread(database.get(), options.url);
+ upload_thread.Start();
+
+ CrashReportExceptionHandler exception_handler(
+ database.get(), &upload_thread, &options.annotations);
+
+ exception_handler_server.Run(&exception_handler);
+
+ upload_thread.Stop();
+
+ return EXIT_SUCCESS;
+}
+
+} // namespace crashpad
diff --git a/handler/handler_main.h b/handler/handler_main.h
new file mode 100644
index 00000000..5b5568ec
--- /dev/null
+++ b/handler/handler_main.h
@@ -0,0 +1,28 @@
+// 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_HANDLER_HANDLER_MAIN_H_
+#define CRASHPAD_HANDLER_HANDLER_MAIN_H_
+
+namespace crashpad {
+
+//! \brief The `main()` of the `crashpad_handler` binary.
+//!
+//! This is exposed so that `crashpad_handler` can be embedded into another
+//! binary, but called and used as if it were a standalone executable.
+int HandlerMain(int argc, char* argv[]);
+
+} // namespace crashpad
+
+#endif // CRASHPAD_HANDLER_HANDLER_MAIN_H_
diff --git a/handler/main.cc b/handler/main.cc
index 0ec6e7f2..03ee87fb 100644
--- a/handler/main.cc
+++ b/handler/main.cc
@@ -1,4 +1,4 @@
-// Copyright 2014 The Crashpad Authors. All rights reserved.
+// 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.
@@ -12,319 +12,10 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-#include <getopt.h>
-#include <stdlib.h>
+#include "handler/main.h"
-#include <map>
-#include <string>
-
-#include "base/files/file_path.h"
-#include "base/files/scoped_file.h"
-#include "base/logging.h"
-#include "base/memory/scoped_ptr.h"
-#include "base/strings/utf_string_conversions.h"
#include "build/build_config.h"
-#include "client/crash_report_database.h"
-#include "client/crashpad_client.h"
#include "tools/tool_support.h"
-#include "handler/crash_report_upload_thread.h"
-#include "util/file/file_io.h"
-#include "util/stdlib/map_insert.h"
-#include "util/stdlib/string_number_conversion.h"
-#include "util/string/split_string.h"
-#include "util/synchronization/semaphore.h"
-
-#if defined(OS_MACOSX)
-#include <libgen.h>
-#include "base/mac/scoped_mach_port.h"
-#include "handler/mac/crash_report_exception_handler.h"
-#include "handler/mac/exception_handler_server.h"
-#include "util/mach/child_port_handshake.h"
-#include "util/mach/mach_extensions.h"
-#include "util/posix/close_stdio.h"
-#elif defined(OS_WIN)
-#include <windows.h>
-#include "handler/win/crash_report_exception_handler.h"
-#include "util/win/exception_handler_server.h"
-#include "util/win/handle.h"
-#endif // OS_MACOSX
-
-namespace crashpad {
-namespace {
-
-void Usage(const base::FilePath& me) {
- fprintf(stderr,
-"Usage: %" PRFilePath " [OPTION]...\n"
-"Crashpad's exception handler server.\n"
-"\n"
-" --annotation=KEY=VALUE set a process annotation in each crash report\n"
-" --database=PATH store the crash report database at PATH\n"
-#if defined(OS_MACOSX)
-" --handshake-fd=FD establish communication with the client over FD\n"
-" --mach-service=SERVICE register SERVICE with the bootstrap server\n"
-" --reset-own-crash-exception-port-to-system-default\n"
-" reset the server's exception handler to default\n"
-#elif defined(OS_WIN)
-" --handshake-handle=HANDLE\n"
-" create a new pipe and send its name via HANDLE\n"
-" --pipe-name=PIPE communicate with the client over PIPE\n"
-#endif // OS_MACOSX
-" --url=URL send crash reports to this Breakpad server URL,\n"
-" only if uploads are enabled for the database\n"
-" --help display this help and exit\n"
-" --version output version information and exit\n",
- me.value().c_str());
- ToolSupport::UsageTail(me);
-}
-
-int HandlerMain(int argc, char* argv[]) {
- const base::FilePath argv0(
- ToolSupport::CommandLineArgumentToFilePathStringType(argv[0]));
- const base::FilePath me(argv0.BaseName());
-
- enum OptionFlags {
- // Long options without short equivalents.
- kOptionLastChar = 255,
- kOptionAnnotation,
- kOptionDatabase,
-#if defined(OS_MACOSX)
- kOptionHandshakeFD,
- kOptionMachService,
- kOptionResetOwnCrashExceptionPortToSystemDefault,
-#elif defined(OS_WIN)
- kOptionHandshakeHandle,
- kOptionPipeName,
-#endif // OS_MACOSX
- kOptionURL,
-
- // Standard options.
- kOptionHelp = -2,
- kOptionVersion = -3,
- };
-
- struct {
- std::map<std::string, std::string> annotations;
- std::string url;
- const char* database;
-#if defined(OS_MACOSX)
- int handshake_fd;
- std::string mach_service;
- bool reset_own_crash_exception_port_to_system_default;
-#elif defined(OS_WIN)
- HANDLE handshake_handle;
- std::string pipe_name;
-#endif // OS_MACOSX
- } options = {};
-#if defined(OS_MACOSX)
- options.handshake_fd = -1;
-#elif defined(OS_WIN)
- options.handshake_handle = INVALID_HANDLE_VALUE;
-#endif
-
- const option long_options[] = {
- {"annotation", required_argument, nullptr, kOptionAnnotation},
- {"database", required_argument, nullptr, kOptionDatabase},
-#if defined(OS_MACOSX)
- {"handshake-fd", required_argument, nullptr, kOptionHandshakeFD},
- {"mach-service", required_argument, nullptr, kOptionMachService},
- {"reset-own-crash-exception-port-to-system-default",
- no_argument,
- nullptr,
- kOptionResetOwnCrashExceptionPortToSystemDefault},
-#elif defined(OS_WIN)
- {"handshake-handle", required_argument, nullptr, kOptionHandshakeHandle},
- {"pipe-name", required_argument, nullptr, kOptionPipeName},
-#endif // OS_MACOSX
- {"url", required_argument, nullptr, kOptionURL},
- {"help", no_argument, nullptr, kOptionHelp},
- {"version", no_argument, nullptr, kOptionVersion},
- {nullptr, 0, nullptr, 0},
- };
-
- int opt;
- while ((opt = getopt_long(argc, argv, "", long_options, nullptr)) != -1) {
- switch (opt) {
- case kOptionAnnotation: {
- std::string key;
- std::string value;
- if (!SplitString(optarg, '=', &key, &value)) {
- ToolSupport::UsageHint(me, "--annotation requires KEY=VALUE");
- return EXIT_FAILURE;
- }
- std::string old_value;
- if (!MapInsertOrReplace(&options.annotations, key, value, &old_value)) {
- LOG(WARNING) << "duplicate key " << key << ", discarding value "
- << old_value;
- }
- break;
- }
- case kOptionDatabase: {
- options.database = optarg;
- break;
- }
-#if defined(OS_MACOSX)
- case kOptionHandshakeFD: {
- if (!StringToNumber(optarg, &options.handshake_fd) ||
- options.handshake_fd < 0) {
- ToolSupport::UsageHint(me,
- "--handshake-fd requires a file descriptor");
- return EXIT_FAILURE;
- }
- break;
- }
- case kOptionMachService: {
- options.mach_service = optarg;
- break;
- }
- case kOptionResetOwnCrashExceptionPortToSystemDefault: {
- options.reset_own_crash_exception_port_to_system_default = true;
- break;
- }
-#elif defined(OS_WIN)
- case kOptionHandshakeHandle: {
- // Use unsigned int, because the handle was presented by the client in
- // 0x%x format.
- unsigned int handle_uint;
- if (!StringToNumber(optarg, &handle_uint) ||
- (options.handshake_handle = IntToHandle(handle_uint)) ==
- INVALID_HANDLE_VALUE) {
- ToolSupport::UsageHint(me, "--handshake-handle requires a HANDLE");
- return EXIT_FAILURE;
- }
- break;
- }
- case kOptionPipeName: {
- options.pipe_name = optarg;
- break;
- }
-#endif // OS_MACOSX
- case kOptionURL: {
- options.url = optarg;
- break;
- }
- case kOptionHelp: {
- Usage(me);
- return EXIT_SUCCESS;
- }
- case kOptionVersion: {
- ToolSupport::Version(me);
- return EXIT_SUCCESS;
- }
- default: {
- ToolSupport::UsageHint(me, nullptr);
- return EXIT_FAILURE;
- }
- }
- }
- argc -= optind;
- argv += optind;
-
-#if defined(OS_MACOSX)
- if (options.handshake_fd < 0 && options.mach_service.empty()) {
- ToolSupport::UsageHint(me, "--handshake-fd or --mach-service is required");
- return EXIT_FAILURE;
- }
- if (options.handshake_fd >= 0 && !options.mach_service.empty()) {
- ToolSupport::UsageHint(
- me, "--handshake-fd and --mach-service are incompatible");
- return EXIT_FAILURE;
- }
-#elif defined(OS_WIN)
- if (options.handshake_handle == INVALID_HANDLE_VALUE &&
- options.pipe_name.empty()) {
- ToolSupport::UsageHint(me, "--handshake-handle or --pipe-name is required");
- return EXIT_FAILURE;
- }
- if (options.handshake_handle != INVALID_HANDLE_VALUE &&
- !options.pipe_name.empty()) {
- ToolSupport::UsageHint(
- me, "--handshake-handle and --pipe-name are incompatible");
- return EXIT_FAILURE;
- }
-#endif // OS_MACOSX
-
- if (!options.database) {
- ToolSupport::UsageHint(me, "--database is required");
- return EXIT_FAILURE;
- }
-
- if (argc) {
- ToolSupport::UsageHint(me, nullptr);
- return EXIT_FAILURE;
- }
-
-#if defined(OS_MACOSX)
- if (options.mach_service.empty()) {
- // Don’t do this when being run by launchd. See launchd.plist(5).
- CloseStdinAndStdout();
- }
-
- if (options.reset_own_crash_exception_port_to_system_default) {
- CrashpadClient::UseSystemDefaultHandler();
- }
-
- base::mac::ScopedMachReceiveRight receive_right;
-
- if (options.handshake_fd >= 0) {
- receive_right.reset(
- ChildPortHandshake::RunServerForFD(
- base::ScopedFD(options.handshake_fd),
- ChildPortHandshake::PortRightType::kReceiveRight));
- } else if (!options.mach_service.empty()) {
- receive_right = BootstrapCheckIn(options.mach_service);
- }
-
- if (!receive_right.is_valid()) {
- return EXIT_FAILURE;
- }
-
- ExceptionHandlerServer exception_handler_server(
- receive_right.Pass(), !options.mach_service.empty());
-#elif defined(OS_WIN)
- ExceptionHandlerServer exception_handler_server(!options.pipe_name.empty());
-
- std::string pipe_name;
- if (!options.pipe_name.empty()) {
- exception_handler_server.SetPipeName(base::UTF8ToUTF16(options.pipe_name));
- } else if (options.handshake_handle != INVALID_HANDLE_VALUE) {
- std::wstring pipe_name = exception_handler_server.CreatePipe();
-
- uint32_t pipe_name_length = static_cast<uint32_t>(pipe_name.size());
- if (!LoggingWriteFile(options.handshake_handle,
- &pipe_name_length,
- sizeof(pipe_name_length))) {
- return EXIT_FAILURE;
- }
- if (!LoggingWriteFile(options.handshake_handle,
- pipe_name.c_str(),
- pipe_name.size() * sizeof(pipe_name[0]))) {
- return EXIT_FAILURE;
- }
- }
-#endif // OS_MACOSX
-
- scoped_ptr<CrashReportDatabase> database(CrashReportDatabase::Initialize(
- base::FilePath(ToolSupport::CommandLineArgumentToFilePathStringType(
- options.database))));
- if (!database) {
- return EXIT_FAILURE;
- }
-
- CrashReportUploadThread upload_thread(database.get(), options.url);
- upload_thread.Start();
-
- CrashReportExceptionHandler exception_handler(
- database.get(), &upload_thread, &options.annotations);
-
- exception_handler_server.Run(&exception_handler);
-
- upload_thread.Stop();
-
- return EXIT_SUCCESS;
-}
-
-} // namespace
-} // namespace crashpad
#if defined(OS_MACOSX)
int main(int argc, char* argv[]) {
From 6c1bd97df0a2f3a483ed4f36b63569485ca8b07e Mon Sep 17 00:00:00 2001
From: Scott Graham <scottmg@chromium.org>
Date: Fri, 6 Nov 2015 10:59:33 -0800
Subject: [PATCH 08/12] Fix compile after 20869d9
TBR=mark@chromium.org
BUG=chromium:546288
Review URL: https://codereview.chromium.org/1431813003 .
---
handler/main.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/handler/main.cc b/handler/main.cc
index 03ee87fb..f326bd7c 100644
--- a/handler/main.cc
+++ b/handler/main.cc
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-#include "handler/main.h"
+#include "handler/handler_main.h"
#include "build/build_config.h"
#include "tools/tool_support.h"
From b666bcbe98c67256a64418313755a40fea2992d6 Mon Sep 17 00:00:00 2001
From: Mark Mentovai <mark@chromium.org>
Date: Fri, 6 Nov 2015 15:03:13 -0500
Subject: [PATCH 09/12] win: Use signed int as the integer representation of
HANDLEs
HandleToInt() and IntToHandle() use int, a signed type, for the 32-bit
integer representation of HANDLE values. For opaque values, an unsigned
type would normally be used, but in this case, signed was chosen for
sign extension to work correctly. INVALID_HANDLE_VALUE is defined as
((HANDLE)(LONG_PTR)-1), and this needs to round-trip through the chosen
integer representation back to the same HANDLE value. Sign extension is
also recommended by
https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203.
As suggested in
https://codereview.chromium.org/1422503015/diff/1/util/win/handle.cc#newcode24
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1422023010 .
---
test/win/win_child_process.cc | 4 ++++
util/win/handle.h | 24 ++++++++++++------------
util/win/process_info.h | 2 +-
util/win/process_info_test.cc | 9 ++++-----
util/win/registration_protocol_win.h | 20 ++++++++++----------
5 files changed, 31 insertions(+), 28 deletions(-)
diff --git a/test/win/win_child_process.cc b/test/win/win_child_process.cc
index b6cbbbd2..6b7a1a07 100644
--- a/test/win/win_child_process.cc
+++ b/test/win/win_child_process.cc
@@ -142,9 +142,13 @@ WinChildProcess::WinChildProcess() {
// values are passed to the child on the command line.
std::string left, right;
CHECK(SplitString(switch_value, '|', &left, &right));
+
+ // left and right were formatted as 0x%x, so they need to be converted as
+ // unsigned ints.
unsigned int write, read;
CHECK(StringToNumber(left, &write));
CHECK(StringToNumber(right, &read));
+
pipe_write_.reset(IntToHandle(write));
pipe_read_.reset(IntToHandle(read));
diff --git a/util/win/handle.h b/util/win/handle.h
index c5a96cae..8a630690 100644
--- a/util/win/handle.h
+++ b/util/win/handle.h
@@ -21,17 +21,17 @@ namespace crashpad {
//! \brief Converts a `HANDLE` to an `int`.
//!
-//! `HANDLE` is a `typedef` for `void *`, but `HANDLE` values aren’t necessarily
-//! pointers to anything. Only 32 bits of shareable `HANDLE`s are significant,
-//! even in 64-bit processes on 64-bit operating systems. See <a
+//! `HANDLE` is a `typedef` for `void *`, but kernel `HANDLE` values aren’t
+//! pointers to anything. Only 32 bits of kernel `HANDLE`s are significant, even
+//! in 64-bit processes on 64-bit operating systems. See <a
//! href="https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203">Interprocess
//! Communication Between 32-bit and 64-bit Applications</a>.
//!
-//! This function safely converts a shareable `HANDLE` to an `int` similarly to
-//! a cast operation. It checks that the operation can be performed safely, and
+//! This function safely converts a kernel `HANDLE` to an `int` similarly to a
+//! cast operation. It checks that the operation can be performed safely, and
//! aborts execution if it cannot.
//!
-//! \param[in] handle The shareable `HANDLE` to convert.
+//! \param[in] handle The kernel `HANDLE` to convert.
//!
//! \return An equivalent `int`, truncated (if necessary) from \a handle. If
//! truncation would have resulted in an `int` that could not be converted
@@ -42,20 +42,20 @@ int HandleToInt(HANDLE handle);
//! \brief Converts an `int` to an `HANDLE`.
//!
-//! `HANDLE` is a `typedef` for `void *`, but `HANDLE` values aren’t necessarily
-//! pointers to anything. Only 32 bits of shareable `HANDLE`s are significant,
-//! even in 64-bit processes on 64-bit operating systems. See <a
+//! `HANDLE` is a `typedef` for `void *`, but kernel `HANDLE` values aren’t
+//! pointers to anything. Only 32 bits of kernel `HANDLE`s are significant, even
+//! in 64-bit processes on 64-bit operating systems. See <a
//! href="https://msdn.microsoft.com/en-us/library/windows/desktop/aa384203">Interprocess
//! Communication Between 32-bit and 64-bit Applications</a>.
//!
-//! This function safely convert an `int` to a shareable `HANDLE` similarly to a
+//! This function safely convert an `int` to a kernel `HANDLE` similarly to a
//! cast operation.
//!
//! \param[in] handle_int The `int` to convert. This must have been produced by
//! HandleToInt(), possibly in a different process.
//!
-//! \return An equivalent shareable `HANDLE`, sign-extended (if necessary) from
-//! \a handle_int.
+//! \return An equivalent kernel `HANDLE`, sign-extended (if necessary) from \a
+//! handle_int.
//!
//! \sa HandleToInt()
HANDLE IntToHandle(int handle_int);
diff --git a/util/win/process_info.h b/util/win/process_info.h
index 49a6f354..fb1b8b3e 100644
--- a/util/win/process_info.h
+++ b/util/win/process_info.h
@@ -58,7 +58,7 @@ class ProcessInfo {
std::wstring type_name;
//! \brief The handle's value.
- uint32_t handle;
+ int handle;
//! \brief The attributes for the handle, e.g. `OBJ_INHERIT`,
//! `OBJ_CASE_INSENSITIVE`, etc.
diff --git a/util/win/process_info_test.cc b/util/win/process_info_test.cc
index 7bf0f4ef..74f086db 100644
--- a/util/win/process_info_test.cc
+++ b/util/win/process_info_test.cc
@@ -576,8 +576,7 @@ TEST(ProcessInfo, Handles) {
bool found_key_handle = false;
bool found_mapping_handle = false;
for (auto handle : info.Handles()) {
- const int handle_int = implicit_cast<int>(handle.handle);
- if (handle_int == HandleToInt(file.get())) {
+ if (handle.handle == HandleToInt(file.get())) {
EXPECT_FALSE(found_file_handle);
found_file_handle = true;
EXPECT_EQ(L"File", handle.type_name);
@@ -587,7 +586,7 @@ TEST(ProcessInfo, Handles) {
handle.granted_access & STANDARD_RIGHTS_ALL);
EXPECT_EQ(0, handle.attributes);
}
- if (handle_int == HandleToInt(inherited_file.get())) {
+ if (handle.handle == HandleToInt(inherited_file.get())) {
EXPECT_FALSE(found_inherited_file_handle);
found_inherited_file_handle = true;
EXPECT_EQ(L"File", handle.type_name);
@@ -601,7 +600,7 @@ TEST(ProcessInfo, Handles) {
const int kObjInherit = 0x2;
EXPECT_EQ(kObjInherit, handle.attributes);
}
- if (handle_int == HandleToInt(scoped_key.get())) {
+ if (handle.handle == HandleToInt(scoped_key.get())) {
EXPECT_FALSE(found_key_handle);
found_key_handle = true;
EXPECT_EQ(L"Key", handle.type_name);
@@ -611,7 +610,7 @@ TEST(ProcessInfo, Handles) {
handle.granted_access & STANDARD_RIGHTS_ALL);
EXPECT_EQ(0, handle.attributes);
}
- if (handle_int == HandleToInt(mapping.get())) {
+ if (handle.handle == HandleToInt(mapping.get())) {
EXPECT_FALSE(found_mapping_handle);
found_mapping_handle = true;
EXPECT_EQ(L"Section", handle.type_name);
diff --git a/util/win/registration_protocol_win.h b/util/win/registration_protocol_win.h
index f2d9e29c..c57c6e20 100644
--- a/util/win/registration_protocol_win.h
+++ b/util/win/registration_protocol_win.h
@@ -92,22 +92,22 @@ struct ClientToServerMessage {
};
};
-//! \brief A client registration response.
+//! \brief A client registration response.
struct RegistrationResponse {
//! \brief An event `HANDLE`, valid in the client process, that should be
- //! signaled to request a crash report. 64-bit clients should convert the
- //! value to a `HANDLE` using sign-extension.
- uint32_t request_crash_dump_event;
+ //! signaled to request a crash report. Clients should convert the value
+ //! to a `HANDLE` by calling IntToHandle().
+ int request_crash_dump_event;
//! \brief An event `HANDLE`, valid in the client process, that should be
- //! signaled to request a non-crashing dump be taken. 64-bit clients
- //! should convert the value to `HANDLEEE` using sign-extension.
- uint32_t request_non_crash_dump_event;
+ //! signaled to request a non-crashing dump be taken. Clients should
+ //! convert the value to a `HANDLE` by calling IntToHandle().
+ int request_non_crash_dump_event;
//! \brief An event `HANDLE`, valid in the client process, that will be
- //! signaled by the server when the non-crashing dump is complete. 64-bit
- //! clients should convert the value to `HANDLEEE` using sign-extension.
- uint32_t non_crash_dump_completed_event;
+ //! signaled by the server when the non-crashing dump is complete. Clients
+ //! should convert the value to a `HANDLE` by calling IntToHandle().
+ int non_crash_dump_completed_event;
};
//! \brief The response sent back to the client via SendToCrashHandlerServer().
From d3825afb25a02aaeae466d37debcf2a9935b7266 Mon Sep 17 00:00:00 2001
From: Mark Mentovai <mark@chromium.org>
Date: Fri, 6 Nov 2015 16:55:31 -0500
Subject: [PATCH 10/12] win: Make StartHandler() restrict HANDLES inherited by
crashpad_handler
This requires Windows NT 6.0 (Vista and Server 2008). On earlier
operating system versions, the existing behavior of inheriting all
inheritable handles is retained.
BUG=crashpad:69
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1427273003 .
---
client/crashpad_client_win.cc | 116 +++++++++++++++++++++++++++++++---
1 file changed, 108 insertions(+), 8 deletions(-)
diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc
index 46b61372..13ef3d62 100644
--- a/client/crashpad_client_win.cc
+++ b/client/crashpad_client_win.cc
@@ -19,6 +19,8 @@
#include "base/atomicops.h"
#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/scoped_generic.h"
#include "base/strings/string16.h"
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
@@ -26,6 +28,7 @@
#include "util/file/file_io.h"
#include "util/win/command_line.h"
#include "util/win/critical_section_with_debug_info.h"
+#include "util/win/get_function.h"
#include "util/win/handle.h"
#include "util/win/registration_protocol_win.h"
#include "util/win/scoped_handle.h"
@@ -110,6 +113,40 @@ std::wstring FormatArgumentString(const std::string& name,
return std::wstring(L"--") + base::UTF8ToUTF16(name) + L"=" + value;
}
+struct ScopedProcThreadAttributeListTraits {
+ static PPROC_THREAD_ATTRIBUTE_LIST InvalidValue() {
+ return nullptr;
+ }
+
+ static void Free(PPROC_THREAD_ATTRIBUTE_LIST proc_thread_attribute_list) {
+ // This is able to use GET_FUNCTION_REQUIRED() instead of GET_FUNCTION()
+ // because it will only be called if InitializeProcThreadAttributeList() and
+ // UpdateProcThreadAttribute() are present.
+ static const auto delete_proc_thread_attribute_list =
+ GET_FUNCTION_REQUIRED(L"kernel32.dll", ::DeleteProcThreadAttributeList);
+ delete_proc_thread_attribute_list(proc_thread_attribute_list);
+ }
+};
+
+using ScopedProcThreadAttributeList =
+ base::ScopedGeneric<PPROC_THREAD_ATTRIBUTE_LIST,
+ ScopedProcThreadAttributeListTraits>;
+
+// Adds |handle| to |handle_list| if it appears valid.
+//
+// Invalid handles (including INVALID_HANDLE_VALUE and null handles) cannot be
+// added to a PPROC_THREAD_ATTRIBUTE_LIST’s PROC_THREAD_ATTRIBUTE_HANDLE_LIST.
+// If INVALID_HANDLE_VALUE appears, CreateProcess() will fail with
+// ERROR_INVALID_PARAMETER. If a null handle appears, the child process will
+// silently not inherit any handles.
+//
+// Use this function to add handles with uncertain validities.
+void AddHandleToListIfValid(std::vector<HANDLE>* handle_list, HANDLE handle) {
+ if (handle && handle != INVALID_HANDLE_VALUE) {
+ handle_list->push_back(handle);
+ }
+}
+
} // namespace
namespace crashpad {
@@ -172,22 +209,85 @@ bool CrashpadClient::StartHandler(
HandleToInt(pipe_write))),
&command_line);
- STARTUPINFO startup_info = {};
- startup_info.cb = sizeof(startup_info);
- startup_info.dwFlags = STARTF_USESTDHANDLES;
- startup_info.hStdInput = GetStdHandle(STD_INPUT_HANDLE);
- startup_info.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE);
- startup_info.hStdError = GetStdHandle(STD_ERROR_HANDLE);
+ DWORD creation_flags;
+ STARTUPINFOEX startup_info = {};
+ startup_info.StartupInfo.dwFlags = STARTF_USESTDHANDLES;
+ startup_info.StartupInfo.hStdInput = GetStdHandle(STD_INPUT_HANDLE);
+ startup_info.StartupInfo.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE);
+ startup_info.StartupInfo.hStdError = GetStdHandle(STD_ERROR_HANDLE);
+
+ std::vector<HANDLE> handle_list;
+ scoped_ptr<uint8_t[]> proc_thread_attribute_list_storage;
+ ScopedProcThreadAttributeList proc_thread_attribute_list_owner;
+
+ static const auto initialize_proc_thread_attribute_list =
+ GET_FUNCTION(L"kernel32.dll", ::InitializeProcThreadAttributeList);
+ static const auto update_proc_thread_attribute =
+ initialize_proc_thread_attribute_list
+ ? GET_FUNCTION(L"kernel32.dll", ::UpdateProcThreadAttribute)
+ : nullptr;
+ if (!initialize_proc_thread_attribute_list || !update_proc_thread_attribute) {
+ // The OS doesn’t allow handle inheritance to be restricted, so the handler
+ // will inherit every inheritable handle.
+ creation_flags = 0;
+ startup_info.StartupInfo.cb = sizeof(startup_info.StartupInfo);
+ } else {
+ // Restrict handle inheritance to just those needed in the handler.
+
+ creation_flags = EXTENDED_STARTUPINFO_PRESENT;
+ startup_info.StartupInfo.cb = sizeof(startup_info);
+ SIZE_T size;
+ rv = initialize_proc_thread_attribute_list(nullptr, 1, 0, &size);
+ if (rv) {
+ LOG(ERROR) << "InitializeProcThreadAttributeList (size) succeeded, "
+ "expected failure";
+ return false;
+ } else if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) {
+ PLOG(ERROR) << "InitializeProcThreadAttributeList (size)";
+ return false;
+ }
+
+ proc_thread_attribute_list_storage.reset(new uint8_t[size]);
+ startup_info.lpAttributeList =
+ reinterpret_cast<PPROC_THREAD_ATTRIBUTE_LIST>(
+ proc_thread_attribute_list_storage.get());
+ rv = initialize_proc_thread_attribute_list(
+ startup_info.lpAttributeList, 1, 0, &size);
+ if (!rv) {
+ PLOG(ERROR) << "InitializeProcThreadAttributeList";
+ return false;
+ }
+ proc_thread_attribute_list_owner.reset(startup_info.lpAttributeList);
+
+ handle_list.reserve(4);
+ handle_list.push_back(pipe_write);
+ AddHandleToListIfValid(&handle_list, startup_info.StartupInfo.hStdInput);
+ AddHandleToListIfValid(&handle_list, startup_info.StartupInfo.hStdOutput);
+ AddHandleToListIfValid(&handle_list, startup_info.StartupInfo.hStdError);
+ rv = update_proc_thread_attribute(
+ startup_info.lpAttributeList,
+ 0,
+ PROC_THREAD_ATTRIBUTE_HANDLE_LIST,
+ &handle_list[0],
+ handle_list.size() * sizeof(handle_list[0]),
+ nullptr,
+ nullptr);
+ if (!rv) {
+ PLOG(ERROR) << "UpdateProcThreadAttribute";
+ return false;
+ }
+ }
+
PROCESS_INFORMATION process_info;
rv = CreateProcess(handler.value().c_str(),
&command_line[0],
nullptr,
nullptr,
true,
- 0,
+ creation_flags,
nullptr,
nullptr,
- &startup_info,
+ &startup_info.StartupInfo,
&process_info);
if (!rv) {
PLOG(ERROR) << "CreateProcess";
From 9e4cd8f07b107edf9cbba5c5760967d5aeb8e2bd Mon Sep 17 00:00:00 2001
From: Scott Graham <scottmg@chromium.org>
Date: Fri, 6 Nov 2015 14:08:13 -0800
Subject: [PATCH 11/12] win: Add DumpAndCrash to client
Something like this is required to implement something like
https://code.google.com/p/chromium/codesearch#chromium/src/components/crash/content/app/breakpad_win.cc&l=397
in Chrome (used by Syzygy and V8 in x64 it looks like).
I didn't want to expose UnhandledExceptionFilter() directly as it's
__stdcall so adding a forwarder to CrashpadClient seemed tidier, but the
functionality matches what is needed.
R=mark@chromium.org
BUG=chromium:546288
Review URL: https://codereview.chromium.org/1416603010 .
---
client/crashpad_client.h | 10 ++++++++++
client/crashpad_client_win.cc | 5 +++++
2 files changed, 15 insertions(+)
diff --git a/client/crashpad_client.h b/client/crashpad_client.h
index eb3fc6de..79a4900e 100644
--- a/client/crashpad_client.h
+++ b/client/crashpad_client.h
@@ -142,6 +142,16 @@ class CrashpadClient {
//! \param[in] context A `CONTEXT`, generally captured by CaptureContext() or
//! similar.
static void DumpWithoutCrash(const CONTEXT& context);
+
+ //! \brief Requests that the handler capture a dump using the given \a
+ //! exception_pointers to get the `EXCEPTION_RECORD` and `CONTEXT`.
+ //!
+ //! This function is not necessary in general usage as an unhandled exception
+ //! filter is installed by UseHandler().
+ //!
+ //! \param[in] exception_pointers An `EXCEPTION_POINTERS`, as would generally
+ //! passed to an unhandled exception filter.
+ static void DumpAndCrash(EXCEPTION_POINTERS* exception_pointers);
#endif
//! \brief Configures the process to direct its crashes to a Crashpad handler.
diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc
index 13ef3d62..8faebbf2 100644
--- a/client/crashpad_client_win.cc
+++ b/client/crashpad_client_win.cc
@@ -435,4 +435,9 @@ void CrashpadClient::DumpWithoutCrash(const CONTEXT& context) {
PLOG_IF(ERROR, wfso_result != WAIT_OBJECT_0) << "WaitForSingleObject";
}
+// static
+void CrashpadClient::DumpAndCrash(EXCEPTION_POINTERS* exception_pointers) {
+ UnhandledExceptionHandler(exception_pointers);
+}
+
} // namespace crashpad
From ff274507dc30cb3b4273a04cd73f3ab5240d02e7 Mon Sep 17 00:00:00 2001
From: Scott Graham <scottmg@chromium.org>
Date: Fri, 6 Nov 2015 15:54:48 -0800
Subject: [PATCH 12/12] win: Only retry in UseHandler() loop on ERROR_PIPE_BUSY
This is better because now end_to_end_test.py fails immediately with
[1180:9020:20151106,145204.830:ERROR registration_protocol_win.cc:39] CreateFile: The system cannot find the file specified. (0x2)
R=mark@chromium.org
BUG=crashpad:75
Review URL: https://codereview.chromium.org/1409693011 .
---
snapshot/win/end_to_end_test.py | 9 +++++++++
util/win/registration_protocol_win.cc | 24 ++++++++++++++++--------
2 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/snapshot/win/end_to_end_test.py b/snapshot/win/end_to_end_test.py
index 3bbcdb98..ddf48ef2 100644
--- a/snapshot/win/end_to_end_test.py
+++ b/snapshot/win/end_to_end_test.py
@@ -21,6 +21,7 @@ import re
import subprocess
import sys
import tempfile
+import time
g_temp_dirs = []
@@ -101,6 +102,14 @@ def GetDumpFromProgram(out_dir, pipe_name, executable_name):
'--database=' + test_database
])
+ # Wait until the server is ready.
+ printed = False
+ while not os.path.exists(pipe_name):
+ if not printed:
+ print 'Waiting for crashpad_handler to be ready...'
+ printed = True
+ time.sleep(0.1)
+
subprocess.call([os.path.join(out_dir, executable_name), pipe_name])
else:
subprocess.call([os.path.join(out_dir, executable_name),
diff --git a/util/win/registration_protocol_win.cc b/util/win/registration_protocol_win.cc
index 3e0fdf21..38b1b451 100644
--- a/util/win/registration_protocol_win.cc
+++ b/util/win/registration_protocol_win.cc
@@ -24,8 +24,8 @@ namespace crashpad {
bool SendToCrashHandlerServer(const base::string16& pipe_name,
const crashpad::ClientToServerMessage& message,
crashpad::ServerToClientMessage* response) {
- int tries = 5;
- while (tries > 0) {
+ int tries = 0;
+ for (;;) {
ScopedFileHANDLE pipe(
CreateFile(pipe_name.c_str(),
GENERIC_READ | GENERIC_WRITE,
@@ -35,10 +35,20 @@ bool SendToCrashHandlerServer(const base::string16& pipe_name,
SECURITY_SQOS_PRESENT | SECURITY_IDENTIFICATION,
nullptr));
if (!pipe.is_valid()) {
- Sleep(10);
- --tries;
+ if (++tries == 5 || GetLastError() != ERROR_PIPE_BUSY) {
+ PLOG(ERROR) << "CreateFile";
+ return false;
+ }
+
+ if (!WaitNamedPipe(pipe_name.c_str(), 1000) &&
+ GetLastError() != ERROR_SEM_TIMEOUT) {
+ PLOG(ERROR) << "WaitNamedPipe";
+ return false;
+ }
+
continue;
}
+
DWORD mode = PIPE_READMODE_MESSAGE;
if (!SetNamedPipeHandleState(pipe.get(), &mode, nullptr, nullptr)) {
PLOG(ERROR) << "SetNamedPipeHandleState";
@@ -55,7 +65,8 @@ bool SendToCrashHandlerServer(const base::string16& pipe_name,
&bytes_read,
nullptr);
if (!result) {
- PLOG(ERROR) << "TransactNamedPipe";
+ LOG(ERROR) << "TransactNamedPipe: expected " << sizeof(*response)
+ << ", observed " << bytes_read;
return false;
}
if (bytes_read != sizeof(*response)) {
@@ -64,9 +75,6 @@ bool SendToCrashHandlerServer(const base::string16& pipe_name,
}
return true;
}
-
- LOG(ERROR) << "failed to connect after retrying";
- return false;
}
} // namespace crashpad