Refactor TaskMemory initialization

Currently, TaskMemory implements the ProcessMemory interface almost
exactly; however, it's initialized using a constructor instead of an
Initialize method which makes it incompatible with a number of
ProcessMemory tests. Change its initialization to match the other
ProcessMemory classes.

Bug: crashpad:263
Change-Id: I8022dc3e1827a5bb398aace0058ce9494b6b6eb6
Reviewed-on: https://chromium-review.googlesource.com/c/1384447
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
This commit is contained in:
Vlad Tsyrklevich 2018-12-19 13:49:55 -08:00 committed by Commit Bot
parent ffd666e16c
commit bf6d2e0283
5 changed files with 63 additions and 22 deletions

View File

@ -113,9 +113,11 @@ bool ProcessReaderMac::Initialize(task_t task) {
return false;
}
is_64_bit_ = process_info_.Is64Bit();
if (!task_memory_.Initialize(task)) {
return false;
}
task_memory_.reset(new TaskMemory(task));
is_64_bit_ = process_info_.Is64Bit();
task_ = task;
INITIALIZATION_STATE_SET_VALID(initialized_);
@ -441,7 +443,7 @@ void ProcessReaderMac::InitializeModules() {
Module module;
module.timestamp = image_info.imageFileModDate;
if (!task_memory_->ReadCString(image_info.imageFilePath, &module.name)) {
if (!task_memory_.ReadCString(image_info.imageFilePath, &module.name)) {
LOG(WARNING) << "could not read dyld_image_info::imageFilePath";
// Proceed anyway with an empty module name.
}

View File

@ -138,7 +138,7 @@ class ProcessReaderMac {
bool CPUTimes(timeval* user_time, timeval* system_time) const;
//! \return Accesses the memory of the target task.
TaskMemory* Memory() { return task_memory_.get(); }
TaskMemory* Memory() { return &task_memory_; }
//! \return The threads that are in the task (process). The first element (at
//! index `0`) corresponds to the main thread.
@ -232,7 +232,7 @@ class ProcessReaderMac {
std::vector<Thread> threads_; // owns send rights
std::vector<Module> modules_;
std::vector<std::unique_ptr<MachOImageReader>> module_readers_;
std::unique_ptr<TaskMemory> task_memory_;
TaskMemory task_memory_;
task_t task_; // weak
InitializationStateDcheck initialized_;

View File

@ -64,10 +64,18 @@ TaskMemory::MappedMemory::MappedMemory(vm_address_t vm_address,
DCHECK_LE(user_end, vm_end);
}
TaskMemory::TaskMemory(task_t task) : task_(task) {
TaskMemory::TaskMemory() : task_(TASK_NULL), initialized_() {}
bool TaskMemory::Initialize(task_t task) {
INITIALIZATION_STATE_SET_INITIALIZING(initialized_);
task_ = task;
INITIALIZATION_STATE_SET_VALID(initialized_);
return true;
}
bool TaskMemory::Read(mach_vm_address_t address, size_t size, void* buffer) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
std::unique_ptr<MappedMemory> memory = ReadMapped(address, size);
if (!memory) {
return false;
@ -80,6 +88,8 @@ bool TaskMemory::Read(mach_vm_address_t address, size_t size, void* buffer) {
std::unique_ptr<TaskMemory::MappedMemory> TaskMemory::ReadMapped(
mach_vm_address_t address,
size_t size) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
if (size == 0) {
return std::unique_ptr<MappedMemory>(new MappedMemory(0, 0, 0, 0));
}
@ -104,12 +114,16 @@ std::unique_ptr<TaskMemory::MappedMemory> TaskMemory::ReadMapped(
}
bool TaskMemory::ReadCString(mach_vm_address_t address, std::string* string) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
return ReadCStringInternal(address, false, 0, string);
}
bool TaskMemory::ReadCStringSizeLimited(mach_vm_address_t address,
mach_vm_size_t size,
std::string* string) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
return ReadCStringInternal(address, true, size, string);
}
@ -117,6 +131,8 @@ bool TaskMemory::ReadCStringInternal(mach_vm_address_t address,
bool has_size,
mach_vm_size_t size,
std::string* string) {
INITIALIZATION_STATE_DCHECK_VALID(initialized_);
if (!has_size) {
size = PAGE_SIZE;
}

View File

@ -23,6 +23,7 @@
#include "base/mac/scoped_mach_vm.h"
#include "base/macros.h"
#include "util/misc/initialization_state_dcheck.h"
namespace crashpad {
@ -87,12 +88,21 @@ class TaskMemory {
DISALLOW_COPY_AND_ASSIGN(MappedMemory);
};
//! \param[in] task A send right to the target tasks task port. This object
//! does not take ownership of the send right.
explicit TaskMemory(task_t task);
TaskMemory();
~TaskMemory() {}
//! \brief Initializes this object to read the memory of a task with the
//! provided task port.
//!
//! This method must be called successfully prior to calling any other method
//! in this class.
//!
//! \param[in] task A send right to the target task's task port. This object
//! does not take ownership of the send right.
//!
//! \return `true` on success, `false` on failure with a message logged.
bool Initialize(task_t task);
//! \brief Copies memory from the target task into a caller-provided buffer in
//! the current task.
//!
@ -170,6 +180,7 @@ class TaskMemory {
std::string* string);
task_t task_; // weak
InitializationStateDcheck initialized_;
DISALLOW_COPY_AND_ASSIGN(TaskMemory);
};

View File

@ -44,7 +44,8 @@ TEST(TaskMemory, ReadSelf) {
region[index] = (index % 256) ^ ((index >> 8) % 256);
}
TaskMemory memory(mach_task_self());
TaskMemory memory;
ASSERT_TRUE(memory.Initialize(mach_task_self()));
// This tests using both the Read() and ReadMapped() interfaces.
std::string result(kSize, '\0');
@ -119,7 +120,8 @@ TEST(TaskMemory, ReadSelfUnmapped) {
mach_task_self(), address + PAGE_SIZE, PAGE_SIZE, FALSE, VM_PROT_NONE);
ASSERT_EQ(kr, KERN_SUCCESS) << MachErrorMessage(kr, "vm_protect");
TaskMemory memory(mach_task_self());
TaskMemory memory;
ASSERT_TRUE(memory.Initialize(mach_task_self()));
std::string result(kSize, '\0');
EXPECT_FALSE(memory.Read(address, kSize, &result[0]));
@ -171,7 +173,8 @@ bool ReadCStringSelf(TaskMemory* memory,
}
TEST(TaskMemory, ReadCStringSelf) {
TaskMemory memory(mach_task_self());
TaskMemory memory;
ASSERT_TRUE(memory.Initialize(mach_task_self()));
std::string result;
const char kConstCharEmpty[] = "";
@ -253,7 +256,8 @@ TEST(TaskMemory, ReadCStringSelfUnmapped) {
mach_task_self(), address + PAGE_SIZE, PAGE_SIZE, FALSE, VM_PROT_NONE);
ASSERT_EQ(kr, KERN_SUCCESS) << MachErrorMessage(kr, "vm_protect");
TaskMemory memory(mach_task_self());
TaskMemory memory;
ASSERT_TRUE(memory.Initialize(mach_task_self()));
std::string result;
EXPECT_FALSE(memory.ReadCString(address, &result));
@ -298,7 +302,8 @@ bool ReadCStringSizeLimitedSelf(TaskMemory* memory,
}
TEST(TaskMemory, ReadCStringSizeLimited_ConstCharEmpty) {
TaskMemory memory(mach_task_self());
TaskMemory memory;
ASSERT_TRUE(memory.Initialize(mach_task_self()));
std::string result;
static constexpr char kConstCharEmpty[] = "";
@ -319,7 +324,8 @@ TEST(TaskMemory, ReadCStringSizeLimited_ConstCharEmpty) {
}
TEST(TaskMemory, ReadCStringSizeLimited_ConstCharShort) {
TaskMemory memory(mach_task_self());
TaskMemory memory;
ASSERT_TRUE(memory.Initialize(mach_task_self()));
std::string result;
static constexpr char kConstCharShort[] = "A short const char[]";
@ -339,7 +345,8 @@ TEST(TaskMemory, ReadCStringSizeLimited_ConstCharShort) {
}
TEST(TaskMemory, ReadCStringSizeLimited_StaticConstCharEmpty) {
TaskMemory memory(mach_task_self());
TaskMemory memory;
ASSERT_TRUE(memory.Initialize(mach_task_self()));
std::string result;
static constexpr char kStaticConstCharEmpty[] = "";
@ -364,7 +371,8 @@ TEST(TaskMemory, ReadCStringSizeLimited_StaticConstCharEmpty) {
}
TEST(TaskMemory, ReadCStringSizeLimited_StaticConstCharShort) {
TaskMemory memory(mach_task_self());
TaskMemory memory;
ASSERT_TRUE(memory.Initialize(mach_task_self()));
std::string result;
static constexpr char kStaticConstCharShort[] =
@ -391,7 +399,8 @@ TEST(TaskMemory, ReadCStringSizeLimited_StaticConstCharShort) {
}
TEST(TaskMemory, ReadCStringSizeLimited_StringShort) {
TaskMemory memory(mach_task_self());
TaskMemory memory;
ASSERT_TRUE(memory.Initialize(mach_task_self()));
std::string result;
std::string string_short("A short std::string in a function");
@ -411,7 +420,8 @@ TEST(TaskMemory, ReadCStringSizeLimited_StringShort) {
}
TEST(TaskMemory, ReadCStringSizeLimited_StringLong) {
TaskMemory memory(mach_task_self());
TaskMemory memory;
ASSERT_TRUE(memory.Initialize(mach_task_self()));
std::string result;
std::string string_long;
@ -477,7 +487,8 @@ TEST(TaskMemory, MappedMemoryDeallocates) {
// hopefully there are either no other threads or theyre all quiescent, so
// nothing else should wind up mapped in the address.
TaskMemory memory(mach_task_self());
TaskMemory memory;
ASSERT_TRUE(memory.Initialize(mach_task_self()));
std::unique_ptr<TaskMemory::MappedMemory> mapped;
static constexpr char kTestBuffer[] = "hello!";
@ -514,7 +525,8 @@ TEST(TaskMemory, MappedMemoryDeallocates) {
TEST(TaskMemory, MappedMemoryReadCString) {
// This tests the behavior of TaskMemory::MappedMemory::ReadCString().
TaskMemory memory(mach_task_self());
TaskMemory memory;
ASSERT_TRUE(memory.Initialize(mach_task_self()));
std::unique_ptr<TaskMemory::MappedMemory> mapped;
static constexpr char kTestBuffer[] = "0\0" "2\0" "45\0" "789";