From 0a4ea0b52dc98083bef0f8521e347172214e6846 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Mon, 27 Oct 2014 15:01:39 -0400 Subject: [PATCH] minidump: Change the ownership model. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All minidump objects now own their all of their children, rather than having them maintain weak pointers and requiring callers to maintain ownership. The only weak object in the entire tree now is the “extra memory” added to a MinidumpMemoryListWriter by its AddExtraMemory() method. Extra memory aliases objects owned elsewhere in the tree, typically by a MinidumpThreadWriter as stack memory. Non-“extra” memory added to a MinidumpMemoryListWriter by its AddMemory() method is strongly owned. Many objects are now deleted through base pointers, and in those cases, the base classes now have public virtual destructors. The ultimate base, MinidumpWritable, is still protected to guard against direct instantiation and deletion, and thus its destructor does not need to be virtual. This updates mini_chromium to eeb3b6a4f020 specifically for that revision, which includes necessary updates to scoped_ptr. It also picks up: eeb3b6a4f020 Update base/move.h and base/memory/scoped_ptr.h to match 67ad2efafaba More porting to Windows be27a006421e AUTHORS: Fix link post-git migration flag day. 05f5b1503230 Add codereview.settings to mini_chromium. a32c2b199811 Beginnings of Windows support in mini_chromium TEST=minidump_test R=rsesek@chromium.org Review URL: https://codereview.chromium.org/674153002 --- DEPS | 2 +- minidump/minidump_context_writer.h | 2 +- minidump/minidump_crashpad_info_writer.cc | 6 +- minidump/minidump_crashpad_info_writer.h | 12 +- .../minidump_crashpad_info_writer_test.cc | 19 ++- minidump/minidump_exception_writer.cc | 10 +- minidump/minidump_exception_writer.h | 11 +- minidump/minidump_exception_writer_test.cc | 38 ++--- minidump/minidump_file_writer.cc | 5 +- minidump/minidump_file_writer.h | 15 +- minidump/minidump_file_writer_test.cc | 36 ++-- minidump/minidump_memory_writer.cc | 17 +- minidump/minidump_memory_writer.h | 15 +- minidump/minidump_memory_writer_test.cc | 49 +++--- minidump/minidump_misc_info_writer.cc | 3 + minidump/minidump_misc_info_writer.h | 2 +- minidump/minidump_misc_info_writer_test.cc | 143 ++++++++-------- .../minidump_module_crashpad_info_writer.cc | 10 +- .../minidump_module_crashpad_info_writer.h | 22 +-- ...nidump_module_crashpad_info_writer_test.cc | 90 +++++----- minidump/minidump_module_writer.cc | 20 ++- minidump/minidump_module_writer.h | 34 ++-- minidump/minidump_module_writer_test.cc | 160 +++++++++--------- ...inidump_simple_string_dictionary_writer.cc | 13 +- ...minidump_simple_string_dictionary_writer.h | 16 +- ...mp_simple_string_dictionary_writer_test.cc | 47 ++--- minidump/minidump_stream_writer.cc | 15 +- minidump/minidump_stream_writer.h | 3 +- minidump/minidump_string_writer.cc | 6 + minidump/minidump_string_writer.h | 6 +- minidump/minidump_system_info_writer.h | 2 +- minidump/minidump_system_info_writer_test.cc | 66 ++++---- minidump/minidump_thread_writer.cc | 23 ++- minidump/minidump_thread_writer.h | 34 ++-- minidump/minidump_thread_writer_test.cc | 154 ++++++++--------- minidump/minidump_writable.h | 8 +- 36 files changed, 611 insertions(+), 503 deletions(-) diff --git a/DEPS b/DEPS index c27021c..3417d0c 100644 --- a/DEPS +++ b/DEPS @@ -28,7 +28,7 @@ deps = { '46282cedf40ff7fe803be4af357b9d59050f02e4', # svn r1977 'crashpad/third_party/mini_chromium/mini_chromium': Var('chromium_git') + '/chromium/mini_chromium@' + - 'ba9b15f1b6a7616ac0cb26069201a479e81845c3', + 'eeb3b6a4f020dbfc34fe8bc42844796fccc60d5b', } hooks = [ diff --git a/minidump/minidump_context_writer.h b/minidump/minidump_context_writer.h index c81a6e4..070f3cf 100644 --- a/minidump/minidump_context_writer.h +++ b/minidump/minidump_context_writer.h @@ -27,7 +27,7 @@ namespace crashpad { //! files. class MinidumpContextWriter : public internal::MinidumpWritable { public: - virtual ~MinidumpContextWriter(); + ~MinidumpContextWriter() override; protected: MinidumpContextWriter() : MinidumpWritable() {} diff --git a/minidump/minidump_crashpad_info_writer.cc b/minidump/minidump_crashpad_info_writer.cc index 5114843..22be597 100644 --- a/minidump/minidump_crashpad_info_writer.cc +++ b/minidump/minidump_crashpad_info_writer.cc @@ -29,10 +29,10 @@ MinidumpCrashpadInfoWriter::~MinidumpCrashpadInfoWriter() { } void MinidumpCrashpadInfoWriter::SetModuleList( - MinidumpModuleCrashpadInfoListWriter* module_list) { + scoped_ptr module_list) { DCHECK_EQ(state(), kStateMutable); - module_list_ = module_list; + module_list_ = module_list.Pass(); } bool MinidumpCrashpadInfoWriter::Freeze() { @@ -61,7 +61,7 @@ MinidumpCrashpadInfoWriter::Children() { std::vector children; if (module_list_) { - children.push_back(module_list_); + children.push_back(module_list_.get()); } return children; diff --git a/minidump/minidump_crashpad_info_writer.h b/minidump/minidump_crashpad_info_writer.h index d459d82..43f6d55 100644 --- a/minidump/minidump_crashpad_info_writer.h +++ b/minidump/minidump_crashpad_info_writer.h @@ -18,6 +18,7 @@ #include #include "base/basictypes.h" +#include "base/memory/scoped_ptr.h" #include "minidump/minidump_extensions.h" #include "minidump/minidump_stream_writer.h" @@ -29,17 +30,18 @@ class MinidumpModuleCrashpadInfoListWriter; class MinidumpCrashpadInfoWriter final : public internal::MinidumpStreamWriter { public: MinidumpCrashpadInfoWriter(); - ~MinidumpCrashpadInfoWriter(); + ~MinidumpCrashpadInfoWriter() override; //! \brief Arranges for MinidumpCrashpadInfo::module_list to point to the //! MinidumpModuleCrashpadInfoList object to be written by \a //! module_list. //! - //! \a module_list will become a child of this object in the overall tree of - //! internal::MinidumpWritable objects. + //! This object takes ownership of \a module_list and becomes its parent in + //! the overall tree of internal::MinidumpWritable objects. //! //! \note Valid in #kStateMutable. - void SetModuleList(MinidumpModuleCrashpadInfoListWriter* module_list); + void SetModuleList( + scoped_ptr module_list); protected: // MinidumpWritable: @@ -53,7 +55,7 @@ class MinidumpCrashpadInfoWriter final : public internal::MinidumpStreamWriter { private: MinidumpCrashpadInfo crashpad_info_; - MinidumpModuleCrashpadInfoListWriter* module_list_; // weak + scoped_ptr module_list_; DISALLOW_COPY_AND_ASSIGN(MinidumpCrashpadInfoWriter); }; diff --git a/minidump/minidump_crashpad_info_writer_test.cc b/minidump/minidump_crashpad_info_writer_test.cc index bbc9ce3..3fcb2e0 100644 --- a/minidump/minidump_crashpad_info_writer_test.cc +++ b/minidump/minidump_crashpad_info_writer_test.cc @@ -50,9 +50,9 @@ void GetCrashpadInfoStream(const std::string& file_contents, TEST(MinidumpCrashpadInfoWriter, Empty) { MinidumpFileWriter minidump_file_writer; - MinidumpCrashpadInfoWriter crashpad_info_writer; + auto crashpad_info_writer = make_scoped_ptr(new MinidumpCrashpadInfoWriter()); - minidump_file_writer.AddStream(&crashpad_info_writer); + minidump_file_writer.AddStream(crashpad_info_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -71,15 +71,16 @@ TEST(MinidumpCrashpadInfoWriter, CrashpadModuleList) { const uint32_t kMinidumpModuleListIndex = 3; MinidumpFileWriter minidump_file_writer; - MinidumpCrashpadInfoWriter crashpad_info_writer; + auto crashpad_info_writer = make_scoped_ptr(new MinidumpCrashpadInfoWriter()); - minidump_file_writer.AddStream(&crashpad_info_writer); + auto module_list_writer = + make_scoped_ptr(new MinidumpModuleCrashpadInfoListWriter()); + auto module_writer = make_scoped_ptr(new MinidumpModuleCrashpadInfoWriter()); + module_writer->SetMinidumpModuleListIndex(kMinidumpModuleListIndex); + module_list_writer->AddModule(module_writer.Pass()); + crashpad_info_writer->SetModuleList(module_list_writer.Pass()); - MinidumpModuleCrashpadInfoListWriter module_list_writer; - MinidumpModuleCrashpadInfoWriter module_writer; - module_writer.SetMinidumpModuleListIndex(kMinidumpModuleListIndex); - module_list_writer.AddModule(&module_writer); - crashpad_info_writer.SetModuleList(&module_list_writer); + minidump_file_writer.AddStream(crashpad_info_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); diff --git a/minidump/minidump_exception_writer.cc b/minidump/minidump_exception_writer.cc index 064c601..2f39497 100644 --- a/minidump/minidump_exception_writer.cc +++ b/minidump/minidump_exception_writer.cc @@ -26,10 +26,14 @@ MinidumpExceptionWriter::MinidumpExceptionWriter() : MinidumpStreamWriter(), exception_(), context_(nullptr) { } -void MinidumpExceptionWriter::SetContext(MinidumpContextWriter* context) { +MinidumpExceptionWriter::~MinidumpExceptionWriter() { +} + +void MinidumpExceptionWriter::SetContext( + scoped_ptr context) { DCHECK_EQ(state(), kStateMutable); - context_ = context; + context_ = context.Pass(); } void MinidumpExceptionWriter::SetExceptionInformation( @@ -76,7 +80,7 @@ std::vector MinidumpExceptionWriter::Children() { DCHECK(context_); std::vector children; - children.push_back(context_); + children.push_back(context_.get()); return children; } diff --git a/minidump/minidump_exception_writer.h b/minidump/minidump_exception_writer.h index b76b1ff..33e877c 100644 --- a/minidump/minidump_exception_writer.h +++ b/minidump/minidump_exception_writer.h @@ -21,6 +21,7 @@ #include #include "base/basictypes.h" +#include "base/memory/scoped_ptr.h" #include "minidump/minidump_stream_writer.h" namespace crashpad { @@ -31,18 +32,18 @@ class MinidumpContextWriter; class MinidumpExceptionWriter final : public internal::MinidumpStreamWriter { public: MinidumpExceptionWriter(); - ~MinidumpExceptionWriter() {} + ~MinidumpExceptionWriter() override; //! \brief Arranges for MINIDUMP_EXCEPTION_STREAM::ThreadContext to point to //! the CPU context to be written by \a context. //! //! A context is required in all MINIDUMP_EXCEPTION_STREAM objects. //! - //! \a context will become a child of this object in the overall tree of - //! internal::MinidumpWritable objects. + //! This object takes ownership of \a context and becomes its parent in the + //! overall tree of internal::MinidumpWritable objects. //! //! \note Valid in #kStateMutable. - void SetContext(MinidumpContextWriter* context); + void SetContext(scoped_ptr context); //! \brief Sets MINIDUMP_EXCEPTION_STREAM::ThreadId. void SetThreadID(uint32_t thread_id) { exception_.ThreadId = thread_id; } @@ -95,7 +96,7 @@ class MinidumpExceptionWriter final : public internal::MinidumpStreamWriter { private: MINIDUMP_EXCEPTION_STREAM exception_; - MinidumpContextWriter* context_; // weak + scoped_ptr context_; DISALLOW_COPY_AND_ASSIGN(MinidumpExceptionWriter); }; diff --git a/minidump/minidump_exception_writer_test.cc b/minidump/minidump_exception_writer_test.cc index 899ba47..ce41fb4 100644 --- a/minidump/minidump_exception_writer_test.cc +++ b/minidump/minidump_exception_writer_test.cc @@ -93,15 +93,15 @@ void ExpectExceptionStream(const MINIDUMP_EXCEPTION_STREAM* expected, TEST(MinidumpExceptionWriter, Minimal) { MinidumpFileWriter minidump_file_writer; - MinidumpExceptionWriter exception_writer; + auto exception_writer = make_scoped_ptr(new MinidumpExceptionWriter()); const uint32_t kSeed = 100; - MinidumpContextX86Writer context_x86_writer; - InitializeMinidumpContextX86(context_x86_writer.context(), kSeed); - exception_writer.SetContext(&context_x86_writer); + auto context_x86_writer = make_scoped_ptr(new MinidumpContextX86Writer()); + InitializeMinidumpContextX86(context_x86_writer->context(), kSeed); + exception_writer->SetContext(context_x86_writer.Pass()); - minidump_file_writer.AddStream(&exception_writer); + minidump_file_writer.AddStream(exception_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -124,7 +124,7 @@ TEST(MinidumpExceptionWriter, Minimal) { TEST(MinidumpExceptionWriter, Standard) { MinidumpFileWriter minidump_file_writer; - MinidumpExceptionWriter exception_writer; + auto exception_writer = make_scoped_ptr(new MinidumpExceptionWriter()); const uint32_t kSeed = 200; const uint32_t kThreadID = 1; @@ -136,30 +136,30 @@ TEST(MinidumpExceptionWriter, Standard) { const uint64_t kExceptionInformation1 = 7; const uint64_t kExceptionInformation2 = 7; - MinidumpContextX86Writer context_x86_writer; - InitializeMinidumpContextX86(context_x86_writer.context(), kSeed); - exception_writer.SetContext(&context_x86_writer); + auto context_x86_writer = make_scoped_ptr(new MinidumpContextX86Writer()); + InitializeMinidumpContextX86(context_x86_writer->context(), kSeed); + exception_writer->SetContext(context_x86_writer.Pass()); - exception_writer.SetThreadID(kThreadID); - exception_writer.SetExceptionCode(kExceptionCode); - exception_writer.SetExceptionFlags(kExceptionFlags); - exception_writer.SetExceptionRecord(kExceptionRecord); - exception_writer.SetExceptionAddress(kExceptionAddress); + exception_writer->SetThreadID(kThreadID); + exception_writer->SetExceptionCode(kExceptionCode); + exception_writer->SetExceptionFlags(kExceptionFlags); + exception_writer->SetExceptionRecord(kExceptionRecord); + exception_writer->SetExceptionAddress(kExceptionAddress); // Set a lot of exception information at first, and then replace it with less. // This tests that the exception that is written does not contain the // “garbage” from the initial SetExceptionInformation() call. std::vector exception_information(EXCEPTION_MAXIMUM_PARAMETERS, 0x5a5a5a5a5a5a5a5a); - exception_writer.SetExceptionInformation(exception_information); + exception_writer->SetExceptionInformation(exception_information); exception_information.clear(); exception_information.push_back(kExceptionInformation0); exception_information.push_back(kExceptionInformation1); exception_information.push_back(kExceptionInformation2); - exception_writer.SetExceptionInformation(exception_information); + exception_writer->SetExceptionInformation(exception_information); - minidump_file_writer.AddStream(&exception_writer); + minidump_file_writer.AddStream(exception_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -194,9 +194,9 @@ TEST(MinidumpExceptionWriter, Standard) { TEST(MinidumpExceptionWriterDeathTest, NoContext) { MinidumpFileWriter minidump_file_writer; - MinidumpExceptionWriter exception_writer; + auto exception_writer = make_scoped_ptr(new MinidumpExceptionWriter()); - minidump_file_writer.AddStream(&exception_writer); + minidump_file_writer.AddStream(exception_writer.Pass()); StringFileWriter file_writer; ASSERT_DEATH(minidump_file_writer.WriteEverything(&file_writer), "context_"); diff --git a/minidump/minidump_file_writer.cc b/minidump/minidump_file_writer.cc index fb3b476..a3a169d 100644 --- a/minidump/minidump_file_writer.cc +++ b/minidump/minidump_file_writer.cc @@ -42,7 +42,8 @@ void MinidumpFileWriter::SetTimestamp(time_t timestamp) { internal::MinidumpWriterUtil::AssignTimeT(&header_.TimeDateStamp, timestamp); } -void MinidumpFileWriter::AddStream(internal::MinidumpStreamWriter* stream) { +void MinidumpFileWriter::AddStream( + scoped_ptr stream) { DCHECK_EQ(state(), kStateMutable); MinidumpStreamType stream_type = stream->StreamType(); @@ -50,7 +51,7 @@ void MinidumpFileWriter::AddStream(internal::MinidumpStreamWriter* stream) { auto rv = stream_types_.insert(stream_type); CHECK(rv.second) << "stream_type " << stream_type << " already present"; - streams_.push_back(stream); + streams_.push_back(stream.release()); DCHECK_EQ(streams_.size(), stream_types_.size()); } diff --git a/minidump/minidump_file_writer.h b/minidump/minidump_file_writer.h index 4d4653f..22b65fc 100644 --- a/minidump/minidump_file_writer.h +++ b/minidump/minidump_file_writer.h @@ -22,9 +22,11 @@ #include #include "base/basictypes.h" +#include "base/memory/scoped_ptr.h" #include "minidump/minidump_extensions.h" #include "minidump/minidump_stream_writer.h" #include "minidump/minidump_writable.h" +#include "util/stdlib/pointer_container.h" namespace crashpad { @@ -35,15 +37,18 @@ namespace crashpad { class MinidumpFileWriter final : public internal::MinidumpWritable { public: MinidumpFileWriter(); - ~MinidumpFileWriter(); + ~MinidumpFileWriter() override; //! \brief Sets MINIDUMP_HEADER::Timestamp. //! //! \note Valid in #kStateMutable. void SetTimestamp(time_t timestamp); - //! \brief Adds a stream to the minidump file as a child of the object, and - //! arranges for a MINIDUMP_DIRECTORY entry to point to it. + //! \brief Adds a stream to the minidump file and arranges for a + //! MINIDUMP_DIRECTORY entry to point to it. + //! + //! This object takes ownership of \a stream and becomes its parent in the + //! overall tree of internal::MinidumpWritable objects. //! //! At most one object of each stream type (as obtained from //! internal::MinidumpStreamWriter::StreamType()) may be added to a @@ -51,7 +56,7 @@ class MinidumpFileWriter final : public internal::MinidumpWritable { //! streams with the same stream type. //! //! \note Valid in #kStateMutable. - void AddStream(internal::MinidumpStreamWriter* stream); + void AddStream(scoped_ptr stream); // MinidumpWritable: @@ -74,7 +79,7 @@ class MinidumpFileWriter final : public internal::MinidumpWritable { private: MINIDUMP_HEADER header_; - std::vector streams_; // weak + PointerVector streams_; // Protects against multiple streams with the same ID being added. std::set stream_types_; diff --git a/minidump/minidump_file_writer_test.cc b/minidump/minidump_file_writer_test.cc index cda9356..c4a9064 100644 --- a/minidump/minidump_file_writer_test.cc +++ b/minidump/minidump_file_writer_test.cc @@ -51,7 +51,7 @@ class TestStream final : public internal::MinidumpStreamWriter { uint8_t stream_value) : stream_data_(stream_size, stream_value), stream_type_(stream_type) {} - ~TestStream() {} + ~TestStream() override {} // MinidumpStreamWriter: MinidumpStreamType StreamType() const override { @@ -85,8 +85,9 @@ TEST(MinidumpFileWriter, OneStream) { const size_t kStreamSize = 5; const MinidumpStreamType kStreamType = static_cast(0x4d); const uint8_t kStreamValue = 0x5a; - TestStream stream(kStreamType, kStreamSize, kStreamValue); - minidump_file.AddStream(&stream); + auto stream = + make_scoped_ptr(new TestStream(kStreamType, kStreamSize, kStreamValue)); + minidump_file.AddStream(stream.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file.WriteEverything(&file_writer)); @@ -123,8 +124,9 @@ TEST(MinidumpFileWriter, ThreeStreams) { const size_t kStream0Size = 5; const MinidumpStreamType kStream0Type = static_cast(0x6d); const uint8_t kStream0Value = 0x5a; - TestStream stream0(kStream0Type, kStream0Size, kStream0Value); - minidump_file.AddStream(&stream0); + auto stream0 = make_scoped_ptr( + new TestStream(kStream0Type, kStream0Size, kStream0Value)); + minidump_file.AddStream(stream0.Pass()); // 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 @@ -132,14 +134,16 @@ TEST(MinidumpFileWriter, ThreeStreams) { const size_t kStream1Size = 3; const MinidumpStreamType kStream1Type = static_cast(0x4d); const uint8_t kStream1Value = 0xa5; - TestStream stream1(kStream1Type, kStream1Size, kStream1Value); - minidump_file.AddStream(&stream1); + auto stream1 = make_scoped_ptr( + new TestStream(kStream1Type, kStream1Size, kStream1Value)); + minidump_file.AddStream(stream1.Pass()); const size_t kStream2Size = 1; const MinidumpStreamType kStream2Type = static_cast(0x7e); const uint8_t kStream2Value = 0x36; - TestStream stream2(kStream2Type, kStream2Size, kStream2Value); - minidump_file.AddStream(&stream2); + auto stream2 = make_scoped_ptr( + new TestStream(kStream2Type, kStream2Size, kStream2Value)); + minidump_file.AddStream(stream2.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file.WriteEverything(&file_writer)); @@ -205,8 +209,8 @@ TEST(MinidumpFileWriter, ZeroLengthStream) { const size_t kStreamSize = 0; const MinidumpStreamType kStreamType = static_cast(0x4d); - TestStream stream(kStreamType, kStreamSize, 0); - minidump_file.AddStream(&stream); + auto stream = make_scoped_ptr(new TestStream(kStreamType, kStreamSize, 0)); + minidump_file.AddStream(stream.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file.WriteEverything(&file_writer)); @@ -234,15 +238,17 @@ TEST(MinidumpFileWriterDeathTest, SameStreamType) { const size_t kStream0Size = 5; const MinidumpStreamType kStream0Type = static_cast(0x4d); const uint8_t kStream0Value = 0x5a; - TestStream stream0(kStream0Type, kStream0Size, kStream0Value); - minidump_file.AddStream(&stream0); + auto stream0 = make_scoped_ptr( + new TestStream(kStream0Type, kStream0Size, kStream0Value)); + minidump_file.AddStream(stream0.Pass()); // It is an error to add a second stream of the same type. const size_t kStream1Size = 3; const MinidumpStreamType kStream1Type = static_cast(0x4d); const uint8_t kStream1Value = 0xa5; - TestStream stream1(kStream1Type, kStream1Size, kStream1Value); - ASSERT_DEATH(minidump_file.AddStream(&stream1), "already present"); + auto stream1 = make_scoped_ptr( + new TestStream(kStream1Type, kStream1Size, kStream1Value)); + ASSERT_DEATH(minidump_file.AddStream(stream1.Pass()), "already present"); } } // namespace diff --git a/minidump/minidump_memory_writer.cc b/minidump/minidump_memory_writer.cc index 536cd14..d79ca7b 100644 --- a/minidump/minidump_memory_writer.cc +++ b/minidump/minidump_memory_writer.cc @@ -20,6 +20,9 @@ namespace crashpad { +MinidumpMemoryWriter::~MinidumpMemoryWriter() { +} + const MINIDUMP_MEMORY_DESCRIPTOR* MinidumpMemoryWriter::MinidumpMemoryDescriptor() const { DCHECK_EQ(state(), kStateWritable); @@ -106,11 +109,12 @@ MinidumpMemoryListWriter::MinidumpMemoryListWriter() MinidumpMemoryListWriter::~MinidumpMemoryListWriter() { } -void MinidumpMemoryListWriter::AddMemory(MinidumpMemoryWriter* memory_writer) { +void MinidumpMemoryListWriter::AddMemory( + scoped_ptr memory_writer) { DCHECK_EQ(state(), kStateMutable); - children_.push_back(memory_writer); - AddExtraMemory(memory_writer); + AddExtraMemory(memory_writer.get()); + children_.push_back(memory_writer.release()); } void MinidumpMemoryListWriter::AddExtraMemory( @@ -152,7 +156,12 @@ std::vector MinidumpMemoryListWriter::Children() { DCHECK_GE(state(), kStateFrozen); DCHECK_LE(children_.size(), memory_writers_.size()); - return children_; + std::vector children; + for (MinidumpMemoryWriter* child : children_) { + children.push_back(child); + } + + return children; } bool MinidumpMemoryListWriter::WriteObject(FileWriterInterface* file_writer) { diff --git a/minidump/minidump_memory_writer.h b/minidump/minidump_memory_writer.h index 24625e2..369fafd 100644 --- a/minidump/minidump_memory_writer.h +++ b/minidump/minidump_memory_writer.h @@ -22,8 +22,10 @@ #include #include "base/basictypes.h" +#include "base/memory/scoped_ptr.h" #include "minidump/minidump_stream_writer.h" #include "minidump/minidump_writable.h" +#include "util/stdlib/pointer_container.h" namespace crashpad { @@ -37,6 +39,8 @@ namespace crashpad { //! to be held in memory simultaneously while a minidump file is being written. class MinidumpMemoryWriter : public internal::MinidumpWritable { public: + ~MinidumpMemoryWriter() override; + //! \brief Returns a MINIDUMP_MEMORY_DESCRIPTOR referencing the data that this //! object writes. //! @@ -60,7 +64,6 @@ class MinidumpMemoryWriter : public internal::MinidumpWritable { protected: MinidumpMemoryWriter(); - ~MinidumpMemoryWriter() {} //! \brief Returns the base address of the memory region in the address space //! of the process that the snapshot describes. @@ -116,15 +119,15 @@ class MinidumpMemoryWriter : public internal::MinidumpWritable { class MinidumpMemoryListWriter final : public internal::MinidumpStreamWriter { public: MinidumpMemoryListWriter(); - ~MinidumpMemoryListWriter(); + ~MinidumpMemoryListWriter() override; //! \brief Adds a MinidumpMemoryWriter to the MINIDUMP_MEMORY_LIST. //! - //! \a memory_writer will become a child of this object in the overall tree of - //! internal::MinidumpWritable objects. + //! This object takes ownership of \a memory_writer and becomes its parent in + //! the overall tree of internal::MinidumpWritable objects. //! //! \note Valid in #kStateMutable. - void AddMemory(MinidumpMemoryWriter* memory_writer); + void AddMemory(scoped_ptr memory_writer); //! \brief Adds a MinidumpMemoryWriter that’s a child of another //! internal::MinidumpWritable object to the MINIDUMP_MEMORY_LIST. @@ -155,7 +158,7 @@ class MinidumpMemoryListWriter final : public internal::MinidumpStreamWriter { private: MINIDUMP_MEMORY_LIST memory_list_base_; std::vector memory_writers_; // weak - std::vector children_; // weak + PointerVector children_; DISALLOW_COPY_AND_ASSIGN(MinidumpMemoryListWriter); }; diff --git a/minidump/minidump_memory_writer_test.cc b/minidump/minidump_memory_writer_test.cc index 8060db7..af7018b 100644 --- a/minidump/minidump_memory_writer_test.cc +++ b/minidump/minidump_memory_writer_test.cc @@ -73,9 +73,9 @@ void GetMemoryListStream(const std::string& file_contents, TEST(MinidumpMemoryWriter, EmptyMemoryList) { MinidumpFileWriter minidump_file_writer; - MinidumpMemoryListWriter memory_list_writer; + auto memory_list_writer = make_scoped_ptr(new MinidumpMemoryListWriter()); - minidump_file_writer.AddStream(&memory_list_writer); + minidump_file_writer.AddStream(memory_list_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -93,16 +93,17 @@ TEST(MinidumpMemoryWriter, EmptyMemoryList) { TEST(MinidumpMemoryWriter, OneMemoryRegion) { MinidumpFileWriter minidump_file_writer; - MinidumpMemoryListWriter memory_list_writer; + auto memory_list_writer = make_scoped_ptr(new MinidumpMemoryListWriter()); const uint64_t kBaseAddress = 0xfedcba9876543210; const uint64_t kSize = 0x1000; const uint8_t kValue = 'm'; - TestMinidumpMemoryWriter memory_writer(kBaseAddress, kSize, kValue); - memory_list_writer.AddMemory(&memory_writer); + auto memory_writer = make_scoped_ptr( + new TestMinidumpMemoryWriter(kBaseAddress, kSize, kValue)); + memory_list_writer->AddMemory(memory_writer.Pass()); - minidump_file_writer.AddStream(&memory_list_writer); + minidump_file_writer.AddStream(memory_list_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -127,7 +128,7 @@ TEST(MinidumpMemoryWriter, OneMemoryRegion) { TEST(MinidumpMemoryWriter, TwoMemoryRegions) { MinidumpFileWriter minidump_file_writer; - MinidumpMemoryListWriter memory_list_writer; + auto memory_list_writer = make_scoped_ptr(new MinidumpMemoryListWriter()); const uint64_t kBaseAddress0 = 0xc0ffee; const uint64_t kSize0 = 0x0100; @@ -136,12 +137,14 @@ TEST(MinidumpMemoryWriter, TwoMemoryRegions) { const uint64_t kSize1 = 0x0200; const uint8_t kValue1 = '!'; - TestMinidumpMemoryWriter memory_writer_0(kBaseAddress0, kSize0, kValue0); - memory_list_writer.AddMemory(&memory_writer_0); - TestMinidumpMemoryWriter memory_writer_1(kBaseAddress1, kSize1, kValue1); - memory_list_writer.AddMemory(&memory_writer_1); + auto memory_writer_0 = make_scoped_ptr( + new TestMinidumpMemoryWriter(kBaseAddress0, kSize0, kValue0)); + memory_list_writer->AddMemory(memory_writer_0.Pass()); + auto memory_writer_1 = make_scoped_ptr( + new TestMinidumpMemoryWriter(kBaseAddress1, kSize1, kValue1)); + memory_list_writer->AddMemory(memory_writer_1.Pass()); - minidump_file_writer.AddStream(&memory_list_writer); + minidump_file_writer.AddStream(memory_list_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -190,9 +193,11 @@ class TestMemoryStream final : public internal::MinidumpStreamWriter { TestMemoryStream(uint64_t base_address, size_t size, uint8_t value) : MinidumpStreamWriter(), memory_(base_address, size, value) {} - ~TestMemoryStream() {} + ~TestMemoryStream() override {} - TestMinidumpMemoryWriter* memory() { return &memory_; } + TestMinidumpMemoryWriter* memory() { + return &memory_; + } // MinidumpStreamWriter: MinidumpStreamType StreamType() const override { @@ -232,21 +237,23 @@ TEST(MinidumpMemoryWriter, ExtraMemory) { const uint64_t kBaseAddress0 = 0x1000; const uint64_t kSize0 = 0x0400; const uint8_t kValue0 = '1'; - TestMemoryStream test_memory_stream(kBaseAddress0, kSize0, kValue0); + auto test_memory_stream = + make_scoped_ptr(new TestMemoryStream(kBaseAddress0, kSize0, kValue0)); - MinidumpMemoryListWriter memory_list_writer; - memory_list_writer.AddExtraMemory(test_memory_stream.memory()); + auto memory_list_writer = make_scoped_ptr(new MinidumpMemoryListWriter()); + memory_list_writer->AddExtraMemory(test_memory_stream->memory()); - minidump_file_writer.AddStream(&test_memory_stream); + minidump_file_writer.AddStream(test_memory_stream.Pass()); const uint64_t kBaseAddress1 = 0x2000; const uint64_t kSize1 = 0x0400; const uint8_t kValue1 = 'm'; - TestMinidumpMemoryWriter memory_writer(kBaseAddress1, kSize1, kValue1); - memory_list_writer.AddMemory(&memory_writer); + auto memory_writer = make_scoped_ptr( + new TestMinidumpMemoryWriter(kBaseAddress1, kSize1, kValue1)); + memory_list_writer->AddMemory(memory_writer.Pass()); - minidump_file_writer.AddStream(&memory_list_writer); + minidump_file_writer.AddStream(memory_list_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); diff --git a/minidump/minidump_misc_info_writer.cc b/minidump/minidump_misc_info_writer.cc index 5226c32..9f829fa 100644 --- a/minidump/minidump_misc_info_writer.cc +++ b/minidump/minidump_misc_info_writer.cc @@ -27,6 +27,9 @@ MinidumpMiscInfoWriter::MinidumpMiscInfoWriter() : MinidumpStreamWriter(), misc_info_() { } +MinidumpMiscInfoWriter::~MinidumpMiscInfoWriter() { +} + void MinidumpMiscInfoWriter::SetProcessId(uint32_t process_id) { DCHECK_EQ(state(), kStateMutable); diff --git a/minidump/minidump_misc_info_writer.h b/minidump/minidump_misc_info_writer.h index 8a74174..0ca5482 100644 --- a/minidump/minidump_misc_info_writer.h +++ b/minidump/minidump_misc_info_writer.h @@ -39,7 +39,7 @@ namespace crashpad { class MinidumpMiscInfoWriter final : public internal::MinidumpStreamWriter { public: MinidumpMiscInfoWriter(); - ~MinidumpMiscInfoWriter() {} + ~MinidumpMiscInfoWriter() override; //! \brief Sets the field referenced by #MINIDUMP_MISC1_PROCESS_ID. void SetProcessId(uint32_t process_id); diff --git a/minidump/minidump_misc_info_writer_test.cc b/minidump/minidump_misc_info_writer_test.cc index a342f27..8a27ad4 100644 --- a/minidump/minidump_misc_info_writer_test.cc +++ b/minidump/minidump_misc_info_writer_test.cc @@ -20,6 +20,7 @@ #include #include "base/basictypes.h" +#include "base/memory/scoped_ptr.h" #include "base/strings/string16.h" #include "base/strings/utf_string_conversions.h" #include "gtest/gtest.h" @@ -155,9 +156,9 @@ void ExpectMiscInfoEqual( TEST(MinidumpMiscInfoWriter, Empty) { MinidumpFileWriter minidump_file_writer; - MinidumpMiscInfoWriter misc_info_writer; + auto misc_info_writer = make_scoped_ptr(new MinidumpMiscInfoWriter()); - minidump_file_writer.AddStream(&misc_info_writer); + minidump_file_writer.AddStream(misc_info_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -172,13 +173,13 @@ TEST(MinidumpMiscInfoWriter, Empty) { TEST(MinidumpMiscInfoWriter, ProcessId) { MinidumpFileWriter minidump_file_writer; - MinidumpMiscInfoWriter misc_info_writer; + auto misc_info_writer = make_scoped_ptr(new MinidumpMiscInfoWriter()); const uint32_t kProcessId = 12345; - misc_info_writer.SetProcessId(kProcessId); + misc_info_writer->SetProcessId(kProcessId); - minidump_file_writer.AddStream(&misc_info_writer); + minidump_file_writer.AddStream(misc_info_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -195,16 +196,16 @@ TEST(MinidumpMiscInfoWriter, ProcessId) { TEST(MinidumpMiscInfoWriter, ProcessTimes) { MinidumpFileWriter minidump_file_writer; - MinidumpMiscInfoWriter misc_info_writer; + auto misc_info_writer = make_scoped_ptr(new MinidumpMiscInfoWriter()); const time_t kProcessCreateTime = 0x15252f00; const uint32_t kProcessUserTime = 10; const uint32_t kProcessKernelTime = 5; - misc_info_writer.SetProcessTimes( + misc_info_writer->SetProcessTimes( kProcessCreateTime, kProcessUserTime, kProcessKernelTime); - minidump_file_writer.AddStream(&misc_info_writer); + minidump_file_writer.AddStream(misc_info_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -223,7 +224,7 @@ TEST(MinidumpMiscInfoWriter, ProcessTimes) { TEST(MinidumpMiscInfoWriter, ProcessorPowerInfo) { MinidumpFileWriter minidump_file_writer; - MinidumpMiscInfoWriter misc_info_writer; + auto misc_info_writer = make_scoped_ptr(new MinidumpMiscInfoWriter()); const uint32_t kProcessorMaxMhz = 2800; const uint32_t kProcessorCurrentMhz = 2300; @@ -231,13 +232,13 @@ TEST(MinidumpMiscInfoWriter, ProcessorPowerInfo) { const uint32_t kProcessorMaxIdleState = 5; const uint32_t kProcessorCurrentIdleState = 1; - misc_info_writer.SetProcessorPowerInfo(kProcessorMaxMhz, - kProcessorCurrentMhz, - kProcessorMhzLimit, - kProcessorMaxIdleState, - kProcessorCurrentIdleState); + misc_info_writer->SetProcessorPowerInfo(kProcessorMaxMhz, + kProcessorCurrentMhz, + kProcessorMhzLimit, + kProcessorMaxIdleState, + kProcessorCurrentIdleState); - minidump_file_writer.AddStream(&misc_info_writer); + minidump_file_writer.AddStream(misc_info_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -258,13 +259,13 @@ TEST(MinidumpMiscInfoWriter, ProcessorPowerInfo) { TEST(MinidumpMiscInfoWriter, ProcessIntegrityLevel) { MinidumpFileWriter minidump_file_writer; - MinidumpMiscInfoWriter misc_info_writer; + auto misc_info_writer = make_scoped_ptr(new MinidumpMiscInfoWriter()); const uint32_t kProcessIntegrityLevel = 0x2000; - misc_info_writer.SetProcessIntegrityLevel(kProcessIntegrityLevel); + misc_info_writer->SetProcessIntegrityLevel(kProcessIntegrityLevel); - minidump_file_writer.AddStream(&misc_info_writer); + minidump_file_writer.AddStream(misc_info_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -281,13 +282,13 @@ TEST(MinidumpMiscInfoWriter, ProcessIntegrityLevel) { TEST(MinidumpMiscInfoWriter, ProcessExecuteFlags) { MinidumpFileWriter minidump_file_writer; - MinidumpMiscInfoWriter misc_info_writer; + auto misc_info_writer = make_scoped_ptr(new MinidumpMiscInfoWriter()); const uint32_t kProcessExecuteFlags = 0x13579bdf; - misc_info_writer.SetProcessExecuteFlags(kProcessExecuteFlags); + misc_info_writer->SetProcessExecuteFlags(kProcessExecuteFlags); - minidump_file_writer.AddStream(&misc_info_writer); + minidump_file_writer.AddStream(misc_info_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -304,13 +305,13 @@ TEST(MinidumpMiscInfoWriter, ProcessExecuteFlags) { TEST(MinidumpMiscInfoWriter, ProtectedProcess) { MinidumpFileWriter minidump_file_writer; - MinidumpMiscInfoWriter misc_info_writer; + auto misc_info_writer = make_scoped_ptr(new MinidumpMiscInfoWriter()); const uint32_t kProtectedProcess = 1; - misc_info_writer.SetProtectedProcess(kProtectedProcess); + misc_info_writer->SetProtectedProcess(kProtectedProcess); - minidump_file_writer.AddStream(&misc_info_writer); + minidump_file_writer.AddStream(misc_info_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -327,7 +328,7 @@ TEST(MinidumpMiscInfoWriter, ProtectedProcess) { TEST(MinidumpMiscInfoWriter, TimeZone) { MinidumpFileWriter minidump_file_writer; - MinidumpMiscInfoWriter misc_info_writer; + auto misc_info_writer = make_scoped_ptr(new MinidumpMiscInfoWriter()); const uint32_t kTimeZoneId = 2; const int32_t kBias = 300; @@ -338,16 +339,16 @@ TEST(MinidumpMiscInfoWriter, TimeZone) { const SYSTEMTIME kDaylightDate = {0, 3, 2, 0, 2, 0, 0, 0}; const int32_t kDaylightBias = -60; - misc_info_writer.SetTimeZone(kTimeZoneId, - kBias, - kStandardName, - kStandardDate, - kStandardBias, - kDaylightName, - kDaylightDate, - kDaylightBias); + misc_info_writer->SetTimeZone(kTimeZoneId, + kBias, + kStandardName, + kStandardDate, + kStandardBias, + kDaylightName, + kDaylightDate, + kDaylightBias); - minidump_file_writer.AddStream(&misc_info_writer); + minidump_file_writer.AddStream(misc_info_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -384,7 +385,7 @@ TEST(MinidumpMiscInfoWriter, TimeZoneStringsOverflow) { // to the widths of their fields. MinidumpFileWriter minidump_file_writer; - MinidumpMiscInfoWriter misc_info_writer; + auto misc_info_writer = make_scoped_ptr(new MinidumpMiscInfoWriter()); const uint32_t kTimeZoneId = 2; const int32_t kBias = 300; @@ -400,16 +401,16 @@ TEST(MinidumpMiscInfoWriter, TimeZoneStringsOverflow) { // provide daylight saving time transition times. const SYSTEMTIME kSystemTimeZero = {}; - misc_info_writer.SetTimeZone(kTimeZoneId, - kBias, - standard_name, - kSystemTimeZero, - kStandardBias, - daylight_name, - kSystemTimeZero, - kDaylightBias); + misc_info_writer->SetTimeZone(kTimeZoneId, + kBias, + standard_name, + kSystemTimeZero, + kStandardBias, + daylight_name, + kSystemTimeZero, + kDaylightBias); - minidump_file_writer.AddStream(&misc_info_writer); + minidump_file_writer.AddStream(misc_info_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -443,14 +444,14 @@ TEST(MinidumpMiscInfoWriter, TimeZoneStringsOverflow) { TEST(MinidumpMiscInfoWriter, BuildStrings) { MinidumpFileWriter minidump_file_writer; - MinidumpMiscInfoWriter misc_info_writer; + auto misc_info_writer = make_scoped_ptr(new MinidumpMiscInfoWriter()); const char kBuildString[] = "build string"; const char kDebugBuildString[] = "debug build string"; - misc_info_writer.SetBuildString(kBuildString, kDebugBuildString); + misc_info_writer->SetBuildString(kBuildString, kDebugBuildString); - minidump_file_writer.AddStream(&misc_info_writer); + minidump_file_writer.AddStream(misc_info_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -477,16 +478,16 @@ TEST(MinidumpMiscInfoWriter, BuildStringsOverflow) { // widths of their fields. MinidumpFileWriter minidump_file_writer; - MinidumpMiscInfoWriter misc_info_writer; + auto misc_info_writer = make_scoped_ptr(new MinidumpMiscInfoWriter()); std::string build_string(arraysize(MINIDUMP_MISC_INFO_N::BuildString) + 1, 'B'); std::string debug_build_string(arraysize(MINIDUMP_MISC_INFO_N::DbgBldStr), 'D'); - misc_info_writer.SetBuildString(build_string, debug_build_string); + misc_info_writer->SetBuildString(build_string, debug_build_string); - minidump_file_writer.AddStream(&misc_info_writer); + minidump_file_writer.AddStream(misc_info_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -510,7 +511,7 @@ TEST(MinidumpMiscInfoWriter, BuildStringsOverflow) { TEST(MinidumpMiscInfoWriter, Everything) { MinidumpFileWriter minidump_file_writer; - MinidumpMiscInfoWriter misc_info_writer; + auto misc_info_writer = make_scoped_ptr(new MinidumpMiscInfoWriter()); const uint32_t kProcessId = 12345; const time_t kProcessCreateTime = 0x15252f00; @@ -534,28 +535,28 @@ TEST(MinidumpMiscInfoWriter, Everything) { const char kBuildString[] = "build string"; const char kDebugBuildString[] = "debug build string"; - misc_info_writer.SetProcessId(kProcessId); - misc_info_writer.SetProcessTimes( + misc_info_writer->SetProcessId(kProcessId); + misc_info_writer->SetProcessTimes( kProcessCreateTime, kProcessUserTime, kProcessKernelTime); - misc_info_writer.SetProcessorPowerInfo(kProcessorMaxMhz, - kProcessorCurrentMhz, - kProcessorMhzLimit, - kProcessorMaxIdleState, - kProcessorCurrentIdleState); - misc_info_writer.SetProcessIntegrityLevel(kProcessIntegrityLevel); - misc_info_writer.SetProcessExecuteFlags(kProcessExecuteFlags); - misc_info_writer.SetProtectedProcess(kProtectedProcess); - misc_info_writer.SetTimeZone(kTimeZoneId, - kBias, - kStandardName, - kSystemTimeZero, - kStandardBias, - kDaylightName, - kSystemTimeZero, - kDaylightBias); - misc_info_writer.SetBuildString(kBuildString, kDebugBuildString); + misc_info_writer->SetProcessorPowerInfo(kProcessorMaxMhz, + kProcessorCurrentMhz, + kProcessorMhzLimit, + kProcessorMaxIdleState, + kProcessorCurrentIdleState); + misc_info_writer->SetProcessIntegrityLevel(kProcessIntegrityLevel); + misc_info_writer->SetProcessExecuteFlags(kProcessExecuteFlags); + misc_info_writer->SetProtectedProcess(kProtectedProcess); + misc_info_writer->SetTimeZone(kTimeZoneId, + kBias, + kStandardName, + kSystemTimeZero, + kStandardBias, + kDaylightName, + kSystemTimeZero, + kDaylightBias); + misc_info_writer->SetBuildString(kBuildString, kDebugBuildString); - minidump_file_writer.AddStream(&misc_info_writer); + minidump_file_writer.AddStream(misc_info_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); diff --git a/minidump/minidump_module_crashpad_info_writer.cc b/minidump/minidump_module_crashpad_info_writer.cc index eae793b..a7afbda 100644 --- a/minidump/minidump_module_crashpad_info_writer.cc +++ b/minidump/minidump_module_crashpad_info_writer.cc @@ -32,10 +32,10 @@ MinidumpModuleCrashpadInfoWriter::~MinidumpModuleCrashpadInfoWriter() { } void MinidumpModuleCrashpadInfoWriter::SetSimpleAnnotations( - MinidumpSimpleStringDictionaryWriter* simple_annotations) { + scoped_ptr simple_annotations) { DCHECK_EQ(state(), kStateMutable); - simple_annotations_ = simple_annotations; + simple_annotations_ = simple_annotations.Pass(); } bool MinidumpModuleCrashpadInfoWriter::Freeze() { @@ -65,7 +65,7 @@ MinidumpModuleCrashpadInfoWriter::Children() { std::vector children; if (simple_annotations_) { - children.push_back(simple_annotations_); + children.push_back(simple_annotations_.get()); } return children; @@ -89,10 +89,10 @@ MinidumpModuleCrashpadInfoListWriter::~MinidumpModuleCrashpadInfoListWriter() { } void MinidumpModuleCrashpadInfoListWriter::AddModule( - MinidumpModuleCrashpadInfoWriter* module) { + scoped_ptr module) { DCHECK_EQ(state(), kStateMutable); - modules_.push_back(module); + modules_.push_back(module.release()); } bool MinidumpModuleCrashpadInfoListWriter::Freeze() { diff --git a/minidump/minidump_module_crashpad_info_writer.h b/minidump/minidump_module_crashpad_info_writer.h index 7c84fa0..1292917 100644 --- a/minidump/minidump_module_crashpad_info_writer.h +++ b/minidump/minidump_module_crashpad_info_writer.h @@ -20,8 +20,10 @@ #include #include "base/basictypes.h" +#include "base/memory/scoped_ptr.h" #include "minidump/minidump_extensions.h" #include "minidump/minidump_writable.h" +#include "util/stdlib/pointer_container.h" namespace crashpad { @@ -33,7 +35,7 @@ class MinidumpModuleCrashpadInfoWriter final : public internal::MinidumpWritable { public: MinidumpModuleCrashpadInfoWriter(); - ~MinidumpModuleCrashpadInfoWriter(); + ~MinidumpModuleCrashpadInfoWriter() override; //! \brief Sets MinidumpModuleCrashpadInfo::minidump_module_list_index. void SetMinidumpModuleListIndex(uint32_t minidump_module_list_index) { @@ -44,12 +46,12 @@ class MinidumpModuleCrashpadInfoWriter final //! point to the MinidumpSimpleStringDictionaryWriter object to be written //! by \a simple_annotations. //! - //! \a simple_annotations will become a child of this object in the overall - //! tree of internal::MinidumpWritable objects. + //! This object takes ownership of \a simple_annotations and becomes its + //! parent in the overall tree of internal::MinidumpWritable objects. //! //! \note Valid in #kStateMutable. void SetSimpleAnnotations( - MinidumpSimpleStringDictionaryWriter* simple_annotations); + scoped_ptr simple_annotations); protected: // MinidumpWritable: @@ -60,7 +62,7 @@ class MinidumpModuleCrashpadInfoWriter final private: MinidumpModuleCrashpadInfo module_; - MinidumpSimpleStringDictionaryWriter* simple_annotations_; // weak + scoped_ptr simple_annotations_; DISALLOW_COPY_AND_ASSIGN(MinidumpModuleCrashpadInfoWriter); }; @@ -71,16 +73,16 @@ class MinidumpModuleCrashpadInfoListWriter final : public internal::MinidumpWritable { public: MinidumpModuleCrashpadInfoListWriter(); - ~MinidumpModuleCrashpadInfoListWriter(); + ~MinidumpModuleCrashpadInfoListWriter() override; //! \brief Adds a MinidumpModuleCrashpadInfo to the //! MinidumpModuleCrashpadInfoList. //! - //! \a module will become a child of this object in the overall tree of - //! internal::MinidumpWritable objects. + //! This object takes ownership of \a module and becomes its parent in the + //! overall tree of internal::MinidumpWritable objects. //! //! \note Valid in #kStateMutable. - void AddModule(MinidumpModuleCrashpadInfoWriter* module); + void AddModule(scoped_ptr module); protected: // MinidumpWritable: @@ -91,7 +93,7 @@ class MinidumpModuleCrashpadInfoListWriter final private: MinidumpModuleCrashpadInfoList module_list_base_; - std::vector modules_; // weak + PointerVector modules_; std::vector module_location_descriptors_; DISALLOW_COPY_AND_ASSIGN(MinidumpModuleCrashpadInfoListWriter); diff --git a/minidump/minidump_module_crashpad_info_writer_test.cc b/minidump/minidump_module_crashpad_info_writer_test.cc index f52c4e0..712edfb 100644 --- a/minidump/minidump_module_crashpad_info_writer_test.cc +++ b/minidump/minidump_module_crashpad_info_writer_test.cc @@ -59,8 +59,9 @@ TEST(MinidumpModuleCrashpadInfoWriter, EmptyModule) { StringFileWriter file_writer; MinidumpModuleCrashpadInfoListWriter module_list_writer; - MinidumpModuleCrashpadInfoWriter module_writer; - module_list_writer.AddModule(&module_writer); + auto module_writer = + make_scoped_ptr(new MinidumpModuleCrashpadInfoWriter()); + module_list_writer.AddModule(module_writer.Pass()); EXPECT_TRUE(module_list_writer.WriteEverything(&file_writer)); ASSERT_EQ(sizeof(MinidumpModuleCrashpadInfoList) + @@ -94,16 +95,18 @@ TEST(MinidumpModuleCrashpadInfoWriter, FullModule) { MinidumpModuleCrashpadInfoListWriter module_list_writer; - MinidumpModuleCrashpadInfoWriter module_writer; - module_writer.SetMinidumpModuleListIndex(kMinidumpModuleListIndex); - MinidumpSimpleStringDictionaryWriter simple_string_dictionary_writer; - MinidumpSimpleStringDictionaryEntryWriter - simple_string_dictionary_entry_writer; - simple_string_dictionary_entry_writer.SetKeyValue(kKey, kValue); - simple_string_dictionary_writer.AddEntry( - &simple_string_dictionary_entry_writer); - module_writer.SetSimpleAnnotations(&simple_string_dictionary_writer); - module_list_writer.AddModule(&module_writer); + auto module_writer = + make_scoped_ptr(new MinidumpModuleCrashpadInfoWriter()); + module_writer->SetMinidumpModuleListIndex(kMinidumpModuleListIndex); + 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()); + module_list_writer.AddModule(module_writer.Pass()); EXPECT_TRUE(module_list_writer.WriteEverything(&file_writer)); ASSERT_EQ(sizeof(MinidumpModuleCrashpadInfoList) + @@ -160,36 +163,43 @@ TEST(MinidumpModuleCrashpadInfoWriter, ThreeModules) { MinidumpModuleCrashpadInfoListWriter module_list_writer; - MinidumpModuleCrashpadInfoWriter module_writer_0; - module_writer_0.SetMinidumpModuleListIndex(kMinidumpModuleListIndex0); - MinidumpSimpleStringDictionaryWriter simple_string_dictionary_writer_0; - MinidumpSimpleStringDictionaryEntryWriter - simple_string_dictionary_entry_writer_0; - simple_string_dictionary_entry_writer_0.SetKeyValue(kKey0, kValue0); - simple_string_dictionary_writer_0.AddEntry( - &simple_string_dictionary_entry_writer_0); - module_writer_0.SetSimpleAnnotations(&simple_string_dictionary_writer_0); - module_list_writer.AddModule(&module_writer_0); + auto module_writer_0 = + make_scoped_ptr(new MinidumpModuleCrashpadInfoWriter()); + module_writer_0->SetMinidumpModuleListIndex(kMinidumpModuleListIndex0); + auto simple_string_dictionary_writer_0 = + make_scoped_ptr(new MinidumpSimpleStringDictionaryWriter()); + auto simple_string_dictionary_entry_writer_0 = + 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()); + module_writer_0->SetSimpleAnnotations( + simple_string_dictionary_writer_0.Pass()); + module_list_writer.AddModule(module_writer_0.Pass()); - MinidumpModuleCrashpadInfoWriter module_writer_1; - module_writer_1.SetMinidumpModuleListIndex(kMinidumpModuleListIndex1); - module_list_writer.AddModule(&module_writer_1); + auto module_writer_1 = + make_scoped_ptr(new MinidumpModuleCrashpadInfoWriter()); + module_writer_1->SetMinidumpModuleListIndex(kMinidumpModuleListIndex1); + module_list_writer.AddModule(module_writer_1.Pass()); - MinidumpModuleCrashpadInfoWriter module_writer_2; - module_writer_2.SetMinidumpModuleListIndex(kMinidumpModuleListIndex2); - MinidumpSimpleStringDictionaryWriter simple_string_dictionary_writer_2; - MinidumpSimpleStringDictionaryEntryWriter - simple_string_dictionary_entry_writer_2a; - simple_string_dictionary_entry_writer_2a.SetKeyValue(kKey2A, kValue2A); - simple_string_dictionary_writer_2.AddEntry( - &simple_string_dictionary_entry_writer_2a); - MinidumpSimpleStringDictionaryEntryWriter - simple_string_dictionary_entry_writer_2b; - simple_string_dictionary_entry_writer_2b.SetKeyValue(kKey2B, kValue2B); - simple_string_dictionary_writer_2.AddEntry( - &simple_string_dictionary_entry_writer_2b); - module_writer_2.SetSimpleAnnotations(&simple_string_dictionary_writer_2); - module_list_writer.AddModule(&module_writer_2); + auto module_writer_2 = + make_scoped_ptr(new MinidumpModuleCrashpadInfoWriter()); + module_writer_2->SetMinidumpModuleListIndex(kMinidumpModuleListIndex2); + auto simple_string_dictionary_writer_2 = + make_scoped_ptr(new MinidumpSimpleStringDictionaryWriter()); + auto simple_string_dictionary_entry_writer_2a = + 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()); + 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()); + module_writer_2->SetSimpleAnnotations( + simple_string_dictionary_writer_2.Pass()); + module_list_writer.AddModule(module_writer_2.Pass()); EXPECT_TRUE(module_list_writer.WriteEverything(&file_writer)); diff --git a/minidump/minidump_module_writer.cc b/minidump/minidump_module_writer.cc index 8aa9680..45ca9e4 100644 --- a/minidump/minidump_module_writer.cc +++ b/minidump/minidump_module_writer.cc @@ -103,6 +103,9 @@ MinidumpModuleMiscDebugRecordWriter::MinidumpModuleMiscDebugRecordWriter() data_utf16_() { } +MinidumpModuleMiscDebugRecordWriter::~MinidumpModuleMiscDebugRecordWriter() { +} + void MinidumpModuleMiscDebugRecordWriter::SetData(const std::string& data, bool utf16) { DCHECK_EQ(state(), kStateMutable); @@ -199,17 +202,17 @@ void MinidumpModuleWriter::SetName(const std::string& name) { } void MinidumpModuleWriter::SetCodeViewRecord( - MinidumpModuleCodeViewRecordWriter* codeview_record) { + scoped_ptr codeview_record) { DCHECK_EQ(state(), kStateMutable); - codeview_record_ = codeview_record; + codeview_record_ = codeview_record.Pass(); } void MinidumpModuleWriter::SetMiscDebugRecord( - MinidumpModuleMiscDebugRecordWriter* misc_debug_record) { + scoped_ptr misc_debug_record) { DCHECK_EQ(state(), kStateMutable); - misc_debug_record_ = misc_debug_record; + misc_debug_record_ = misc_debug_record.Pass(); } void MinidumpModuleWriter::SetTimestamp(time_t timestamp) { @@ -288,10 +291,10 @@ std::vector MinidumpModuleWriter::Children() { std::vector children; children.push_back(name_.get()); if (codeview_record_) { - children.push_back(codeview_record_); + children.push_back(codeview_record_.get()); } if (misc_debug_record_) { - children.push_back(misc_debug_record_); + children.push_back(misc_debug_record_.get()); } return children; @@ -313,10 +316,11 @@ MinidumpModuleListWriter::MinidumpModuleListWriter() MinidumpModuleListWriter::~MinidumpModuleListWriter() { } -void MinidumpModuleListWriter::AddModule(MinidumpModuleWriter* module) { +void MinidumpModuleListWriter::AddModule( + scoped_ptr module) { DCHECK_EQ(state(), kStateMutable); - modules_.push_back(module); + modules_.push_back(module.release()); } bool MinidumpModuleListWriter::Freeze() { diff --git a/minidump/minidump_module_writer.h b/minidump/minidump_module_writer.h index 6cef8ad..4ec6ed4 100644 --- a/minidump/minidump_module_writer.h +++ b/minidump/minidump_module_writer.h @@ -28,6 +28,7 @@ #include "minidump/minidump_extensions.h" #include "minidump/minidump_stream_writer.h" #include "minidump/minidump_writable.h" +#include "util/stdlib/pointer_container.h" namespace crashpad { @@ -39,7 +40,7 @@ class MinidumpUTF16StringWriter; //! MINIDUMP_MODULE::CvRecord in minidump files. class MinidumpModuleCodeViewRecordWriter : public internal::MinidumpWritable { public: - virtual ~MinidumpModuleCodeViewRecordWriter(); + ~MinidumpModuleCodeViewRecordWriter() override; protected: MinidumpModuleCodeViewRecordWriter() : MinidumpWritable() {} @@ -134,7 +135,7 @@ class MinidumpModuleMiscDebugRecordWriter final : public internal::MinidumpWritable { public: MinidumpModuleMiscDebugRecordWriter(); - ~MinidumpModuleMiscDebugRecordWriter() {} + ~MinidumpModuleMiscDebugRecordWriter() override; //! \brief Sets IMAGE_DEBUG_MISC::DataType. void SetDataType(uint32_t data_type) { @@ -173,7 +174,7 @@ class MinidumpModuleMiscDebugRecordWriter final class MinidumpModuleWriter final : public internal::MinidumpWritable { public: MinidumpModuleWriter(); - ~MinidumpModuleWriter(); + ~MinidumpModuleWriter() override; //! \brief Returns a MINIDUMP_MODULE referencing this object’s data. //! @@ -194,21 +195,22 @@ class MinidumpModuleWriter final : public internal::MinidumpWritable { //! \brief Arranges for MINIDUMP_MODULE::CvRecord to point to a CodeView //! record to be written by \a codeview_record. //! - //! \a codeview_record will become a child of this object in the overall tree - //! of internal::MinidumpWritable objects. + //! This object takes ownership of \a codeview_record and becomes its parent + //! in the overall tree of internal::MinidumpWritable objects. //! //! \note Valid in #kStateMutable. - void SetCodeViewRecord(MinidumpModuleCodeViewRecordWriter* codeview_record); + void SetCodeViewRecord( + scoped_ptr codeview_record); //! \brief Arranges for MINIDUMP_MODULE::MiscRecord to point to an //! IMAGE_DEBUG_MISC object to be written by \a misc_debug_record. //! - //! \a misc_debug_record will become a child of this object in the overall - //! tree of internal::MinidumpWritable objects. + //! This object takes ownership of \a misc_debug_record and becomes its parent + //! in the overall tree of internal::MinidumpWritable objects. //! //! \note Valid in #kStateMutable. void SetMiscDebugRecord( - MinidumpModuleMiscDebugRecordWriter* misc_debug_record); + scoped_ptr misc_debug_record); //! \brief Sets IMAGE_DEBUG_MISC::BaseOfImage. void SetImageBaseAddress(uint64_t image_base_address) { @@ -279,8 +281,8 @@ class MinidumpModuleWriter final : public internal::MinidumpWritable { private: MINIDUMP_MODULE module_; scoped_ptr name_; - MinidumpModuleCodeViewRecordWriter* codeview_record_; // weak - MinidumpModuleMiscDebugRecordWriter* misc_debug_record_; // weak + scoped_ptr codeview_record_; + scoped_ptr misc_debug_record_; DISALLOW_COPY_AND_ASSIGN(MinidumpModuleWriter); }; @@ -290,15 +292,15 @@ class MinidumpModuleWriter final : public internal::MinidumpWritable { class MinidumpModuleListWriter final : public internal::MinidumpStreamWriter { public: MinidumpModuleListWriter(); - ~MinidumpModuleListWriter(); + ~MinidumpModuleListWriter() override; //! \brief Adds a MinidumpModuleWriter to the MINIDUMP_MODULE_LIST. //! - //! \a module will become a child of this object in the overall tree of - //! internal::MinidumpWritable objects. + //! This object takes ownership of \a module and becomes its parent in the + //! overall tree of internal::MinidumpWritable objects. //! //! \note Valid in #kStateMutable. - void AddModule(MinidumpModuleWriter* module); + void AddModule(scoped_ptr module); protected: // MinidumpWritable: @@ -312,7 +314,7 @@ class MinidumpModuleListWriter final : public internal::MinidumpStreamWriter { private: MINIDUMP_MODULE_LIST module_list_base_; - std::vector modules_; // weak + PointerVector modules_; DISALLOW_COPY_AND_ASSIGN(MinidumpModuleListWriter); }; diff --git a/minidump/minidump_module_writer_test.cc b/minidump/minidump_module_writer_test.cc index 3a00def..ed5f10c 100644 --- a/minidump/minidump_module_writer_test.cc +++ b/minidump/minidump_module_writer_test.cc @@ -59,9 +59,9 @@ void GetModuleListStream(const std::string& file_contents, TEST(MinidumpModuleWriter, EmptyModuleList) { MinidumpFileWriter minidump_file_writer; - MinidumpModuleListWriter module_list_writer; + auto module_list_writer = make_scoped_ptr(new MinidumpModuleListWriter()); - minidump_file_writer.AddStream(&module_list_writer); + minidump_file_writer.AddStream(module_list_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -265,15 +265,15 @@ void ExpectModule(const MINIDUMP_MODULE* expected, TEST(MinidumpModuleWriter, EmptyModule) { MinidumpFileWriter minidump_file_writer; - MinidumpModuleListWriter module_list_writer; + auto module_list_writer = make_scoped_ptr(new MinidumpModuleListWriter()); const char kModuleName[] = "test_executable"; - MinidumpModuleWriter module_writer; - module_writer.SetName(kModuleName); + auto module_writer = make_scoped_ptr(new MinidumpModuleWriter()); + module_writer->SetName(kModuleName); - module_list_writer.AddModule(&module_writer); - minidump_file_writer.AddStream(&module_list_writer); + module_list_writer->AddModule(module_writer.Pass()); + minidump_file_writer.AddStream(module_list_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -304,7 +304,7 @@ TEST(MinidumpModuleWriter, EmptyModule) { TEST(MinidumpModuleWriter, OneModule) { MinidumpFileWriter minidump_file_writer; - MinidumpModuleListWriter module_list_writer; + auto module_list_writer = make_scoped_ptr(new MinidumpModuleListWriter()); const char kModuleName[] = "statically_linked"; const uint64_t kModuleBase = 0x10da69000; @@ -333,36 +333,38 @@ TEST(MinidumpModuleWriter, OneModule) { const char kDebugName[] = "statical.dbg"; const bool kDebugUTF16 = false; - MinidumpModuleWriter module_writer; - module_writer.SetName(kModuleName); - module_writer.SetImageBaseAddress(kModuleBase); - module_writer.SetImageSize(kModuleSize); - module_writer.SetChecksum(kChecksum); - module_writer.SetTimestamp(kTimestamp); - module_writer.SetFileVersion(kFileVersionMS >> 16, - kFileVersionMS & 0xffff, - kFileVersionLS >> 16, - kFileVersionLS & 0xffff); - module_writer.SetProductVersion(kProductVersionMS >> 16, - kProductVersionMS & 0xffff, - kProductVersionLS >> 16, - kProductVersionLS & 0xffff); - module_writer.SetFileFlagsAndMask(kFileFlags, kFileFlagsMask); - module_writer.SetFileOS(kFileOS); - module_writer.SetFileTypeAndSubtype(kFileType, kFileSubtype); + auto module_writer = make_scoped_ptr(new MinidumpModuleWriter()); + module_writer->SetName(kModuleName); + module_writer->SetImageBaseAddress(kModuleBase); + module_writer->SetImageSize(kModuleSize); + module_writer->SetChecksum(kChecksum); + module_writer->SetTimestamp(kTimestamp); + module_writer->SetFileVersion(kFileVersionMS >> 16, + kFileVersionMS & 0xffff, + kFileVersionLS >> 16, + kFileVersionLS & 0xffff); + module_writer->SetProductVersion(kProductVersionMS >> 16, + kProductVersionMS & 0xffff, + kProductVersionLS >> 16, + kProductVersionLS & 0xffff); + module_writer->SetFileFlagsAndMask(kFileFlags, kFileFlagsMask); + module_writer->SetFileOS(kFileOS); + module_writer->SetFileTypeAndSubtype(kFileType, kFileSubtype); - MinidumpModuleCodeViewRecordPDB70Writer codeview_pdb70_writer; - codeview_pdb70_writer.SetPDBName(kPDBName); - codeview_pdb70_writer.SetUUIDAndAge(pdb_uuid, kPDBAge); - module_writer.SetCodeViewRecord(&codeview_pdb70_writer); + auto codeview_pdb70_writer = + make_scoped_ptr(new MinidumpModuleCodeViewRecordPDB70Writer()); + codeview_pdb70_writer->SetPDBName(kPDBName); + codeview_pdb70_writer->SetUUIDAndAge(pdb_uuid, kPDBAge); + module_writer->SetCodeViewRecord(codeview_pdb70_writer.Pass()); - MinidumpModuleMiscDebugRecordWriter misc_debug_writer; - misc_debug_writer.SetDataType(kDebugType); - misc_debug_writer.SetData(kDebugName, kDebugUTF16); - module_writer.SetMiscDebugRecord(&misc_debug_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_list_writer.AddModule(&module_writer); - minidump_file_writer.AddStream(&module_list_writer); + module_list_writer->AddModule(module_writer.Pass()); + minidump_file_writer.AddStream(module_list_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -411,7 +413,7 @@ TEST(MinidumpModuleWriter, OneModule_CodeViewUsesPDB20_MiscUsesUTF16) { // alternatives, a PDB 2.0 link as the CodeView record and an IMAGE_DEBUG_MISC // record with UTF-16 data. MinidumpFileWriter minidump_file_writer; - MinidumpModuleListWriter module_list_writer; + auto module_list_writer = make_scoped_ptr(new MinidumpModuleListWriter()); const char kModuleName[] = "dinosaur"; const char kPDBName[] = "d1n05.pdb"; @@ -421,21 +423,23 @@ TEST(MinidumpModuleWriter, OneModule_CodeViewUsesPDB20_MiscUsesUTF16) { const char kDebugName[] = "d1n05.dbg"; const bool kDebugUTF16 = true; - MinidumpModuleWriter module_writer; - module_writer.SetName(kModuleName); + auto module_writer = make_scoped_ptr(new MinidumpModuleWriter()); + module_writer->SetName(kModuleName); - MinidumpModuleCodeViewRecordPDB20Writer codeview_pdb20_writer; - codeview_pdb20_writer.SetPDBName(kPDBName); - codeview_pdb20_writer.SetTimestampAndAge(kPDBTimestamp, kPDBAge); - module_writer.SetCodeViewRecord(&codeview_pdb20_writer); + auto codeview_pdb20_writer = + make_scoped_ptr(new MinidumpModuleCodeViewRecordPDB20Writer()); + codeview_pdb20_writer->SetPDBName(kPDBName); + codeview_pdb20_writer->SetTimestampAndAge(kPDBTimestamp, kPDBAge); + module_writer->SetCodeViewRecord(codeview_pdb20_writer.Pass()); - MinidumpModuleMiscDebugRecordWriter misc_debug_writer; - misc_debug_writer.SetDataType(kDebugType); - misc_debug_writer.SetData(kDebugName, kDebugUTF16); - module_writer.SetMiscDebugRecord(&misc_debug_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_list_writer.AddModule(&module_writer); - minidump_file_writer.AddStream(&module_list_writer); + module_list_writer->AddModule(module_writer.Pass()); + minidump_file_writer.AddStream(module_list_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -470,7 +474,7 @@ TEST(MinidumpModuleWriter, ThreeModules) { // its CodeView record, one with no CodeView record, and one with a PDB 2.0 // link as its CodeView record. MinidumpFileWriter minidump_file_writer; - MinidumpModuleListWriter module_list_writer; + auto module_list_writer = make_scoped_ptr(new MinidumpModuleListWriter()); const char kModuleName0[] = "main"; const uint64_t kModuleBase0 = 0x100101000; @@ -494,38 +498,40 @@ TEST(MinidumpModuleWriter, ThreeModules) { const time_t kPDBTimestamp2 = 0x386d4380; const uint32_t kPDBAge2 = 2; - MinidumpModuleWriter module_writer_0; - module_writer_0.SetName(kModuleName0); - module_writer_0.SetImageBaseAddress(kModuleBase0); - module_writer_0.SetImageSize(kModuleSize0); + auto module_writer_0 = make_scoped_ptr(new MinidumpModuleWriter()); + module_writer_0->SetName(kModuleName0); + module_writer_0->SetImageBaseAddress(kModuleBase0); + module_writer_0->SetImageSize(kModuleSize0); - MinidumpModuleCodeViewRecordPDB70Writer codeview_pdb70_writer_0; - codeview_pdb70_writer_0.SetPDBName(kPDBName0); - codeview_pdb70_writer_0.SetUUIDAndAge(pdb_uuid_0, kPDBAge0); - module_writer_0.SetCodeViewRecord(&codeview_pdb70_writer_0); + auto codeview_pdb70_writer_0 = + 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_list_writer.AddModule(&module_writer_0); + module_list_writer->AddModule(module_writer_0.Pass()); - MinidumpModuleWriter module_writer_1; - module_writer_1.SetName(kModuleName1); - module_writer_1.SetImageBaseAddress(kModuleBase1); - module_writer_1.SetImageSize(kModuleSize1); + 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); + module_list_writer->AddModule(module_writer_1.Pass()); - MinidumpModuleWriter module_writer_2; - module_writer_2.SetName(kModuleName2); - module_writer_2.SetImageBaseAddress(kModuleBase2); - module_writer_2.SetImageSize(kModuleSize2); + auto module_writer_2 = make_scoped_ptr(new MinidumpModuleWriter()); + module_writer_2->SetName(kModuleName2); + module_writer_2->SetImageBaseAddress(kModuleBase2); + module_writer_2->SetImageSize(kModuleSize2); - MinidumpModuleCodeViewRecordPDB20Writer codeview_pdb70_writer_2; - codeview_pdb70_writer_2.SetPDBName(kPDBName2); - codeview_pdb70_writer_2.SetTimestampAndAge(kPDBTimestamp2, kPDBAge2); - module_writer_2.SetCodeViewRecord(&codeview_pdb70_writer_2); + auto codeview_pdb70_writer_2 = + 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_list_writer.AddModule(&module_writer_2); + module_list_writer->AddModule(module_writer_2.Pass()); - minidump_file_writer.AddStream(&module_list_writer); + minidump_file_writer.AddStream(module_list_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -602,10 +608,10 @@ TEST(MinidumpModuleWriter, ThreeModules) { TEST(MinidumpSystemInfoWriterDeathTest, NoModuleName) { MinidumpFileWriter minidump_file_writer; - MinidumpModuleListWriter module_list_writer; - MinidumpModuleWriter module_writer; - module_list_writer.AddModule(&module_writer); - minidump_file_writer.AddStream(&module_list_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()); StringFileWriter file_writer; ASSERT_DEATH(minidump_file_writer.WriteEverything(&file_writer), "name_"); diff --git a/minidump/minidump_simple_string_dictionary_writer.cc b/minidump/minidump_simple_string_dictionary_writer.cc index ef3a1ec..9218bd9 100644 --- a/minidump/minidump_simple_string_dictionary_writer.cc +++ b/minidump/minidump_simple_string_dictionary_writer.cc @@ -15,6 +15,7 @@ #include "minidump/minidump_simple_string_dictionary_writer.h" #include "base/logging.h" +#include "base/stl_util.h" #include "util/file/file_writer.h" #include "util/numeric/safe_assignment.h" @@ -92,13 +93,21 @@ MinidumpSimpleStringDictionaryWriter::MinidumpSimpleStringDictionaryWriter() } MinidumpSimpleStringDictionaryWriter::~MinidumpSimpleStringDictionaryWriter() { + STLDeleteContainerPairSecondPointers(entries_.begin(), entries_.end()); } void MinidumpSimpleStringDictionaryWriter::AddEntry( - MinidumpSimpleStringDictionaryEntryWriter* entry) { + scoped_ptr entry) { DCHECK_GE(state(), kStateMutable); - entries_[entry->Key()] = entry; + const std::string& key = entry->Key(); + auto iterator = entries_.find(key); + if (iterator != entries_.end()) { + delete iterator->second; + iterator->second = entry.release(); + } else { + entries_[key] = entry.release(); + } } bool MinidumpSimpleStringDictionaryWriter::Freeze() { diff --git a/minidump/minidump_simple_string_dictionary_writer.h b/minidump/minidump_simple_string_dictionary_writer.h index 8a6c31d..17075fe 100644 --- a/minidump/minidump_simple_string_dictionary_writer.h +++ b/minidump/minidump_simple_string_dictionary_writer.h @@ -22,6 +22,7 @@ #include #include "base/basictypes.h" +#include "base/memory/scoped_ptr.h" #include "minidump/minidump_extensions.h" #include "minidump/minidump_string_writer.h" #include "minidump/minidump_writable.h" @@ -40,7 +41,7 @@ class MinidumpSimpleStringDictionaryEntryWriter final : public internal::MinidumpWritable { public: MinidumpSimpleStringDictionaryEntryWriter(); - ~MinidumpSimpleStringDictionaryEntryWriter(); + ~MinidumpSimpleStringDictionaryEntryWriter() override; //! \brief Returns a MinidumpSimpleStringDictionaryEntry referencing this //! object’s data. @@ -88,20 +89,20 @@ class MinidumpSimpleStringDictionaryWriter final : public internal::MinidumpWritable { public: MinidumpSimpleStringDictionaryWriter(); - ~MinidumpSimpleStringDictionaryWriter(); + ~MinidumpSimpleStringDictionaryWriter() override; //! \brief Adds a MinidumpSimpleStringDictionaryEntryWriter to the //! MinidumpSimpleStringDictionary. //! - //! \a entry will become a child of this object in the overall tree of - //! internal::MinidumpWritable objects. + //! This object takes ownership of \a entry and becomes its parent in the + //! overall tree of internal::MinidumpWritable objects. //! //! If the key contained in \a entry duplicates the key of an entry already //! present in the MinidumpSimpleStringDictionary, the new \a entry will //! replace the previous one. //! //! \note Valid in #kStateMutable. - void AddEntry(MinidumpSimpleStringDictionaryEntryWriter* entry); + void AddEntry(scoped_ptr entry); protected: // MinidumpWritable: @@ -113,8 +114,9 @@ class MinidumpSimpleStringDictionaryWriter final private: MinidumpSimpleStringDictionary simple_string_dictionary_base_; - std::map - entries_; // weak + + // This object owns the MinidumpSimpleStringDictionaryEntryWriter objects. + std::map entries_; DISALLOW_COPY_AND_ASSIGN(MinidumpSimpleStringDictionaryWriter); }; diff --git a/minidump/minidump_simple_string_dictionary_writer_test.cc b/minidump/minidump_simple_string_dictionary_writer_test.cc index 7ab62a8..62b1479 100644 --- a/minidump/minidump_simple_string_dictionary_writer_test.cc +++ b/minidump/minidump_simple_string_dictionary_writer_test.cc @@ -57,8 +57,9 @@ TEST(MinidumpSimpleStringDictionaryWriter, EmptyKeyValue) { StringFileWriter file_writer; MinidumpSimpleStringDictionaryWriter dictionary_writer; - MinidumpSimpleStringDictionaryEntryWriter entry_writer; - dictionary_writer.AddEntry(&entry_writer); + auto entry_writer = + make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); + dictionary_writer.AddEntry(entry_writer.Pass()); EXPECT_TRUE(dictionary_writer.WriteEverything(&file_writer)); ASSERT_EQ(sizeof(MinidumpSimpleStringDictionary) + @@ -87,9 +88,10 @@ TEST(MinidumpSimpleStringDictionaryWriter, OneKeyValue) { char kValue[] = "value"; MinidumpSimpleStringDictionaryWriter dictionary_writer; - MinidumpSimpleStringDictionaryEntryWriter entry_writer; - entry_writer.SetKeyValue(kKey, kValue); - dictionary_writer.AddEntry(&entry_writer); + auto entry_writer = + make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); + entry_writer->SetKeyValue(kKey, kValue); + dictionary_writer.AddEntry(entry_writer.Pass()); EXPECT_TRUE(dictionary_writer.WriteEverything(&file_writer)); ASSERT_EQ(sizeof(MinidumpSimpleStringDictionary) + @@ -122,15 +124,18 @@ TEST(MinidumpSimpleStringDictionaryWriter, ThreeKeysValues) { char kValue2[] = "val2"; MinidumpSimpleStringDictionaryWriter dictionary_writer; - MinidumpSimpleStringDictionaryEntryWriter entry_writer_0; - entry_writer_0.SetKeyValue(kKey0, kValue0); - dictionary_writer.AddEntry(&entry_writer_0); - MinidumpSimpleStringDictionaryEntryWriter entry_writer_1; - entry_writer_1.SetKeyValue(kKey1, kValue1); - dictionary_writer.AddEntry(&entry_writer_1); - MinidumpSimpleStringDictionaryEntryWriter entry_writer_2; - entry_writer_2.SetKeyValue(kKey2, kValue2); - dictionary_writer.AddEntry(&entry_writer_2); + auto entry_writer_0 = + make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); + entry_writer_0->SetKeyValue(kKey0, kValue0); + dictionary_writer.AddEntry(entry_writer_0.Pass()); + auto entry_writer_1 = + make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); + entry_writer_1->SetKeyValue(kKey1, kValue1); + dictionary_writer.AddEntry(entry_writer_1.Pass()); + auto entry_writer_2 = + make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); + entry_writer_2->SetKeyValue(kKey2, kValue2); + dictionary_writer.AddEntry(entry_writer_2.Pass()); EXPECT_TRUE(dictionary_writer.WriteEverything(&file_writer)); ASSERT_EQ(sizeof(MinidumpSimpleStringDictionary) + @@ -185,12 +190,14 @@ TEST(MinidumpSimpleStringDictionaryWriter, DuplicateKeyValue) { char kValue1[] = "value"; MinidumpSimpleStringDictionaryWriter dictionary_writer; - MinidumpSimpleStringDictionaryEntryWriter entry_writer_0; - entry_writer_0.SetKeyValue(kKey, kValue0); - dictionary_writer.AddEntry(&entry_writer_0); - MinidumpSimpleStringDictionaryEntryWriter entry_writer_1; - entry_writer_1.SetKeyValue(kKey, kValue1); - dictionary_writer.AddEntry(&entry_writer_1); + auto entry_writer_0 = + make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); + entry_writer_0->SetKeyValue(kKey, kValue0); + dictionary_writer.AddEntry(entry_writer_0.Pass()); + auto entry_writer_1 = + make_scoped_ptr(new MinidumpSimpleStringDictionaryEntryWriter()); + entry_writer_1->SetKeyValue(kKey, kValue1); + dictionary_writer.AddEntry(entry_writer_1.Pass()); EXPECT_TRUE(dictionary_writer.WriteEverything(&file_writer)); ASSERT_EQ(sizeof(MinidumpSimpleStringDictionary) + diff --git a/minidump/minidump_stream_writer.cc b/minidump/minidump_stream_writer.cc index f036706..8e5dcb3 100644 --- a/minidump/minidump_stream_writer.cc +++ b/minidump/minidump_stream_writer.cc @@ -19,6 +19,15 @@ namespace crashpad { namespace internal { +MinidumpStreamWriter::~MinidumpStreamWriter() { +} + +const MINIDUMP_DIRECTORY* MinidumpStreamWriter::DirectoryListEntry() const { + DCHECK_EQ(state(), kStateWritable); + + return &directory_list_entry_; +} + MinidumpStreamWriter::MinidumpStreamWriter() : MinidumpWritable(), directory_list_entry_() { } @@ -36,11 +45,5 @@ bool MinidumpStreamWriter::Freeze() { return true; } -const MINIDUMP_DIRECTORY* MinidumpStreamWriter::DirectoryListEntry() const { - DCHECK_EQ(state(), kStateWritable); - - return &directory_list_entry_; -} - } // namespace internal } // namespace crashpad diff --git a/minidump/minidump_stream_writer.h b/minidump/minidump_stream_writer.h index 4fb5b89..b327005 100644 --- a/minidump/minidump_stream_writer.h +++ b/minidump/minidump_stream_writer.h @@ -31,6 +31,8 @@ namespace internal { //! MinidumpFileWriter object. class MinidumpStreamWriter : public MinidumpWritable { public: + ~MinidumpStreamWriter() override; + //! \brief Returns an object’s stream type. //! //! \note Valid in any state. @@ -47,7 +49,6 @@ class MinidumpStreamWriter : public MinidumpWritable { protected: MinidumpStreamWriter(); - ~MinidumpStreamWriter() {} // MinidumpWritable: bool Freeze() override; diff --git a/minidump/minidump_string_writer.cc b/minidump/minidump_string_writer.cc index f3623d8..8bcb702 100644 --- a/minidump/minidump_string_writer.cc +++ b/minidump/minidump_string_writer.cc @@ -83,11 +83,17 @@ bool MinidumpStringWriter::WriteObject( template class MinidumpStringWriter; template class MinidumpStringWriter; +MinidumpUTF16StringWriter::~MinidumpUTF16StringWriter() { +} + void MinidumpUTF16StringWriter::SetUTF8(const std::string& string_utf8) { DCHECK_EQ(state(), kStateMutable); set_string(MinidumpWriterUtil::ConvertUTF8ToUTF16(string_utf8)); } +MinidumpUTF8StringWriter::~MinidumpUTF8StringWriter() { +} + } // namespace internal } // namespace crashpad diff --git a/minidump/minidump_string_writer.h b/minidump/minidump_string_writer.h index 9c0e609..bde25b5 100644 --- a/minidump/minidump_string_writer.h +++ b/minidump/minidump_string_writer.h @@ -51,7 +51,7 @@ template class MinidumpStringWriter : public MinidumpWritable { public: MinidumpStringWriter(); - ~MinidumpStringWriter(); + ~MinidumpStringWriter() override; protected: typedef typename Traits::MinidumpStringType MinidumpStringType; @@ -87,7 +87,7 @@ class MinidumpUTF16StringWriter final : public MinidumpStringWriter { public: MinidumpUTF16StringWriter() : MinidumpStringWriter() {} - ~MinidumpUTF16StringWriter() {} + ~MinidumpUTF16StringWriter() override; //! \brief Converts a UTF-8 string to UTF-16 and sets it as the string to be //! written. @@ -108,7 +108,7 @@ class MinidumpUTF8StringWriter final : public MinidumpStringWriter { public: MinidumpUTF8StringWriter() : MinidumpStringWriter() {} - ~MinidumpUTF8StringWriter() {} + ~MinidumpUTF8StringWriter() override; //! \brief Sets the string to be written. //! diff --git a/minidump/minidump_system_info_writer.h b/minidump/minidump_system_info_writer.h index 1bc0392..6cd133f 100644 --- a/minidump/minidump_system_info_writer.h +++ b/minidump/minidump_system_info_writer.h @@ -37,7 +37,7 @@ class MinidumpUTF16StringWriter; class MinidumpSystemInfoWriter final : public internal::MinidumpStreamWriter { public: MinidumpSystemInfoWriter(); - ~MinidumpSystemInfoWriter(); + ~MinidumpSystemInfoWriter() override; //! \brief Sets MINIDUMP_SYSTEM_INFO::ProcessorArchitecture. void SetCPUArchitecture(MinidumpCPUArchitecture processor_architecture) { diff --git a/minidump/minidump_system_info_writer_test.cc b/minidump/minidump_system_info_writer_test.cc index 3ea0f63..e6f0030 100644 --- a/minidump/minidump_system_info_writer_test.cc +++ b/minidump/minidump_system_info_writer_test.cc @@ -73,11 +73,11 @@ void GetSystemInfoStream(const std::string& file_contents, TEST(MinidumpSystemInfoWriter, Empty) { MinidumpFileWriter minidump_file_writer; - MinidumpSystemInfoWriter system_info_writer; + auto system_info_writer = make_scoped_ptr(new MinidumpSystemInfoWriter()); - system_info_writer.SetCSDVersion(std::string()); + system_info_writer->SetCSDVersion(std::string()); - minidump_file_writer.AddStream(&system_info_writer); + minidump_file_writer.AddStream(system_info_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -113,7 +113,7 @@ TEST(MinidumpSystemInfoWriter, Empty) { TEST(MinidumpSystemInfoWriter, X86_Win) { MinidumpFileWriter minidump_file_writer; - MinidumpSystemInfoWriter system_info_writer; + auto system_info_writer = make_scoped_ptr(new MinidumpSystemInfoWriter()); const MinidumpCPUArchitecture kCPUArchitecture = kMinidumpCPUArchitectureX86; const uint16_t kCPULevel = 0x0010; @@ -135,20 +135,20 @@ TEST(MinidumpSystemInfoWriter, X86_Win) { ASSERT_EQ(sizeof(cpu_vendor_registers), strlen(kCPUVendor)); memcpy(cpu_vendor_registers, kCPUVendor, sizeof(cpu_vendor_registers)); - system_info_writer.SetCPUArchitecture(kCPUArchitecture); - system_info_writer.SetCPULevelAndRevision(kCPULevel, kCPURevision); - system_info_writer.SetCPUCount(kCPUCount); - system_info_writer.SetOS(kOS); - system_info_writer.SetOSType(kMinidumpOSTypeWorkstation); - system_info_writer.SetOSVersion( + system_info_writer->SetCPUArchitecture(kCPUArchitecture); + system_info_writer->SetCPULevelAndRevision(kCPULevel, kCPURevision); + system_info_writer->SetCPUCount(kCPUCount); + system_info_writer->SetOS(kOS); + system_info_writer->SetOSType(kMinidumpOSTypeWorkstation); + system_info_writer->SetOSVersion( kOSVersionMajor, kOSVersionMinor, kOSVersionBuild); - system_info_writer.SetCSDVersion(kCSDVersion); - system_info_writer.SetSuiteMask(kSuiteMask); - system_info_writer.SetCPUX86VendorString(kCPUVendor); - system_info_writer.SetCPUX86VersionAndFeatures(kCPUVersion, kCPUFeatures); - system_info_writer.SetCPUX86AMDExtendedFeatures(kAMDFeatures); + system_info_writer->SetCSDVersion(kCSDVersion); + system_info_writer->SetSuiteMask(kSuiteMask); + system_info_writer->SetCPUX86VendorString(kCPUVendor); + system_info_writer->SetCPUX86VersionAndFeatures(kCPUVersion, kCPUFeatures); + system_info_writer->SetCPUX86AMDExtendedFeatures(kAMDFeatures); - minidump_file_writer.AddStream(&system_info_writer); + minidump_file_writer.AddStream(system_info_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -183,7 +183,7 @@ TEST(MinidumpSystemInfoWriter, X86_Win) { TEST(MinidumpSystemInfoWriter, X86_64_Mac) { MinidumpFileWriter minidump_file_writer; - MinidumpSystemInfoWriter system_info_writer; + auto system_info_writer = make_scoped_ptr(new MinidumpSystemInfoWriter()); const MinidumpCPUArchitecture kCPUArchitecture = kMinidumpCPUArchitectureAMD64; @@ -198,17 +198,17 @@ TEST(MinidumpSystemInfoWriter, X86_64_Mac) { const char kCSDVersion[] = "13E28"; const uint64_t kCPUFeatures[2] = {0x10427f4c, 0x00000000}; - system_info_writer.SetCPUArchitecture(kCPUArchitecture); - system_info_writer.SetCPULevelAndRevision(kCPULevel, kCPURevision); - system_info_writer.SetCPUCount(kCPUCount); - system_info_writer.SetOS(kOS); - system_info_writer.SetOSType(kMinidumpOSTypeWorkstation); - system_info_writer.SetOSVersion( + system_info_writer->SetCPUArchitecture(kCPUArchitecture); + system_info_writer->SetCPULevelAndRevision(kCPULevel, kCPURevision); + system_info_writer->SetCPUCount(kCPUCount); + system_info_writer->SetOS(kOS); + system_info_writer->SetOSType(kMinidumpOSTypeWorkstation); + system_info_writer->SetOSVersion( kOSVersionMajor, kOSVersionMinor, kOSVersionBuild); - system_info_writer.SetCSDVersion(kCSDVersion); - system_info_writer.SetCPUOtherFeatures(kCPUFeatures[0], kCPUFeatures[1]); + system_info_writer->SetCSDVersion(kCSDVersion); + system_info_writer->SetCPUOtherFeatures(kCPUFeatures[0], kCPUFeatures[1]); - minidump_file_writer.AddStream(&system_info_writer); + minidump_file_writer.AddStream(system_info_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -240,17 +240,17 @@ TEST(MinidumpSystemInfoWriter, X86_CPUVendorFromRegisters) { // This test exercises SetCPUX86Vendor() to set the vendor from register // values. MinidumpFileWriter minidump_file_writer; - MinidumpSystemInfoWriter system_info_writer; + auto system_info_writer = make_scoped_ptr(new MinidumpSystemInfoWriter()); const MinidumpCPUArchitecture kCPUArchitecture = kMinidumpCPUArchitectureX86; const uint32_t kCPUVendor[] = {'uneG', 'Ieni', 'letn'}; - system_info_writer.SetCPUArchitecture(kCPUArchitecture); - system_info_writer.SetCPUX86Vendor( + system_info_writer->SetCPUArchitecture(kCPUArchitecture); + system_info_writer->SetCPUX86Vendor( kCPUVendor[0], kCPUVendor[1], kCPUVendor[2]); - system_info_writer.SetCSDVersion(std::string()); + system_info_writer->SetCSDVersion(std::string()); - minidump_file_writer.AddStream(&system_info_writer); + minidump_file_writer.AddStream(system_info_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -271,8 +271,8 @@ TEST(MinidumpSystemInfoWriter, X86_CPUVendorFromRegisters) { TEST(MinidumpSystemInfoWriterDeathTest, NoCSDVersion) { MinidumpFileWriter minidump_file_writer; - MinidumpSystemInfoWriter system_info_writer; - minidump_file_writer.AddStream(&system_info_writer); + auto system_info_writer = make_scoped_ptr(new MinidumpSystemInfoWriter()); + minidump_file_writer.AddStream(system_info_writer.Pass()); StringFileWriter file_writer; ASSERT_DEATH(minidump_file_writer.WriteEverything(&file_writer), diff --git a/minidump/minidump_thread_writer.cc b/minidump/minidump_thread_writer.cc index 1dc3bc2..efca1bd 100644 --- a/minidump/minidump_thread_writer.cc +++ b/minidump/minidump_thread_writer.cc @@ -28,22 +28,26 @@ MinidumpThreadWriter::MinidumpThreadWriter() : MinidumpWritable(), thread_(), stack_(nullptr), context_(nullptr) { } +MinidumpThreadWriter::~MinidumpThreadWriter() { +} + const MINIDUMP_THREAD* MinidumpThreadWriter::MinidumpThread() const { DCHECK_EQ(state(), kStateWritable); return &thread_; } -void MinidumpThreadWriter::SetStack(MinidumpMemoryWriter* stack) { +void MinidumpThreadWriter::SetStack(scoped_ptr stack) { DCHECK_EQ(state(), kStateMutable); - stack_ = stack; + stack_ = stack.Pass(); } -void MinidumpThreadWriter::SetContext(MinidumpContextWriter* context) { +void MinidumpThreadWriter::SetContext( + scoped_ptr context) { DCHECK_EQ(state(), kStateMutable); - context_ = context; + context_ = context.Pass(); } bool MinidumpThreadWriter::Freeze() { @@ -78,9 +82,9 @@ std::vector MinidumpThreadWriter::Children() { std::vector children; if (stack_) { - children.push_back(stack_); + children.push_back(stack_.get()); } - children.push_back(context_); + children.push_back(context_.get()); return children; } @@ -112,17 +116,18 @@ void MinidumpThreadListWriter::SetMemoryListWriter( memory_list_writer_ = memory_list_writer; } -void MinidumpThreadListWriter::AddThread(MinidumpThreadWriter* thread) { +void MinidumpThreadListWriter::AddThread( + scoped_ptr thread) { DCHECK_EQ(state(), kStateMutable); - threads_.push_back(thread); - if (memory_list_writer_) { MinidumpMemoryWriter* stack = thread->Stack(); if (stack) { memory_list_writer_->AddExtraMemory(stack); } } + + threads_.push_back(thread.release()); } bool MinidumpThreadListWriter::Freeze() { diff --git a/minidump/minidump_thread_writer.h b/minidump/minidump_thread_writer.h index 8fce7cb..5ce0481 100644 --- a/minidump/minidump_thread_writer.h +++ b/minidump/minidump_thread_writer.h @@ -21,8 +21,10 @@ #include #include "base/basictypes.h" +#include "base/memory/scoped_ptr.h" #include "minidump/minidump_stream_writer.h" #include "minidump/minidump_writable.h" +#include "util/stdlib/pointer_container.h" namespace crashpad { @@ -39,7 +41,7 @@ class MinidumpMemoryWriter; class MinidumpThreadWriter final : public internal::MinidumpWritable { public: MinidumpThreadWriter(); - ~MinidumpThreadWriter() {} + ~MinidumpThreadWriter() override; //! \brief Returns a MINIDUMP_THREAD referencing this object’s data. //! @@ -53,7 +55,7 @@ class MinidumpThreadWriter final : public internal::MinidumpWritable { //! corresponding to this object’s stack. //! //! If the thread does not have a stack, or its stack could not be determined, - //! this will return nullptr. + //! this will return `nullptr`. //! //! This method is provided so that MinidumpThreadListWriter can obtain thread //! stack memory regions for the purposes of adding them to a @@ -62,27 +64,27 @@ class MinidumpThreadWriter final : public internal::MinidumpWritable { //! MinidumpMemoryListWriter::AddExtraMemory(). //! //! \note Valid in any state. - MinidumpMemoryWriter* Stack() const { return stack_; } + MinidumpMemoryWriter* Stack() const { return stack_.get(); } //! \brief Arranges for MINIDUMP_THREAD::Stack to point to the MINIDUMP_MEMORY //! object to be written by \a stack. //! - //! \a stack will become a child of this object in the overall tree of - //! internal::MinidumpWritable objects. + //! This object takes ownership of \a stack and becomes its parent in the + //! overall tree of internal::MinidumpWritable objects. //! //! \note Valid in #kStateMutable. - void SetStack(MinidumpMemoryWriter* stack); + void SetStack(scoped_ptr stack); //! \brief Arranges for MINIDUMP_THREAD::ThreadContext to point to the CPU //! context to be written by \a context. //! //! A context is required in all MINIDUMP_THREAD objects. //! - //! \a context will become a child of this object in the overall tree of - //! internal::MinidumpWritable objects. + //! This object takes ownership of \a context and becomes its parent in the + //! overall tree of internal::MinidumpWritable objects. //! //! \note Valid in #kStateMutable. - void SetContext(MinidumpContextWriter* context); + void SetContext(scoped_ptr context); //! \brief Sets MINIDUMP_THREAD::ThreadId. void SetThreadID(uint32_t thread_id) { thread_.ThreadId = thread_id; } @@ -112,8 +114,8 @@ class MinidumpThreadWriter final : public internal::MinidumpWritable { private: MINIDUMP_THREAD thread_; - MinidumpMemoryWriter* stack_; // weak - MinidumpContextWriter* context_; // weak + scoped_ptr stack_; + scoped_ptr context_; DISALLOW_COPY_AND_ASSIGN(MinidumpThreadWriter); }; @@ -123,7 +125,7 @@ class MinidumpThreadWriter final : public internal::MinidumpWritable { class MinidumpThreadListWriter final : public internal::MinidumpStreamWriter { public: MinidumpThreadListWriter(); - ~MinidumpThreadListWriter(); + ~MinidumpThreadListWriter() override; //! \brief Sets the MinidumpMemoryListWriter that each thread’s stack memory //! region should be added to as extra memory. @@ -154,11 +156,11 @@ class MinidumpThreadListWriter final : public internal::MinidumpStreamWriter { //! \brief Adds a MinidumpThreadWriter to the MINIDUMP_THREAD_LIST. //! - //! \a thread will become a child of this object in the overall tree of - //! internal::MinidumpWritable objects. + //! This object takes ownership of \a thread and becomes its parent in the + //! overall tree of internal::MinidumpWritable objects. //! //! \note Valid in #kStateMutable. - void AddThread(MinidumpThreadWriter* thread); + void AddThread(scoped_ptr thread); protected: // MinidumpWritable: @@ -172,7 +174,7 @@ class MinidumpThreadListWriter final : public internal::MinidumpStreamWriter { private: MINIDUMP_THREAD_LIST thread_list_base_; - std::vector threads_; // weak + PointerVector threads_; MinidumpMemoryListWriter* memory_list_writer_; // weak DISALLOW_COPY_AND_ASSIGN(MinidumpThreadListWriter); diff --git a/minidump/minidump_thread_writer_test.cc b/minidump/minidump_thread_writer_test.cc index db9d9c5..0e2458d 100644 --- a/minidump/minidump_thread_writer_test.cc +++ b/minidump/minidump_thread_writer_test.cc @@ -70,9 +70,9 @@ void GetThreadListStream(const std::string& file_contents, TEST(MinidumpThreadWriter, EmptyThreadList) { MinidumpFileWriter minidump_file_writer; - MinidumpThreadListWriter thread_list_writer; + auto thread_list_writer = make_scoped_ptr(new MinidumpThreadListWriter()); - minidump_file_writer.AddStream(&thread_list_writer); + minidump_file_writer.AddStream(thread_list_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -131,7 +131,7 @@ void ExpectThread(const MINIDUMP_THREAD* expected, TEST(MinidumpThreadWriter, OneThread_x86_NoStack) { MinidumpFileWriter minidump_file_writer; - MinidumpThreadListWriter thread_list_writer; + auto thread_list_writer = make_scoped_ptr(new MinidumpThreadListWriter()); const uint32_t kThreadID = 0x11111111; const uint32_t kSuspendCount = 1; @@ -140,19 +140,19 @@ TEST(MinidumpThreadWriter, OneThread_x86_NoStack) { const uint64_t kTEB = 0x55555555; const uint32_t kSeed = 123; - MinidumpThreadWriter thread_writer; - thread_writer.SetThreadID(kThreadID); - thread_writer.SetSuspendCount(kSuspendCount); - thread_writer.SetPriorityClass(kPriorityClass); - thread_writer.SetPriority(kPriority); - thread_writer.SetTEB(kTEB); + auto thread_writer = make_scoped_ptr(new MinidumpThreadWriter()); + thread_writer->SetThreadID(kThreadID); + thread_writer->SetSuspendCount(kSuspendCount); + thread_writer->SetPriorityClass(kPriorityClass); + thread_writer->SetPriority(kPriority); + thread_writer->SetTEB(kTEB); - MinidumpContextX86Writer context_x86_writer; - InitializeMinidumpContextX86(context_x86_writer.context(), kSeed); - thread_writer.SetContext(&context_x86_writer); + auto context_x86_writer = make_scoped_ptr(new MinidumpContextX86Writer()); + InitializeMinidumpContextX86(context_x86_writer->context(), kSeed); + thread_writer->SetContext(context_x86_writer.Pass()); - thread_list_writer.AddThread(&thread_writer); - minidump_file_writer.AddStream(&thread_list_writer); + thread_list_writer->AddThread(thread_writer.Pass()); + minidump_file_writer.AddStream(thread_list_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -189,7 +189,7 @@ TEST(MinidumpThreadWriter, OneThread_x86_NoStack) { TEST(MinidumpThreadWriter, OneThread_AMD64_Stack) { MinidumpFileWriter minidump_file_writer; - MinidumpThreadListWriter thread_list_writer; + auto thread_list_writer = make_scoped_ptr(new MinidumpThreadListWriter()); const uint32_t kThreadID = 0x22222222; const uint32_t kSuspendCount = 2; @@ -201,23 +201,23 @@ TEST(MinidumpThreadWriter, OneThread_AMD64_Stack) { const uint8_t kMemoryValue = 99; const uint32_t kSeed = 456; - MinidumpThreadWriter thread_writer; - thread_writer.SetThreadID(kThreadID); - thread_writer.SetSuspendCount(kSuspendCount); - thread_writer.SetPriorityClass(kPriorityClass); - thread_writer.SetPriority(kPriority); - thread_writer.SetTEB(kTEB); + auto thread_writer = make_scoped_ptr(new MinidumpThreadWriter()); + thread_writer->SetThreadID(kThreadID); + thread_writer->SetSuspendCount(kSuspendCount); + thread_writer->SetPriorityClass(kPriorityClass); + thread_writer->SetPriority(kPriority); + thread_writer->SetTEB(kTEB); - TestMinidumpMemoryWriter memory_writer( - kMemoryBase, kMemorySize, kMemoryValue); - thread_writer.SetStack(&memory_writer); + auto memory_writer = make_scoped_ptr( + new TestMinidumpMemoryWriter(kMemoryBase, kMemorySize, kMemoryValue)); + thread_writer->SetStack(memory_writer.Pass()); - MinidumpContextAMD64Writer context_amd64_writer; - InitializeMinidumpContextAMD64(context_amd64_writer.context(), kSeed); - thread_writer.SetContext(&context_amd64_writer); + auto context_amd64_writer = make_scoped_ptr(new MinidumpContextAMD64Writer()); + InitializeMinidumpContextAMD64(context_amd64_writer->context(), kSeed); + thread_writer->SetContext(context_amd64_writer.Pass()); - thread_list_writer.AddThread(&thread_writer); - minidump_file_writer.AddStream(&thread_list_writer); + thread_list_writer->AddThread(thread_writer.Pass()); + minidump_file_writer.AddStream(thread_list_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -263,9 +263,9 @@ TEST(MinidumpThreadWriter, OneThread_AMD64_Stack) { TEST(MinidumpThreadWriter, ThreeThreads_x86_MemoryList) { MinidumpFileWriter minidump_file_writer; - MinidumpThreadListWriter thread_list_writer; - MinidumpMemoryListWriter memory_list_writer; - thread_list_writer.SetMemoryListWriter(&memory_list_writer); + auto thread_list_writer = make_scoped_ptr(new MinidumpThreadListWriter()); + auto memory_list_writer = make_scoped_ptr(new MinidumpMemoryListWriter()); + thread_list_writer->SetMemoryListWriter(memory_list_writer.get()); const uint32_t kThreadID0 = 1111111; const uint32_t kSuspendCount0 = 111111; @@ -277,22 +277,22 @@ TEST(MinidumpThreadWriter, ThreeThreads_x86_MemoryList) { const uint8_t kMemoryValue0 = 11; const uint32_t kSeed0 = 1; - MinidumpThreadWriter thread_writer_0; - thread_writer_0.SetThreadID(kThreadID0); - thread_writer_0.SetSuspendCount(kSuspendCount0); - thread_writer_0.SetPriorityClass(kPriorityClass0); - thread_writer_0.SetPriority(kPriority0); - thread_writer_0.SetTEB(kTEB0); + auto thread_writer_0 = make_scoped_ptr(new MinidumpThreadWriter()); + thread_writer_0->SetThreadID(kThreadID0); + thread_writer_0->SetSuspendCount(kSuspendCount0); + thread_writer_0->SetPriorityClass(kPriorityClass0); + thread_writer_0->SetPriority(kPriority0); + thread_writer_0->SetTEB(kTEB0); - TestMinidumpMemoryWriter memory_writer_0( - kMemoryBase0, kMemorySize0, kMemoryValue0); - thread_writer_0.SetStack(&memory_writer_0); + auto memory_writer_0 = make_scoped_ptr( + new TestMinidumpMemoryWriter(kMemoryBase0, kMemorySize0, kMemoryValue0)); + thread_writer_0->SetStack(memory_writer_0.Pass()); - MinidumpContextX86Writer context_x86_writer_0; - InitializeMinidumpContextX86(context_x86_writer_0.context(), kSeed0); - thread_writer_0.SetContext(&context_x86_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_list_writer.AddThread(&thread_writer_0); + thread_list_writer->AddThread(thread_writer_0.Pass()); const uint32_t kThreadID1 = 2222222; const uint32_t kSuspendCount1 = 222222; @@ -304,22 +304,22 @@ TEST(MinidumpThreadWriter, ThreeThreads_x86_MemoryList) { const uint8_t kMemoryValue1 = 22; const uint32_t kSeed1 = 2; - MinidumpThreadWriter thread_writer_1; - thread_writer_1.SetThreadID(kThreadID1); - thread_writer_1.SetSuspendCount(kSuspendCount1); - thread_writer_1.SetPriorityClass(kPriorityClass1); - thread_writer_1.SetPriority(kPriority1); - thread_writer_1.SetTEB(kTEB1); + auto thread_writer_1 = make_scoped_ptr(new MinidumpThreadWriter()); + thread_writer_1->SetThreadID(kThreadID1); + thread_writer_1->SetSuspendCount(kSuspendCount1); + thread_writer_1->SetPriorityClass(kPriorityClass1); + thread_writer_1->SetPriority(kPriority1); + thread_writer_1->SetTEB(kTEB1); - TestMinidumpMemoryWriter memory_writer_1( - kMemoryBase1, kMemorySize1, kMemoryValue1); - thread_writer_1.SetStack(&memory_writer_1); + auto memory_writer_1 = make_scoped_ptr( + new TestMinidumpMemoryWriter(kMemoryBase1, kMemorySize1, kMemoryValue1)); + thread_writer_1->SetStack(memory_writer_1.Pass()); - MinidumpContextX86Writer context_x86_writer_1; - InitializeMinidumpContextX86(context_x86_writer_1.context(), kSeed1); - thread_writer_1.SetContext(&context_x86_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_list_writer.AddThread(&thread_writer_1); + thread_list_writer->AddThread(thread_writer_1.Pass()); const uint32_t kThreadID2 = 3333333; const uint32_t kSuspendCount2 = 333333; @@ -331,25 +331,25 @@ TEST(MinidumpThreadWriter, ThreeThreads_x86_MemoryList) { const uint8_t kMemoryValue2 = 33; const uint32_t kSeed2 = 3; - MinidumpThreadWriter thread_writer_2; - thread_writer_2.SetThreadID(kThreadID2); - thread_writer_2.SetSuspendCount(kSuspendCount2); - thread_writer_2.SetPriorityClass(kPriorityClass2); - thread_writer_2.SetPriority(kPriority2); - thread_writer_2.SetTEB(kTEB2); + auto thread_writer_2 = make_scoped_ptr(new MinidumpThreadWriter()); + thread_writer_2->SetThreadID(kThreadID2); + thread_writer_2->SetSuspendCount(kSuspendCount2); + thread_writer_2->SetPriorityClass(kPriorityClass2); + thread_writer_2->SetPriority(kPriority2); + thread_writer_2->SetTEB(kTEB2); - TestMinidumpMemoryWriter memory_writer_2( - kMemoryBase2, kMemorySize2, kMemoryValue2); - thread_writer_2.SetStack(&memory_writer_2); + auto memory_writer_2 = make_scoped_ptr( + new TestMinidumpMemoryWriter(kMemoryBase2, kMemorySize2, kMemoryValue2)); + thread_writer_2->SetStack(memory_writer_2.Pass()); - MinidumpContextX86Writer context_x86_writer_2; - InitializeMinidumpContextX86(context_x86_writer_2.context(), kSeed2); - thread_writer_2.SetContext(&context_x86_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_list_writer.AddThread(&thread_writer_2); + thread_list_writer->AddThread(thread_writer_2.Pass()); - minidump_file_writer.AddStream(&thread_list_writer); - minidump_file_writer.AddStream(&memory_list_writer); + minidump_file_writer.AddStream(thread_list_writer.Pass()); + minidump_file_writer.AddStream(memory_list_writer.Pass()); StringFileWriter file_writer; ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); @@ -472,12 +472,12 @@ TEST(MinidumpThreadWriter, ThreeThreads_x86_MemoryList) { TEST(MinidumpThreadWriterDeathTest, NoContext) { MinidumpFileWriter minidump_file_writer; - MinidumpThreadListWriter thread_list_writer; + auto thread_list_writer = make_scoped_ptr(new MinidumpThreadListWriter()); - MinidumpThreadWriter thread_writer; + auto thread_writer = make_scoped_ptr(new MinidumpThreadWriter()); - thread_list_writer.AddThread(&thread_writer); - minidump_file_writer.AddStream(&thread_list_writer); + thread_list_writer->AddThread(thread_writer.Pass()); + minidump_file_writer.AddStream(thread_list_writer.Pass()); StringFileWriter file_writer; ASSERT_DEATH(minidump_file_writer.WriteEverything(&file_writer), "context_"); diff --git a/minidump/minidump_writable.h b/minidump/minidump_writable.h index ccace8c..cfababf 100644 --- a/minidump/minidump_writable.h +++ b/minidump/minidump_writable.h @@ -133,7 +133,13 @@ class MinidumpWritable { static const size_t kInvalidSize; MinidumpWritable(); - ~MinidumpWritable(); + + // This doesn’t really need to be virtual because nothing ever deletes a + // MinidumpWritable* through an interface pointer with that type, and this is + // guaranteed by being protected. Regardless, the style guide is somewhat + // insistent. + // http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Inheritance + virtual ~MinidumpWritable(); //! \brief The state of the object. State state() const { return state_; }