Make OSVersionFull work for SystemSnapshotMinidump

Bug: crashpad:10
Change-Id: I98c630d4c9c9ba4b5a4d7f9605102827bf185cc3
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1575663
Commit-Queue: Casey Dahlin <sadmac@google.com>
Reviewed-by: Mark Mentovai <mark@chromium.org>

snapshot/minidump/process_snapshot_minidump.cc

@@ -16,6 +16,7 @@
 
 #include <utility>
 
+#include "base/strings/utf_string_conversions.h"
 #include "minidump/minidump_extensions.h"
 #include "snapshot/memory_map_region_snapshot.h"
 #include "snapshot/minidump/minidump_simple_string_dictionary_reader.h"
@@ -60,8 +61,7 @@ ProcessSnapshotMinidump::ProcessSnapshotMinidump()
       process_id_(static_cast<pid_t>(-1)),
       initialized_() {}
 
-ProcessSnapshotMinidump::~ProcessSnapshotMinidump() {
+ProcessSnapshotMinidump::~ProcessSnapshotMinidump() {}
-}
 
 bool ProcessSnapshotMinidump::Initialize(FileReaderInterface* file_reader) {
   INITIALIZATION_STATE_SET_INITIALIZING(initialized_);
@@ -310,6 +310,9 @@ bool ProcessSnapshotMinidump::InitializeMiscInfo() {
   switch (stream_it->second->DataSize) {
     case sizeof(MINIDUMP_MISC_INFO_5):
     case sizeof(MINIDUMP_MISC_INFO_4):
+      full_version_ = base::UTF16ToUTF8(info.BuildString);
+      full_version_ = full_version_.substr(0, full_version_.find(";"));
+      FALLTHROUGH;
     case sizeof(MINIDUMP_MISC_INFO_3):
     case sizeof(MINIDUMP_MISC_INFO_2):
     case sizeof(MINIDUMP_MISC_INFO):
@@ -347,7 +350,7 @@ bool ProcessSnapshotMinidump::InitializeModules() {
   }
 
   if (sizeof(MINIDUMP_MODULE_LIST) + module_count * sizeof(MINIDUMP_MODULE) !=
-          stream_it->second->DataSize) {
+      stream_it->second->DataSize) {
     LOG(ERROR) << "module_list size mismatch";
     return false;
   }
@@ -389,7 +392,7 @@ bool ProcessSnapshotMinidump::InitializeModulesCrashpadInfo(
   }
 
   if (crashpad_info_.module_list.DataSize <
-          sizeof(MinidumpModuleCrashpadInfoList)) {
+      sizeof(MinidumpModuleCrashpadInfoList)) {
     LOG(ERROR) << "module_crashpad_info_list size mismatch";
     return false;
   }
@@ -405,8 +408,8 @@ bool ProcessSnapshotMinidump::InitializeModulesCrashpadInfo(
   }
 
   if (crashpad_info_.module_list.DataSize !=
-          sizeof(MinidumpModuleCrashpadInfoList) +
+      sizeof(MinidumpModuleCrashpadInfoList) +
-              crashpad_module_count * sizeof(MinidumpModuleCrashpadInfoLink)) {
+          crashpad_module_count * sizeof(MinidumpModuleCrashpadInfoLink)) {
     LOG(ERROR) << "module_crashpad_info_list size mismatch";
     return false;
   }
@@ -426,7 +429,8 @@ bool ProcessSnapshotMinidump::InitializeModulesCrashpadInfo(
         minidump_links[crashpad_module_index];
     if (!module_crashpad_info_links
              ->insert(std::make_pair(minidump_link.minidump_module_list_index,
-                                     minidump_link.location)).second) {
+                                     minidump_link.location))
+             .second) {
       LOG(WARNING)
           << "duplicate module_crashpad_info_list minidump_module_list_index "
           << minidump_link.minidump_module_list_index;
@@ -467,7 +471,8 @@ bool ProcessSnapshotMinidump::InitializeMemoryInfo() {
   }
 
   if (sizeof(MINIDUMP_MEMORY_INFO_LIST) +
-      list.NumberOfEntries * list.SizeOfEntry != stream_it->second->DataSize) {
+          list.NumberOfEntries * list.SizeOfEntry !=
+      stream_it->second->DataSize) {
     LOG(ERROR) << "memory_info_list size mismatch";
     return false;
   }
@@ -480,7 +485,7 @@ bool ProcessSnapshotMinidump::InitializeMemoryInfo() {
     }
 
     mem_regions_.emplace_back(
-      std::make_unique<internal::MemoryMapRegionSnapshotMinidump>(info));
+        std::make_unique<internal::MemoryMapRegionSnapshotMinidump>(info));
     mem_regions_exposed_.emplace_back(mem_regions_.back().get());
   }
 
@@ -508,7 +513,7 @@ bool ProcessSnapshotMinidump::InitializeThreads() {
   }
 
   if (sizeof(MINIDUMP_THREAD_LIST) + thread_count * sizeof(MINIDUMP_THREAD) !=
-          stream_it->second->DataSize) {
+      stream_it->second->DataSize) {
     LOG(ERROR) << "thread_list size mismatch";
     return false;
   }
@@ -539,7 +544,8 @@ bool ProcessSnapshotMinidump::InitializeSystemSnapshot() {
     return false;
   }
 
-  if (!system_snapshot_.Initialize(file_reader_, stream_it->second->Rva)) {
+  if (!system_snapshot_.Initialize(
+          file_reader_, stream_it->second->Rva, full_version_)) {
     return false;
   }
 

snapshot/minidump/process_snapshot_minidump.h

@@ -149,6 +149,7 @@ class ProcessSnapshotMinidump final : public ProcessSnapshot {
   internal::ExceptionSnapshotMinidump exception_snapshot_;
   CPUArchitecture arch_;
   std::map<std::string, std::string> annotations_simple_map_;
+  std::string full_version_;
   FileReaderInterface* file_reader_;  // weak
   pid_t process_id_;
   InitializationStateDcheck initialized_;

snapshot/minidump/process_snapshot_minidump_test.cc

@@ -18,6 +18,7 @@
 #include <dbghelp.h>
 #include <string.h>
 
+#include <algorithm>
 #include <memory>
 
 #include "base/numerics/safe_math.h"
@@ -647,6 +648,13 @@ TEST(ProcessSnapshotMinidump, System) {
   minidump_system_info.Cpu.X86CpuInfo.VendorId[1] = cpu_info_bytes[1];
   minidump_system_info.Cpu.X86CpuInfo.VendorId[2] = cpu_info_bytes[2];
 
+  MINIDUMP_MISC_INFO_5 minidump_misc_info = {};
+  base::string16 build_string;
+  ASSERT_TRUE(base::UTF8ToUTF16(
+      "MyOSVersion; MyMachineDescription", 33, &build_string));
+  std::copy(build_string.begin(), build_string.end(),
+            minidump_misc_info.BuildString);
+
   MINIDUMP_DIRECTORY minidump_system_info_directory = {};
   minidump_system_info_directory.StreamType = kMinidumpStreamTypeSystemInfo;
   minidump_system_info_directory.Location.DataSize =
@@ -657,13 +665,24 @@ TEST(ProcessSnapshotMinidump, System) {
   ASSERT_TRUE(
       string_file.Write(&minidump_system_info, sizeof(minidump_system_info)));
 
+  MINIDUMP_DIRECTORY minidump_misc_info_directory = {};
+  minidump_misc_info_directory.StreamType = kMinidumpStreamTypeMiscInfo;
+  minidump_misc_info_directory.Location.DataSize = sizeof(MINIDUMP_MISC_INFO_5);
+  minidump_misc_info_directory.Location.Rva =
+      static_cast<RVA>(string_file.SeekGet());
+
+  ASSERT_TRUE(
+      string_file.Write(&minidump_misc_info, sizeof(minidump_misc_info)));
+
   header.StreamDirectoryRva = static_cast<RVA>(string_file.SeekGet());
   ASSERT_TRUE(string_file.Write(&minidump_system_info_directory,
                                 sizeof(minidump_system_info_directory)));
+  ASSERT_TRUE(string_file.Write(&minidump_misc_info_directory,
+                                sizeof(minidump_misc_info_directory)));
 
   header.Signature = MINIDUMP_SIGNATURE;
   header.Version = MINIDUMP_VERSION;
-  header.NumberOfStreams = 1;
+  header.NumberOfStreams = 2;
   EXPECT_TRUE(string_file.SeekSet(0));
   EXPECT_TRUE(string_file.Write(&header, sizeof(header)));
 
@@ -677,6 +696,7 @@ TEST(ProcessSnapshotMinidump, System) {
   EXPECT_EQ(s->CPUVendor(), "GenuineIntel");
   EXPECT_EQ(s->GetOperatingSystem(),
             SystemSnapshot::OperatingSystem::kOperatingSystemFuchsia);
+  EXPECT_EQ(s->OSVersionFull(), "MyOSVersion");
 
   int major, minor, bugfix;
   std::string build;

snapshot/minidump/system_snapshot_minidump.cc

@@ -20,17 +20,17 @@ namespace crashpad {
 namespace internal {
 
 SystemSnapshotMinidump::SystemSnapshotMinidump()
-    : SystemSnapshot(),
+    : SystemSnapshot(), minidump_system_info_(), initialized_() {}
-      minidump_system_info_(),
-      initialized_() {
-}
 
-SystemSnapshotMinidump::~SystemSnapshotMinidump() {
+SystemSnapshotMinidump::~SystemSnapshotMinidump() {}
-}
 
 bool SystemSnapshotMinidump::Initialize(FileReaderInterface* file_reader,
-                                        RVA minidump_system_info_rva) {
+                                        RVA minidump_system_info_rva,
+                                        const std::string& version) {
   INITIALIZATION_STATE_SET_INITIALIZING(initialized_);
+
+  full_version_ = version;
+
   if (!file_reader->SeekSet(minidump_system_info_rva)) {
     return false;
   }
@@ -40,7 +40,8 @@ bool SystemSnapshotMinidump::Initialize(FileReaderInterface* file_reader,
     return false;
   }
 
-  if (!ReadMinidumpUTF8String(file_reader, minidump_system_info_.CSDVersionRva,
+  if (!ReadMinidumpUTF8String(file_reader,
+                              minidump_system_info_.CSDVersionRva,
                               &minidump_build_name_)) {
     return false;
   }
@@ -52,23 +53,23 @@ bool SystemSnapshotMinidump::Initialize(FileReaderInterface* file_reader,
 CPUArchitecture SystemSnapshotMinidump::GetCPUArchitecture() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
   switch (minidump_system_info_.ProcessorArchitecture) {
-  case kMinidumpCPUArchitectureAMD64:
+    case kMinidumpCPUArchitectureAMD64:
-    return kCPUArchitectureX86_64;
+      return kCPUArchitectureX86_64;
-  case kMinidumpCPUArchitectureX86:
+    case kMinidumpCPUArchitectureX86:
-  case kMinidumpCPUArchitectureX86Win64:
+    case kMinidumpCPUArchitectureX86Win64:
-    return kCPUArchitectureX86;
+      return kCPUArchitectureX86;
-  case kMinidumpCPUArchitectureARM:
+    case kMinidumpCPUArchitectureARM:
-  case kMinidumpCPUArchitectureARM32Win64:
+    case kMinidumpCPUArchitectureARM32Win64:
-    return kCPUArchitectureARM;
+      return kCPUArchitectureARM;
-  case kMinidumpCPUArchitectureARM64:
+    case kMinidumpCPUArchitectureARM64:
-  case kMinidumpCPUArchitectureARM64Breakpad:
+    case kMinidumpCPUArchitectureARM64Breakpad:
-    return kCPUArchitectureARM64;
+      return kCPUArchitectureARM64;
-  case kMinidumpCPUArchitectureMIPS:
+    case kMinidumpCPUArchitectureMIPS:
-    return kCPUArchitectureMIPSEL;
+      return kCPUArchitectureMIPSEL;
-  // No word on how MIPS64 is signalled
+    // No word on how MIPS64 is signalled
 
-  default:
+    default:
-    return CPUArchitecture::kCPUArchitectureUnknown;
+      return CPUArchitecture::kCPUArchitectureUnknown;
   }
 }
 
@@ -85,9 +86,8 @@ uint8_t SystemSnapshotMinidump::CPUCount() const {
 std::string SystemSnapshotMinidump::CPUVendor() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
   if (GetCPUArchitecture() == kCPUArchitectureX86) {
-    const char* ptr =
+    const char* ptr = reinterpret_cast<const char*>(
-      reinterpret_cast<const char*>(minidump_system_info_.Cpu.X86CpuInfo.
+        minidump_system_info_.Cpu.X86CpuInfo.VendorId);
-                                    VendorId);
     return std::string(ptr, ptr + (3 * sizeof(uint32_t)));
   } else {
     return std::string();
@@ -130,25 +130,25 @@ bool SystemSnapshotMinidump::CPUX86SupportsDAZ() const {
   return false;
 }
 
-SystemSnapshot::OperatingSystem
+SystemSnapshot::OperatingSystem SystemSnapshotMinidump::GetOperatingSystem()
-    SystemSnapshotMinidump::GetOperatingSystem() const {
+    const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
 
   switch (minidump_system_info_.PlatformId) {
-  case kMinidumpOSMacOSX:
+    case kMinidumpOSMacOSX:
-    return OperatingSystem::kOperatingSystemMacOSX;
+      return OperatingSystem::kOperatingSystemMacOSX;
-  case kMinidumpOSWin32s:
+    case kMinidumpOSWin32s:
-  case kMinidumpOSWin32Windows:
+    case kMinidumpOSWin32Windows:
-  case kMinidumpOSWin32NT:
+    case kMinidumpOSWin32NT:
-    return OperatingSystem::kOperatingSystemWindows;
+      return OperatingSystem::kOperatingSystemWindows;
-  case kMinidumpOSLinux:
+    case kMinidumpOSLinux:
-    return OperatingSystem::kOperatingSystemLinux;
+      return OperatingSystem::kOperatingSystemLinux;
-  case kMinidumpOSAndroid:
+    case kMinidumpOSAndroid:
-    return OperatingSystem::kOperatingSystemAndroid;
+      return OperatingSystem::kOperatingSystemAndroid;
-  case kMinidumpOSFuchsia:
+    case kMinidumpOSFuchsia:
-    return OperatingSystem::kOperatingSystemFuchsia;
+      return OperatingSystem::kOperatingSystemFuchsia;
-  default:
+    default:
-    return OperatingSystem::kOperatingSystemUnknown;
+      return OperatingSystem::kOperatingSystemUnknown;
   }
 }
 
@@ -170,8 +170,7 @@ void SystemSnapshotMinidump::OSVersion(int* major,
 
 std::string SystemSnapshotMinidump::OSVersionFull() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://crashpad.chromium.org/bug/10
+  return full_version_;
-  return std::string();
 }
 
 std::string SystemSnapshotMinidump::MachineDescription() const {

snapshot/minidump/system_snapshot_minidump.h

@@ -38,11 +38,14 @@ class SystemSnapshotMinidump : public SystemSnapshot {
   //!     The file reader must support seeking.
   //! \param[in] minidump_system_info_rva The file offset in \a file_reader at
   //!     which the thread’s MINIDUMP_SYSTEM_INFO structure is located.
+  //! \param[in] version The OS version taken from the build string in
+  //!     MINIDUMP_MISC_INFO_4.
   //!
   //! \return `true` if the snapshot could be created, `false` otherwise with
   //!     an appropriate message logged.
   bool Initialize(FileReaderInterface* file_reader,
-                  RVA minidump_system_info_rva);
+                  RVA minidump_system_info_rva,
+                  const std::string& version);
 
   CPUArchitecture GetCPUArchitecture() const override;
   uint32_t CPURevision() const override;
@@ -68,9 +71,11 @@ class SystemSnapshotMinidump : public SystemSnapshot {
                 int* daylight_offset_seconds,
                 std::string* standard_name,
                 std::string* daylight_name) const override;
+
  private:
   MINIDUMP_SYSTEM_INFO minidump_system_info_;
   std::string minidump_build_name_;
+  std::string full_version_;
   InitializationStateDcheck initialized_;
 
   DISALLOW_COPY_AND_ASSIGN(SystemSnapshotMinidump);