diff --git a/client/crashpad_client_mac.cc b/client/crashpad_client_mac.cc index a180fd19..5797fba8 100644 --- a/client/crashpad_client_mac.cc +++ b/client/crashpad_client_mac.cc @@ -92,7 +92,7 @@ bool CrashpadClient::StartHandler( const std::string& url, const std::map& annotations, const std::vector& arguments) { - DCHECK_EQ(exception_port_, kMachPortNull); + DCHECK(!exception_port_.is_valid()); // Set up the arguments for execve() first. These aren’t needed until execve() // is called, but it’s dangerous to do this in a child process after fork(). @@ -211,13 +211,13 @@ bool CrashpadClient::StartHandler( // Rendezvous with the handler running in the grandchild process. exception_port_.reset(child_port_handshake.RunServer()); - return exception_port_ ? true : false; + return exception_port_.is_valid(); } bool CrashpadClient::UseHandler() { - DCHECK_NE(exception_port_, kMachPortNull); + DCHECK(exception_port_.is_valid()); - return SetCrashExceptionPorts(exception_port_); + return SetCrashExceptionPorts(exception_port_.get()); } // static @@ -227,7 +227,7 @@ void CrashpadClient::UseSystemDefaultHandler() { // Proceed even if SystemCrashReporterHandler() failed, setting MACH_PORT_NULL // to clear the current exception ports. - if (!SetCrashExceptionPorts(system_crash_reporter_handler)) { + if (!SetCrashExceptionPorts(system_crash_reporter_handler.get())) { SetCrashExceptionPorts(MACH_PORT_NULL); } } diff --git a/client/simulate_crash_mac.cc b/client/simulate_crash_mac.cc index cda6b76d..71d5d904 100644 --- a/client/simulate_crash_mac.cc +++ b/client/simulate_crash_mac.cc @@ -221,7 +221,7 @@ void SimulateCrash(const NativeCPUContext& cpu_context) { DCHECK_LE(handlers.size(), 1u); if (handlers.size() == 1) { DCHECK(handlers[0].mask & EXC_MASK_CRASH); - success = DeliverException(thread, + success = DeliverException(thread.get(), mach_task_self(), exception, codes, diff --git a/compat/non_win/dbghelp.h b/compat/non_win/dbghelp.h index baf088ba..08f07ae8 100644 --- a/compat/non_win/dbghelp.h +++ b/compat/non_win/dbghelp.h @@ -157,6 +157,9 @@ enum MINIDUMP_STREAM_TYPE { //! \brief The stream type for MINIDUMP_SYSTEM_INFO. SystemInfoStream = 7, + //! \brief The stream contains information about active `HANDLE`s. + HandleDataStream = 12, + //! \brief The stream type for MINIDUMP_MISC_INFO, MINIDUMP_MISC_INFO_2, //! MINIDUMP_MISC_INFO_3, and MINIDUMP_MISC_INFO_4. //! diff --git a/doc/appengine/README b/doc/appengine/README new file mode 100644 index 00000000..d3f4d227 --- /dev/null +++ b/doc/appengine/README @@ -0,0 +1,39 @@ +This is the App Engine app that serves https://crashpad-home.appspot.com/. + +To work on this app, obtain the App Engine SDK for Go from +https://cloud.google.com/appengine/downloads. Unpacking it produces a +go_appengine directory. This may be added to your $PATH for convenience, +although it is not necessary. + +The commands in this README are expected to be run from the directory containing +app.yaml. + +The App Engine SDK for Go provides App Engine packages at the “appengine” import +path, but not the newer “google.golang.org/appengine” path. The Crashpad app +uses the newer paths. See +https://github.com/golang/appengine#2-update-import-paths and +https://code.google.com/p/googleappengine/issues/detail?id=11670. To make these +available, obtain a Go release from https://golang.org/dl/, and run: + +$ GOROOT=…/go_appengine/goroot GOPATH=…/go_appengine/gopath go get -d + +To test locally: + +$ goapp serve + +Look for the “Starting module "default" running at: http://localhost:8080” line, +which tells you the URL of the local running instance of the app. + +To deploy: + +$ version=$(git rev-parse --short=12 HEAD) +$ [[ -n "$(git status --porcelain)" ]] && version+=-dirty +$ goapp deploy -version "${version}" + +Note that app.yaml does not name a “version” to encourage you to use a git hash +as the version, as above. + +Activate a newly-deployed version by visiting the App Engine console at +https://appengine.google.com/deployment?&app_id=s~crashpad-home, selecting it, +and choosing “Make Default”. It is also possible to delete old versions from +this page when they are no longer needed. diff --git a/doc/appengine/app.yaml b/doc/appengine/app.yaml index bcf370c8..3d3fbdff 100644 --- a/doc/appengine/app.yaml +++ b/doc/appengine/app.yaml @@ -1,5 +1,4 @@ application: crashpad-home -version: 1 runtime: go api_version: go1 diff --git a/doc/appengine/main.go b/doc/appengine/main.go index 261440d4..e05870f5 100644 --- a/doc/appengine/main.go +++ b/doc/appengine/main.go @@ -31,13 +31,16 @@ import ( "google.golang.org/appengine/urlfetch" ) -const baseURL = "https://chromium.googlesource.com/crashpad/crashpad/+/doc/doc/generated/?format=TEXT" - func init() { http.HandleFunc("/", handler) } func handler(w http.ResponseWriter, r *http.Request) { + const ( + baseURL = "https://chromium.googlesource.com/crashpad/crashpad/+/doc/doc/generated/?format=TEXT" + bugBaseURL = "https://bugs.chromium.org/p/crashpad/" + ) + ctx := appengine.NewContext(r) client := urlfetch.Client(ctx) @@ -47,6 +50,17 @@ func handler(w http.ResponseWriter, r *http.Request) { return } + if r.URL.Path == "/bug" || r.URL.Path == "/bug/" { + http.Redirect(w, r, bugBaseURL, http.StatusFound) + return + } else if r.URL.Path == "/bug/new" { + http.Redirect(w, r, bugBaseURL+"issues/entry", http.StatusFound) + return + } else if strings.HasPrefix(r.URL.Path, "/bug/") { + http.Redirect(w, r, bugBaseURL+"issues/detail?id="+r.URL.Path[5:], http.StatusFound) + return + } + u, err := url.Parse(baseURL) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) diff --git a/doc/status.ad b/doc/status.ad index 1d5933b5..301195fc 100644 --- a/doc/status.ad +++ b/doc/status.ad @@ -28,15 +28,18 @@ https://chromium.googlesource.com/chromium/src/+/d413b2dcb54d523811d386f1ff4084f == In Progress Crashpad is actively being bootstrapped on -https://code.google.com/p/crashpad/issues/detail?id=1[Windows]. With the -exception of the snapshot library, most major components are now working on -Windows, and work is progressing on the Windows snapshot implementation. +https://code.google.com/p/crashpad/issues/detail?id=1[Windows]. All major +components are now working on Windows, and integration into Chromium is expected +shortly. + +Initial work on a Crashpad client for +https://code.google.com/p/crashpad/issues/detail?id=30[Android] has begun. This +is currently in the design phase. == Future There are plans to bring Crashpad clients to other operating systems in the -future, including -https://code.google.com/p/crashpad/issues/detail?id=30[Android] and, more -generically, Linux. There are also plans to implement a +future, including a more generic non-Android Linux implementation. There are +also plans to implement a https://code.google.com/p/crashpad/issues/detail?id=29[crash report processor] as part of Crashpad. No timeline for completing this work has been set yet. diff --git a/doc/support/generate_git.sh b/doc/support/generate_git.sh new file mode 100755 index 00000000..209c11b9 --- /dev/null +++ b/doc/support/generate_git.sh @@ -0,0 +1,85 @@ +#!/bin/bash + +# 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. + +set -e + +# Run from the Crashpad project root directory. +cd "$(dirname "${0}")/../.." + +source doc/support/compat.sh + +basename="$(basename "${0}")" + +status="$(git status --porcelain)" +if [[ -n "${status}" ]]; then + echo "${basename}: the working directory must be clean" >& 2 + git status + exit 1 +fi + +# git symbolic-ref gives the current branch name, but fails for detached HEAD. +# In that case, git rev-parse will give the current hash. +original_branch="$(git symbolic-ref --short --quiet HEAD || git rev-parse HEAD)" + +local_branch="\ +$(${sed_ext} -e 's/(.*)\..*/\1/' <<< "${basename}").${$}.${RANDOM}" + +remote_name=origin +remote_master_branch_name=master +remote_master_branch="${remote_name}/${remote_master_branch_name}" +remote_doc_branch_name=doc +remote_doc_branch="${remote_name}/${remote_doc_branch_name}" + +git fetch + +git checkout -b "${local_branch}" "${remote_doc_branch}" + +dirty= + +function cleanup() { + if [[ "${dirty}" ]]; then + git reset --hard HEAD + git clean --force + fi + + git checkout "${original_branch}" + git branch -D "${local_branch}" +} + +trap cleanup EXIT + +master_hash=$(git rev-parse --short=12 "${remote_master_branch}") +git merge "${remote_master_branch}" \ + -m "Merge ${remote_master_branch_name} ${master_hash} into doc" + +dirty=y + +doc/support/generate.sh + +git add -A doc/generated + +count="$(git diff --staged --numstat | wc -l)" +if [[ $count -gt 0 ]]; then + git commit \ + -m "Update documentation to ${remote_master_branch_name} ${master_hash}" + dirty= + + git push "${remote_name}" "HEAD:${remote_doc_branch_name}" +else + dirty= +fi + +# cleanup will run on exit diff --git a/handler/handler.gyp b/handler/handler.gyp index 28bc9e35..04433ac8 100644 --- a/handler/handler.gyp +++ b/handler/handler.gyp @@ -91,6 +91,24 @@ 'win/crashy_test_program.cc', ], }, + { + 'target_name': 'self_destroying_program', + 'type': 'executable', + 'dependencies': [ + '../client/client.gyp:crashpad_client', + '../compat/compat.gyp:crashpad_compat', + '../snapshot/snapshot.gyp:crashpad_snapshot', + '../third_party/mini_chromium/mini_chromium.gyp:base', + '../tools/tools.gyp:crashpad_tool_support', + '../util/util.gyp:crashpad_util', + ], + 'include_dirs': [ + '..', + ], + 'sources': [ + 'win/self_destroying_test_program.cc', + ], + }, ], }, { 'targets': [], diff --git a/handler/mac/crash_report_exception_handler.cc b/handler/mac/crash_report_exception_handler.cc index a54cd0dd..288148e9 100644 --- a/handler/mac/crash_report_exception_handler.cc +++ b/handler/mac/crash_report_exception_handler.cc @@ -186,7 +186,7 @@ kern_return_t CrashReportExceptionHandler::CatchMachException( // vendor. base::mac::ScopedMachSendRight system_crash_reporter_handler(SystemCrashReporterHandler()); - if (system_crash_reporter_handler) { + if (system_crash_reporter_handler.get()) { // Make copies of mutable out parameters so that the system crash reporter // can’t influence the state returned by this method. thread_state_flavor_t flavor_forward = *flavor; @@ -205,7 +205,7 @@ kern_return_t CrashReportExceptionHandler::CatchMachException( // will be available. kern_return_t kr = UniversalExceptionRaise( EXCEPTION_STATE_IDENTITY | MACH_EXCEPTION_CODES, - system_crash_reporter_handler, + system_crash_reporter_handler.get(), thread, task, exception, diff --git a/handler/mac/exception_handler_server.cc b/handler/mac/exception_handler_server.cc index 39348ec4..52da8faa 100644 --- a/handler/mac/exception_handler_server.cc +++ b/handler/mac/exception_handler_server.cc @@ -41,7 +41,7 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface, exception_port_(exception_port), notify_port_(NewMachPort(MACH_PORT_RIGHT_RECEIVE)), running_(true) { - CHECK_NE(notify_port_, kMachPortNull); + CHECK(notify_port_.is_valid()); composite_mach_message_server_.AddHandler(&mach_exc_server_); composite_mach_message_server_.AddHandler(¬ify_server_); @@ -61,7 +61,7 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface, exception_port_, MACH_NOTIFY_NO_SENDERS, 0, - notify_port_, + notify_port_.get(), MACH_MSG_TYPE_MAKE_SEND_ONCE, &previous); MACH_CHECK(kr == KERN_SUCCESS, kr) << "mach_port_request_notification"; @@ -83,11 +83,11 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface, NewMachPort(MACH_PORT_RIGHT_PORT_SET)); kr = mach_port_insert_member( - mach_task_self(), exception_port_, server_port_set); + mach_task_self(), exception_port_, server_port_set.get()); MACH_CHECK(kr == KERN_SUCCESS, kr) << "mach_port_insert_member"; kr = mach_port_insert_member( - mach_task_self(), notify_port_, server_port_set); + mach_task_self(), notify_port_.get(), server_port_set.get()); MACH_CHECK(kr == KERN_SUCCESS, kr) << "mach_port_insert_member"; // Run the server in kOneShot mode so that running_ can be reevaluated after @@ -98,7 +98,7 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface, // DoMachNotifyNoSenders() as appropriate. mach_msg_return_t mr = MachMessageServer::Run(&composite_mach_message_server_, - server_port_set, + server_port_set.get(), kMachMessageReceiveAuditTrailer, MachMessageServer::kOneShot, MachMessageServer::kReceiveLargeIgnore, @@ -221,7 +221,7 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface, ExceptionHandlerServer::ExceptionHandlerServer() : receive_port_(NewMachPort(MACH_PORT_RIGHT_RECEIVE)) { - CHECK_NE(receive_port_, kMachPortNull); + CHECK(receive_port_.is_valid()); } ExceptionHandlerServer::~ExceptionHandlerServer() { @@ -229,7 +229,7 @@ ExceptionHandlerServer::~ExceptionHandlerServer() { void ExceptionHandlerServer::Run( UniversalMachExcServer::Interface* exception_interface) { - ExceptionHandlerServerRun run(receive_port_, exception_interface); + ExceptionHandlerServerRun run(receive_port_.get(), exception_interface); run.Run(); } diff --git a/handler/mac/exception_handler_server.h b/handler/mac/exception_handler_server.h index 37c61a0f..83e131be 100644 --- a/handler/mac/exception_handler_server.h +++ b/handler/mac/exception_handler_server.h @@ -57,7 +57,7 @@ class ExceptionHandlerServer { //! //! The caller does not take ownership of this port. The caller must not use //! this port for any purpose other than to make send rights for clients. - mach_port_t receive_port() const { return receive_port_; } + mach_port_t receive_port() const { return receive_port_.get(); } private: base::mac::ScopedMachReceiveRight receive_port_; diff --git a/handler/win/self_destroying_test_program.cc b/handler/win/self_destroying_test_program.cc new file mode 100644 index 00000000..d00fa476 --- /dev/null +++ b/handler/win/self_destroying_test_program.cc @@ -0,0 +1,97 @@ +// Copyright 2015 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include +#include +#include + +#include "base/logging.h" +#include "base/strings/stringprintf.h" +#include "client/crashpad_client.h" +#include "snapshot/win/process_reader_win.h" +#include "tools/tool_support.h" + +namespace crashpad { +namespace { + +// We VirtualFree a region in ourselves (the stack) to confirm that the +// exception reporter captures as much as possible in the minidump and doesn't +// abort. __debugbreak() immediately after doing so because the process is +// clearly in a very broken state at this point. +bool FreeOwnStackAndBreak() { + ProcessReaderWin process_reader; + if (!process_reader.Initialize(GetCurrentProcess(), + ProcessSuspensionState::kRunning)) { + LOG(ERROR) << "ProcessReaderWin Initialize"; + return false; + } + + const std::vector threads = + process_reader.Threads(); + if (threads.empty()) { + LOG(ERROR) << "no threads"; + return false; + } + + // Push the stack up a bit so that hopefully the crash handler can succeed, + // but won't be able to read the base of the stack. + _alloca(16384); + + // We can't succeed at MEM_RELEASEing this memory, but MEM_DECOMMIT is good + // enough to make it inaccessible. + if (!VirtualFree(reinterpret_cast(threads[0].stack_region_address), + 100, + MEM_DECOMMIT)) { + PLOG(ERROR) << "VirtualFree"; + return false; + } + + // If the VirtualFree() succeeds, we may have already crashed. __debugbreak() + // just to be sure. + __debugbreak(); + + return true; +} + +int SelfDestroyingMain(int argc, char* argv[]) { + if (argc != 2) { + fprintf(stderr, "Usage: %s \n", argv[0]); + return EXIT_FAILURE; + } + + CrashpadClient client; + if (!client.SetHandler(argv[1])) { + LOG(ERROR) << "SetHandler"; + return EXIT_FAILURE; + } + if (!client.UseHandler()) { + LOG(ERROR) << "UseHandler"; + return EXIT_FAILURE; + } + + if (!FreeOwnStackAndBreak()) + return EXIT_FAILURE; + + // This will never be reached. On success, we'll have crashed above, or + // otherwise returned before here. + return EXIT_SUCCESS; +} + +} // namespace +} // namespace crashpad + +int wmain(int argc, wchar_t* argv[]) { + return crashpad::ToolSupport::Wmain(argc, argv, crashpad::SelfDestroyingMain); +} diff --git a/minidump/minidump.gyp b/minidump/minidump.gyp index 21041a9e..0e7fb0ad 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_handle_writer.cc', + 'minidump_handle_writer.h', 'minidump_memory_info_writer.cc', 'minidump_memory_info_writer.h', 'minidump_memory_writer.cc', diff --git a/minidump/minidump_extensions.h b/minidump/minidump_extensions.h index 3cb39dc2..5e43055a 100644 --- a/minidump/minidump_extensions.h +++ b/minidump/minidump_extensions.h @@ -71,6 +71,11 @@ enum MinidumpStreamType : uint32_t { //! \sa SystemInfoStream kMinidumpStreamTypeSystemInfo = SystemInfoStream, + //! \brief The stream type for MINIDUMP_HANDLE_DATA_STREAM. + //! + //! \sa HandleDataStream + kMinidumpStreamTypeHandleData = HandleDataStream, + //! \brief The stream type for MINIDUMP_MISC_INFO, MINIDUMP_MISC_INFO_2, //! MINIDUMP_MISC_INFO_3, and MINIDUMP_MISC_INFO_4. //! diff --git a/minidump/minidump_file_writer.cc b/minidump/minidump_file_writer.cc index 88986561..ff353191 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_handle_writer.h" #include "minidump/minidump_memory_info_writer.h" #include "minidump/minidump_memory_writer.h" #include "minidump/minidump_misc_info_writer.h" @@ -108,6 +109,13 @@ void MinidumpFileWriter::InitializeFromSnapshot( AddStream(memory_info_list.Pass()); } + std::vector handles_snapshot = process_snapshot->Handles(); + if (!handles_snapshot.empty()) { + auto handle_data_writer = make_scoped_ptr(new MinidumpHandleDataWriter()); + handle_data_writer->InitializeFromSnapshot(handles_snapshot); + AddStream(handle_data_writer.Pass()); + } + memory_list->AddFromSnapshot(process_snapshot->ExtraMemory()); AddStream(memory_list.Pass()); diff --git a/minidump/minidump_handle_writer.cc b/minidump/minidump_handle_writer.cc new file mode 100644 index 00000000..73eeb0c7 --- /dev/null +++ b/minidump/minidump_handle_writer.cc @@ -0,0 +1,127 @@ +// 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_handle_writer.h" + +#include + +#include "base/logging.h" +#include "base/stl_util.h" +#include "minidump/minidump_extensions.h" +#include "util/file/file_writer.h" +#include "util/numeric/safe_assignment.h" + +namespace crashpad { + +MinidumpHandleDataWriter::MinidumpHandleDataWriter() + : handle_data_stream_base_(), handle_descriptors_(), strings_() { +} + +MinidumpHandleDataWriter::~MinidumpHandleDataWriter() { + STLDeleteContainerPairSecondPointers(strings_.begin(), strings_.end()); +} + +void MinidumpHandleDataWriter::InitializeFromSnapshot( + const std::vector& handle_snapshots) { + DCHECK_EQ(state(), kStateMutable); + + DCHECK(handle_descriptors_.empty()); + // Because we RegisterRVA() on the string writer below, we preallocate and + // never resize the handle_descriptors_ vector. + handle_descriptors_.resize(handle_snapshots.size()); + for (size_t i = 0; i < handle_snapshots.size(); ++i) { + const HandleSnapshot& handle_snapshot = handle_snapshots[i]; + MINIDUMP_HANDLE_DESCRIPTOR& descriptor = handle_descriptors_[i]; + + descriptor.Handle = handle_snapshot.handle; + + if (handle_snapshot.type_name.empty()) { + descriptor.TypeNameRva = 0; + } else { + auto it = strings_.lower_bound(handle_snapshot.type_name); + internal::MinidumpUTF16StringWriter* writer; + if (it != strings_.end() && it->first == handle_snapshot.type_name) { + writer = it->second; + } else { + writer = new internal::MinidumpUTF16StringWriter(); + strings_.insert(it, std::make_pair(handle_snapshot.type_name, writer)); + writer->SetUTF8(handle_snapshot.type_name); + } + writer->RegisterRVA(&descriptor.TypeNameRva); + } + + descriptor.ObjectNameRva = 0; + descriptor.Attributes = handle_snapshot.attributes; + descriptor.GrantedAccess = handle_snapshot.granted_access; + descriptor.HandleCount = handle_snapshot.handle_count; + descriptor.PointerCount = handle_snapshot.pointer_count; + } +} + +bool MinidumpHandleDataWriter::Freeze() { + DCHECK_EQ(state(), kStateMutable); + + if (!MinidumpStreamWriter::Freeze()) + return false; + + handle_data_stream_base_.SizeOfHeader = sizeof(handle_data_stream_base_); + handle_data_stream_base_.SizeOfDescriptor = sizeof(handle_descriptors_[0]); + const size_t handle_count = handle_descriptors_.size(); + if (!AssignIfInRange(&handle_data_stream_base_.NumberOfDescriptors, + handle_count)) { + LOG(ERROR) << "handle_count " << handle_count << " out of range"; + return false; + } + handle_data_stream_base_.Reserved = 0; + + return true; +} + +size_t MinidumpHandleDataWriter::SizeOfObject() { + DCHECK_GE(state(), kStateFrozen); + return sizeof(handle_data_stream_base_) + + sizeof(handle_descriptors_[0]) * handle_descriptors_.size(); +} + +std::vector MinidumpHandleDataWriter::Children() { + DCHECK_GE(state(), kStateFrozen); + + std::vector children; + for (const auto& pair : strings_) + children.push_back(pair.second); + return children; +} + +bool MinidumpHandleDataWriter::WriteObject(FileWriterInterface* file_writer) { + DCHECK_EQ(state(), kStateWritable); + + WritableIoVec iov; + iov.iov_base = &handle_data_stream_base_; + iov.iov_len = sizeof(handle_data_stream_base_); + std::vector iovecs(1, iov); + + for (const auto& descriptor : handle_descriptors_) { + iov.iov_base = &descriptor; + iov.iov_len = sizeof(descriptor); + iovecs.push_back(iov); + } + + return file_writer->WriteIoVec(&iovecs); +} + +MinidumpStreamType MinidumpHandleDataWriter::StreamType() const { + return kMinidumpStreamTypeHandleData; +} + +} // namespace crashpad diff --git a/minidump/minidump_handle_writer.h b/minidump/minidump_handle_writer.h new file mode 100644 index 00000000..43bdac6b --- /dev/null +++ b/minidump/minidump_handle_writer.h @@ -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. + +#ifndef CRASHPAD_MINIDUMP_MINIDUMP_HANDLE_WRITER_H_ +#define CRASHPAD_MINIDUMP_MINIDUMP_HANDLE_WRITER_H_ + +#include +#include + +#include +#include +#include + +#include "minidump/minidump_stream_writer.h" +#include "minidump/minidump_string_writer.h" +#include "minidump/minidump_writable.h" +#include "snapshot/handle_snapshot.h" + +namespace crashpad { + +//! \brief The writer for a MINIDUMP_HANDLE_DATA_STREAM stream in a minidump +//! and its contained MINIDUMP_HANDLE_DESCRIPTOR s. +//! +//! As we currently do not track any data beyond what MINIDUMP_HANDLE_DESCRIPTOR +//! supports, we only write that type of record rather than the newer +//! MINIDUMP_HANDLE_DESCRIPTOR_2. +//! +//! Note that this writer writes both the header (MINIDUMP_HANDLE_DATA_STREAM) +//! and the list of objects (MINIDUMP_HANDLE_DESCRIPTOR), which is different +//! from some of the other list writers. +class MinidumpHandleDataWriter final : public internal::MinidumpStreamWriter { + public: + MinidumpHandleDataWriter(); + ~MinidumpHandleDataWriter() override; + + //! \brief Adds a MINIDUMP_HANDLE_DESCRIPTOR for each handle in \a + //! handle_snapshot to the MINIDUMP_HANDLE_DATA_STREAM. + //! + //! \param[in] handle_snapshots The handle snapshots to use as source data. + //! + //! \note Valid in #kStateMutable. + void InitializeFromSnapshot( + const std::vector& handle_snapshots); + + 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_HANDLE_DATA_STREAM handle_data_stream_base_; + std::vector handle_descriptors_; + std::map strings_; + + DISALLOW_COPY_AND_ASSIGN(MinidumpHandleDataWriter); +}; + +} // namespace crashpad + +#endif // CRASHPAD_MINIDUMP_MINIDUMP_HANDLE_WRITER_H_ diff --git a/minidump/minidump_handle_writer_test.cc b/minidump/minidump_handle_writer_test.cc new file mode 100644 index 00000000..e8f23eb5 --- /dev/null +++ b/minidump/minidump_handle_writer_test.cc @@ -0,0 +1,201 @@ +// 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_handle_writer.h" + +#include + +#include "base/strings/utf_string_conversions.h" +#include "gtest/gtest.h" +#include "minidump/minidump_file_writer.h" +#include "minidump/test/minidump_file_writer_test_util.h" +#include "minidump/test/minidump_string_writer_test_util.h" +#include "minidump/test/minidump_writable_test_util.h" +#include "util/file/string_file.h" + +namespace crashpad { +namespace test { +namespace { + +// The handle data stream is expected to be the only stream. +void GetHandleDataStream( + const std::string& file_contents, + const MINIDUMP_HANDLE_DATA_STREAM** handle_data_stream) { + const size_t kDirectoryOffset = sizeof(MINIDUMP_HEADER); + const size_t kHandleDataStreamOffset = + 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(kMinidumpStreamTypeHandleData, + directory[kDirectoryIndex].StreamType); + EXPECT_EQ(kHandleDataStreamOffset, directory[kDirectoryIndex].Location.Rva); + + *handle_data_stream = + MinidumpWritableAtLocationDescriptor( + file_contents, directory[kDirectoryIndex].Location); + ASSERT_TRUE(*handle_data_stream); +} + +TEST(MinidumpHandleDataWriter, Empty) { + MinidumpFileWriter minidump_file_writer; + auto handle_data_writer = make_scoped_ptr(new MinidumpHandleDataWriter()); + minidump_file_writer.AddStream(handle_data_writer.Pass()); + + StringFile string_file; + ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); + + ASSERT_EQ(sizeof(MINIDUMP_HEADER) + sizeof(MINIDUMP_DIRECTORY) + + sizeof(MINIDUMP_HANDLE_DATA_STREAM), + string_file.string().size()); + + const MINIDUMP_HANDLE_DATA_STREAM* handle_data_stream = nullptr; + ASSERT_NO_FATAL_FAILURE( + GetHandleDataStream(string_file.string(), &handle_data_stream)); + + EXPECT_EQ(0u, handle_data_stream->NumberOfDescriptors); +} + +TEST(MinidumpHandleDataWriter, OneHandle) { + MinidumpFileWriter minidump_file_writer; + auto handle_data_writer = make_scoped_ptr(new MinidumpHandleDataWriter()); + + HandleSnapshot handle_snapshot; + handle_snapshot.handle = 0x1234; + handle_snapshot.type_name = "Something"; + handle_snapshot.attributes = 0x12345678; + handle_snapshot.granted_access = 0x9abcdef0; + handle_snapshot.pointer_count = 4567; + handle_snapshot.handle_count = 9876; + + std::vector snapshot; + snapshot.push_back(handle_snapshot); + + handle_data_writer->InitializeFromSnapshot(snapshot); + + minidump_file_writer.AddStream(handle_data_writer.Pass()); + + StringFile string_file; + ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); + + const size_t kTypeNameStringDataLength = + (handle_snapshot.type_name.size() + 1) * sizeof(base::char16); + ASSERT_EQ(sizeof(MINIDUMP_HEADER) + sizeof(MINIDUMP_DIRECTORY) + + sizeof(MINIDUMP_HANDLE_DATA_STREAM) + + sizeof(MINIDUMP_HANDLE_DESCRIPTOR) + sizeof(MINIDUMP_STRING) + + kTypeNameStringDataLength, + string_file.string().size()); + + const MINIDUMP_HANDLE_DATA_STREAM* handle_data_stream = nullptr; + ASSERT_NO_FATAL_FAILURE( + GetHandleDataStream(string_file.string(), &handle_data_stream)); + + EXPECT_EQ(1u, handle_data_stream->NumberOfDescriptors); + const MINIDUMP_HANDLE_DESCRIPTOR* handle_descriptor = + reinterpret_cast( + &handle_data_stream[1]); + EXPECT_EQ(handle_snapshot.handle, handle_descriptor->Handle); + EXPECT_EQ(handle_snapshot.type_name, + base::UTF16ToUTF8(MinidumpStringAtRVAAsString( + string_file.string(), handle_descriptor->TypeNameRva))); + EXPECT_EQ(0u, handle_descriptor->ObjectNameRva); + EXPECT_EQ(handle_snapshot.attributes, handle_descriptor->Attributes); + EXPECT_EQ(handle_snapshot.granted_access, handle_descriptor->GrantedAccess); + EXPECT_EQ(handle_snapshot.handle_count, handle_descriptor->HandleCount); + EXPECT_EQ(handle_snapshot.pointer_count, handle_descriptor->PointerCount); +} + +TEST(MinidumpHandleDataWriter, RepeatedTypeName) { + MinidumpFileWriter minidump_file_writer; + auto handle_data_writer = make_scoped_ptr(new MinidumpHandleDataWriter()); + + HandleSnapshot handle_snapshot; + handle_snapshot.handle = 0x1234; + handle_snapshot.type_name = "Something"; + handle_snapshot.attributes = 0x12345678; + handle_snapshot.granted_access = 0x9abcdef0; + handle_snapshot.pointer_count = 4567; + handle_snapshot.handle_count = 9876; + + HandleSnapshot handle_snapshot2; + handle_snapshot2.handle = 0x4321; + handle_snapshot2.type_name = "Something"; // Note: same as above. + handle_snapshot2.attributes = 0x87654321; + handle_snapshot2.granted_access = 0x0fedcba9; + handle_snapshot2.pointer_count = 7654; + handle_snapshot2.handle_count = 6789; + + std::vector snapshot; + snapshot.push_back(handle_snapshot); + snapshot.push_back(handle_snapshot2); + + handle_data_writer->InitializeFromSnapshot(snapshot); + + minidump_file_writer.AddStream(handle_data_writer.Pass()); + + StringFile string_file; + ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); + + const size_t kTypeNameStringDataLength = + (handle_snapshot.type_name.size() + 1) * sizeof(base::char16); + ASSERT_EQ(sizeof(MINIDUMP_HEADER) + sizeof(MINIDUMP_DIRECTORY) + + sizeof(MINIDUMP_HANDLE_DATA_STREAM) + + (sizeof(MINIDUMP_HANDLE_DESCRIPTOR) * 2) + + sizeof(MINIDUMP_STRING) + kTypeNameStringDataLength, + string_file.string().size()); + + const MINIDUMP_HANDLE_DATA_STREAM* handle_data_stream = nullptr; + ASSERT_NO_FATAL_FAILURE( + GetHandleDataStream(string_file.string(), &handle_data_stream)); + + EXPECT_EQ(2u, handle_data_stream->NumberOfDescriptors); + const MINIDUMP_HANDLE_DESCRIPTOR* handle_descriptor = + reinterpret_cast( + &handle_data_stream[1]); + EXPECT_EQ(handle_snapshot.handle, handle_descriptor->Handle); + EXPECT_EQ(handle_snapshot.type_name, + base::UTF16ToUTF8(MinidumpStringAtRVAAsString( + string_file.string(), handle_descriptor->TypeNameRva))); + EXPECT_EQ(0u, handle_descriptor->ObjectNameRva); + EXPECT_EQ(handle_snapshot.attributes, handle_descriptor->Attributes); + EXPECT_EQ(handle_snapshot.granted_access, handle_descriptor->GrantedAccess); + EXPECT_EQ(handle_snapshot.handle_count, handle_descriptor->HandleCount); + EXPECT_EQ(handle_snapshot.pointer_count, handle_descriptor->PointerCount); + + const MINIDUMP_HANDLE_DESCRIPTOR* handle_descriptor2 = + reinterpret_cast( + reinterpret_cast(&handle_data_stream[1]) + + sizeof(MINIDUMP_HANDLE_DESCRIPTOR)); + EXPECT_EQ(handle_snapshot2.handle, handle_descriptor2->Handle); + EXPECT_EQ(handle_snapshot2.type_name, + base::UTF16ToUTF8(MinidumpStringAtRVAAsString( + string_file.string(), handle_descriptor2->TypeNameRva))); + EXPECT_EQ(0u, handle_descriptor2->ObjectNameRva); + EXPECT_EQ(handle_snapshot2.attributes, handle_descriptor2->Attributes); + EXPECT_EQ(handle_snapshot2.granted_access, handle_descriptor2->GrantedAccess); + EXPECT_EQ(handle_snapshot2.handle_count, handle_descriptor2->HandleCount); + EXPECT_EQ(handle_snapshot2.pointer_count, handle_descriptor2->PointerCount); + + EXPECT_EQ(handle_descriptor->TypeNameRva, handle_descriptor2->TypeNameRva); +} + +} // namespace +} // namespace test +} // namespace crashpad diff --git a/minidump/minidump_memory_info_writer_test.cc b/minidump/minidump_memory_info_writer_test.cc index 21081d53..b2ff4a86 100644 --- a/minidump/minidump_memory_info_writer_test.cc +++ b/minidump/minidump_memory_info_writer_test.cc @@ -51,7 +51,7 @@ void GetMemoryInfoListStream( *memory_info_list = MinidumpWritableAtLocationDescriptor( file_contents, directory[kDirectoryIndex].Location); - ASSERT_TRUE(memory_info_list); + ASSERT_TRUE(*memory_info_list); } TEST(MinidumpMemoryInfoWriter, Empty) { diff --git a/minidump/minidump_test.gyp b/minidump/minidump_test.gyp index 09b80cc2..6f8dc198 100644 --- a/minidump/minidump_test.gyp +++ b/minidump/minidump_test.gyp @@ -36,6 +36,7 @@ 'minidump_context_writer_test.cc', 'minidump_crashpad_info_writer_test.cc', 'minidump_exception_writer_test.cc', + 'minidump_handle_writer_test.cc', 'minidump_file_writer_test.cc', 'minidump_memory_info_writer_test.cc', 'minidump_memory_writer_test.cc', diff --git a/minidump/test/minidump_writable_test_util.cc b/minidump/test/minidump_writable_test_util.cc index ef00f40a..c8ea4f44 100644 --- a/minidump/test/minidump_writable_test_util.cc +++ b/minidump/test/minidump_writable_test_util.cc @@ -181,6 +181,14 @@ struct MinidumpThreadListTraits { } }; +struct MinidumpHandleDataStreamTraits { + using ListType = MINIDUMP_HANDLE_DATA_STREAM; + enum : size_t { kElementSize = sizeof(MINIDUMP_HANDLE_DESCRIPTOR) }; + static size_t ElementCount(const ListType* list) { + return static_cast(list->NumberOfDescriptors); + } +}; + struct MinidumpMemoryInfoListTraits { using ListType = MINIDUMP_MEMORY_INFO_LIST; enum : size_t { kElementSize = sizeof(MINIDUMP_MEMORY_INFO) }; @@ -252,6 +260,14 @@ const MINIDUMP_THREAD_LIST* MinidumpWritableAtLocationDescriptor< file_contents, location); } +template <> +const MINIDUMP_HANDLE_DATA_STREAM* MinidumpWritableAtLocationDescriptor< + MINIDUMP_HANDLE_DATA_STREAM>(const std::string& file_contents, + const MINIDUMP_LOCATION_DESCRIPTOR& location) { + return MinidumpListAtLocationDescriptor( + file_contents, location); +} + template <> const MINIDUMP_MEMORY_INFO_LIST* MinidumpWritableAtLocationDescriptor< MINIDUMP_MEMORY_INFO_LIST>(const std::string& file_contents, diff --git a/minidump/test/minidump_writable_test_util.h b/minidump/test/minidump_writable_test_util.h index 9cba8be0..f6190283 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_HANDLE_DATA_STREAM); MINIDUMP_ALLOW_OVERSIZED_DATA(MINIDUMP_MEMORY_INFO_LIST); MINIDUMP_ALLOW_OVERSIZED_DATA(MinidumpModuleCrashpadInfoList); MINIDUMP_ALLOW_OVERSIZED_DATA(MinidumpRVAList); @@ -190,6 +191,11 @@ const MINIDUMP_THREAD_LIST* MinidumpWritableAtLocationDescriptor< MINIDUMP_THREAD_LIST>(const std::string& file_contents, const MINIDUMP_LOCATION_DESCRIPTOR& location); +template <> +const MINIDUMP_HANDLE_DATA_STREAM* MinidumpWritableAtLocationDescriptor< + MINIDUMP_HANDLE_DATA_STREAM>(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, diff --git a/snapshot/handle_snapshot.h b/snapshot/handle_snapshot.h index b86706e7..1346b41d 100644 --- a/snapshot/handle_snapshot.h +++ b/snapshot/handle_snapshot.h @@ -25,8 +25,8 @@ struct HandleSnapshot { HandleSnapshot(); ~HandleSnapshot(); - //! \brief A string representation of the handle's type. - std::wstring type_name; + //! \brief A UTF-8 string representation of the handle's type. + std::string type_name; //! \brief The handle's value. uint32_t handle; diff --git a/snapshot/win/end_to_end_test.py b/snapshot/win/end_to_end_test.py index a09edd46..9ed5fda7 100644 --- a/snapshot/win/end_to_end_test.py +++ b/snapshot/win/end_to_end_test.py @@ -76,8 +76,8 @@ def GetCdbPath(): return None -def GetDumpFromCrashyProgram(out_dir, pipe_name): - """Initialize a crash database, run crashpad_handler, run crashy_program +def GetDumpFromProgram(out_dir, pipe_name, executable_name): + """Initialize a crash database, run crashpad_handler, run |executable_name| connecting to the crash_handler. Returns the minidump generated by crash_handler for further testing. """ @@ -97,7 +97,7 @@ def GetDumpFromCrashyProgram(out_dir, pipe_name): '--database=' + test_database ]) - subprocess.call([os.path.join(out_dir, 'crashy_program.exe'), pipe_name]) + subprocess.call([os.path.join(out_dir, executable_name), pipe_name]) out = subprocess.check_output([ os.path.join(out_dir, 'crashpad_database_util.exe'), @@ -113,6 +113,14 @@ def GetDumpFromCrashyProgram(out_dir, pipe_name): handler.kill() +def GetDumpFromCrashyProgram(out_dir, pipe_name): + return GetDumpFromProgram(out_dir, pipe_name, 'crashy_program.exe') + + +def GetDumpFromSelfDestroyingProgram(out_dir, pipe_name): + return GetDumpFromProgram(out_dir, pipe_name, 'self_destroying_program.exe') + + 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 @@ -147,7 +155,7 @@ class CdbRun(object): sys.exit(1) -def RunTests(cdb_path, dump_path, pipe_name): +def RunTests(cdb_path, dump_path, destroyed_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. @@ -183,10 +191,26 @@ 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') - if float(platform.win32_ver()[0]) != 7: + if 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') + out = CdbRun(cdb_path, dump_path, '!handle') + out.Check(r'\d+ Handles', 'captured handles') + out.Check(r'Event\s+\d+', 'capture some event handles') + out.Check(r'File\s+\d+', 'capture some file handles') + + out = CdbRun(cdb_path, destroyed_dump_path, '.ecxr;!peb;k 2') + out.Check(r'Ldr\.InMemoryOrderModuleList:.*\d+ \. \d+', 'PEB_LDR_DATA saved') + out.Check(r'ntdll\.dll', 'ntdll present', re.IGNORECASE) + + # Check that there is no stack trace in the self-destroyed process. Confirm + # that the top is where we expect it (that's based only on IP), but subsequent + # stack entries will not be available. This confirms that we have a mostly + # valid dump, but that the stack was omitted. + out.Check(r'self_destroying_program!crashpad::`anonymous namespace\'::' + r'FreeOwnStackAndBreak.*\nquit:', + 'at correct location, no additional stack entries') def main(args): try: @@ -202,17 +226,23 @@ def main(args): # Make sure we can download Windows symbols. if not os.environ.get('_NT_SYMBOL_PATH'): symbol_dir = MakeTempDir() + protocol = 'https' if platform.win32_ver()[0] != 'XP' else 'http' os.environ['_NT_SYMBOL_PATH'] = ( - 'SRV*' + symbol_dir + '*https://msdl.microsoft.com/download/symbols') + 'SRV*' + symbol_dir + '*' + + protocol + '://msdl.microsoft.com/download/symbols') 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: + crashy_dump_path = GetDumpFromCrashyProgram(args[0], pipe_name) + if not crashy_dump_path: return 1 - RunTests(cdb_path, dump_path, pipe_name) + destroyed_dump_path = GetDumpFromSelfDestroyingProgram(args[0], pipe_name) + if not destroyed_dump_path: + return 1 + + RunTests(cdb_path, crashy_dump_path, destroyed_dump_path, pipe_name) return 0 finally: diff --git a/snapshot/win/process_snapshot_win.cc b/snapshot/win/process_snapshot_win.cc index ca26d4b8..bc3b7b1a 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 "base/strings/utf_string_conversions.h" #include "snapshot/win/memory_snapshot_win.h" #include "snapshot/win/module_snapshot_win.h" #include "util/win/registration_protocol_win.h" @@ -211,7 +212,10 @@ std::vector ProcessSnapshotWin::Handles() const { std::vector result; for (const auto& handle : process_reader_.GetProcessInfo().Handles()) { HandleSnapshot snapshot; - snapshot.type_name = handle.type_name; + // This is probably not strictly correct, but these are not localized so we + // expect them all to be in ASCII range anyway. This will need to be more + // carefully done if the object name is added. + snapshot.type_name = base::UTF16ToUTF8(handle.type_name); snapshot.handle = handle.handle; snapshot.attributes = handle.attributes; snapshot.granted_access = handle.granted_access; @@ -340,19 +344,8 @@ void ProcessSnapshotWin::AddMemorySnapshot( if (size == 0) return; - // Ensure that the entire range is readable. TODO(scottmg): Consider - // generalizing this as part of - // https://code.google.com/p/crashpad/issues/detail?id=59. - auto ranges = process_reader_.GetProcessInfo().GetReadableRanges( - CheckedRange(address, size)); - if (ranges.size() != 1) { - LOG(ERROR) << base::StringPrintf( - "range at 0x%llx, size 0x%llx fully unreadable", address, size); - return; - } - if (ranges[0].base() != address || ranges[0].size() != size) { - LOG(ERROR) << base::StringPrintf( - "some of range at 0x%llx, size 0x%llx unreadable", address, size); + if (!process_reader_.GetProcessInfo().LoggingRangeIsFullyReadable( + CheckedRange(address, size))) { return; } diff --git a/snapshot/win/thread_snapshot_win.cc b/snapshot/win/thread_snapshot_win.cc index 8a0de39e..3593a7e1 100644 --- a/snapshot/win/thread_snapshot_win.cc +++ b/snapshot/win/thread_snapshot_win.cc @@ -41,11 +41,23 @@ bool ThreadSnapshotWin::Initialize( INITIALIZATION_STATE_SET_INITIALIZING(initialized_); thread_ = process_reader_thread; - // TODO(scottmg): Ensure these regions are readable - // https://code.google.com/p/crashpad/issues/detail?id=59 - stack_.Initialize( - process_reader, thread_.stack_region_address, thread_.stack_region_size); - teb_.Initialize(process_reader, thread_.teb_address, thread_.teb_size); + if (process_reader->GetProcessInfo().LoggingRangeIsFullyReadable( + CheckedRange(thread_.stack_region_address, + thread_.stack_region_size))) { + stack_.Initialize(process_reader, + thread_.stack_region_address, + thread_.stack_region_size); + } else { + stack_.Initialize(process_reader, 0, 0); + } + + if (process_reader->GetProcessInfo().LoggingRangeIsFullyReadable( + CheckedRange(thread_.teb_address, + thread_.teb_size))) { + teb_.Initialize(process_reader, thread_.teb_address, thread_.teb_size); + } else { + teb_.Initialize(process_reader, 0, 0); + } #if defined(ARCH_CPU_X86_64) if (process_reader->Is64Bit()) { diff --git a/test/mac/mach_multiprocess.cc b/test/mac/mach_multiprocess.cc index 6d410d38..65ea46df 100644 --- a/test/mac/mach_multiprocess.cc +++ b/test/mac/mach_multiprocess.cc @@ -98,22 +98,22 @@ void MachMultiprocess::PreFork() { } info_->local_port = BootstrapCheckIn(info_->service_name); - ASSERT_NE(kMachPortNull, info_->local_port); + ASSERT_TRUE(info_->local_port.is_valid()); } mach_port_t MachMultiprocess::LocalPort() const { - EXPECT_NE(kMachPortNull, info_->local_port); - return info_->local_port; + EXPECT_TRUE(info_->local_port.is_valid()); + return info_->local_port.get(); } mach_port_t MachMultiprocess::RemotePort() const { - EXPECT_NE(kMachPortNull, info_->remote_port); - return info_->remote_port; + EXPECT_TRUE(info_->remote_port.is_valid()); + return info_->remote_port.get(); } task_t MachMultiprocess::ChildTask() const { - EXPECT_NE(TASK_NULL, info_->child_task); - return info_->child_task; + EXPECT_TRUE(info_->child_task.is_valid()); + return info_->child_task.get(); } void MachMultiprocess::MultiprocessParent() { @@ -123,7 +123,7 @@ void MachMultiprocess::MultiprocessParent() { MACH_RCV_MSG | kMachMessageReceiveAuditTrailer, 0, sizeof(message), - info_->local_port, + info_->local_port.get(), MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL); ASSERT_EQ(MACH_MSG_SUCCESS, kr) << MachErrorMessage(kr, "mach_msg"); @@ -198,7 +198,7 @@ void MachMultiprocess::MultiprocessParent() { // Verify that the child’s task port is what it purports to be. int mach_pid; - kr = pid_for_task(info_->child_task, &mach_pid); + kr = pid_for_task(info_->child_task.get(), &mach_pid); ASSERT_EQ(KERN_SUCCESS, kr) << MachErrorMessage(kr, "pid_for_task"); ASSERT_EQ(ChildPID(), mach_pid); @@ -229,8 +229,8 @@ void MachMultiprocess::MultiprocessChild() { MACH_MSGH_BITS(MACH_MSG_TYPE_COPY_SEND, MACH_MSG_TYPE_MAKE_SEND) | MACH_MSGH_BITS_COMPLEX; message.header.msgh_size = sizeof(message); - message.header.msgh_remote_port = info_->remote_port; - message.header.msgh_local_port = info_->local_port; + message.header.msgh_remote_port = info_->remote_port.get(); + message.header.msgh_local_port = info_->local_port.get(); message.body.msgh_descriptor_count = 1; message.port_descriptor.name = mach_task_self(); message.port_descriptor.disposition = MACH_MSG_TYPE_COPY_SEND; diff --git a/tools/mac/catch_exception_tool.cc b/tools/mac/catch_exception_tool.cc index b70ce0a5..32c6aed2 100644 --- a/tools/mac/catch_exception_tool.cc +++ b/tools/mac/catch_exception_tool.cc @@ -301,7 +301,7 @@ int CatchExceptionToolMain(int argc, char* argv[]) { } mach_msg_return_t mr = MachMessageServer::Run(&universal_mach_exc_server, - service_port, + service_port.get(), MACH_MSG_OPTION_NONE, options.persistent, receive_large, diff --git a/tools/mac/exception_port_tool.cc b/tools/mac/exception_port_tool.cc index e26c1bb0..b873e68b 100644 --- a/tools/mac/exception_port_tool.cc +++ b/tools/mac/exception_port_tool.cc @@ -195,7 +195,7 @@ void ShowBootstrapService(const std::string& service_name, return; } - mach_send_right_pool->AddSendRight(service_port); + mach_send_right_pool->AddSendRight(service_port.get()); printf("service %s %#x\n", service_name.c_str(), service_port.get()); } @@ -300,7 +300,7 @@ bool SetExceptionPort(const ExceptionHandlerDescription* description, ExceptionPorts exception_ports(description->target_type, target_port); if (!exception_ports.SetExceptionPort(description->mask, - service_port, + service_port.get(), description->behavior, description->flavor)) { return false; diff --git a/util/mach/child_port_handshake.cc b/util/mach/child_port_handshake.cc index b01af7a7..119a53c0 100644 --- a/util/mach/child_port_handshake.cc +++ b/util/mach/child_port_handshake.cc @@ -105,7 +105,7 @@ mach_port_t ChildPortHandshake::RunServer() { // Check the new service in with the bootstrap server, obtaining a receive // right for it. base::mac::ScopedMachReceiveRight server_port(BootstrapCheckIn(service_name)); - CHECK_NE(server_port, kMachPortNull); + CHECK(server_port.is_valid()); // Share the service name with the client via the pipe. uint32_t service_name_length = service_name.size(); @@ -125,10 +125,10 @@ mach_port_t ChildPortHandshake::RunServer() { // requires a port set. Create a new port set and add the receive right to it. base::mac::ScopedMachPortSet server_port_set( NewMachPort(MACH_PORT_RIGHT_PORT_SET)); - CHECK_NE(server_port_set, kMachPortNull); + CHECK(server_port_set.is_valid()); - kern_return_t kr = - mach_port_insert_member(mach_task_self(), server_port, server_port_set); + kern_return_t kr = mach_port_insert_member( + mach_task_self(), server_port.get(), server_port_set.get()); MACH_CHECK(kr == KERN_SUCCESS, kr) << "mach_port_insert_member"; // Set up a kqueue to monitor both the server’s receive right and the write @@ -140,7 +140,7 @@ mach_port_t ChildPortHandshake::RunServer() { struct kevent changelist[2]; EV_SET(&changelist[0], - server_port_set, + server_port_set.get(), EVFILT_MACHPORT, EV_ADD | EV_CLEAR, 0, @@ -194,7 +194,7 @@ mach_port_t ChildPortHandshake::RunServer() { switch (event.filter) { case EVFILT_MACHPORT: { // There’s something to receive on the port set. - DCHECK_EQ(event.ident, server_port_set); + DCHECK_EQ(event.ident, server_port_set.get()); // Run the message server in an inner loop instead of using // MachMessageServer::kPersistent. This allows the loop to exit as soon @@ -207,7 +207,7 @@ mach_port_t ChildPortHandshake::RunServer() { // this will call HandleChildPortCheckIn(). mach_msg_return_t mr = MachMessageServer::Run(&child_port_server, - server_port_set, + server_port_set.get(), MACH_MSG_OPTION_NONE, MachMessageServer::kOneShot, MachMessageServer::kReceiveLargeIgnore, @@ -329,10 +329,11 @@ void ChildPortHandshake::RunClientInternal_SendCheckIn( // Get a send right to the server by looking up the service with the bootstrap // server by name. base::mac::ScopedMachSendRight server_port(BootstrapLookUp(service_name)); - CHECK_NE(server_port, kMachPortNull); + CHECK(server_port.is_valid()); // Check in with the server. - kern_return_t kr = child_port_check_in(server_port, token, port, right_type); + kern_return_t kr = + child_port_check_in(server_port.get(), token, port, right_type); MACH_CHECK(kr == KERN_SUCCESS, kr) << "child_port_check_in"; } diff --git a/util/mach/exception_ports_test.cc b/util/mach/exception_ports_test.cc index 7384c022..e6e1d56d 100644 --- a/util/mach/exception_ports_test.cc +++ b/util/mach/exception_ports_test.cc @@ -376,9 +376,9 @@ class TestExceptionPorts : public MachMultiprocess, threads_need_owners.Disarm(); ExceptionPorts main_thread_ports(ExceptionPorts::kTargetTypeThread, - main_thread); + main_thread.get()); ExceptionPorts other_thread_ports(ExceptionPorts::kTargetTypeThread, - other_thread); + other_thread.get()); EXPECT_STREQ("thread", main_thread_ports.TargetTypeName()); EXPECT_STREQ("thread", other_thread_ports.TargetTypeName()); @@ -586,7 +586,8 @@ TEST(ExceptionPorts, HostExceptionPorts) { const bool expect_success = geteuid() == 0; base::mac::ScopedMachSendRight host(mach_host_self()); - ExceptionPorts explicit_host_ports(ExceptionPorts::kTargetTypeHost, host); + ExceptionPorts explicit_host_ports(ExceptionPorts::kTargetTypeHost, + host.get()); EXPECT_STREQ("host", explicit_host_ports.TargetTypeName()); ExceptionPorts::ExceptionHandlerVector explicit_handlers; diff --git a/util/mach/mach_extensions_test.cc b/util/mach/mach_extensions_test.cc index 2b987b37..04af5dfe 100644 --- a/util/mach/mach_extensions_test.cc +++ b/util/mach/mach_extensions_test.cc @@ -34,7 +34,7 @@ TEST(MachExtensions, NewMachPort_Receive) { ASSERT_NE(kMachPortNull, port); mach_port_type_t type; - kern_return_t kr = mach_port_type(mach_task_self(), port, &type); + kern_return_t kr = mach_port_type(mach_task_self(), port.get(), &type); ASSERT_EQ(KERN_SUCCESS, kr) << MachErrorMessage(kr, "mach_port_get_type"); EXPECT_EQ(MACH_PORT_TYPE_RECEIVE, type); @@ -45,7 +45,7 @@ TEST(MachExtensions, NewMachPort_PortSet) { ASSERT_NE(kMachPortNull, port); mach_port_type_t type; - kern_return_t kr = mach_port_type(mach_task_self(), port, &type); + kern_return_t kr = mach_port_type(mach_task_self(), port.get(), &type); ASSERT_EQ(KERN_SUCCESS, kr) << MachErrorMessage(kr, "mach_port_get_type"); EXPECT_EQ(MACH_PORT_TYPE_PORT_SET, type); @@ -56,7 +56,7 @@ TEST(MachExtensions, NewMachPort_DeadName) { ASSERT_NE(kMachPortNull, port); mach_port_type_t type; - kern_return_t kr = mach_port_type(mach_task_self(), port, &type); + kern_return_t kr = mach_port_type(mach_task_self(), port.get(), &type); ASSERT_EQ(KERN_SUCCESS, kr) << MachErrorMessage(kr, "mach_port_get_type"); EXPECT_EQ(MACH_PORT_TYPE_DEAD_NAME, type); @@ -173,7 +173,7 @@ TEST(MachExtensions, BootstrapCheckInAndLookUp) { TEST(MachExtensions, SystemCrashReporterHandler) { base::mac::ScopedMachSendRight system_crash_reporter_handler(SystemCrashReporterHandler()); - EXPECT_TRUE(system_crash_reporter_handler); + EXPECT_TRUE(system_crash_reporter_handler.is_valid()); } } // namespace diff --git a/util/mach/mach_message_server_test.cc b/util/mach/mach_message_server_test.cc index 6878cf95..daa64a93 100644 --- a/util/mach/mach_message_server_test.cc +++ b/util/mach/mach_message_server_test.cc @@ -467,8 +467,8 @@ class TestMachMessageServer : public MachMessageServer::Interface, // carried in the request message to the server. By the time the server // looks at the right, it will have become a dead name. local_receive_port_owner.reset(NewMachPort(MACH_PORT_RIGHT_RECEIVE)); - ASSERT_NE(kMachPortNull, local_receive_port_owner); - request.header.msgh_local_port = local_receive_port_owner; + ASSERT_TRUE(local_receive_port_owner.is_valid()); + request.header.msgh_local_port = local_receive_port_owner.get(); break; } } @@ -479,8 +479,8 @@ class TestMachMessageServer : public MachMessageServer::Interface, // properly handles ownership of resources received in complex messages. request.body.msgh_descriptor_count = 1; child_complex_message_port_.reset(NewMachPort(MACH_PORT_RIGHT_RECEIVE)); - ASSERT_NE(kMachPortNull, child_complex_message_port_); - request.port_descriptor.name = child_complex_message_port_; + ASSERT_TRUE(child_complex_message_port_.is_valid()); + request.port_descriptor.name = child_complex_message_port_.get(); request.port_descriptor.disposition = MACH_MSG_TYPE_MAKE_SEND; request.port_descriptor.type = MACH_MSG_PORT_DESCRIPTOR; } else { diff --git a/util/mach/mach_message_test.cc b/util/mach/mach_message_test.cc index a3fca519..5e79b212 100644 --- a/util/mach/mach_message_test.cc +++ b/util/mach/mach_message_test.cc @@ -115,7 +115,7 @@ TEST(MachMessage, AuditPIDFromMachMessageTrailer) { mach_msg_empty_send_t send = {}; send.header.msgh_bits = MACH_MSGH_BITS(MACH_MSG_TYPE_MAKE_SEND_ONCE, 0); send.header.msgh_size = sizeof(send); - send.header.msgh_remote_port = port; + send.header.msgh_remote_port = port.get(); mach_msg_return_t mr = MachMessageWithDeadline(&send.header, MACH_SEND_MSG, @@ -138,7 +138,7 @@ TEST(MachMessage, AuditPIDFromMachMessageTrailer) { mr = MachMessageWithDeadline(&receive.header, MACH_RCV_MSG | kMachMessageReceiveAuditTrailer, sizeof(receive), - port, + port.get(), kMachMessageDeadlineNonblocking, MACH_PORT_NULL, false); diff --git a/util/mach/notify_server_test.cc b/util/mach/notify_server_test.cc index a852bd0e..445e35bf 100644 --- a/util/mach/notify_server_test.cc +++ b/util/mach/notify_server_test.cc @@ -256,12 +256,12 @@ class NotifyServerTestBase : public testing::Test, //! established for the current test. On failure, returns `MACH_PORT_NULL` //! with a gtest failure added. mach_port_t ServerPort() { - if (!server_port_) { + if (!server_port_.is_valid()) { server_port_.reset(NewMachPort(MACH_PORT_RIGHT_RECEIVE)); - EXPECT_NE(kMachPortNull, server_port_); + EXPECT_TRUE(server_port_.is_valid()); } - return server_port_; + return server_port_.get(); } // testing::Test: @@ -309,14 +309,14 @@ TEST_F(NotifyServerTest, NoNotification) { TEST_F(NotifyServerTest, MachNotifyPortDeleted) { base::mac::ScopedMachReceiveRight receive_right( NewMachPort(MACH_PORT_RIGHT_RECEIVE)); - ASSERT_NE(kMachPortNull, receive_right); + ASSERT_TRUE(receive_right.is_valid()); base::mac::ScopedMachSendRight send_once_right( - SendOnceRightFromReceiveRight(receive_right)); - ASSERT_NE(kMachPortNull, send_once_right); + SendOnceRightFromReceiveRight(receive_right.get())); + ASSERT_TRUE(send_once_right.is_valid()); - ASSERT_TRUE( - RequestMachPortNotification(send_once_right, MACH_NOTIFY_DEAD_NAME, 0)); + ASSERT_TRUE(RequestMachPortNotification( + send_once_right.get(), MACH_NOTIFY_DEAD_NAME, 0)); EXPECT_CALL( *this, @@ -336,10 +336,10 @@ TEST_F(NotifyServerTest, MachNotifyPortDeleted) { TEST_F(NotifyServerTest, MachNotifyPortDestroyed) { base::mac::ScopedMachReceiveRight receive_right( NewMachPort(MACH_PORT_RIGHT_RECEIVE)); - ASSERT_NE(kMachPortNull, receive_right); + ASSERT_TRUE(receive_right.is_valid()); ASSERT_TRUE(RequestMachPortNotification( - receive_right, MACH_NOTIFY_PORT_DESTROYED, 0)); + receive_right.get(), MACH_NOTIFY_PORT_DESTROYED, 0)); EXPECT_CALL( *this, @@ -360,10 +360,10 @@ TEST_F(NotifyServerTest, MachNotifyPortDestroyed) { TEST_F(NotifyServerTest, MachNotifyPortDestroyed_NoNotification) { base::mac::ScopedMachReceiveRight receive_right( NewMachPort(MACH_PORT_RIGHT_RECEIVE)); - ASSERT_NE(kMachPortNull, receive_right); + ASSERT_TRUE(receive_right.is_valid()); ASSERT_TRUE(RequestMachPortNotification( - receive_right, MACH_NOTIFY_PORT_DESTROYED, 0)); + receive_right.get(), MACH_NOTIFY_PORT_DESTROYED, 0)); RunServer(); } @@ -373,10 +373,10 @@ TEST_F(NotifyServerTest, MachNotifyPortDestroyed_NoNotification) { TEST_F(NotifyServerTest, MachNotifyNoSenders_NoSendRight) { base::mac::ScopedMachReceiveRight receive_right( NewMachPort(MACH_PORT_RIGHT_RECEIVE)); - ASSERT_NE(kMachPortNull, receive_right); + ASSERT_TRUE(receive_right.is_valid()); - ASSERT_TRUE( - RequestMachPortNotification(receive_right, MACH_NOTIFY_NO_SENDERS, 0)); + ASSERT_TRUE(RequestMachPortNotification( + receive_right.get(), MACH_NOTIFY_NO_SENDERS, 0)); EXPECT_CALL(*this, DoMachNotifyNoSenders( @@ -393,14 +393,14 @@ TEST_F(NotifyServerTest, MachNotifyNoSenders_NoSendRight) { TEST_F(NotifyServerTest, MachNotifyNoSenders_SendRightDeallocated) { base::mac::ScopedMachReceiveRight receive_right( NewMachPort(MACH_PORT_RIGHT_RECEIVE)); - ASSERT_NE(kMachPortNull, receive_right); + ASSERT_TRUE(receive_right.is_valid()); base::mac::ScopedMachSendRight send_right( - SendRightFromReceiveRight(receive_right)); - ASSERT_NE(kMachPortNull, send_right); + SendRightFromReceiveRight(receive_right.get())); + ASSERT_TRUE(send_right.is_valid()); - ASSERT_TRUE( - RequestMachPortNotification(receive_right, MACH_NOTIFY_NO_SENDERS, 1)); + ASSERT_TRUE(RequestMachPortNotification( + receive_right.get(), MACH_NOTIFY_NO_SENDERS, 1)); EXPECT_CALL(*this, DoMachNotifyNoSenders( @@ -418,25 +418,25 @@ TEST_F(NotifyServerTest, MachNotifyNoSenders_SendRightDeallocated) { TEST_F(NotifyServerTest, MachNotifyNoSenders_NoNotification) { base::mac::ScopedMachReceiveRight receive_right( NewMachPort(MACH_PORT_RIGHT_RECEIVE)); - ASSERT_NE(kMachPortNull, receive_right); + ASSERT_TRUE(receive_right.is_valid()); base::mac::ScopedMachSendRight send_right_0( - SendRightFromReceiveRight(receive_right)); - ASSERT_NE(kMachPortNull, send_right_0); + SendRightFromReceiveRight(receive_right.get())); + ASSERT_TRUE(send_right_0.is_valid()); base::mac::ScopedMachSendRight send_right_1( - SendRightFromReceiveRight(receive_right)); - ASSERT_NE(kMachPortNull, send_right_1); + SendRightFromReceiveRight(receive_right.get())); + ASSERT_TRUE(send_right_1.is_valid()); - ASSERT_TRUE( - RequestMachPortNotification(receive_right, MACH_NOTIFY_NO_SENDERS, 1)); + ASSERT_TRUE(RequestMachPortNotification( + receive_right.get(), MACH_NOTIFY_NO_SENDERS, 1)); send_right_1.reset(); RunServer(); - EXPECT_EQ(1u, RightRefCount(receive_right, MACH_PORT_RIGHT_RECEIVE)); - EXPECT_EQ(1u, RightRefCount(receive_right, MACH_PORT_RIGHT_SEND)); + EXPECT_EQ(1u, RightRefCount(receive_right.get(), MACH_PORT_RIGHT_RECEIVE)); + EXPECT_EQ(1u, RightRefCount(receive_right.get(), MACH_PORT_RIGHT_SEND)); } // When a send-once right is deallocated without being used, a send-once @@ -444,7 +444,7 @@ TEST_F(NotifyServerTest, MachNotifyNoSenders_NoNotification) { TEST_F(NotifyServerTest, MachNotifySendOnce_ExplicitDeallocation) { base::mac::ScopedMachSendRight send_once_right( SendOnceRightFromReceiveRight(ServerPort())); - ASSERT_NE(kMachPortNull, send_once_right); + ASSERT_TRUE(send_once_right.is_valid()); EXPECT_CALL(*this, DoMachNotifySendOnce(ServerPort(), @@ -463,13 +463,13 @@ TEST_F(NotifyServerTest, MachNotifySendOnce_ExplicitDeallocation) { TEST_F(NotifyServerTest, MachNotifySendOnce_ImplicitDeallocation) { base::mac::ScopedMachReceiveRight receive_right( NewMachPort(MACH_PORT_RIGHT_RECEIVE)); - ASSERT_NE(kMachPortNull, receive_right); + ASSERT_TRUE(receive_right.is_valid()); mach_msg_empty_send_t message = {}; message.header.msgh_bits = MACH_MSGH_BITS(MACH_MSG_TYPE_MAKE_SEND, MACH_MSG_TYPE_MAKE_SEND_ONCE); message.header.msgh_size = sizeof(message); - message.header.msgh_remote_port = receive_right; + message.header.msgh_remote_port = receive_right.get(); message.header.msgh_local_port = ServerPort(); mach_msg_return_t mr = mach_msg(&message.header, MACH_SEND_MSG | MACH_SEND_TIMEOUT, @@ -497,14 +497,14 @@ TEST_F(NotifyServerTest, MachNotifySendOnce_ImplicitDeallocation) { TEST_F(NotifyServerTest, MachNotifyDeadName) { base::mac::ScopedMachReceiveRight receive_right( NewMachPort(MACH_PORT_RIGHT_RECEIVE)); - ASSERT_NE(kMachPortNull, receive_right); + ASSERT_TRUE(receive_right.is_valid()); base::mac::ScopedMachSendRight send_once_right( - SendOnceRightFromReceiveRight(receive_right)); - ASSERT_NE(kMachPortNull, send_once_right); + SendOnceRightFromReceiveRight(receive_right.get())); + ASSERT_TRUE(send_once_right.is_valid()); - ASSERT_TRUE( - RequestMachPortNotification(send_once_right, MACH_NOTIFY_DEAD_NAME, 0)); + ASSERT_TRUE(RequestMachPortNotification( + send_once_right.get(), MACH_NOTIFY_DEAD_NAME, 0)); // send_once_right becomes a dead name with the send-once right’s original // user reference count of 1, but the dead-name notification increments the @@ -523,10 +523,11 @@ TEST_F(NotifyServerTest, MachNotifyDeadName) { RunServer(); - EXPECT_TRUE(IsRight(send_once_right, MACH_PORT_TYPE_DEAD_NAME)); + EXPECT_TRUE(IsRight(send_once_right.get(), MACH_PORT_TYPE_DEAD_NAME)); - EXPECT_EQ(0u, RightRefCount(send_once_right, MACH_PORT_RIGHT_SEND_ONCE)); - EXPECT_EQ(1u, DeadNameRightRefCount(send_once_right)); + EXPECT_EQ(0u, + RightRefCount(send_once_right.get(), MACH_PORT_RIGHT_SEND_ONCE)); + EXPECT_EQ(1u, DeadNameRightRefCount(send_once_right.get())); } // When the receive right corresponding to a send-once right with a dead-name @@ -535,21 +536,22 @@ TEST_F(NotifyServerTest, MachNotifyDeadName) { TEST_F(NotifyServerTest, MachNotifyDeadName_NoNotification) { base::mac::ScopedMachReceiveRight receive_right( NewMachPort(MACH_PORT_RIGHT_RECEIVE)); - ASSERT_NE(kMachPortNull, receive_right); + ASSERT_TRUE(receive_right.is_valid()); base::mac::ScopedMachSendRight send_once_right( - SendOnceRightFromReceiveRight(receive_right)); - ASSERT_NE(kMachPortNull, send_once_right); + SendOnceRightFromReceiveRight(receive_right.get())); + ASSERT_TRUE(send_once_right.is_valid()); - ASSERT_TRUE( - RequestMachPortNotification(send_once_right, MACH_NOTIFY_DEAD_NAME, 0)); + ASSERT_TRUE(RequestMachPortNotification( + send_once_right.get(), MACH_NOTIFY_DEAD_NAME, 0)); RunServer(); - EXPECT_FALSE(IsRight(send_once_right, MACH_PORT_TYPE_DEAD_NAME)); + EXPECT_FALSE(IsRight(send_once_right.get(), MACH_PORT_TYPE_DEAD_NAME)); - EXPECT_EQ(1u, RightRefCount(send_once_right, MACH_PORT_RIGHT_SEND_ONCE)); - EXPECT_EQ(0u, DeadNameRightRefCount(send_once_right)); + EXPECT_EQ(1u, + RightRefCount(send_once_right.get(), MACH_PORT_RIGHT_SEND_ONCE)); + EXPECT_EQ(0u, DeadNameRightRefCount(send_once_right.get())); } } // namespace diff --git a/util/win/process_info.cc b/util/win/process_info.cc index 7f3ff286..1478d044 100644 --- a/util/win/process_info.cc +++ b/util/win/process_info.cc @@ -600,6 +600,26 @@ ProcessInfo::GetReadableRanges( return GetReadableRangesOfMemoryMap(range, MemoryInfo()); } +bool ProcessInfo::LoggingRangeIsFullyReadable( + const CheckedRange& range) const { + const auto ranges = GetReadableRanges(range); + if (ranges.size() != 1) { + LOG(ERROR) << base::StringPrintf( + "range at 0x%llx, size 0x%llx fully unreadable", + range.base(), + range.size()); + return false; + } + if (ranges[0].base() != range.base() || ranges[0].size() != range.size()) { + LOG(ERROR) << base::StringPrintf( + "some of range at 0x%llx, size 0x%llx unreadable", + range.base(), + range.size()); + return false; + } + return true; +} + const std::vector& ProcessInfo::Handles() const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); if (handles_.empty()) diff --git a/util/win/process_info.h b/util/win/process_info.h index 41b92598..f8ed4ae4 100644 --- a/util/win/process_info.h +++ b/util/win/process_info.h @@ -139,6 +139,16 @@ class ProcessInfo { std::vector> GetReadableRanges( const CheckedRange& range) const; + //! \brief Given a range in the target process, determines if the entire range + //! is readable. + //! + //! \param[in] range The range being inspected. + //! + //! \return `true` if the range is fully readable, otherwise `false` with a + //! message logged. + bool LoggingRangeIsFullyReadable( + const CheckedRange& range) const; + //! \brief Retrieves information about open handles in the target process. const std::vector& Handles() const; diff --git a/util/win/process_info_test.cc b/util/win/process_info_test.cc index 075f8eb4..81e5613e 100644 --- a/util/win/process_info_test.cc +++ b/util/win/process_info_test.cc @@ -623,6 +623,20 @@ TEST(ProcessInfo, Handles) { EXPECT_TRUE(found_mapping_handle); } +TEST(ProcessInfo, OutOfRangeCheck) { + const size_t kAllocationSize = 12345; + scoped_ptr safe_memory(new char[kAllocationSize]); + + ProcessInfo info; + info.Initialize(GetCurrentProcess()); + + EXPECT_TRUE( + info.LoggingRangeIsFullyReadable(CheckedRange( + reinterpret_cast(safe_memory.get()), kAllocationSize))); + EXPECT_FALSE(info.LoggingRangeIsFullyReadable( + CheckedRange(0, 1024))); +} + } // namespace } // namespace test } // namespace crashpad