diff --git a/minidump/minidump_crashpad_info_writer.cc b/minidump/minidump_crashpad_info_writer.cc index b77796ea..b60b2e16 100644 --- a/minidump/minidump_crashpad_info_writer.cc +++ b/minidump/minidump_crashpad_info_writer.cc @@ -16,13 +16,17 @@ #include "base/logging.h" #include "minidump/minidump_module_crashpad_info_writer.h" +#include "minidump/minidump_simple_string_dictionary_writer.h" #include "snapshot/process_snapshot.h" #include "util/file/file_writer.h" namespace crashpad { MinidumpCrashpadInfoWriter::MinidumpCrashpadInfoWriter() - : MinidumpStreamWriter(), crashpad_info_(), module_list_(nullptr) { + : MinidumpStreamWriter(), + crashpad_info_(), + simple_annotations_(nullptr), + module_list_(nullptr) { crashpad_info_.version = MinidumpCrashpadInfo::kVersion; } @@ -34,6 +38,14 @@ void MinidumpCrashpadInfoWriter::InitializeFromSnapshot( DCHECK_EQ(state(), kStateMutable); DCHECK(!module_list_); + auto simple_annotations = + make_scoped_ptr(new MinidumpSimpleStringDictionaryWriter()); + simple_annotations->InitializeFromMap( + process_snapshot->AnnotationsSimpleMap()); + if (simple_annotations->IsUseful()) { + SetSimpleAnnotations(simple_annotations.Pass()); + } + auto modules = make_scoped_ptr(new MinidumpModuleCrashpadInfoListWriter()); modules->InitializeFromSnapshot(process_snapshot->Modules()); @@ -42,6 +54,13 @@ void MinidumpCrashpadInfoWriter::InitializeFromSnapshot( } } +void MinidumpCrashpadInfoWriter::SetSimpleAnnotations( + scoped_ptr simple_annotations) { + DCHECK_EQ(state(), kStateMutable); + + simple_annotations_ = simple_annotations.Pass(); +} + void MinidumpCrashpadInfoWriter::SetModuleList( scoped_ptr module_list) { DCHECK_EQ(state(), kStateMutable); @@ -56,6 +75,10 @@ bool MinidumpCrashpadInfoWriter::Freeze() { return false; } + if (simple_annotations_) { + simple_annotations_->RegisterLocationDescriptor( + &crashpad_info_.simple_annotations); + } if (module_list_) { module_list_->RegisterLocationDescriptor(&crashpad_info_.module_list); } @@ -74,6 +97,9 @@ MinidumpCrashpadInfoWriter::Children() { DCHECK_GE(state(), kStateFrozen); std::vector children; + if (simple_annotations_) { + children.push_back(simple_annotations_.get()); + } if (module_list_) { children.push_back(module_list_.get()); } @@ -92,7 +118,7 @@ MinidumpStreamType MinidumpCrashpadInfoWriter::StreamType() const { } bool MinidumpCrashpadInfoWriter::IsUseful() const { - return module_list_; + return simple_annotations_ || module_list_; } } // namespace crashpad diff --git a/minidump/minidump_crashpad_info_writer.h b/minidump/minidump_crashpad_info_writer.h index 22c60747..1ef3a801 100644 --- a/minidump/minidump_crashpad_info_writer.h +++ b/minidump/minidump_crashpad_info_writer.h @@ -25,6 +25,7 @@ namespace crashpad { class MinidumpModuleCrashpadInfoListWriter; +class MinidumpSimpleStringDictionaryWriter; class ProcessSnapshot; //! \brief The writer for a MinidumpCrashpadInfo stream in a minidump file. @@ -50,6 +51,17 @@ class MinidumpCrashpadInfoWriter final : public internal::MinidumpStreamWriter { //! methods after this method. void InitializeFromSnapshot(const ProcessSnapshot* process_snapshot); + //! \brief Arranges for MinidumpCrashpadInfo::simple_annotations to point to + //! the MinidumpSimpleStringDictionaryWriter object to be written by \a + //! simple_annotations. + //! + //! 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( + scoped_ptr simple_annotations); + //! \brief Arranges for MinidumpCrashpadInfo::module_list to point to the //! MinidumpModuleCrashpadInfoList object to be written by \a //! module_list. @@ -82,6 +94,7 @@ class MinidumpCrashpadInfoWriter final : public internal::MinidumpStreamWriter { private: MinidumpCrashpadInfo crashpad_info_; + scoped_ptr simple_annotations_; 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 1c775214..6b8d72f6 100644 --- a/minidump/minidump_crashpad_info_writer_test.cc +++ b/minidump/minidump_crashpad_info_writer_test.cc @@ -17,10 +17,14 @@ #include #include +#include +#include + #include "gtest/gtest.h" #include "minidump/minidump_extensions.h" #include "minidump/minidump_file_writer.h" #include "minidump/minidump_module_crashpad_info_writer.h" +#include "minidump/minidump_simple_string_dictionary_writer.h" #include "minidump/test/minidump_file_writer_test_util.h" #include "minidump/test/minidump_string_writer_test_util.h" #include "minidump/test/minidump_writable_test_util.h" @@ -32,9 +36,11 @@ namespace crashpad { namespace test { namespace { -void GetCrashpadInfoStream(const std::string& file_contents, - const MinidumpCrashpadInfo** crashpad_info, - const MinidumpModuleCrashpadInfoList** module_list) { +void GetCrashpadInfoStream( + const std::string& file_contents, + const MinidumpCrashpadInfo** crashpad_info, + const MinidumpSimpleStringDictionary** simple_annotations, + const MinidumpModuleCrashpadInfoList** module_list) { const MINIDUMP_DIRECTORY* directory; const MINIDUMP_HEADER* header = MinidumpHeaderAtStart(file_contents, &directory); @@ -47,6 +53,10 @@ void GetCrashpadInfoStream(const std::string& file_contents, file_contents, directory[0].Location); ASSERT_TRUE(*crashpad_info); + *simple_annotations = + MinidumpWritableAtLocationDescriptor( + file_contents, (*crashpad_info)->simple_annotations); + *module_list = MinidumpWritableAtLocationDescriptor( file_contents, (*crashpad_info)->module_list); @@ -63,13 +73,62 @@ TEST(MinidumpCrashpadInfoWriter, Empty) { ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); const MinidumpCrashpadInfo* crashpad_info = nullptr; + const MinidumpSimpleStringDictionary* simple_annotations = nullptr; const MinidumpModuleCrashpadInfoList* module_list = nullptr; ASSERT_NO_FATAL_FAILURE(GetCrashpadInfoStream( - file_writer.string(), &crashpad_info, &module_list)); + file_writer.string(), &crashpad_info, &simple_annotations, &module_list)); + + EXPECT_EQ(MinidumpCrashpadInfo::kVersion, crashpad_info->version); + EXPECT_FALSE(simple_annotations); + EXPECT_FALSE(module_list); +} + +TEST(MinidumpCrashpadInfoWriter, SimpleAnnotations) { + MinidumpFileWriter minidump_file_writer; + auto crashpad_info_writer = make_scoped_ptr(new MinidumpCrashpadInfoWriter()); + + const char kKey[] = + "a thing that provides a means of gaining access to or understanding " + "something"; + const char kValue[] = + "the numerical amount denoted by an algebraic term; a magnitude, " + "quantity, or number"; + 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()); + crashpad_info_writer->SetSimpleAnnotations( + simple_string_dictionary_writer.Pass()); + + EXPECT_TRUE(crashpad_info_writer->IsUseful()); + + minidump_file_writer.AddStream(crashpad_info_writer.Pass()); + + StringFileWriter file_writer; + ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); + + const MinidumpCrashpadInfo* crashpad_info = nullptr; + const MinidumpSimpleStringDictionary* simple_annotations = nullptr; + const MinidumpModuleCrashpadInfoList* module_list = nullptr; + + ASSERT_NO_FATAL_FAILURE(GetCrashpadInfoStream( + file_writer.string(), &crashpad_info, &simple_annotations, &module_list)); EXPECT_EQ(MinidumpCrashpadInfo::kVersion, crashpad_info->version); EXPECT_FALSE(module_list); + + ASSERT_TRUE(simple_annotations); + ASSERT_EQ(1u, simple_annotations->count); + EXPECT_EQ(kKey, + MinidumpUTF8StringAtRVAAsString( + file_writer.string(), simple_annotations->entries[0].key)); + EXPECT_EQ(kValue, + MinidumpUTF8StringAtRVAAsString( + file_writer.string(), simple_annotations->entries[0].value)); } TEST(MinidumpCrashpadInfoWriter, CrashpadModuleList) { @@ -93,12 +152,15 @@ TEST(MinidumpCrashpadInfoWriter, CrashpadModuleList) { ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); const MinidumpCrashpadInfo* crashpad_info = nullptr; - const MinidumpModuleCrashpadInfoList* module_list; + const MinidumpSimpleStringDictionary* simple_annotations = nullptr; + const MinidumpModuleCrashpadInfoList* module_list = nullptr; ASSERT_NO_FATAL_FAILURE(GetCrashpadInfoStream( - file_writer.string(), &crashpad_info, &module_list)); + file_writer.string(), &crashpad_info, &simple_annotations, &module_list)); EXPECT_EQ(MinidumpCrashpadInfo::kVersion, crashpad_info->version); + EXPECT_FALSE(simple_annotations); + ASSERT_TRUE(module_list); ASSERT_EQ(1u, module_list->count); @@ -116,6 +178,8 @@ TEST(MinidumpCrashpadInfoWriter, CrashpadModuleList) { } TEST(MinidumpCrashpadInfoWriter, InitializeFromSnapshot) { + const char kKey[] = "version"; + const char kValue[] = "40.0.2214.111"; const char kEntry[] = "This is a simple annotation in a list."; // Test with a useless module, one that doesn’t carry anything that would @@ -132,6 +196,10 @@ TEST(MinidumpCrashpadInfoWriter, InitializeFromSnapshot) { // Try again with a useful module. process_snapshot.reset(new TestProcessSnapshot()); + std::map annotations_simple_map; + annotations_simple_map[kKey] = kValue; + process_snapshot->SetAnnotationsSimpleMap(annotations_simple_map); + module_snapshot.reset(new TestModuleSnapshot()); std::vector annotations_list(1, std::string(kEntry)); module_snapshot->SetAnnotationsVector(annotations_list); @@ -148,11 +216,22 @@ TEST(MinidumpCrashpadInfoWriter, InitializeFromSnapshot) { ASSERT_TRUE(minidump_file_writer.WriteEverything(&file_writer)); const MinidumpCrashpadInfo* info = nullptr; + const MinidumpSimpleStringDictionary* simple_annotations; const MinidumpModuleCrashpadInfoList* module_list; ASSERT_NO_FATAL_FAILURE(GetCrashpadInfoStream( - file_writer.string(), &info, &module_list)); + file_writer.string(), &info, &simple_annotations, &module_list)); EXPECT_EQ(MinidumpCrashpadInfo::kVersion, info->version); + + ASSERT_TRUE(simple_annotations); + ASSERT_EQ(1u, simple_annotations->count); + EXPECT_EQ(kKey, + MinidumpUTF8StringAtRVAAsString( + file_writer.string(), simple_annotations->entries[0].key)); + EXPECT_EQ(kValue, + MinidumpUTF8StringAtRVAAsString( + file_writer.string(), simple_annotations->entries[0].value)); + ASSERT_TRUE(module_list); ASSERT_EQ(1u, module_list->count); @@ -171,13 +250,13 @@ TEST(MinidumpCrashpadInfoWriter, InitializeFromSnapshot) { ASSERT_EQ(1u, list_annotations->count); EXPECT_EQ(kEntry, - MinidumpUTF8StringAtRVAAsString( - file_writer.string(), list_annotations->children[0])); + MinidumpUTF8StringAtRVAAsString(file_writer.string(), + list_annotations->children[0])); - const MinidumpSimpleStringDictionary* simple_annotations = + const MinidumpSimpleStringDictionary* module_simple_annotations = MinidumpWritableAtLocationDescriptor( file_writer.string(), module->simple_annotations); - EXPECT_FALSE(simple_annotations); + EXPECT_FALSE(module_simple_annotations); } } // namespace diff --git a/minidump/minidump_extensions.h b/minidump/minidump_extensions.h index 20fe2dda..5db6e6c1 100644 --- a/minidump/minidump_extensions.h +++ b/minidump/minidump_extensions.h @@ -456,6 +456,15 @@ struct ALIGNAS(4) PACKED MinidumpCrashpadInfo { //! no need for any fields present in later versions. uint32_t version; + //! \brief A MinidumpSimpleStringDictionary pointing to strings interpreted as + //! key-value pairs. + //! + //! These key-value pairs correspond to + //! ProcessSnapshot::AnnotationsSimpleMap(). + //! + //! This field is present when #version is at least `1`. + MINIDUMP_LOCATION_DESCRIPTOR simple_annotations; + //! \brief A pointer to a #MinidumpModuleCrashpadInfoList structure. //! //! This field is present when #version is at least `1`. diff --git a/snapshot/mac/process_snapshot_mac.cc b/snapshot/mac/process_snapshot_mac.cc index c241c982..dd462a90 100644 --- a/snapshot/mac/process_snapshot_mac.cc +++ b/snapshot/mac/process_snapshot_mac.cc @@ -23,6 +23,7 @@ ProcessSnapshotMac::ProcessSnapshotMac() modules_(), exception_(), process_reader_(), + annotations_simple_map_(), snapshot_time_(), initialized_() { } @@ -104,6 +105,11 @@ void ProcessSnapshotMac::ProcessCPUTimes(timeval* user_time, process_reader_.CPUTimes(user_time, system_time); } +const std::map& +ProcessSnapshotMac::AnnotationsSimpleMap() const { + return annotations_simple_map_; +} + const SystemSnapshot* ProcessSnapshotMac::System() const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); return &system_; diff --git a/snapshot/mac/process_snapshot_mac.h b/snapshot/mac/process_snapshot_mac.h index 4c1278e7..ef50b70c 100644 --- a/snapshot/mac/process_snapshot_mac.h +++ b/snapshot/mac/process_snapshot_mac.h @@ -19,6 +19,8 @@ #include #include +#include +#include #include #include "base/basictypes.h" @@ -72,6 +74,17 @@ class ProcessSnapshotMac final : public ProcessSnapshot { const natural_t* state, mach_msg_type_number_t state_count); + //! \brief Sets the value to be returned by AnnotationsSimpleMap(). + //! + //! On Mac OS X, all process annotations are under the control of the snapshot + //! producer, which may call this method to establish these annotations. + //! Contrast this with module annotations, which are under the control of the + //! process being snapshotted. + void SetAnnotationsSimpleMap( + const std::map& annotations_simple_map) { + annotations_simple_map_ = annotations_simple_map; + } + // ProcessSnapshot: pid_t ProcessID() const override; @@ -79,6 +92,8 @@ class ProcessSnapshotMac final : public ProcessSnapshot { void SnapshotTime(timeval* snapshot_time) const override; void ProcessStartTime(timeval* start_time) const override; void ProcessCPUTimes(timeval* user_time, timeval* system_time) const override; + const std::map& AnnotationsSimpleMap() + const override; const SystemSnapshot* System() const override; std::vector Threads() const override; std::vector Modules() const override; @@ -96,6 +111,7 @@ class ProcessSnapshotMac final : public ProcessSnapshot { PointerVector modules_; scoped_ptr exception_; ProcessReader process_reader_; + std::map annotations_simple_map_; timeval snapshot_time_; InitializationStateDcheck initialized_; diff --git a/snapshot/module_snapshot.h b/snapshot/module_snapshot.h index ef2a9c1f..d8f1ae6b 100644 --- a/snapshot/module_snapshot.h +++ b/snapshot/module_snapshot.h @@ -142,10 +142,14 @@ class ModuleSnapshot { //! //! For Mac OS X snapshots, these annotations are found by interpreting the //! `__DATA, __crashpad_info` section as `CrashpadInfo`. Clients can use the - //! Crashpad client interface to store annotations in this structure. + //! Crashpad client interface to store annotations in this structure. Most + //! annotations under the client’s direct control will be retrievable by this + //! method. For clients such as Chrome, this includes the process type. //! //! The annotations returned by this method do not duplicate those returned by - //! AnnotationsVector(). + //! AnnotationsVector(). Additional annotations related to the process, + //! system, or snapshot producer may be obtained by calling + //! ProcessSnapshot::AnnotationsSimpleMap(). virtual std::map AnnotationsSimpleMap() const = 0; }; diff --git a/snapshot/process_snapshot.h b/snapshot/process_snapshot.h index 3fe317fa..40811b06 100644 --- a/snapshot/process_snapshot.h +++ b/snapshot/process_snapshot.h @@ -18,6 +18,8 @@ #include #include +#include +#include #include namespace crashpad { @@ -75,6 +77,32 @@ class ProcessSnapshot { virtual void ProcessCPUTimes(timeval* user_time, timeval* system_time) const = 0; + //! \brief Returns key-value string annotations recorded for the process, + //! system, or snapshot producer. + //! + //! This method retrieves annotations recorded for a process. These + //! annotations are intended for diagnostic use, including crash analysis. + //! “Simple annotations” are structured as a sequence of key-value pairs, + //! where all keys and values are strings. These are referred to in Chrome as + //! “crash keys.” + //! + //! Annotations stored here may reflect the process, system, or snapshot + //! producer. Most annotations not under the client’s direct control will be + //! retrievable by this method. For clients such as Chrome, this includes the + //! product name and version. + //! + //! Additional per-module annotations may be obtained by calling + //! ModuleSnapshot::AnnotationsSimpleMap(). + // + // This interface currently returns a const& because all implementations store + // the data within their objects in this format, and are therefore able to + // provide this access without requiring a copy. Future implementations may + // obtain data on demand or store data in a different format, at which point + // the cost of maintaining this data in ProcessSnapshot subclass objects will + // need to be taken into account, and this interface possibly revised. + virtual const std::map& AnnotationsSimpleMap() + const = 0; + //! \brief Returns a SystemSnapshot reflecting the characteristics of the //! system that ran the snapshot process at the time of the snapshot. //! diff --git a/snapshot/test/test_process_snapshot.cc b/snapshot/test/test_process_snapshot.cc index ae524595..13d18e34 100644 --- a/snapshot/test/test_process_snapshot.cc +++ b/snapshot/test/test_process_snapshot.cc @@ -26,6 +26,7 @@ TestProcessSnapshot::TestProcessSnapshot() process_start_time_(), process_cpu_user_time_(), process_cpu_system_time_(), + annotations_simple_map_(), system_(), threads_(), modules_(), @@ -57,6 +58,11 @@ void TestProcessSnapshot::ProcessCPUTimes(timeval* user_time, *system_time = process_cpu_system_time_; } +const std::map& +TestProcessSnapshot::AnnotationsSimpleMap() const { + return annotations_simple_map_; +} + const SystemSnapshot* TestProcessSnapshot::System() const { return system_.get(); } diff --git a/snapshot/test/test_process_snapshot.h b/snapshot/test/test_process_snapshot.h index 38026706..a557285e 100644 --- a/snapshot/test/test_process_snapshot.h +++ b/snapshot/test/test_process_snapshot.h @@ -19,6 +19,8 @@ #include #include +#include +#include #include #include "base/basictypes.h" @@ -55,6 +57,10 @@ class TestProcessSnapshot final : public ProcessSnapshot { process_cpu_user_time_ = user_time; process_cpu_system_time_ = system_time; } + void SetAnnotationsSimpleMap( + const std::map& annotations_simple_map) { + annotations_simple_map_ = annotations_simple_map; + } //! \brief Sets the system snapshot to be returned by System(). //! @@ -93,6 +99,8 @@ class TestProcessSnapshot final : public ProcessSnapshot { void SnapshotTime(timeval* snapshot_time) const override; void ProcessStartTime(timeval* start_time) const override; void ProcessCPUTimes(timeval* user_time, timeval* system_time) const override; + const std::map& AnnotationsSimpleMap() + const override; const SystemSnapshot* System() const override; std::vector Threads() const override; std::vector Modules() const override; @@ -105,6 +113,7 @@ class TestProcessSnapshot final : public ProcessSnapshot { timeval process_start_time_; timeval process_cpu_user_time_; timeval process_cpu_system_time_; + std::map annotations_simple_map_; scoped_ptr system_; PointerVector threads_; PointerVector modules_;