From 30385d4e4772704625cc796331e63210e5cdb60f Mon Sep 17 00:00:00 2001 From: Sigurdur Asgeirsson Date: Tue, 11 Apr 2017 14:48:10 -0400 Subject: [PATCH] handler: Add user extensibility stream call-out. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: crashpad:167 Test: Add crashpad_handler_test. Change-Id: I79b0b71dc4f61e6dce6bc10083e2f924dc83c940 Reviewed-on: https://chromium-review.googlesource.com/463746 Commit-Queue: Sigurður Ásgeirsson Reviewed-by: Mark Mentovai --- build/run_tests.py | 5 + crashpad.gyp | 1 + handler/crashpad_handler_test.cc | 132 ++++++++++++++++++ .../crashpad_handler_test_extended_handler.cc | 70 ++++++++++ handler/handler.gyp | 2 + handler/handler_main.cc | 10 +- handler/handler_main.h | 11 +- handler/handler_test.gyp | 64 +++++++++ handler/mac/crash_report_exception_handler.cc | 11 +- handler/mac/crash_report_exception_handler.h | 5 +- handler/main.cc | 12 +- handler/user_stream_data_source.cc | 43 ++++++ handler/user_stream_data_source.h | 68 +++++++++ handler/win/crash_report_exception_handler.cc | 11 +- handler/win/crash_report_exception_handler.h | 5 +- minidump/minidump_file_writer.cc | 5 +- minidump/minidump_file_writer_test.cc | 40 +++++- minidump/minidump_test.gyp | 43 ++++-- ...idump_user_extension_stream_data_source.cc | 8 +- ...nidump_user_extension_stream_data_source.h | 51 +++++-- minidump/minidump_user_stream_writer.cc | 40 +++--- minidump/minidump_user_stream_writer.h | 15 +- minidump/minidump_user_stream_writer_test.cc | 14 +- .../minidump_user_extension_stream_util.cc | 43 ++++++ .../minidump_user_extension_stream_util.h | 53 +++++++ test/win/win_multiprocess_with_temp_dir.h | 2 +- 26 files changed, 682 insertions(+), 82 deletions(-) create mode 100644 handler/crashpad_handler_test.cc create mode 100644 handler/crashpad_handler_test_extended_handler.cc create mode 100644 handler/handler_test.gyp create mode 100644 handler/user_stream_data_source.cc create mode 100644 handler/user_stream_data_source.h create mode 100644 minidump/test/minidump_user_extension_stream_util.cc create mode 100644 minidump/test/minidump_user_extension_stream_util.h diff --git a/build/run_tests.py b/build/run_tests.py index 00b40f6b..c5cbb03b 100755 --- a/build/run_tests.py +++ b/build/run_tests.py @@ -38,6 +38,11 @@ def main(args): 'crashpad_test_test', 'crashpad_util_test', ] + + if sys.platform == 'win32': + tests.append('crashpad_handler_test') + tests = sorted(tests) + for test in tests: print '-' * 80 print test diff --git a/crashpad.gyp b/crashpad.gyp index 42fe0a26..f30a9df9 100644 --- a/crashpad.gyp +++ b/crashpad.gyp @@ -22,6 +22,7 @@ 'client/client_test.gyp:*', 'compat/compat.gyp:*', 'handler/handler.gyp:*', + 'handler/handler_test.gyp:*', 'minidump/minidump.gyp:*', 'minidump/minidump_test.gyp:*', 'snapshot/snapshot.gyp:*', diff --git a/handler/crashpad_handler_test.cc b/handler/crashpad_handler_test.cc new file mode 100644 index 00000000..7237e8c8 --- /dev/null +++ b/handler/crashpad_handler_test.cc @@ -0,0 +1,132 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include +#include + +#include +#include +#include + +#include "base/files/file_path.h" +#include "client/crash_report_database.h" +#include "client/crashpad_client.h" +#include "gtest/gtest.h" +#include "minidump/test/minidump_file_writer_test_util.h" +#include "test/test_paths.h" +#include "test/win/win_multiprocess_with_temp_dir.h" +#include "util/file/file_reader.h" + +namespace crashpad { +namespace test { +namespace { + +void StartAndCrashWithExtendedHandler(const base::FilePath& temp_dir) { + base::FilePath handler_path = TestPaths::Executable().DirName().Append( + FILE_PATH_LITERAL("crashpad_handler_test_extended_handler.exe")); + + CrashpadClient client; + ASSERT_TRUE(client.StartHandler(handler_path, + temp_dir, + base::FilePath(), + "", + std::map(), + std::vector(), + false, + false)); + + __debugbreak(); +} + +class CrashWithExtendedHandler final : public WinMultiprocessWithTempDir { + public: + CrashWithExtendedHandler() : WinMultiprocessWithTempDir() {} + ~CrashWithExtendedHandler() {} + + private: + void ValidateGeneratedDump(); + + void WinMultiprocessParent() override { + SetExpectedChildExitCode(EXCEPTION_BREAKPOINT); + } + + void WinMultiprocessChild() override { + StartAndCrashWithExtendedHandler(GetTempDirPath()); + } + + void WinMultiprocessParentAfterChild(HANDLE child) override { + // At this point the child has exited, which means the crash report should + // have been written. + ValidateGeneratedDump(); + + // Delegate the cleanup to the superclass. + WinMultiprocessWithTempDir::WinMultiprocessParentAfterChild(child); + } +}; + +void CrashWithExtendedHandler::ValidateGeneratedDump() { + // Open the database and find the sole dump that should have been created. + std::unique_ptr database( + CrashReportDatabase::Initialize(GetTempDirPath())); + ASSERT_TRUE(database); + + std::vector reports; + ASSERT_EQ(database->GetCompletedReports(&reports), + CrashReportDatabase::kNoError); + ASSERT_EQ(reports.size(), 1u); + + // Open the dump and validate that it has the extension stream with the + // expected contents. + FileReader reader; + ASSERT_TRUE(reader.Open(reports[0].file_path)); + + // Read the header. + MINIDUMP_HEADER header = {}; + ASSERT_TRUE(reader.ReadExactly(&header, sizeof(header))); + + // Read the directory. + std::vector directory(header.NumberOfStreams); + ASSERT_TRUE(reader.SeekSet(header.StreamDirectoryRva)); + ASSERT_TRUE(reader.ReadExactly(directory.data(), + directory.size() * sizeof(directory[0]))); + + // Search for the extension stream. + size_t found_extension_streams = 0; + for (const auto& entry : directory) { + if (entry.StreamType == 0xCAFEBABE) { + ++found_extension_streams; + + ASSERT_TRUE(reader.SeekSet(entry.Location.Rva)); + + std::vector data; + data.resize(entry.Location.DataSize); + + ASSERT_TRUE(reader.ReadExactly(data.data(), data.size())); + + static const char kExpectedData[] = "Injected extension stream!"; + EXPECT_EQ(memcmp(kExpectedData, data.data(), sizeof(kExpectedData)), 0); + } + } + + EXPECT_EQ(found_extension_streams, 1u); +} + +TEST(CrashpadHandler, ExtensibilityCalloutsWork) { + WinMultiprocessWithTempDir::Run(); +} + +} // namespace +} // namespace test +} // namespace crashpad diff --git a/handler/crashpad_handler_test_extended_handler.cc b/handler/crashpad_handler_test_extended_handler.cc new file mode 100644 index 00000000..ede0f6e7 --- /dev/null +++ b/handler/crashpad_handler_test_extended_handler.cc @@ -0,0 +1,70 @@ +// Copyright 2017 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 "base/macros.h" +#include "base/memory/ptr_util.h" +#include "build/build_config.h" +#include "handler/handler_main.h" +#include "minidump/test/minidump_user_extension_stream_util.h" +#include "tools/tool_support.h" + +#if defined(OS_WIN) +#include +#endif + +namespace { + +class TestUserStreamDataSource : public crashpad::UserStreamDataSource { + public: + TestUserStreamDataSource() {} + + std::unique_ptr + ProduceStreamData(crashpad::ProcessSnapshot* process_snapshot) override; + + private: + DISALLOW_COPY_AND_ASSIGN(TestUserStreamDataSource); +}; + +std::unique_ptr +TestUserStreamDataSource::ProduceStreamData( + crashpad::ProcessSnapshot* process_snapshot) { + static const char kTestData[] = "Injected extension stream!"; + + return base::WrapUnique(new crashpad::test::BufferExtensionStreamDataSource( + 0xCAFEBABE, kTestData, sizeof(kTestData))); +} + +int ExtendedHandlerMain(int argc, char* argv[]) { + crashpad::UserStreamDataSources user_stream_data_sources; + user_stream_data_sources.push_back( + base::WrapUnique(new TestUserStreamDataSource())); + + return crashpad::HandlerMain(argc, argv, &user_stream_data_sources); +} + +} // namespace + +#if defined(OS_MACOSX) + +int main(int argc, char* argv[]) { + return ExtendedHandlerMain(argc, argv); +} + +#elif defined(OS_WIN) + +int wmain(int argc, wchar_t* argv[]) { + return crashpad::ToolSupport::Wmain(argc, argv, &ExtendedHandlerMain); +} + +#endif // OS_MACOSX diff --git a/handler/handler.gyp b/handler/handler.gyp index 6a51222c..d6e4c271 100644 --- a/handler/handler.gyp +++ b/handler/handler.gyp @@ -45,6 +45,8 @@ 'mac/exception_handler_server.h', 'prune_crash_reports_thread.cc', 'prune_crash_reports_thread.h', + 'user_stream_data_source.cc', + 'user_stream_data_source.h', 'win/crash_report_exception_handler.cc', 'win/crash_report_exception_handler.h', ], diff --git a/handler/handler_main.cc b/handler/handler_main.cc index f58f1ba4..641bf0f6 100644 --- a/handler/handler_main.cc +++ b/handler/handler_main.cc @@ -388,7 +388,9 @@ void MonitorSelf(const Options& options) { } // namespace -int HandlerMain(int argc, char* argv[]) { +int HandlerMain(int argc, + char* argv[], + const UserStreamDataSources* user_stream_sources) { InstallCrashHandler(); CallMetricsRecordNormalExit metrics_record_normal_exit; @@ -727,8 +729,10 @@ int HandlerMain(int argc, char* argv[]) { PruneCondition::GetDefault()); prune_thread.Start(); - CrashReportExceptionHandler exception_handler( - database.get(), &upload_thread, &options.annotations); + CrashReportExceptionHandler exception_handler(database.get(), + &upload_thread, + &options.annotations, + user_stream_sources); #if defined(OS_WIN) if (options.initial_client_data.IsValid()) { diff --git a/handler/handler_main.h b/handler/handler_main.h index 5b5568ec..1e78aa56 100644 --- a/handler/handler_main.h +++ b/handler/handler_main.h @@ -15,13 +15,22 @@ #ifndef CRASHPAD_HANDLER_HANDLER_MAIN_H_ #define CRASHPAD_HANDLER_HANDLER_MAIN_H_ +#include "handler/user_stream_data_source.h" + namespace crashpad { //! \brief The `main()` of the `crashpad_handler` binary. //! //! This is exposed so that `crashpad_handler` can be embedded into another //! binary, but called and used as if it were a standalone executable. -int HandlerMain(int argc, char* argv[]); +//! +//! \param[in] user_stream_sources An optional vector containing the +//! extensibility data sources to call on crash. Each time a minidump is +//! created, the sources are called in turn. Any streams returned are added +//! to the minidump. +int HandlerMain(int argc, + char* argv[], + const UserStreamDataSources* user_stream_sources); } // namespace crashpad diff --git a/handler/handler_test.gyp b/handler/handler_test.gyp new file mode 100644 index 00000000..e5264182 --- /dev/null +++ b/handler/handler_test.gyp @@ -0,0 +1,64 @@ +# Copyright 2017 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. + +{ + 'includes': [ + '../build/crashpad.gypi', + ], + 'targets': [ + { + 'target_name': 'crashpad_handler_test_extended_handler', + 'type': 'executable', + 'dependencies': [ + '../compat/compat.gyp:crashpad_compat', + '../minidump/minidump_test.gyp:crashpad_minidump_test_lib', + '../third_party/mini_chromium/mini_chromium.gyp:base', + '../tools/tools.gyp:crashpad_tool_support', + 'handler.gyp:crashpad_handler_lib', + ], + 'include_dirs': [ + '..', + ], + 'sources': [ + 'crashpad_handler_test_extended_handler.cc', + ], + }, + ], + 'conditions': [ + ['OS=="win"', { + 'targets': [{ + # The handler is only tested on Windows for now. + 'target_name': 'crashpad_handler_test', + 'type': 'executable', + 'dependencies': [ + 'crashpad_handler_test_extended_handler', + 'handler.gyp:crashpad_handler_lib', + '../client/client.gyp:crashpad_client', + '../compat/compat.gyp:crashpad_compat', + '../test/test.gyp:crashpad_gtest_main', + '../test/test.gyp:crashpad_test', + '../third_party/gtest/gtest.gyp:gtest', + '../third_party/mini_chromium/mini_chromium.gyp:base', + '../util/util.gyp:crashpad_util', + ], + 'include_dirs': [ + '..', + ], + 'sources': [ + 'crashpad_handler_test.cc', + ], + }], + }], + ], +} diff --git a/handler/mac/crash_report_exception_handler.cc b/handler/mac/crash_report_exception_handler.cc index 7784afef..a96131cd 100644 --- a/handler/mac/crash_report_exception_handler.cc +++ b/handler/mac/crash_report_exception_handler.cc @@ -22,6 +22,7 @@ #include "base/strings/stringprintf.h" #include "client/settings.h" #include "minidump/minidump_file_writer.h" +#include "minidump/minidump_user_extension_stream_data_source.h" #include "snapshot/crashpad_info_client_options.h" #include "snapshot/mac/process_snapshot_mac.h" #include "util/file/file_writer.h" @@ -41,11 +42,12 @@ namespace crashpad { CrashReportExceptionHandler::CrashReportExceptionHandler( CrashReportDatabase* database, CrashReportUploadThread* upload_thread, - const std::map* process_annotations) + const std::map* process_annotations, + const UserStreamDataSources* user_stream_data_sources) : database_(database), upload_thread_(upload_thread), - process_annotations_(process_annotations) { -} + process_annotations_(process_annotations), + user_stream_data_sources_(user_stream_data_sources) {} CrashReportExceptionHandler::~CrashReportExceptionHandler() { } @@ -169,6 +171,9 @@ kern_return_t CrashReportExceptionHandler::CatchMachException( MinidumpFileWriter minidump; minidump.InitializeFromSnapshot(&process_snapshot); + AddUserExtensionStreams( + user_stream_data_sources_, &process_snapshot, &minidump); + if (!minidump.WriteEverything(&file_writer)) { Metrics::ExceptionCaptureResult( Metrics::CaptureResult::kMinidumpWriteFailed); diff --git a/handler/mac/crash_report_exception_handler.h b/handler/mac/crash_report_exception_handler.h index 63d9bef3..b878cfe4 100644 --- a/handler/mac/crash_report_exception_handler.h +++ b/handler/mac/crash_report_exception_handler.h @@ -23,6 +23,7 @@ #include "base/macros.h" #include "client/crash_report_database.h" #include "handler/crash_report_upload_thread.h" +#include "handler/user_stream_data_source.h" #include "util/mach/exc_server_variants.h" namespace crashpad { @@ -49,7 +50,8 @@ class CrashReportExceptionHandler : public UniversalMachExcServer::Interface { CrashReportExceptionHandler( CrashReportDatabase* database, CrashReportUploadThread* upload_thread, - const std::map* process_annotations); + const std::map* process_annotations, + const UserStreamDataSources* user_stream_data_sources); ~CrashReportExceptionHandler(); @@ -77,6 +79,7 @@ class CrashReportExceptionHandler : public UniversalMachExcServer::Interface { CrashReportDatabase* database_; // weak CrashReportUploadThread* upload_thread_; // weak const std::map* process_annotations_; // weak + const UserStreamDataSources* user_stream_data_sources_; // weak DISALLOW_COPY_AND_ASSIGN(CrashReportExceptionHandler); }; diff --git a/handler/main.cc b/handler/main.cc index 7a169e2d..bc4ab99b 100644 --- a/handler/main.cc +++ b/handler/main.cc @@ -23,10 +23,18 @@ #if defined(OS_MACOSX) int main(int argc, char* argv[]) { - return crashpad::HandlerMain(argc, argv); + return crashpad::HandlerMain(argc, argv, nullptr); } #elif defined(OS_WIN) +namespace { + +int HandlerMainAdaptor(int argc, char* argv[]) { + return crashpad::HandlerMain(argc, argv, nullptr); +} + +} // namespace + int APIENTRY wWinMain(HINSTANCE, HINSTANCE, wchar_t*, int) { - return crashpad::ToolSupport::Wmain(__argc, __wargv, crashpad::HandlerMain); + return crashpad::ToolSupport::Wmain(__argc, __wargv, HandlerMainAdaptor); } #endif // OS_MACOSX diff --git a/handler/user_stream_data_source.cc b/handler/user_stream_data_source.cc new file mode 100644 index 00000000..7e30fa8c --- /dev/null +++ b/handler/user_stream_data_source.cc @@ -0,0 +1,43 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "handler/user_stream_data_source.h" + +#include "base/logging.h" +#include "minidump/minidump_file_writer.h" +#include "minidump/minidump_user_extension_stream_data_source.h" +#include "snapshot/process_snapshot.h" + +namespace crashpad { + +void AddUserExtensionStreams( + const UserStreamDataSources* user_stream_data_sources, + ProcessSnapshot* process_snapshot, + MinidumpFileWriter* minidump_file_writer) { + if (!user_stream_data_sources) + return; + for (const auto& source : *user_stream_data_sources) { + std::unique_ptr data_source( + source->ProduceStreamData(process_snapshot)); + if (data_source && + !minidump_file_writer->AddUserExtensionStream(std::move(data_source))) { + // This should only happen if multiple user stream sources yield the + // same stream type. It's the user's responsibility to make sure + // sources don't collide on the same stream type. + LOG(ERROR) << "AddUserExtensionStream failed"; + } + } +} + +} // namespace crashpad diff --git a/handler/user_stream_data_source.h b/handler/user_stream_data_source.h new file mode 100644 index 00000000..11bb9c3d --- /dev/null +++ b/handler/user_stream_data_source.h @@ -0,0 +1,68 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_HANDLER_USER_STREAM_DATA_SOURCE_H_ +#define CRASHPAD_HANDLER_USER_STREAM_DATA_SOURCE_H_ + +#include +#include + +namespace crashpad { + +class MinidumpFileWriter; +class MinidumpUserExtensionStreamDataSource; +class ProcessSnapshot; + +//! \brief Extensibility interface for embedders who wish to add custom streams +//! to minidumps. +class UserStreamDataSource { + public: + virtual ~UserStreamDataSource() {} + + //! \brief Produce the contents for an extension stream for a crashed program. + //! + //! Called after \a process_snapshot has been initialized for the crashed + //! process to (optionally) produce the contents of a user extension stream + //! that will be attached to the minidump. + //! + //! \param[in] process_snapshot An initialized snapshot for the crashed + //! process. + //! + //! \return A new data source for the stream to add to the minidump or + //! `nullptr` on failure or to opt out of adding a stream. + virtual std::unique_ptr + ProduceStreamData(ProcessSnapshot* process_snapshot) = 0; +}; + +using UserStreamDataSources = + std::vector>; + +//! \brief Adds user extension streams to a minidump. +//! +//! Dispatches to each source in \a user_stream_data_sources and adds returned +//! extension streams to \a minidump_file_writer. +//! +//! \param[in] user_stream_data_sources A pointer to the data sources, or +//! `nullptr`. +//! \param[in] process_snapshot An initialized snapshot to the crashing process. +//! \param[in] minidump_file_writer Any extension streams will be added to this +//! minidump. +void AddUserExtensionStreams( + const UserStreamDataSources* user_stream_data_sources, + ProcessSnapshot* process_snapshot, + MinidumpFileWriter* minidump_file_writer); + +} // namespace crashpad + +#endif // CRASHPAD_HANDLER_USER_STREAM_DATA_SOURCE_H_ diff --git a/handler/win/crash_report_exception_handler.cc b/handler/win/crash_report_exception_handler.cc index 180e6cb0..7828aac6 100644 --- a/handler/win/crash_report_exception_handler.cc +++ b/handler/win/crash_report_exception_handler.cc @@ -20,6 +20,7 @@ #include "client/settings.h" #include "handler/crash_report_upload_thread.h" #include "minidump/minidump_file_writer.h" +#include "minidump/minidump_user_extension_stream_data_source.h" #include "snapshot/win/process_snapshot_win.h" #include "util/file/file_writer.h" #include "util/misc/metrics.h" @@ -32,11 +33,12 @@ namespace crashpad { CrashReportExceptionHandler::CrashReportExceptionHandler( CrashReportDatabase* database, CrashReportUploadThread* upload_thread, - const std::map* process_annotations) + const std::map* process_annotations, + const UserStreamDataSources* user_stream_data_sources) : database_(database), upload_thread_(upload_thread), - process_annotations_(process_annotations) { -} + process_annotations_(process_annotations), + user_stream_data_sources_(user_stream_data_sources) {} CrashReportExceptionHandler::~CrashReportExceptionHandler() { } @@ -107,6 +109,9 @@ unsigned int CrashReportExceptionHandler::ExceptionHandlerServerException( MinidumpFileWriter minidump; minidump.InitializeFromSnapshot(&process_snapshot); + AddUserExtensionStreams( + user_stream_data_sources_, &process_snapshot, &minidump); + if (!minidump.WriteEverything(&file_writer)) { LOG(ERROR) << "WriteEverything failed"; Metrics::ExceptionCaptureResult( diff --git a/handler/win/crash_report_exception_handler.h b/handler/win/crash_report_exception_handler.h index 11b440c4..521ccd36 100644 --- a/handler/win/crash_report_exception_handler.h +++ b/handler/win/crash_report_exception_handler.h @@ -21,6 +21,7 @@ #include #include "base/macros.h" +#include "handler/user_stream_data_source.h" #include "util/win/exception_handler_server.h" namespace crashpad { @@ -50,7 +51,8 @@ class CrashReportExceptionHandler : public ExceptionHandlerServer::Delegate { CrashReportExceptionHandler( CrashReportDatabase* database, CrashReportUploadThread* upload_thread, - const std::map* process_annotations); + const std::map* process_annotations, + const UserStreamDataSources* user_stream_data_sources); ~CrashReportExceptionHandler() override; @@ -68,6 +70,7 @@ class CrashReportExceptionHandler : public ExceptionHandlerServer::Delegate { CrashReportDatabase* database_; // weak CrashReportUploadThread* upload_thread_; // weak const std::map* process_annotations_; // weak + const UserStreamDataSources* user_stream_data_sources_; // weak DISALLOW_COPY_AND_ASSIGN(CrashReportExceptionHandler); }; diff --git a/minidump/minidump_file_writer.cc b/minidump/minidump_file_writer.cc index 3e99d2a3..5a34f59c 100644 --- a/minidump/minidump_file_writer.cc +++ b/minidump/minidump_file_writer.cc @@ -206,9 +206,8 @@ bool MinidumpFileWriter::AddUserExtensionStream( DCHECK_EQ(state(), kStateMutable); auto user_stream = base::WrapUnique(new MinidumpUserStreamWriter()); - user_stream->InitializeFromBuffer(user_extension_stream_data->stream_type(), - user_extension_stream_data->buffer(), - user_extension_stream_data->buffer_size()); + user_stream->InitializeFromUserExtensionStream( + std::move(user_extension_stream_data)); return AddStream(std::move(user_stream)); } diff --git a/minidump/minidump_file_writer_test.cc b/minidump/minidump_file_writer_test.cc index 4429c119..63f2ade8 100644 --- a/minidump/minidump_file_writer_test.cc +++ b/minidump/minidump_file_writer_test.cc @@ -28,6 +28,7 @@ #include "minidump/minidump_user_extension_stream_data_source.h" #include "minidump/minidump_writable.h" #include "minidump/test/minidump_file_writer_test_util.h" +#include "minidump/test/minidump_user_extension_stream_util.h" #include "minidump/test/minidump_writable_test_util.h" #include "snapshot/test/test_cpu_context.h" #include "snapshot/test/test_exception_snapshot.h" @@ -137,14 +138,14 @@ TEST(MinidumpFileWriter, AddUserExtensionStream) { const size_t kStreamSize = arraysize(kStreamData); const MinidumpStreamType kStreamType = static_cast(0x4d); - auto stream = base::WrapUnique(new MinidumpUserExtensionStreamDataSource( + auto data_source = base::WrapUnique(new test::BufferExtensionStreamDataSource( kStreamType, kStreamData, kStreamSize)); - ASSERT_TRUE(minidump_file.AddUserExtensionStream(std::move(stream))); + ASSERT_TRUE(minidump_file.AddUserExtensionStream(std::move(data_source))); // Adding the same stream type a second time should fail. - stream = base::WrapUnique(new MinidumpUserExtensionStreamDataSource( + data_source = base::WrapUnique(new test::BufferExtensionStreamDataSource( kStreamType, kStreamData, kStreamSize)); - ASSERT_FALSE(minidump_file.AddUserExtensionStream(std::move(stream))); + ASSERT_FALSE(minidump_file.AddUserExtensionStream(std::move(data_source))); StringFile string_file; ASSERT_TRUE(minidump_file.WriteEverything(&string_file)); @@ -172,6 +173,37 @@ TEST(MinidumpFileWriter, AddUserExtensionStream) { EXPECT_EQ(memcmp(stream_data, kStreamData, kStreamSize), 0); } +TEST(MinidumpFileWriter, AddEmptyUserExtensionStream) { + MinidumpFileWriter minidump_file; + const time_t kTimestamp = 0x155d2fb8; + minidump_file.SetTimestamp(kTimestamp); + + const MinidumpStreamType kStreamType = static_cast(0x4d); + + auto data_source = base::WrapUnique( + new test::BufferExtensionStreamDataSource(kStreamType, nullptr, 0)); + ASSERT_TRUE(minidump_file.AddUserExtensionStream(std::move(data_source))); + + StringFile string_file; + ASSERT_TRUE(minidump_file.WriteEverything(&string_file)); + + const size_t kDirectoryOffset = sizeof(MINIDUMP_HEADER); + const size_t kStreamOffset = kDirectoryOffset + sizeof(MINIDUMP_DIRECTORY); + const size_t kFileSize = kStreamOffset; + + ASSERT_EQ(string_file.string().size(), kFileSize); + + const MINIDUMP_DIRECTORY* directory; + const MINIDUMP_HEADER* header = + MinidumpHeaderAtStart(string_file.string(), &directory); + ASSERT_NO_FATAL_FAILURE(VerifyMinidumpHeader(header, 1, kTimestamp)); + ASSERT_TRUE(directory); + + EXPECT_EQ(directory[0].StreamType, kStreamType); + EXPECT_EQ(directory[0].Location.DataSize, 0u); + EXPECT_EQ(directory[0].Location.Rva, kStreamOffset); +} + TEST(MinidumpFileWriter, ThreeStreams) { MinidumpFileWriter minidump_file; const time_t kTimestamp = 0x155d2fb8; diff --git a/minidump/minidump_test.gyp b/minidump/minidump_test.gyp index acf4d635..80f803db 100644 --- a/minidump/minidump_test.gyp +++ b/minidump/minidump_test.gyp @@ -17,10 +17,39 @@ '../build/crashpad.gypi', ], 'targets': [ + { + 'target_name': 'crashpad_minidump_test_lib', + 'type': 'static_library', + 'dependencies': [ + 'minidump.gyp:crashpad_minidump', + '../third_party/gtest/gtest.gyp:gtest', + '../third_party/mini_chromium/mini_chromium.gyp:base', + ], + 'include_dirs': [ + '..', + ], + 'sources': [ + 'test/minidump_context_test_util.cc', + 'test/minidump_context_test_util.h', + 'test/minidump_file_writer_test_util.cc', + 'test/minidump_file_writer_test_util.h', + 'test/minidump_memory_writer_test_util.cc', + 'test/minidump_memory_writer_test_util.h', + 'test/minidump_rva_list_test_util.cc', + 'test/minidump_rva_list_test_util.h', + 'test/minidump_string_writer_test_util.cc', + 'test/minidump_string_writer_test_util.h', + 'test/minidump_user_extension_stream_util.cc', + 'test/minidump_user_extension_stream_util.h', + 'test/minidump_writable_test_util.cc', + 'test/minidump_writable_test_util.h', + ], + }, { 'target_name': 'crashpad_minidump_test', 'type': 'executable', 'dependencies': [ + 'crashpad_minidump_test_lib', 'minidump.gyp:crashpad_minidump', '../snapshot/snapshot_test.gyp:crashpad_snapshot_test_lib', '../test/test.gyp:crashpad_gtest_main', @@ -36,8 +65,8 @@ 'minidump_context_writer_test.cc', 'minidump_crashpad_info_writer_test.cc', 'minidump_exception_writer_test.cc', - 'minidump_handle_writer_test.cc', 'minidump_file_writer_test.cc', + 'minidump_handle_writer_test.cc', 'minidump_memory_info_writer_test.cc', 'minidump_memory_writer_test.cc', 'minidump_misc_info_writer_test.cc', @@ -52,18 +81,6 @@ 'minidump_unloaded_module_writer_test.cc', 'minidump_user_stream_writer_test.cc', 'minidump_writable_test.cc', - 'test/minidump_context_test_util.cc', - 'test/minidump_context_test_util.h', - 'test/minidump_file_writer_test_util.cc', - 'test/minidump_file_writer_test_util.h', - 'test/minidump_memory_writer_test_util.cc', - 'test/minidump_memory_writer_test_util.h', - 'test/minidump_rva_list_test_util.cc', - 'test/minidump_rva_list_test_util.h', - 'test/minidump_string_writer_test_util.cc', - 'test/minidump_string_writer_test_util.h', - 'test/minidump_writable_test_util.cc', - 'test/minidump_writable_test_util.h', ], }, ], diff --git a/minidump/minidump_user_extension_stream_data_source.cc b/minidump/minidump_user_extension_stream_data_source.cc index 8056011c..884bf859 100644 --- a/minidump/minidump_user_extension_stream_data_source.cc +++ b/minidump/minidump_user_extension_stream_data_source.cc @@ -17,12 +17,8 @@ namespace crashpad { MinidumpUserExtensionStreamDataSource::MinidumpUserExtensionStreamDataSource( - uint32_t stream_type, - const void* buffer, - size_t buffer_size) - : stream_type_(static_cast(stream_type)), - buffer_(buffer), - buffer_size_(buffer_size) {} + uint32_t stream_type) + : stream_type_(static_cast(stream_type)) {} MinidumpUserExtensionStreamDataSource:: ~MinidumpUserExtensionStreamDataSource() {} diff --git a/minidump/minidump_user_extension_stream_data_source.h b/minidump/minidump_user_extension_stream_data_source.h index 1afb1661..3eb0c874 100644 --- a/minidump/minidump_user_extension_stream_data_source.h +++ b/minidump/minidump_user_extension_stream_data_source.h @@ -27,25 +27,54 @@ namespace crashpad { //! \brief Describes a user extension data stream in a minidump. class MinidumpUserExtensionStreamDataSource { public: + //! \brief An interface implemented by readers of + //! MinidumpUserExtensionStreamDataSource. + class Delegate { + public: + //! \brief Called by MinidumpUserExtensionStreamDataSource::Read() to + //! provide data requested by a call to that method. + //! + //! \param[in] data A pointer to the data that was read. The callee does not + //! take ownership of this data. This data is only valid for the + //! duration of the call to this method. This parameter may be `nullptr` + //! if \a size is `0`. + //! \param[in] size The size of the data that was read. + //! + //! \return `true` on success, `false` on failure. + //! MinidumpUserExtensionStreamDataSource::ReadStreamData() will use + //! this as its own return value. + virtual bool ExtensionStreamDataSourceRead(const void* data, + size_t size) = 0; + + protected: + ~Delegate() {} + }; + //! \brief Constructs a MinidumpUserExtensionStreamDataSource. //! //! \param[in] stream_type The type of the user extension stream. - //! \param[in] buffer Points to the data for this stream. \a buffer is not - //! owned, and must outlive the use of this object. - //! \param[in] buffer_size The length of data in \a buffer. - MinidumpUserExtensionStreamDataSource(uint32_t stream_type, - const void* buffer, - size_t buffer_size); - ~MinidumpUserExtensionStreamDataSource(); + explicit MinidumpUserExtensionStreamDataSource(uint32_t stream_type); + virtual ~MinidumpUserExtensionStreamDataSource(); MinidumpStreamType stream_type() const { return stream_type_; } - const void* buffer() const { return buffer_; } - size_t buffer_size() const { return buffer_size_; } + + //! \brief The size of this data stream. + virtual size_t StreamDataSize() = 0; + + //! \brief Calls Delegate::UserStreamDataSourceRead(), providing it with + //! the stream data. + //! + //! Implementations do not necessarily compute the stream data prior to + //! this method being called. The stream data may be computed or loaded + //! lazily and may be discarded after being passed to the delegate. + //! + //! \return `false` on failure, otherwise, the return value of + //! Delegate::ExtensionStreamDataSourceRead(), which should be `true` on + //! success and `false` on failure. + virtual bool ReadStreamData(Delegate* delegate) = 0; private: MinidumpStreamType stream_type_; - const void* buffer_; // weak - size_t buffer_size_; DISALLOW_COPY_AND_ASSIGN(MinidumpUserExtensionStreamDataSource); }; diff --git a/minidump/minidump_user_stream_writer.cc b/minidump/minidump_user_stream_writer.cc index 6520f0f4..2cd50352 100644 --- a/minidump/minidump_user_stream_writer.cc +++ b/minidump/minidump_user_stream_writer.cc @@ -56,22 +56,32 @@ class MinidumpUserStreamWriter::SnapshotContentsWriter final DISALLOW_COPY_AND_ASSIGN(SnapshotContentsWriter); }; -class MinidumpUserStreamWriter::BufferContentsWriter final - : public MinidumpUserStreamWriter::ContentsWriter { +class MinidumpUserStreamWriter::ExtensionStreamContentsWriter final + : public MinidumpUserStreamWriter::ContentsWriter, + public MinidumpUserExtensionStreamDataSource::Delegate { public: - BufferContentsWriter(const void* buffer, size_t buffer_size) - : buffer_(buffer), buffer_size_(buffer_size) {} + explicit ExtensionStreamContentsWriter( + std::unique_ptr data_source) + : data_source_(std::move(data_source)), writer_(nullptr) {} bool WriteContents(FileWriterInterface* writer) override { - return writer->Write(buffer_, buffer_size_); + DCHECK(!writer_); + + writer_ = writer; + return data_source_->ReadStreamData(this); + } + + size_t GetSize() const override { return data_source_->StreamDataSize(); } + + bool ExtensionStreamDataSourceRead(const void* data, size_t size) override { + return writer_->Write(data, size); } - size_t GetSize() const override { return buffer_size_; } private: - const void* buffer_; - size_t buffer_size_; + std::unique_ptr data_source_; + FileWriterInterface* writer_; - DISALLOW_COPY_AND_ASSIGN(BufferContentsWriter); + DISALLOW_COPY_AND_ASSIGN(ExtensionStreamContentsWriter); }; MinidumpUserStreamWriter::MinidumpUserStreamWriter() : stream_type_() {} @@ -89,16 +99,14 @@ void MinidumpUserStreamWriter::InitializeFromSnapshot( base::WrapUnique(new SnapshotContentsWriter(stream->memory())); } -void MinidumpUserStreamWriter::InitializeFromBuffer( - MinidumpStreamType stream_type, - const void* buffer, - size_t buffer_size) { +void MinidumpUserStreamWriter::InitializeFromUserExtensionStream( + std::unique_ptr data_source) { DCHECK_EQ(state(), kStateMutable); DCHECK(!contents_writer_.get()); - stream_type_ = stream_type; - contents_writer_ = - base::WrapUnique(new BufferContentsWriter(buffer, buffer_size)); + stream_type_ = data_source->stream_type(); + contents_writer_ = base::WrapUnique( + new ExtensionStreamContentsWriter(std::move(data_source))); } bool MinidumpUserStreamWriter::Freeze() { diff --git a/minidump/minidump_user_stream_writer.h b/minidump/minidump_user_stream_writer.h index 838ed0de..48698fdc 100644 --- a/minidump/minidump_user_stream_writer.h +++ b/minidump/minidump_user_stream_writer.h @@ -25,6 +25,7 @@ #include "minidump/minidump_extensions.h" #include "minidump/minidump_stream_writer.h" #include "minidump/minidump_writable.h" +#include "minidump/minidump_user_extension_stream_data_source.h" #include "snapshot/module_snapshot.h" namespace crashpad { @@ -42,17 +43,13 @@ class MinidumpUserStreamWriter final : public internal::MinidumpStreamWriter { //! \note Valid in #kStateMutable. void InitializeFromSnapshot(const UserMinidumpStream* stream); - //! \brief Initializes a MINIDUMP_USER_STREAM based on \a stream_type, - //! \a buffer and \a buffer_size. + //! \brief Initializes a MINIDUMP_USER_STREAM based on \a data_source. //! - //! \param[in] stream_type The type of the stream. - //! \param[in] buffer The data for the stream. - //! \param[in] buffer_size The length of \a buffer, and the resulting stream. + //! \param[in] data_source The content and type of the stream. //! //! \note Valid in #kStateMutable. - void InitializeFromBuffer(MinidumpStreamType stream_type, - const void* buffer, - size_t buffer_size); + void InitializeFromUserExtensionStream( + std::unique_ptr data_source); protected: // MinidumpWritable: @@ -67,7 +64,7 @@ class MinidumpUserStreamWriter final : public internal::MinidumpStreamWriter { private: class ContentsWriter; class SnapshotContentsWriter; - class BufferContentsWriter; + class ExtensionStreamContentsWriter; std::unique_ptr contents_writer_; diff --git a/minidump/minidump_user_stream_writer_test.cc b/minidump/minidump_user_stream_writer_test.cc index 45089caf..e6627e62 100644 --- a/minidump/minidump_user_stream_writer_test.cc +++ b/minidump/minidump_user_stream_writer_test.cc @@ -21,6 +21,7 @@ #include "gtest/gtest.h" #include "minidump/minidump_file_writer.h" #include "minidump/test/minidump_file_writer_test_util.h" +#include "minidump/test/minidump_user_extension_stream_util.h" #include "minidump/test/minidump_writable_test_util.h" #include "snapshot/test/test_memory_snapshot.h" #include "util/file/string_file.h" @@ -74,10 +75,12 @@ TEST(MinidumpUserStreamWriter, InitializeFromSnapshotNoData) { string_file.string(), &user_stream_location, kTestStreamId, 0u)); } -TEST(MinidumpUserStreamWriter, InitializeFromBufferNoData) { +TEST(MinidumpUserStreamWriter, InitializeFromUserExtensionStreamNoData) { MinidumpFileWriter minidump_file_writer; + auto data_source = base::WrapUnique( + new test::BufferExtensionStreamDataSource(kTestStreamId, nullptr, 0)); auto user_stream_writer = base::WrapUnique(new MinidumpUserStreamWriter()); - user_stream_writer->InitializeFromBuffer(kTestStreamId, nullptr, 0); + user_stream_writer->InitializeFromUserExtensionStream(std::move(data_source)); minidump_file_writer.AddStream(std::move(user_stream_writer)); StringFile string_file; @@ -121,12 +124,13 @@ TEST(MinidumpUserStreamWriter, InitializeFromSnapshotOneStream) { TEST(MinidumpUserStreamWriter, InitializeFromBufferOneStream) { MinidumpFileWriter minidump_file_writer; - auto user_stream_writer = base::WrapUnique(new MinidumpUserStreamWriter()); const size_t kStreamSize = 128; std::vector data(kStreamSize, 'c'); - user_stream_writer->InitializeFromBuffer( - kTestStreamId, &data[0], data.size()); + auto data_source = base::WrapUnique(new test::BufferExtensionStreamDataSource( + kTestStreamId, &data[0], data.size())); + auto user_stream_writer = base::WrapUnique(new MinidumpUserStreamWriter()); + user_stream_writer->InitializeFromUserExtensionStream(std::move(data_source)); minidump_file_writer.AddStream(std::move(user_stream_writer)); StringFile string_file; diff --git a/minidump/test/minidump_user_extension_stream_util.cc b/minidump/test/minidump_user_extension_stream_util.cc new file mode 100644 index 00000000..50100822 --- /dev/null +++ b/minidump/test/minidump_user_extension_stream_util.cc @@ -0,0 +1,43 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "minidump/test/minidump_user_extension_stream_util.h" + +#include + +namespace crashpad { +namespace test { + +BufferExtensionStreamDataSource::BufferExtensionStreamDataSource( + uint32_t stream_type, + const void* data, + size_t data_size) + : MinidumpUserExtensionStreamDataSource(stream_type) { + data_.resize(data_size); + + if (data_size) + memcpy(data_.data(), data, data_size); +} + +size_t BufferExtensionStreamDataSource::StreamDataSize() { + return data_.size(); +} + +bool BufferExtensionStreamDataSource::ReadStreamData(Delegate* delegate) { + return delegate->ExtensionStreamDataSourceRead( + data_.size() ? data_.data() : nullptr, data_.size()); +} + +} // namespace test +} // namespace crashpad diff --git a/minidump/test/minidump_user_extension_stream_util.h b/minidump/test/minidump_user_extension_stream_util.h new file mode 100644 index 00000000..3a31b948 --- /dev/null +++ b/minidump/test/minidump_user_extension_stream_util.h @@ -0,0 +1,53 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_MINIDUMP_TEST_MINIDUMP_USER_EXTENSION_STREAM_UTIL_H_ +#define CRASHPAD_MINIDUMP_TEST_MINIDUMP_USER_EXTENSION_STREAM_UTIL_H_ + +#include "minidump/minidump_user_extension_stream_data_source.h" + +#include +#include + +#include + +namespace crashpad { +namespace test { + +//! \brief A user extension data source that wraps a buffer. +class BufferExtensionStreamDataSource final + : public MinidumpUserExtensionStreamDataSource { + public: + //! \brief Creates a data source with \a stream_type. + //! + //! param[in] stream_type The type of the stream. + //! param[in] data The data of the stream. + //! param[in] data_size The length of \a data. + BufferExtensionStreamDataSource(uint32_t stream_type, + const void* data, + size_t data_size); + + size_t StreamDataSize() override; + bool ReadStreamData(Delegate* delegate) override; + + private: + std::vector data_; + + DISALLOW_COPY_AND_ASSIGN(BufferExtensionStreamDataSource); +}; + +} // namespace test +} // namespace crashpad + +#endif // CRASHPAD_MINIDUMP_TEST_MINIDUMP_USER_EXTENSION_STREAM_UTIL_H_ diff --git a/test/win/win_multiprocess_with_temp_dir.h b/test/win/win_multiprocess_with_temp_dir.h index 61db8f76..c9b48607 100644 --- a/test/win/win_multiprocess_with_temp_dir.h +++ b/test/win/win_multiprocess_with_temp_dir.h @@ -77,4 +77,4 @@ class WinMultiprocessWithTempDir : public WinMultiprocess { } // namespace test } // namespace crashpad -#endif // CRASHPAD_TEST_WIN_WIN_MULTIPROCESS_H_ +#endif // CRASHPAD_TEST_WIN_WIN_MULTIPROCESS_WITH_TEMPDIR_H_