win: Add memory map range intersection helper

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 <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 {
 
 //! \brief Gathers information about a process given its `HANDLE`. This consists
 //!     primarily of information stored in the Process Environment Block.
 class ProcessInfo {
  public:
   //! \brief Contains information about a module loaded into a process.
   struct Module {
     Module();
     ~Module();
 
     //! \brief The pathname used to load the module from disk.
     std::wstring name;
 
     //! \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 {
+    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
@@ skipping 85 matching lines @@
   WinVMSize peb_size_;
   std::vector<Module> modules_;
   std::vector<MemoryInfo> memory_info_;
   bool is_64_bit_;
   bool is_wow64_;
   InitializationStateDcheck initialized_;
 
   DISALLOW_COPY_AND_ASSIGN(ProcessInfo);
 };
 
+//! \brief Given a process memory map, gets a set of regions that correspond to

[Mark Mentovai, 2015/09/29 21:35:50]

“set” is slightly confusing when this returns a vector and there’s also
std::set.

[scottmg, 2015/09/30 17:40:26]

Done.

+//!     the readable regions (that are not `MEM_FREE`).
+//!
+//! \param[in] address The base address of the range being identified
+//! \param[in] size The size of the range being identified.
+//! \param[in] memory_info Memory map of a process, normally retrieved from
+//!     ProcessInfo::MemoryInformation().

[Mark Mentovai, 2015/09/29 21:35:50]

Since you mention MemoryInformation() and otherwise discuss the memory map, you
should also discuss coalescing.

[scottmg, 2015/09/30 17:40:26]

After renaming the function, I tried to rephrase the description so that it
makes it clear that it's retrieving the parts of the target process's memory
that are part of the input range, so that they'd be expected to be coalesced.

+//!
+//! \return A list of ranges corresponding to the portion of the range

[Mark Mentovai, 2015/09/29 21:35:50]

If the first mapping starts lower than address, will its base address be
adjusted upwards (and size adjusted downwards) to start at the address specified
by the user? If the last mapping ends higher than address + size, will its size
be adjusted downwards?

[scottmg, 2015/09/30 17:40:26]

I think this is addressed by the rename/description too.

+//!     identified by \a address and \a size that are readable based on the
+//!     memory map.
+std::vector<CheckedRange<WinVMAddress, WinVMSize>>

[Mark Mentovai, 2015/09/29 21:35:50]

I’m definitely cool with CheckedRange in the implementation, but I’m not so sure
about using it for the return value, especially when you haven’t really used
much of the range features or checked features of it. Maybe you’d be better off
with a simple std::pair<>. I can probably be talked into/out of however you want
to handle this, though.

[scottmg, 2015/09/30 17:40:26]

The main reason is that I don't like pair.first/.second in the receiving code. I
could make another (unchecked) Range, but there doesn't seem to be much upside?

[Mark Mentovai, 2015/10/01 17:38:58]

scottmg wrote:
It’s OK, now that you’re using the checking features of CheckedRange, it makes
more sense.

+GetAccessibleRangesInMemoryMap(

[Mark Mentovai, 2015/09/29 21:35:50]

Accessible → Readable? Based on the documentation above, I’d expect “readable”,
but based on the implementation, not so much.

[scottmg, 2015/09/30 17:40:26]

Renamed.

+    WinVMAddress address,
+    WinVMSize size,
+    const std::vector<ProcessInfo::MemoryInfo>& memory_info);

[Mark Mentovai, 2015/09/29 21:35:50]

It’s quirky for this to be a loose function that takes a vector<MemoryInfo>
instead of being a member of ProcessInfo that can get the memory map itself. I
can see that you probably did that for testability, but outside of testing, I
don’t know if that’s a great design. (As always, speak up if you had other
reasons.)

An alternative that still keeps the flexibility for testing would be to make
this a nonstatic method that’s backed by either
 - a private static helper using FRIEND_TEST (but that probably won’t work in
our configuration)
 - this function, advertising why it’s exposed in its documentation, and
advising the use of the member function instead

[scottmg, 2015/09/30 17:40:26]

Yes, it was primarily for testing. Added a member and noted that it should be
preferred.

+
 }  // namespace crashpad
 
 #endif  // CRASHPAD_UTIL_WIN_PROCESS_INFO_H_