Have MinidumpMemoryListWriter deal directly in SnapshotMinidumpMemoryWriters

This is as a precursor to
https://chromium-review.googlesource.com/374539 which merges
MemorySnapshots and so needs to be able to update them from the minidump
code.

MinidumpMemoryWriter existed to be able to mock for tests; that
behaviour is wrapped up in TestMemorySnapshot now.

BUG=crashpad:61, chromium:638370

Change-Id: I825ec57493b12fc1848018585c14544faa7e66d4
Reviewed-on: https://chromium-review.googlesource.com/374019
Reviewed-by: Mark Mentovai <mark@chromium.org>
This commit is contained in:
Scott Graham 2016-08-25 15:01:27 -07:00
parent 357c7c7b7b
commit c6f88d164e
6 changed files with 87 additions and 189 deletions

View File

@ -24,77 +24,45 @@
#include "util/numeric/safe_assignment.h"
namespace crashpad {
namespace {
class SnapshotMinidumpMemoryWriter final : public MinidumpMemoryWriter,
public MemorySnapshot::Delegate {
public:
explicit SnapshotMinidumpMemoryWriter(const MemorySnapshot* memory_snapshot)
: MinidumpMemoryWriter(),
MemorySnapshot::Delegate(),
memory_snapshot_(memory_snapshot),
file_writer_(nullptr) {
}
SnapshotMinidumpMemoryWriter::SnapshotMinidumpMemoryWriter(
const MemorySnapshot* memory_snapshot)
: internal::MinidumpWritable(),
MemorySnapshot::Delegate(),
memory_descriptor_(),
registered_memory_descriptors_(),
memory_snapshot_(memory_snapshot),
file_writer_(nullptr) {}
~SnapshotMinidumpMemoryWriter() override {}
SnapshotMinidumpMemoryWriter::~SnapshotMinidumpMemoryWriter() {}
// MemorySnapshot::Delegate:
bool MemorySnapshotDelegateRead(void* data, size_t size) override {
DCHECK_EQ(state(), kStateWritable);
DCHECK_EQ(size, MemoryRangeSize());
return file_writer_->Write(data, size);
}
protected:
// MinidumpMemoryWriter:
bool WriteObject(FileWriterInterface* file_writer) override {
DCHECK_EQ(state(), kStateWritable);
DCHECK(!file_writer_);
base::AutoReset<FileWriterInterface*> file_writer_reset(&file_writer_,
file_writer);
// This will result in MemorySnapshotDelegateRead() being called.
return memory_snapshot_->Read(this);
}
uint64_t MemoryRangeBaseAddress() const override {
DCHECK_EQ(state(), kStateFrozen);
return memory_snapshot_->Address();
}
size_t MemoryRangeSize() const override {
DCHECK_GE(state(), kStateFrozen);
return memory_snapshot_->Size();
}
private:
const MemorySnapshot* memory_snapshot_;
FileWriterInterface* file_writer_;
DISALLOW_COPY_AND_ASSIGN(SnapshotMinidumpMemoryWriter);
};
} // namespace
MinidumpMemoryWriter::~MinidumpMemoryWriter() {
bool SnapshotMinidumpMemoryWriter::MemorySnapshotDelegateRead(void* data,
size_t size) {
DCHECK_EQ(state(), kStateWritable);
DCHECK_EQ(size, UnderlyingSnapshot().Size());
return file_writer_->Write(data, size);
}
std::unique_ptr<MinidumpMemoryWriter> MinidumpMemoryWriter::CreateFromSnapshot(
const MemorySnapshot* memory_snapshot) {
return base::WrapUnique(new SnapshotMinidumpMemoryWriter(memory_snapshot));
bool SnapshotMinidumpMemoryWriter::WriteObject(
FileWriterInterface* file_writer) {
DCHECK_EQ(state(), kStateWritable);
DCHECK(!file_writer_);
base::AutoReset<FileWriterInterface*> file_writer_reset(&file_writer_,
file_writer);
// This will result in MemorySnapshotDelegateRead() being called.
return memory_snapshot_->Read(this);
}
const MINIDUMP_MEMORY_DESCRIPTOR*
MinidumpMemoryWriter::MinidumpMemoryDescriptor() const {
SnapshotMinidumpMemoryWriter::MinidumpMemoryDescriptor() const {
DCHECK_EQ(state(), kStateWritable);
return &memory_descriptor_;
}
void MinidumpMemoryWriter::RegisterMemoryDescriptor(
void SnapshotMinidumpMemoryWriter::RegisterMemoryDescriptor(
MINIDUMP_MEMORY_DESCRIPTOR* memory_descriptor) {
DCHECK_LE(state(), kStateFrozen);
@ -102,13 +70,7 @@ void MinidumpMemoryWriter::RegisterMemoryDescriptor(
RegisterLocationDescriptor(&memory_descriptor->Memory);
}
MinidumpMemoryWriter::MinidumpMemoryWriter()
: MinidumpWritable(),
memory_descriptor_(),
registered_memory_descriptors_() {
}
bool MinidumpMemoryWriter::Freeze() {
bool SnapshotMinidumpMemoryWriter::Freeze() {
DCHECK_EQ(state(), kStateMutable);
if (!MinidumpWritable::Freeze()) {
@ -120,26 +82,26 @@ bool MinidumpMemoryWriter::Freeze() {
return true;
}
size_t MinidumpMemoryWriter::Alignment() {
size_t SnapshotMinidumpMemoryWriter::Alignment() {
DCHECK_GE(state(), kStateFrozen);
return 16;
}
size_t MinidumpMemoryWriter::SizeOfObject() {
size_t SnapshotMinidumpMemoryWriter::SizeOfObject() {
DCHECK_GE(state(), kStateFrozen);
return MemoryRangeSize();
return UnderlyingSnapshot().Size();
}
bool MinidumpMemoryWriter::WillWriteAtOffsetImpl(FileOffset offset) {
bool SnapshotMinidumpMemoryWriter::WillWriteAtOffsetImpl(FileOffset offset) {
DCHECK_EQ(state(), kStateFrozen);
// There will always be at least one registered descriptor, the one for this
// objects own memory_descriptor_ field.
DCHECK_GE(registered_memory_descriptors_.size(), 1u);
uint64_t base_address = MemoryRangeBaseAddress();
uint64_t base_address = UnderlyingSnapshot().Address();
decltype(registered_memory_descriptors_[0]->StartOfMemoryRange) local_address;
if (!AssignIfInRange(&local_address, base_address)) {
LOG(ERROR) << "base_address " << base_address << " out of range";
@ -154,7 +116,7 @@ bool MinidumpMemoryWriter::WillWriteAtOffsetImpl(FileOffset offset) {
return MinidumpWritable::WillWriteAtOffsetImpl(offset);
}
internal::MinidumpWritable::Phase MinidumpMemoryWriter::WritePhase() {
internal::MinidumpWritable::Phase SnapshotMinidumpMemoryWriter::WritePhase() {
// Memory dumps are large and are unlikely to be consumed in their entirety.
// Data accesses are expected to be sparse and sporadic, and are expected to
// occur after all of the other structural and informational data from the
@ -178,14 +140,14 @@ void MinidumpMemoryListWriter::AddFromSnapshot(
DCHECK_EQ(state(), kStateMutable);
for (const MemorySnapshot* memory_snapshot : memory_snapshots) {
std::unique_ptr<MinidumpMemoryWriter> memory =
MinidumpMemoryWriter::CreateFromSnapshot(memory_snapshot);
std::unique_ptr<SnapshotMinidumpMemoryWriter> memory(
new SnapshotMinidumpMemoryWriter(memory_snapshot));
AddMemory(std::move(memory));
}
}
void MinidumpMemoryListWriter::AddMemory(
std::unique_ptr<MinidumpMemoryWriter> memory_writer) {
std::unique_ptr<SnapshotMinidumpMemoryWriter> memory_writer) {
DCHECK_EQ(state(), kStateMutable);
AddExtraMemory(memory_writer.get());
@ -193,7 +155,7 @@ void MinidumpMemoryListWriter::AddMemory(
}
void MinidumpMemoryListWriter::AddExtraMemory(
MinidumpMemoryWriter* memory_writer) {
SnapshotMinidumpMemoryWriter* memory_writer) {
DCHECK_EQ(state(), kStateMutable);
memory_writers_.push_back(memory_writer);
@ -232,7 +194,7 @@ std::vector<internal::MinidumpWritable*> MinidumpMemoryListWriter::Children() {
DCHECK_LE(children_.size(), memory_writers_.size());
std::vector<MinidumpWritable*> children;
for (MinidumpMemoryWriter* child : children_) {
for (SnapshotMinidumpMemoryWriter* child : children_) {
children.push_back(child);
}
@ -247,7 +209,7 @@ bool MinidumpMemoryListWriter::WriteObject(FileWriterInterface* file_writer) {
iov.iov_len = sizeof(memory_list_base_);
std::vector<WritableIoVec> iovecs(1, iov);
for (const MinidumpMemoryWriter* memory_writer : memory_writers_) {
for (const SnapshotMinidumpMemoryWriter* memory_writer : memory_writers_) {
iov.iov_base = memory_writer->MinidumpMemoryDescriptor();
iov.iov_len = sizeof(MINIDUMP_MEMORY_DESCRIPTOR);
iovecs.push_back(iov);

View File

@ -26,34 +26,19 @@
#include "base/macros.h"
#include "minidump/minidump_stream_writer.h"
#include "minidump/minidump_writable.h"
#include "snapshot/memory_snapshot.h"
#include "util/file/file_io.h"
#include "util/stdlib/pointer_container.h"
namespace crashpad {
class MemorySnapshot;
//! \brief The base class for writers of memory ranges pointed to by
//! MINIDUMP_MEMORY_DESCRIPTOR objects in a minidump file.
//!
//! This is an abstract base class because users are expected to provide their
//! own implementations that, when possible, obtain the memory contents
//! on-demand in their WriteObject() methods. Memory ranges may be large, and
//! the alternative construction would require the contents of multiple ranges
//! to be held in memory simultaneously while a minidump file is being written.
class MinidumpMemoryWriter : public internal::MinidumpWritable {
class SnapshotMinidumpMemoryWriter : public internal::MinidumpWritable,
public MemorySnapshot::Delegate {
public:
~MinidumpMemoryWriter() override;
//! \brief Creates a concrete initialized MinidumpMemoryWriter based on \a
//! memory_snapshot.
//!
//! \param[in] memory_snapshot The memory snapshot to use as source data.
//!
//! \return An object of a MinidumpMemoryWriter subclass initialized using the
//! source data in \a memory_snapshot.
static std::unique_ptr<MinidumpMemoryWriter> CreateFromSnapshot(
const MemorySnapshot* memory_snapshot);
explicit SnapshotMinidumpMemoryWriter(const MemorySnapshot* memory_snapshot);
~SnapshotMinidumpMemoryWriter() override;
//! \brief Returns a MINIDUMP_MEMORY_DESCRIPTOR referencing the data that this
//! object writes.
@ -76,24 +61,14 @@ class MinidumpMemoryWriter : public internal::MinidumpWritable {
//! \note Valid in #kStateFrozen or any preceding state.
void RegisterMemoryDescriptor(MINIDUMP_MEMORY_DESCRIPTOR* memory_descriptor);
protected:
MinidumpMemoryWriter();
//! \brief Returns the base address of the memory region in the address space
//! of the process that the snapshot describes.
//!
//! \note This method will only be called in #kStateFrozen.
virtual uint64_t MemoryRangeBaseAddress() const = 0;
//! \brief Returns the size of the memory region in bytes.
//!
//! \note This method will only be called in #kStateFrozen or a subsequent
//! state.
virtual size_t MemoryRangeSize() const = 0;
private:
// MemorySnapshot::Delegate:
bool MemorySnapshotDelegateRead(void* data, size_t size) override;
// MinidumpWritable:
bool Freeze() override;
size_t SizeOfObject() final;
bool WriteObject(FileWriterInterface* file_writer) override;
//! \brief Returns the objects desired byte-boundary alignment.
//!
@ -119,13 +94,18 @@ class MinidumpMemoryWriter : public internal::MinidumpWritable {
//! \note Valid in any state.
Phase WritePhase() final;
private:
//! \brief Gets the underlying memory snapshot that the memory writer will
//! write to the minidump.
const MemorySnapshot& UnderlyingSnapshot() const { return *memory_snapshot_; }
MINIDUMP_MEMORY_DESCRIPTOR memory_descriptor_;
// weak
std::vector<MINIDUMP_MEMORY_DESCRIPTOR*> registered_memory_descriptors_;
const MemorySnapshot* memory_snapshot_;
FileWriterInterface* file_writer_;
DISALLOW_COPY_AND_ASSIGN(MinidumpMemoryWriter);
DISALLOW_COPY_AND_ASSIGN(SnapshotMinidumpMemoryWriter);
};
//! \brief The writer for a MINIDUMP_MEMORY_LIST stream in a minidump file,
@ -135,8 +115,8 @@ class MinidumpMemoryListWriter final : public internal::MinidumpStreamWriter {
MinidumpMemoryListWriter();
~MinidumpMemoryListWriter() override;
//! \brief Adds a concrete initialized MinidumpMemoryWriter for each memory
//! snapshot in \a memory_snapshots to the MINIDUMP_MEMORY_LIST.
//! \brief Adds a concrete initialized SnapshotMinidumpMemoryWriter for each
//! memory snapshot in \a memory_snapshots to the MINIDUMP_MEMORY_LIST.
//!
//! Memory snapshots are added in the fashion of AddMemory().
//!
@ -146,15 +126,15 @@ class MinidumpMemoryListWriter final : public internal::MinidumpStreamWriter {
void AddFromSnapshot(
const std::vector<const MemorySnapshot*>& memory_snapshots);
//! \brief Adds a MinidumpMemoryWriter to the MINIDUMP_MEMORY_LIST.
//! \brief Adds a SnapshotMinidumpMemoryWriter to the MINIDUMP_MEMORY_LIST.
//!
//! This object takes ownership of \a memory_writer and becomes its parent in
//! the overall tree of internal::MinidumpWritable objects.
//!
//! \note Valid in #kStateMutable.
void AddMemory(std::unique_ptr<MinidumpMemoryWriter> memory_writer);
void AddMemory(std::unique_ptr<SnapshotMinidumpMemoryWriter> memory_writer);
//! \brief Adds a MinidumpMemoryWriter thats a child of another
//! \brief Adds a SnapshotMinidumpMemoryWriter thats a child of another
//! internal::MinidumpWritable object to the MINIDUMP_MEMORY_LIST.
//!
//! \a memory_writer does not become a child of this object, but the
@ -163,12 +143,12 @@ class MinidumpMemoryListWriter final : public internal::MinidumpStreamWriter {
//! internal::MinidumpWritable tree.
//!
//! This method exists to be called by objects that have their own
//! MinidumpMemoryWriter children but wish for them to also appear in the
//! minidump files MINIDUMP_MEMORY_LIST. MinidumpThreadWriter, which has a
//! MinidumpMemoryWriter for thread stack memory, is an example.
//! SnapshotMinidumpMemoryWriter children but wish for them to also appear in
//! the minidump files MINIDUMP_MEMORY_LIST. MinidumpThreadWriter, which has
//! a SnapshotMinidumpMemoryWriter for thread stack memory, is an example.
//!
//! \note Valid in #kStateMutable.
void AddExtraMemory(MinidumpMemoryWriter* memory_writer);
void AddExtraMemory(SnapshotMinidumpMemoryWriter* memory_writer);
protected:
// MinidumpWritable:
@ -181,8 +161,8 @@ class MinidumpMemoryListWriter final : public internal::MinidumpStreamWriter {
MinidumpStreamType StreamType() const override;
private:
std::vector<MinidumpMemoryWriter*> memory_writers_; // weak
PointerVector<MinidumpMemoryWriter> children_;
std::vector<SnapshotMinidumpMemoryWriter*> memory_writers_; // weak
PointerVector<SnapshotMinidumpMemoryWriter> children_;
MINIDUMP_MEMORY_LIST memory_list_base_;
DISALLOW_COPY_AND_ASSIGN(MinidumpMemoryListWriter);

View File

@ -51,8 +51,8 @@ void MinidumpThreadWriter::InitializeFromSnapshot(
const MemorySnapshot* stack_snapshot = thread_snapshot->Stack();
if (stack_snapshot && stack_snapshot->Size() > 0) {
std::unique_ptr<MinidumpMemoryWriter> stack =
MinidumpMemoryWriter::CreateFromSnapshot(stack_snapshot);
std::unique_ptr<SnapshotMinidumpMemoryWriter> stack(
new SnapshotMinidumpMemoryWriter(stack_snapshot));
SetStack(std::move(stack));
}
@ -68,7 +68,7 @@ const MINIDUMP_THREAD* MinidumpThreadWriter::MinidumpThread() const {
}
void MinidumpThreadWriter::SetStack(
std::unique_ptr<MinidumpMemoryWriter> stack) {
std::unique_ptr<SnapshotMinidumpMemoryWriter> stack) {
DCHECK_EQ(state(), kStateMutable);
stack_ = std::move(stack);
@ -172,7 +172,7 @@ void MinidumpThreadListWriter::AddThread(
DCHECK_EQ(state(), kStateMutable);
if (memory_list_writer_) {
MinidumpMemoryWriter* stack = thread->Stack();
SnapshotMinidumpMemoryWriter* stack = thread->Stack();
if (stack) {
memory_list_writer_->AddExtraMemory(stack);
}

View File

@ -33,7 +33,7 @@ namespace crashpad {
class MinidumpContextWriter;
class MinidumpMemoryListWriter;
class MinidumpMemoryWriter;
class SnapshotMinidumpMemoryWriter;
class ThreadSnapshot;
//! \brief The writer for a MINIDUMP_THREAD object in a minidump file.
@ -67,8 +67,8 @@ class MinidumpThreadWriter final : public internal::MinidumpWritable {
//! \note Valid in #kStateWritable.
const MINIDUMP_THREAD* MinidumpThread() const;
//! \brief Returns a MinidumpMemoryWriter that will write the memory region
//! corresponding to this objects stack.
//! \brief Returns a SnapshotMinidumpMemoryWriter that will write the memory
//! region corresponding to this objects stack.
//!
//! If the thread does not have a stack, or its stack could not be determined,
//! this will return `nullptr`.
@ -80,7 +80,7 @@ class MinidumpThreadWriter final : public internal::MinidumpWritable {
//! MinidumpMemoryListWriter::AddExtraMemory().
//!
//! \note Valid in any state.
MinidumpMemoryWriter* Stack() const { return stack_.get(); }
SnapshotMinidumpMemoryWriter* Stack() const { return stack_.get(); }
//! \brief Arranges for MINIDUMP_THREAD::Stack to point to the MINIDUMP_MEMORY
//! object to be written by \a stack.
@ -89,7 +89,7 @@ class MinidumpThreadWriter final : public internal::MinidumpWritable {
//! overall tree of internal::MinidumpWritable objects.
//!
//! \note Valid in #kStateMutable.
void SetStack(std::unique_ptr<MinidumpMemoryWriter> stack);
void SetStack(std::unique_ptr<SnapshotMinidumpMemoryWriter> stack);
//! \brief Arranges for MINIDUMP_THREAD::ThreadContext to point to the CPU
//! context to be written by \a context.
@ -130,7 +130,7 @@ class MinidumpThreadWriter final : public internal::MinidumpWritable {
private:
MINIDUMP_THREAD thread_;
std::unique_ptr<MinidumpMemoryWriter> stack_;
std::unique_ptr<SnapshotMinidumpMemoryWriter> stack_;
std::unique_ptr<MinidumpContextWriter> context_;
DISALLOW_COPY_AND_ASSIGN(MinidumpThreadWriter);
@ -161,9 +161,9 @@ class MinidumpThreadListWriter final : public internal::MinidumpStreamWriter {
//! region should be added to as extra memory.
//!
//! Each MINIDUMP_THREAD object can contain a reference to a
//! MinidumpMemoryWriter object that contains a snapshot of its stack memory.
//! In the overall tree of internal::MinidumpWritable objects, these
//! MinidumpMemoryWriter objects are considered children of their
//! SnapshotMinidumpMemoryWriter object that contains a snapshot of its stac
//! memory. In the overall tree of internal::MinidumpWritable objects, these
//! SnapshotMinidumpMemoryWriter objects are considered children of their
//! MINIDUMP_THREAD, and are referenced by a MINIDUMP_MEMORY_DESCRIPTOR
//! contained in the MINIDUMP_THREAD. It is also possible for the same memory
//! regions to have MINIDUMP_MEMORY_DESCRIPTOR objects present in a

View File

@ -22,48 +22,15 @@ namespace test {
TestMinidumpMemoryWriter::TestMinidumpMemoryWriter(uint64_t base_address,
size_t size,
uint8_t value)
: MinidumpMemoryWriter(),
base_address_(base_address),
expected_offset_(-1),
size_(size),
value_(value) {
: SnapshotMinidumpMemoryWriter(&test_snapshot_) {
test_snapshot_.SetAddress(base_address);
test_snapshot_.SetSize(size);
test_snapshot_.SetValue(value);
}
TestMinidumpMemoryWriter::~TestMinidumpMemoryWriter() {
}
uint64_t TestMinidumpMemoryWriter::MemoryRangeBaseAddress() const {
EXPECT_EQ(state(), kStateFrozen);
return base_address_;
}
size_t TestMinidumpMemoryWriter::MemoryRangeSize() const {
EXPECT_GE(state(), kStateFrozen);
return size_;
}
bool TestMinidumpMemoryWriter::WillWriteAtOffsetImpl(FileOffset offset) {
EXPECT_EQ(state(), kStateFrozen);
expected_offset_ = offset;
bool rv = MinidumpMemoryWriter::WillWriteAtOffsetImpl(offset);
EXPECT_TRUE(rv);
return rv;
}
bool TestMinidumpMemoryWriter::WriteObject(FileWriterInterface* file_writer) {
EXPECT_EQ(state(), kStateWritable);
EXPECT_EQ(expected_offset_, file_writer->Seek(0, SEEK_CUR));
bool rv = true;
if (size_ > 0) {
std::string data(size_, value_);
rv = file_writer->Write(&data[0], size_);
EXPECT_TRUE(rv);
}
return rv;
}
void ExpectMinidumpMemoryDescriptor(
const MINIDUMP_MEMORY_DESCRIPTOR* expected,
const MINIDUMP_MEMORY_DESCRIPTOR* observed) {

View File

@ -25,34 +25,23 @@
#include <string>
#include "base/macros.h"
#include "snapshot/test/test_memory_snapshot.h"
#include "util/file/file_writer.h"
namespace crashpad {
namespace test {
//! \brief A MinidumpMemoryWriter implementation used for testing.
//! \brief A SnapshotMinidumpMemoryWriter implementation used for testing.
//!
//! TestMinidumpMemoryWriter objects are created with a fixed base address and
//! size, and will write the same byte (\a value) repeatedly, \a size times.
class TestMinidumpMemoryWriter final : public MinidumpMemoryWriter {
class TestMinidumpMemoryWriter final : public SnapshotMinidumpMemoryWriter {
public:
TestMinidumpMemoryWriter(uint64_t base_address, size_t size, uint8_t value);
~TestMinidumpMemoryWriter();
protected:
// MinidumpMemoryWriter:
uint64_t MemoryRangeBaseAddress() const override;
size_t MemoryRangeSize() const override;
// MinidumpWritable:
bool WillWriteAtOffsetImpl(FileOffset offset) override;
bool WriteObject(FileWriterInterface* file_writer) override;
private:
uint64_t base_address_;
FileOffset expected_offset_;
size_t size_;
uint8_t value_;
TestMemorySnapshot test_snapshot_;
DISALLOW_COPY_AND_ASSIGN(TestMinidumpMemoryWriter);
};