Add list_annotations to MinidumpModuleCrashpadInfo.

TEST=MinidumpModuleCrashpadInfoWriter.*
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/705153002

minidump/minidump_extensions.h

@@ -374,9 +374,22 @@ struct __attribute__((packed, aligned(4))) MinidumpModuleCrashpadInfo {
   //! This field is present when #version is at least `1`.
   uint32_t minidump_module_list_index;
 
+  //! \brief A MinidumpRVAList pointing to MinidumpUTF8String objects. The
+  //!     module controls the data that appears here.
+  //!
+  //! These strings correspond to ModuleSnapshot::AnnotationsVector() and do not
+  //! duplicate anything in #simple_annotations.
+  //!
+  //! This field is present when #version is at least `1`.
+  MINIDUMP_LOCATION_DESCRIPTOR list_annotations;
+
   //! \brief A MinidumpSimpleStringDictionary pointing to strings interpreted as
   //!     key-value pairs. The module controls the data that appears here.
   //!
+  //! These key-value pairs correspond to
+  //! ModuleSnapshot::AnnotationsSimpleMap() and do not duplicate anything in
+  //! #list_annotations.
+  //!
   //! This field is present when #version is at least `1`.
   MINIDUMP_LOCATION_DESCRIPTOR simple_annotations;
 };

minidump/minidump_module_crashpad_info_writer.cc

@@ -25,7 +25,10 @@
 namespace crashpad {
 
 MinidumpModuleCrashpadInfoWriter::MinidumpModuleCrashpadInfoWriter()
-    : MinidumpWritable(), module_(), simple_annotations_() {
+    : MinidumpWritable(),
+      module_(),
+      list_annotations_(),
+      simple_annotations_() {
   module_.version = MinidumpModuleCrashpadInfo::kVersion;
 }
 
@@ -35,6 +38,7 @@ MinidumpModuleCrashpadInfoWriter::~MinidumpModuleCrashpadInfoWriter() {
 void MinidumpModuleCrashpadInfoWriter::InitializeFromSnapshot(
     const ModuleSnapshot* module_snapshot, size_t module_list_index) {
   DCHECK_EQ(state(), kStateMutable);
+  DCHECK(!list_annotations_);
   DCHECK(!simple_annotations_);
 
   uint32_t module_list_index_u32;
@@ -44,6 +48,13 @@ void MinidumpModuleCrashpadInfoWriter::InitializeFromSnapshot(
   }
   SetMinidumpModuleListIndex(module_list_index_u32);
 
+  auto list_annotations =
+      make_scoped_ptr(new internal::MinidumpUTF8StringListWriter());
+  list_annotations->InitializeFromVector(module_snapshot->AnnotationsVector());
+  if (list_annotations->IsUseful()) {
+    SetListAnnotations(list_annotations.Pass());
+  }
+
   auto simple_annotations =
       make_scoped_ptr(new MinidumpSimpleStringDictionaryWriter());
   simple_annotations->InitializeFromMap(
@@ -53,6 +64,13 @@ void MinidumpModuleCrashpadInfoWriter::InitializeFromSnapshot(
   }
 }
 
+void MinidumpModuleCrashpadInfoWriter::SetListAnnotations(
+    scoped_ptr<internal::MinidumpUTF8StringListWriter> list_annotations) {
+  DCHECK_EQ(state(), kStateMutable);
+
+  list_annotations_ = list_annotations.Pass();
+}
+
 void MinidumpModuleCrashpadInfoWriter::SetSimpleAnnotations(
     scoped_ptr<MinidumpSimpleStringDictionaryWriter> simple_annotations) {
   DCHECK_EQ(state(), kStateMutable);
@@ -61,7 +79,7 @@ void MinidumpModuleCrashpadInfoWriter::SetSimpleAnnotations(
 }
 
 bool MinidumpModuleCrashpadInfoWriter::IsUseful() const {
-  return simple_annotations_;
+  return list_annotations_ || simple_annotations_;
 }
 
 bool MinidumpModuleCrashpadInfoWriter::Freeze() {
@@ -71,6 +89,10 @@ bool MinidumpModuleCrashpadInfoWriter::Freeze() {
     return false;
   }
 
+  if (list_annotations_) {
+    list_annotations_->RegisterLocationDescriptor(&module_.list_annotations);
+  }
+
   if (simple_annotations_) {
     simple_annotations_->RegisterLocationDescriptor(
         &module_.simple_annotations);
@@ -90,6 +112,9 @@ MinidumpModuleCrashpadInfoWriter::Children() {
   DCHECK_GE(state(), kStateFrozen);
 
   std::vector<MinidumpWritable*> children;
+  if (list_annotations_) {
+    children.push_back(list_annotations_.get());
+  }
   if (simple_annotations_) {
     children.push_back(simple_annotations_.get());
   }

minidump/minidump_module_crashpad_info_writer.h

@@ -24,6 +24,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "minidump/minidump_extensions.h"
 #include "minidump/minidump_location_descriptor_list_writer.h"
+#include "minidump/minidump_string_writer.h"
 #include "minidump/minidump_writable.h"
 #include "util/stdlib/pointer_container.h"
 
@@ -63,6 +64,17 @@ class MinidumpModuleCrashpadInfoWriter final
     module_.minidump_module_list_index = minidump_module_list_index;
   }
 
+  //! \brief Arranges for MinidumpModuleCrashpadInfo::list_annotations to point
+  //!     to the internal::MinidumpUTF8StringListWriter object to be written by
+  //!     \a list_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 SetListAnnotations(
+      scoped_ptr<internal::MinidumpUTF8StringListWriter> list_annotations);
+
   //! \brief Arranges for MinidumpModuleCrashpadInfo::simple_annotations to
   //!     point to the MinidumpSimpleStringDictionaryWriter object to be written
   //!     by \a simple_annotations.
@@ -77,8 +89,8 @@ class MinidumpModuleCrashpadInfoWriter final
   //! \brief Determines whether the object is useful.
   //!
   //! A useful object is one that carries data that makes a meaningful
-  //! contribution to a minidump file. An object carrying simple annotations
+  //! contribution to a minidump file. An object carrying list annotations or
-  //! would be considered useful.
+  //! simple annotations would be considered useful.
   //!
   //! \return `true` if the object is useful, `false` otherwise.
   bool IsUseful() const;
@@ -92,6 +104,7 @@ class MinidumpModuleCrashpadInfoWriter final
 
  private:
   MinidumpModuleCrashpadInfo module_;
+  scoped_ptr<internal::MinidumpUTF8StringListWriter> list_annotations_;
   scoped_ptr<MinidumpSimpleStringDictionaryWriter> simple_annotations_;
 
   DISALLOW_COPY_AND_ASSIGN(MinidumpModuleCrashpadInfoWriter);

minidump/minidump_module_crashpad_info_writer_test.cc

@@ -70,6 +70,8 @@ TEST(MinidumpModuleCrashpadInfoWriter, EmptyModule) {
 
   EXPECT_EQ(MinidumpModuleCrashpadInfo::kVersion, module->version);
   EXPECT_EQ(0u, module->minidump_module_list_index);
+  EXPECT_EQ(0u, module->list_annotations.DataSize);
+  EXPECT_EQ(0u, module->list_annotations.Rva);
   EXPECT_EQ(0u, module->simple_annotations.DataSize);
   EXPECT_EQ(0u, module->simple_annotations.Rva);
 }
@@ -78,6 +80,8 @@ TEST(MinidumpModuleCrashpadInfoWriter, FullModule) {
   const uint32_t kMinidumpModuleListIndex = 1;
   const char kKey[] = "key";
   const char kValue[] = "value";
+  const char kEntry[] = "entry";
+  std::vector<std::string> vector(1, std::string(kEntry));
 
   StringFileWriter file_writer;
 
@@ -86,6 +90,10 @@ TEST(MinidumpModuleCrashpadInfoWriter, FullModule) {
   auto module_writer =
       make_scoped_ptr(new MinidumpModuleCrashpadInfoWriter());
   module_writer->SetMinidumpModuleListIndex(kMinidumpModuleListIndex);
+  auto string_list_writer =
+      make_scoped_ptr(new internal::MinidumpUTF8StringListWriter());
+  string_list_writer->InitializeFromVector(vector);
+  module_writer->SetListAnnotations(string_list_writer.Pass());
   auto simple_string_dictionary_writer =
       make_scoped_ptr(new MinidumpSimpleStringDictionaryWriter());
   auto simple_string_dictionary_entry_writer =
@@ -101,8 +109,11 @@ TEST(MinidumpModuleCrashpadInfoWriter, FullModule) {
   ASSERT_EQ(sizeof(MinidumpModuleCrashpadInfoList) +
                 sizeof(MINIDUMP_LOCATION_DESCRIPTOR) +
                 sizeof(MinidumpModuleCrashpadInfo) +
+                sizeof(MinidumpRVAList) +
+                sizeof(RVA) +
                 sizeof(MinidumpSimpleStringDictionary) +
                 sizeof(MinidumpSimpleStringDictionaryEntry) +
+                sizeof(MinidumpUTF8String) + arraysize(kEntry) + 2 +  // padding
                 sizeof(MinidumpUTF8String) + arraysize(kKey) +
                 sizeof(MinidumpUTF8String) + arraysize(kValue),
             file_writer.string().size());
@@ -118,9 +129,20 @@ TEST(MinidumpModuleCrashpadInfoWriter, FullModule) {
 
   EXPECT_EQ(MinidumpModuleCrashpadInfo::kVersion, module->version);
   EXPECT_EQ(kMinidumpModuleListIndex, module->minidump_module_list_index);
+  EXPECT_NE(0u, module->list_annotations.DataSize);
+  EXPECT_NE(0u, module->list_annotations.Rva);
   EXPECT_NE(0u, module->simple_annotations.DataSize);
   EXPECT_NE(0u, module->simple_annotations.Rva);
 
+  const MinidumpRVAList* list_annotations =
+      MinidumpWritableAtLocationDescriptor<MinidumpRVAList>(
+          file_writer.string(), module->list_annotations);
+  ASSERT_TRUE(list_annotations);
+
+  ASSERT_EQ(1u, list_annotations->count);
+  EXPECT_EQ(kEntry, MinidumpUTF8StringAtRVAAsString(
+      file_writer.string(), list_annotations->children[0]));
+
   const MinidumpSimpleStringDictionary* simple_annotations =
       MinidumpWritableAtLocationDescriptor<MinidumpSimpleStringDictionary>(
           file_writer.string(), module->simple_annotations);
@@ -205,6 +227,11 @@ TEST(MinidumpModuleCrashpadInfoWriter, ThreeModules) {
   EXPECT_EQ(MinidumpModuleCrashpadInfo::kVersion, module_0->version);
   EXPECT_EQ(kMinidumpModuleListIndex0, module_0->minidump_module_list_index);
 
+  const MinidumpRVAList* list_annotations_0 =
+      MinidumpWritableAtLocationDescriptor<MinidumpRVAList>(
+          file_writer.string(), module_0->list_annotations);
+  EXPECT_FALSE(list_annotations_0);
+
   const MinidumpSimpleStringDictionary* simple_annotations_0 =
       MinidumpWritableAtLocationDescriptor<MinidumpSimpleStringDictionary>(
           file_writer.string(), module_0->simple_annotations);
@@ -226,6 +253,11 @@ TEST(MinidumpModuleCrashpadInfoWriter, ThreeModules) {
   EXPECT_EQ(MinidumpModuleCrashpadInfo::kVersion, module_1->version);
   EXPECT_EQ(kMinidumpModuleListIndex1, module_1->minidump_module_list_index);
 
+  const MinidumpRVAList* list_annotations_1 =
+      MinidumpWritableAtLocationDescriptor<MinidumpRVAList>(
+          file_writer.string(), module_1->list_annotations);
+  EXPECT_FALSE(list_annotations_1);
+
   const MinidumpSimpleStringDictionary* simple_annotations_1 =
       MinidumpWritableAtLocationDescriptor<MinidumpSimpleStringDictionary>(
           file_writer.string(), module_1->simple_annotations);
@@ -239,6 +271,11 @@ TEST(MinidumpModuleCrashpadInfoWriter, ThreeModules) {
   EXPECT_EQ(MinidumpModuleCrashpadInfo::kVersion, module_2->version);
   EXPECT_EQ(kMinidumpModuleListIndex2, module_2->minidump_module_list_index);
 
+  const MinidumpRVAList* list_annotations_2 =
+      MinidumpWritableAtLocationDescriptor<MinidumpRVAList>(
+          file_writer.string(), module_2->list_annotations);
+  EXPECT_FALSE(list_annotations_2);
+
   const MinidumpSimpleStringDictionary* simple_annotations_2 =
       MinidumpWritableAtLocationDescriptor<MinidumpSimpleStringDictionary>(
           file_writer.string(), module_2->simple_annotations);
@@ -266,6 +303,8 @@ TEST(MinidumpModuleCrashpadInfoWriter, InitializeFromSnapshot) {
   const char kValue0B[] = "estuary";
   const char kKey2[] = "k";
   const char kValue2[] = "different_value";
+  const char kEntry3A[] = "list";
+  const char kEntry3B[] = "erine";
 
   std::vector<const ModuleSnapshot*> module_snapshots;
 
@@ -287,6 +326,13 @@ TEST(MinidumpModuleCrashpadInfoWriter, InitializeFromSnapshot) {
   module_snapshot_2.SetAnnotationsSimpleMap(annotations_simple_map_2);
   module_snapshots.push_back(&module_snapshot_2);
 
+  TestModuleSnapshot module_snapshot_3;
+  std::vector<std::string> annotations_vector_3;
+  annotations_vector_3.push_back(kEntry3A);
+  annotations_vector_3.push_back(kEntry3B);
+  module_snapshot_3.SetAnnotationsVector(annotations_vector_3);
+  module_snapshots.push_back(&module_snapshot_3);
+
   MinidumpModuleCrashpadInfoListWriter module_list_writer;
   module_list_writer.InitializeFromSnapshot(module_snapshots);
 
@@ -294,7 +340,7 @@ TEST(MinidumpModuleCrashpadInfoWriter, InitializeFromSnapshot) {
   ASSERT_TRUE(module_list_writer.WriteEverything(&file_writer));
 
   const MinidumpModuleCrashpadInfoList* module_list =
-      MinidumpLocationDescriptorListAtStart(file_writer.string(), 2);
+      MinidumpLocationDescriptorListAtStart(file_writer.string(), 3);
   ASSERT_TRUE(module_list);
 
   const MinidumpModuleCrashpadInfo* module_0 =
@@ -305,6 +351,11 @@ TEST(MinidumpModuleCrashpadInfoWriter, InitializeFromSnapshot) {
   EXPECT_EQ(MinidumpModuleCrashpadInfo::kVersion, module_0->version);
   EXPECT_EQ(0u, module_0->minidump_module_list_index);
 
+  const MinidumpRVAList* list_annotations_0 =
+      MinidumpWritableAtLocationDescriptor<MinidumpRVAList>(
+          file_writer.string(), module_0->list_annotations);
+  EXPECT_FALSE(list_annotations_0);
+
   const MinidumpSimpleStringDictionary* simple_annotations_0 =
       MinidumpWritableAtLocationDescriptor<MinidumpSimpleStringDictionary>(
           file_writer.string(), module_0->simple_annotations);
@@ -332,6 +383,11 @@ TEST(MinidumpModuleCrashpadInfoWriter, InitializeFromSnapshot) {
   EXPECT_EQ(MinidumpModuleCrashpadInfo::kVersion, module_2->version);
   EXPECT_EQ(2u, module_2->minidump_module_list_index);
 
+  const MinidumpRVAList* list_annotations_2 =
+      MinidumpWritableAtLocationDescriptor<MinidumpRVAList>(
+          file_writer.string(), module_2->list_annotations);
+  EXPECT_FALSE(list_annotations_2);
+
   const MinidumpSimpleStringDictionary* simple_annotations_2 =
       MinidumpWritableAtLocationDescriptor<MinidumpSimpleStringDictionary>(
           file_writer.string(), module_2->simple_annotations);
@@ -344,6 +400,32 @@ TEST(MinidumpModuleCrashpadInfoWriter, InitializeFromSnapshot) {
   EXPECT_EQ(kValue2,
             MinidumpUTF8StringAtRVAAsString(
                 file_writer.string(), simple_annotations_2->entries[0].value));
+
+  const MinidumpModuleCrashpadInfo* module_3 =
+      MinidumpWritableAtLocationDescriptor<MinidumpModuleCrashpadInfo>(
+          file_writer.string(), module_list->children[2]);
+  ASSERT_TRUE(module_3);
+
+  EXPECT_EQ(MinidumpModuleCrashpadInfo::kVersion, module_3->version);
+  EXPECT_EQ(3u, module_3->minidump_module_list_index);
+
+  const MinidumpRVAList* list_annotations_3 =
+      MinidumpWritableAtLocationDescriptor<MinidumpRVAList>(
+          file_writer.string(), module_3->list_annotations);
+  ASSERT_TRUE(list_annotations_3);
+
+  ASSERT_EQ(annotations_vector_3.size(), list_annotations_3->count);
+  EXPECT_EQ(kEntry3A,
+            MinidumpUTF8StringAtRVAAsString(
+                file_writer.string(), list_annotations_3->children[0]));
+  EXPECT_EQ(kEntry3B,
+            MinidumpUTF8StringAtRVAAsString(
+                file_writer.string(), list_annotations_3->children[1]));
+
+  const MinidumpSimpleStringDictionary* simple_annotations_3 =
+      MinidumpWritableAtLocationDescriptor<MinidumpSimpleStringDictionary>(
+          file_writer.string(), module_3->simple_annotations);
+  EXPECT_FALSE(simple_annotations_3);
 }
 
 }  // namespace