Use MEMORY_BASIC_INFORMATION64 rather than a custom MemoryInfo

util/win/process_info.h

 // Copyright 2015 The Crashpad Authors. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
 //
 //     http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
 #ifndef CRASHPAD_UTIL_WIN_PROCESS_INFO_H_
 #define CRASHPAD_UTIL_WIN_PROCESS_INFO_H_
 
+#include <windows.h>
+#include <dbghelp.h>
 #include <sys/types.h>
-#include <windows.h>
 
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "util/misc/initialization_state_dcheck.h"
 #include "util/numeric/checked_range.h"
 #include "util/win/address_types.h"
 
 namespace crashpad {
@@ skipping 13 matching lines @@
     //! \brief The base address of the loaded DLL.
     WinVMAddress dll_base;
 
     //! \brief The size of the module.
     WinVMSize size;
 
     //! \brief The module's timestamp.
     time_t timestamp;
   };
 
-  // \brief Contains information about a range of pages in the virtual address
-  //    space of a process.
-  struct MemoryInfo {
-    explicit MemoryInfo(const MEMORY_BASIC_INFORMATION& mbi);
-    ~MemoryInfo();
-
-    //! \brief The base address of the region of pages.
-    WinVMAddress base_address;
-
-    //! \brief The size of the region beginning at base_address in bytes.
-    WinVMSize region_size;
-
-    //! \brief The base address of a range of pages that was allocated by
-    //!     `VirtualAlloc()`. The page pointed to base_address is within this
-    //!     range of pages.
-    WinVMAddress allocation_base;
-
-    //! \brief The state of the pages, one of `MEM_COMMIT`, `MEM_FREE`, or
-    //!     `MEM_RESERVE`.
-    uint32_t state;
-
-    //! \brief The memory protection option when this page was originally
-    //!     allocated. This will be `PAGE_EXECUTE`, `PAGE_EXECUTE_READ`, etc.
-    uint32_t allocation_protect;
-
-    //! \brief The current memoryprotection state. This will be `PAGE_EXECUTE`,
-    //!   `PAGE_EXECUTE_READ`, etc.
-    uint32_t protect;
-
-    //! \brief The type of the pages. This will be one of `MEM_IMAGE`,
-    //!     `MEM_MAPPED`, or `MEM_PRIVATE`.
-    uint32_t type;
-  };
-
   ProcessInfo();
   ~ProcessInfo();
 
   //! \brief Initializes this object with information about the given
   //!     \a process.
   //!
   //! This method must be called successfully prior to calling any other
   //! method in this class. This method may only be called once.
   //!
   //! \return `true` on success, `false` on failure with a message logged.
@@ skipping 24 matching lines @@
   void Peb(WinVMAddress* peb_address, WinVMSize* peb_size) const;
 
   //! \brief Retrieves the modules loaded into the target process.
   //!
   //! The modules are enumerated in initialization order as detailed in the
   //!     Process Environment Block. The main executable will always be the
   //!     first element.
   bool Modules(std::vector<Module>* modules) const;
 
   //! \brief Retrieves information about all pages mapped into the process.
-  const std::vector<MemoryInfo>& MemoryInformation() const;
+  const std::vector<MINIDUMP_MEMORY_INFO>& MemoryInfo() const;

[Mark Mentovai, 2015/10/06 20:57:14]

OK, this was what I was missing when reading
https://codereview.chromium.org/1377133006/. I see now.

Should this be MEMORY_BASIC_INFORMATION here? (I think you said that the 64 was
broken, but since we can’t easily equip for 32-reads-64, that should be fine.)
All the way in util, we’re more working with live processes than minidumps. The
structs are functionally equivalent, and it’d probably let you get rid of some
conversiony stuff here at the expense of having to write it somewhere else, but
I think that having the conversion thing happen elsewhere closer to
minidump-land is probably more correct.

[scottmg, 2015/10/06 22:43:26]

Makes sense. I went with MEMORY_BASIC_INFORMATION64 since that one's ULONG64s
for addresses instead of void*, which makes more sense when we're discussing the
memory of a foreign process's address space.

 
   //! \brief Given a range to be read from the target process, returns a vector
   //!     of ranges, representing the readable portions of the original range.
   //!
   //! \param[in] range The range being identified.
   //!
   //! \return A vector of ranges corresponding to the portion of \a range that
   //!     is readable based on the memory map.
   std::vector<CheckedRange<WinVMAddress, WinVMSize>> GetReadableRanges(
       const CheckedRange<WinVMAddress, WinVMSize>& range) const;
@@ skipping 13 matching lines @@
   friend bool ReadMemoryInfo(HANDLE process,
                              bool is_64_bit,
                              ProcessInfo* process_info);
 
   pid_t process_id_;
   pid_t inherited_from_process_id_;
   std::wstring command_line_;
   WinVMAddress peb_address_;
   WinVMSize peb_size_;
   std::vector<Module> modules_;
-  std::vector<MemoryInfo> memory_info_;
+  std::vector<MINIDUMP_MEMORY_INFO> memory_info_;
   bool is_64_bit_;
   bool is_wow64_;
   InitializationStateDcheck initialized_;
 
   DISALLOW_COPY_AND_ASSIGN(ProcessInfo);
 };
 
 //! \brief Given a memory map of a process, and a range to be read from the
 //!     target process, returns a vector of ranges, representing the readable
 //!     portions of the original range.
 //!
 //! This is a free function for testing, but prefer
 //! ProcessInfo::GetReadableRanges().
 std::vector<CheckedRange<WinVMAddress, WinVMSize>> GetReadableRangesOfMemoryMap(
     const CheckedRange<WinVMAddress, WinVMSize>& range,
-    const std::vector<ProcessInfo::MemoryInfo>& memory_info);
+    const std::vector<MINIDUMP_MEMORY_INFO>& memory_info);
 
 }  // namespace crashpad
 
 #endif  // CRASHPAD_UTIL_WIN_PROCESS_INFO_H_