lemo updated this revision to Diff 155080.
Herald added a subscriber: mgorny.
https://reviews.llvm.org/D49202
Files:
source/Plugins/Process/minidump/MinidumpParser.cpp
source/Plugins/Process/minidump/MinidumpParser.h
source/Plugins/Process/minidump/MinidumpTypes.cpp
source/Plugins/Process/minidump/ProcessMinidump.cpp
unittests/Process/minidump/CMakeLists.txt
unittests/Process/minidump/Inputs/bad_duplicate_streams.dmp
unittests/Process/minidump/Inputs/bad_overlapping_streams.dmp
unittests/Process/minidump/MinidumpParserTest.cpp
Index: unittests/Process/minidump/MinidumpParserTest.cpp
===================================================================
--- unittests/Process/minidump/MinidumpParserTest.cpp
+++ unittests/Process/minidump/MinidumpParserTest.cpp
@@ -38,16 +38,32 @@
class MinidumpParserTest : public testing::Test {
public:
- void SetUpData(const char *minidump_filename,
- uint64_t load_size = UINT64_MAX) {
+ void SetUpData(const char *minidump_filename) {
std::string filename = GetInputFilePath(minidump_filename);
- auto BufferPtr = DataBufferLLVM::CreateSliceFromPath(filename, load_size, 0);
+ auto BufferPtr = DataBufferLLVM::CreateSliceFromPath(filename, -1, 0);
+ ASSERT_NE(BufferPtr, nullptr);
+ llvm::Optional<MinidumpParser> optional_parser =
+ MinidumpParser::Create(BufferPtr);
+ ASSERT_TRUE(optional_parser.hasValue());
+ parser.reset(new MinidumpParser(optional_parser.getValue()));
+ ASSERT_GT(parser->GetData().size(), 0UL);
+ auto result = parser->Initialize();
+ ASSERT_TRUE(result.Success()) << result.AsCString();
+ }
+
+ void InvalidMinidump(const char *minidump_filename, uint64_t load_size) {
+ std::string filename = GetInputFilePath(minidump_filename);
+ auto BufferPtr =
+ DataBufferLLVM::CreateSliceFromPath(filename, load_size, 0);
+ ASSERT_NE(BufferPtr, nullptr);
llvm::Optional<MinidumpParser> optional_parser =
MinidumpParser::Create(BufferPtr);
ASSERT_TRUE(optional_parser.hasValue());
parser.reset(new MinidumpParser(optional_parser.getValue()));
ASSERT_GT(parser->GetData().size(), 0UL);
+ auto result = parser->Initialize();
+ ASSERT_TRUE(result.Fail());
}
std::unique_ptr<MinidumpParser> parser;
@@ -68,12 +84,15 @@
EXPECT_EQ(1232UL, context.size());
}
-TEST_F(MinidumpParserTest, GetThreadsTruncatedFile) {
- SetUpData("linux-x86_64.dmp", 200);
- llvm::ArrayRef<MinidumpThread> thread_list;
+TEST_F(MinidumpParserTest, TruncatedMinidumps) {
+ InvalidMinidump("linux-x86_64.dmp", 32);
+ InvalidMinidump("linux-x86_64.dmp", 100);
+ InvalidMinidump("linux-x86_64.dmp", 20 * 1024);
+}
- thread_list = parser->GetThreads();
- ASSERT_EQ(0UL, thread_list.size());
+TEST_F(MinidumpParserTest, IllFormedMinidumps) {
+ InvalidMinidump("bad_duplicate_streams.dmp", -1);
+ InvalidMinidump("bad_overlapping_streams.dmp", -1);
}
TEST_F(MinidumpParserTest, GetArchitecture) {
Index: unittests/Process/minidump/CMakeLists.txt
===================================================================
--- unittests/Process/minidump/CMakeLists.txt
+++ unittests/Process/minidump/CMakeLists.txt
@@ -17,6 +17,8 @@
linux-x86_64.dmp
linux-x86_64_not_crashed.dmp
fizzbuzz_no_heap.dmp
- fizzbuzz_wow64.dmp)
+ fizzbuzz_wow64.dmp
+ bad_duplicate_streams.dmp
+ bad_overlapping_streams.dmp)
add_unittest_inputs(LLDBMinidumpTests "${test_inputs}")
Index: source/Plugins/Process/minidump/ProcessMinidump.cpp
===================================================================
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -105,7 +105,7 @@
if (!DataPtr)
return nullptr;
- assert(DataPtr->GetByteSize() == header_size);
+ lldbassert(DataPtr->GetByteSize() == header_size);
// first, only try to parse the header, beacuse we need to be fast
llvm::ArrayRef<uint8_t> HeaderBytes = DataPtr->GetData();
@@ -164,10 +164,29 @@
Status ProcessMinidump::DoLoadCore() {
Status error;
+ // Minidump parser initialization & consistency checks
+ error = m_minidump_parser.Initialize();
+ if (error.Fail())
+ return error;
+
+ // Do we support the minidump's architecture?
+ ArchSpec arch = GetArchitecture();
+ switch (arch.GetMachine()) {
+ case llvm::Triple::x86:
+ case llvm::Triple::x86_64:
+ // supported
+ break;
+
+ default:
+ error.SetErrorStringWithFormat("unsupported minidump architecture: %s",
+ arch.GetArchitectureName());
+ return error;
+ }
+
m_thread_list = m_minidump_parser.GetThreads();
m_active_exception = m_minidump_parser.GetExceptionStream();
ReadModuleList();
- GetTarget().SetArchitecture(GetArchitecture());
+ GetTarget().SetArchitecture(arch);
llvm::Optional<lldb::pid_t> pid = m_minidump_parser.GetPid();
if (!pid) {
Index: source/Plugins/Process/minidump/MinidumpTypes.cpp
===================================================================
--- source/Plugins/Process/minidump/MinidumpTypes.cpp
+++ source/Plugins/Process/minidump/MinidumpTypes.cpp
@@ -33,9 +33,6 @@
version != MinidumpHeaderConstants::Version)
return nullptr;
- // TODO check for max number of streams ?
- // TODO more sanity checks ?
-
return header;
}
Index: source/Plugins/Process/minidump/MinidumpParser.h
===================================================================
--- source/Plugins/Process/minidump/MinidumpParser.h
+++ source/Plugins/Process/minidump/MinidumpParser.h
@@ -89,14 +89,16 @@
llvm::Optional<MemoryRegionInfo> GetMemoryRegionInfo(lldb::addr_t);
+ // Perform consistency checks and initialize internal data structures
+ Status Initialize();
+
+private:
+ MinidumpParser(const lldb::DataBufferSP &data_buf_sp);
+
private:
lldb::DataBufferSP m_data_sp;
- const MinidumpHeader *m_header;
+ const MinidumpHeader *m_header = nullptr;
llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> m_directory_map;
-
- MinidumpParser(
- const lldb::DataBufferSP &data_buf_sp, const MinidumpHeader *header,
- llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> &&directory_map);
};
} // end namespace minidump
Index: source/Plugins/Process/minidump/MinidumpParser.cpp
===================================================================
--- source/Plugins/Process/minidump/MinidumpParser.cpp
+++ source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -14,10 +14,13 @@
// Other libraries and framework includes
#include "lldb/Target/MemoryRegionInfo.h"
+#include "lldb/Utility/LLDBAssert.h"
// C includes
// C++ includes
+#include <algorithm>
#include <map>
+#include <vector>
using namespace lldb_private;
using namespace minidump;
@@ -27,47 +30,11 @@
if (data_buf_sp->GetByteSize() < sizeof(MinidumpHeader)) {
return llvm::None;
}
-
- llvm::ArrayRef<uint8_t> header_data(data_buf_sp->GetBytes(),
- sizeof(MinidumpHeader));
- const MinidumpHeader *header = MinidumpHeader::Parse(header_data);
-
- if (header == nullptr) {
- return llvm::None;
- }
-
- lldb::offset_t directory_list_offset = header->stream_directory_rva;
- // check if there is enough data for the parsing of the directory list
- if ((directory_list_offset +
- sizeof(MinidumpDirectory) * header->streams_count) >
- data_buf_sp->GetByteSize()) {
- return llvm::None;
- }
-
- const MinidumpDirectory *directory = nullptr;
- Status error;
- llvm::ArrayRef<uint8_t> directory_data(
- data_buf_sp->GetBytes() + directory_list_offset,
- sizeof(MinidumpDirectory) * header->streams_count);
- llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> directory_map;
-
- for (uint32_t i = 0; i < header->streams_count; ++i) {
- error = consumeObject(directory_data, directory);
- if (error.Fail()) {
- return llvm::None;
- }
- directory_map[static_cast<const uint32_t>(directory->stream_type)] =
- directory->location;
- }
-
- return MinidumpParser(data_buf_sp, header, std::move(directory_map));
+ return MinidumpParser(data_buf_sp);
}
-MinidumpParser::MinidumpParser(
- const lldb::DataBufferSP &data_buf_sp, const MinidumpHeader *header,
- llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> &&directory_map)
- : m_data_sp(data_buf_sp), m_header(header), m_directory_map(directory_map) {
-}
+MinidumpParser::MinidumpParser(const lldb::DataBufferSP &data_buf_sp)
+ : m_data_sp(data_buf_sp) {}
llvm::ArrayRef<uint8_t> MinidumpParser::GetData() {
return llvm::ArrayRef<uint8_t>(m_data_sp->GetBytes(),
@@ -481,3 +448,110 @@
// appear truncated.
return info;
}
+
+Status MinidumpParser::Initialize() {
+ Status error;
+
+ lldbassert(m_header == nullptr);
+ lldbassert(m_directory_map.empty());
+
+ llvm::ArrayRef<uint8_t> header_data(m_data_sp->GetBytes(),
+ sizeof(MinidumpHeader));
+ m_header = MinidumpHeader::Parse(header_data);
+ if (m_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 (m_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(m_header->stream_directory_rva,
+ m_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 < m_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
+ std::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;
+}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
