From d3bdb23ffe0f930b82452c74c8d129786c044f6c Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Wed, 7 Oct 2015 12:23:08 -0700 Subject: [PATCH] Use MEMORY_BASIC_INFORMATION64 rather than a custom MemoryInfo We already use all the shared constants for page protection and type, so rather than making various incompatible structures, just use the MEMORY_BASIC_INFORMATION64 one directly, so that it can be directly used. R=mark@chromium.org BUG=crashpad:20, crashpad:46 Review URL: https://codereview.chromium.org/1375313005 . --- util/win/process_info.cc | 75 +++++++++--------- util/win/process_info.h | 40 +--------- util/win/process_info_test.cc | 143 ++++++++++++++++------------------ 3 files changed, 110 insertions(+), 148 deletions(-) diff --git a/util/win/process_info.cc b/util/win/process_info.cc index bfc83a60..fa413cc3 100644 --- a/util/win/process_info.cc +++ b/util/win/process_info.cc @@ -21,6 +21,7 @@ #include "base/logging.h" #include "base/strings/stringprintf.h" +#include "base/template_util.h" #include "build/build_config.h" #include "util/numeric/safe_assignment.h" #include "util/win/ntstatus_logging.h" @@ -107,10 +108,23 @@ bool ReadStruct(HANDLE process, WinVMAddress at, T* into) { return true; } -bool RegionIsAccessible(const ProcessInfo::MemoryInfo& memory_info) { - return memory_info.state == MEM_COMMIT && - (memory_info.protect & PAGE_NOACCESS) == 0 && - (memory_info.protect & PAGE_GUARD) == 0; +bool RegionIsAccessible(const MEMORY_BASIC_INFORMATION64& memory_info) { + return memory_info.State == MEM_COMMIT && + (memory_info.Protect & PAGE_NOACCESS) == 0 && + (memory_info.Protect & PAGE_GUARD) == 0; +} + +MEMORY_BASIC_INFORMATION64 MemoryBasicInformationToMemoryBasicInformation64( + const MEMORY_BASIC_INFORMATION& mbi) { + MEMORY_BASIC_INFORMATION64 mbi64 = {0}; + mbi64.BaseAddress = reinterpret_cast(mbi.BaseAddress); + mbi64.AllocationBase = reinterpret_cast(mbi.AllocationBase); + mbi64.AllocationProtect = mbi.AllocationProtect; + mbi64.RegionSize = mbi.RegionSize; + mbi64.State = mbi.State; + mbi64.Protect = mbi.Protect; + mbi64.Type = mbi.Type; + return mbi64; } } // namespace @@ -288,7 +302,8 @@ bool ReadMemoryInfo(HANDLE process, bool is_64_bit, ProcessInfo* process_info) { } process_info->memory_info_.push_back( - ProcessInfo::MemoryInfo(memory_basic_information)); + MemoryBasicInformationToMemoryBasicInformation64( + memory_basic_information)); if (memory_basic_information.RegionSize == 0) { LOG(ERROR) << "RegionSize == 0"; @@ -305,19 +320,6 @@ ProcessInfo::Module::Module() : name(), dll_base(0), size(0), timestamp() { ProcessInfo::Module::~Module() { } -ProcessInfo::MemoryInfo::MemoryInfo(const MEMORY_BASIC_INFORMATION& mbi) - : base_address(reinterpret_cast(mbi.BaseAddress)), - region_size(mbi.RegionSize), - allocation_base(reinterpret_cast(mbi.AllocationBase)), - state(mbi.State), - allocation_protect(mbi.AllocationProtect), - protect(mbi.Protect), - type(mbi.Type) { -} - -ProcessInfo::MemoryInfo::~MemoryInfo() { -} - ProcessInfo::ProcessInfo() : process_id_(), inherited_from_process_id_(), @@ -426,8 +428,7 @@ bool ProcessInfo::Modules(std::vector* modules) const { return true; } -const std::vector& ProcessInfo::MemoryInformation() - const { +const std::vector& ProcessInfo::MemoryInfo() const { INITIALIZATION_STATE_DCHECK_VALID(initialized_); return memory_info_; } @@ -435,39 +436,43 @@ const std::vector& ProcessInfo::MemoryInformation() std::vector> ProcessInfo::GetReadableRanges( const CheckedRange& range) const { - return GetReadableRangesOfMemoryMap(range, MemoryInformation()); + return GetReadableRangesOfMemoryMap(range, MemoryInfo()); } std::vector> GetReadableRangesOfMemoryMap( const CheckedRange& range, - const std::vector& memory_info) { + const std::vector& memory_info) { using Range = CheckedRange; // Find all the ranges that overlap the target range, maintaining their order. - std::vector overlapping; + std::vector overlapping; for (const auto& mi : memory_info) { - if (range.OverlapsRange(Range(mi.base_address, mi.region_size))) + static_assert(base::is_same::value, + "expected range address to be WinVMAddress"); + static_assert(base::is_same::value, + "expected range size to be WinVMSize"); + if (range.OverlapsRange(Range(mi.BaseAddress, mi.RegionSize))) overlapping.push_back(mi); } if (overlapping.empty()) return std::vector(); // For the first and last, trim to the boundary of the incoming range. - ProcessInfo::MemoryInfo& front = overlapping.front(); - WinVMAddress original_front_base_address = front.base_address; - front.base_address = std::max(front.base_address, range.base()); - front.region_size = - (original_front_base_address + front.region_size) - front.base_address; + MEMORY_BASIC_INFORMATION64& front = overlapping.front(); + WinVMAddress original_front_base_address = front.BaseAddress; + front.BaseAddress = std::max(front.BaseAddress, range.base()); + front.RegionSize = + (original_front_base_address + front.RegionSize) - front.BaseAddress; - ProcessInfo::MemoryInfo& back = overlapping.back(); - WinVMAddress back_end = back.base_address + back.region_size; - back.region_size = std::min(range.end(), back_end) - back.base_address; + MEMORY_BASIC_INFORMATION64& back = overlapping.back(); + WinVMAddress back_end = back.BaseAddress + back.RegionSize; + back.RegionSize = std::min(range.end(), back_end) - back.BaseAddress; // Discard all non-accessible. overlapping.erase(std::remove_if(overlapping.begin(), overlapping.end(), - [](const ProcessInfo::MemoryInfo& mi) { - return !RegionIsAccessible(mi); + [](const MEMORY_BASIC_INFORMATION64& mbi) { + return !RegionIsAccessible(mbi); }), overlapping.end()); if (overlapping.empty()) @@ -476,7 +481,7 @@ std::vector> GetReadableRangesOfMemoryMap( // Convert to return type. std::vector as_ranges; for (const auto& mi : overlapping) { - as_ranges.push_back(Range(mi.base_address, mi.region_size)); + as_ranges.push_back(Range(mi.BaseAddress, mi.RegionSize)); DCHECK(as_ranges.back().IsValid()); } diff --git a/util/win/process_info.h b/util/win/process_info.h index 9501f755..dac7b7c9 100644 --- a/util/win/process_info.h +++ b/util/win/process_info.h @@ -50,40 +50,6 @@ class ProcessInfo { 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(); @@ -128,7 +94,7 @@ class ProcessInfo { bool Modules(std::vector* modules) const; //! \brief Retrieves information about all pages mapped into the process. - const std::vector& MemoryInformation() const; + const std::vector& MemoryInfo() const; //! \brief Given a range to be read from the target process, returns a vector //! of ranges, representing the readable portions of the original range. @@ -162,7 +128,7 @@ class ProcessInfo { WinVMAddress peb_address_; WinVMSize peb_size_; std::vector modules_; - std::vector memory_info_; + std::vector memory_info_; bool is_64_bit_; bool is_wow64_; InitializationStateDcheck initialized_; @@ -178,7 +144,7 @@ class ProcessInfo { //! ProcessInfo::GetReadableRanges(). std::vector> GetReadableRangesOfMemoryMap( const CheckedRange& range, - const std::vector& memory_info); + const std::vector& memory_info); } // namespace crashpad diff --git a/util/win/process_info_test.cc b/util/win/process_info_test.cc index 71269a59..ce9e8ac7 100644 --- a/util/win/process_info_test.cc +++ b/util/win/process_info_test.cc @@ -14,7 +14,7 @@ #include "util/win/process_info.h" -#include +#include #include #include @@ -34,15 +34,6 @@ namespace { const wchar_t kNtdllName[] = L"\\ntdll.dll"; -time_t GetTimestampForModule(HMODULE module) { - char filename[MAX_PATH]; - // `char` and GetModuleFileNameA because ImageLoad is ANSI only. - if (!GetModuleFileNameA(module, filename, arraysize(filename))) - return 0; - LOADED_IMAGE* loaded_image = ImageLoad(filename, nullptr); - return loaded_image->FileHeader->FileHeader.TimeDateStamp; -} - bool IsProcessWow64(HANDLE process_handle) { static decltype(IsWow64Process)* is_wow64_process = reinterpret_cast( @@ -61,15 +52,15 @@ void VerifyAddressInInCodePage(const ProcessInfo& process_info, WinVMAddress code_address) { // Make sure the child code address is an code page address with the right // information. - const std::vector& memory_info = - process_info.MemoryInformation(); + const std::vector& memory_info = + process_info.MemoryInfo(); bool found_region = false; for (const auto& mi : memory_info) { - if (mi.base_address <= code_address && - mi.base_address + mi.region_size > code_address) { - EXPECT_EQ(MEM_COMMIT, mi.state); - EXPECT_EQ(PAGE_EXECUTE_READ, mi.protect); - EXPECT_EQ(MEM_IMAGE, mi.type); + if (mi.BaseAddress <= code_address && + mi.BaseAddress + mi.RegionSize > code_address) { + EXPECT_EQ(MEM_COMMIT, mi.State); + EXPECT_EQ(PAGE_EXECUTE_READ, mi.Protect); + EXPECT_EQ(MEM_IMAGE, mi.Type); EXPECT_FALSE(found_region); found_region = true; } @@ -110,16 +101,16 @@ TEST(ProcessInfo, Self) { kNtdllName, modules[1].name.substr(modules[1].name.size() - wcslen(kNtdllName))); - EXPECT_EQ(modules[0].dll_base, - reinterpret_cast(GetModuleHandle(nullptr))); - EXPECT_EQ(modules[1].dll_base, - reinterpret_cast(GetModuleHandle(L"ntdll.dll"))); + EXPECT_EQ(reinterpret_cast(GetModuleHandle(nullptr)), + modules[0].dll_base); + EXPECT_EQ(reinterpret_cast(GetModuleHandle(L"ntdll.dll")), + modules[1].dll_base); EXPECT_GT(modules[0].size, 0); EXPECT_GT(modules[1].size, 0); - EXPECT_EQ(modules[0].timestamp, - GetTimestampForModule(GetModuleHandle(nullptr))); + EXPECT_EQ(GetTimestampForLoadedLibrary(GetModuleHandle(nullptr)), + modules[0].timestamp); // System modules are forced to particular stamps and the file header values // don't match the on-disk times. Just make sure we got some data here. EXPECT_GT(modules[1].timestamp, 0); @@ -201,13 +192,13 @@ TEST(ProcessInfo, OtherProcessWOW64) { #endif // ARCH_CPU_64_BITS TEST(ProcessInfo, AccessibleRangesNone) { - std::vector memory_info; - MEMORY_BASIC_INFORMATION mbi = {0}; + std::vector memory_info; + MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; mbi.RegionSize = 10; mbi.State = MEM_FREE; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); std::vector> result = GetReadableRangesOfMemoryMap(CheckedRange(2, 4), @@ -217,13 +208,13 @@ TEST(ProcessInfo, AccessibleRangesNone) { } TEST(ProcessInfo, AccessibleRangesOneInside) { - std::vector memory_info; - MEMORY_BASIC_INFORMATION mbi = {0}; + std::vector memory_info; + MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; mbi.RegionSize = 10; mbi.State = MEM_COMMIT; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); std::vector> result = GetReadableRangesOfMemoryMap(CheckedRange(2, 4), @@ -235,18 +226,18 @@ TEST(ProcessInfo, AccessibleRangesOneInside) { } TEST(ProcessInfo, AccessibleRangesOneTruncatedSize) { - std::vector memory_info; - MEMORY_BASIC_INFORMATION mbi = {0}; + std::vector memory_info; + MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; mbi.RegionSize = 10; mbi.State = MEM_COMMIT; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); - mbi.BaseAddress = reinterpret_cast(10); + mbi.BaseAddress = 10; mbi.RegionSize = 20; mbi.State = MEM_FREE; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); std::vector> result = GetReadableRangesOfMemoryMap(CheckedRange(5, 10), @@ -258,18 +249,18 @@ TEST(ProcessInfo, AccessibleRangesOneTruncatedSize) { } TEST(ProcessInfo, AccessibleRangesOneMovedStart) { - std::vector memory_info; - MEMORY_BASIC_INFORMATION mbi = {0}; + std::vector memory_info; + MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; mbi.RegionSize = 10; mbi.State = MEM_FREE; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); - mbi.BaseAddress = reinterpret_cast(10); + mbi.BaseAddress = 10; mbi.RegionSize = 20; mbi.State = MEM_COMMIT; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); std::vector> result = GetReadableRangesOfMemoryMap(CheckedRange(5, 10), @@ -281,18 +272,18 @@ TEST(ProcessInfo, AccessibleRangesOneMovedStart) { } TEST(ProcessInfo, ReserveIsInaccessible) { - std::vector memory_info; - MEMORY_BASIC_INFORMATION mbi = {0}; + std::vector memory_info; + MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; mbi.RegionSize = 10; mbi.State = MEM_RESERVE; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); - mbi.BaseAddress = reinterpret_cast(10); + mbi.BaseAddress = 10; mbi.RegionSize = 20; mbi.State = MEM_COMMIT; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); std::vector> result = GetReadableRangesOfMemoryMap(CheckedRange(5, 10), @@ -304,20 +295,20 @@ TEST(ProcessInfo, ReserveIsInaccessible) { } TEST(ProcessInfo, PageGuardIsInaccessible) { - std::vector memory_info; - MEMORY_BASIC_INFORMATION mbi = {0}; + std::vector memory_info; + MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; mbi.RegionSize = 10; mbi.State = MEM_COMMIT; mbi.Protect = PAGE_GUARD; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); - mbi.BaseAddress = reinterpret_cast(10); + mbi.BaseAddress = 10; mbi.RegionSize = 20; mbi.State = MEM_COMMIT; mbi.Protect = 0; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); std::vector> result = GetReadableRangesOfMemoryMap(CheckedRange(5, 10), @@ -329,20 +320,20 @@ TEST(ProcessInfo, PageGuardIsInaccessible) { } TEST(ProcessInfo, PageNoAccessIsInaccessible) { - std::vector memory_info; - MEMORY_BASIC_INFORMATION mbi = {0}; + std::vector memory_info; + MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; mbi.RegionSize = 10; mbi.State = MEM_COMMIT; mbi.Protect = PAGE_NOACCESS; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); - mbi.BaseAddress = reinterpret_cast(10); + mbi.BaseAddress = 10; mbi.RegionSize = 20; mbi.State = MEM_COMMIT; mbi.Protect = 0; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); std::vector> result = GetReadableRangesOfMemoryMap(CheckedRange(5, 10), @@ -354,23 +345,23 @@ TEST(ProcessInfo, PageNoAccessIsInaccessible) { } TEST(ProcessInfo, AccessibleRangesCoalesced) { - std::vector memory_info; - MEMORY_BASIC_INFORMATION mbi = {0}; + std::vector memory_info; + MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; mbi.RegionSize = 10; mbi.State = MEM_FREE; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); - mbi.BaseAddress = reinterpret_cast(10); + mbi.BaseAddress = 10; mbi.RegionSize = 2; mbi.State = MEM_COMMIT; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); - mbi.BaseAddress = reinterpret_cast(12); + mbi.BaseAddress = 12; mbi.RegionSize = 5; mbi.State = MEM_COMMIT; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); std::vector> result = GetReadableRangesOfMemoryMap(CheckedRange(11, 4), @@ -382,23 +373,23 @@ TEST(ProcessInfo, AccessibleRangesCoalesced) { } TEST(ProcessInfo, AccessibleRangesMiddleUnavailable) { - std::vector memory_info; - MEMORY_BASIC_INFORMATION mbi = {0}; + std::vector memory_info; + MEMORY_BASIC_INFORMATION64 mbi = {0}; mbi.BaseAddress = 0; mbi.RegionSize = 10; mbi.State = MEM_COMMIT; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); - mbi.BaseAddress = reinterpret_cast(10); + mbi.BaseAddress = 10; mbi.RegionSize = 5; mbi.State = MEM_FREE; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); - mbi.BaseAddress = reinterpret_cast(15); + mbi.BaseAddress = 15; mbi.RegionSize = 100; mbi.State = MEM_COMMIT; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); std::vector> result = GetReadableRangesOfMemoryMap(CheckedRange(5, 45), @@ -412,13 +403,13 @@ TEST(ProcessInfo, AccessibleRangesMiddleUnavailable) { } TEST(ProcessInfo, RequestedBeforeMap) { - std::vector memory_info; - MEMORY_BASIC_INFORMATION mbi = {0}; + std::vector memory_info; + MEMORY_BASIC_INFORMATION64 mbi = {0}; - mbi.BaseAddress = reinterpret_cast(10); + mbi.BaseAddress = 10; mbi.RegionSize = 10; mbi.State = MEM_COMMIT; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); std::vector> result = GetReadableRangesOfMemoryMap(CheckedRange(5, 10), @@ -430,13 +421,13 @@ TEST(ProcessInfo, RequestedBeforeMap) { } TEST(ProcessInfo, RequestedAfterMap) { - std::vector memory_info; - MEMORY_BASIC_INFORMATION mbi = {0}; + std::vector memory_info; + MEMORY_BASIC_INFORMATION64 mbi = {0}; - mbi.BaseAddress = reinterpret_cast(10); + mbi.BaseAddress = 10; mbi.RegionSize = 10; mbi.State = MEM_COMMIT; - memory_info.push_back(ProcessInfo::MemoryInfo(mbi)); + memory_info.push_back(mbi); std::vector> result = GetReadableRangesOfMemoryMap(