diff --git a/client/client.gyp b/client/client.gyp index 765a2b47..3524be41 100644 --- a/client/client.gyp +++ b/client/client.gyp @@ -21,6 +21,7 @@ 'target_name': 'crashpad_client', 'type': 'static_library', 'dependencies': [ + '../compat/compat.gyp:crashpad_compat', '../third_party/mini_chromium/mini_chromium.gyp:base', '../util/util.gyp:crashpad_util', ], diff --git a/client/crash_report_database_test.cc b/client/crash_report_database_test.cc index 54dcdd75..ca839f47 100644 --- a/client/crash_report_database_test.cc +++ b/client/crash_report_database_test.cc @@ -555,6 +555,36 @@ TEST_F(CrashReportDatabaseTest, DeleteReport) { db()->LookUpCrashReport(keep_completed.uuid, &keep_completed)); } +TEST_F(CrashReportDatabaseTest, DeleteReportEmptyingDatabase) { + CrashReportDatabase::Report report; + CreateCrashReport(&report); + + EXPECT_TRUE(FileExists(report.file_path)); + + UploadReport(report.uuid, true, "1"); + + EXPECT_EQ(CrashReportDatabase::kNoError, + db()->LookUpCrashReport(report.uuid, &report)); + + EXPECT_TRUE(FileExists(report.file_path)); + + // This causes an empty database to be written, make sure this is handled. + EXPECT_EQ(CrashReportDatabase::kNoError, db()->DeleteReport(report.uuid)); + EXPECT_FALSE(FileExists(report.file_path)); +} + +TEST_F(CrashReportDatabaseTest, ReadEmptyDatabase) { + CrashReportDatabase::Report report; + CreateCrashReport(&report); + EXPECT_EQ(CrashReportDatabase::kNoError, db()->DeleteReport(report.uuid)); + + // Deleting and the creating another report causes an empty database to be + // loaded. Make sure this is handled. + + CrashReportDatabase::Report report2; + CreateCrashReport(&report2); +} + } // namespace } // namespace test } // namespace crashpad diff --git a/client/crash_report_database_win.cc b/client/crash_report_database_win.cc index 5eabdee0..d7968bd1 100644 --- a/client/crash_report_database_win.cc +++ b/client/crash_report_database_win.cc @@ -18,6 +18,8 @@ #include #include +#include + #include "base/logging.h" #include "base/numerics/safe_math.h" #include "base/strings/string16.h" @@ -416,26 +418,29 @@ void Metadata::Read() { return; } - std::vector records(header.num_records); - if (!LoggingReadFile(handle_.get(), &records[0], records_size.ValueOrDie())) { - LOG(ERROR) << "failed to read records"; - return; - } - - std::string string_table = ReadRestOfFileAsString(handle_.get()); - if (string_table.empty() || string_table.back() != '\0') { - LOG(ERROR) << "bad string table"; - return; - } - std::vector reports; - for (const auto& record : records) { - if (record.file_path_index >= string_table.size() || - record.id_index >= string_table.size()) { - LOG(ERROR) << "invalid string table index"; + if (header.num_records > 0) { + std::vector records(header.num_records); + if (!LoggingReadFile( + handle_.get(), &records[0], records_size.ValueOrDie())) { + LOG(ERROR) << "failed to read records"; return; } - reports.push_back(ReportDisk(record, report_dir_, string_table)); + + std::string string_table = ReadRestOfFileAsString(handle_.get()); + if (string_table.empty() || string_table.back() != '\0') { + LOG(ERROR) << "bad string table"; + return; + } + + for (const auto& record : records) { + if (record.file_path_index >= string_table.size() || + record.id_index >= string_table.size()) { + LOG(ERROR) << "invalid string table index"; + return; + } + reports.push_back(ReportDisk(record, report_dir_, string_table)); + } } reports_.swap(reports); } @@ -465,6 +470,9 @@ void Metadata::Write() { return; } + if (num_records == 0) + return; + // Build the records and string table we're going to write. std::string string_table; std::vector records; @@ -815,7 +823,7 @@ scoped_ptr InitializeInternal( scoped_ptr database_win( new CrashReportDatabaseWin(path)); return database_win->Initialize(may_create) - ? database_win.Pass() + ? std::move(database_win) : scoped_ptr(); } diff --git a/client/crashpad_client_mac.cc b/client/crashpad_client_mac.cc index 480828be..cce11d1e 100644 --- a/client/crashpad_client_mac.cc +++ b/client/crashpad_client_mac.cc @@ -20,6 +20,8 @@ #include #include +#include + #include "base/logging.h" #include "base/mac/mach_logging.h" #include "base/posix/eintr_wrapper.h" @@ -158,7 +160,7 @@ class HandlerStarter final : public NotifyServer::DefaultInterface { url, annotations, arguments, - receive_right.Pass(), + std::move(receive_right), handler_restarter.get(), false)) { return base::mac::ScopedMachSendRight(); @@ -538,7 +540,7 @@ bool CrashpadClient::StartHandler( return false; } - SetHandlerMachPort(exception_port.Pass()); + SetHandlerMachPort(std::move(exception_port)); return true; } @@ -548,14 +550,14 @@ bool CrashpadClient::SetHandlerMachService(const std::string& service_name) { return false; } - SetHandlerMachPort(exception_port.Pass()); + SetHandlerMachPort(std::move(exception_port)); return true; } void CrashpadClient::SetHandlerMachPort( base::mac::ScopedMachSendRight exception_port) { DCHECK(exception_port.is_valid()); - exception_port_ = exception_port.Pass(); + exception_port_ = std::move(exception_port); } bool CrashpadClient::UseHandler() { diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc index 40a856f3..c7cb09a9 100644 --- a/client/crashpad_client_win.cc +++ b/client/crashpad_client_win.cc @@ -132,6 +132,18 @@ using ScopedProcThreadAttributeList = base::ScopedGeneric; +bool IsInheritableHandle(HANDLE handle) { + if (!handle || handle == INVALID_HANDLE_VALUE) + return false; + + // File handles (FILE_TYPE_DISK) and pipe handles (FILE_TYPE_PIPE) are known + // to be inheritable. Console handles (FILE_TYPE_CHAR) are not inheritable via + // PROC_THREAD_ATTRIBUTE_HANDLE_LIST. See + // https://crashpad.chromium.org/bug/77. + DWORD handle_type = GetFileType(handle); + return handle_type == FILE_TYPE_DISK || handle_type == FILE_TYPE_PIPE; +} + // Adds |handle| to |handle_list| if it appears valid, and is not already in // |handle_list|. // @@ -142,11 +154,12 @@ using ScopedProcThreadAttributeList = // silently not inherit any handles. // // Use this function to add handles with uncertain validities. -void AddHandleToListIfValid(std::vector* handle_list, HANDLE handle) { +void AddHandleToListIfValidAndInheritable(std::vector* handle_list, + HANDLE handle) { // There doesn't seem to be any documentation of this, but if there's a handle // duplicated in this list, CreateProcess() fails with // ERROR_INVALID_PARAMETER. - if (handle && handle != INVALID_HANDLE_VALUE && + if (IsInheritableHandle(handle) && std::find(handle_list->begin(), handle_list->end(), handle) == handle_list->end()) { handle_list->push_back(handle); @@ -267,9 +280,12 @@ bool CrashpadClient::StartHandler( 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); + AddHandleToListIfValidAndInheritable(&handle_list, + startup_info.StartupInfo.hStdInput); + AddHandleToListIfValidAndInheritable(&handle_list, + startup_info.StartupInfo.hStdOutput); + AddHandleToListIfValidAndInheritable(&handle_list, + startup_info.StartupInfo.hStdError); rv = update_proc_thread_attribute( startup_info.lpAttributeList, 0, diff --git a/client/settings.cc b/client/settings.cc index 32297811..b991a388 100644 --- a/client/settings.cc +++ b/client/settings.cc @@ -235,7 +235,7 @@ Settings::ScopedLockedFileHandle Settings::OpenForWritingAndReadSettings( return ScopedLockedFileHandle(); } - return handle.Pass(); + return handle; } bool Settings::ReadSettings(FileHandle handle, diff --git a/compat/compat.gyp b/compat/compat.gyp index 176fe4f9..f1cd4933 100644 --- a/compat/compat.gyp +++ b/compat/compat.gyp @@ -22,12 +22,14 @@ 'type': 'static_library', 'sources': [ 'mac/AvailabilityMacros.h', - 'mac/kern/exc_resource.h' + 'mac/kern/exc_resource.h', 'mac/mach/mach.h', 'mac/mach-o/getsect.cc', 'mac/mach-o/getsect.h', 'mac/mach-o/loader.h', 'mac/sys/resource.h', + 'non_cxx11_lib/type_traits', + 'non_cxx11_lib/utility', 'non_mac/mach/mach.h', 'non_win/dbghelp.h', 'non_win/minwinbase.h', @@ -50,10 +52,12 @@ ], 'include_dirs': [ 'mac', + 'non_cxx11_lib', ], 'direct_dependent_settings': { 'include_dirs': [ 'mac', + 'non_cxx11_lib', ], }, }], diff --git a/compat/non_cxx11_lib/type_traits b/compat/non_cxx11_lib/type_traits new file mode 100644 index 00000000..9f11fd4a --- /dev/null +++ b/compat/non_cxx11_lib/type_traits @@ -0,0 +1,37 @@ +// Copyright 2015 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_COMPAT_NON_CXX11_LIB_TYPE_TRAITS_ +#define CRASHPAD_COMPAT_NON_CXX11_LIB_TYPE_TRAITS_ + +#include "util/stdlib/cxx.h" + +#if CXX_LIBRARY_VERSION >= 2011 + +#include_next + +#else + +namespace std { + +template +struct remove_reference { using type = T; }; +template +struct remove_reference { using type = T; }; + +} // namespace std + +#endif // CXX_LIBRARY_VERSION + +#endif // CRASHPAD_COMPAT_NON_CXX11_LIB_TYPE_TRAITS_ diff --git a/compat/non_cxx11_lib/utility b/compat/non_cxx11_lib/utility new file mode 100644 index 00000000..8aa3de1c --- /dev/null +++ b/compat/non_cxx11_lib/utility @@ -0,0 +1,46 @@ +// 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_COMPAT_NON_CXX11_LIB_UTILITY_ +#define CRASHPAD_COMPAT_NON_CXX11_LIB_UTILITY_ + +#include_next + +#include "util/stdlib/cxx.h" + +#if CXX_LIBRARY_VERSION < 2011 + +#include + +namespace std { + +template +T&& forward(typename remove_reference::type& t) noexcept { + return static_cast(t); +} +template +T&& forward(typename remove_reference::type&& t) noexcept { + return static_cast(t); +} + +template +typename remove_reference::type&& move(T&& t) noexcept { + return static_cast::type&&>(t); +} + +} // namespace std + +#endif // CXX_LIBRARY_VERSION + +#endif // CRASHPAD_COMPAT_NON_CXX11_LIB_UTILITY_ diff --git a/doc/support/compat.sh b/doc/support/compat.sh index e7135239..8c5d30d3 100644 --- a/doc/support/compat.sh +++ b/doc/support/compat.sh @@ -1,5 +1,3 @@ -#!/bin/bash - # Copyright 2015 The Crashpad Authors. All rights reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/handler/crash_report_upload_thread.cc b/handler/crash_report_upload_thread.cc index b88f3e1b..7d29aa11 100644 --- a/handler/crash_report_upload_thread.cc +++ b/handler/crash_report_upload_thread.cc @@ -366,7 +366,7 @@ CrashReportUploadThread::UploadResult CrashReportUploadThread::UploadReport( HTTPHeaders::value_type content_type = http_multipart_builder.GetContentType(); http_transport->SetHeader(content_type.first, content_type.second); - http_transport->SetBodyStream(http_multipart_builder.GetBodyStream().Pass()); + http_transport->SetBodyStream(http_multipart_builder.GetBodyStream()); // TODO(mark): The timeout should be configurable by the client. http_transport->SetTimeout(60.0); // 1 minute. diff --git a/handler/handler_main.cc b/handler/handler_main.cc index 578b8862..7ad13f6f 100644 --- a/handler/handler_main.cc +++ b/handler/handler_main.cc @@ -19,6 +19,7 @@ #include #include +#include #include "base/auto_reset.h" #include "base/files/file_path.h" @@ -314,7 +315,7 @@ int HandlerMain(int argc, char* argv[]) { } ExceptionHandlerServer exception_handler_server( - receive_right.Pass(), !options.mach_service.empty()); + std::move(receive_right), !options.mach_service.empty()); base::AutoReset reset_g_exception_handler_server( &g_exception_handler_server, &exception_handler_server); @@ -336,9 +337,12 @@ int HandlerMain(int argc, char* argv[]) { reset_sigterm.reset(&old_sa); } #elif defined(OS_WIN) + // Shut down as late as possible relative to programs we're watching. + if (!SetProcessShutdownParameters(0x100, SHUTDOWN_NORETRY)) + PLOG(ERROR) << "SetProcessShutdownParameters"; + 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) { diff --git a/handler/mac/exception_handler_server.cc b/handler/mac/exception_handler_server.cc index 845b34dd..38a016e9 100644 --- a/handler/mac/exception_handler_server.cc +++ b/handler/mac/exception_handler_server.cc @@ -14,6 +14,8 @@ #include "handler/mac/exception_handler_server.h" +#include + #include "base/logging.h" #include "base/mac/mach_logging.h" #include "util/mach/composite_mach_message_server.h" @@ -183,7 +185,7 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface, ExceptionHandlerServer::ExceptionHandlerServer( base::mac::ScopedMachReceiveRight receive_port, bool launchd) - : receive_port_(receive_port.Pass()), + : receive_port_(std::move(receive_port)), notify_port_(NewMachPort(MACH_PORT_RIGHT_RECEIVE)), launchd_(launchd) { CHECK(receive_port_.is_valid()); diff --git a/handler/win/z7_test.dll b/handler/win/z7_test.dll old mode 100644 new mode 100755 diff --git a/minidump/minidump_crashpad_info_writer.cc b/minidump/minidump_crashpad_info_writer.cc index 9314224b..2f2cd552 100644 --- a/minidump/minidump_crashpad_info_writer.cc +++ b/minidump/minidump_crashpad_info_writer.cc @@ -14,6 +14,8 @@ #include "minidump/minidump_crashpad_info_writer.h" +#include + #include "base/logging.h" #include "minidump/minidump_module_crashpad_info_writer.h" #include "minidump/minidump_simple_string_dictionary_writer.h" @@ -51,14 +53,14 @@ void MinidumpCrashpadInfoWriter::InitializeFromSnapshot( simple_annotations->InitializeFromMap( process_snapshot->AnnotationsSimpleMap()); if (simple_annotations->IsUseful()) { - SetSimpleAnnotations(simple_annotations.Pass()); + SetSimpleAnnotations(std::move(simple_annotations)); } auto modules = make_scoped_ptr(new MinidumpModuleCrashpadInfoListWriter()); modules->InitializeFromSnapshot(process_snapshot->Modules()); if (modules->IsUseful()) { - SetModuleList(modules.Pass()); + SetModuleList(std::move(modules)); } } @@ -78,14 +80,14 @@ void MinidumpCrashpadInfoWriter::SetSimpleAnnotations( scoped_ptr simple_annotations) { DCHECK_EQ(state(), kStateMutable); - simple_annotations_ = simple_annotations.Pass(); + simple_annotations_ = std::move(simple_annotations); } void MinidumpCrashpadInfoWriter::SetModuleList( scoped_ptr module_list) { DCHECK_EQ(state(), kStateMutable); - module_list_ = module_list.Pass(); + module_list_ = std::move(module_list); } bool MinidumpCrashpadInfoWriter::Freeze() { diff --git a/minidump/minidump_crashpad_info_writer_test.cc b/minidump/minidump_crashpad_info_writer_test.cc index 06733edb..d5ff65ce 100644 --- a/minidump/minidump_crashpad_info_writer_test.cc +++ b/minidump/minidump_crashpad_info_writer_test.cc @@ -19,6 +19,7 @@ #include #include +#include #include "gtest/gtest.h" #include "minidump/minidump_extensions.h" @@ -67,7 +68,7 @@ TEST(MinidumpCrashpadInfoWriter, Empty) { auto crashpad_info_writer = make_scoped_ptr(new MinidumpCrashpadInfoWriter()); EXPECT_FALSE(crashpad_info_writer->IsUseful()); - minidump_file_writer.AddStream(crashpad_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(crashpad_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -102,7 +103,7 @@ TEST(MinidumpCrashpadInfoWriter, ReportAndClientID) { EXPECT_TRUE(crashpad_info_writer->IsUseful()); - minidump_file_writer.AddStream(crashpad_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(crashpad_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -137,13 +138,13 @@ TEST(MinidumpCrashpadInfoWriter, SimpleAnnotations) { make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); simple_string_dictionary_entry_writer->SetKeyValue(kKey, kValue); simple_string_dictionary_writer->AddEntry( - simple_string_dictionary_entry_writer.Pass()); + std::move(simple_string_dictionary_entry_writer)); crashpad_info_writer->SetSimpleAnnotations( - simple_string_dictionary_writer.Pass()); + std::move(simple_string_dictionary_writer)); EXPECT_TRUE(crashpad_info_writer->IsUseful()); - minidump_file_writer.AddStream(crashpad_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(crashpad_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -177,12 +178,13 @@ TEST(MinidumpCrashpadInfoWriter, CrashpadModuleList) { auto module_list_writer = make_scoped_ptr(new MinidumpModuleCrashpadInfoListWriter()); auto module_writer = make_scoped_ptr(new MinidumpModuleCrashpadInfoWriter()); - module_list_writer->AddModule(module_writer.Pass(), kMinidumpModuleListIndex); - crashpad_info_writer->SetModuleList(module_list_writer.Pass()); + module_list_writer->AddModule(std::move(module_writer), + kMinidumpModuleListIndex); + crashpad_info_writer->SetModuleList(std::move(module_list_writer)); EXPECT_TRUE(crashpad_info_writer->IsUseful()); - minidump_file_writer.AddStream(crashpad_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(crashpad_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -232,7 +234,7 @@ TEST(MinidumpCrashpadInfoWriter, InitializeFromSnapshot) { auto process_snapshot = make_scoped_ptr(new TestProcessSnapshot()); auto module_snapshot = make_scoped_ptr(new TestModuleSnapshot()); - process_snapshot->AddModule(module_snapshot.Pass()); + process_snapshot->AddModule(std::move(module_snapshot)); auto info_writer = make_scoped_ptr(new MinidumpCrashpadInfoWriter()); info_writer->InitializeFromSnapshot(process_snapshot.get()); @@ -251,14 +253,14 @@ TEST(MinidumpCrashpadInfoWriter, InitializeFromSnapshot) { module_snapshot.reset(new TestModuleSnapshot()); std::vector annotations_list(1, std::string(kEntry)); module_snapshot->SetAnnotationsVector(annotations_list); - process_snapshot->AddModule(module_snapshot.Pass()); + process_snapshot->AddModule(std::move(module_snapshot)); info_writer.reset(new MinidumpCrashpadInfoWriter()); info_writer->InitializeFromSnapshot(process_snapshot.get()); EXPECT_TRUE(info_writer->IsUseful()); MinidumpFileWriter minidump_file_writer; - minidump_file_writer.AddStream(info_writer.Pass()); + minidump_file_writer.AddStream(std::move(info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); diff --git a/minidump/minidump_exception_writer.cc b/minidump/minidump_exception_writer.cc index 3d0c3414..cd471b9c 100644 --- a/minidump/minidump_exception_writer.cc +++ b/minidump/minidump_exception_writer.cc @@ -16,6 +16,8 @@ #include +#include + #include "base/logging.h" #include "base/numerics/safe_conversions.h" #include "minidump/minidump_context_writer.h" @@ -48,14 +50,14 @@ void MinidumpExceptionWriter::InitializeFromSnapshot( scoped_ptr context = MinidumpContextWriter::CreateFromSnapshot(exception_snapshot->Context()); - SetContext(context.Pass()); + SetContext(std::move(context)); } void MinidumpExceptionWriter::SetContext( scoped_ptr context) { DCHECK_EQ(state(), kStateMutable); - context_ = context.Pass(); + context_ = std::move(context); } void MinidumpExceptionWriter::SetExceptionInformation( diff --git a/minidump/minidump_exception_writer_test.cc b/minidump/minidump_exception_writer_test.cc index 881c36eb..bc3bfec9 100644 --- a/minidump/minidump_exception_writer_test.cc +++ b/minidump/minidump_exception_writer_test.cc @@ -20,6 +20,7 @@ #include #include +#include #include #include "gtest/gtest.h" @@ -104,9 +105,9 @@ TEST(MinidumpExceptionWriter, Minimal) { auto context_x86_writer = make_scoped_ptr(new MinidumpContextX86Writer()); InitializeMinidumpContextX86(context_x86_writer->context(), kSeed); - exception_writer->SetContext(context_x86_writer.Pass()); + exception_writer->SetContext(std::move(context_x86_writer)); - minidump_file_writer.AddStream(exception_writer.Pass()); + minidump_file_writer.AddStream(std::move(exception_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -144,7 +145,7 @@ TEST(MinidumpExceptionWriter, Standard) { auto context_x86_writer = make_scoped_ptr(new MinidumpContextX86Writer()); InitializeMinidumpContextX86(context_x86_writer->context(), kSeed); - exception_writer->SetContext(context_x86_writer.Pass()); + exception_writer->SetContext(std::move(context_x86_writer)); exception_writer->SetThreadID(kThreadID); exception_writer->SetExceptionCode(kExceptionCode); @@ -165,7 +166,7 @@ TEST(MinidumpExceptionWriter, Standard) { exception_information.push_back(kExceptionInformation2); exception_writer->SetExceptionInformation(exception_information); - minidump_file_writer.AddStream(exception_writer.Pass()); + minidump_file_writer.AddStream(std::move(exception_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -238,7 +239,7 @@ TEST(MinidumpExceptionWriter, InitializeFromSnapshot) { exception_writer->InitializeFromSnapshot(&exception_snapshot, thread_id_map); MinidumpFileWriter minidump_file_writer; - minidump_file_writer.AddStream(exception_writer.Pass()); + minidump_file_writer.AddStream(std::move(exception_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -260,7 +261,7 @@ TEST(MinidumpExceptionWriterDeathTest, NoContext) { MinidumpFileWriter minidump_file_writer; auto exception_writer = make_scoped_ptr(new MinidumpExceptionWriter()); - minidump_file_writer.AddStream(exception_writer.Pass()); + minidump_file_writer.AddStream(std::move(exception_writer)); StringFile string_file; ASSERT_DEATH_CHECK(minidump_file_writer.WriteEverything(&string_file), diff --git a/minidump/minidump_file_writer.cc b/minidump/minidump_file_writer.cc index 2b13433a..b9a9eeab 100644 --- a/minidump/minidump_file_writer.cc +++ b/minidump/minidump_file_writer.cc @@ -14,6 +14,8 @@ #include "minidump/minidump_file_writer.h" +#include + #include "base/logging.h" #include "minidump/minidump_crashpad_info_writer.h" #include "minidump/minidump_exception_writer.h" @@ -67,11 +69,11 @@ void MinidumpFileWriter::InitializeFromSnapshot( const SystemSnapshot* system_snapshot = process_snapshot->System(); auto system_info = make_scoped_ptr(new MinidumpSystemInfoWriter()); system_info->InitializeFromSnapshot(system_snapshot); - AddStream(system_info.Pass()); + AddStream(std::move(system_info)); auto misc_info = make_scoped_ptr(new MinidumpMiscInfoWriter()); misc_info->InitializeFromSnapshot(process_snapshot); - AddStream(misc_info.Pass()); + AddStream(std::move(misc_info)); auto memory_list = make_scoped_ptr(new MinidumpMemoryListWriter()); auto thread_list = make_scoped_ptr(new MinidumpThreadListWriter()); @@ -79,18 +81,18 @@ void MinidumpFileWriter::InitializeFromSnapshot( MinidumpThreadIDMap thread_id_map; thread_list->InitializeFromSnapshot(process_snapshot->Threads(), &thread_id_map); - AddStream(thread_list.Pass()); + AddStream(std::move(thread_list)); const ExceptionSnapshot* exception_snapshot = process_snapshot->Exception(); if (exception_snapshot) { auto exception = make_scoped_ptr(new MinidumpExceptionWriter()); exception->InitializeFromSnapshot(exception_snapshot, thread_id_map); - AddStream(exception.Pass()); + AddStream(std::move(exception)); } auto module_list = make_scoped_ptr(new MinidumpModuleListWriter()); module_list->InitializeFromSnapshot(process_snapshot->Modules()); - AddStream(module_list.Pass()); + AddStream(std::move(module_list)); auto crashpad_info = make_scoped_ptr(new MinidumpCrashpadInfoWriter()); crashpad_info->InitializeFromSnapshot(process_snapshot); @@ -98,7 +100,7 @@ void MinidumpFileWriter::InitializeFromSnapshot( // Since the MinidumpCrashpadInfo stream is an extension, it’s safe to not add // it to the minidump file if it wouldn’t carry any useful information. if (crashpad_info->IsUseful()) { - AddStream(crashpad_info.Pass()); + AddStream(std::move(crashpad_info)); } std::vector memory_map_snapshot = @@ -106,19 +108,19 @@ void MinidumpFileWriter::InitializeFromSnapshot( if (!memory_map_snapshot.empty()) { auto memory_info_list = make_scoped_ptr(new MinidumpMemoryInfoListWriter()); memory_info_list->InitializeFromSnapshot(memory_map_snapshot); - AddStream(memory_info_list.Pass()); + AddStream(std::move(memory_info_list)); } 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()); + AddStream(std::move(handle_data_writer)); } memory_list->AddFromSnapshot(process_snapshot->ExtraMemory()); - AddStream(memory_list.Pass()); + AddStream(std::move(memory_list)); } void MinidumpFileWriter::SetTimestamp(time_t timestamp) { diff --git a/minidump/minidump_file_writer_test.cc b/minidump/minidump_file_writer_test.cc index 44c39530..314c758a 100644 --- a/minidump/minidump_file_writer_test.cc +++ b/minidump/minidump_file_writer_test.cc @@ -18,6 +18,7 @@ #include #include +#include #include "base/basictypes.h" #include "base/compiler_specific.h" @@ -96,7 +97,7 @@ TEST(MinidumpFileWriter, OneStream) { const uint8_t kStreamValue = 0x5a; auto stream = make_scoped_ptr(new TestStream(kStreamType, kStreamSize, kStreamValue)); - minidump_file.AddStream(stream.Pass()); + minidump_file.AddStream(std::move(stream)); StringFile string_file; ASSERT_TRUE(minidump_file.WriteEverything(&string_file)); @@ -135,7 +136,7 @@ TEST(MinidumpFileWriter, ThreeStreams) { const uint8_t kStream0Value = 0x5a; auto stream0 = make_scoped_ptr( new TestStream(kStream0Type, kStream0Size, kStream0Value)); - minidump_file.AddStream(stream0.Pass()); + minidump_file.AddStream(std::move(stream0)); // Make the second stream’s type be a smaller quantity than the first stream’s // to test that the streams show up in the order that they were added, not in @@ -145,14 +146,14 @@ TEST(MinidumpFileWriter, ThreeStreams) { const uint8_t kStream1Value = 0xa5; auto stream1 = make_scoped_ptr( new TestStream(kStream1Type, kStream1Size, kStream1Value)); - minidump_file.AddStream(stream1.Pass()); + minidump_file.AddStream(std::move(stream1)); const size_t kStream2Size = 1; const MinidumpStreamType kStream2Type = static_cast(0x7e); const uint8_t kStream2Value = 0x36; auto stream2 = make_scoped_ptr( new TestStream(kStream2Type, kStream2Size, kStream2Value)); - minidump_file.AddStream(stream2.Pass()); + minidump_file.AddStream(std::move(stream2)); StringFile string_file; ASSERT_TRUE(minidump_file.WriteEverything(&string_file)); @@ -219,7 +220,7 @@ TEST(MinidumpFileWriter, ZeroLengthStream) { const size_t kStreamSize = 0; const MinidumpStreamType kStreamType = static_cast(0x4d); auto stream = make_scoped_ptr(new TestStream(kStreamType, kStreamSize, 0)); - minidump_file.AddStream(stream.Pass()); + minidump_file.AddStream(std::move(stream)); StringFile string_file; ASSERT_TRUE(minidump_file.WriteEverything(&string_file)); @@ -251,7 +252,7 @@ TEST(MinidumpFileWriter, InitializeFromSnapshot_Basic) { auto system_snapshot = make_scoped_ptr(new TestSystemSnapshot()); system_snapshot->SetCPUArchitecture(kCPUArchitectureX86_64); system_snapshot->SetOperatingSystem(SystemSnapshot::kOperatingSystemMacOSX); - process_snapshot.SetSystem(system_snapshot.Pass()); + process_snapshot.SetSystem(std::move(system_snapshot)); auto peb_snapshot = make_scoped_ptr(new TestMemorySnapshot()); const uint64_t kPebAddress = 0x07f90000; @@ -259,7 +260,7 @@ TEST(MinidumpFileWriter, InitializeFromSnapshot_Basic) { const size_t kPebSize = 0x280; peb_snapshot->SetSize(kPebSize); peb_snapshot->SetValue('p'); - process_snapshot.AddExtraMemory(peb_snapshot.Pass()); + process_snapshot.AddExtraMemory(std::move(peb_snapshot)); MinidumpFileWriter minidump_file_writer; minidump_file_writer.InitializeFromSnapshot(&process_snapshot); @@ -315,22 +316,22 @@ TEST(MinidumpFileWriter, InitializeFromSnapshot_Exception) { auto system_snapshot = make_scoped_ptr(new TestSystemSnapshot()); system_snapshot->SetCPUArchitecture(kCPUArchitectureX86_64); system_snapshot->SetOperatingSystem(SystemSnapshot::kOperatingSystemMacOSX); - process_snapshot.SetSystem(system_snapshot.Pass()); + process_snapshot.SetSystem(std::move(system_snapshot)); auto thread_snapshot = make_scoped_ptr(new TestThreadSnapshot()); InitializeCPUContextX86_64(thread_snapshot->MutableContext(), 5); - process_snapshot.AddThread(thread_snapshot.Pass()); + process_snapshot.AddThread(std::move(thread_snapshot)); auto exception_snapshot = make_scoped_ptr(new TestExceptionSnapshot()); InitializeCPUContextX86_64(exception_snapshot->MutableContext(), 11); - process_snapshot.SetException(exception_snapshot.Pass()); + process_snapshot.SetException(std::move(exception_snapshot)); // The module does not have anything that needs to be represented in a // MinidumpModuleCrashpadInfo structure, so no such structure is expected to // be present, which will in turn suppress the addition of a // MinidumpCrashpadInfo stream. auto module_snapshot = make_scoped_ptr(new TestModuleSnapshot()); - process_snapshot.AddModule(module_snapshot.Pass()); + process_snapshot.AddModule(std::move(module_snapshot)); MinidumpFileWriter minidump_file_writer; minidump_file_writer.InitializeFromSnapshot(&process_snapshot); @@ -379,22 +380,22 @@ TEST(MinidumpFileWriter, InitializeFromSnapshot_CrashpadInfo) { auto system_snapshot = make_scoped_ptr(new TestSystemSnapshot()); system_snapshot->SetCPUArchitecture(kCPUArchitectureX86_64); system_snapshot->SetOperatingSystem(SystemSnapshot::kOperatingSystemMacOSX); - process_snapshot.SetSystem(system_snapshot.Pass()); + process_snapshot.SetSystem(std::move(system_snapshot)); auto thread_snapshot = make_scoped_ptr(new TestThreadSnapshot()); InitializeCPUContextX86_64(thread_snapshot->MutableContext(), 5); - process_snapshot.AddThread(thread_snapshot.Pass()); + process_snapshot.AddThread(std::move(thread_snapshot)); auto exception_snapshot = make_scoped_ptr(new TestExceptionSnapshot()); InitializeCPUContextX86_64(exception_snapshot->MutableContext(), 11); - process_snapshot.SetException(exception_snapshot.Pass()); + process_snapshot.SetException(std::move(exception_snapshot)); // The module needs an annotation for the MinidumpCrashpadInfo stream to be // considered useful and be included. auto module_snapshot = make_scoped_ptr(new TestModuleSnapshot()); std::vector annotations_list(1, std::string("annotation")); module_snapshot->SetAnnotationsVector(annotations_list); - process_snapshot.AddModule(module_snapshot.Pass()); + process_snapshot.AddModule(std::move(module_snapshot)); MinidumpFileWriter minidump_file_writer; minidump_file_writer.InitializeFromSnapshot(&process_snapshot); @@ -445,7 +446,7 @@ TEST(MinidumpFileWriterDeathTest, SameStreamType) { const uint8_t kStream0Value = 0x5a; auto stream0 = make_scoped_ptr( new TestStream(kStream0Type, kStream0Size, kStream0Value)); - minidump_file.AddStream(stream0.Pass()); + minidump_file.AddStream(std::move(stream0)); // It is an error to add a second stream of the same type. const size_t kStream1Size = 3; @@ -453,7 +454,7 @@ TEST(MinidumpFileWriterDeathTest, SameStreamType) { const uint8_t kStream1Value = 0xa5; auto stream1 = make_scoped_ptr( new TestStream(kStream1Type, kStream1Size, kStream1Value)); - ASSERT_DEATH_CHECK(minidump_file.AddStream(stream1.Pass()), + ASSERT_DEATH_CHECK(minidump_file.AddStream(std::move(stream1)), "already present"); } diff --git a/minidump/minidump_handle_writer_test.cc b/minidump/minidump_handle_writer_test.cc index e8f23eb5..2a5445d7 100644 --- a/minidump/minidump_handle_writer_test.cc +++ b/minidump/minidump_handle_writer_test.cc @@ -15,6 +15,7 @@ #include "minidump/minidump_handle_writer.h" #include +#include #include "base/strings/utf_string_conversions.h" #include "gtest/gtest.h" @@ -57,7 +58,7 @@ void GetHandleDataStream( TEST(MinidumpHandleDataWriter, Empty) { MinidumpFileWriter minidump_file_writer; auto handle_data_writer = make_scoped_ptr(new MinidumpHandleDataWriter()); - minidump_file_writer.AddStream(handle_data_writer.Pass()); + minidump_file_writer.AddStream(std::move(handle_data_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -90,7 +91,7 @@ TEST(MinidumpHandleDataWriter, OneHandle) { handle_data_writer->InitializeFromSnapshot(snapshot); - minidump_file_writer.AddStream(handle_data_writer.Pass()); + minidump_file_writer.AddStream(std::move(handle_data_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -148,7 +149,7 @@ TEST(MinidumpHandleDataWriter, RepeatedTypeName) { handle_data_writer->InitializeFromSnapshot(snapshot); - minidump_file_writer.AddStream(handle_data_writer.Pass()); + minidump_file_writer.AddStream(std::move(handle_data_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); diff --git a/minidump/minidump_memory_info_writer_test.cc b/minidump/minidump_memory_info_writer_test.cc index b2ff4a86..f59cbb85 100644 --- a/minidump/minidump_memory_info_writer_test.cc +++ b/minidump/minidump_memory_info_writer_test.cc @@ -15,6 +15,7 @@ #include "minidump/minidump_memory_info_writer.h" #include +#include #include "gtest/gtest.h" #include "minidump/minidump_file_writer.h" @@ -58,7 +59,7 @@ TEST(MinidumpMemoryInfoWriter, Empty) { MinidumpFileWriter minidump_file_writer; auto memory_info_list_writer = make_scoped_ptr(new MinidumpMemoryInfoListWriter()); - minidump_file_writer.AddStream(memory_info_list_writer.Pass()); + minidump_file_writer.AddStream(std::move(memory_info_list_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -95,7 +96,7 @@ TEST(MinidumpMemoryInfoWriter, OneRegion) { memory_map.push_back(memory_map_region.get()); memory_info_list_writer->InitializeFromSnapshot(memory_map); - minidump_file_writer.AddStream(memory_info_list_writer.Pass()); + minidump_file_writer.AddStream(std::move(memory_info_list_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); diff --git a/minidump/minidump_memory_writer.cc b/minidump/minidump_memory_writer.cc index 606623e4..35286556 100644 --- a/minidump/minidump_memory_writer.cc +++ b/minidump/minidump_memory_writer.cc @@ -14,6 +14,8 @@ #include "minidump/minidump_memory_writer.h" +#include + #include "base/auto_reset.h" #include "base/logging.h" #include "snapshot/memory_snapshot.h" @@ -177,7 +179,7 @@ void MinidumpMemoryListWriter::AddFromSnapshot( for (const MemorySnapshot* memory_snapshot : memory_snapshots) { scoped_ptr memory = MinidumpMemoryWriter::CreateFromSnapshot(memory_snapshot); - AddMemory(memory.Pass()); + AddMemory(std::move(memory)); } } diff --git a/minidump/minidump_memory_writer_test.cc b/minidump/minidump_memory_writer_test.cc index 6292ea0a..cefe7475 100644 --- a/minidump/minidump_memory_writer_test.cc +++ b/minidump/minidump_memory_writer_test.cc @@ -18,6 +18,8 @@ #include #include +#include + #include "base/basictypes.h" #include "base/format_macros.h" #include "base/strings/stringprintf.h" @@ -80,7 +82,7 @@ TEST(MinidumpMemoryWriter, EmptyMemoryList) { MinidumpFileWriter minidump_file_writer; auto memory_list_writer = make_scoped_ptr(new MinidumpMemoryListWriter()); - minidump_file_writer.AddStream(memory_list_writer.Pass()); + minidump_file_writer.AddStream(std::move(memory_list_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -106,9 +108,9 @@ TEST(MinidumpMemoryWriter, OneMemoryRegion) { auto memory_writer = make_scoped_ptr( new TestMinidumpMemoryWriter(kBaseAddress, kSize, kValue)); - memory_list_writer->AddMemory(memory_writer.Pass()); + memory_list_writer->AddMemory(std::move(memory_writer)); - minidump_file_writer.AddStream(memory_list_writer.Pass()); + minidump_file_writer.AddStream(std::move(memory_list_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -144,12 +146,12 @@ TEST(MinidumpMemoryWriter, TwoMemoryRegions) { auto memory_writer_0 = make_scoped_ptr( new TestMinidumpMemoryWriter(kBaseAddress0, kSize0, kValue0)); - memory_list_writer->AddMemory(memory_writer_0.Pass()); + memory_list_writer->AddMemory(std::move(memory_writer_0)); auto memory_writer_1 = make_scoped_ptr( new TestMinidumpMemoryWriter(kBaseAddress1, kSize1, kValue1)); - memory_list_writer->AddMemory(memory_writer_1.Pass()); + memory_list_writer->AddMemory(std::move(memory_writer_1)); - minidump_file_writer.AddStream(memory_list_writer.Pass()); + minidump_file_writer.AddStream(std::move(memory_list_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -248,7 +250,7 @@ TEST(MinidumpMemoryWriter, ExtraMemory) { auto memory_list_writer = make_scoped_ptr(new MinidumpMemoryListWriter()); memory_list_writer->AddExtraMemory(test_memory_stream->memory()); - minidump_file_writer.AddStream(test_memory_stream.Pass()); + minidump_file_writer.AddStream(std::move(test_memory_stream)); const uint64_t kBaseAddress1 = 0x2000; const size_t kSize1 = 0x0400; @@ -256,9 +258,9 @@ TEST(MinidumpMemoryWriter, ExtraMemory) { auto memory_writer = make_scoped_ptr( new TestMinidumpMemoryWriter(kBaseAddress1, kSize1, kValue1)); - memory_list_writer->AddMemory(memory_writer.Pass()); + memory_list_writer->AddMemory(std::move(memory_writer)); - minidump_file_writer.AddStream(memory_list_writer.Pass()); + minidump_file_writer.AddStream(std::move(memory_list_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -336,7 +338,7 @@ TEST(MinidumpMemoryWriter, AddFromSnapshot) { memory_list_writer->AddFromSnapshot(memory_snapshots); MinidumpFileWriter minidump_file_writer; - minidump_file_writer.AddStream(memory_list_writer.Pass()); + minidump_file_writer.AddStream(std::move(memory_list_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); diff --git a/minidump/minidump_misc_info_writer_test.cc b/minidump/minidump_misc_info_writer_test.cc index 5e5fa926..de8cde3b 100644 --- a/minidump/minidump_misc_info_writer_test.cc +++ b/minidump/minidump_misc_info_writer_test.cc @@ -19,6 +19,7 @@ #include #include +#include #include "base/basictypes.h" #include "base/compiler_specific.h" @@ -168,7 +169,7 @@ TEST(MinidumpMiscInfoWriter, Empty) { MinidumpFileWriter minidump_file_writer; auto misc_info_writer = make_scoped_ptr(new MinidumpMiscInfoWriter()); - minidump_file_writer.AddStream(misc_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(misc_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -189,7 +190,7 @@ TEST(MinidumpMiscInfoWriter, ProcessId) { misc_info_writer->SetProcessID(kProcessId); - minidump_file_writer.AddStream(misc_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(misc_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -215,7 +216,7 @@ TEST(MinidumpMiscInfoWriter, ProcessTimes) { misc_info_writer->SetProcessTimes( kProcessCreateTime, kProcessUserTime, kProcessKernelTime); - minidump_file_writer.AddStream(misc_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(misc_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -248,7 +249,7 @@ TEST(MinidumpMiscInfoWriter, ProcessorPowerInfo) { kProcessorMaxIdleState, kProcessorCurrentIdleState); - minidump_file_writer.AddStream(misc_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(misc_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -275,7 +276,7 @@ TEST(MinidumpMiscInfoWriter, ProcessIntegrityLevel) { misc_info_writer->SetProcessIntegrityLevel(kProcessIntegrityLevel); - minidump_file_writer.AddStream(misc_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(misc_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -298,7 +299,7 @@ TEST(MinidumpMiscInfoWriter, ProcessExecuteFlags) { misc_info_writer->SetProcessExecuteFlags(kProcessExecuteFlags); - minidump_file_writer.AddStream(misc_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(misc_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -321,7 +322,7 @@ TEST(MinidumpMiscInfoWriter, ProtectedProcess) { misc_info_writer->SetProtectedProcess(kProtectedProcess); - minidump_file_writer.AddStream(misc_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(misc_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -358,7 +359,7 @@ TEST(MinidumpMiscInfoWriter, TimeZone) { kDaylightDate, kDaylightBias); - minidump_file_writer.AddStream(misc_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(misc_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -419,7 +420,7 @@ TEST(MinidumpMiscInfoWriter, TimeZoneStringsOverflow) { kSystemTimeZero, kDaylightBias); - minidump_file_writer.AddStream(misc_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(misc_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -460,7 +461,7 @@ TEST(MinidumpMiscInfoWriter, BuildStrings) { misc_info_writer->SetBuildString(kBuildString, kDebugBuildString); - minidump_file_writer.AddStream(misc_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(misc_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -497,7 +498,7 @@ TEST(MinidumpMiscInfoWriter, BuildStringsOverflow) { misc_info_writer->SetBuildString(build_string, debug_build_string); - minidump_file_writer.AddStream(misc_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(misc_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -567,7 +568,7 @@ TEST(MinidumpMiscInfoWriter, Everything) { kDaylightBias); misc_info_writer->SetBuildString(kBuildString, kDebugBuildString); - minidump_file_writer.AddStream(misc_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(misc_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -701,13 +702,13 @@ TEST(MinidumpMiscInfoWriter, InitializeFromSnapshot) { system_snapshot->SetOSVersionFull(kOSVersionFull); system_snapshot->SetMachineDescription(kMachineDescription); - process_snapshot.SetSystem(system_snapshot.Pass()); + process_snapshot.SetSystem(std::move(system_snapshot)); auto misc_info_writer = make_scoped_ptr(new MinidumpMiscInfoWriter()); misc_info_writer->InitializeFromSnapshot(&process_snapshot); MinidumpFileWriter minidump_file_writer; - minidump_file_writer.AddStream(misc_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(misc_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); diff --git a/minidump/minidump_module_crashpad_info_writer.cc b/minidump/minidump_module_crashpad_info_writer.cc index 8868badb..c4a09c33 100644 --- a/minidump/minidump_module_crashpad_info_writer.cc +++ b/minidump/minidump_module_crashpad_info_writer.cc @@ -16,6 +16,8 @@ #include +#include + #include "base/logging.h" #include "minidump/minidump_simple_string_dictionary_writer.h" #include "snapshot/module_snapshot.h" @@ -44,7 +46,7 @@ void MinidumpModuleCrashpadInfoWriter::InitializeFromSnapshot( auto list_annotations = make_scoped_ptr(new MinidumpUTF8StringListWriter()); list_annotations->InitializeFromVector(module_snapshot->AnnotationsVector()); if (list_annotations->IsUseful()) { - SetListAnnotations(list_annotations.Pass()); + SetListAnnotations(std::move(list_annotations)); } auto simple_annotations = @@ -52,7 +54,7 @@ void MinidumpModuleCrashpadInfoWriter::InitializeFromSnapshot( simple_annotations->InitializeFromMap( module_snapshot->AnnotationsSimpleMap()); if (simple_annotations->IsUseful()) { - SetSimpleAnnotations(simple_annotations.Pass()); + SetSimpleAnnotations(std::move(simple_annotations)); } } @@ -60,14 +62,14 @@ void MinidumpModuleCrashpadInfoWriter::SetListAnnotations( scoped_ptr list_annotations) { DCHECK_EQ(state(), kStateMutable); - list_annotations_ = list_annotations.Pass(); + list_annotations_ = std::move(list_annotations); } void MinidumpModuleCrashpadInfoWriter::SetSimpleAnnotations( scoped_ptr simple_annotations) { DCHECK_EQ(state(), kStateMutable); - simple_annotations_ = simple_annotations.Pass(); + simple_annotations_ = std::move(simple_annotations); } bool MinidumpModuleCrashpadInfoWriter::IsUseful() const { @@ -144,7 +146,7 @@ void MinidumpModuleCrashpadInfoListWriter::InitializeFromSnapshot( auto module = make_scoped_ptr(new MinidumpModuleCrashpadInfoWriter()); module->InitializeFromSnapshot(module_snapshot); if (module->IsUseful()) { - AddModule(module.Pass(), index); + AddModule(std::move(module), index); } } } diff --git a/minidump/minidump_module_crashpad_info_writer_test.cc b/minidump/minidump_module_crashpad_info_writer_test.cc index 458913fe..07f44c14 100644 --- a/minidump/minidump_module_crashpad_info_writer_test.cc +++ b/minidump/minidump_module_crashpad_info_writer_test.cc @@ -17,6 +17,8 @@ #include #include +#include + #include "gtest/gtest.h" #include "minidump/minidump_extensions.h" #include "minidump/minidump_simple_string_dictionary_writer.h" @@ -77,7 +79,7 @@ TEST(MinidumpModuleCrashpadInfoWriter, EmptyModule) { make_scoped_ptr(new MinidumpModuleCrashpadInfoListWriter()); auto module_writer = make_scoped_ptr(new MinidumpModuleCrashpadInfoWriter()); EXPECT_FALSE(module_writer->IsUseful()); - module_list_writer->AddModule(module_writer.Pass(), 0); + module_list_writer->AddModule(std::move(module_writer), 0); EXPECT_TRUE(module_list_writer->IsUseful()); @@ -119,17 +121,19 @@ TEST(MinidumpModuleCrashpadInfoWriter, FullModule) { auto module_writer = make_scoped_ptr(new MinidumpModuleCrashpadInfoWriter()); auto string_list_writer = make_scoped_ptr(new MinidumpUTF8StringListWriter()); string_list_writer->InitializeFromVector(vector); - module_writer->SetListAnnotations(string_list_writer.Pass()); + module_writer->SetListAnnotations(std::move(string_list_writer)); auto simple_string_dictionary_writer = make_scoped_ptr(new MinidumpSimpleStringDictionaryWriter()); auto simple_string_dictionary_entry_writer = make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); simple_string_dictionary_entry_writer->SetKeyValue(kKey, kValue); simple_string_dictionary_writer->AddEntry( - simple_string_dictionary_entry_writer.Pass()); - module_writer->SetSimpleAnnotations(simple_string_dictionary_writer.Pass()); + std::move(simple_string_dictionary_entry_writer)); + module_writer->SetSimpleAnnotations( + std::move(simple_string_dictionary_writer)); EXPECT_TRUE(module_writer->IsUseful()); - module_list_writer->AddModule(module_writer.Pass(), kMinidumpModuleListIndex); + module_list_writer->AddModule(std::move(module_writer), + kMinidumpModuleListIndex); EXPECT_TRUE(module_list_writer->IsUseful()); @@ -211,17 +215,17 @@ TEST(MinidumpModuleCrashpadInfoWriter, ThreeModules) { make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); simple_string_dictionary_entry_writer_0->SetKeyValue(kKey0, kValue0); simple_string_dictionary_writer_0->AddEntry( - simple_string_dictionary_entry_writer_0.Pass()); + std::move(simple_string_dictionary_entry_writer_0)); module_writer_0->SetSimpleAnnotations( - simple_string_dictionary_writer_0.Pass()); + std::move(simple_string_dictionary_writer_0)); EXPECT_TRUE(module_writer_0->IsUseful()); - module_list_writer->AddModule(module_writer_0.Pass(), + module_list_writer->AddModule(std::move(module_writer_0), kMinidumpModuleListIndex0); auto module_writer_1 = make_scoped_ptr(new MinidumpModuleCrashpadInfoWriter()); EXPECT_FALSE(module_writer_1->IsUseful()); - module_list_writer->AddModule(module_writer_1.Pass(), + module_list_writer->AddModule(std::move(module_writer_1), kMinidumpModuleListIndex1); auto module_writer_2 = @@ -232,16 +236,16 @@ TEST(MinidumpModuleCrashpadInfoWriter, ThreeModules) { make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); simple_string_dictionary_entry_writer_2a->SetKeyValue(kKey2A, kValue2A); simple_string_dictionary_writer_2->AddEntry( - simple_string_dictionary_entry_writer_2a.Pass()); + std::move(simple_string_dictionary_entry_writer_2a)); auto simple_string_dictionary_entry_writer_2b = make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); simple_string_dictionary_entry_writer_2b->SetKeyValue(kKey2B, kValue2B); simple_string_dictionary_writer_2->AddEntry( - simple_string_dictionary_entry_writer_2b.Pass()); + std::move(simple_string_dictionary_entry_writer_2b)); module_writer_2->SetSimpleAnnotations( - simple_string_dictionary_writer_2.Pass()); + std::move(simple_string_dictionary_writer_2)); EXPECT_TRUE(module_writer_2->IsUseful()); - module_list_writer->AddModule(module_writer_2.Pass(), + module_list_writer->AddModule(std::move(module_writer_2), kMinidumpModuleListIndex2); EXPECT_TRUE(module_list_writer->IsUseful()); diff --git a/minidump/minidump_module_writer.cc b/minidump/minidump_module_writer.cc index fd78b9f8..c3a0e839 100644 --- a/minidump/minidump_module_writer.cc +++ b/minidump/minidump_module_writer.cc @@ -17,6 +17,7 @@ #include #include +#include #include "base/logging.h" #include "base/numerics/safe_conversions.h" @@ -244,7 +245,7 @@ void MinidumpModuleWriter::InitializeFromSnapshot( auto codeview_record = make_scoped_ptr(new MinidumpModuleCodeViewRecordPDB70Writer()); codeview_record->InitializeFromSnapshot(module_snapshot); - SetCodeViewRecord(codeview_record.Pass()); + SetCodeViewRecord(std::move(codeview_record)); } const MINIDUMP_MODULE* MinidumpModuleWriter::MinidumpModule() const { @@ -266,14 +267,14 @@ void MinidumpModuleWriter::SetCodeViewRecord( scoped_ptr codeview_record) { DCHECK_EQ(state(), kStateMutable); - codeview_record_ = codeview_record.Pass(); + codeview_record_ = std::move(codeview_record); } void MinidumpModuleWriter::SetMiscDebugRecord( scoped_ptr misc_debug_record) { DCHECK_EQ(state(), kStateMutable); - misc_debug_record_ = misc_debug_record.Pass(); + misc_debug_record_ = std::move(misc_debug_record); } void MinidumpModuleWriter::SetTimestamp(time_t timestamp) { @@ -385,7 +386,7 @@ void MinidumpModuleListWriter::InitializeFromSnapshot( for (const ModuleSnapshot* module_snapshot : module_snapshots) { auto module = make_scoped_ptr(new MinidumpModuleWriter()); module->InitializeFromSnapshot(module_snapshot); - AddModule(module.Pass()); + AddModule(std::move(module)); } } diff --git a/minidump/minidump_module_writer_test.cc b/minidump/minidump_module_writer_test.cc index f0cc36c1..8d6272de 100644 --- a/minidump/minidump_module_writer_test.cc +++ b/minidump/minidump_module_writer_test.cc @@ -20,6 +20,8 @@ #include #include +#include + #include "base/format_macros.h" #include "base/strings/stringprintf.h" #include "base/strings/utf_string_conversions.h" @@ -68,7 +70,7 @@ TEST(MinidumpModuleWriter, EmptyModuleList) { MinidumpFileWriter minidump_file_writer; auto module_list_writer = make_scoped_ptr(new MinidumpModuleListWriter()); - minidump_file_writer.AddStream(module_list_writer.Pass()); + minidump_file_writer.AddStream(std::move(module_list_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -276,8 +278,8 @@ TEST(MinidumpModuleWriter, EmptyModule) { auto module_writer = make_scoped_ptr(new MinidumpModuleWriter()); module_writer->SetName(kModuleName); - module_list_writer->AddModule(module_writer.Pass()); - minidump_file_writer.AddStream(module_list_writer.Pass()); + module_list_writer->AddModule(std::move(module_writer)); + minidump_file_writer.AddStream(std::move(module_list_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -359,16 +361,16 @@ TEST(MinidumpModuleWriter, OneModule) { make_scoped_ptr(new MinidumpModuleCodeViewRecordPDB70Writer()); codeview_pdb70_writer->SetPDBName(kPDBName); codeview_pdb70_writer->SetUUIDAndAge(pdb_uuid, kPDBAge); - module_writer->SetCodeViewRecord(codeview_pdb70_writer.Pass()); + module_writer->SetCodeViewRecord(std::move(codeview_pdb70_writer)); auto misc_debug_writer = make_scoped_ptr(new MinidumpModuleMiscDebugRecordWriter()); misc_debug_writer->SetDataType(kDebugType); misc_debug_writer->SetData(kDebugName, kDebugUTF16); - module_writer->SetMiscDebugRecord(misc_debug_writer.Pass()); + module_writer->SetMiscDebugRecord(std::move(misc_debug_writer)); - module_list_writer->AddModule(module_writer.Pass()); - minidump_file_writer.AddStream(module_list_writer.Pass()); + module_list_writer->AddModule(std::move(module_writer)); + minidump_file_writer.AddStream(std::move(module_list_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -434,16 +436,16 @@ TEST(MinidumpModuleWriter, OneModule_CodeViewUsesPDB20_MiscUsesUTF16) { make_scoped_ptr(new MinidumpModuleCodeViewRecordPDB20Writer()); codeview_pdb20_writer->SetPDBName(kPDBName); codeview_pdb20_writer->SetTimestampAndAge(kPDBTimestamp, kPDBAge); - module_writer->SetCodeViewRecord(codeview_pdb20_writer.Pass()); + module_writer->SetCodeViewRecord(std::move(codeview_pdb20_writer)); auto misc_debug_writer = make_scoped_ptr(new MinidumpModuleMiscDebugRecordWriter()); misc_debug_writer->SetDataType(kDebugType); misc_debug_writer->SetData(kDebugName, kDebugUTF16); - module_writer->SetMiscDebugRecord(misc_debug_writer.Pass()); + module_writer->SetMiscDebugRecord(std::move(misc_debug_writer)); - module_list_writer->AddModule(module_writer.Pass()); - minidump_file_writer.AddStream(module_list_writer.Pass()); + module_list_writer->AddModule(std::move(module_writer)); + minidump_file_writer.AddStream(std::move(module_list_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -511,16 +513,16 @@ TEST(MinidumpModuleWriter, ThreeModules) { make_scoped_ptr(new MinidumpModuleCodeViewRecordPDB70Writer()); codeview_pdb70_writer_0->SetPDBName(kPDBName0); codeview_pdb70_writer_0->SetUUIDAndAge(pdb_uuid_0, kPDBAge0); - module_writer_0->SetCodeViewRecord(codeview_pdb70_writer_0.Pass()); + module_writer_0->SetCodeViewRecord(std::move(codeview_pdb70_writer_0)); - module_list_writer->AddModule(module_writer_0.Pass()); + module_list_writer->AddModule(std::move(module_writer_0)); auto module_writer_1 = make_scoped_ptr(new MinidumpModuleWriter()); module_writer_1->SetName(kModuleName1); module_writer_1->SetImageBaseAddress(kModuleBase1); module_writer_1->SetImageSize(kModuleSize1); - module_list_writer->AddModule(module_writer_1.Pass()); + module_list_writer->AddModule(std::move(module_writer_1)); auto module_writer_2 = make_scoped_ptr(new MinidumpModuleWriter()); module_writer_2->SetName(kModuleName2); @@ -531,11 +533,11 @@ TEST(MinidumpModuleWriter, ThreeModules) { make_scoped_ptr(new MinidumpModuleCodeViewRecordPDB20Writer()); codeview_pdb70_writer_2->SetPDBName(kPDBName2); codeview_pdb70_writer_2->SetTimestampAndAge(kPDBTimestamp2, kPDBAge2); - module_writer_2->SetCodeViewRecord(codeview_pdb70_writer_2.Pass()); + module_writer_2->SetCodeViewRecord(std::move(codeview_pdb70_writer_2)); - module_list_writer->AddModule(module_writer_2.Pass()); + module_list_writer->AddModule(std::move(module_writer_2)); - minidump_file_writer.AddStream(module_list_writer.Pass()); + minidump_file_writer.AddStream(std::move(module_list_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -728,7 +730,7 @@ TEST(MinidumpModuleWriter, InitializeFromSnapshot) { module_list_writer->InitializeFromSnapshot(module_snapshots); MinidumpFileWriter minidump_file_writer; - minidump_file_writer.AddStream(module_list_writer.Pass()); + minidump_file_writer.AddStream(std::move(module_list_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -759,8 +761,8 @@ TEST(MinidumpModuleWriterDeathTest, NoModuleName) { MinidumpFileWriter minidump_file_writer; auto module_list_writer = make_scoped_ptr(new MinidumpModuleListWriter()); auto module_writer = make_scoped_ptr(new MinidumpModuleWriter()); - module_list_writer->AddModule(module_writer.Pass()); - minidump_file_writer.AddStream(module_list_writer.Pass()); + module_list_writer->AddModule(std::move(module_writer)); + minidump_file_writer.AddStream(std::move(module_list_writer)); StringFile string_file; ASSERT_DEATH_CHECK(minidump_file_writer.WriteEverything(&string_file), diff --git a/minidump/minidump_rva_list_writer_test.cc b/minidump/minidump_rva_list_writer_test.cc index f2594003..fa18dc5d 100644 --- a/minidump/minidump_rva_list_writer_test.cc +++ b/minidump/minidump_rva_list_writer_test.cc @@ -14,6 +14,8 @@ #include "minidump/minidump_rva_list_writer.h" +#include + #include "base/format_macros.h" #include "base/strings/stringprintf.h" #include "gtest/gtest.h" @@ -32,7 +34,7 @@ class TestMinidumpRVAListWriter final : public internal::MinidumpRVAListWriter { void AddChild(uint32_t value) { auto child = make_scoped_ptr(new TestUInt32MinidumpWritable(value)); - MinidumpRVAListWriter::AddChild(child.Pass()); + MinidumpRVAListWriter::AddChild(std::move(child)); } private: diff --git a/minidump/minidump_simple_string_dictionary_writer.cc b/minidump/minidump_simple_string_dictionary_writer.cc index c7d81e2f..b3283e59 100644 --- a/minidump/minidump_simple_string_dictionary_writer.cc +++ b/minidump/minidump_simple_string_dictionary_writer.cc @@ -14,6 +14,8 @@ #include "minidump/minidump_simple_string_dictionary_writer.h" +#include + #include "base/logging.h" #include "base/stl_util.h" #include "util/file/file_writer.h" @@ -109,7 +111,7 @@ void MinidumpSimpleStringDictionaryWriter::InitializeFromMap( auto entry = make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); entry->SetKeyValue(iterator.first, iterator.second); - AddEntry(entry.Pass()); + AddEntry(std::move(entry)); } } diff --git a/minidump/minidump_simple_string_dictionary_writer_test.cc b/minidump/minidump_simple_string_dictionary_writer_test.cc index 0f9a71bd..1e401d45 100644 --- a/minidump/minidump_simple_string_dictionary_writer_test.cc +++ b/minidump/minidump_simple_string_dictionary_writer_test.cc @@ -16,6 +16,7 @@ #include #include +#include #include "gtest/gtest.h" #include "minidump/minidump_extensions.h" @@ -62,7 +63,7 @@ TEST(MinidumpSimpleStringDictionaryWriter, EmptyKeyValue) { MinidumpSimpleStringDictionaryWriter dictionary_writer; auto entry_writer = make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); - dictionary_writer.AddEntry(entry_writer.Pass()); + dictionary_writer.AddEntry(std::move(entry_writer)); EXPECT_TRUE(dictionary_writer.IsUseful()); @@ -96,7 +97,7 @@ TEST(MinidumpSimpleStringDictionaryWriter, OneKeyValue) { auto entry_writer = make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); entry_writer->SetKeyValue(kKey, kValue); - dictionary_writer.AddEntry(entry_writer.Pass()); + dictionary_writer.AddEntry(std::move(entry_writer)); EXPECT_TRUE(dictionary_writer.IsUseful()); @@ -134,15 +135,15 @@ TEST(MinidumpSimpleStringDictionaryWriter, ThreeKeysValues) { auto entry_writer_0 = make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); entry_writer_0->SetKeyValue(kKey0, kValue0); - dictionary_writer.AddEntry(entry_writer_0.Pass()); + dictionary_writer.AddEntry(std::move(entry_writer_0)); auto entry_writer_1 = make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); entry_writer_1->SetKeyValue(kKey1, kValue1); - dictionary_writer.AddEntry(entry_writer_1.Pass()); + dictionary_writer.AddEntry(std::move(entry_writer_1)); auto entry_writer_2 = make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); entry_writer_2->SetKeyValue(kKey2, kValue2); - dictionary_writer.AddEntry(entry_writer_2.Pass()); + dictionary_writer.AddEntry(std::move(entry_writer_2)); EXPECT_TRUE(dictionary_writer.IsUseful()); @@ -202,11 +203,11 @@ TEST(MinidumpSimpleStringDictionaryWriter, DuplicateKeyValue) { auto entry_writer_0 = make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); entry_writer_0->SetKeyValue(kKey, kValue0); - dictionary_writer.AddEntry(entry_writer_0.Pass()); + dictionary_writer.AddEntry(std::move(entry_writer_0)); auto entry_writer_1 = make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); entry_writer_1->SetKeyValue(kKey, kValue1); - dictionary_writer.AddEntry(entry_writer_1.Pass()); + dictionary_writer.AddEntry(std::move(entry_writer_1)); EXPECT_TRUE(dictionary_writer.IsUseful()); diff --git a/minidump/minidump_string_writer.cc b/minidump/minidump_string_writer.cc index 95c0ad59..bf40d256 100644 --- a/minidump/minidump_string_writer.cc +++ b/minidump/minidump_string_writer.cc @@ -16,6 +16,8 @@ #include +#include + #include "base/logging.h" #include "minidump/minidump_writer_util.h" #include "util/file/file_writer.h" @@ -121,7 +123,7 @@ void MinidumpStringListWriter::AddStringUTF8( const std::string& string_utf8) { auto string_writer = make_scoped_ptr(new MinidumpStringWriterType()); string_writer->SetUTF8(string_utf8); - AddChild(string_writer.Pass()); + AddChild(std::move(string_writer)); } template diff --git a/minidump/minidump_system_info_writer_test.cc b/minidump/minidump_system_info_writer_test.cc index daf81f60..88bb0e01 100644 --- a/minidump/minidump_system_info_writer_test.cc +++ b/minidump/minidump_system_info_writer_test.cc @@ -21,6 +21,7 @@ #include #include +#include #include "base/compiler_specific.h" #include "gtest/gtest.h" @@ -83,7 +84,7 @@ TEST(MinidumpSystemInfoWriter, Empty) { system_info_writer->SetCSDVersion(std::string()); - minidump_file_writer.AddStream(system_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(system_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -154,7 +155,7 @@ TEST(MinidumpSystemInfoWriter, X86_Win) { system_info_writer->SetCPUX86VersionAndFeatures(kCPUVersion, kCPUFeatures); system_info_writer->SetCPUX86AMDExtendedFeatures(kAMDFeatures); - minidump_file_writer.AddStream(system_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(system_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -214,7 +215,7 @@ TEST(MinidumpSystemInfoWriter, AMD64_Mac) { system_info_writer->SetCSDVersion(kCSDVersion); system_info_writer->SetCPUOtherFeatures(kCPUFeatures[0], kCPUFeatures[1]); - minidump_file_writer.AddStream(system_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(system_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -256,7 +257,7 @@ TEST(MinidumpSystemInfoWriter, X86_CPUVendorFromRegisters) { kCPUVendor[0], kCPUVendor[1], kCPUVendor[2]); system_info_writer->SetCSDVersion(std::string()); - minidump_file_writer.AddStream(system_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(system_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -336,7 +337,7 @@ TEST(MinidumpSystemInfoWriter, InitializeFromSnapshot_X86) { system_info_writer->InitializeFromSnapshot(&system_snapshot); MinidumpFileWriter minidump_file_writer; - minidump_file_writer.AddStream(system_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(system_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -431,7 +432,7 @@ TEST(MinidumpSystemInfoWriter, InitializeFromSnapshot_AMD64) { system_info_writer->InitializeFromSnapshot(&system_snapshot); MinidumpFileWriter minidump_file_writer; - minidump_file_writer.AddStream(system_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(system_info_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -469,7 +470,7 @@ TEST(MinidumpSystemInfoWriter, InitializeFromSnapshot_AMD64) { TEST(MinidumpSystemInfoWriterDeathTest, NoCSDVersion) { MinidumpFileWriter minidump_file_writer; auto system_info_writer = make_scoped_ptr(new MinidumpSystemInfoWriter()); - minidump_file_writer.AddStream(system_info_writer.Pass()); + minidump_file_writer.AddStream(std::move(system_info_writer)); StringFile string_file; ASSERT_DEATH_CHECK(minidump_file_writer.WriteEverything(&string_file), diff --git a/minidump/minidump_thread_writer.cc b/minidump/minidump_thread_writer.cc index 72050bdb..d41d2dd4 100644 --- a/minidump/minidump_thread_writer.cc +++ b/minidump/minidump_thread_writer.cc @@ -16,6 +16,8 @@ #include +#include + #include "base/logging.h" #include "minidump/minidump_context_writer.h" #include "minidump/minidump_memory_writer.h" @@ -52,12 +54,12 @@ void MinidumpThreadWriter::InitializeFromSnapshot( if (stack_snapshot && stack_snapshot->Size() > 0) { scoped_ptr stack = MinidumpMemoryWriter::CreateFromSnapshot(stack_snapshot); - SetStack(stack.Pass()); + SetStack(std::move(stack)); } scoped_ptr context = MinidumpContextWriter::CreateFromSnapshot(thread_snapshot->Context()); - SetContext(context.Pass()); + SetContext(std::move(context)); } const MINIDUMP_THREAD* MinidumpThreadWriter::MinidumpThread() const { @@ -69,14 +71,14 @@ const MINIDUMP_THREAD* MinidumpThreadWriter::MinidumpThread() const { void MinidumpThreadWriter::SetStack(scoped_ptr stack) { DCHECK_EQ(state(), kStateMutable); - stack_ = stack.Pass(); + stack_ = std::move(stack); } void MinidumpThreadWriter::SetContext( scoped_ptr context) { DCHECK_EQ(state(), kStateMutable); - context_ = context.Pass(); + context_ = std::move(context); } bool MinidumpThreadWriter::Freeze() { @@ -148,7 +150,7 @@ void MinidumpThreadListWriter::InitializeFromSnapshot( for (const ThreadSnapshot* thread_snapshot : thread_snapshots) { auto thread = make_scoped_ptr(new MinidumpThreadWriter()); thread->InitializeFromSnapshot(thread_snapshot, thread_id_map); - AddThread(thread.Pass()); + AddThread(std::move(thread)); } // Do this in a separate loop to keep the thread stacks earlier in the dump, diff --git a/minidump/minidump_thread_writer_test.cc b/minidump/minidump_thread_writer_test.cc index 9571f7c2..ab0561a9 100644 --- a/minidump/minidump_thread_writer_test.cc +++ b/minidump/minidump_thread_writer_test.cc @@ -19,6 +19,7 @@ #include #include +#include #include "base/compiler_specific.h" #include "base/format_macros.h" @@ -83,7 +84,7 @@ TEST(MinidumpThreadWriter, EmptyThreadList) { MinidumpFileWriter minidump_file_writer; auto thread_list_writer = make_scoped_ptr(new MinidumpThreadListWriter()); - minidump_file_writer.AddStream(thread_list_writer.Pass()); + minidump_file_writer.AddStream(std::move(thread_list_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -160,10 +161,10 @@ TEST(MinidumpThreadWriter, OneThread_x86_NoStack) { auto context_x86_writer = make_scoped_ptr(new MinidumpContextX86Writer()); InitializeMinidumpContextX86(context_x86_writer->context(), kSeed); - thread_writer->SetContext(context_x86_writer.Pass()); + thread_writer->SetContext(std::move(context_x86_writer)); - thread_list_writer->AddThread(thread_writer.Pass()); - minidump_file_writer.AddStream(thread_list_writer.Pass()); + thread_list_writer->AddThread(std::move(thread_writer)); + minidump_file_writer.AddStream(std::move(thread_list_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -222,15 +223,15 @@ TEST(MinidumpThreadWriter, OneThread_AMD64_Stack) { auto memory_writer = make_scoped_ptr( new TestMinidumpMemoryWriter(kMemoryBase, kMemorySize, kMemoryValue)); - thread_writer->SetStack(memory_writer.Pass()); + thread_writer->SetStack(std::move(memory_writer)); MSVC_SUPPRESS_WARNING(4316); // Object allocated on heap may not be aligned. auto context_amd64_writer = make_scoped_ptr(new MinidumpContextAMD64Writer()); InitializeMinidumpContextAMD64(context_amd64_writer->context(), kSeed); - thread_writer->SetContext(context_amd64_writer.Pass()); + thread_writer->SetContext(std::move(context_amd64_writer)); - thread_list_writer->AddThread(thread_writer.Pass()); - minidump_file_writer.AddStream(thread_list_writer.Pass()); + thread_list_writer->AddThread(std::move(thread_writer)); + minidump_file_writer.AddStream(std::move(thread_list_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -300,13 +301,13 @@ TEST(MinidumpThreadWriter, ThreeThreads_x86_MemoryList) { auto memory_writer_0 = make_scoped_ptr( new TestMinidumpMemoryWriter(kMemoryBase0, kMemorySize0, kMemoryValue0)); - thread_writer_0->SetStack(memory_writer_0.Pass()); + thread_writer_0->SetStack(std::move(memory_writer_0)); auto context_x86_writer_0 = make_scoped_ptr(new MinidumpContextX86Writer()); InitializeMinidumpContextX86(context_x86_writer_0->context(), kSeed0); - thread_writer_0->SetContext(context_x86_writer_0.Pass()); + thread_writer_0->SetContext(std::move(context_x86_writer_0)); - thread_list_writer->AddThread(thread_writer_0.Pass()); + thread_list_writer->AddThread(std::move(thread_writer_0)); const uint32_t kThreadID1 = 2222222; const uint32_t kSuspendCount1 = 222222; @@ -327,13 +328,13 @@ TEST(MinidumpThreadWriter, ThreeThreads_x86_MemoryList) { auto memory_writer_1 = make_scoped_ptr( new TestMinidumpMemoryWriter(kMemoryBase1, kMemorySize1, kMemoryValue1)); - thread_writer_1->SetStack(memory_writer_1.Pass()); + thread_writer_1->SetStack(std::move(memory_writer_1)); auto context_x86_writer_1 = make_scoped_ptr(new MinidumpContextX86Writer()); InitializeMinidumpContextX86(context_x86_writer_1->context(), kSeed1); - thread_writer_1->SetContext(context_x86_writer_1.Pass()); + thread_writer_1->SetContext(std::move(context_x86_writer_1)); - thread_list_writer->AddThread(thread_writer_1.Pass()); + thread_list_writer->AddThread(std::move(thread_writer_1)); const uint32_t kThreadID2 = 3333333; const uint32_t kSuspendCount2 = 333333; @@ -354,16 +355,16 @@ TEST(MinidumpThreadWriter, ThreeThreads_x86_MemoryList) { auto memory_writer_2 = make_scoped_ptr( new TestMinidumpMemoryWriter(kMemoryBase2, kMemorySize2, kMemoryValue2)); - thread_writer_2->SetStack(memory_writer_2.Pass()); + thread_writer_2->SetStack(std::move(memory_writer_2)); auto context_x86_writer_2 = make_scoped_ptr(new MinidumpContextX86Writer()); InitializeMinidumpContextX86(context_x86_writer_2->context(), kSeed2); - thread_writer_2->SetContext(context_x86_writer_2.Pass()); + thread_writer_2->SetContext(std::move(context_x86_writer_2)); - thread_list_writer->AddThread(thread_writer_2.Pass()); + thread_list_writer->AddThread(std::move(thread_writer_2)); - minidump_file_writer.AddStream(thread_list_writer.Pass()); - minidump_file_writer.AddStream(memory_list_writer.Pass()); + minidump_file_writer.AddStream(std::move(thread_list_writer)); + minidump_file_writer.AddStream(std::move(memory_list_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -600,7 +601,7 @@ void RunInitializeFromSnapshotTest(bool thread_id_collision) { expect_threads[index].Stack.StartOfMemoryRange); memory_snapshot->SetSize(expect_threads[index].Stack.Memory.DataSize); memory_snapshot->SetValue(memory_values[index]); - thread_snapshot->SetStack(memory_snapshot.Pass()); + thread_snapshot->SetStack(std::move(memory_snapshot)); } Traits::InitializeCPUContext(thread_snapshot->MutableContext(), @@ -610,7 +611,7 @@ void RunInitializeFromSnapshotTest(bool thread_id_collision) { teb_snapshot->SetAddress(expect_threads[index].Teb); teb_snapshot->SetSize(kTebSize); teb_snapshot->SetValue(static_cast('t' + index)); - thread_snapshot->AddExtraMemory(teb_snapshot.Pass()); + thread_snapshot->AddExtraMemory(std::move(teb_snapshot)); thread_snapshots.push_back(thread_snapshot); } @@ -622,8 +623,8 @@ void RunInitializeFromSnapshotTest(bool thread_id_collision) { thread_list_writer->InitializeFromSnapshot(thread_snapshots, &thread_id_map); MinidumpFileWriter minidump_file_writer; - minidump_file_writer.AddStream(thread_list_writer.Pass()); - minidump_file_writer.AddStream(memory_list_writer.Pass()); + minidump_file_writer.AddStream(std::move(thread_list_writer)); + minidump_file_writer.AddStream(std::move(memory_list_writer)); StringFile string_file; ASSERT_TRUE(minidump_file_writer.WriteEverything(&string_file)); @@ -700,8 +701,8 @@ TEST(MinidumpThreadWriterDeathTest, NoContext) { auto thread_writer = make_scoped_ptr(new MinidumpThreadWriter()); - thread_list_writer->AddThread(thread_writer.Pass()); - minidump_file_writer.AddStream(thread_list_writer.Pass()); + thread_list_writer->AddThread(std::move(thread_writer)); + minidump_file_writer.AddStream(std::move(thread_list_writer)); StringFile string_file; ASSERT_DEATH_CHECK(minidump_file_writer.WriteEverything(&string_file), diff --git a/package.h b/package.h index a18d7b82..af4d1fc5 100644 --- a/package.h +++ b/package.h @@ -23,7 +23,7 @@ #define PACKAGE_NAME "Crashpad" #define PACKAGE_STRING PACKAGE_NAME " " PACKAGE_VERSION #define PACKAGE_TARNAME "crashpad" -#define PACKAGE_VERSION "0.7.0" +#define PACKAGE_VERSION "0.8.0" #define PACKAGE_URL "https://crashpad.chromium.org/" #endif // CRASHPAD_PACKAGE_H_ diff --git a/snapshot/snapshot.gyp b/snapshot/snapshot.gyp index 0a78e90c..3f7c63ee 100644 --- a/snapshot/snapshot.gyp +++ b/snapshot/snapshot.gyp @@ -103,10 +103,14 @@ 'win/pe_image_annotations_reader.h', 'win/pe_image_reader.cc', 'win/pe_image_reader.h', + 'win/pe_image_resource_reader.cc', + 'win/pe_image_resource_reader.h', 'win/process_reader_win.cc', 'win/process_reader_win.h', 'win/process_snapshot_win.cc', 'win/process_snapshot_win.h', + 'win/process_subrange_reader.cc', + 'win/process_subrange_reader.h', 'win/system_snapshot_win.cc', 'win/system_snapshot_win.h', 'win/thread_snapshot_win.cc', diff --git a/snapshot/test/test_process_snapshot.h b/snapshot/test/test_process_snapshot.h index ec124481..6e81354f 100644 --- a/snapshot/test/test_process_snapshot.h +++ b/snapshot/test/test_process_snapshot.h @@ -21,6 +21,7 @@ #include #include +#include #include #include "base/basictypes.h" @@ -71,7 +72,9 @@ class TestProcessSnapshot final : public ProcessSnapshot { //! //! \param[in] system The system snapshot that System() will return. The //! TestProcessSnapshot object takes ownership of \a system. - void SetSystem(scoped_ptr system) { system_ = system.Pass(); } + void SetSystem(scoped_ptr system) { + system_ = std::move(system); + } //! \brief Adds a thread snapshot to be returned by Threads(). //! @@ -94,7 +97,7 @@ class TestProcessSnapshot final : public ProcessSnapshot { //! \param[in] exception The exception snapshot that Exception() will return. //! The TestProcessSnapshot object takes ownership of \a exception. void SetException(scoped_ptr exception) { - exception_ = exception.Pass(); + exception_ = std::move(exception); } //! \brief Adds a memory map region snapshot to be returned by MemoryMap(). diff --git a/snapshot/test/test_thread_snapshot.h b/snapshot/test/test_thread_snapshot.h index 9b00d32f..29e3f5fb 100644 --- a/snapshot/test/test_thread_snapshot.h +++ b/snapshot/test/test_thread_snapshot.h @@ -17,6 +17,7 @@ #include +#include #include #include "base/basictypes.h" @@ -55,7 +56,7 @@ class TestThreadSnapshot final : public ThreadSnapshot { //! //! \param[in] stack The memory region that Stack() will return. The //! TestThreadSnapshot object takes ownership of \a stack. - void SetStack(scoped_ptr stack) { stack_ = stack.Pass(); } + void SetStack(scoped_ptr stack) { stack_ = std::move(stack); } void SetThreadID(uint64_t thread_id) { thread_id_ = thread_id; } void SetSuspendCount(int suspend_count) { suspend_count_ = suspend_count; } diff --git a/snapshot/win/end_to_end_test.py b/snapshot/win/end_to_end_test.py old mode 100644 new mode 100755 index ddf48ef2..f3172ba7 --- a/snapshot/win/end_to_end_test.py +++ b/snapshot/win/end_to_end_test.py @@ -221,12 +221,14 @@ def RunTests(cdb_path, out.Check('LastStatusValue: \(NTSTATUS\) 0xc000000f - {File Not Found} The ' 'file %hs does not exist.', '!gle gets last ntstatus') - 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 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') + if False: + # TODO(scottmg): Re-enable when we grab ntdll!RtlCriticalSectionList. + 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 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') diff --git a/snapshot/win/module_snapshot_win.cc b/snapshot/win/module_snapshot_win.cc index 49478880..0553f35b 100644 --- a/snapshot/win/module_snapshot_win.cc +++ b/snapshot/win/module_snapshot_win.cc @@ -19,7 +19,6 @@ #include "snapshot/win/pe_image_reader.h" #include "util/misc/tri_state.h" #include "util/misc/uuid.h" -#include "util/win/module_version.h" namespace crashpad { namespace internal { @@ -210,7 +209,7 @@ const VS_FIXEDFILEINFO* ModuleSnapshotWin::VSFixedFileInfo() const { if (initialized_vs_fixed_file_info_.is_uninitialized()) { initialized_vs_fixed_file_info_.set_invalid(); - if (GetModuleVersionAndType(base::FilePath(name_), &vs_fixed_file_info_)) { + if (pe_image_reader_->VSFixedFileInfo(&vs_fixed_file_info_)) { initialized_vs_fixed_file_info_.set_valid(); } } diff --git a/snapshot/win/pe_image_reader.cc b/snapshot/win/pe_image_reader.cc index 82bce064..9abf0b13 100644 --- a/snapshot/win/pe_image_reader.cc +++ b/snapshot/win/pe_image_reader.cc @@ -18,9 +18,8 @@ #include "base/logging.h" #include "base/memory/scoped_ptr.h" -#include "base/strings/stringprintf.h" #include "client/crashpad_info.h" -#include "snapshot/win/process_reader_win.h" +#include "snapshot/win/pe_image_resource_reader.h" #include "util/misc/pdb_structures.h" #include "util/win/process_structs.h" @@ -28,13 +27,6 @@ namespace crashpad { namespace { -std::string RangeToString(const CheckedWinAddressRange& range) { - return base::StringPrintf("[0x%llx + 0x%llx (%s)]", - range.Base(), - range.Size(), - range.Is64Bit() ? "64" : "32"); -} - // Map from Traits to an IMAGE_NT_HEADERSxx. template struct NtHeadersForTraits; @@ -52,9 +44,7 @@ struct NtHeadersForTraits { } // namespace PEImageReader::PEImageReader() - : process_reader_(nullptr), - module_range_(), - module_name_(), + : module_subrange_reader_(), initialized_() { } @@ -67,14 +57,10 @@ bool PEImageReader::Initialize(ProcessReaderWin* process_reader, const std::string& module_name) { INITIALIZATION_STATE_SET_INITIALIZING(initialized_); - process_reader_ = process_reader; - module_range_.SetRange(process_reader_->Is64Bit(), address, size); - if (!module_range_.IsValid()) { - LOG(WARNING) << "invalid module range for " << module_name << ": " - << RangeToString(module_range_); + if (!module_subrange_reader_.Initialize( + process_reader, address, size, module_name)) { return false; } - module_name_ = module_name; INITIALIZATION_STATE_SET_VALID(initialized_); return true; @@ -86,41 +72,41 @@ bool PEImageReader::GetCrashpadInfo( INITIALIZATION_STATE_DCHECK_VALID(initialized_); IMAGE_SECTION_HEADER section; - if (!GetSectionByName::type>("CPADinfo", §ion)) + if (!GetSectionByName::type>("CPADinfo", + §ion)) { return false; + } if (section.Misc.VirtualSize < sizeof(process_types::CrashpadInfo)) { LOG(WARNING) << "small crashpad info section size " - << section.Misc.VirtualSize << ", " << module_name_; + << section.Misc.VirtualSize << ", " + << module_subrange_reader_.name(); return false; } - WinVMAddress crashpad_info_address = Address() + section.VirtualAddress; - CheckedWinAddressRange crashpad_info_range(process_reader_->Is64Bit(), - crashpad_info_address, - section.Misc.VirtualSize); - if (!crashpad_info_range.IsValid()) { - LOG(WARNING) << "invalid range for crashpad info: " - << RangeToString(crashpad_info_range); + ProcessSubrangeReader crashpad_info_subrange_reader; + const WinVMAddress crashpad_info_address = Address() + section.VirtualAddress; + if (!crashpad_info_subrange_reader.InitializeSubrange( + module_subrange_reader_, + crashpad_info_address, + section.Misc.VirtualSize, + "crashpad_info")) { return false; } - if (!module_range_.ContainsRange(crashpad_info_range)) { - LOG(WARNING) << "crashpad info does not fall inside module " - << module_name_; - return false; - } - - if (!process_reader_->ReadMemory(crashpad_info_address, - sizeof(process_types::CrashpadInfo), - crashpad_info)) { - LOG(WARNING) << "could not read crashpad info " << module_name_; + if (!crashpad_info_subrange_reader.ReadMemory( + crashpad_info_address, + sizeof(process_types::CrashpadInfo), + crashpad_info)) { + LOG(WARNING) << "could not read crashpad info from " + << module_subrange_reader_.name(); return false; } if (crashpad_info->signature != CrashpadInfo::kSignature || crashpad_info->version < 1) { - LOG(WARNING) << "unexpected crashpad info data " << module_name_; + LOG(WARNING) << "unexpected crashpad info data in " + << module_subrange_reader_.name(); return false; } @@ -130,46 +116,23 @@ bool PEImageReader::GetCrashpadInfo( bool PEImageReader::DebugDirectoryInformation(UUID* uuid, DWORD* age, std::string* pdbname) const { - if (process_reader_->Is64Bit()) { - return ReadDebugDirectoryInformation( - uuid, age, pdbname); - } else { - return ReadDebugDirectoryInformation( - uuid, age, pdbname); - } -} + INITIALIZATION_STATE_DCHECK_VALID(initialized_); -template -bool PEImageReader::ReadDebugDirectoryInformation(UUID* uuid, - DWORD* age, - std::string* pdbname) const { - NtHeadersType nt_headers; - if (!ReadNtHeaders(&nt_headers, nullptr)) + IMAGE_DATA_DIRECTORY data_directory; + if (!ImageDataDirectoryEntry(IMAGE_DIRECTORY_ENTRY_DEBUG, &data_directory)) return false; - if (nt_headers.FileHeader.SizeOfOptionalHeader < - offsetof(decltype(nt_headers.OptionalHeader), - DataDirectory[IMAGE_DIRECTORY_ENTRY_DEBUG]) + - sizeof(nt_headers.OptionalHeader - .DataDirectory[IMAGE_DIRECTORY_ENTRY_DEBUG]) || - nt_headers.OptionalHeader.NumberOfRvaAndSizes <= - IMAGE_DIRECTORY_ENTRY_DEBUG) { - return false; - } - - const IMAGE_DATA_DIRECTORY& data_directory = - nt_headers.OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_DEBUG]; - if (data_directory.VirtualAddress == 0 || data_directory.Size == 0) - return false; IMAGE_DEBUG_DIRECTORY debug_directory; if (data_directory.Size % sizeof(debug_directory) != 0) return false; for (size_t offset = 0; offset < data_directory.Size; offset += sizeof(debug_directory)) { - if (!CheckedReadMemory(Address() + data_directory.VirtualAddress + offset, - sizeof(debug_directory), - &debug_directory)) { - LOG(WARNING) << "could not read data directory"; + if (!module_subrange_reader_.ReadMemory( + Address() + data_directory.VirtualAddress + offset, + sizeof(debug_directory), + &debug_directory)) { + LOG(WARNING) << "could not read data directory from " + << module_subrange_reader_.name(); return false; } @@ -178,21 +141,25 @@ bool PEImageReader::ReadDebugDirectoryInformation(UUID* uuid, if (debug_directory.AddressOfRawData) { if (debug_directory.SizeOfData < sizeof(CodeViewRecordPDB70)) { - LOG(WARNING) << "CodeView debug entry of unexpected size"; + LOG(WARNING) << "CodeView debug entry of unexpected size in " + << module_subrange_reader_.name(); continue; } scoped_ptr data(new char[debug_directory.SizeOfData]); - if (!CheckedReadMemory(Address() + debug_directory.AddressOfRawData, - debug_directory.SizeOfData, - data.get())) { - LOG(WARNING) << "could not read debug directory"; + if (!module_subrange_reader_.ReadMemory( + Address() + debug_directory.AddressOfRawData, + debug_directory.SizeOfData, + data.get())) { + LOG(WARNING) << "could not read debug directory from " + << module_subrange_reader_.name(); return false; } if (*reinterpret_cast(data.get()) != CodeViewRecordPDB70::kSignature) { - LOG(WARNING) << "encountered non-7.0 CodeView debug record"; + LOG(WARNING) << "encountered non-7.0 CodeView debug record in " + << module_subrange_reader_.name(); continue; } @@ -216,29 +183,120 @@ bool PEImageReader::ReadDebugDirectoryInformation(UUID* uuid, return false; } +bool PEImageReader::VSFixedFileInfo( + VS_FIXEDFILEINFO* vs_fixed_file_info) const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + + IMAGE_DATA_DIRECTORY data_directory; + if (!ImageDataDirectoryEntry(IMAGE_DIRECTORY_ENTRY_RESOURCE, + &data_directory)) { + return false; + } + + PEImageResourceReader resource_reader; + if (!resource_reader.Initialize(module_subrange_reader_, data_directory)) { + return false; + } + + WinVMAddress address; + WinVMSize size; + if (!resource_reader.FindResourceByID( + reinterpret_cast(VS_FILE_INFO), // RT_VERSION + VS_VERSION_INFO, + MAKELANGID(LANG_NEUTRAL, SUBLANG_NEUTRAL), + &address, + &size, + nullptr)) { + return false; + } + + // This structure is not declared anywhere in the SDK, but is documented at + // https://msdn.microsoft.com/en-us/library/windows/desktop/ms647001.aspx. + struct VS_VERSIONINFO { + WORD wLength; + WORD wValueLength; + WORD wType; + + // The structure documentation on MSDN doesn’t show the [16], but it does + // say that it’s supposed to be L"VS_VERSION_INFO", which is is in fact a + // 16-character string (including its NUL terminator). + WCHAR szKey[16]; + + WORD Padding1; + VS_FIXEDFILEINFO Value; + + // Don’t include Children or the Padding2 that precedes it, because they may + // not be present. + // WORD Padding2; + // WORD Children; + }; + VS_VERSIONINFO version_info; + + if (size < sizeof(version_info)) { + LOG(WARNING) << "version info size " << size + << " too small for structure of size " << sizeof(version_info) + << " in " << module_subrange_reader_.name(); + return false; + } + + if (!module_subrange_reader_.ReadMemory( + address, sizeof(version_info), &version_info)) { + LOG(WARNING) << "could not read version info from " + << module_subrange_reader_.name(); + return false; + } + + if (version_info.wLength < sizeof(version_info) || + version_info.wValueLength != sizeof(version_info.Value) || + version_info.wType != 0 || + wcsncmp(version_info.szKey, + L"VS_VERSION_INFO", + arraysize(version_info.szKey)) != 0) { + LOG(WARNING) << "unexpected VS_VERSIONINFO in " + << module_subrange_reader_.name(); + return false; + } + + if (version_info.Value.dwSignature != VS_FFI_SIGNATURE || + version_info.Value.dwStrucVersion != VS_FFI_STRUCVERSION) { + LOG(WARNING) << "unexpected VS_FIXEDFILEINFO in " + << module_subrange_reader_.name(); + return false; + } + + *vs_fixed_file_info = version_info.Value; + vs_fixed_file_info->dwFileFlags &= vs_fixed_file_info->dwFileFlagsMask; + return true; +} + template bool PEImageReader::ReadNtHeaders(NtHeadersType* nt_headers, WinVMAddress* nt_headers_address) const { IMAGE_DOS_HEADER dos_header; - if (!CheckedReadMemory(Address(), sizeof(IMAGE_DOS_HEADER), &dos_header)) { - LOG(WARNING) << "could not read dos header of " << module_name_; + if (!module_subrange_reader_.ReadMemory( + Address(), sizeof(IMAGE_DOS_HEADER), &dos_header)) { + LOG(WARNING) << "could not read dos header from " + << module_subrange_reader_.name(); return false; } if (dos_header.e_magic != IMAGE_DOS_SIGNATURE) { - LOG(WARNING) << "invalid e_magic in dos header of " << module_name_; + LOG(WARNING) << "invalid e_magic in dos header of " + << module_subrange_reader_.name(); return false; } WinVMAddress local_nt_headers_address = Address() + dos_header.e_lfanew; - if (!CheckedReadMemory( + if (!module_subrange_reader_.ReadMemory( local_nt_headers_address, sizeof(NtHeadersType), nt_headers)) { - LOG(WARNING) << "could not read nt headers of " << module_name_; + LOG(WARNING) << "could not read nt headers from " + << module_subrange_reader_.name(); return false; } if (nt_headers->Signature != IMAGE_NT_SIGNATURE) { - LOG(WARNING) << "invalid signature in nt headers of " << module_name_; + LOG(WARNING) << "invalid signature in nt headers of " + << module_subrange_reader_.name(); return false; } @@ -267,9 +325,10 @@ bool PEImageReader::GetSectionByName(const std::string& name, for (DWORD i = 0; i < nt_headers.FileHeader.NumberOfSections; ++i) { WinVMAddress section_address = first_section_address + sizeof(IMAGE_SECTION_HEADER) * i; - if (!CheckedReadMemory( + if (!module_subrange_reader_.ReadMemory( section_address, sizeof(IMAGE_SECTION_HEADER), section)) { - LOG(WARNING) << "could not read section " << i << " of " << module_name_; + LOG(WARNING) << "could not read section " << i << " from " + << module_subrange_reader_.name(); return false; } if (strncmp(reinterpret_cast(section->Name), @@ -282,20 +341,36 @@ bool PEImageReader::GetSectionByName(const std::string& name, return false; } -bool PEImageReader::CheckedReadMemory(WinVMAddress address, - WinVMSize size, - void* into) const { - CheckedWinAddressRange read_range(process_reader_->Is64Bit(), address, size); - if (!read_range.IsValid()) { - LOG(WARNING) << "invalid read range: " << RangeToString(read_range); +bool PEImageReader::ImageDataDirectoryEntry(size_t index, + IMAGE_DATA_DIRECTORY* entry) const { + bool rv; + if (module_subrange_reader_.Is64Bit()) { + rv = ImageDataDirectoryEntryT(index, entry); + } else { + rv = ImageDataDirectoryEntryT(index, entry); + } + + return rv && entry->VirtualAddress != 0 && entry->Size != 0; +} + +template +bool PEImageReader::ImageDataDirectoryEntryT( + size_t index, + IMAGE_DATA_DIRECTORY* entry) const { + NtHeadersType nt_headers; + if (!ReadNtHeaders(&nt_headers, nullptr)) { return false; } - if (!module_range_.ContainsRange(read_range)) { - LOG(WARNING) << "attempt to read outside of module " << module_name_ - << " at range: " << RangeToString(read_range); + + if (nt_headers.FileHeader.SizeOfOptionalHeader < + offsetof(decltype(nt_headers.OptionalHeader), DataDirectory[index]) + + sizeof(nt_headers.OptionalHeader.DataDirectory[index]) || + nt_headers.OptionalHeader.NumberOfRvaAndSizes <= index) { return false; } - return process_reader_->ReadMemory(address, size, into); + + *entry = nt_headers.OptionalHeader.DataDirectory[index]; + return true; } // Explicit instantiations with the only 2 valid template arguments to avoid diff --git a/snapshot/win/pe_image_reader.h b/snapshot/win/pe_image_reader.h index f6364d99..87e3a8f4 100644 --- a/snapshot/win/pe_image_reader.h +++ b/snapshot/win/pe_image_reader.h @@ -20,10 +20,10 @@ #include #include "base/basictypes.h" +#include "snapshot/win/process_subrange_reader.h" #include "util/misc/initialization_state_dcheck.h" #include "util/misc/uuid.h" #include "util/win/address_types.h" -#include "util/win/checked_win_address_range.h" #include "util/win/process_structs.h" namespace crashpad { @@ -51,6 +51,7 @@ struct CrashpadInfo { //! bitness of the remote process. //! //! \sa PEImageAnnotationsReader +//! \sa PEImageResourceReader class PEImageReader { public: PEImageReader(); @@ -65,8 +66,8 @@ class PEImageReader { //! \param[in] address The address, in the remote process' address space, //! where the `IMAGE_DOS_HEADER` is located. //! \param[in] size The size of the image. - //! \param[in] name The module's name, a string to be used in logged messages. - //! This string is for diagnostic purposes. + //! \param[in] module_name The module's name, a string to be used in logged + //! messages. This string is for diagnostic purposes. //! //! \return `true` if the image was read successfully, `false` otherwise, with //! an appropriate message logged. @@ -78,12 +79,12 @@ class PEImageReader { //! \brief Returns the image's load address. //! //! This is the value passed as \a address to Initialize(). - WinVMAddress Address() const { return module_range_.Base(); } + WinVMAddress Address() const { return module_subrange_reader_.Base(); } //! \brief Returns the image's size. //! //! This is the value passed as \a size to Initialize(). - WinVMSize Size() const { return module_range_.Size(); } + WinVMSize Size() const { return module_subrange_reader_.Size(); } //! \brief Obtains the module's CrashpadInfo structure. //! @@ -100,25 +101,34 @@ class PEImageReader { //! \param[out] age The age field for the pdb (the number of times it's been //! relinked). //! \param[out] pdbname Name of the pdb file. - //! \return `true` on success, or `false` if the module has no debug directory - //! entry. - bool DebugDirectoryInformation(UUID* uuid, - DWORD* age, - std::string* pdbname) const; - - private: - //! \brief Implementation helper for DebugDirectoryInformation() templated by - //! `IMAGE_NT_HEADERS` type for different bitnesses. //! //! \return `true` on success, with the parameters set appropriately. `false` //! on failure. This method may return `false` without logging anything in //! the case of a module that does not contain relevant debugging //! information but is otherwise properly structured. - template - bool ReadDebugDirectoryInformation(UUID* uuid, - DWORD* age, - std::string* pdbname) const; + bool DebugDirectoryInformation(UUID* uuid, + DWORD* age, + std::string* pdbname) const; + //! \brief Obtains the module’s `VS_FIXEDFILEINFO`, containing its version and + //! type information. + //! + //! The data obtained from this method should be equivalent to what could be + //! obtained by calling GetModuleVersionAndType(). Avoiding that function + //! ensures that the data in the module loaded into the remote process will be + //! used as-is, without the risks associated with loading the module into the + //! reading process. + //! + //! \param[out] vs_fixed_file_info The VS_FIXEDFILEINFO on success. + //! VS_FIXEDFILEINFO::dwFileFlags will have been masked with + //! VS_FIXEDFILEINFO::dwFileFlagsMask already. + //! + //! \return `true` on success. `false` if the module does not contain this + //! information, without logging any messages. `false` on failure, with + //! a message logged. + bool VSFixedFileInfo(VS_FIXEDFILEINFO* vs_fixed_file_info) const; + + private: //! \brief Reads the `IMAGE_NT_HEADERS` from the beginning of the image. //! //! \param[out] nt_headers The contents of the templated NtHeadersType @@ -139,18 +149,25 @@ class PEImageReader { bool GetSectionByName(const std::string& name, IMAGE_SECTION_HEADER* section) const; - //! \brief Reads memory from target process, first checking whether the range - //! requested falls inside module_range_. + //! \brief Finds the `IMAGE_DATA_DIRECTORY` in + //! `IMAGE_OPTIONAL_HEADER::DataDirectory` at the specified \a index. //! - //! \return `true` on success, with \a into filled out, otherwise `false` and - //! a message will be logged. - bool CheckedReadMemory(WinVMAddress address, - WinVMSize size, - void* into) const; + //! \param[in] index An `IMAGE_DIRECTORY_ENTRY_*` constant specifying the + //! data to be returned. + //! \param[out] entry The `IMAGE_DATA_DIRECTORY` found within the module. + //! + //! \return `true` on success, with \a entry set appropriately. `false` if the + //! module does not contain the specified information, without logging a + //! message. `false` on failure, with a message logged. + bool ImageDataDirectoryEntry(size_t index, IMAGE_DATA_DIRECTORY* entry) const; - ProcessReaderWin* process_reader_; // weak - CheckedWinAddressRange module_range_; - std::string module_name_; + //! \brief A templatized helper for ImageDataDirectoryEntry() to account for + //! differences in \a NtHeadersType. + template + bool ImageDataDirectoryEntryT(size_t index, + IMAGE_DATA_DIRECTORY* entry) const; + + ProcessSubrangeReader module_subrange_reader_; InitializationStateDcheck initialized_; DISALLOW_COPY_AND_ASSIGN(PEImageReader); diff --git a/snapshot/win/pe_image_reader_test.cc b/snapshot/win/pe_image_reader_test.cc index 3928fee9..46795cfb 100644 --- a/snapshot/win/pe_image_reader_test.cc +++ b/snapshot/win/pe_image_reader_test.cc @@ -17,9 +17,14 @@ #define PSAPI_VERSION 1 #include +#include "base/files/file_path.h" +#include "base/strings/utf_string_conversions.h" #include "gtest/gtest.h" #include "snapshot/win/process_reader_win.h" +#include "test/errors.h" #include "util/win/get_function.h" +#include "util/win/module_version.h" +#include "util/win/process_info.h" extern "C" IMAGE_DOS_HEADER __ImageBase; @@ -28,9 +33,9 @@ namespace test { namespace { BOOL CrashpadGetModuleInformation(HANDLE process, - HMODULE module, - MODULEINFO* module_info, - DWORD cb) { + HMODULE module, + MODULEINFO* module_info, + DWORD cb) { static const auto get_module_information = GET_FUNCTION_REQUIRED(L"psapi.dll", ::GetModuleInformation); return get_module_information(process, module, module_info, cb); @@ -44,9 +49,10 @@ TEST(PEImageReader, DebugDirectory) { HMODULE self = reinterpret_cast(&__ImageBase); MODULEINFO module_info; ASSERT_TRUE(CrashpadGetModuleInformation( - GetCurrentProcess(), self, &module_info, sizeof(module_info))); + GetCurrentProcess(), self, &module_info, sizeof(module_info))) + << ErrorMessage("GetModuleInformation"); EXPECT_EQ(self, module_info.lpBaseOfDll); - EXPECT_TRUE(pe_image_reader.Initialize(&process_reader, + ASSERT_TRUE(pe_image_reader.Initialize(&process_reader, reinterpret_cast(self), module_info.SizeOfImage, "self")); @@ -61,6 +67,107 @@ TEST(PEImageReader, DebugDirectory) { pdbname.compare(pdbname.size() - suffix.size(), suffix.size(), suffix)); } +void TestVSFixedFileInfo(ProcessReaderWin* process_reader, + const ProcessInfo::Module& module, + bool known_dll) { + PEImageReader pe_image_reader; + ASSERT_TRUE(pe_image_reader.Initialize(process_reader, + module.dll_base, + module.size, + base::UTF16ToUTF8(module.name))); + + VS_FIXEDFILEINFO observed; + const bool observed_rv = pe_image_reader.VSFixedFileInfo(&observed); + ASSERT_TRUE(observed_rv || !known_dll); + + if (observed_rv) { + EXPECT_EQ(VS_FFI_SIGNATURE, observed.dwSignature); + EXPECT_EQ(VS_FFI_STRUCVERSION, observed.dwStrucVersion); + EXPECT_EQ(0, observed.dwFileFlags & ~observed.dwFileFlagsMask); + EXPECT_EQ(VOS_NT_WINDOWS32, observed.dwFileOS); + if (known_dll) { + EXPECT_EQ(VFT_DLL, observed.dwFileType); + } else { + EXPECT_TRUE(observed.dwFileType == VFT_APP || + observed.dwFileType == VFT_DLL); + } + } + + base::FilePath module_path(module.name); + + const DWORD version = GetVersion(); + const int major_version = LOBYTE(LOWORD(version)); + const int minor_version = HIBYTE(LOWORD(version)); + if (major_version > 6 || (major_version == 6 && minor_version >= 2)) { + // Windows 8 or later. + // + // Use BaseName() to ensure that GetModuleVersionAndType() finds the + // already-loaded module with the specified name. Otherwise, dwFileVersionMS + // may not match. This appears to be related to the changes made in Windows + // 8.1 to GetVersion() and GetVersionEx() for non-manifested applications + module_path = module_path.BaseName(); + } + + VS_FIXEDFILEINFO expected; + const bool expected_rv = GetModuleVersionAndType(module_path, &expected); + ASSERT_TRUE(expected_rv || !known_dll); + + EXPECT_EQ(expected_rv, observed_rv); + + if (observed_rv && expected_rv) { + EXPECT_EQ(expected.dwSignature, observed.dwSignature); + EXPECT_EQ(expected.dwStrucVersion, observed.dwStrucVersion); + EXPECT_EQ(expected.dwFileVersionMS, observed.dwFileVersionMS); + EXPECT_EQ(expected.dwFileVersionLS, observed.dwFileVersionLS); + EXPECT_EQ(expected.dwProductVersionMS, observed.dwProductVersionMS); + EXPECT_EQ(expected.dwProductVersionLS, observed.dwProductVersionLS); + EXPECT_EQ(expected.dwFileFlagsMask, observed.dwFileFlagsMask); + EXPECT_EQ(expected.dwFileFlags, observed.dwFileFlags); + EXPECT_EQ(expected.dwFileOS, observed.dwFileOS); + EXPECT_EQ(expected.dwFileType, observed.dwFileType); + EXPECT_EQ(expected.dwFileSubtype, observed.dwFileSubtype); + EXPECT_EQ(expected.dwFileDateMS, observed.dwFileDateMS); + EXPECT_EQ(expected.dwFileDateLS, observed.dwFileDateLS); + } +} + +TEST(PEImageReader, VSFixedFileInfo_OneModule) { + ProcessReaderWin process_reader; + ASSERT_TRUE(process_reader.Initialize(GetCurrentProcess(), + ProcessSuspensionState::kRunning)); + + const wchar_t kModuleName[] = L"kernel32.dll"; + const HMODULE module_handle = GetModuleHandle(kModuleName); + ASSERT_TRUE(module_handle) << ErrorMessage("GetModuleHandle"); + + MODULEINFO module_info; + ASSERT_TRUE(CrashpadGetModuleInformation( + GetCurrentProcess(), module_handle, &module_info, sizeof(module_info))) + << ErrorMessage("GetModuleInformation"); + EXPECT_EQ(module_handle, module_info.lpBaseOfDll); + + ProcessInfo::Module module; + module.name = kModuleName; + module.dll_base = reinterpret_cast(module_info.lpBaseOfDll); + module.size = module_info.SizeOfImage; + + TestVSFixedFileInfo(&process_reader, module, true); +} + +TEST(PEImageReader, VSFixedFileInfo_AllModules) { + ProcessReaderWin process_reader; + ASSERT_TRUE(process_reader.Initialize(GetCurrentProcess(), + ProcessSuspensionState::kRunning)); + + const std::vector& modules = process_reader.Modules(); + EXPECT_GT(modules.size(), 2u); + + for (const auto& module : modules) { + SCOPED_TRACE(base::UTF16ToUTF8(module.name)); + TestVSFixedFileInfo(&process_reader, module, false); + } +} + } // namespace } // namespace test } // namespace crashpad diff --git a/snapshot/win/pe_image_resource_reader.cc b/snapshot/win/pe_image_resource_reader.cc new file mode 100644 index 00000000..3a754a97 --- /dev/null +++ b/snapshot/win/pe_image_resource_reader.cc @@ -0,0 +1,279 @@ +// Copyright 2015 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "snapshot/win/pe_image_resource_reader.h" + +#include + +#include "base/logging.h" +#include "base/memory/scoped_ptr.h" + +namespace { + +void AddLanguageAndNeutralSublanguage(std::vector* languages, + uint16_t language) { + languages->push_back(language); + if (SUBLANGID(language) != SUBLANG_NEUTRAL) { + languages->push_back(MAKELANGID(PRIMARYLANGID(language), SUBLANG_NEUTRAL)); + } +} + +} // namespace + +namespace crashpad { + +PEImageResourceReader::PEImageResourceReader() + : resources_subrange_reader_(), + module_base_(0), + initialized_() { +} + +PEImageResourceReader::~PEImageResourceReader() {} + +bool PEImageResourceReader::Initialize( + const ProcessSubrangeReader& module_subrange_reader, + const IMAGE_DATA_DIRECTORY& resources_directory_entry) { + INITIALIZATION_STATE_SET_INITIALIZING(initialized_); + + module_base_ = module_subrange_reader.Base(); + + if (!resources_subrange_reader_.InitializeSubrange( + module_subrange_reader, + module_base_ + resources_directory_entry.VirtualAddress, + resources_directory_entry.Size, + "resources")) { + return false; + } + + INITIALIZATION_STATE_SET_VALID(initialized_); + return true; +} + +bool PEImageResourceReader::FindResourceByID(uint16_t type, + uint16_t name, + uint16_t language, + WinVMAddress* address, + WinVMSize* size, + uint32_t* code_page) const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + + // The root resource directory is at the beginning of the resources area + // within the module. + const uint32_t name_directory_offset = + GetEntryFromResourceDirectoryByID(0, type, true); + if (!name_directory_offset) { + return false; + } + + const uint32_t language_directory_offset = + GetEntryFromResourceDirectoryByID(name_directory_offset, name, true); + if (!language_directory_offset) { + return false; + } + + // The definition of IMAGE_RESOURCE_DIRECTORY_ENTRY in has a comment + // saying that its offsets are relative to “the resource directory of the data + // associated with this directory entry”. That could be interpreted to mean + // that language_directory_offset is relative to name_directory_offset, since + // the language directory entry is found within the name directory. This is + // not correct. All resource offsets are relative to the resources area within + // the module. + const uint32_t data_offset = GetEntryFromResourceDirectoryByLanguage( + language_directory_offset, language); + if (!data_offset) { + return false; + } + + IMAGE_RESOURCE_DATA_ENTRY data_entry; + if (!resources_subrange_reader_.ReadMemory( + resources_subrange_reader_.Base() + data_offset, + sizeof(data_entry), + &data_entry)) { + LOG(WARNING) << "could not read resource data entry from " + << resources_subrange_reader_.name(); + return false; + } + + // The definition of IMAGE_RESOURCE_DATA_ENTRY in has a comment + // saying that OffsetToData is relative to the beginning of the resource data. + // This is not correct. It’s module-relative. + *address = module_base_ + data_entry.OffsetToData; + *size = data_entry.Size; + if (code_page) { + *code_page = data_entry.CodePage; + } + + return true; +} + +uint32_t PEImageResourceReader::GetEntryFromResourceDirectoryByID( + uint32_t language_directory_offset, + uint16_t id, + bool want_subdirectory) const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + + std::vector entries_by_id; + if (!ReadResourceDirectory( + language_directory_offset, nullptr, nullptr, &entries_by_id)) { + return 0; + } + + const auto entry_it = + std::find_if(entries_by_id.begin(), + entries_by_id.end(), + [id](const IMAGE_RESOURCE_DIRECTORY_ENTRY& entry) { + return !entry.NameIsString && entry.Id == id; + }); + if (entry_it != entries_by_id.end()) { + if ((entry_it->DataIsDirectory != 0) != want_subdirectory) { + LOG(WARNING) << "expected " << (want_subdirectory ? "" : "non-") + << "directory for entry id " << id << " in " + << resources_subrange_reader_.name(); + return 0; + } + + return entry_it->DataIsDirectory ? entry_it->OffsetToDirectory + : entry_it->OffsetToData; + } + + return 0; +} + +uint32_t PEImageResourceReader::GetEntryFromResourceDirectoryByLanguage( + uint32_t resource_directory_offset, + uint16_t language) const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + + std::vector entries_by_language; + if (!ReadResourceDirectory( + resource_directory_offset, nullptr, nullptr, &entries_by_language)) { + return 0; + } + + if (entries_by_language.empty()) { + return 0; + } + + // https://msdn.microsoft.com/en-us/library/cc194810.aspx + // + // TODO(mark): It seems like FindResourceEx() might do something more complex. + // It would be best to mimic its behavior. + std::vector try_languages; + if (PRIMARYLANGID(language) != LANG_NEUTRAL) { + AddLanguageAndNeutralSublanguage(&try_languages, language); + } else { + if (SUBLANGID(language) != SUBLANG_SYS_DEFAULT) { + AddLanguageAndNeutralSublanguage(&try_languages, + LANGIDFROMLCID(GetThreadLocale())); + AddLanguageAndNeutralSublanguage(&try_languages, + LANGIDFROMLCID(GetUserDefaultLCID())); + } + if (SUBLANGID(language) != SUBLANG_DEFAULT) { + AddLanguageAndNeutralSublanguage(&try_languages, + LANGIDFROMLCID(GetSystemDefaultLCID())); + } + } + + try_languages.push_back(MAKELANGID(LANG_NEUTRAL, SUBLANG_NEUTRAL)); + try_languages.push_back(MAKELANGID(LANG_ENGLISH, SUBLANG_DEFAULT)); + + for (const auto try_language : try_languages) { + const auto entry_it = std::find_if( + entries_by_language.begin(), + entries_by_language.end(), + [try_language](const IMAGE_RESOURCE_DIRECTORY_ENTRY& entry) { + return !entry.NameIsString && entry.Id == try_language; + }); + if (entry_it != entries_by_language.end()) { + if (entry_it->DataIsDirectory) { + LOG(WARNING) << "expected non-directory for entry language " + << try_language << " in " + << resources_subrange_reader_.name(); + return 0; + } + + return entry_it->OffsetToData; + } + } + + // Fall back to the first entry in the list. + const auto& entry = entries_by_language.front(); + if (entry.DataIsDirectory) { + LOG(WARNING) << "expected non-directory for entry in " + << resources_subrange_reader_.name(); + return 0; + } + + return entry.OffsetToData; +} + +bool PEImageResourceReader::ReadResourceDirectory( + uint32_t resource_directory_offset, + IMAGE_RESOURCE_DIRECTORY* resource_directory, + std::vector* named_entries, + std::vector* id_entries) const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + + // resource_directory is optional, but it’s still needed locally even if the + // caller isn’t interested in it. + scoped_ptr local_resource_directory; + if (!resource_directory) { + local_resource_directory.reset(new IMAGE_RESOURCE_DIRECTORY); + resource_directory = local_resource_directory.get(); + } + + const WinVMAddress address = + resources_subrange_reader_.Base() + resource_directory_offset; + + if (!resources_subrange_reader_.ReadMemory( + address, sizeof(*resource_directory), resource_directory)) { + LOG(WARNING) << "could not read resource directory from " + << resources_subrange_reader_.name(); + return false; + } + + if (named_entries) { + named_entries->clear(); + named_entries->resize(resource_directory->NumberOfNamedEntries); + if (!named_entries->empty() && + !resources_subrange_reader_.ReadMemory( + address + sizeof(*resource_directory), + named_entries->size() * sizeof((*named_entries)[0]), + &(*named_entries)[0])) { + LOG(WARNING) << "could not read resource directory named entries from " + << resources_subrange_reader_.name(); + return false; + } + } + + if (id_entries) { + id_entries->clear(); + id_entries->resize(resource_directory->NumberOfIdEntries); + if (!id_entries->empty() && + !resources_subrange_reader_.ReadMemory( + address + sizeof(*resource_directory) + + resource_directory->NumberOfNamedEntries * + sizeof(IMAGE_RESOURCE_DIRECTORY_ENTRY), + id_entries->size() * sizeof((*id_entries)[0]), + &(*id_entries)[0])) { + LOG(WARNING) << "could not read resource directory ID entries from " + << resources_subrange_reader_.name(); + return false; + } + } + + return true; +} + +} // namespace crashpad diff --git a/snapshot/win/pe_image_resource_reader.h b/snapshot/win/pe_image_resource_reader.h new file mode 100644 index 00000000..59a6d954 --- /dev/null +++ b/snapshot/win/pe_image_resource_reader.h @@ -0,0 +1,179 @@ +// Copyright 2015 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_SNAPSHOT_WIN_PE_IMAGE_RESOURCE_READER_H_ +#define CRASHPAD_SNAPSHOT_WIN_PE_IMAGE_RESOURCE_READER_H_ + +#include +#include + +#include + +#include "base/basictypes.h" +#include "snapshot/win/process_subrange_reader.h" +#include "util/misc/initialization_state_dcheck.h" +#include "util/win/address_types.h" + +namespace crashpad { + +//! \brief A reader for resources stored in PE images mapped into another +//! process. +//! +//! \sa PEImageReader +class PEImageResourceReader { + public: + PEImageResourceReader(); + ~PEImageResourceReader(); + + //! \brief Initializes the resource reader. + //! + //! \param[in] module_subrange_reader The reader for the module. + //! \param[in] resources_directory_entry The module’s `IMAGE_DATA_DIRECTORY` + //! for its resources area. This is taken from the module’s + //! `IMAGE_OPTIONAL_HEADER::DataDirectory` at index + //! `IMAGE_DIRECTORY_ENTRY_RESOURCE`. + //! + //! \return `true` on success, `false` on failure with a message logged. + bool Initialize(const ProcessSubrangeReader& module_subrange_reader, + const IMAGE_DATA_DIRECTORY& resources_directory_entry); + + //! \brief Locates a resource in a module by its ID. + //! + //! This method is similar to `FindResourceEx()`, but it operates on modules + //! loaded in a remote process’ address space. It is not necessary to + //! `LoadLibrary()` a module into a process in order to use this method. + //! + //! No support is provided at present for locating resources by \a type or \a + //! name using strings as opposed to integer identifiers. + //! + //! Languages are scanned in the order determined by + //! GetEntryFromResourceDirectoryByLanguage(). + //! + //! \param[in] type The integer identifier of the resource type, as in the + //! `lpType` parameter of `FindResourceEx()`. + //! \param[in] name The integer identifier of the resource, as in the `lpName` + //! parameter of `FindResourceEx()`. + //! \param[in] language The language of the resource, as in the `wLanguage` + //! parameter of `FindResourceEx()`. + //! \param[out] address The address, in the remote process’ address space, of + //! the resource data. + //! \param[out] size The size of the resource data. + //! \param[out] code_page The code page used to encode textual resource data. + //! This parameter is optional. + //! + //! \return `true` on success, with the out parameters set appropriately. + //! `false` if the resource was not found, without logging any messages. + //! `false` on failure, with a message logged. + bool FindResourceByID(uint16_t type, + uint16_t name, + uint16_t language, + WinVMAddress* address, + WinVMSize* size, + uint32_t* code_page) const; + + private: + //! \brief Locates a resource directory entry within a resource directory by + //! integer ID. + //! + //! \param[in] resource_directory_offset The offset, in the module’s resources + //! area, of the resource directory to search. + //! \param[in] id The integer identifier of the resource to search for. + //! \param[in] want_subdirectory `true` if the resource directory entry is + //! expected to be a resource directory itself, `false` otherwise. + //! + //! \return The offset, in the module’s resources area, of the entry that was + //! found. On failure, `0`. `0` is technically a valid offset, but it + //! corresponds to the root resource directory, which should never be the + //! offset of another resource directory entry. If \a id was not found, + //! `0` will be returned without logging anything. For other failures, a + //! message will be logged. + uint32_t GetEntryFromResourceDirectoryByID(uint32_t resource_directory_offset, + uint16_t id, + bool want_subdirectory) const; + + //! \brief Locates a resource directory entry within a resource directory by + //! language. + //! + //! This method is similar to GetEntryFromResourceDirectoryByID() with \a + //! want_subdirectory set to `false`. Attempts are made to locate the resource + //! by using these languages: + //!
    + //!
  • If \a language is `LANG_NEUTRAL`:
    • + //!
      + //!
    • Unless `SUBLANG_SYS_DEFAULT` is specified, the language of the + //! thread’s locale, with its normal sublanguage and with + //! `SUBLANG_NEUTRAL`.
      • + //!
    • Unless `SUBLANG_SYS_DEFAULT` is specified, the language of the + //! user’s default locale, with its normal sublanguage and with + //! `SUBLANG_NEUTRAL`.
      • + //!
    • Unless `SUBLANG_DEFAULT` is specified, the language of the + //! system’s default locale, with its normal sublanguage and with + //! `SUBLANG_NEUTRAL`.
      • + //!
    + //!
  • If \a language is not `LANG_NEUTRAL`:
    • + //!
      + //!
    • \a language
      • + //!
    • \a language, with `SUBLANG_NEUTRAL`
      • + //!
    + //!
  • `LANG_NEUTRAL` with `SUBLANG_NEUTRAL`
    • + //!
  • `LANG_ENGLISH` with `SUBLANG_DEFAULT`
    • + //!
  • If none of the above match, the first language found
    • + //!
+ //! + //! If only a specific language is desired without any fallbacks, call + //! GetEntryFromResourceDirectoryByID() with the language directory’s offset + //! instead, passing the desired language in the \a id parameter, and `false` + //! for \a want_subdirectory. + //! + //! \param[in] language_directory_offset The offset, in the module’s resources + //! area, of the resource directory to search. + //! \param[in] language The language of the resource to search for. + //! + //! \return The return value is as in GetEntryFromResourceDirectoryByID(). + uint32_t GetEntryFromResourceDirectoryByLanguage( + uint32_t language_directory_offset, + uint16_t language) const; + + //! \brief Reads a resource directory. + //! + //! \param[in] resource_directory_offset The offset, in the module’s resources + //! area, of the resource directory to read. + //! \param[out] resource_directory The `IMAGE_RESOURCE_DIRECTORY` structure. + //! This parameter is optional. + //! \param[out] named_entries A vector of \a + //! resource_directory->NumberOfNamedEntries + //! `IMAGE_RESOURCE_DIRECTORY_ENTRY` items that follow the resource + //! directory. This parameter is optional. + //! \param[out] id_entries A vector of \a + //! resource_directory->NumberOfIdEntries `IMAGE_RESOURCE_DIRECTORY_ENTRY` + //! items that follow the named entries. This parameter is optional. + //! + //! \return `true` on success, with the out parameters set appropriately. + //! `false` on failure with a message logged. + bool ReadResourceDirectory( + uint32_t resource_directory_offset, + IMAGE_RESOURCE_DIRECTORY* resource_directory, + std::vector* named_entries, + std::vector* id_entries) const; + + ProcessSubrangeReader resources_subrange_reader_; + WinVMAddress module_base_; + InitializationStateDcheck initialized_; + + DISALLOW_COPY_AND_ASSIGN(PEImageResourceReader); +}; + +} // namespace crashpad + +#endif // CRASHPAD_SNAPSHOT_WIN_PE_IMAGE_RESOURCE_READER_H_ diff --git a/snapshot/win/process_reader_win.cc b/snapshot/win/process_reader_win.cc index 373124d6..f3b63237 100644 --- a/snapshot/win/process_reader_win.cc +++ b/snapshot/win/process_reader_win.cc @@ -136,7 +136,7 @@ bool FillThreadContextAndSuspendCount(HANDLE thread_handle, CaptureContext(&thread->context.native); } else { DWORD previous_suspend_count = SuspendThread(thread_handle); - if (previous_suspend_count == -1) { + if (previous_suspend_count == static_cast(-1)) { PLOG(ERROR) << "SuspendThread"; return false; } diff --git a/snapshot/win/process_snapshot_win.cc b/snapshot/win/process_snapshot_win.cc index 4146ae8d..8c184c26 100644 --- a/snapshot/win/process_snapshot_win.cc +++ b/snapshot/win/process_snapshot_win.cc @@ -326,15 +326,16 @@ void ProcessSnapshotWin::InitializePebData( DetermineSizeOfEnvironmentBlock(process_parameters.Environment), &extra_memory_); - // Walk the loader lock which is directly referenced by the PEB. It may or may - // not have a .DebugInfo list, but doesn't on more recent OSs (it does on - // Vista). If it does, then we may walk the lock list more than once, but - // AddMemorySnapshot() will take care of deduplicating the added regions. - ReadLocks(peb_data.LoaderLock, &extra_memory_); + // Walk the loader lock which is directly referenced by the PEB. + ReadLock(peb_data.LoaderLock, &extra_memory_); - // Traverse the locks with valid .DebugInfo if a starting point was supplied. - if (debug_critical_section_address) - ReadLocks(debug_critical_section_address, &extra_memory_); + // TODO(scottmg): Use debug_critical_section_address to walk the list of + // locks (see history of this file for walking code). In some configurations + // this can walk many thousands of locks, so we may want to get some + // annotation from the client for which locks to grab. Unfortunately, without + // walking the list, the !locks command in windbg won't work because it + // requires the lock pointed to by ntdll!RtlCriticalSectionList, which we + // won't have captured. } void ProcessSnapshotWin::AddMemorySnapshot( @@ -380,9 +381,9 @@ void ProcessSnapshotWin::AddMemorySnapshotForLdrLIST_ENTRY( PointerVector* into) { // Walk the doubly-linked list of entries, adding the list memory itself, as // well as pointed-to strings. - Traits::Pointer last = le.Blink; + typename Traits::Pointer last = le.Blink; process_types::LDR_DATA_TABLE_ENTRY entry; - Traits::Pointer cur = le.Flink; + typename Traits::Pointer cur = le.Flink; for (;;) { // |cur| is the pointer to LIST_ENTRY embedded in the LDR_DATA_TABLE_ENTRY. // So we need to offset back to the beginning of the structure. @@ -425,7 +426,7 @@ WinVMSize ProcessSnapshotWin::DetermineSizeOfEnvironmentBlock( } template -void ProcessSnapshotWin::ReadLocks( +void ProcessSnapshotWin::ReadLock( WinVMAddress start, PointerVector* into) { // We're walking the RTL_CRITICAL_SECTION_DEBUG ProcessLocksList, but starting @@ -439,82 +440,17 @@ void ProcessSnapshotWin::ReadLocks( return; } + AddMemorySnapshot( + start, sizeof(process_types::RTL_CRITICAL_SECTION), into); + const decltype(critical_section.DebugInfo) kInvalid = static_cast(-1); if (critical_section.DebugInfo == kInvalid) return; - const WinVMAddress start_address_backward = critical_section.DebugInfo; - WinVMAddress current_address = start_address_backward; - WinVMAddress last_good_address; - - // Typically, this seems to be a circular list, but it's not clear that it - // always is, so follow Blink fields back to the head (or where we started) - // before following Flink to capture memory. - do { - last_good_address = current_address; - // Read the RTL_CRITICAL_SECTION_DEBUG structure to get ProcessLocksList. - process_types::RTL_CRITICAL_SECTION_DEBUG critical_section_debug; - if (!process_reader_.ReadMemory(current_address, - sizeof(critical_section_debug), - &critical_section_debug)) { - LOG(ERROR) << "failed to read RTL_CRITICAL_SECTION_DEBUG"; - return; - } - - if (critical_section_debug.ProcessLocksList.Blink == 0) { - // At the head of the list. - break; - } - - // Move to the previous RTL_CRITICAL_SECTION_DEBUG by walking - // ProcessLocksList.Blink. - current_address = - critical_section_debug.ProcessLocksList.Blink - - offsetof(process_types::RTL_CRITICAL_SECTION_DEBUG, - ProcessLocksList); - } while (current_address != start_address_backward && - current_address != kInvalid); - - if (current_address == kInvalid) { - // Unexpectedly encountered a bad record, so step back one. - current_address = last_good_address; - } - - const WinVMAddress start_address_forward = current_address; - - // current_address is now the head of the list, walk Flink to add the whole - // list. - do { - // Read the RTL_CRITICAL_SECTION_DEBUG structure to get ProcessLocksList. - process_types::RTL_CRITICAL_SECTION_DEBUG critical_section_debug; - if (!process_reader_.ReadMemory(current_address, - sizeof(critical_section_debug), - &critical_section_debug)) { - LOG(ERROR) << "failed to read RTL_CRITICAL_SECTION_DEBUG"; - return; - } - - // Add both RTL_CRITICAL_SECTION_DEBUG and RTL_CRITICAL_SECTION to the extra - // memory to be saved. - AddMemorySnapshot(current_address, - sizeof(process_types::RTL_CRITICAL_SECTION_DEBUG), - into); - AddMemorySnapshot(critical_section_debug.CriticalSection, - sizeof(process_types::RTL_CRITICAL_SECTION), - into); - - if (critical_section_debug.ProcessLocksList.Flink == 0) - break; - - // Move to the next RTL_CRITICAL_SECTION_DEBUG by walking - // ProcessLocksList.Flink. - current_address = - critical_section_debug.ProcessLocksList.Flink - - offsetof(process_types::RTL_CRITICAL_SECTION_DEBUG, - ProcessLocksList); - } while (current_address != start_address_forward && - current_address != kInvalid); + AddMemorySnapshot(critical_section.DebugInfo, + sizeof(process_types::RTL_CRITICAL_SECTION_DEBUG), + into); } } // namespace crashpad diff --git a/snapshot/win/process_snapshot_win.h b/snapshot/win/process_snapshot_win.h index a7238fd7..a1dd7233 100644 --- a/snapshot/win/process_snapshot_win.h +++ b/snapshot/win/process_snapshot_win.h @@ -168,13 +168,11 @@ class ProcessSnapshotWin final : public ProcessSnapshot { WinVMSize DetermineSizeOfEnvironmentBlock( WinVMAddress start_of_environment_block); - // Starting from the address of a CRITICAL_SECTION, walks the doubly-linked - // list stored in RTL_CRITICAL_SECTION.DebugInfo.ProcessLocksList adding both - // the RTL_CRITICAL_SECTION and the RTL_CRITICAL_SECTION_DEBUG memory blocks - // to the snapshot. + // Starting from the address of a CRITICAL_SECTION, add a lock and, if valid, + // its .DebugInfo field to the snapshot. template - void ReadLocks(WinVMAddress start, - PointerVector* into); + void ReadLock(WinVMAddress start, + PointerVector* into); internal::SystemSnapshotWin system_; PointerVector extra_memory_; diff --git a/snapshot/win/process_subrange_reader.cc b/snapshot/win/process_subrange_reader.cc new file mode 100644 index 00000000..996fbdf3 --- /dev/null +++ b/snapshot/win/process_subrange_reader.cc @@ -0,0 +1,104 @@ +// Copyright 2015 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "snapshot/win/process_subrange_reader.h" + +#include "base/logging.h" +#include "snapshot/win/process_reader_win.h" + +namespace crashpad { + +ProcessSubrangeReader::ProcessSubrangeReader() + : name_(), + range_(), + process_reader_(nullptr) { +} + +ProcessSubrangeReader::~ProcessSubrangeReader() { +} + +bool ProcessSubrangeReader::Initialize(ProcessReaderWin* process_reader, + WinVMAddress base, + WinVMSize size, + const std::string& name) { + INITIALIZATION_STATE_SET_INITIALIZING(initialized_); + + if (!InitializeInternal(process_reader, base, size, name)) { + return false; + } + + INITIALIZATION_STATE_SET_VALID(initialized_); + return true; +} + +bool ProcessSubrangeReader::InitializeSubrange( + const ProcessSubrangeReader& that, + WinVMAddress base, + WinVMSize size, + const std::string& sub_name) { + INITIALIZATION_STATE_SET_INITIALIZING(initialized_); + INITIALIZATION_STATE_DCHECK_VALID(that.initialized_); + + if (!InitializeInternal( + that.process_reader_, base, size, that.name_ + " " + sub_name)) { + return false; + } + + if (!that.range_.ContainsRange(range_)) { + LOG(WARNING) << "range " << range_.AsString() << " outside of range " + << that.range_.AsString() << " for " << name_; + return false; + } + + INITIALIZATION_STATE_SET_VALID(initialized_); + return true; +} + +bool ProcessSubrangeReader::ReadMemory(WinVMAddress address, + WinVMSize size, + void* into) const { + INITIALIZATION_STATE_DCHECK_VALID(initialized_); + + CheckedWinAddressRange read_range(process_reader_->Is64Bit(), address, size); + if (!read_range.IsValid()) { + LOG(WARNING) << "invalid read range " << read_range.AsString(); + return false; + } + + if (!range_.ContainsRange(read_range)) { + LOG(WARNING) << "attempt to read outside of " << name_ << " range " + << range_.AsString() << " at range " << read_range.AsString(); + return false; + } + + return process_reader_->ReadMemory(address, size, into); +} + +bool ProcessSubrangeReader::InitializeInternal(ProcessReaderWin* process_reader, + WinVMAddress base, + WinVMSize size, + const std::string& name) { + range_.SetRange(process_reader->Is64Bit(), base, size); + if (!range_.IsValid()) { + LOG(WARNING) << "invalid range " << range_.AsString() << " for " << name; + return false; + } + + name_ = name; + process_reader_ = process_reader; + + return true; +} + +} // namespace crashpad diff --git a/snapshot/win/process_subrange_reader.h b/snapshot/win/process_subrange_reader.h new file mode 100644 index 00000000..e8da592c --- /dev/null +++ b/snapshot/win/process_subrange_reader.h @@ -0,0 +1,114 @@ +// Copyright 2015 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_SNAPSHOT_WIN_PROCESS_SUBRANGE_READER_WIN_H_ +#define CRASHPAD_SNAPSHOT_WIN_PROCESS_SUBRANGE_READER_WIN_H_ + +#include + +#include "base/basictypes.h" +#include "util/misc/initialization_state_dcheck.h" +#include "util/win/address_types.h" +#include "util/win/checked_win_address_range.h" + +namespace crashpad { + +class ProcessReaderWin; + +//! \brief A wrapper for ProcessReaderWin that only allows a specific subrange +//! to be read from. +//! +//! This class is useful to restrict reads to a specific address range, such as +//! the address range occupied by a loaded module, or a specific section within +//! a module. +class ProcessSubrangeReader { + public: + ProcessSubrangeReader(); + ~ProcessSubrangeReader(); + + //! \brief Initializes the object. + //! + //! \param[in] process_reader A reader for a remote process. + //! \param[in] base The base address for the range that reads should be + //! restricted to. + //! \param[in] size The size of the range that reads should be restricted to. + //! \param[in] name The range’s name, a string to be used in logged messages. + //! This string is for diagnostic purposes. + //! + //! \return `true` on success, `false` on failure with a message logged. The + //! other methods in this class must not be called unless this method or + //! InitializeSubrange() has returned true. + bool Initialize(ProcessReaderWin* process_reader, + WinVMAddress base, + WinVMSize size, + const std::string& name); + + //! \brief Initializes the object to a subrange of an existing + //! ProcessSubrangeReader. + //! + //! The subrange identified by \a base and \a size must be contained within + //! the subrange in \a that. + //! + //! \param[in] that The existing ProcessSubrangeReader to base the new object + //! on. + //! \param[in] base The base address for the range that reads should be + //! restricted to. + //! \param[in] size The size of the range that reads should be restricted to. + //! \param[in] sub_name A description of the subrange, which will be appended + //! to the \a name in \a that and used in logged messages. This string is + //! for diagnostic purposes. + //! + //! \return `true` on success, `false` on failure with a message logged. The + //! other methods in this class must not be called unless this method or + //! Initialize() has returned true. + bool InitializeSubrange(const ProcessSubrangeReader& that, + WinVMAddress base, + WinVMSize size, + const std::string& sub_name); + + bool Is64Bit() const { return range_.Is64Bit(); } + WinVMAddress Base() const { return range_.Base(); } + WinVMAddress Size() const { return range_.Size(); } + const std::string& name() const { return name_; } + + //! \brief Reads memory from the remote process. + //! + //! The range specified by \a address and \a size must be contained within + //! the range that this object is permitted to read. + //! + //! \param[in] address The address to read from. + //! \param[in] size The size of data to read, in bytes. + //! \param[out] into The buffer to read data into. + //! + //! \return `true` on success, `false` on failure with a message logged. + bool ReadMemory(WinVMAddress address, WinVMSize size, void* into) const; + + private: + // Common helper for Initialize() and InitializeSubrange(). + bool InitializeInternal(ProcessReaderWin* process_reader, + WinVMAddress base, + WinVMSize size, + const std::string& name); + + std::string name_; + CheckedWinAddressRange range_; + ProcessReaderWin* process_reader_; // weak + InitializationStateDcheck initialized_; + + DISALLOW_COPY_AND_ASSIGN(ProcessSubrangeReader); +}; + +} // namespace crashpad + +#endif // CRASHPAD_SNAPSHOT_WIN_PROCESS_SUBRANGE_READER_WIN_H_ diff --git a/snapshot/win/system_snapshot_win.cc b/snapshot/win/system_snapshot_win.cc index 88827a5d..a62bf5ac 100644 --- a/snapshot/win/system_snapshot_win.cc +++ b/snapshot/win/system_snapshot_win.cc @@ -86,35 +86,37 @@ void SystemSnapshotWin::Initialize(ProcessReaderWin* process_reader) { process_reader_ = process_reader; - // We use both GetVersionEx and VerQueryValue. GetVersionEx is not trustworthy - // after Windows 8 (depending on the application manifest) so its data is used - // only to fill the os_server_ field, and the rest comes from the version + // We use both GetVersionEx() and GetModuleVersionAndType() (which uses + // VerQueryValue() internally). GetVersionEx() is not trustworthy after + // Windows 8 (depending on the application manifest) so its data is used only + // to fill the os_server_ field, and the rest comes from the version // information stamped on kernel32.dll. OSVERSIONINFOEX version_info = {sizeof(version_info)}; if (!GetVersionEx(reinterpret_cast(&version_info))) { PLOG(WARNING) << "GetVersionEx"; } else { - const wchar_t kSystemDll[] = L"kernel32.dll"; - VS_FIXEDFILEINFO ffi; - if (GetModuleVersionAndType(base::FilePath(kSystemDll), &ffi)) { - std::string flags_string = GetStringForFileFlags(ffi.dwFileFlags); - os_server_ = version_info.wProductType != VER_NT_WORKSTATION; - std::string os_name = GetStringForFileOS(ffi.dwFileOS); - os_version_major_ = ffi.dwFileVersionMS >> 16; - os_version_minor_ = ffi.dwFileVersionMS & 0xffff; - os_version_bugfix_ = ffi.dwFileVersionLS >> 16; - os_version_build_ = - base::StringPrintf("%d", ffi.dwFileVersionLS & 0xffff); - os_version_full_ = base::StringPrintf( - "%s %d.%d.%d.%s%s", - os_name.c_str(), - os_version_major_, - os_version_minor_, - os_version_bugfix_, - os_version_build_.c_str(), - flags_string.empty() ? "" : (std::string(" (") + flags_string + ")") - .c_str()); - } + os_server_ = version_info.wProductType != VER_NT_WORKSTATION; + } + + const wchar_t kSystemDll[] = L"kernel32.dll"; + VS_FIXEDFILEINFO ffi; + if (GetModuleVersionAndType(base::FilePath(kSystemDll), &ffi)) { + std::string flags_string = GetStringForFileFlags(ffi.dwFileFlags); + std::string os_name = GetStringForFileOS(ffi.dwFileOS); + os_version_major_ = ffi.dwFileVersionMS >> 16; + os_version_minor_ = ffi.dwFileVersionMS & 0xffff; + os_version_bugfix_ = ffi.dwFileVersionLS >> 16; + os_version_build_ = + base::StringPrintf("%d", ffi.dwFileVersionLS & 0xffff); + os_version_full_ = base::StringPrintf( + "%s %d.%d.%d.%s%s", + os_name.c_str(), + os_version_major_, + os_version_minor_, + os_version_bugfix_, + os_version_build_.c_str(), + flags_string.empty() ? "" : (std::string(" (") + flags_string + ")") + .c_str()); } INITIALIZATION_STATE_SET_VALID(initialized_); diff --git a/test/mac/mach_multiprocess.cc b/test/mac/mach_multiprocess.cc index 641eeba0..8c122c4a 100644 --- a/test/mac/mach_multiprocess.cc +++ b/test/mac/mach_multiprocess.cc @@ -207,7 +207,7 @@ void MachMultiprocess::MultiprocessParent() { } void MachMultiprocess::MultiprocessChild() { - ScopedForbidReturn forbid_return;; + ScopedForbidReturn forbid_return; // local_port is not valid in the forked child process. ignore_result(info_->local_port.release()); diff --git a/test/win/win_child_process.cc b/test/win/win_child_process.cc index 6b7a1a07..41cacfa7 100644 --- a/test/win/win_child_process.cc +++ b/test/win/win_child_process.cc @@ -18,6 +18,7 @@ #include #include +#include #include "base/logging.h" #include "base/strings/stringprintf.h" @@ -125,8 +126,8 @@ bool CreateInheritablePipe(ScopedFileHANDLE* read_handle, if (!write_inheritable && !UnsetHandleInheritance(temp_write.get())) return false; - *read_handle = temp_read.Pass(); - *write_handle = temp_write.Pass(); + *read_handle = std::move(temp_read); + *write_handle = std::move(temp_write); return true; } @@ -212,7 +213,7 @@ scoped_ptr WinChildProcess::Launch() { return scoped_ptr(); } - return handles_for_parent.Pass(); + return std::move(handles_for_parent); } FileHandle WinChildProcess::ReadPipeHandle() const { diff --git a/tools/crashpad_database_util.cc b/tools/crashpad_database_util.cc index 433b4769..a1ad52b5 100644 --- a/tools/crashpad_database_util.cc +++ b/tools/crashpad_database_util.cc @@ -22,6 +22,7 @@ #include #include +#include #include #include "base/basictypes.h" @@ -552,7 +553,7 @@ int DatabaseUtilMain(int argc, char* argv[]) { return EXIT_FAILURE; } - file_reader = file_path_reader.Pass(); + file_reader = std::move(file_path_reader); } CrashReportDatabase::NewReport* new_report; diff --git a/util/mach/child_port_handshake.cc b/util/mach/child_port_handshake.cc index e7cc7eed..a7b6e57c 100644 --- a/util/mach/child_port_handshake.cc +++ b/util/mach/child_port_handshake.cc @@ -23,6 +23,7 @@ #include #include +#include #include "base/logging.h" #include "base/mac/mach_logging.h" @@ -352,30 +353,30 @@ ChildPortHandshake::~ChildPortHandshake() { base::ScopedFD ChildPortHandshake::ClientReadFD() { DCHECK(client_read_fd_.is_valid()); - return client_read_fd_.Pass(); + return std::move(client_read_fd_); } base::ScopedFD ChildPortHandshake::ServerWriteFD() { DCHECK(server_write_fd_.is_valid()); - return server_write_fd_.Pass(); + return std::move(server_write_fd_); } mach_port_t ChildPortHandshake::RunServer(PortRightType port_right_type) { client_read_fd_.reset(); - return RunServerForFD(server_write_fd_.Pass(), port_right_type); + return RunServerForFD(std::move(server_write_fd_), port_right_type); } bool ChildPortHandshake::RunClient(mach_port_t port, mach_msg_type_name_t right_type) { server_write_fd_.reset(); - return RunClientForFD(client_read_fd_.Pass(), port, right_type); + return RunClientForFD(std::move(client_read_fd_), port, right_type); } // static mach_port_t ChildPortHandshake::RunServerForFD(base::ScopedFD server_write_fd, PortRightType port_right_type) { ChildPortHandshakeServer server; - return server.RunServer(server_write_fd.Pass(), port_right_type); + return server.RunServer(std::move(server_write_fd), port_right_type); } // static diff --git a/util/mach/child_port_handshake.h b/util/mach/child_port_handshake.h index 8956daf7..666f59b1 100644 --- a/util/mach/child_port_handshake.h +++ b/util/mach/child_port_handshake.h @@ -138,7 +138,7 @@ class ChildPortHandshakeTest; //! // Obtain a receive right from the parent process. //! base::mac::ScopedMachReceiveRight receive_right( //! ChildPortHandshake::RunServerForFD( -//! server_write_fd.Pass(), +//! std::move(server_write_fd), //! ChildPortHandshake::PortRightType::kReceiveRight)); //! } //! \endcode diff --git a/util/mach/task_memory_test.cc b/util/mach/task_memory_test.cc index 2247ed2f..963bc4da 100644 --- a/util/mach/task_memory_test.cc +++ b/util/mach/task_memory_test.cc @@ -445,7 +445,7 @@ bool IsAddressMapped(vm_address_t address) { return false; } - ADD_FAILURE() << MachErrorMessage(kr, "vm_region_64");; + ADD_FAILURE() << MachErrorMessage(kr, "vm_region_64"); return false; } diff --git a/util/net/http_transport.cc b/util/net/http_transport.cc index f11ee95c..8587d0aa 100644 --- a/util/net/http_transport.cc +++ b/util/net/http_transport.cc @@ -14,6 +14,8 @@ #include "util/net/http_transport.h" +#include + #include "util/net/http_body.h" namespace crashpad { @@ -43,7 +45,7 @@ void HTTPTransport::SetHeader(const std::string& header, } void HTTPTransport::SetBodyStream(scoped_ptr stream) { - body_stream_ = stream.Pass(); + body_stream_ = std::move(stream); } void HTTPTransport::SetTimeout(double timeout) { diff --git a/util/net/http_transport_test.cc b/util/net/http_transport_test.cc index 16b1cd0a..46e7a5f7 100644 --- a/util/net/http_transport_test.cc +++ b/util/net/http_transport_test.cc @@ -18,6 +18,7 @@ #include #include +#include #include #include "base/files/file_path.h" @@ -51,7 +52,7 @@ class HTTPTransportTestFixture : public MultiprocessExec { RequestValidator request_validator) : MultiprocessExec(), headers_(headers), - body_stream_(body_stream.Pass()), + body_stream_(std::move(body_stream)), response_code_(http_response_code), request_validator_(request_validator) { base::FilePath server_path = Paths::TestDataRoot().Append( @@ -102,7 +103,7 @@ class HTTPTransportTestFixture : public MultiprocessExec { for (const auto& pair : headers_) { transport->SetHeader(pair.first, pair.second); } - transport->SetBodyStream(body_stream_.Pass()); + transport->SetBodyStream(std::move(body_stream_)); std::string response_body; bool success = transport->ExecuteSynchronously(&response_body); @@ -270,8 +271,8 @@ TEST(HTTPTransport, UnchunkedPlainText) { headers[kContentType] = kTextPlain; headers[kContentLength] = base::StringPrintf("%" PRIuS, strlen(kTextBody)); - HTTPTransportTestFixture test(headers, body_stream.Pass(), 200, - &UnchunkedPlainText); + HTTPTransportTestFixture test( + headers, std::move(body_stream), 200, &UnchunkedPlainText); test.Run(); } @@ -291,7 +292,10 @@ void RunUpload33k(bool has_content_length) { headers[kContentLength] = base::StringPrintf("%" PRIuS, request_string.size()); } - HTTPTransportTestFixture test(headers, body_stream.Pass(), 200, + HTTPTransportTestFixture test( + headers, + std::move(body_stream), + 200, [](HTTPTransportTestFixture* fixture, const std::string& request) { size_t body_start = request.rfind("\r\n"); EXPECT_EQ(33 * 1024u + 2, request.size() - body_start); diff --git a/util/numeric/checked_address_range.cc b/util/numeric/checked_address_range.cc index dcbde23e..1d50696f 100644 --- a/util/numeric/checked_address_range.cc +++ b/util/numeric/checked_address_range.cc @@ -14,6 +14,8 @@ #include "util/numeric/checked_address_range.h" +#include "base/strings/stringprintf.h" + #if defined(OS_MACOSX) #include #elif defined(OS_WIN) @@ -109,6 +111,12 @@ bool CheckedAddressRangeGeneric::ContainsRange( : range_32_.ContainsRange(that.range_32_); } +template +std::string CheckedAddressRangeGeneric::AsString() const { + return base::StringPrintf( + "0x%llx + 0x%llx (%s)", Base(), Size(), Is64Bit() ? "64" : "32"); +} + // Explicit instantiations for the cases we use. #if defined(OS_MACOSX) template class CheckedAddressRangeGeneric; diff --git a/util/numeric/checked_address_range.h b/util/numeric/checked_address_range.h index e9514bbd..65f85fbd 100644 --- a/util/numeric/checked_address_range.h +++ b/util/numeric/checked_address_range.h @@ -17,6 +17,8 @@ #include +#include + #include "build/build_config.h" #include "util/numeric/checked_range.h" @@ -108,6 +110,12 @@ class CheckedAddressRangeGeneric { //! CheckedAddressRangeGeneric objects involved. bool ContainsRange(const CheckedAddressRangeGeneric& that) const; + //! \brief Returns a string describing the address range. + //! + //! The string will be formatted as `"0x123 + 0x45 (64)"`, where the + //! individual components are the address, size, and bitness. + std::string AsString() const; + private: #if defined(COMPILER_MSVC) // MSVC cannot handle a union containing CheckedRange (with constructor, etc.) diff --git a/util/stdlib/aligned_allocator.cc b/util/stdlib/aligned_allocator.cc new file mode 100644 index 00000000..363022c8 --- /dev/null +++ b/util/stdlib/aligned_allocator.cc @@ -0,0 +1,78 @@ +// 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/stdlib/aligned_allocator.h" + +#include + +#include "build/build_config.h" + +#if defined(OS_POSIX) +#include +#elif defined(OS_WIN) +#include +#include +#endif // OS_POSIX + +namespace { + +// Throws std::bad_alloc() by calling an internal function provided by the C++ +// library to do so. This works even if C++ exceptions are disabled, causing +// program termination if uncaught. +void ThrowBadAlloc() { +#if defined(OS_POSIX) + // This works with both libc++ and libstdc++. + std::__throw_bad_alloc(); +#elif defined(OS_WIN) + std::_Xbad_alloc(); +#endif // OS_POSIX +} + +} // namespace + +namespace crashpad { +namespace internal { + +void* AlignedAllocate(size_t alignment, size_t size) { +#if defined(OS_POSIX) + // posix_memalign() requires that alignment be at least sizeof(void*), so the + // power-of-2 check needs to happen before potentially changing the alignment. + if (alignment == 0 || alignment & (alignment - 1)) { + ThrowBadAlloc(); + } + + void* pointer; + if (posix_memalign(&pointer, std::max(alignment, sizeof(void*)), size) != 0) { + ThrowBadAlloc(); + } +#elif defined(OS_WIN) + void* pointer = _aligned_malloc(size, alignment); + if (pointer == nullptr) { + ThrowBadAlloc(); + } +#endif // OS_POSIX + + return pointer; +} + +void AlignedFree(void* pointer) { +#if defined(OS_POSIX) + free(pointer); +#elif defined(OS_WIN) + _aligned_free(pointer); +#endif // OS_POSIX +} + +} // namespace internal +} // namespace crashpad diff --git a/util/stdlib/aligned_allocator.h b/util/stdlib/aligned_allocator.h new file mode 100644 index 00000000..04d3dc46 --- /dev/null +++ b/util/stdlib/aligned_allocator.h @@ -0,0 +1,140 @@ +// 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_STDLIB_ALIGNED_ALLOCATOR_H_ +#define CRASHPAD_UTIL_STDLIB_ALIGNED_ALLOCATOR_H_ + +#include + +#include +#include +#include +#include +#include + +#include "base/compiler_specific.h" +#include "build/build_config.h" +#include "util/stdlib/cxx.h" + +#if defined(COMPILER_MSVC) && _MSC_VER < 1900 +#define CRASHPAD_NOEXCEPT _NOEXCEPT +#else +#define CRASHPAD_NOEXCEPT noexcept +#endif + +namespace crashpad { +namespace internal { + +//! \brief Allocates memory with the specified alignment constraint. +//! +//! This function wraps `posix_memalign()` or `_aligned_malloc()`. Memory +//! allocated by this function must be released by AlignFree(). +void* AlignedAllocate(size_t alignment, size_t size); + +//! \brief Frees memory allocated by AlignedAllocate(). +//! +//! This function wraps `free()` or `_aligned_free()`. +void AlignedFree(void* pointer); + +} // namespace internal + +//! \brief A standard allocator that aligns its allocations as requested, +//! suitable for use as an allocator in standard containers. +//! +//! This is similar to `std::allocator`, with the addition of an alignment +//! guarantee. \a Alignment must be a power of 2. If \a Alignment is not +//! specified, the default alignment for type \a T is used. +template +struct AlignedAllocator { + public: + using value_type = T; + using pointer = T*; + using const_pointer = const T*; + using reference = T&; + using const_reference = const T&; + using size_type = size_t; + using difference_type = ptrdiff_t; + + template + struct rebind { + using other = AlignedAllocator; + }; + + AlignedAllocator() CRASHPAD_NOEXCEPT {} + AlignedAllocator(const AlignedAllocator& other) CRASHPAD_NOEXCEPT {} + + template + AlignedAllocator(const AlignedAllocator& other) + CRASHPAD_NOEXCEPT {} + + ~AlignedAllocator() {} + + pointer address(reference x) const CRASHPAD_NOEXCEPT { return &x; } + const_pointer address(const_reference x) const CRASHPAD_NOEXCEPT { + return &x; + } + + pointer allocate(size_type n, std::allocator::const_pointer hint = 0) { + return reinterpret_cast( + internal::AlignedAllocate(Alignment, sizeof(value_type) * n)); + } + + void deallocate(pointer p, size_type n) { internal::AlignedFree(p); } + + size_type max_size() const CRASHPAD_NOEXCEPT { + return std::numeric_limits::max() / sizeof(value_type); + } + +#if CXX_LIBRARY_VERSION < 2011 + void construct(pointer p, const T& val) { + new (reinterpret_cast(p)) T(val); + } +#else + template + void construct(U* p, Args&&... args) { + new (reinterpret_cast(p)) U(std::forward(args)...); + } +#endif + + template + void destroy(U* p) { + p->~U(); + } +}; + +template +bool operator==(const AlignedAllocator& lhs, + const AlignedAllocator& rhs) CRASHPAD_NOEXCEPT { + return true; +} + +template +bool operator!=(const AlignedAllocator& lhs, + const AlignedAllocator& rhs) CRASHPAD_NOEXCEPT { + return false; +} + +//! \brief A `std::vector` using AlignedAllocator. +//! +//! This is similar to `std::vector`, with the addition of an alignment +//! guarantee. \a Alignment must be a power of 2. If \a Alignment is not +//! specified, the default alignment for type \a T is used. +template +using AlignedVector = std::vector>; + +} // namespace crashpad + +#undef CRASHPAD_NOEXCEPT + +#endif // CRASHPAD_UTIL_STDLIB_ALIGNED_ALLOCATOR_H_ diff --git a/util/stdlib/aligned_allocator_test.cc b/util/stdlib/aligned_allocator_test.cc new file mode 100644 index 00000000..fe3b9e65 --- /dev/null +++ b/util/stdlib/aligned_allocator_test.cc @@ -0,0 +1,117 @@ +// 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/stdlib/aligned_allocator.h" + +#include + +#include "gtest/gtest.h" + +#if defined(OS_WIN) +#include +#endif + +namespace crashpad { +namespace test { +namespace { + +bool IsAligned(void* pointer, size_t alignment) { + uintptr_t address = reinterpret_cast(pointer); + return (address & (alignment - 1)) == 0; +} + +TEST(AlignedAllocator, AlignedVector) { + // Test a structure with natural alignment. + struct NaturalAlignedStruct { + int i; + }; + + AlignedVector natural_aligned_vector; + natural_aligned_vector.push_back(NaturalAlignedStruct()); + EXPECT_TRUE( + IsAligned(&natural_aligned_vector[0], ALIGNOF(NaturalAlignedStruct))); + + natural_aligned_vector.resize(3); + EXPECT_TRUE( + IsAligned(&natural_aligned_vector[0], ALIGNOF(NaturalAlignedStruct))); + EXPECT_TRUE( + IsAligned(&natural_aligned_vector[1], ALIGNOF(NaturalAlignedStruct))); + EXPECT_TRUE( + IsAligned(&natural_aligned_vector[2], ALIGNOF(NaturalAlignedStruct))); + + // Test a structure that declares its own alignment. + struct ALIGNAS(32) AlignedStruct { + int i; + }; + ASSERT_EQ(32u, ALIGNOF(AlignedStruct)); + + AlignedVector aligned_vector; + aligned_vector.push_back(AlignedStruct()); + EXPECT_TRUE(IsAligned(&aligned_vector[0], ALIGNOF(AlignedStruct))); + + aligned_vector.resize(3); + EXPECT_TRUE(IsAligned(&aligned_vector[0], ALIGNOF(AlignedStruct))); + EXPECT_TRUE(IsAligned(&aligned_vector[1], ALIGNOF(AlignedStruct))); + EXPECT_TRUE(IsAligned(&aligned_vector[2], ALIGNOF(AlignedStruct))); + + // Try a custom alignment. Since the structure itself doesn’t specify an + // alignment constraint, only the base address will be aligned to the + // requested boundary. + AlignedVector custom_aligned_vector; + custom_aligned_vector.push_back(NaturalAlignedStruct()); + EXPECT_TRUE(IsAligned(&custom_aligned_vector[0], 64)); + + // Try a structure with a pretty big alignment request. + struct ALIGNAS(1024) BigAlignedStruct { + int i; + }; + ASSERT_EQ(1024u, ALIGNOF(BigAlignedStruct)); + + AlignedVector big_aligned_vector; + big_aligned_vector.push_back(BigAlignedStruct()); + EXPECT_TRUE(IsAligned(&big_aligned_vector[0], ALIGNOF(BigAlignedStruct))); + + big_aligned_vector.resize(3); + EXPECT_TRUE(IsAligned(&big_aligned_vector[0], ALIGNOF(BigAlignedStruct))); + EXPECT_TRUE(IsAligned(&big_aligned_vector[1], ALIGNOF(BigAlignedStruct))); + EXPECT_TRUE(IsAligned(&big_aligned_vector[2], ALIGNOF(BigAlignedStruct))); +} + +void BadAlignmentTest() { +#if defined(OS_WIN) + // Suppress the assertion MessageBox() normally displayed by the CRT in debug + // mode. + int previous = _CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_DEBUG); + + // In release mode, _CrtSetReportMode() is #defined to ((int)0), so |previous| + // would appear unused. + ALLOW_UNUSED_LOCAL(previous); +#endif + + // Alignment constraints must be powers of 2. 7 is not valid. + AlignedVector bad_aligned_vector; + bad_aligned_vector.push_back(0); + +#if defined(OS_WIN) + _CrtSetReportMode(_CRT_ASSERT, previous); +#endif +} + +TEST(AlignedAllocatorDeathTest, BadAlignment) { + ASSERT_DEATH(BadAlignmentTest(), ""); +} + +} // namespace +} // namespace test +} // namespace crashpad diff --git a/util/util.gyp b/util/util.gyp index b4919652..83f2c559 100644 --- a/util/util.gyp +++ b/util/util.gyp @@ -128,6 +128,8 @@ 'posix/process_info_mac.cc', 'posix/symbolic_constants_posix.cc', 'posix/symbolic_constants_posix.h', + 'stdlib/aligned_allocator.cc', + 'stdlib/aligned_allocator.h', 'stdlib/cxx.h', 'stdlib/map_insert.h', 'stdlib/objc.h', diff --git a/util/util_test.gyp b/util/util_test.gyp index 2d94ea71..f5adc039 100644 --- a/util/util_test.gyp +++ b/util/util_test.gyp @@ -72,6 +72,7 @@ 'numeric/int128_test.cc', 'posix/process_info_test.cc', 'posix/symbolic_constants_posix_test.cc', + 'stdlib/aligned_allocator_test.cc', 'stdlib/map_insert_test.cc', 'stdlib/string_number_conversion_test.cc', 'stdlib/strlcpy_test.cc', diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc index 05087f1e..5559efbe 100644 --- a/util/win/exception_handler_server.cc +++ b/util/win/exception_handler_server.cc @@ -17,6 +17,8 @@ #include #include +#include + #include "base/logging.h" #include "base/numerics/safe_conversions.h" #include "base/rand_util.h" @@ -179,7 +181,7 @@ class ClientData { CreateEvent(nullptr, false /* auto reset */, false, nullptr)), non_crash_dump_completed_event_( CreateEvent(nullptr, false /* auto reset */, false, nullptr)), - process_(process.Pass()), + process_(std::move(process)), crash_exception_information_address_( crash_exception_information_address), non_crash_exception_information_address_( @@ -346,7 +348,7 @@ std::wstring ExceptionHandlerServer::CreatePipe() { void ExceptionHandlerServer::Run(Delegate* delegate) { uint64_t shutdown_token = base::RandUint64(); ScopedKernelHANDLE thread_handles[kPipeInstances]; - for (int i = 0; i < arraysize(thread_handles); ++i) { + for (size_t i = 0; i < arraysize(thread_handles); ++i) { HANDLE pipe; if (first_pipe_instance_.is_valid()) { pipe = first_pipe_instance_.release(); @@ -398,7 +400,7 @@ void ExceptionHandlerServer::Run(Delegate* delegate) { } // Signal to the named pipe instances that they should terminate. - for (int i = 0; i < arraysize(thread_handles); ++i) { + for (size_t i = 0; i < arraysize(thread_handles); ++i) { ClientToServerMessage message; memset(&message, 0, sizeof(message)); message.type = ClientToServerMessage::kShutdown; diff --git a/util/win/module_version.h b/util/win/module_version.h index be56eb9d..afc2a508 100644 --- a/util/win/module_version.h +++ b/util/win/module_version.h @@ -24,9 +24,15 @@ namespace crashpad { //! \brief Retrieve the type and version information from a given module (exe, //! dll, etc.) //! +//! This function calls `GetFileVersionInfo()`, which can implicitly call +//! `LoadLibrary()` to load \a path into the calling process. Do not call this +//! function on an untrusted module, because there is a risk of executing the +//! module’s code. +//! //! \param[in] path The path to the module to be inspected. -//! \param[out] vs_fixedfileinfo The `VS_FIXEDFILEINFO` on success. -//! `dwFileFlags` will have been masked with `dwFileFlagsMask` already. +//! \param[out] vs_fixedfileinfo The VS_FIXEDFILEINFO on success. +//! VS_FIXEDFILEINFO::dwFileFlags will have been masked with +//! VS_FIXEDFILEINFO::dwFileFlagsMask already. //! //! \return `true` on success, or `false` on failure with a message logged. If //! the module has no `VERSIONINFO` resource, `false` will be returned diff --git a/util/win/process_info.cc b/util/win/process_info.cc index 3e048cf0..5628e04b 100644 --- a/util/win/process_info.cc +++ b/util/win/process_info.cc @@ -150,11 +150,11 @@ scoped_ptr QueryObject( if (!NT_SUCCESS(status)) { NTSTATUS_LOG(ERROR, status) << "NtQueryObject"; - return scoped_ptr(); + return nullptr; } DCHECK_GE(return_length, minimum_size); - return buffer.Pass(); + return buffer; } } // namespace @@ -224,7 +224,7 @@ bool ReadProcessData(HANDLE process, ProcessInfo* process_info) { typename Traits::Pointer peb_address; if (!AssignIfInRange(&peb_address, peb_address_vmaddr)) { - LOG(ERROR) << base::StringPrintf("peb address 0x%x out of range", + LOG(ERROR) << base::StringPrintf("peb address 0x%llx out of range", peb_address_vmaddr); return false; } @@ -589,7 +589,8 @@ bool ProcessInfo::Modules(std::vector* modules) const { return true; } -const std::vector& ProcessInfo::MemoryInfo() const { +const ProcessInfo::MemoryBasicInformation64Vector& ProcessInfo::MemoryInfo() + const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); return memory_info_; } @@ -629,11 +630,11 @@ const std::vector& ProcessInfo::Handles() const { std::vector> GetReadableRangesOfMemoryMap( const CheckedRange& range, - const std::vector& memory_info) { + const ProcessInfo::MemoryBasicInformation64Vector& memory_info) { using Range = CheckedRange; // Find all the ranges that overlap the target range, maintaining their order. - std::vector overlapping; + ProcessInfo::MemoryBasicInformation64Vector overlapping; for (const auto& mi : memory_info) { static_assert(base::is_same::value, "expected range address to be WinVMAddress"); diff --git a/util/win/process_info.h b/util/win/process_info.h index fb1b8b3e..03624451 100644 --- a/util/win/process_info.h +++ b/util/win/process_info.h @@ -24,6 +24,7 @@ #include "base/basictypes.h" #include "util/misc/initialization_state_dcheck.h" #include "util/numeric/checked_range.h" +#include "util/stdlib/aligned_allocator.h" #include "util/win/address_types.h" namespace crashpad { @@ -32,6 +33,10 @@ namespace crashpad { //! primarily of information stored in the Process Environment Block. class ProcessInfo { public: + //! \brief The return type of MemoryInfo(), for convenience. + using MemoryBasicInformation64Vector = + AlignedVector; + //! \brief Contains information about a module loaded into a process. struct Module { Module(); @@ -124,7 +129,7 @@ class ProcessInfo { bool Modules(std::vector* modules) const; //! \brief Retrieves information about all pages mapped into the process. - const std::vector& MemoryInfo() const; + const MemoryBasicInformation64Vector& MemoryInfo() const; //! \brief Given a range to be read from the target process, returns a vector //! of ranges, representing the readable portions of the original range. @@ -174,7 +179,19 @@ class ProcessInfo { WinVMAddress peb_address_; WinVMSize peb_size_; std::vector modules_; - std::vector memory_info_; + + // memory_info_ is a MemoryBasicInformation64Vector instead of a + // std::vector because MEMORY_BASIC_INFORMATION64 + // is declared with __declspec(align(16)), but std::vector<> does not maintain + // this alignment on 32-bit x86. clang-cl (but not MSVC cl) takes advantage of + // the presumed alignment and emits SSE instructions that require aligned + // storage. clang-cl should relax (unfortunately), but in the mean time, this + // provides aligned storage. See https://crbug.com/564691 and + // http://llvm.org/PR25779. + // + // TODO(mark): Remove this workaround when http://llvm.org/PR25779 is fixed + // and the fix is present in the clang-cl that compiles this code. + MemoryBasicInformation64Vector memory_info_; // Handles() is logically const, but updates this member on first retrieval. // See https://crashpad.chromium.org/bug/9. @@ -195,7 +212,7 @@ class ProcessInfo { //! ProcessInfo::GetReadableRanges(). std::vector> GetReadableRangesOfMemoryMap( const CheckedRange& range, - const std::vector& memory_info); + const ProcessInfo::MemoryBasicInformation64Vector& memory_info); } // namespace crashpad diff --git a/util/win/process_info_test.cc b/util/win/process_info_test.cc index e5bde2f6..6917ed9e 100644 --- a/util/win/process_info_test.cc +++ b/util/win/process_info_test.cc @@ -59,7 +59,7 @@ void VerifyAddressInInCodePage(const ProcessInfo& process_info, WinVMAddress code_address) { // Make sure the child code address is an code page address with the right // information. - const std::vector& memory_info = + const ProcessInfo::MemoryBasicInformation64Vector& memory_info = process_info.MemoryInfo(); bool found_region = false; for (const auto& mi : memory_info) { @@ -199,7 +199,7 @@ TEST(ProcessInfo, OtherProcessWOW64) { #endif // ARCH_CPU_64_BITS TEST(ProcessInfo, AccessibleRangesNone) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; @@ -215,7 +215,7 @@ TEST(ProcessInfo, AccessibleRangesNone) { } TEST(ProcessInfo, AccessibleRangesOneInside) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; @@ -233,7 +233,7 @@ TEST(ProcessInfo, AccessibleRangesOneInside) { } TEST(ProcessInfo, AccessibleRangesOneTruncatedSize) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; @@ -256,7 +256,7 @@ TEST(ProcessInfo, AccessibleRangesOneTruncatedSize) { } TEST(ProcessInfo, AccessibleRangesOneMovedStart) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; @@ -279,7 +279,7 @@ TEST(ProcessInfo, AccessibleRangesOneMovedStart) { } TEST(ProcessInfo, ReserveIsInaccessible) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; @@ -302,7 +302,7 @@ TEST(ProcessInfo, ReserveIsInaccessible) { } TEST(ProcessInfo, PageGuardIsInaccessible) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; @@ -327,7 +327,7 @@ TEST(ProcessInfo, PageGuardIsInaccessible) { } TEST(ProcessInfo, PageNoAccessIsInaccessible) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; @@ -352,7 +352,7 @@ TEST(ProcessInfo, PageNoAccessIsInaccessible) { } TEST(ProcessInfo, AccessibleRangesCoalesced) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; @@ -380,7 +380,7 @@ TEST(ProcessInfo, AccessibleRangesCoalesced) { } TEST(ProcessInfo, AccessibleRangesMiddleUnavailable) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; @@ -410,7 +410,7 @@ TEST(ProcessInfo, AccessibleRangesMiddleUnavailable) { } TEST(ProcessInfo, RequestedBeforeMap) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 10; @@ -428,7 +428,7 @@ TEST(ProcessInfo, RequestedBeforeMap) { } TEST(ProcessInfo, RequestedAfterMap) { - std::vector memory_info; + ProcessInfo::MemoryBasicInformation64Vector memory_info; MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 10;