A simple minidump writer for postmortem stability reports.

components/browser_watcher/postmortem_minidump_writer_win.cc

Index: components/browser_watcher/postmortem_minidump_writer_win.cc
diff --git a/components/browser_watcher/postmortem_minidump_writer_win.cc b/components/browser_watcher/postmortem_minidump_writer_win.cc
new file mode 100644
index 0000000000000000000000000000000000000000..1486fa32d6ee12f191d947b7e2e737a390465ea2
--- /dev/null
+++ b/components/browser_watcher/postmortem_minidump_writer_win.cc
@@ -0,0 +1,270 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// Note: aside from using windows headers to obtain the definitions of minidump
+//     structures, nothing here is windows specific. This seems like the best
+//     approach given this code is for temporary experimentation on Windows.
+//     Longer term, CrashPad will take over the minidump writing in this case as
+//     well.
+
+#include "components/browser_watcher/postmortem_minidump_writer.h"
+
+#include <windows.h>  // NOLINT
+#include <dbghelp.h>
+
+#include <string>
+
+#include "base/files/file_util.h"
+#include "base/numerics/safe_math.h"
+#include "third_party/crashpad/crashpad/minidump/minidump_extensions.h"
+
+namespace browser_watcher {
+
+namespace {
+
+bool IsFileAtOffset(base::File* file, uint32_t expected_offset) {
+  DCHECK(file);
+  int64_t actual_offset = file->Seek(base::File::FROM_CURRENT, 0ULL);
+  return actual_offset == static_cast<int64_t>(expected_offset);
+}
+
+// Returns true if the file is empty, and false if the file is not empty or if
+// there is an error.
+bool AssertFileEmpty(base::File* file) {

[Sigurður Ásgeirsson, 2016/09/13 17:36:56]

Sorry, should have been clearer - I'd either DCHECK() inside this function,
return void and name it AssertXXX.
Else name it something like IsXXX and return bool.

[manzagop (departed), 2016/09/13 21:19:41]

I've gone with the external dcheck to avoid the seek in non-debug. Done.

+  DCHECK(file);
+  int64_t end = file->Seek(base::File::FROM_END, 0ULL);
+  return end == 0ULL;
+}
+
+}  // namespace
+
+// The stream type assigned to the minidump stream that holds the serialized
+// stability report.
+// Note: the value was obtained by adding 1 to the stream type used for holding
+// the SyzyAsan proto.
+// TODO(manzagop): centralize the stream type definitions to avoid issues.
+const uint32_t kStabilityReportStreamType = 0x4B6B0002;
+
+void PostmortemMinidumpWriter::Initialize() {
+  next_available_byte_ = 0U;
+  directory_.clear();
+}
+
+bool PostmortemMinidumpWriter::WriteDump(
+    base::PlatformFile minidump_platform_file,
+    const StabilityReport& report,
+    const ReportInfo& report_info) {
+  DCHECK_NE(base::kInvalidPlatformFile, minidump_platform_file);
+
+  Initialize();
+  DCHECK_EQ(0U, next_available_byte_);
+  DCHECK(directory_.empty());
+
+  // We do not own |minidump_platform_file|, but we want to rely on base::File's

[scottmg, 2016/09/13 18:07:43]

Bleh, never liked base::File. :( I think I'd probably avoid ::DuplicateHandle()
by having a ScopedFileOwnershipGrant (or something) that creates the base::File
and then TakePlatformFile()s it back in ~. But if that gets ugly this is fine
too.

[manzagop (departed), 2016/09/13 21:19:41]

That's actually what I started with, but it turns out it trips the handle
checker.

+  // API, and so we need to duplicate it.
+  HANDLE duplicated_handle;
+  BOOL success = ::DuplicateHandle(
+      ::GetCurrentProcess(), minidump_platform_file, GetCurrentProcess(),

[Sigurður Ásgeirsson, 2016/09/13 17:36:56]

nit: ::GetCurrentProcess

[manzagop (departed), 2016/09/13 21:19:41]

Done.

+      &duplicated_handle, 0, FALSE, DUPLICATE_SAME_ACCESS);
+  if (!success)
+    return false;
+  base::File minidump_file(duplicated_handle);
+  DCHECK(minidump_file.IsValid());
+  minidump_file_ = &minidump_file;
+
+  // Allocate space for the header and seek the cursor.
+  FilePosition pos = 0U;
+  if (!Allocate(sizeof(MINIDUMP_HEADER), &pos))
+    return false;
+  if (!SeekCursor(sizeof(MINIDUMP_HEADER)))
+    return false;
+  DCHECK_EQ(kHeaderPos, pos);
+  DCHECK(IsFileAtOffset(minidump_file_, 0U));
+  DCHECK(AssertFileEmpty(minidump_file_));
+
+  // Write the proto to the file.
+  std::string serialized_report;
+  if (!report.SerializeToString(&serialized_report))
+    return false;
+  FilePosition report_pos = 0U;
+  if (!AppendBytes(serialized_report, &report_pos))
+    return false;
+
+  // The directory entry for the stability report's stream.
+  RegisterDirectoryEntry(kStabilityReportStreamType, report_pos,
+                         serialized_report.length());
+
+  // Write mandatory crash keys. These will be read by crashpad and used as
+  // http request parameters for the upload. Keys and values should match
+  // server side configuration.
+  // TODO(manzagop): use product and version from the stability report. The
+  // current executable's values are an (imperfect) proxy.
+  std::map<std::string, std::string> crash_keys = {
+      {"product", report_info.product_name + "_Postmortem"},
+      {"version", report_info.version_number}};
+  if (!AppendCrashpadInfo(report_info.client_id, report_info.report_id,
+                          crash_keys))
+    return false;
+
+  // Write the directory.
+  FilePosition directory_pos = 0U;
+  if (!AppendVec(directory_, &directory_pos))
+    return false;
+
+  // Write the header.
+  MINIDUMP_HEADER header;
+  header.Signature = MINIDUMP_SIGNATURE;
+  header.Version = MINIDUMP_VERSION;
+  header.NumberOfStreams = directory_.size();
+  header.StreamDirectoryRva = directory_pos;
+  if (!SeekCursor(0U))
+    return false;
+  return Write(kHeaderPos, header);
+}
+
+bool PostmortemMinidumpWriter::AppendCrashpadInfo(
+    const crashpad::UUID& client_id,
+    const crashpad::UUID& report_id,
+    const std::map<std::string, std::string>& crash_keys) {
+  // Write the crash keys as the contents of a crashpad dictionary.
+  std::vector<crashpad::MinidumpSimpleStringDictionaryEntry> entries;
+  for (const auto& crash_key : crash_keys) {
+    if (!AppendCrashpadDictionaryEntry(crash_key.first, crash_key.second,
+                                       &entries)) {
+      return false;
+    }
+  }
+
+  // Write the dictionary's index.
+  FilePosition dict_pos = 0U;
+  uint32_t entry_count = entries.size();
+  if (entry_count > 0) {
+    if (!Append(entry_count, &dict_pos))
+      return false;
+    FilePosition unused_pos = 0U;
+    if (!AppendVec(entries, &unused_pos))
+      return false;
+  }
+
+  MINIDUMP_LOCATION_DESCRIPTOR simple_annotations = {0};
+  simple_annotations.DataSize = 0U;
+  if (entry_count > 0)
+    simple_annotations.DataSize = next_available_byte_ - dict_pos;
+  // Note: an RVA of 0 indicates the absence of a dictionary.
+  simple_annotations.Rva = dict_pos;
+
+  // Write the crashpad info.
+  crashpad::MinidumpCrashpadInfo crashpad_info;
+  crashpad_info.version = crashpad::MinidumpCrashpadInfo::kVersion;
+  crashpad_info.report_id = report_id;
+  crashpad_info.client_id = client_id;
+  crashpad_info.simple_annotations = simple_annotations;
+  // Note: module_list is left at 0, which means none.
+
+  FilePosition crashpad_pos = 0U;
+  if (!Append(crashpad_info, &crashpad_pos))
+    return false;
+
+  // Append a directory entry for the crashpad info stream.
+  RegisterDirectoryEntry(crashpad::kMinidumpStreamTypeCrashpadInfo,
+                         crashpad_pos, sizeof(crashpad::MinidumpCrashpadInfo));
+
+  return true;
+}
+
+bool PostmortemMinidumpWriter::AppendCrashpadDictionaryEntry(
+    const std::string& key,
+    const std::string& value,
+    std::vector<crashpad::MinidumpSimpleStringDictionaryEntry>* entries) {
+  DCHECK_NE(nullptr, entries);
+
+  FilePosition key_pos = 0U;
+  if (!AppendUtf8String(key, &key_pos))
+    return false;
+  FilePosition value_pos = 0U;
+  if (!AppendUtf8String(value, &value_pos))
+    return false;
+
+  crashpad::MinidumpSimpleStringDictionaryEntry entry = {0};
+  entry.key = key_pos;
+  entry.value = value_pos;
+  entries->push_back(entry);
+
+  return true;
+}
+
+bool PostmortemMinidumpWriter::Allocate(size_t size_bytes, FilePosition* pos) {
+  DCHECK(pos);
+  *pos = next_available_byte_;
+
+  base::CheckedNumeric<FilePosition> next = next_available_byte_;
+  next += size_bytes;
+  if (!next.IsValid())
+    return false;
+
+  next_available_byte_ += size_bytes;
+  return true;
+}
+
+bool PostmortemMinidumpWriter::SeekCursor(FilePosition destination) {
+  DCHECK_NE(nullptr, minidump_file_);

[Sigurður Ásgeirsson, 2016/09/13 17:36:56]

yeah, these checks will never trip, as you never null the pointer. 
Null it in constructor, check on entry to WriteDump, and null it on exit? Maybe
move the actual work to a WriteDumpImpl function, and leave the setup in the
interface function?

[manzagop (departed), 2016/09/13 21:19:41]

Done.

+  DCHECK(minidump_file_->IsValid());
+
+  // Validate the write does not extend past the allocated space.
+  if (destination > next_available_byte_)
+    return false;
+
+  int64_t new_pos = minidump_file_->Seek(base::File::FROM_BEGIN,
+                                         static_cast<int64_t>(destination));
+  return new_pos != -1;
+}
+
+bool PostmortemMinidumpWriter::WriteBytes(FilePosition pos,
+                                          size_t size_bytes,
+                                          const char* data) {
+  DCHECK(data);
+  DCHECK_NE(nullptr, minidump_file_);
+  DCHECK(minidump_file_->IsValid());
+  DCHECK(IsFileAtOffset(minidump_file_, pos));
+
+  // Validate the write does not extend past the next available byte.
+  base::CheckedNumeric<FilePosition> pos_end = pos;
+  pos_end += size_bytes;
+  if (!pos_end.IsValid() || pos_end.ValueOrDie() > next_available_byte_)
+    return false;
+
+  int written_bytes = minidump_file_->WriteAtCurrentPos(data, size_bytes);
+  return written_bytes == size_bytes;
+}
+
+bool PostmortemMinidumpWriter::AppendUtf8String(base::StringPiece data,
+                                                FilePosition* pos) {
+  DCHECK(pos);
+  uint32_t string_size = data.size();
+  if (!Append(string_size, pos))
+    return false;
+
+  FilePosition unused_pos = 0U;
+  return AppendBytes(data, &unused_pos);
+}
+
+bool PostmortemMinidumpWriter::AppendBytes(base::StringPiece data,
+                                           FilePosition* pos) {
+  DCHECK(pos);
+  if (!Allocate(data.length(), pos))
+    return false;
+  return WriteBytes(*pos, data.length(), data.data());
+}
+
+void PostmortemMinidumpWriter::RegisterDirectoryEntry(uint32_t stream_type,
+                                                      FilePosition pos,
+                                                      uint32_t size) {
+  MINIDUMP_DIRECTORY entry = {0};
+  entry.StreamType = stream_type;
+  entry.Location.Rva = pos;
+  entry.Location.DataSize = size;
+  directory_.push_back(entry);
+}
+
+}  // namespace browser_watcher