Avoid two-stage initialization of MinidumpParser
remove the Initialize function, move the things that can fail into the
static factory function. The factory function now returns
Expected<Parser> instead of Optional<Parser> so that it can give a
reason why creation failed.
llvm-svn: 354668
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index d2f3b87a..a423a4e 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -23,16 +23,106 @@
using namespace lldb_private;
using namespace minidump;
-llvm::Optional<MinidumpParser>
-MinidumpParser::Create(const lldb::DataBufferSP &data_buf_sp) {
- if (data_buf_sp->GetByteSize() < sizeof(MinidumpHeader)) {
- return llvm::None;
- }
- return MinidumpParser(data_buf_sp);
+static llvm::Error stringError(llvm::StringRef Err) {
+ return llvm::make_error<llvm::StringError>(Err,
+ llvm::inconvertibleErrorCode());
}
-MinidumpParser::MinidumpParser(const lldb::DataBufferSP &data_buf_sp)
- : m_data_sp(data_buf_sp) {}
+llvm::Expected<MinidumpParser>
+MinidumpParser::Create(const lldb::DataBufferSP &data_sp) {
+ if (data_sp->GetByteSize() < sizeof(MinidumpHeader))
+ return stringError("Buffer too small.");
+
+ llvm::ArrayRef<uint8_t> header_data(data_sp->GetBytes(),
+ sizeof(MinidumpHeader));
+ const MinidumpHeader *header = MinidumpHeader::Parse(header_data);
+ if (!header)
+ return stringError("invalid minidump: can't parse the header");
+
+ // A minidump without at least one stream is clearly ill-formed
+ if (header->streams_count == 0)
+ return stringError("invalid minidump: no streams present");
+
+ struct FileRange {
+ uint32_t offset = 0;
+ uint32_t size = 0;
+
+ FileRange(uint32_t offset, uint32_t size) : offset(offset), size(size) {}
+ uint32_t end() const { return offset + size; }
+ };
+
+ const uint32_t file_size = data_sp->GetByteSize();
+
+ // Build a global minidump file map, checking for:
+ // - overlapping streams/data structures
+ // - truncation (streams pointing past the end of file)
+ std::vector<FileRange> minidump_map;
+
+ minidump_map.emplace_back(0, sizeof(MinidumpHeader));
+
+ // Add the directory entries to the file map
+ FileRange directory_range(header->stream_directory_rva,
+ header->streams_count * sizeof(MinidumpDirectory));
+ if (directory_range.end() > file_size)
+ return stringError("invalid minidump: truncated streams directory");
+ minidump_map.push_back(directory_range);
+
+ llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> directory_map;
+
+ // Parse stream directory entries
+ llvm::ArrayRef<uint8_t> directory_data(
+ data_sp->GetBytes() + directory_range.offset, directory_range.size);
+ for (uint32_t i = 0; i < header->streams_count; ++i) {
+ const MinidumpDirectory *directory_entry = nullptr;
+ Status error = consumeObject(directory_data, directory_entry);
+ if (error.Fail())
+ return error.ToError();
+ if (directory_entry->stream_type == 0) {
+ // Ignore dummy streams (technically ill-formed, but a number of
+ // existing minidumps seem to contain such streams)
+ if (directory_entry->location.data_size == 0)
+ continue;
+ return stringError("invalid minidump: bad stream type");
+ }
+ // Update the streams map, checking for duplicate stream types
+ if (!directory_map
+ .insert({directory_entry->stream_type, directory_entry->location})
+ .second)
+ return stringError("invalid minidump: duplicate stream type");
+
+ // Ignore the zero-length streams for layout checks
+ if (directory_entry->location.data_size != 0) {
+ minidump_map.emplace_back(directory_entry->location.rva,
+ directory_entry->location.data_size);
+ }
+ }
+
+ // Sort the file map ranges by start offset
+ llvm::sort(minidump_map.begin(), minidump_map.end(),
+ [](const FileRange &a, const FileRange &b) {
+ return a.offset < b.offset;
+ });
+
+ // Check for overlapping streams/data structures
+ for (size_t i = 1; i < minidump_map.size(); ++i) {
+ const auto &prev_range = minidump_map[i - 1];
+ if (prev_range.end() > minidump_map[i].offset)
+ return stringError("invalid minidump: overlapping streams");
+ }
+
+ // Check for streams past the end of file
+ const auto &last_range = minidump_map.back();
+ if (last_range.end() > file_size)
+ return stringError("invalid minidump: truncated stream");
+
+ return MinidumpParser(std::move(data_sp), std::move(directory_map));
+}
+
+MinidumpParser::MinidumpParser(
+ lldb::DataBufferSP data_sp,
+ llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> directory_map)
+ : m_data_sp(std::move(data_sp)), m_directory_map(std::move(directory_map)) {
+}
llvm::ArrayRef<uint8_t> MinidumpParser::GetData() {
return llvm::ArrayRef<uint8_t>(m_data_sp->GetBytes(),
@@ -563,112 +653,6 @@
return m_regions;
}
-Status MinidumpParser::Initialize() {
- Status error;
-
- lldbassert(m_directory_map.empty());
-
- llvm::ArrayRef<uint8_t> header_data(m_data_sp->GetBytes(),
- sizeof(MinidumpHeader));
- const MinidumpHeader *header = MinidumpHeader::Parse(header_data);
- if (header == nullptr) {
- error.SetErrorString("invalid minidump: can't parse the header");
- return error;
- }
-
- // A minidump without at least one stream is clearly ill-formed
- if (header->streams_count == 0) {
- error.SetErrorString("invalid minidump: no streams present");
- return error;
- }
-
- struct FileRange {
- uint32_t offset = 0;
- uint32_t size = 0;
-
- FileRange(uint32_t offset, uint32_t size) : offset(offset), size(size) {}
- uint32_t end() const { return offset + size; }
- };
-
- const uint32_t file_size = m_data_sp->GetByteSize();
-
- // Build a global minidump file map, checking for:
- // - overlapping streams/data structures
- // - truncation (streams pointing past the end of file)
- std::vector<FileRange> minidump_map;
-
- // Add the minidump header to the file map
- if (sizeof(MinidumpHeader) > file_size) {
- error.SetErrorString("invalid minidump: truncated header");
- return error;
- }
- minidump_map.emplace_back( 0, sizeof(MinidumpHeader) );
-
- // Add the directory entries to the file map
- FileRange directory_range(header->stream_directory_rva,
- header->streams_count *
- sizeof(MinidumpDirectory));
- if (directory_range.end() > file_size) {
- error.SetErrorString("invalid minidump: truncated streams directory");
- return error;
- }
- minidump_map.push_back(directory_range);
-
- // Parse stream directory entries
- llvm::ArrayRef<uint8_t> directory_data(
- m_data_sp->GetBytes() + directory_range.offset, directory_range.size);
- for (uint32_t i = 0; i < header->streams_count; ++i) {
- const MinidumpDirectory *directory_entry = nullptr;
- error = consumeObject(directory_data, directory_entry);
- if (error.Fail())
- return error;
- if (directory_entry->stream_type == 0) {
- // Ignore dummy streams (technically ill-formed, but a number of
- // existing minidumps seem to contain such streams)
- if (directory_entry->location.data_size == 0)
- continue;
- error.SetErrorString("invalid minidump: bad stream type");
- return error;
- }
- // Update the streams map, checking for duplicate stream types
- if (!m_directory_map
- .insert({directory_entry->stream_type, directory_entry->location})
- .second) {
- error.SetErrorString("invalid minidump: duplicate stream type");
- return error;
- }
- // Ignore the zero-length streams for layout checks
- if (directory_entry->location.data_size != 0) {
- minidump_map.emplace_back(directory_entry->location.rva,
- directory_entry->location.data_size);
- }
- }
-
- // Sort the file map ranges by start offset
- llvm::sort(minidump_map.begin(), minidump_map.end(),
- [](const FileRange &a, const FileRange &b) {
- return a.offset < b.offset;
- });
-
- // Check for overlapping streams/data structures
- for (size_t i = 1; i < minidump_map.size(); ++i) {
- const auto &prev_range = minidump_map[i - 1];
- if (prev_range.end() > minidump_map[i].offset) {
- error.SetErrorString("invalid minidump: overlapping streams");
- return error;
- }
- }
-
- // Check for streams past the end of file
- const auto &last_range = minidump_map.back();
- if (last_range.end() > file_size) {
- error.SetErrorString("invalid minidump: truncated stream");
- return error;
- }
-
- return error;
-}
-
#define ENUM_TO_CSTR(ST) case (uint32_t)MinidumpStreamType::ST: return #ST
llvm::StringRef
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.h b/lldb/source/Plugins/Process/minidump/MinidumpParser.h
index fa0d6d6..7c2cf1b 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.h
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.h
@@ -44,7 +44,7 @@
class MinidumpParser {
public:
- static llvm::Optional<MinidumpParser>
+ static llvm::Expected<MinidumpParser>
Create(const lldb::DataBufferSP &data_buf_sp);
llvm::ArrayRef<uint8_t> GetData();
@@ -92,9 +92,6 @@
const MemoryRegionInfos &GetMemoryRegions();
- // Perform consistency checks and initialize internal data structures
- Status Initialize();
-
static llvm::StringRef GetStreamTypeAsString(uint32_t stream_type);
const llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> &
@@ -103,7 +100,9 @@
}
private:
- MinidumpParser(const lldb::DataBufferSP &data_buf_sp);
+ MinidumpParser(
+ lldb::DataBufferSP data_sp,
+ llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> directory_map);
MemoryRegionInfo FindMemoryRegion(lldb::addr_t load_addr) const;
diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
index c170016..1f9dc54 100644
--- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -123,13 +123,8 @@
if (!AllData)
return nullptr;
- auto minidump_parser = MinidumpParser::Create(AllData);
- // check if the parser object is valid
- if (!minidump_parser)
- return nullptr;
-
return std::make_shared<ProcessMinidump>(target_sp, listener_sp, *crash_file,
- minidump_parser.getValue());
+ std::move(AllData));
}
bool ProcessMinidump::CanDebug(lldb::TargetSP target_sp,
@@ -140,9 +135,9 @@
ProcessMinidump::ProcessMinidump(lldb::TargetSP target_sp,
lldb::ListenerSP listener_sp,
const FileSpec &core_file,
- MinidumpParser minidump_parser)
- : Process(target_sp, listener_sp), m_minidump_parser(minidump_parser),
- m_core_file(core_file), m_is_wow64(false) {}
+ DataBufferSP core_data)
+ : Process(target_sp, listener_sp), m_core_file(core_file),
+ m_core_data(std::move(core_data)), m_is_wow64(false) {}
ProcessMinidump::~ProcessMinidump() {
Clear();
@@ -168,12 +163,12 @@
}
Status ProcessMinidump::DoLoadCore() {
- Status error;
+ auto expected_parser = MinidumpParser::Create(m_core_data);
+ if (!expected_parser)
+ return Status(expected_parser.takeError());
+ m_minidump_parser = std::move(*expected_parser);
- // Minidump parser initialization & consistency checks
- error = m_minidump_parser.Initialize();
- if (error.Fail())
- return error;
+ Status error;
// Do we support the minidump's architecture?
ArchSpec arch = GetArchitecture();
@@ -192,11 +187,11 @@
}
GetTarget().SetArchitecture(arch, true /*set_platform*/);
- m_thread_list = m_minidump_parser.GetThreads();
- m_active_exception = m_minidump_parser.GetExceptionStream();
+ m_thread_list = m_minidump_parser->GetThreads();
+ m_active_exception = m_minidump_parser->GetExceptionStream();
ReadModuleList();
- llvm::Optional<lldb::pid_t> pid = m_minidump_parser.GetPid();
+ llvm::Optional<lldb::pid_t> pid = m_minidump_parser->GetPid();
if (!pid) {
error.SetErrorString("failed to parse PID");
return error;
@@ -267,7 +262,7 @@
size_t ProcessMinidump::DoReadMemory(lldb::addr_t addr, void *buf, size_t size,
Status &error) {
- llvm::ArrayRef<uint8_t> mem = m_minidump_parser.GetMemory(addr, size);
+ llvm::ArrayRef<uint8_t> mem = m_minidump_parser->GetMemory(addr, size);
if (mem.empty()) {
error.SetErrorString("could not parse memory info");
return 0;
@@ -279,7 +274,7 @@
ArchSpec ProcessMinidump::GetArchitecture() {
if (!m_is_wow64) {
- return m_minidump_parser.GetArchitecture();
+ return m_minidump_parser->GetArchitecture();
}
llvm::Triple triple;
@@ -291,13 +286,13 @@
Status ProcessMinidump::GetMemoryRegionInfo(lldb::addr_t load_addr,
MemoryRegionInfo &range_info) {
- range_info = m_minidump_parser.GetMemoryRegionInfo(load_addr);
+ range_info = m_minidump_parser->GetMemoryRegionInfo(load_addr);
return Status();
}
Status ProcessMinidump::GetMemoryRegions(
lldb_private::MemoryRegionInfos ®ion_list) {
- region_list = m_minidump_parser.GetMemoryRegions();
+ region_list = m_minidump_parser->GetMemoryRegions();
return Status();
}
@@ -316,9 +311,9 @@
llvm::ArrayRef<uint8_t> context;
if (!m_is_wow64)
- context = m_minidump_parser.GetThreadContext(context_location);
+ context = m_minidump_parser->GetThreadContext(context_location);
else
- context = m_minidump_parser.GetThreadContextWow64(thread);
+ context = m_minidump_parser->GetThreadContextWow64(thread);
lldb::ThreadSP thread_sp(new ThreadMinidump(*this, thread, context));
new_thread_list.AddThread(thread_sp);
@@ -328,11 +323,11 @@
void ProcessMinidump::ReadModuleList() {
std::vector<const MinidumpModule *> filtered_modules =
- m_minidump_parser.GetFilteredModuleList();
+ m_minidump_parser->GetFilteredModuleList();
for (auto module : filtered_modules) {
llvm::Optional<std::string> name =
- m_minidump_parser.GetMinidumpString(module->module_name_rva);
+ m_minidump_parser->GetMinidumpString(module->module_name_rva);
if (!name)
continue;
@@ -353,7 +348,7 @@
m_is_wow64 = true;
}
- const auto uuid = m_minidump_parser.GetModuleUUID(module);
+ const auto uuid = m_minidump_parser->GetModuleUUID(module);
auto file_spec = FileSpec(name.getValue(), GetArchitecture().GetTriple());
FileSystem::Instance().Resolve(file_spec);
ModuleSpec module_spec(file_spec, uuid);
@@ -666,7 +661,7 @@
m_interpreter.GetExecutionContext().GetProcessPtr());
result.SetStatus(eReturnStatusSuccessFinishResult);
Stream &s = result.GetOutputStream();
- MinidumpParser &minidump = process->m_minidump_parser;
+ MinidumpParser &minidump = *process->m_minidump_parser;
if (DumpDirectory()) {
s.Printf("RVA SIZE TYPE MinidumpStreamType\n");
s.Printf("---------- ---------- ---------- --------------------------\n");
diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h
index 4c56a72..6da6a38 100644
--- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h
+++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h
@@ -41,7 +41,7 @@
static const char *GetPluginDescriptionStatic();
ProcessMinidump(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
- const FileSpec &core_file, MinidumpParser minidump_parser);
+ const FileSpec &core_file, lldb::DataBufferSP code_data);
~ProcessMinidump() override;
@@ -92,7 +92,7 @@
return error;
}
- MinidumpParser m_minidump_parser;
+ llvm::Optional<MinidumpParser> m_minidump_parser;
protected:
void Clear();
@@ -106,6 +106,7 @@
private:
FileSpec m_core_file;
+ lldb::DataBufferSP m_core_data;
llvm::ArrayRef<MinidumpThread> m_thread_list;
const MinidumpExceptionStream *m_active_exception;
lldb::CommandObjectSP m_command_sp;