Use CrashpadInfoReader in ModuleSnapshotWin

In preparation for deleting the custom CrashpadInfo reading routines in
the PEImageReader and also deleting the PEImageAnnotationsReader, this
change moves ModuleSnapshotWin to using the platform-independent
CrashpadInfoReader.

Bug: crashpad:270
Change-Id: Idad5de173200068243eacb2bb11b2d95b6438e90
Reviewed-on: https://chromium-review.googlesource.com/c/1388017
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>

snapshot/BUILD.gn

@@ -142,10 +142,16 @@ static_library("snapshot") {
     ]
   }
 
-  if (crashpad_is_linux || crashpad_is_android || crashpad_is_fuchsia) {
+  if (crashpad_is_linux || crashpad_is_android || crashpad_is_fuchsia ||
+      crashpad_is_win) {
     sources += [
       "crashpad_types/crashpad_info_reader.cc",
       "crashpad_types/crashpad_info_reader.h",
+    ]
+  }
+
+  if (crashpad_is_linux || crashpad_is_android || crashpad_is_fuchsia) {
+    sources += [
       "crashpad_types/image_annotation_reader.cc",
       "crashpad_types/image_annotation_reader.h",
       "elf/elf_dynamic_array_reader.cc",
@@ -332,9 +338,13 @@ source_set("snapshot_test") {
     sources += [ "crashpad_info_client_options_test.cc" ]
   }
 
+  if (crashpad_is_linux || crashpad_is_android || crashpad_is_fuchsia ||
+      crashpad_is_win) {
+    sources += [ "crashpad_types/crashpad_info_reader_test.cc" ]
+  }
+
   if (crashpad_is_linux || crashpad_is_android || crashpad_is_fuchsia) {
     sources += [
-      "crashpad_types/crashpad_info_reader_test.cc",
       "crashpad_types/image_annotation_reader_test.cc",
       "elf/elf_image_reader_test.cc",
       "elf/elf_image_reader_test_note.S",

snapshot/crashpad_types/crashpad_info_reader.cc

@@ -66,9 +66,8 @@ class CrashpadInfoReader::InfoContainerSpecific : public InfoContainer {
       return false;
     }
 
-    if (!memory->Read(address,
+    if (!memory->Read(
-                      std::min(VMSize{info.size}, VMSize{sizeof(info)}),
+            address, std::min<VMSize>(info.size, sizeof(info)), &info)) {
-                      &info)) {
       return false;
     }
 
@@ -116,7 +115,7 @@ class CrashpadInfoReader::InfoContainerSpecific : public InfoContainer {
 #define NATIVE_TRAITS Traits32
 #endif
   static_assert(!std::is_same<Traits, NATIVE_TRAITS>::value ||
-                    sizeof(info) == sizeof(CrashpadInfo),
+                    sizeof(decltype(info)) == sizeof(CrashpadInfo),
                 "CrashpadInfo size mismtach");
 #undef NATIVE_TRAITS
 };

snapshot/crashpad_types/crashpad_info_reader_test.cc

@@ -15,7 +15,6 @@
 #include "snapshot/crashpad_types/crashpad_info_reader.h"
 
 #include <sys/types.h>
-#include <unistd.h>
 
 #include <memory>
 
@@ -31,10 +30,6 @@
 #include "util/misc/from_pointer_cast.h"
 #include "util/process/process_memory_native.h"
 
-#if defined(OS_FUCHSIA)
-#include <zircon/process.h>
-#endif
-
 namespace crashpad {
 namespace test {
 namespace {

snapshot/win/module_snapshot_win.cc

@@ -33,14 +33,16 @@ ModuleSnapshotWin::ModuleSnapshotWin()
       name_(),
       pdb_name_(),
       uuid_(),
-      pe_image_reader_(),
+      memory_range_(),
+      streams_(),
+      vs_fixed_file_info_(),
+      initialized_vs_fixed_file_info_(),
       process_reader_(nullptr),
+      pe_image_reader_(),
+      crashpad_info_(),
       timestamp_(0),
       age_(0),
-      initialized_(),
+      initialized_() {}
-      vs_fixed_file_info_(),
-      initialized_vs_fixed_file_info_() {
-}
 
 ModuleSnapshotWin::~ModuleSnapshotWin() {
 }
@@ -75,6 +77,26 @@ bool ModuleSnapshotWin::Initialize(
     pdb_name_ = base::UTF16ToUTF8(name_);
   }
 
+  if (!memory_range_.Initialize(process_reader_->Memory(),
+                                process_reader_->Is64Bit())) {
+    return false;
+  }
+
+  WinVMAddress crashpad_info_address;
+  WinVMSize crashpad_info_size;
+  if (pe_image_reader_->GetCrashpadInfoSection(&crashpad_info_address,
+                                               &crashpad_info_size)) {
+    ProcessMemoryRange info_range;
+    info_range.Initialize(memory_range_);
+    info_range.RestrictRange(crashpad_info_address,
+                             crashpad_info_address + crashpad_info_size);
+
+    auto info = std::make_unique<CrashpadInfoReader>();
+    if (info->Initialize(&info_range, crashpad_info_address)) {
+      crashpad_info_ = std::move(info);
+    }
+  }
+
   INITIALIZATION_STATE_SET_VALID(initialized_);
   return true;
 }
@@ -228,8 +250,7 @@ ModuleSnapshotWin::CustomMinidumpStreams() const {
 template <class Traits>
 void ModuleSnapshotWin::GetCrashpadOptionsInternal(
     CrashpadInfoClientOptions* options) {
-  process_types::CrashpadInfo<Traits> crashpad_info;
+  if (!crashpad_info_) {
-  if (!pe_image_reader_->GetCrashpadInfo(&crashpad_info)) {
     options->crashpad_handler_behavior = TriState::kUnset;
     options->system_crash_reporter_forwarding = TriState::kUnset;
     options->gather_indirectly_referenced_memory = TriState::kUnset;
@@ -238,19 +259,13 @@ void ModuleSnapshotWin::GetCrashpadOptionsInternal(
   }
 
   options->crashpad_handler_behavior =
-      CrashpadInfoClientOptions::TriStateFromCrashpadInfo(
+      crashpad_info_->CrashpadHandlerBehavior();
-          crashpad_info.crashpad_handler_behavior);
-
   options->system_crash_reporter_forwarding =
-      CrashpadInfoClientOptions::TriStateFromCrashpadInfo(
+      crashpad_info_->SystemCrashReporterForwarding();
-          crashpad_info.system_crash_reporter_forwarding);
-
   options->gather_indirectly_referenced_memory =
-      CrashpadInfoClientOptions::TriStateFromCrashpadInfo(
+      crashpad_info_->GatherIndirectlyReferencedMemory();
-          crashpad_info.gather_indirectly_referenced_memory);
-
   options->indirectly_referenced_memory_cap =
-      crashpad_info.indirectly_referenced_memory_cap;
+      crashpad_info_->IndirectlyReferencedMemoryCap();
 }
 
 const VS_FIXEDFILEINFO* ModuleSnapshotWin::VSFixedFileInfo() const {
@@ -270,16 +285,13 @@ const VS_FIXEDFILEINFO* ModuleSnapshotWin::VSFixedFileInfo() const {
 template <class Traits>
 void ModuleSnapshotWin::GetCrashpadExtraMemoryRanges(
     std::set<CheckedRange<uint64_t>>* ranges) const {
-  process_types::CrashpadInfo<Traits> crashpad_info;
+  if (!crashpad_info_ || !crashpad_info_->ExtraMemoryRanges())
-  if (!pe_image_reader_->GetCrashpadInfo(&crashpad_info) ||
-      !crashpad_info.extra_address_ranges) {
     return;
-  }
 
   std::vector<SimpleAddressRangeBag::Entry> simple_ranges(
       SimpleAddressRangeBag::num_entries);
   if (!process_reader_->Memory()->Read(
-          crashpad_info.extra_address_ranges,
+          crashpad_info_->ExtraMemoryRanges(),
           simple_ranges.size() * sizeof(simple_ranges[0]),
           &simple_ranges[0])) {
     LOG(WARNING) << "could not read simple address_ranges from "
@@ -298,11 +310,10 @@ void ModuleSnapshotWin::GetCrashpadExtraMemoryRanges(
 template <class Traits>
 void ModuleSnapshotWin::GetCrashpadUserMinidumpStreams(
     std::vector<std::unique_ptr<const UserMinidumpStream>>* streams) const {
-  process_types::CrashpadInfo<Traits> crashpad_info;
+  if (!crashpad_info_)
-  if (!pe_image_reader_->GetCrashpadInfo(&crashpad_info))
     return;
 
-  for (uint64_t cur = crashpad_info.user_data_minidump_stream_head; cur;) {
+  for (uint64_t cur = crashpad_info_->UserDataMinidumpStreamHead(); cur;) {
     internal::UserDataMinidumpStreamListEntry list_entry;
     if (!process_reader_->Memory()->Read(
             cur, sizeof(list_entry), &list_entry)) {

snapshot/win/module_snapshot_win.h

@@ -25,6 +25,7 @@
 
 #include "base/macros.h"
 #include "snapshot/crashpad_info_client_options.h"
+#include "snapshot/crashpad_types/crashpad_info_reader.h"
 #include "snapshot/module_snapshot.h"
 #include "snapshot/win/process_reader_win.h"
 #include "util/misc/initialization_state.h"
@@ -109,18 +110,19 @@ class ModuleSnapshotWin final : public ModuleSnapshot {
   std::wstring name_;
   std::string pdb_name_;
   UUID uuid_;
-  std::unique_ptr<PEImageReader> pe_image_reader_;
+  ProcessMemoryRange memory_range_;
-  ProcessReaderWin* process_reader_;  // weak
-  time_t timestamp_;
-  uint32_t age_;
   // Too const-y: https://crashpad.chromium.org/bug/9.
   mutable std::vector<std::unique_ptr<const UserMinidumpStream>> streams_;
-  InitializationStateDcheck initialized_;
-
   // VSFixedFileInfo() is logically const, but updates these members on the
   // call. See https://crashpad.chromium.org/bug/9.
   mutable VS_FIXEDFILEINFO vs_fixed_file_info_;
   mutable InitializationState initialized_vs_fixed_file_info_;
+  ProcessReaderWin* process_reader_;  // weak
+  std::unique_ptr<PEImageReader> pe_image_reader_;
+  std::unique_ptr<CrashpadInfoReader> crashpad_info_;
+  time_t timestamp_;
+  uint32_t age_;
+  InitializationStateDcheck initialized_;
 
   DISALLOW_COPY_AND_ASSIGN(ModuleSnapshotWin);
 };

snapshot/win/pe_image_reader.cc

@@ -72,6 +72,18 @@ bool PEImageReader::Initialize(ProcessReaderWin* process_reader,
   return true;
 }
 
+bool PEImageReader::GetCrashpadInfoSection(WinVMAddress* address,
+                                           WinVMSize* size) const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  if (module_subrange_reader_.Is64Bit()) {
+    return GetCrashpadInfoSectionInternal<process_types::internal::Traits64>(
+        address, size);
+  } else {
+    return GetCrashpadInfoSectionInternal<process_types::internal::Traits32>(
+        address, size);
+  }
+}
+
 template <class Traits>
 bool PEImageReader::GetCrashpadInfo(
     process_types::CrashpadInfo<Traits>* crashpad_info) const {
@@ -294,6 +306,31 @@ bool PEImageReader::VSFixedFileInfo(
   return true;
 }
 
+template <class Traits>
+bool PEImageReader::GetCrashpadInfoSectionInternal(WinVMAddress* address,
+                                                   WinVMSize* size) const {
+  IMAGE_SECTION_HEADER section;
+  if (!GetSectionByName<typename NtHeadersForTraits<Traits>::type>("CPADinfo",
+                                                                   &section)) {
+    return false;
+  }
+
+  process_types::CrashpadInfo<Traits> crashpad_info;
+  if (section.Misc.VirtualSize <
+      offsetof(process_types::CrashpadInfo<Traits>, size) +
+          sizeof(crashpad_info.size)) {
+    LOG(WARNING) << "small crashpad info section size "
+                 << section.Misc.VirtualSize << ", "
+                 << module_subrange_reader_.name();
+    return false;
+  }
+
+  *address = Address() + section.VirtualAddress;
+  *size = std::min<WinVMSize>(sizeof(crashpad_info), section.Misc.VirtualSize);
+
+  return true;
+}
+
 template <class NtHeadersType>
 bool PEImageReader::ReadNtHeaders(NtHeadersType* nt_headers,
                                   WinVMAddress* nt_headers_address) const {

snapshot/win/pe_image_reader.h

@@ -94,6 +94,16 @@ class PEImageReader {
   //! This is the value passed as \a size to Initialize().
   WinVMSize Size() const { return module_subrange_reader_.Size(); }
 
+  //! \brief Obtains the module's CrashpadInfo structure address and size.
+  //!
+  //! \param[out] address The CrashpadInfo structure address.
+  //! \param[out] size The CrashpadInfo structure size.
+  //!
+  //! \return `true` on success, `false` on failure. If the module does not have
+  //!     a `CPADinfo` section, this will return `false` without logging any
+  //!     messages. Other failures will result in messages being logged.
+  bool GetCrashpadInfoSection(WinVMAddress* address, WinVMSize* size) const;
+
   //! \brief Obtains the module's CrashpadInfo structure.
   //!
   //! \return `true` on success, `false` on failure. If the module does not have
@@ -137,6 +147,13 @@ class PEImageReader {
   bool VSFixedFileInfo(VS_FIXEDFILEINFO* vs_fixed_file_info) const;
 
  private:
+  //! \brief Performs the internal logic for GetCrashpadInfoSection().
+  //!
+  //! \sa GetCrashpadInfoSection
+  template <class Traits>
+  bool GetCrashpadInfoSectionInternal(WinVMAddress* address,
+                                      WinVMSize* size) const;
+
   //! \brief Reads the `IMAGE_NT_HEADERS` from the beginning of the image.
   //!
   //! \param[out] nt_headers The contents of the templated NtHeadersType