diff --git a/minidump/minidump.gyp b/minidump/minidump.gyp index ceaf951f..7dfcd51d 100644 --- a/minidump/minidump.gyp +++ b/minidump/minidump.gyp @@ -41,6 +41,8 @@ 'minidump_extensions.h', 'minidump_file_writer.cc', 'minidump_file_writer.h', + 'minidump_location_descriptor_list_writer.cc', + 'minidump_location_descriptor_list_writer.h', 'minidump_memory_writer.cc', 'minidump_memory_writer.h', 'minidump_misc_info_writer.cc', @@ -85,6 +87,7 @@ 'minidump_crashpad_info_writer_test.cc', 'minidump_exception_writer_test.cc', 'minidump_file_writer_test.cc', + 'minidump_location_descriptor_list_writer_test.cc', 'minidump_memory_writer_test.cc', 'minidump_misc_info_writer_test.cc', 'minidump_module_crashpad_info_writer_test.cc', @@ -99,6 +102,8 @@ 'test/minidump_context_test_util.h', 'test/minidump_file_writer_test_util.cc', 'test/minidump_file_writer_test_util.h', + 'test/minidump_location_descriptor_list_test_util.cc', + 'test/minidump_location_descriptor_list_test_util.h', 'test/minidump_memory_writer_test_util.cc', 'test/minidump_memory_writer_test_util.h', 'test/minidump_string_writer_test_util.cc', diff --git a/minidump/minidump_crashpad_info_writer_test.cc b/minidump/minidump_crashpad_info_writer_test.cc index 3fcb2e06..2840d348 100644 --- a/minidump/minidump_crashpad_info_writer_test.cc +++ b/minidump/minidump_crashpad_info_writer_test.cc @@ -97,7 +97,7 @@ TEST(MinidumpCrashpadInfoWriter, CrashpadModuleList) { const MinidumpModuleCrashpadInfo* module = MinidumpWritableAtLocationDescriptor( - file_writer.string(), module_list->modules[0]); + file_writer.string(), module_list->children[0]); ASSERT_TRUE(module); EXPECT_EQ(MinidumpModuleCrashpadInfo::kVersion, module->version); diff --git a/minidump/minidump_extensions.h b/minidump/minidump_extensions.h index 7386a76f..f88b0cde 100644 --- a/minidump/minidump_extensions.h +++ b/minidump/minidump_extensions.h @@ -296,6 +296,15 @@ struct MinidumpModuleCodeViewRecordPDB70 { uint8_t pdb_name[1]; }; +//! \brief A list of MINIDUMP_LOCATION_DESCRIPTOR objects. +struct __attribute__((packed, aligned(4))) MinidumpLocationDescriptorList { + //! \brief The number of children present in the #children array. + uint32_t count; + + //! \brief Pointers to other structures in the minidump file. + MINIDUMP_LOCATION_DESCRIPTOR children[0]; +}; + //! \brief A key-value pair. struct __attribute__((packed, aligned(4))) MinidumpSimpleStringDictionaryEntry { //! \brief ::RVA of a MinidumpUTF8String containing the key of a key-value @@ -330,7 +339,7 @@ struct __attribute__((packed, aligned(4))) MinidumpSimpleStringDictionary { //! #version, so that newer parsers will be able to determine whether the added //! fields are valid or not. //! -//! \sa MinidumpModuleCrashpadInfoList +//! \sa #MinidumpModuleCrashpadInfoList struct __attribute__((packed, aligned(4))) MinidumpModuleCrashpadInfo { //! \brief The structure’s currently-defined version number. //! @@ -369,22 +378,18 @@ struct __attribute__((packed, aligned(4))) MinidumpModuleCrashpadInfo { //! This structure augments the information provided by //! MINIDUMP_MODULE_LIST. The minidump file must contain a module list stream //! (::kMinidumpStreamTypeModuleList) in order for this structure to appear. -struct __attribute__((packed, aligned(4))) MinidumpModuleCrashpadInfoList { - //! \brief The number of modules present in the #modules array. - //! - //! This may be less than the value of MINIDUMP_MODULE_LIST::NumberOfModules - //! because not every MINIDUMP_MODULE structure carried within the minidump - //! file will necessarily have Crashpad-specific information provided by a - //! MinidumpModuleCrashpadInfo structure. - uint32_t count; - - //! \brief Pointers to MinidumpModuleCrashpadInfo structures. - //! - //! These are referenced indirectly through MINIDUMP_LOCATION_DESCRIPTOR - //! pointers to allow for future growth of the MinidumpModuleCrashpadInfo - //! structure. - MINIDUMP_LOCATION_DESCRIPTOR modules[0]; -}; +//! +//! MinidumpModuleCrashpadInfoList::count may be less than the value of +//! MINIDUMP_MODULE_LIST::NumberOfModules because not every MINIDUMP_MODULE +//! structure carried within the minidump file will necessarily have +//! Crashpad-specific information provided by a MinidumpModuleCrashpadInfo +//! structure. +//! +//! MinidumpModuleCrashpadInfoList::children references +//! MinidumpModuleCrashpadInfo children indirectly through +//! MINIDUMP_LOCATION_DESCRIPTOR pointers to allow for future growth of the +//! MinidumpModuleCrashpadInfo structure. +using MinidumpModuleCrashpadInfoList = MinidumpLocationDescriptorList; //! \brief Additional Crashpad-specific information carried within a minidump //! file. @@ -412,7 +417,7 @@ struct __attribute__((packed, aligned(4))) MinidumpCrashpadInfo { //! no need for any fields present in later versions. uint32_t version; - //! \brief A pointer to a MinidumpModuleCrashpadInfoList structure. + //! \brief A pointer to a #MinidumpModuleCrashpadInfoList structure. //! //! This field is present when #version is at least `1`. MINIDUMP_LOCATION_DESCRIPTOR module_list; diff --git a/minidump/minidump_location_descriptor_list_writer.cc b/minidump/minidump_location_descriptor_list_writer.cc new file mode 100644 index 00000000..b3f6d9e8 --- /dev/null +++ b/minidump/minidump_location_descriptor_list_writer.cc @@ -0,0 +1,102 @@ +// Copyright 2014 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "minidump/minidump_location_descriptor_list_writer.h" + +#include "base/logging.h" +#include "util/file/file_writer.h" +#include "util/numeric/safe_assignment.h" + +namespace crashpad { + +MinidumpLocationDescriptorListWriter::MinidumpLocationDescriptorListWriter() + : MinidumpWritable(), + location_descriptor_list_base_(), + children_(), + child_location_descriptors_() { +} + +MinidumpLocationDescriptorListWriter::~MinidumpLocationDescriptorListWriter() { +} + +void MinidumpLocationDescriptorListWriter::AddChild( + scoped_ptr child) { + DCHECK_EQ(state(), kStateMutable); + + children_.push_back(child.release()); +} + +bool MinidumpLocationDescriptorListWriter::Freeze() { + DCHECK_EQ(state(), kStateMutable); + DCHECK(child_location_descriptors_.empty()); + + if (!MinidumpWritable::Freeze()) { + return false; + } + + size_t child_count = children_.size(); + if (!AssignIfInRange(&location_descriptor_list_base_.count, child_count)) { + LOG(ERROR) << "child_count " << child_count << " out of range"; + return false; + } + + child_location_descriptors_.resize(child_count); + for (size_t index = 0; index < child_count; ++index) { + children_[index]->RegisterLocationDescriptor( + &child_location_descriptors_[index]); + } + + return true; +} + +size_t MinidumpLocationDescriptorListWriter::SizeOfObject() { + DCHECK_GE(state(), kStateFrozen); + + return sizeof(location_descriptor_list_base_) + + children_.size() * sizeof(MINIDUMP_LOCATION_DESCRIPTOR); +} + +std::vector +MinidumpLocationDescriptorListWriter::Children() { + DCHECK_GE(state(), kStateFrozen); + + std::vector children; + for (MinidumpWritable* child : children_) { + children.push_back(child); + } + + return children; +} + +bool MinidumpLocationDescriptorListWriter::WriteObject( + FileWriterInterface* file_writer) { + DCHECK_EQ(state(), kStateWritable); + DCHECK_EQ(children_.size(), child_location_descriptors_.size()); + + WritableIoVec iov; + iov.iov_base = &location_descriptor_list_base_; + iov.iov_len = sizeof(location_descriptor_list_base_); + std::vector iovecs(1, iov); + + if (!child_location_descriptors_.empty()) { + iov.iov_base = &child_location_descriptors_[0]; + iov.iov_len = child_location_descriptors_.size() * + sizeof(MINIDUMP_LOCATION_DESCRIPTOR); + iovecs.push_back(iov); + } + + return file_writer->WriteIoVec(&iovecs); +} + +} // namespace crashpad diff --git a/minidump/minidump_location_descriptor_list_writer.h b/minidump/minidump_location_descriptor_list_writer.h new file mode 100644 index 00000000..59bbb249 --- /dev/null +++ b/minidump/minidump_location_descriptor_list_writer.h @@ -0,0 +1,82 @@ +// Copyright 2014 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_MINIDUMP_LOCATION_DESCRIPTOR_LIST_WRITER_H_ +#define CRASHPAD_MINIDUMP_LOCATION_DESCRIPTOR_LIST_WRITER_H_ + +#include +#include + +#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 { + +//! \brief The writer for a MinidumpLocationDescriptorList object in a minidump +//! file, containing a list of MINIDUMP_LOCATION_DESCRIPTOR objects. +class MinidumpLocationDescriptorListWriter : public internal::MinidumpWritable { + protected: + MinidumpLocationDescriptorListWriter(); + ~MinidumpLocationDescriptorListWriter() override; + + //! \brief Adds a MINIDUMP_LOCATION_DESCRIPTOR referencing an + //! internal::MinidumpWritable to the MinidumpLocationDescriptorList. + //! + //! This object takes ownership of \a child and becomes its parent in the + //! overall tree of internal::MinidumpWritable objects. + //! + //! To provide type-correctness, subclasses are expected to provide a public + //! method that accepts a `scoped_ptr`-wrapped argument of the proper + //! internal::MinidumpWritable subclass, and call this method with that + //! argument. + //! + //! \note Valid in #kStateMutable. + void AddChild(scoped_ptr child); + + //! \brief Returns `true` if no child objects have been added by AddChild(), + //! and `false` if child objects are present. + bool IsEmpty() const { return children_.empty(); } + + //! \brief Returns an object’s MINIDUMP_LOCATION_DESCRIPTOR objects + //! referencing its children. + //! + //! \note The returned vector will be empty until the object advances to + //! #kStateFrozen or beyond. + const std::vector& child_location_descriptors() + const { + return child_location_descriptors_; + } + + // MinidumpWritable: + bool Freeze() override; + size_t SizeOfObject() override; + std::vector Children() override; + bool WriteObject(FileWriterInterface* file_writer) override; + + private: + MinidumpLocationDescriptorList location_descriptor_list_base_; + PointerVector children_; + std::vector child_location_descriptors_; + + DISALLOW_COPY_AND_ASSIGN(MinidumpLocationDescriptorListWriter); +}; + +} // namespace crashpad + +#endif // CRASHPAD_MINIDUMP_LOCATION_DESCRIPTOR_LIST_WRITER_H_ diff --git a/minidump/minidump_location_descriptor_list_writer_test.cc b/minidump/minidump_location_descriptor_list_writer_test.cc new file mode 100644 index 00000000..e193078b --- /dev/null +++ b/minidump/minidump_location_descriptor_list_writer_test.cc @@ -0,0 +1,133 @@ +// Copyright 2014 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "minidump/minidump_location_descriptor_list_writer.h" + +#include "base/strings/stringprintf.h" +#include "gtest/gtest.h" +#include "minidump/test/minidump_location_descriptor_list_test_util.h" +#include "minidump/test/minidump_writable_test_util.h" +#include "util/file/string_file_writer.h" + +namespace crashpad { +namespace test { +namespace { + +class TestMinidumpWritable final : public internal::MinidumpWritable { + public: + explicit TestMinidumpWritable(uint32_t value) + : MinidumpWritable(), + value_(value) { + } + + ~TestMinidumpWritable() override {} + + protected: + // MinidumpWritable: + + size_t SizeOfObject() override { return sizeof(value_); } + + bool WriteObject(FileWriterInterface* file_writer) override { + return file_writer->Write(&value_, sizeof(value_)); + } + + private: + uint32_t value_; + + DISALLOW_COPY_AND_ASSIGN(TestMinidumpWritable); +}; + +class TestMinidumpLocationDescriptorListWriter final + : public MinidumpLocationDescriptorListWriter { + public: + TestMinidumpLocationDescriptorListWriter() + : MinidumpLocationDescriptorListWriter() { + } + + ~TestMinidumpLocationDescriptorListWriter() override {} + + void AddChild(uint32_t value) { + auto child = make_scoped_ptr(new TestMinidumpWritable(value)); + MinidumpLocationDescriptorListWriter::AddChild(child.Pass()); + } + + private: + DISALLOW_COPY_AND_ASSIGN(TestMinidumpLocationDescriptorListWriter); +}; + +TEST(MinidumpLocationDescriptorListWriter, Empty) { + TestMinidumpLocationDescriptorListWriter list_writer; + + StringFileWriter file_writer; + + ASSERT_TRUE(list_writer.WriteEverything(&file_writer)); + EXPECT_EQ(sizeof(MinidumpLocationDescriptorList), + file_writer.string().size()); + + const MinidumpLocationDescriptorList* list = + MinidumpLocationDescriptorListAtStart(file_writer.string(), 0); + ASSERT_TRUE(list); +} + +TEST(MinidumpLocationDescriptorListWriter, OneChild) { + TestMinidumpLocationDescriptorListWriter list_writer; + + const uint32_t kValue = 0; + list_writer.AddChild(kValue); + + StringFileWriter file_writer; + + ASSERT_TRUE(list_writer.WriteEverything(&file_writer)); + + const MinidumpLocationDescriptorList* list = + MinidumpLocationDescriptorListAtStart(file_writer.string(), 1); + ASSERT_TRUE(list); + + const uint32_t* child = MinidumpWritableAtLocationDescriptor( + file_writer.string(), list->children[0]); + ASSERT_TRUE(child); + EXPECT_EQ(kValue, *child); +} + +TEST(MinidumpLocationDescriptorListWriter, ThreeChildren) { + TestMinidumpLocationDescriptorListWriter list_writer; + + const uint32_t kValues[] = { 0x80000000, 0x55555555, 0x66006600 }; + + list_writer.AddChild(kValues[0]); + list_writer.AddChild(kValues[1]); + list_writer.AddChild(kValues[2]); + + StringFileWriter file_writer; + + ASSERT_TRUE(list_writer.WriteEverything(&file_writer)); + + const MinidumpLocationDescriptorList* list = + MinidumpLocationDescriptorListAtStart(file_writer.string(), + arraysize(kValues)); + ASSERT_TRUE(list); + + for (size_t index = 0; index < arraysize(kValues); ++index) { + SCOPED_TRACE(base::StringPrintf("index %zu", index)); + + const uint32_t* child = MinidumpWritableAtLocationDescriptor( + file_writer.string(), list->children[index]); + ASSERT_TRUE(child); + EXPECT_EQ(kValues[index], *child); + } +} + +} // namespace +} // namespace test +} // namespace crashpad diff --git a/minidump/minidump_module_crashpad_info_writer.cc b/minidump/minidump_module_crashpad_info_writer.cc index e1a2a231..0c5cfc4e 100644 --- a/minidump/minidump_module_crashpad_info_writer.cc +++ b/minidump/minidump_module_crashpad_info_writer.cc @@ -105,10 +105,7 @@ bool MinidumpModuleCrashpadInfoWriter::WriteObject( } MinidumpModuleCrashpadInfoListWriter::MinidumpModuleCrashpadInfoListWriter() - : MinidumpWritable(), - module_list_base_(), - modules_(), - module_location_descriptors_() { + : MinidumpLocationDescriptorListWriter() { } MinidumpModuleCrashpadInfoListWriter::~MinidumpModuleCrashpadInfoListWriter() { @@ -117,8 +114,8 @@ MinidumpModuleCrashpadInfoListWriter::~MinidumpModuleCrashpadInfoListWriter() { void MinidumpModuleCrashpadInfoListWriter::InitializeFromSnapshot( const std::vector& module_snapshots) { DCHECK_EQ(state(), kStateMutable); - DCHECK(modules_.empty()); - DCHECK(module_location_descriptors_.empty()); + DCHECK(IsEmpty()); + DCHECK(child_location_descriptors().empty()); size_t count = module_snapshots.size(); for (size_t index = 0; index < count; ++index) { @@ -136,69 +133,7 @@ void MinidumpModuleCrashpadInfoListWriter::AddModule( scoped_ptr module) { DCHECK_EQ(state(), kStateMutable); - modules_.push_back(module.release()); -} - -bool MinidumpModuleCrashpadInfoListWriter::Freeze() { - DCHECK_EQ(state(), kStateMutable); - DCHECK(module_location_descriptors_.empty()); - - if (!MinidumpWritable::Freeze()) { - return false; - } - - size_t module_count = modules_.size(); - if (!AssignIfInRange(&module_list_base_.count, module_count)) { - LOG(ERROR) << "module_count " << module_count << " out of range"; - return false; - } - - module_location_descriptors_.resize(module_count); - for (size_t index = 0; index < module_count; ++index) { - modules_[index]->RegisterLocationDescriptor( - &module_location_descriptors_[index]); - } - - return true; -} - -size_t MinidumpModuleCrashpadInfoListWriter::SizeOfObject() { - DCHECK_GE(state(), kStateFrozen); - - return sizeof(module_list_base_) + - modules_.size() * sizeof(MINIDUMP_LOCATION_DESCRIPTOR); -} - -std::vector -MinidumpModuleCrashpadInfoListWriter::Children() { - DCHECK_GE(state(), kStateFrozen); - - std::vector children; - for (MinidumpModuleCrashpadInfoWriter* module : modules_) { - children.push_back(module); - } - - return children; -} - -bool MinidumpModuleCrashpadInfoListWriter::WriteObject( - FileWriterInterface* file_writer) { - DCHECK_EQ(state(), kStateWritable); - DCHECK_EQ(modules_.size(), module_location_descriptors_.size()); - - WritableIoVec iov; - iov.iov_base = &module_list_base_; - iov.iov_len = sizeof(module_list_base_); - std::vector iovecs(1, iov); - - if (!module_location_descriptors_.empty()) { - iov.iov_base = &module_location_descriptors_[0]; - iov.iov_len = module_location_descriptors_.size() * - sizeof(MINIDUMP_LOCATION_DESCRIPTOR); - iovecs.push_back(iov); - } - - return file_writer->WriteIoVec(&iovecs); + AddChild(module.Pass()); } } // namespace crashpad diff --git a/minidump/minidump_module_crashpad_info_writer.h b/minidump/minidump_module_crashpad_info_writer.h index 2db42a73..8fc304c9 100644 --- a/minidump/minidump_module_crashpad_info_writer.h +++ b/minidump/minidump_module_crashpad_info_writer.h @@ -23,6 +23,7 @@ #include "base/basictypes.h" #include "base/memory/scoped_ptr.h" #include "minidump/minidump_extensions.h" +#include "minidump/minidump_location_descriptor_list_writer.h" #include "minidump/minidump_writable.h" #include "util/stdlib/pointer_container.h" @@ -99,7 +100,7 @@ class MinidumpModuleCrashpadInfoWriter final //! \brief The writer for a MinidumpModuleCrashpadInfoList object in a minidump //! file, containing a list of MinidumpModuleCrashpadInfo objects. class MinidumpModuleCrashpadInfoListWriter final - : public internal::MinidumpWritable { + : public MinidumpLocationDescriptorListWriter { public: MinidumpModuleCrashpadInfoListWriter(); ~MinidumpModuleCrashpadInfoListWriter() override; @@ -128,18 +129,7 @@ class MinidumpModuleCrashpadInfoListWriter final //! \note Valid in #kStateMutable. void AddModule(scoped_ptr module); - protected: - // MinidumpWritable: - bool Freeze() override; - size_t SizeOfObject() override; - std::vector Children() override; - bool WriteObject(FileWriterInterface* file_writer) override; - private: - MinidumpModuleCrashpadInfoList module_list_base_; - 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 b354a73b..854771d2 100644 --- a/minidump/minidump_module_crashpad_info_writer_test.cc +++ b/minidump/minidump_module_crashpad_info_writer_test.cc @@ -20,6 +20,7 @@ #include "minidump/minidump_extensions.h" #include "minidump/minidump_simple_string_dictionary_writer.h" #include "minidump/test/minidump_file_writer_test_util.h" +#include "minidump/test/minidump_location_descriptor_list_test_util.h" #include "minidump/test/minidump_string_writer_test_util.h" #include "minidump/test/minidump_writable_test_util.h" #include "snapshot/test/test_module_snapshot.h" @@ -29,17 +30,6 @@ namespace crashpad { namespace test { namespace { -const MinidumpModuleCrashpadInfoList* MinidumpModuleCrashpadInfoListAtStart( - const std::string& file_contents, - size_t count) { - MINIDUMP_LOCATION_DESCRIPTOR location_descriptor; - location_descriptor.DataSize = sizeof(MinidumpModuleCrashpadInfoList) + - count * sizeof(MINIDUMP_LOCATION_DESCRIPTOR); - location_descriptor.Rva = 0; - return MinidumpWritableAtLocationDescriptor( - file_contents, location_descriptor); -} - TEST(MinidumpModuleCrashpadInfoWriter, EmptyList) { StringFileWriter file_writer; @@ -50,10 +40,8 @@ TEST(MinidumpModuleCrashpadInfoWriter, EmptyList) { file_writer.string().size()); const MinidumpModuleCrashpadInfoList* module_list = - MinidumpModuleCrashpadInfoListAtStart(file_writer.string(), 0); + MinidumpLocationDescriptorListAtStart(file_writer.string(), 0); ASSERT_TRUE(module_list); - - EXPECT_EQ(0u, module_list->count); } TEST(MinidumpModuleCrashpadInfoWriter, EmptyModule) { @@ -72,14 +60,12 @@ TEST(MinidumpModuleCrashpadInfoWriter, EmptyModule) { file_writer.string().size()); const MinidumpModuleCrashpadInfoList* module_list = - MinidumpModuleCrashpadInfoListAtStart(file_writer.string(), 1); + MinidumpLocationDescriptorListAtStart(file_writer.string(), 1); ASSERT_TRUE(module_list); - ASSERT_EQ(1u, module_list->count); - const MinidumpModuleCrashpadInfo* module = MinidumpWritableAtLocationDescriptor( - file_writer.string(), module_list->modules[0]); + file_writer.string(), module_list->children[0]); ASSERT_TRUE(module); EXPECT_EQ(MinidumpModuleCrashpadInfo::kVersion, module->version); @@ -122,14 +108,12 @@ TEST(MinidumpModuleCrashpadInfoWriter, FullModule) { file_writer.string().size()); const MinidumpModuleCrashpadInfoList* module_list = - MinidumpModuleCrashpadInfoListAtStart(file_writer.string(), 1); + MinidumpLocationDescriptorListAtStart(file_writer.string(), 1); ASSERT_TRUE(module_list); - ASSERT_EQ(1u, module_list->count); - const MinidumpModuleCrashpadInfo* module = MinidumpWritableAtLocationDescriptor( - file_writer.string(), module_list->modules[0]); + file_writer.string(), module_list->children[0]); ASSERT_TRUE(module); EXPECT_EQ(MinidumpModuleCrashpadInfo::kVersion, module->version); @@ -210,14 +194,12 @@ TEST(MinidumpModuleCrashpadInfoWriter, ThreeModules) { EXPECT_TRUE(module_list_writer.WriteEverything(&file_writer)); const MinidumpModuleCrashpadInfoList* module_list = - MinidumpModuleCrashpadInfoListAtStart(file_writer.string(), 3); + MinidumpLocationDescriptorListAtStart(file_writer.string(), 3); ASSERT_TRUE(module_list); - ASSERT_EQ(3u, module_list->count); - const MinidumpModuleCrashpadInfo* module_0 = MinidumpWritableAtLocationDescriptor( - file_writer.string(), module_list->modules[0]); + file_writer.string(), module_list->children[0]); ASSERT_TRUE(module_0); EXPECT_EQ(MinidumpModuleCrashpadInfo::kVersion, module_0->version); @@ -238,7 +220,7 @@ TEST(MinidumpModuleCrashpadInfoWriter, ThreeModules) { const MinidumpModuleCrashpadInfo* module_1 = MinidumpWritableAtLocationDescriptor( - file_writer.string(), module_list->modules[1]); + file_writer.string(), module_list->children[1]); ASSERT_TRUE(module_1); EXPECT_EQ(MinidumpModuleCrashpadInfo::kVersion, module_1->version); @@ -251,7 +233,7 @@ TEST(MinidumpModuleCrashpadInfoWriter, ThreeModules) { const MinidumpModuleCrashpadInfo* module_2 = MinidumpWritableAtLocationDescriptor( - file_writer.string(), module_list->modules[2]); + file_writer.string(), module_list->children[2]); ASSERT_TRUE(module_2); EXPECT_EQ(MinidumpModuleCrashpadInfo::kVersion, module_2->version); @@ -312,14 +294,12 @@ TEST(MinidumpModuleCrashpadInfoWriter, InitializeFromSnapshot) { ASSERT_TRUE(module_list_writer.WriteEverything(&file_writer)); const MinidumpModuleCrashpadInfoList* module_list = - MinidumpModuleCrashpadInfoListAtStart(file_writer.string(), 2); + MinidumpLocationDescriptorListAtStart(file_writer.string(), 2); ASSERT_TRUE(module_list); - ASSERT_EQ(2u, module_list->count); - const MinidumpModuleCrashpadInfo* module_0 = MinidumpWritableAtLocationDescriptor( - file_writer.string(), module_list->modules[0]); + file_writer.string(), module_list->children[0]); ASSERT_TRUE(module_0); EXPECT_EQ(MinidumpModuleCrashpadInfo::kVersion, module_0->version); @@ -346,7 +326,7 @@ TEST(MinidumpModuleCrashpadInfoWriter, InitializeFromSnapshot) { const MinidumpModuleCrashpadInfo* module_2 = MinidumpWritableAtLocationDescriptor( - file_writer.string(), module_list->modules[1]); + file_writer.string(), module_list->children[1]); ASSERT_TRUE(module_2); EXPECT_EQ(MinidumpModuleCrashpadInfo::kVersion, module_2->version); diff --git a/minidump/minidump_writable.cc b/minidump/minidump_writable.cc index 5e87c941..5925a885 100644 --- a/minidump/minidump_writable.cc +++ b/minidump/minidump_writable.cc @@ -31,6 +31,9 @@ const size_t kMaximumAlignment = 16; namespace crashpad { namespace internal { +MinidumpWritable::~MinidumpWritable() { +} + bool MinidumpWritable::WriteEverything(FileWriterInterface* file_writer) { DCHECK_EQ(state_, kStateMutable); @@ -89,9 +92,6 @@ MinidumpWritable::MinidumpWritable() state_(kStateMutable) { } -MinidumpWritable::~MinidumpWritable() { -} - bool MinidumpWritable::Freeze() { DCHECK_EQ(state_, kStateMutable); state_ = kStateFrozen; diff --git a/minidump/minidump_writable.h b/minidump/minidump_writable.h index cfababf9..fb34ea28 100644 --- a/minidump/minidump_writable.h +++ b/minidump/minidump_writable.h @@ -32,6 +32,8 @@ namespace internal { //! file. class MinidumpWritable { public: + virtual ~MinidumpWritable(); + //! \brief Writes an object and all of its children to a minidump file. //! //! Use this on the root object of a tree of MinidumpWritable objects, @@ -134,13 +136,6 @@ class 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_; } diff --git a/minidump/test/minidump_location_descriptor_list_test_util.cc b/minidump/test/minidump_location_descriptor_list_test_util.cc new file mode 100644 index 00000000..0a770b3f --- /dev/null +++ b/minidump/test/minidump_location_descriptor_list_test_util.cc @@ -0,0 +1,48 @@ +// Copyright 2014 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "minidump/test/minidump_location_descriptor_list_test_util.h" + +#include + +#include "minidump/minidump_extensions.h" +#include "minidump/test/minidump_writable_test_util.h" + +namespace crashpad { +namespace test { + +const MinidumpLocationDescriptorList* MinidumpLocationDescriptorListAtStart( + const std::string& file_contents, size_t count) { + MINIDUMP_LOCATION_DESCRIPTOR location_descriptor; + location_descriptor.DataSize = sizeof(MinidumpLocationDescriptorList) + + count * sizeof(MINIDUMP_LOCATION_DESCRIPTOR); + location_descriptor.Rva = 0; + + const MinidumpLocationDescriptorList* list = + MinidumpWritableAtLocationDescriptor( + file_contents, location_descriptor); + if (!list) { + return nullptr; + } + + if (list->count != count) { + EXPECT_EQ(count, list->count); + return nullptr; + } + + return list; +} + +} // namespace test +} // namespace crashpad diff --git a/minidump/test/minidump_location_descriptor_list_test_util.h b/minidump/test/minidump_location_descriptor_list_test_util.h new file mode 100644 index 00000000..b510f7d5 --- /dev/null +++ b/minidump/test/minidump_location_descriptor_list_test_util.h @@ -0,0 +1,45 @@ +// Copyright 2014 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_MINIDUMP_TEST_MINIDUMP_LOCATION_DESCRIPTOR_LIST_TEST_UTIL_H_ +#define CRASHPAD_MINIDUMP_TEST_MINIDUMP_LOCATION_DESCRIPTOR_LIST_TEST_UTIL_H_ + +#include + +#include + +namespace crashpad { + +struct MinidumpLocationDescriptorList; + +namespace test { + +//! \brief Returns the MinidumpLocationDescriptorList at the start of a minidump +//! file. +//! +//! \param[in] file_contents The contents of the minidump file. +//! \param[in] count The number of MINIDUMP_LOCATION_DESCRIPTOR objects expected +//! in the MinidumpLocationDescriptorList. This function will only be +//! successful if exactly this many objects are present, and if space for +//! them exists in \a file_contents. +//! +//! \return On success, the MinidumpLocationDescriptorList at the beginning of +//! the file. On failure, raises a gtest assertion and returns `nullptr`. +const MinidumpLocationDescriptorList* MinidumpLocationDescriptorListAtStart( + const std::string& file_contents, size_t count); + +} // namespace test +} // namespace crashpad + +#endif // CRASHPAD_MINIDUMP_TEST_MINIDUMP_LOCATION_DESCRIPTOR_LIST_TEST_UTIL_H_ diff --git a/minidump/test/minidump_writable_test_util.cc b/minidump/test/minidump_writable_test_util.cc index 42d0e7a7..a964ad27 100644 --- a/minidump/test/minidump_writable_test_util.cc +++ b/minidump/test/minidump_writable_test_util.cc @@ -178,8 +178,8 @@ struct MinidumpThreadListTraits { } }; -struct MinidumpModuleCrashpadInfoListTraits { - using ListType = MinidumpModuleCrashpadInfoList; +struct MinidumpLocationDescriptorListTraits { + using ListType = MinidumpLocationDescriptorList; static constexpr size_t kElementSize = sizeof(MINIDUMP_LOCATION_DESCRIPTOR); static size_t ElementCount(const ListType* list) { return list->count; @@ -243,11 +243,11 @@ const MINIDUMP_THREAD_LIST* MinidumpWritableAtLocationDescriptor< } template <> -const MinidumpModuleCrashpadInfoList* -MinidumpWritableAtLocationDescriptor( +const MinidumpLocationDescriptorList* +MinidumpWritableAtLocationDescriptor( const std::string& file_contents, const MINIDUMP_LOCATION_DESCRIPTOR& location) { - return MinidumpListAtLocationDescriptor( + return MinidumpListAtLocationDescriptor( file_contents, location); } diff --git a/minidump/test/minidump_writable_test_util.h b/minidump/test/minidump_writable_test_util.h index ad9f6344..7030d693 100644 --- a/minidump/test/minidump_writable_test_util.h +++ b/minidump/test/minidump_writable_test_util.h @@ -83,7 +83,7 @@ MINIDUMP_ALLOW_OVERSIZED_DATA(MINIDUMP_DIRECTORY); MINIDUMP_ALLOW_OVERSIZED_DATA(MINIDUMP_MEMORY_LIST); MINIDUMP_ALLOW_OVERSIZED_DATA(MINIDUMP_MODULE_LIST); MINIDUMP_ALLOW_OVERSIZED_DATA(MINIDUMP_THREAD_LIST); -MINIDUMP_ALLOW_OVERSIZED_DATA(MinidumpModuleCrashpadInfoList); +MINIDUMP_ALLOW_OVERSIZED_DATA(MinidumpLocationDescriptorList); MINIDUMP_ALLOW_OVERSIZED_DATA(MinidumpSimpleStringDictionary); // These types have final fields carrying variable-sized data (typically string @@ -182,8 +182,8 @@ const MINIDUMP_THREAD_LIST* MinidumpWritableAtLocationDescriptor< const MINIDUMP_LOCATION_DESCRIPTOR& location); template <> -const MinidumpModuleCrashpadInfoList* -MinidumpWritableAtLocationDescriptor( +const MinidumpLocationDescriptorList* +MinidumpWritableAtLocationDescriptor( const std::string& file_contents, const MINIDUMP_LOCATION_DESCRIPTOR& location);